[TableGen] Fix validateOperandClass for non Phyical Reg

[jsji]

b71704436e61
Rewrote the register operands handling,
but the Table only contains physical regs, we will SEGV when there are
non physical regs.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Jinsong Ji (jsji)

Changes

b71704436e61
Rewrote the register operands handling,
but the Table only contains physical regs, we will SEGV when there are
non physical regs.

---

Full diff: https://github.com/llvm/llvm-project/pull/118146.diff

1 Files Affected:

• (modified) llvm/utils/TableGen/AsmMatcherEmitter.cpp (+3-2)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 28efd780c5c615..1823783e82be4a 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -2522,8 +2522,9 @@ static void emitValidateOperandClass(const CodeGenTarget &Target,
   for (auto &MatchClassName : Table)
     OS << "      " << MatchClassName << ",\n";
   OS << "    };\n\n";
-  OS << "    MatchClassKind OpKind = "
-        "(MatchClassKind)Table[Operand.getReg().id()];\n";
+  OS << "    auto RegID=Operand.getReg().id();\n";
+  OS << "    MatchClassKind OpKind =  Register::isPhysicalRegister(RegID)?"
+        "(MatchClassKind)Table[RegID]: InvalidMatchClass;\n";
   OS << "    return isSubclass(OpKind, Kind) ? "
      << "(unsigned)MCTargetAsmParser::Match_Success :\n                     "
      << "                 getDiagKindFromRegisterClass(Kind);\n  }\n\n";

[s-barannikov]

How can a register not be physical here?

[jsji]

How can a register not be physical here?

Target can allow using non-physical registers in assembly.
We do have a downstream target that use virtual registers.
The original code also supports such scenarios in the default clause.

[s-barannikov]

nitpicking

• auto is discouraged by the coding standard
• the generated code is misformatted around '=', '?' and ':'

[s-barannikov]

How can a register not be physical here?

Target can allow using non-physical registers in assembly. We do have a downstream target that use virtual registers. The original code also supports such scenarios in the default clause.

I see, thanks.

[jsji]

Thanks @s-barannikov !

[jayfoad]

Can you add a test case for this?

[jsji]

Can you add a test case for this?

Sorry, I don't see easy way to do so, it involves defining a new target

[jayfoad]

Can you add a test case for this?

Sorry, I don't see easy way to do so, it involves defining a new target

Well, OK, but with no test and no in-tree use there is nothing to stop your use case being broken again in future. Did you consider just carrying the tablegen change downstream?