[PATCH] D142834: [RFC] Update and clarify the behavoir of functions getX86SubSuperRegister*

[Kan Shengchen via Phabricator via llvm-commits]

skan created this revision.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
skan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

  MCRegister getX86SubSuperRegister*(MCRegister Reg, unsigned Size,
                                    bool High = false);

A strange behaviour of the functions `getX86SubSuperRegister*` was
introduced by llvm-svn:145579 for specifying any of the 8/16/32/64 register
names interchangeably in asm constraints: The returned register may not
match the parameters when a 8-bit high register is required.

And llvm-svn: 175762 refined the code and dropped the comments, then we
knew nothing happened there from the code :-(

>From the comments in `FixupBWInstPass::getSuperRegDestIfDead`:

  // If the original destination register was the low 8-bit subregister and
  // we also need to check the 16-bit subregister and the high 8-bit subregister.

I think we only need to care about the 16-bit register when the 8-bit
high register is not available.

`getX86SubSuperRegister` is just a wrapper of `getX86SubSuperRegisterOrZero`,
which may throw an error. To make sure the erro can be detected in
release build, I changed the `assert` to `report_fatal_error`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142834

Files:
  llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h


Index: llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
===================================================================
--- llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
+++ llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
@@ -135,14 +135,24 @@
 std::unique_ptr<MCObjectTargetWriter>
 createX86WinCOFFObjectWriter(bool Is64Bit);
 
-/// Returns the sub or super register of a specific X86 register.
-/// e.g. getX86SubSuperRegister(X86::EAX, 16) returns X86::AX.
-/// Aborts on error.
-MCRegister getX86SubSuperRegister(MCRegister, unsigned, bool High=false);
-
-/// Returns the sub or super register of a specific X86 register.
-/// Like getX86SubSuperRegister() but returns 0 on error.
-MCRegister getX86SubSuperRegisterOrZero(MCRegister, unsigned,
+/// \param Reg speicifed register.
+/// \param Size the bit size of returned register.
+/// \param High requires the high register.
+///
+/// \returns the sub or super register of a specific X86 register, if the
+/// 8-bit high register of does not exist, a 16-bit sub-register will be
+/// returned. Abort when no register is found.
+MCRegister getX86SubSuperRegister(MCRegister Reg, unsigned Size,
+                                  bool High = false);
+
+/// \param Reg speicifed register.
+/// \param Size the bit size of returned register.
+/// \param High requires the high register.
+///
+/// \returns the sub or super register of a specific X86 register, if the
+/// 8-bit high register of does not exist, a 16-bit sub-register will be
+/// returned.
+MCRegister getX86SubSuperRegisterOrZero(MCRegister Reg, unsigned Size,
                                         bool High = false);
 
 } // End llvm namespace
Index: llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
===================================================================
--- llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
+++ llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
@@ -749,15 +749,7 @@
   case 8:
     if (High) {
       switch (Reg.id()) {
-      default: return getX86SubSuperRegisterOrZero(Reg, 64);
-      case X86::SIL: case X86::SI: case X86::ESI: case X86::RSI:
-        return X86::SI;
-      case X86::DIL: case X86::DI: case X86::EDI: case X86::RDI:
-        return X86::DI;
-      case X86::BPL: case X86::BP: case X86::EBP: case X86::RBP:
-        return X86::BP;
-      case X86::SPL: case X86::SP: case X86::ESP: case X86::RSP:
-        return X86::SP;
+      default: return getX86SubSuperRegisterOrZero(Reg, 16);
       case X86::AH: case X86::AL: case X86::AX: case X86::EAX: case X86::RAX:
         return X86::AH;
       case X86::DH: case X86::DL: case X86::DX: case X86::EDX: case X86::RDX:
@@ -915,10 +907,10 @@
   }
 }
 
-MCRegister llvm::getX86SubSuperRegister(MCRegister Reg, unsigned Size, bool High) {
+MCRegister llvm::getX86SubSuperRegister(MCRegister Reg, unsigned Size,
+                                        bool High) {
   MCRegister Res = getX86SubSuperRegisterOrZero(Reg, Size, High);
-  assert(Res != X86::NoRegister && "Unexpected register or VT");
+  if (Res == X86::NoRegister)
+    report_fatal_error("Unexpected register or size");
   return Res;
 }
-
-


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D142834.493072.patch
Type: text/x-patch
Size: 3157 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230129/026e2a73/attachment.bin>