[RISCV][NFC] Replace the pseudos for instructions that depend on lmul with variants that encode the SEW into the name

[nitinjohnraj]

Summary

• vcompress
• vrgather.vv/vrgatherei16.vv (vrgather.vi/vx are fine)
• vdiv/vdivu/vfdiv
• vsqr

These instructions depend on vl, so we will create pseudos for these instructions that depend on SEW, since VLUpperBound = LMUL * VLEN / SEW. This will, in the future, help us reason about them more finely in the scheduler model.

These instructions also depend on vl, and will be dealt with in the next revision:

• strided and indexed load/stores.
• segment load/stores
• v(w)(f)red*

[michaelmaitland]

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

1919

If we're going to use suffix, should we use it in WriteVCompressV_MX_E and ReadVCompressV_MX_E too?

2041–2061

WriteVGatherV_MX_E and ReadVGatherV_MX_E dont depend on sew from EEWList. Can we pull out the loop over sews from SchedSEWSet<mx>.val to simplify?

2336

Use suffix in WriteVFSqrtV_MX and ReadVFSqrtV_MX. Also need to add _E to WriteVFSqrtV_MX and ReadVFSqrtV_MX

2353

Where did you land on making this SEW aware?

2469–2470

nit: define sews closer to where you use it?

2470

Why did we change ReadVGatherV to ReadVGatherI?

[michaelmaitland]

llvm/lib/Target/RISCV/RISCVScheduleV.td

24

Why don't we make this case SEW aware?

46

Why don't we make this case SEW aware?

50

Why don't we make this case SEW aware?

61

Why don't we make this case SEW aware?

[frasercrmck]

Just a glancing review but, to me, "expand pseudos" has a more immediate connotation of the many pseudo expansion passes (e.g., RISCVExpandPseudoInsts) than "expanding the set of pseudos" as you seem to intend it.

[nitinjohnraj]

In D144925#4157772, @frasercrmck wrote:

Just a glancing review but, to me, "expand pseudos" has a more immediate connotation of the many pseudo expansion passes (e.g., RISCVExpandPseudoInsts) than "expanding the set of pseudos" as you seem to intend it.

Thanks for the comment, that makes sense. :)

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

1919

Good point, yeah.

2041–2061

Thanks for the review! We can certainly do that, but does it really simplify the code? It's a nested loop in either case, so I'm not clear which version performs better/looks simpler.

2336

Thanks, will do!

2469–2470

Appreciated, I agree with that nit, seems better for clarity.

2470

After discussion with @craig.topper, we did this because otherwise we'd need to maintain both SEW-aware and SEW-unaware versions of ReadVGatherV:

1. The SEW-aware version on line 2481 for the _VV binary, which is SEW-aware itself
2. The SEW-unaware version for the _VI binary, which is not SEW aware

This change helps us generate only the SEW-aware ReadVGatherV SchedReads. I'd be glad to talk about this more if you think we should approach this differently though.

2477–2478

I feel particularly uneasy about this change. I could leave it as is, or I could create ReadVGatherX_MX with no SEW in RISCVScheduleV.td.

llvm/lib/Target/RISCV/RISCVScheduleV.td

24

If I remember correctly, that was causing some other files to complain and I put this in as

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

2470

We should move this part to a separate patch. vrgather.vi, vrgather.vx, vrgather.vv are all sort of different.

Let's consider a uarch that splits a LMUL>1 into multiple subvectors acrosss multiple uops:
vrgather.vi knows at decode time which subvector of the full source is needed for the destination subvectors
vrgather.vx doesn't know until the scalar instruction executes which subvector of the full source is needed. But only one subvector will be needed.
vrgather.vv every subvector of the destination can be dependent on multiple subvectors of the source.

[michaelmaitland]

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

2041–2061

You are right. Good as you have it :)

llvm/lib/Target/RISCV/RISCVScheduleV.td

24

I think this should be sew aware here and below and we need to fix the other files that start complaining.

[nitinjohnraj]

Made the SchedReads and SchedWrites sew-aware

[craig.topper]

A lot of the content from your original patch is no longer showing in this review. For example, SchedSEWSet is no longer created in this diff.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

2353

I think the answer is no.

[nitinjohnraj]

Created a diff from a commit range this time

[michaelmaitland]

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

2336

defvar WriteVFSqrtV_MX_E and defvar WriteVFSqrtV_MX_E

2353

If this does not need to be SEW aware, should we remove the TODO?

[nitinjohnraj]

Diff updated to add another commit that was missed last time (Sorry, never used commit ranges with arcanist before.)

[nitinjohnraj]

Refactoring and removing comments

[nitinjohnraj]

llvm/lib/Target/RISCV/RISCVScheduleV.td

50

Did we want to make UpperBound SEW-aware, or handle it in a different way? IIRC we discussed using it as a defined variable?

71

Same as line 62

80

Same as line 62

90

Same as line 62

[michaelmaitland]

llvm/lib/Target/RISCV/RISCVScheduleV.td

50

I think the best solution for now is to use the largest LMUL and the largest SEW for that given LMUL as the UpperBound SchedWrite. This assumes that the largest LMUL and SEW has the largest latency and uses the most resource cycles. This assumption may not always true, but I think is a good starting point.

[craig.topper]

llvm/lib/Target/RISCV/RISCVScheduleV.td

50

If the worst case is the number of elements rather than the size of elements you'd want to use smallest sew.

[nitinjohnraj]

Attempting to fix the diff

[michaelmaitland]

LGTM. As we discussed offline, today UpperBound refers to the largest LMUL supported. This should be changed in a future patch. I propose that it should be removed from MxList and its meaning should be tied the SchedWrite with the worst case behaviour. Now that we are supporting SchedWrite by LMUL + SEW or just LMUL depending on the instruction, the specific scheduler model can decide which LMUL or LMUL + SEW corresponds to the worst case behaviour. This means no assumption is made on whether the Largest LMUL or smallest SEW behaves the worst.