This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Enable CGP to sink splat operands of Add/Sub/Mul/Shl/LShr/AShr
ClosedPublic

Authored by craig.topper on Sep 7 2021, 2:38 PM.

Details

Summary

LICM may have pulled out a splat, but with .vx instructions we
can fold it into an operation.

This patch enables CGP to reverse the LICM transform and move the
splat back into the loop.

I've started with the commutable integer operations and shifts, but we can
extend this with more operations in future patches.

Diff Detail

Event Timeline

craig.topper created this revision.Sep 7 2021, 2:38 PM
craig.topper requested review of this revision.Sep 7 2021, 2:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 7 2021, 2:38 PM
Herald added a subscriber: MaskRay. · View Herald Transcript

Thanks for this patch! I've seen this problem locally but I hadn't found the time to investigate this; I didn't know there was already a handy utility to help sink these splats.

The code and principle definitely LGTM/SGTM, just I had questions about how else we may be able to go about it.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1053

Do you think the IR pattern matching library would be appropriate to use here?

While I'm on that subject, do you think it's worth separating this logic out into a new pattern matcher (e.g. m_VectorSplat) if there isn't one already?

llvm/test/CodeGen/RISCV/rvv/sink-splat-operands.ll
1

Might be worth pre-committing these tests just to help this patch shine.

Use PatternMatch.
Base the implementation on ARM instead of AArch64. This should give better options for future additions.

craig.topper added inline comments.Sep 8 2021, 10:58 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1053

I've switched to basing this on ARM's implementation which uses PatternMatch. I dropped the code for looking through a bitcast though. I'm uncomfortable using an m_VectorSplat because of the Ops.push_back(&Op->getOperandUse(0)). We need to know exactly what instructions we matched for that to be correct.

frasercrmck added inline comments.Sep 9 2021, 1:26 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
1053

Yeah I think this is an improvement, thanks. Fair enough on m_VectorSplat; I hadn't fully understood exactly what we're supposed to return in this vector.

1065

Is it worth performing this check before the match? I'm assuming this early-exits more often and more cheaply than the match.

Move isSinker call earlier.

Add support shl/lshr/ashr as well.

craig.topper retitled this revision from [RISCV] Enable CGP to sink splat operands of Add/Sub/Mul. to [RISCV] Enable CGP to sink splat operands of Add/Sub/Mul/Shl/LShr/AShr.Sep 9 2021, 12:46 PM
craig.topper edited the summary of this revision. (Show Details)
frasercrmck accepted this revision.Sep 10 2021, 2:21 AM

LGTM, thanks. I suppose we'll need to look into either better placement or hoisting of vsetvli ahead/out of the loop.

This revision is now accepted and ready to land.Sep 10 2021, 2:21 AM
This revision was landed with ongoing or failed builds.Sep 10 2021, 9:13 AM
This revision was automatically updated to reflect the committed changes.