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

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas 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;
----------------
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.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110053