This adds support for the most commonly used wide load types:
<8xi32>, <16xi32>, <4xi64>, and <8xi64>
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 27456 Build 27455: arc lint + arc unit
Event Timeline
lib/Target/AMDGPU/AMDGPUGenRegisterBankInfo.def | ||
---|---|---|
185–188 | I have most of a patch for this | |
209 | Extra space before ( | |
lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
141 | extra newline | |
198–199 | No, the verifier rejects physical registers in G_ instructions | |
407 | This should be split to a new function | |
451–454 | You can do just MF.getMachineMemOperand(MMO, Offset, Size). I hit a bug in it though for D57122 | |
451–454 | Actually for this entire part I think once my other load/store patches are committed, you could just re-use the functions in LegalizerHelper | |
569–570 | No, the verifier rejects physical registers in G_ instructions | |
test/CodeGen/AMDGPU/GlobalISel/regbankselect-load.mir | ||
4 | This isn't needed anymore | |
7 | I want to split the memory tests per address space, so split this into regbankselect-load-global, regbankselect-load-constant |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
82 ↗ | (On Diff #208577) | This is identical to ApplySALUMapping, so maybe just add an argument to the constructor for the register bank? |
930–932 ↗ | (On Diff #208577) | I don't understand this, because in SSA there must be exactly 1 def |
933 ↗ | (On Diff #208577) | s/asset/assert |
969 ↗ | (On Diff #208577) | Registser |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
930–932 ↗ | (On Diff #208577) | It's just a temporary state, because RegBankSelect::applyMapping() calls RegBankSelect::repairReg() which inserts a "Repair Instruction" that writes to the register being repaired. This is done before calling applyMapping() which it assumes will delete the original instruction writing the register. |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
930–932 ↗ | (On Diff #208577) | So you should be checking the registers from OpdMapper instead? This needs a comment at leasts |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
930–932 ↗ | (On Diff #208577) | I can try that for getting the RepairInst, but we still need a way to get the LegalizedInst a little lower in the function. The other solution I had for this was to use the Observer and just record the last instruction added by the LegalizeHelper, but this seemed like we would be relying too much on the implementation details of LegalizeHelper::fewerVectorElements. |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
930–932 ↗ | (On Diff #208577) | There are definitely issues trying to using the LegalizerHelper from RegBankSelect. In another case we're now relying on the behavior that the LegalizerHelper happens to modify the instruction in place. |
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | ||
---|---|---|
67 ↗ | (On Diff #208587) | This should use SCC for the SALU version and VCC for the VALU version |
I added a better comment to getOtherVRegDef. I tried to come up with
a solution using the OperandsMapper, but in the end I had to implement
something similar to getOtherVRegDef to in order to verify that we retreived the
correct LegalizedInst from the LegalizeHelper. It was also way more code and
might be more confusing. Hopefully the new comment on getOtherVRegDef makes
it clear what is going on.
I have most of a patch for this