[PATCH] D103326: [GlobalISel] Add combine for PTR_ADD with regbanks
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 1 16:19:42 PDT 2021
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
More information about the llvm-commits
mailing list