Kernel IR refactoring: part 4#274
Conversation
This reverts commit fc09c1b5a7240701da093406753908eba6f41e1d.
| const std::vector<bool> contiguity_; | ||
| }; | ||
|
|
||
| class TORCH_CUDA_API TensorView : public Val { |
There was a problem hiding this comment.
contrast the lowered kir::TensorView with the fuser::TensorView definition
| const auto in1 = lowerOperand(top->in1(), top->out()); | ||
| const auto in2 = lowerOperand(top->in2(), top->out()); | ||
| const auto in3 = lowerOperand(top->in3(), top->out()); | ||
| const auto out = lowerOutput(top); |
| std::vector<Statement*> inds; | ||
| for (auto* ind : ti->indices()) | ||
| inds.push_back(mutateAsVal(ind)); | ||
|
|
||
| bool changed = false; | ||
| for (decltype(inds.size()) i{0}; i < inds.size(); i++) { | ||
| TORCH_INTERNAL_ASSERT(inds[i]->isVal() && inds[i]->asVal()->isAnInt()); | ||
| if (!inds[i]->sameAs(ti->index(i))) | ||
| changed = true; | ||
| } | ||
|
|
||
| if (!changed) | ||
| return ti; | ||
|
|
||
| std::vector<Val*> valInds(inds.size(), nullptr); | ||
| for (decltype(inds.size()) i{0}; i < inds.size(); i++) | ||
| valInds[i] = inds[i]->asVal(); | ||
|
|
||
| Val* mutated_val = new kir::TensorIndex(ti->view(), valInds); | ||
| registerMutation(ti, mutated_val); | ||
| return mutated_val; |
There was a problem hiding this comment.
Doesn't seem to be needed. We may be able to eliminate the need for OptOutMutator completely after we refactor the lowering code.
There was a problem hiding this comment.
Why are only the bodies of the overloads for the kernel IR nodes removed? Wouldn't the class become inconsistent? Wouldn't it make more sense to remove the class entirely if it's not needed?
There was a problem hiding this comment.
That would be the end goal, although I believe that removing the class completely would require additional changes.
I've been chipping away slowly, to get to the current state where we can have some confidence that it can be removed (when I started this was not as clear)
I opened #276 to track this.
csarofeen
left a comment
There was a problem hiding this comment.
Looks like a good step in the right direction.
|
|
||
| // Open a new inner most for loop | ||
| kir::ForLoop* openFor(Expr* scope, IterDomain* id) { | ||
| const auto kir_id = new kir::IterDomain(id); |
| if (dim->isReduction()) | ||
| continue; | ||
| ids.push_back(dim); | ||
| ids.push_back(new kir::IterDomain(dim)); |
This iteration completes the definition of all the nodes which are present in the Kernel IR. In this PR, they are kept in sync with the original definitions as much as possible, but the goal is to specialize the Kernel IR nodes to fit their lowered roles, for example:
kir::TensorView->kir::Array?kir::IterDomain-> ??? rangekir::TensorDomain-> ??? dimensionThese are not plan of record, but just to illustrate that we'll have the opportunity to define simpler and more specific abstractions in the Kernel IR.
Note that the Fusion IR / Kernel IR split is still not completed. Some Kernel IRs may still point to Fusion IR nodes, although this is becoming more limited to a few cases, which will be the subject of the next iteration.