[PATCH] D103326: [GlobalISel] Add combine for PTR_ADD with regbanks

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I wonder if we should try to intelligently pick the register banks for constants before adding combine logic relying on the current scheme where all constants end up in SGPRs.

I'm not sure how we want to address this though. Theoretically RegBankSelect itself should be selecting the G_CONSTANT bank based on the users, it just doesn't try to do this now. We could also have a different post regbank combine. We do have situations where depending on the value and how many uses there are, we may just want to rematerialize the constant multiple times. I'm not sure if that's something RegBankSelect should be doing itself



================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1716
+
+  MachineInstr *Add2Def = MRI.getUniqueVRegDef(Add2);
+  if (!Add2Def || Add2Def->getOpcode() != TargetOpcode::G_PTR_ADD)
----------------
Use regular getVRegDef


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1727-1728
+  auto &MF = *MI.getParent()->getParent();
+  const RegisterBankInfo &RBI = *MF.getSubtarget().getRegBankInfo();
+  const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
+  unsigned Bank = RBI.getRegBank(Imm1, MRI, TRI)->getID();
----------------
Probably should cache these in CombinerHelper if they're not there already


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1744
+  assert(MI.getOpcode() == TargetOpcode::G_PTR_ADD && "Expected G_PTR_ADD");
+  MachineIRBuilder MIB(MI);
+  LLT OffsetTy = MRI.getType(MI.getOperand(2).getReg());
----------------
Should not need to construct a new builder


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1749
+  const RegisterBankInfo &RBI = *MF.getSubtarget().getRegBankInfo();
+  MRI.setRegBank(NewOffset.getReg(0), RBI.getRegBank(MatchInfo.Bank));
+  Observer.changingInstr(MI);
----------------
The hard part of this is how to avoid spreading the ugly manual regbank preservation everywhere. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103326