This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Enable machine copy propagation for copy-like instructions
ClosedPublic

Authored by pcwang-thead on Feb 21 2023, 10:18 PM.

Details

Summary

Like what has been done in AArch64 (D125335).

We enable this under -O2 to show the codegen diffs here but we
may only do this under -O3 like AArch64.

There are two cases that we may produce these eliminable copies:

  1. ISel of FrameIndex. Like rvv/fixed-vectors-calling-conv.ll.
  2. Tail duplication. Like select-optimize-multiple.ll.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2023, 10:18 PM
pcwang-thead requested review of this revision.Feb 21 2023, 10:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2023, 10:18 PM
craig.topper added inline comments.Feb 22 2023, 10:13 AM
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
355

Can this be before the two Expand passes? Especially the one that says "last possible moment"?

Insert MachineCopyPropagation pass before RISCVExpandAtomicPseudo.

pcwang-thead marked an inline comment as done.Feb 22 2023, 8:28 PM
pcwang-thead added inline comments.
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
355

How about between the two Expand passes? we may produce MVs when expanding PseudoCCMOVGPR.

craig.topper added inline comments.Feb 22 2023, 8:33 PM
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
355

Would optimizing them break the macrofusion that PseudoCCMOVGPR represents?

pcwang-thead marked 2 inline comments as done.Feb 22 2023, 10:27 PM
pcwang-thead added inline comments.
llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
355

After thinking, I think MVs produced by PseudoCCMOVGPR won't be eliminatable.

dst = falsev
branch
  |   \
  |   TrueBB: 
  |     dst = truev
  |   /
MergeBB:
  usage of dst

For these kinds of CFGs, truev could be propagated only if we do tail duplication and CFG becomes like below, but we won't run tail duplication again after expanding.

dst = falsev
branch
  |         \
MergeBB   TrueBB: 
  |         dst = truev
  |         duplicated MergeBB
  |         /
Successor of MergeBB

So I think it has no impact on the macrofusion.


Another reason why we inserted it after expanding pseudos is because some internal pseudos in our downstream will produce MVs.

pcwang-thead marked an inline comment as done.Mar 2 2023, 6:33 PM

Gentle ping.

Is this expensive compile time wise? Why did AArch64 make it -O3 only?

Is this expensive compile time wise? Why did AArch64 make it -O3 only?

I haven't evaluated the impact on compile time, there is about 0.16% increase from AArch64's result. And I think it won't be too expensive since there are already two runs of Machine Copy Propagation Pass in the pipeline.
The reason why AArch64 made it -O3 only is because the gains is not very large compared with the increase of compile time, may we be more aggressive since the gains seem to be larger for RISCV?

This revision is now accepted and ready to land.Mar 3 2023, 10:28 AM
This revision was landed with ongoing or failed builds.Mar 7 2023, 1:54 AM
This revision was automatically updated to reflect the committed changes.