[PATCH] D91701: [AMDGPU] Implement flat scratch init for pal

Sebastian Neubauer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 19 10:17:24 PST 2020


sebastian-ne added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:320
+  MBB.addLiveIn(GitPtrLo);
+  BuildMI(MBB, I, DL, SMovB32, TargetLo).addReg(GitPtrLo);
+}
----------------
rampitec wrote:
> arsenm wrote:
> > sebastian-ne wrote:
> > > rampitec wrote:
> > > > rampitec wrote:
> > > > > .addReg() on the next line.
> > > > Isn't this a first subreg def, so this one shall have superreg def?
> > > > .addReg() on the next line.
> > > 
> > > The autoformatter complained about that, shall I do it anyway?
> > > 
> > > > Isn't this a first subreg def, so this one shall have superreg def?
> > > 
> > > I’m not sure? In the if-branch, `TargetHi` is defined, in the else-branch the whole `TargetReg` is defined.
> > The formatter is stupid and should be ignored
> I mean in the final ISA TergetLo will be defined first, so probably it is TargetLo should carry implicit def of the super-reg?
The ISA looks like this:
```
s_getpc_b64 s[0:1]    <- Created else-branch before, explicitly defines TargetReg
s_mov_b32 s0, s8      <- This move replaces the lower register (TargetLo)
```

So I think the first definition the whole register is in the if-else and this move does not need an implicit-def anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91701



More information about the llvm-commits mailing list