Adding data copy between host and device #323
Conversation
| } | ||
|
|
||
| SingletonModeFormat::SingletonModeFormat(bool isFull, bool isOrdered, | ||
| bool isUnique, long long allocSize) : |
There was a problem hiding this comment.
Shouldn't this constructor also be modified to take in isZeroless as an argument?
There was a problem hiding this comment.
Oh, you are right. Thank you. By the way, it is wired because I modified the code as you suggested about a week ago, but the pull request does not seem to reflect it.
src/lower/iterator.cpp
Outdated
| @@ -191,6 +191,12 @@ bool Iterator::isCompact() const { | |||
| return getMode().defined() && getMode().getModeFormat().isCompact(); | |||
| } | |||
|
|
|||
| bool Iterator::isZeroless() const { | |||
| taco_iassert(defined()); | |||
| if (isDimensionIterator()) return true; | |||
There was a problem hiding this comment.
Should return false for dimension iterators I think?
There was a problem hiding this comment.
Oh, yes. it should return false for dimension iterators. Thank you for correcting me.
|
Could you accept the feature for cudaMalloc and cudaMemcpy, or give me a comment? |
hochawa
changed the title
src/codegen/codegen.h
Outdated
| @@ -1,6 +1,10 @@ | |||
| #ifndef TACO_CODEGEN_H | |||
| #define TACO_CODEGEN_H | |||
|
|
|||
| #define PRINT_FUNC 0 | |||
| #define PRINT_MEM_HOST_TO_DEV 1 | |||
| #define PRINT_MEM_DEV_TO_HOST 2 | |||
There was a problem hiding this comment.
Should replace these with an enum.
There was a problem hiding this comment.
Oh, I see. I will replace them with an enum.
| @@ -312,7 +313,7 @@ class CodeGen_CUDA::DeviceFunctionCollector : public IRVisitor { | |||
| taco_iassert(var) << "Outputs must be vars in codegen"; | |||
| taco_iassert(scopeMap.count(var) == 0) << | |||
| "Duplicate output found in codegen"; | |||
|
|
|||
| output_tensor = var->name; // Isn't there only one output? | |||
There was a problem hiding this comment.
The code generator was designed with the thought that we might eventually support kernels with multiple outputs, though that's not something we've actually fully implemented. It's probably fine to assume for now that there's only one output, though you should probably add an assertion to check for that (e.g., taco_iassert(outputs.size() == 1)) .
There was a problem hiding this comment.
I see. Thanks for explanation. I know understand why the code looks like that. I will add an assertion to check there is only one output.
There was a problem hiding this comment.
Thanks for your time to take a look at the parts I modified!
|
I've added a couple more comments above. On the whole the changes look good though; I'll merge them into master after you've addressed those comments. |
Could you give me comments or accept the pull request?