[PATCH] D110053: [AMDGPU] Add a regclass flag for scalar registers

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp:1173
     if (Desc.OpInfo[I].RegClass == -1 ||
-        !TRI->isVGPRClass(TRI->getRegClass(Desc.OpInfo[I].RegClass)))
+        !TRI->isVSSuperClass(TRI->getRegClass(Desc.OpInfo[I].RegClass)))
       continue;
----------------
cdevadas wrote:
> rampitec wrote:
> > cdevadas wrote:
> > > rampitec wrote:
> > > > Why do you need to change this?
> > > I assumed that check is specifically for VS_32/VS_64 classes. Isn't it the case?
> > SDWA cannot use SGPR operand, the original instruction can. So if the operand is not a VGPR it should bail. isVGPRClass() is correct here.
> The check here is not to bail out, I guess, but to legalize the Op.
> The code here https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp#L1182
>  changes the operands of SGPRs to VGPRs by introducing a copy. (For gxf9 we can have at most 1 SGPR)
> 
> Look at this instruction:
> %29:vgpr_32 = V_MIN_I16_sdwa 1, **%12:sreg_32**, 1, **%22:sreg_32**, 0, 1, 0, 1, 1,
> The 2nd and 4th operands of this SDWA instruction are of **VS_32** type as per Desc. For gfx9, the 4th operand should be changed to VGPR class.
> The VS_32 class earlier contained only VGPR flag and it was ok to have `isVGPRClass` query to identify it. After introducing SGPR flag, we could use `hasVGPRs` query instead. But to make the check more precise, I used the combined VS class. 
> 
> Otherwise, MIR verifier reports an error.
> 
> 
> 
Thanks. Understood.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110053