Instcombine will currently sink identical shuffles though vector binary operations. This is probably generally useful, but can break up the code pattern we use to represent an interleaving load group. This patch reverses that in the InterleaveAccessPass to re-recognise the pattern of shuffles sunk past binary operations and folds them back if an interleave group can be created.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Nice codegen changes! This patch looks good to me. I haven't looked too much at InterleavedAccessPass though, so perhaps wait a day just in case there are more comments.
I haven't looked at this pass much, so just pointed out some minors.
There are tests specifically for this pass in test/Transforms/InterleavedAccess/AArch64. So it would be nice to have minimal IR tests using "opt < %s -interleaved-access". If possible, create a test where a load has both shuffle users and shuffle-of-binop users, so we test a scenario where both of those data structures are populated.
llvm/lib/CodeGen/InterleavedAccessPass.cpp | ||
---|---|---|
300–305 | Should all of these be SmallSetVector? | |
307 | typo: modified | |
314 | auto -> auto * | |
319 | Use auto * here too for consistency. | |
345–346 | for (unsigned i =0, e = Shuffle.size(); i != e; ++i) { | |
355 |
Added InterleavedAccess and addressed other review comments.
llvm/lib/CodeGen/InterleavedAccessPass.cpp | ||
---|---|---|
300–305 | I had to remind myself why this was needed. We need to ensure the BinOpShuffles are only replaced once, even if both operands of the binop are the same interleaving load. |
Thanks for the cleanups and tests. I still have not stepped through all of the transforms made by this pass, but LGTM.
Should all of these be SmallSetVector?