Kernel IR refactoring: part 6 #314
Conversation
| } | ||
|
|
||
| for (Val* output : expr->outputs()) { | ||
| TORCH_CHECK(inKernelIr(output)); | ||
| assertInFusion(output); | ||
| TORCH_CHECK(origin_.insert({output, expr}).second); | ||
| TORCH_CHECK(lowered_origin_.insert({output, expr}).second); |
There was a problem hiding this comment.
nitpick: can we emplace(output, expr) instead of insert({output, expr}). Upstream guys are slowly updating that in our code base: https://github.com/pytorch/pytorch/pull/43223/files
There was a problem hiding this comment.
thanks. the whole emplace thing is borderline silly (as it's technically unsound in this case - emplace is only relevant when the copy is expensive, which is not the case here). so indiscriminately using emplace is almost as bad as overusing std::move() :(
There was a problem hiding this comment.
PS. the upstream changes don't seem to be about insert() vs emplace(), as much as 1. cleaning up an emplace() miss-use (which resulted in the copy operation, ie. practically same as insert()) and 2. working around an old GCC library issue (related to std::unique_ptr not being copyable)
There was a problem hiding this comment.
Ha thanks for the education, I totally missed the unique_ptr in their PR 😢
Splits the origin (definition) links between Fusion IR and Kernel IR. This will allow moving the nodes into different containers (as well as cleaning up parts which are not really needed for the Kernel IR, ex. cloning)
Also fixing
isConstScalar()and a couple of build warnings inkernel_cache.cpp