[PATCH] D57399: AMDGPU/GlobalISel: Add support for wide loads >= 256-bits

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:930-932
+static MachineInstr *getOtherVRegDef(const MachineRegisterInfo &MRI,
+                                     Register Reg,
+                                     const MachineInstr &MI) {
----------------
tstellar wrote:
> arsenm wrote:
> > tstellar wrote:
> > > arsenm wrote:
> > > > I don't understand this, because in SSA there must be exactly 1 def
> > > 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.
> > So you should be checking the registers from OpdMapper instead? This needs a comment at leasts
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57399/new/

https://reviews.llvm.org/D57399