[PATCH] D139646: AMDGPU: Check if operand with vgpr RC contains reg when printing

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

Overall looks good.



================
Comment at: llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp:678
+        StringRef RCName(MRI.getRegClassName(RC));
+        if (RCName.contains("VReg_")) {
+          O << "/*Invalid register, operand has \'" << RCName
----------------
Petar.Avramovic wrote:
> I don't know what is going on here but if we check for other classes there is a lot of errors. Related to registers with special name not being member of register class, common error is that M0 is not in SReg_32.
The reason for these failures is that we have two kinds of registers - preudo and real. The latter are necessary to reflect the fact that some registers have different encodings on different GPUs. So you have to use `mc2PseudoReg`.

But this correction is insufficient. We also have some registers which are used in MC only. They are defined as 32-bit registers in td, but MC can use them where wider operands are expected.

So your code may be corrected as follows:

    const MCRegisterClass *RC = &MRI.getRegClass(Desc.OpInfo[OpNo].RegClass);
    auto Reg = mc2PseudoReg(Op.getReg());
    if (!RC->contains(Reg) && !isInlineValue(Reg)) {
        StringRef RCName(MRI.getRegClassName(RC));
        O << "/*Invalid register, operand has \'" << RCName
          << "\' register class*/";
      }
    }



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

https://reviews.llvm.org/D139646