This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add SEW to RISCVInversePseudoTable
ClosedPublic

Authored by michaelmaitland on Jun 29 2023, 12:46 PM.

Details

Summary

Now that scheduler resources are split by SEW for some instructions,
add the ability to map (BaseInstr, LMUL, SEW) -> Pseudo. For
BaseInstrs that are not split by SEW, 0 is the default key.

This keeps the number of Pseudos the same and does not change
the size of RISCVInversePseudoTable. This table is only imported
when llvm-mca is built, so if you don't build llvm-mca, the size of
PseudoTables does not change.

This change will be used in a future patch by llvm-mca to determine
the schedule class based off of SEW data, now that the scheduler
now accounts for SEW for some instructions.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 12:46 PM
michaelmaitland requested review of this revision.Jun 29 2023, 12:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 12:46 PM
craig.topper added inline comments.Jun 29 2023, 8:08 PM
llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
34

I dont' think this changes the table size. There was an 8 bit hole here.

llvm/lib/Target/RISCV/RISCVInstrFormats.td
222 ↗(On Diff #535958)

nit: bits<2> not bit<2>

224 ↗(On Diff #535958)

Can this be in the RISCVVPseudo class?

myhsu added inline comments.Jun 29 2023, 9:52 PM
llvm/lib/Target/RISCV/RISCVInstrFormats.td
222 ↗(On Diff #535958)

making the field smaller: using something similar to MxSet might help. For instance:

let SEW = MxSet2<eew>.m in ...
michaelmaitland marked 4 inline comments as done.

Represent SEW in bits<3>

llvm/lib/Target/RISCV/RISCVInstrFormats.td
224 ↗(On Diff #535958)

It did, but it feels slightly out of place since VLMul exists in RVInst. I wonder if VLMul should be moved to RISCVVPseudo in a separate patch.

michaelmaitland edited the summary of this revision. (Show Details)Jun 30 2023, 8:33 AM
craig.topper added inline comments.Jun 30 2023, 9:03 AM
llvm/lib/Target/RISCV/RISCVInstrFormats.td
224 ↗(On Diff #535958)

VLMul has to be there because it is part of TSFlags.

llvm/lib/Target/RISCV/RISCVInstrFormats.td
224 ↗(On Diff #535958)

I understand now. I've moved SEW to RISCVVPseudo.

myhsu added a comment.Jul 6 2023, 3:07 PM

this patch LGTM, @craig.topper do you have any other comments?

michaelmaitland marked an inline comment as done.Jul 6 2023, 3:54 PM
craig.topper added inline comments.Jul 6 2023, 4:38 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
512

Why do we need to compress it 3 bits in tablegen, but we use 8 bits in RISCVCustomBehaviour.cpp?

michaelmaitland added inline comments.Jul 6 2023, 4:50 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
512

I can revert it to 8. I made it 3 because it looked like we were trying to pack tightly in this class. 3 bits ID as tight as you can get to represent this encoding. We can hold off on making it 3 until we’d like to add a field to the inverse table and we need the space. Let me know what you think.

craig.topper added inline comments.Jul 7 2023, 3:59 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
512

I think I'd prefer to use raw SEW rather than new format until we really need it.

michaelmaitland marked 2 inline comments as done.

Use 8bit sew

This revision is now accepted and ready to land.Jul 13 2023, 8:17 PM
craig.topper added inline comments.Jul 13 2023, 8:19 PM
llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
104

Does passing 0 here mean it can only find pseudos where SEW is 0?

llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp
104

Yes, but this change only deals with LMUL specific pseudos, not LMUL and SEW specific pseudos. Please see the child patch to see how we deal with SEW specific pseudos.

This revision was automatically updated to reflect the committed changes.