Page MenuHomePhabricator

[mlir] scf::ForOp: Propagate constants in loop body to trigger simplifications

Authored by chelini on Mar 31 2021, 1:56 AM.

Diff Detail

Event Timeline

chelini created this revision.Mar 31 2021, 1:56 AM
chelini requested review of this revision.Mar 31 2021, 1:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2021, 1:56 AM
ftynse requested changes to this revision.Apr 6 2021, 2:50 AM

Would you mind providing a commit description in addition to the one-line summary? A good description explains _why_ this new functionality is useful, rather than what the change does.


Nit: could you put the results of all std::get<N>(it) into named variables? It would make the code more readable.


Nit: could this match .* instead of the specific regexp?


I don't think it's important for this code to be on the next line, so let's just use CHECK and avoid over-constraining the tests.


It looks like this tests for a sequence of canonicalizations. Could we in addition have a test that exercises only the new canonicalization you are adding? This helps bisecting problems.

This revision now requires changes to proceed.Apr 6 2021, 2:50 AM
wsmoses added inline comments.Wed, Apr 21, 7:43 PM

Is there a reason why this has to be constant for this to apply. This looks like LICM to me and otherwise doesn't need the constantop req?

chelini abandoned this revision.Thu, Apr 22, 1:17 AM

This is a wrong approach in using the pattern rewrite see: I will close this.