[RISCV] Add new pass to transform undef to pseudo for vector values.

[BeMg]

Summary

RISC-V vector instruction has register overlapping constraint for certain
instructions, and will cause illegal instruction trap if violated, we use
early clobber to model this constraint, but it can't prevent register allocator
allocated same or overlapped if the input register is undef value, so convert
IMPLICIT_DEF to temporary pseudo could prevent that happen, it's not best way
to resolve this. Ideally we should model the constraint right, but before we
model the constraint right, it's the approach to prevent that happen.

See also: https://github.com/llvm/llvm-project/issues/50157

[BeMg]

Update testcase

[craig.topper]

pseudo is misspelled in the title

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

2

pesudo -> pseudo

11

"pesudo" -> "pseudo" in two places

19

pesudo -> pseudo

19

latter -> later

246

Pesudo -> Pseudo

247

regitser -> register

289

Candidata -> Candidate?

llvm/test/CodeGen/RISCV/regalloc-last-chance-recoloring-failure.ll

29

Are we treating insert_subreg for segment load tuples the same as inserting a small LMUL into a wider LMUL?

[craig.topper]

llvm/test/CodeGen/RISCV/rvv/subregister-undef-early-clobber-vrm4.mir

118 ↗

(On Diff #478437)

Why is there an PseudoRVVInitUnde pseudo in the IR before the pass runs?

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

74

You can pass Register by value

106

origin -> original

111

Can't we do something like

VRRegClass.hasSubClassEq(MRI->getRegClass(R)) ||
VRM2RegClass.hasSubClassEq(MRI->getRegClass(R)) ||
VRM4RegClass.hasSubClassEq(MRI->getRegClass(R)) ||
VRM8RegClass.hasSubClassEq(MRI->getRegClass(R))

why do we need to use getVRLargestSuperClass?

[BeMg]

1. Fix spell
2. Update isVectorRegClass

[BeMg]

1. Rplace PseudoRVVInitUndef with VLE in MIR test
2. Move MIR test into precommit

[BeMg]

llvm/test/CodeGen/RISCV/regalloc-last-chance-recoloring-failure.ll

29

This pass doesn't consider segment load as instruction that assign sub-register.

The following Insert_subreg work like put %5:vrm2 into %6:vrm4

%1:vrm4 = IMPLICIT_DEF
%5:vrm2 = PseudoVLE32_V_M2 killed %4, 0, 5 /* e32 */
%6:vrm4 = INSERT_SUBREG %1, %5, %subreg.sub_vrm2_0

Do we should treat vloxseg2ei32 as INSERT_SUBREG in this patch?

[BeMg]

Ping

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

172

Why passing Insts by value? That will make a copy but it doesn't look like we need a copy.

182

Why passing Insts by value? That will make a copy but it doesn't look like we need a copy.

183

unsigned -> Register

197

unsigned -> Register

197

Why passing Insts by value? That will make a copy but it doesn't look like we need a copy.

200

unsigned -> Register

299

unsigned -> Register

300

unsigned -> Register

313

createVirtualRegister returns Register not unsigned

317

createVirtualRegister returns Register not unsigned

llvm/test/CodeGen/RISCV/regalloc-last-chance-recoloring-failure.ll

29

Nevermind, I didn't realize this test had so many undef and poison operands. I suspect llvm-reduce or bugpoint. I dislike tests with undef/poison operands. It makes things very fragile. It would be legal for DAG combine to delete a large portion of this test.

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

279

"the " is unnecessary in this sentence

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

28

"is be occupied" -> "is occupied"

[craig.topper]

Is it possible to insert the PseudoRVVInitUndef instructions after we've left SSA and the LiveIntervals have been built. Would that make it easier to find the undef lanes?

[BeMg]

1. unsigned -> Register
2. use const std::vector<MachineInstr *> & instead of call by value

[BeMg]

rebase

[BeMg]

Reuse DetectDeadLanes pass info for undefined subregister

[BeMg]

Move DetectDeadLanes pass change into another patch

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

202

Isn't this calculating for the entire function? But handleSubReg is called for individual instructions. So we'll be recomputing information right?

[arsenm]

This could certainly use some new MIR tests. I didn't look super closely but I'm not sure you're correctly handling undef vs. not-undef subreg defs

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

54

DenseMap

91–101

I don't understand the point of this function, getSuperClasses is already sorted by largest. You can just take the first?

106

Could this use the new getPhysRegBaseClass?

108

Don't call getRegClass for each use

142

Reg.isVirtual()

201

unique_ptr

[BeMg]

1. Use unique_ptr for VRegInfo
2. Only run once DetectDeadLanes for each function
3. Remove Seen and PHINodeLaneBitRecord
4. Only call getRegClass once

[BeMg]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

91–101

When this function take VRNoV0RegClassID as input. the getSuperClasses will return as following order.

AnyRegRegClassID 1
VMRegClassID 20
VRRegClassID 21 <- stop here
...

This patch only want those four RegClass as result.

106

Could this hook use for virtual register? It seem only for physical register but this pass run before register allocation.

202

Yes, you're right. we don't need recompute this info for each instruction. It call only once now.

[BeMg]

Add more subregister testcase

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

40

Are there any maps in the code anymore?

llvm/test/CodeGen/RISCV/O3-pipeline.ll

110

If I understand correctly, we're effectively running DetectDeadLanes inside of RISCV init undef pass and then running the real DetectDeadLanes pass which won't do anything because we already did it?

[craig.topper]

llvm/test/CodeGen/RISCV/O3-pipeline.ll

110

Can we run DetectDeadLanes, then run our pass and just use the portion of the DetectDeadLanes that computes the Lane Masks in our pass?

[BeMg]

1. Move init undef pass after DetectDeadLanes
2. Remove <map>

[craig.topper]

llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp

282

Should this be std::unique_ptr<VRegInfo[]> since it points to an array?

[BeMg]

Update std::unique_ptr<VRegInfo> with std::unique_ptr<VRegInfo[]>

[BeMg]

rebase

[BeMg]

rebase

[BeMg]

rebaseY

[BeMg]

Use D141993 as DetectDeadLane user interface