Make an exploiting to use mtvsrdd instruction when save called-saved GPR registers to VSR registers.
Because mtvsrdd can save two GPRs together at one time and make the space/time more efficient.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | ||
---|---|---|
2134 | The comment are comparing to old implementation? This is unnecessary? | |
2142 | No need to check hasP9Vector() here, we already early exit above when !Subtarget.hasP9Vector() in line 2107. | |
2152 | Similar to above, we only enable spill to VSR for P9, so no need to check here. | |
2185 | Can this be a lamda or function? | |
2190 | Please add comments about the mapping from GPR to VSR? | |
2256 | Do we want to save the two GPRs in the order of endian order? or maybe fixed litten endian? | |
2266 | Never use assert(0...), that's what llvm_unreachable is for. | |
2430 | Maybe the order here should be align with the restore order? | |
2440 | Never use assert(0), that's what llvm_unreachable is for. | |
llvm/lib/Target/PowerPC/PPCFrameLowering.h | ||
32 | several GPRs? 1 or 2? | |
llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir | ||
16 ↗ | (On Diff #201814) | Please add comments about checking points. |
23 ↗ | (On Diff #201814) | It looks weird to have x14 before x15, although they should be the same. |
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | ||
---|---|---|
2256 | Here it always uses big-endian element order to save and restore, so it doesn't matter? |
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp | ||
---|---|---|
2259 | This name is strange. We are not spilling VSR's here. Perhaps LastVSRUsedForSpill? | |
2310 | I think an assert to ensure we are not trying to put more than two GPRs in a VSR should be added here. | |
2369 | Please avoid asserts without text explaining the assert. | |
llvm/lib/Target/PowerPC/PPCFrameLowering.h | ||
33 | I am curious why this maps from unsigned. Why not from Register? Finally, I think the name is confusing. I understand that it is mapping a VSR to GPRs, but since this optimization is moving stuff between registers, it can easily be misunderstood to mean that we are moving a VSR to GPRs. I think a better name would be something like VSRContainsGPRs. |
Address comments from Nemanja
llvm/lib/Target/PowerPC/PPCFrameLowering.h | ||
---|---|---|
33 | Ah, I just used unsigned to align with class CalleeSavedInfo definition: class CalleeSavedInfo { Register Reg; union { int FrameIdx; unsigned DstReg; }; //... }; |
several GPRs? 1 or 2?