Page MenuHomePhabricator

[NARY-REASSOCIATE] Simplify traversal logic by post deleting dead instructions
ClosedPublic

Authored by ebrevnov on Sep 25 2020, 1:42 AM.

Details

Summary

Currently we delete optimized instructions as we go. That has several negative consequences. First it complicates traversal logic itself. Second if newly generated instruction has been deleted the traversal is repeated from scratch.

But real motivation for the change is upcoming change with support for min/max reassociation. Here we employ SCEV expander to generate code. As a result newly generated instructions may be inserted not right before original instruction (because SCEV may do hoisting) and there is no way to know 'next' instruction.

Diff Detail

Event Timeline

ebrevnov created this revision.Sep 25 2020, 1:42 AM
ebrevnov requested review of this revision.Sep 25 2020, 1:42 AM
mkazantsev added inline comments.Oct 29 2020, 9:24 PM
llvm/lib/Transforms/Scalar/NaryReassociate.cpp
244

This is unused outside if, please move def closer to use.

276

nit: { } not needed.

llvm/test/Transforms/NaryReassociate/pr24301.ll
5

Not clear what has changed with your patch. Could you please auto-update the test, commit it as NFC and rebase on top of it to show the difference?

ebrevnov added inline comments.Dec 2 2020, 2:26 AM
llvm/lib/Transforms/Scalar/NaryReassociate.cpp
244

Actually, it is used in else part.

276

Ok

mkazantsev accepted this revision.Dec 3 2020, 4:45 AM
This revision is now accepted and ready to land.Dec 3 2020, 4:45 AM