[PATCH] D139422: [AMDGPU] Accelerate SIRegisterInfo::getPhysRegClass

[Carl Ritson via Phabricator via llvm-commits]

critson marked 2 inline comments as done.
critson added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1434
+  if (!Reg.isPhysical())
+    return false;
+  auto RC = TRI.getPhysRegBaseClass(Reg);
----------------
foad wrote:
> arsenm wrote:
> > critson wrote:
> > > arsenm wrote:
> > > > It's even easier to do the class test on the virtual register
> > > I am sorry, I don't know what you mean by "easier".
> > > Easier as in less lines of code? Or, easier as in more computationally efficient?
> > > 
> > > Perhaps you are suggesting we replace most of this code with a call to `TRI->isSGPRReg()`?
> > > However, this function should only be true for physical registers (not any SGPR reg).
> > I mean saying any virtual registers are not SGPRs is wrong; if this is actually reachable with virtual registers it should try to handle them. If not it should assert
> If so that should be a separate patch. This one is NFC.
I'll work on a separate patch, it is definitely called on virtual registers so an assertion would not be appropriate.  Change it to `TRI->isSGPRReg()` causes failures in later passes so I will have to investigate that in detail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139422