[PATCH] D121524: [AMDGPU] use scalar shift for SALU users in frame index elimination

[Alexander via Phabricator via llvm-commits]

alex-t marked an inline comment as done.
alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp:2237
+        bool IsSALU = isSGPRClass(TII->getOpRegClass(*MI, FIOperandNum));
+        bool LiveSCC = RS->isRegUsed(AMDGPU::SCC);
+        const TargetRegisterClass *RC = IsSALU && !LiveSCC
----------------
rampitec wrote:
> arsenm wrote:
> > rampitec wrote:
> > > arsenm wrote:
> > > > rampitec wrote:
> > > > > alex-t wrote:
> > > > > > rampitec wrote:
> > > > > > > alex-t wrote:
> > > > > > > > rampitec wrote:
> > > > > > > > > Is not isRegUsed for the whole function and will always return true on practice?
> > > > > > > > This function is used to determine if SCC is live in all other places around the code.
> > > > > > > > Another option is to scan upwards until the block start is reached.
> > > > > > > All other places shall mean the whole function? I'd suggest using MBB->computeRegisterLiveness().
> > > > > > I mean checks like this:
> > > > > > 
> > > > > > CanClobberSCC = !RS->isRegUsed(AMDGPU::SCC);
> > > > > > 
> > > > > > they really interested if SCC is live at exact point.
> > > > > > 
> > > > > > In case this is wrong we have lots of problems
> > > > > But what point is it? The call does not take an iterator.
> > > > It's at the scavenging point, which is separately advanced
> > > Was it advanced before the call?
> > It should have been advanced by the scavengeRegister call (assuming there is one on the path that reaches here). Plus PrologEpilogInserter advances before the call in the normal usage
> OK, thanks.
RegisterScavenger::forward takes care of that. It is called from PEI::eliminateFrameIndices on each iteration. So it updates live information stepping each instruction.
The very first call of the SIRegisterInfo::eliminateFrameIndex happens with the LiveRegs updated on the BB entrance with LiveIns.
Each next call happens after the RS->forward updated live info.
I have a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121524