[NFC][PowerPC] Use tablegen's MatchRegisterName()

[lei137]

Use PPC MatchRegisterName() that is auto generated by table gen.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

Author: Lei Huang (lei137)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp (+13-41)
• (modified) llvm/lib/Target/PowerPC/PPC.td (+2-1)

diff --git a/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp b/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
index cc20ad7822df3b..bf512481cf6460 100644
--- a/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
+++ b/llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp
@@ -1291,6 +1291,9 @@ bool PPCAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   llvm_unreachable("Implement any new match types added!");
 }
 
+#define GET_REGISTER_MATCHER
+#include "PPCGenAsmMatcher.inc"
+
 MCRegister PPCAsmParser::matchRegisterName(int64_t &IntVal) {
   if (getParser().getTok().is(AsmToken::Percent))
     getParser().Lex(); // Eat the '%'.
@@ -1298,55 +1301,25 @@ MCRegister PPCAsmParser::matchRegisterName(int64_t &IntVal) {
   if (!getParser().getTok().is(AsmToken::Identifier))
     return MCRegister();
 
-  MCRegister RegNo;
   StringRef Name = getParser().getTok().getString();
+  MCRegister RegNo = MatchRegisterName(Name);
+  if (!RegNo)
+    return RegNo;
+
+  Name.substr(Name.find_first_of("1234567890")).getAsInteger(10, IntVal);
+
+  // MatchRegisterName doesn't seem to have special handling for 64bit vs 32bit
+  // register types.
   if (Name.equals_insensitive("lr")) {
     RegNo = isPPC64() ? PPC::LR8 : PPC::LR;
     IntVal = 8;
   } else if (Name.equals_insensitive("ctr")) {
     RegNo = isPPC64() ? PPC::CTR8 : PPC::CTR;
     IntVal = 9;
-  } else if (Name.equals_insensitive("vrsave")) {
-    RegNo = PPC::VRSAVE;
+  } else if (Name.equals_insensitive("vrsave"))
     IntVal = 256;
-  } else if (Name.starts_with_insensitive("r") &&
-             !Name.substr(1).getAsInteger(10, IntVal) && IntVal < 32) {
+  else if (Name.starts_with_insensitive("r"))
     RegNo = isPPC64() ? XRegs[IntVal] : RRegs[IntVal];
-  } else if (Name.starts_with_insensitive("f") &&
-             !Name.substr(1).getAsInteger(10, IntVal) && IntVal < 32) {
-    RegNo = FRegs[IntVal];
-  } else if (Name.starts_with_insensitive("vs") &&
-             !Name.substr(2).getAsInteger(10, IntVal) && IntVal < 64) {
-    RegNo = VSRegs[IntVal];
-  } else if (Name.starts_with_insensitive("v") &&
-             !Name.substr(1).getAsInteger(10, IntVal) && IntVal < 32) {
-    RegNo = VRegs[IntVal];
-  } else if (Name.starts_with_insensitive("cr") &&
-             !Name.substr(2).getAsInteger(10, IntVal) && IntVal < 8) {
-    RegNo = CRRegs[IntVal];
-  } else if (Name.starts_with_insensitive("acc") &&
-             !Name.substr(3).getAsInteger(10, IntVal) && IntVal < 8) {
-    RegNo = ACCRegs[IntVal];
-  } else if (Name.starts_with_insensitive("wacc_hi") &&
-             !Name.substr(7).getAsInteger(10, IntVal) && IntVal < 8) {
-    RegNo = ACCRegs[IntVal];
-  } else if (Name.starts_with_insensitive("wacc") &&
-             !Name.substr(4).getAsInteger(10, IntVal) && IntVal < 8) {
-    RegNo = WACCRegs[IntVal];
-  } else if (Name.starts_with_insensitive("dmrrowp") &&
-             !Name.substr(7).getAsInteger(10, IntVal) && IntVal < 32) {
-    RegNo = DMRROWpRegs[IntVal];
-  } else if (Name.starts_with_insensitive("dmrrow") &&
-             !Name.substr(6).getAsInteger(10, IntVal) && IntVal < 64) {
-    RegNo = DMRROWRegs[IntVal];
-  } else if (Name.starts_with_insensitive("dmrp") &&
-             !Name.substr(4).getAsInteger(10, IntVal) && IntVal < 4) {
-    RegNo = DMRROWpRegs[IntVal];
-  } else if (Name.starts_with_insensitive("dmr") &&
-             !Name.substr(3).getAsInteger(10, IntVal) && IntVal < 8) {
-    RegNo = DMRRegs[IntVal];
-  } else
-    return MCRegister();
 
   getParser().Lex();
   return RegNo;
@@ -1874,7 +1847,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializePowerPCAsmParser() {
   RegisterMCAsmParser<PPCAsmParser> D(getThePPC64LETarget());
 }
 
-#define GET_REGISTER_MATCHER
 #define GET_MATCHER_IMPLEMENTATION
 #define GET_MNEMONIC_SPELL_CHECKER
 #include "PPCGenAsmMatcher.inc"
diff --git a/llvm/lib/Target/PowerPC/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index da31a993b9c694..72c5909f10c3be 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -719,7 +719,8 @@ def PPCAsmWriter : AsmWriter {
 }
 
 def PPCAsmParser : AsmParser {
-  let ShouldEmitMatchRegisterName = 0;
+  let ShouldEmitMatchRegisterName = 1;
+  let AllowDuplicateRegisterNames = 1;
 }
 
 def PPCAsmParserVariant : AsmParserVariant {

[amy-kwan]

LGTM.
Also, since we have no tests, should this be marked with NFC?

[tstellar]

I think this commit caused a regression in the dotnet9.0 build for Fedora:

Consolidate compiler generated dependencies of target utilcodestaticnohost
make[3]: Leaving directory '/builddir/build/BUILD/dotnet9.0-9.0.102-build/dotnet-9.0.1/src/runtime/artifacts/obj/coreclr/linux.ppc64le.Release'
make  -f utilcode/CMakeFiles/utilcodestaticnohost.dir/build.make utilcode/CMakeFiles/utilcodestaticnohost.dir/build
/builddir/build/BUILD/dotnet9.0-9.0.102-build/dotnet-9.0.1/src/runtime/src/coreclr/pal/src/arch/ppc64le/context2.S:140:15: error: invalid register name
              ld %r0, 0 +8(%R3)
                           ^

[lei137]

I think this commit caused a regression in the dotnet9.0 build for Fedora:

Consolidate compiler generated dependencies of target utilcodestaticnohost
make[3]: Leaving directory '/builddir/build/BUILD/dotnet9.0-9.0.102-build/dotnet-9.0.1/src/runtime/artifacts/obj/coreclr/linux.ppc64le.Release'
make  -f utilcode/CMakeFiles/utilcodestaticnohost.dir/build.make utilcode/CMakeFiles/utilcodestaticnohost.dir/build
/builddir/build/BUILD/dotnet9.0-9.0.102-build/dotnet-9.0.1/src/runtime/src/coreclr/pal/src/arch/ppc64le/context2.S:140:15: error: invalid register name
              ld %r0, 0 +8(%R3)
                           ^

Is the problem R3 vs r3? I don't see a way for this patch to cause register name to resolve to R* in some cases and the valid r* in other.... The code that returns either r* or x* register types have actually not changed.

else if (Name.starts_with_insensitive("r"))
    RegNo = isPPC64() ? XRegs[IntVal] : RRegs[IntVal];

I might be missing something, can you please provide more info as to why you think this patch is the issue?

[tstellar]

I didn't see your response in time and just filed an issue for this here: #126786

Maybe we can discuss it there.

[redstar]

I might be missing something,

MCRegister RegNo = MatchRegisterName(Name);

With a brief look at the generated code it seems to me that it matches only lower case names.

[tstellar]

I might be missing something,

MCRegister RegNo = MatchRegisterName(Name);

With a brief look at the generated code it seems to me that it matches only lower case names.

Yes, I think that's the issue.

[lei137]

I might be missing something,

MCRegister RegNo = MatchRegisterName(Name);
With a brief look at the generated code it seems to me that it matches only lower case names.

Yes, I think that's the issue.

That behaviour have not changed AFAIK.

[tstellar]

I might be missing something,

MCRegister RegNo = MatchRegisterName(Name);
With a brief look at the generated code it seems to me that it matches only lower case names.

Yes, I think that's the issue.

That behaviour have not changed AFAIK.

The behavior of the generated function has not changed, but that function is now being used to parse register names where it wasn't before.