After D86836, we can define multiple cost values for
different cost models. So here we set CostPerUse to
1 iff RVC is enabled to avoid potential impact on RA.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp | ||
---|---|---|
353 | Stylistically I'd prefer this as MF.getSubtarget<RISCVSubtarget>().hasStdExtC() ? 1 : 0 That makes it a little more obvious it is returning an index and not boolean. | |
llvm/test/CodeGen/RISCV/rvv/fixed-vector-strided-load-store.ll | ||
1 | Drop this line. the test is generated by both update_llc_test_checks.py and update_test_checks.py. Having the comment at the top prevents the opposite script from being able to run on the file. |
This scheme seems rather inflexible. We can get away with it currently since the default behaviour of zero-filling CostPerUse elements out to the maximum list size works in every case, but I could see there being issues in future where you end up needing to be extremely verbose to avoid such behaviour.
Correct me if I am wrong, we shall define CostPerUse=[0, 0] explicitly for RVC registers?
I have one question: Should we set CostPerUse for floating point registers? If so, we may need to unroll the register definitions for F&D manually.
If I understand correctly, that would be enough to address @jrtc27's comment, no?
Overall LGTM. Maybe just clarify and address that previous point?
No, I would rather not do that. My concern is that if we ever want more than one dimension, much like HwMode, we will have to expand out the 2^N possibilities for every register, with most entries being duplicates. That would only be solved if you could, say, choose the index based on what register is being asked about, not just the MachineFunction, as then you could have different meanings for CostPerUse indices between different sets of registers.
llvm/lib/Target/RISCV/RISCVRegisterInfo.td | ||
---|---|---|
181 | We discussed this at the RISC-V sync up this morning. I don't believe this should be part of this patch. Relaxing the restriction on non-RVC as you did in your original revision is non-controversial. Applying a new restriction on floating point with RVC may require performance measurements to be done. |
llvm/lib/Target/RISCV/RISCVRegisterInfo.td | ||
---|---|---|
181 | Thanks. I will revert this patch to the original version and create another one for floating point registers after running SPEC FP (if it's profitable). |
LGTM
llvm/lib/Target/RISCV/RISCVRegisterInfo.td | ||
---|---|---|
181 | Code size numbers would be good too. |
Stylistically I'd prefer this as MF.getSubtarget<RISCVSubtarget>().hasStdExtC() ? 1 : 0
That makes it a little more obvious it is returning an index and not boolean.