[RISCV] Add a pass to merge moving parameter registers instructions for Zcmp

[VincentWu]

Summary

This patch adds a pass to generate cm.mvsa01 & cm.mva01s.

RISCVMoveOptimizer.cpp which combines two mv inst into one cm.mva01s or cm.mva01s.

[kito-cheng]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1296–1305 ↗

(On Diff #521517)

This change seems right, but that should be a separated patch :)

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1296–1305 ↗

(On Diff #521517)

Do we ever create such instructions?

[VincentWu]

address comments

[craig.topper]

llvm/lib/Target/RISCV/RISCVMoveOptimizer.cpp

62 ↗

(On Diff #521878)

const DestSourcePair &RegPair

67 ↗

(On Diff #521878)

Join the two ifs with &&

68 ↗

(On Diff #521878)

SR07RegClass.contains(Source) should work I think

74 ↗

(On Diff #521878)

const DestSourcePair &RegPair

80 ↗

(On Diff #521878)

SR07RegClass.contains(Dest)

80 ↗

(On Diff #521878)

Join the two ifs with &&

134 ↗

(On Diff #521878)

Can you pass the DestSourcePair from the caller?

173 ↗

(On Diff #521878)

Why are the available checks different between the two cases? Aren't we always moving the paired copy earlier in the basic block to the location of the first copy. So shouldn't the register checks be the same?

192 ↗

(On Diff #521878)

There is no reason to shorten Move to Mov in this function name. And function names should typically start with a verb. optimizeBlock is probably a better name

229 ↗

(On Diff #521878)

Use Fn.getSubtarget<RISCVSubtarget>()

230 ↗

(On Diff #521878)

Drop curly braces

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

352

Should we disable for -O0?

[craig.topper]

llvm/test/CodeGen/RISCV/cm_mvas_mvsa.ll

14

Shouldn't we have more tests to cover cases where the copies aren't next to each other and cases where registers are modified/used in between?

[VincentWu]

address comments

[craig.topper]

llvm/lib/Target/RISCV/RISCVMoveOptimizer.cpp

31 ↗

(On Diff #526402)

Subtarget doesn't need to be a member of the class. It's only used in runOnMachineFunction

110 ↗

(On Diff #526402)

Put ARegInFirstPair == RISCV::X10 in a variable instead of repeating it 4 times.

162 ↗

(On Diff #526402)

Why don't we need to check UsedRegUnits.available(DestReg) like we do for CM_MVSA01?

168 ↗

(On Diff #526402)

Why don't we need to we check ModifiedRegUnits.available(DestReg) like we do for RISCV::CM_MVA01S?

187 ↗

(On Diff #526402)

This function name is just a shortened form of the pass name. It should be a verb phrase as documented here https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

[VincentWu]

address comments

[craig.topper]

llvm/lib/Target/RISCV/RISCVMoveOptimizer.cpp

181 ↗

(On Diff #528303)

Function names should start with a lower case letter

203 ↗

(On Diff #528303)

could -> can

231 ↗

(On Diff #528303)

Drop the curly braces

[VincentWu]

address comment

[craig.topper]

llvm/lib/Target/RISCV/RISCVMoveOptimizer.cpp

9 ↗

(On Diff #530424)

This description should mention Zcmp

223 ↗

(On Diff #530424)

This static_cast isn't necessary. RISCVSubtarget has an implementation of getInstrInfo that returns RISCVInstrInfo *

[craig.topper]

llvm/lib/Target/RISCV/RISCVMoveOptimizer.cpp

19 ↗

(On Diff #530424)

We're inconsistently using "move optimizer" and "move merging". Can we pick one?

[VincentWu]

address comments

[craig.topper]

LGTM

[VincentWu]

rebase