X86: Rework inline asm integer register specification.

[MatzeB]

Summary

This is a new version of http://reviews.llvm.org/D10260.

It turned out that when you specify an integer register in inline asm on
x86 you get the register of the required type size back. That means that
X86TargetLowering::getRegForInlineAsmConstraint() has to accept any of
the integer registers and adapt its size to the given target size which
may be any 8/16/32/64 bit sized type. Surprisingly that means given a
constraint of "{ax}" and a type of MVT::F32 we need to return X86::EAX.

This change makes this face explicit, the previous code seemed like
working by accident because there it never returned an error once a
register was found. On the other hand this rewrite allows to actually
return errors for invalid situations like requesting an integer register
for an i128 type.

[echristo]

Hi Matthias,

LGTM with a fix for the testcases (missing FileCheck etc). Also if you can avoid the temporary file it'd be great.

Thanks!

-eric

test/CodeGen/X86/asm-reject-reg-type-mismatch.ll

1 ↗

(On Diff #28706)

Do you mean -filetype=asm here?

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

if (Constraint == "A") {
       return Res;
     }
     return Res;
   }
 
   // Otherwise, check to see if this is a register class of the wrong value
   // type.  For example, we want to map "{ax},i32" -> {eax}, we don't want it to
   // turn into {ax},{dx}.
-  if (Res.second->hasType(VT))
+  // MVT::Other is used to specify clobber names.
+  if (Res.second->hasType(VT) || VT == MVT::Other)
     return Res;   // Correct type already, nothing to do.
 
-  // All of the single-register GCC register classes map their values onto
-  // 16-bit register pieces "ax","dx","cx","bx","si","di","bp","sp".  If we
-  // really want an 8-bit or 32-bit register, map to the appropriate register
-  // class and return the appropriate register.
-  if (Res.second == &X86::GR16RegClass) {
-    if (VT == MVT::i8 || VT == MVT::i1) {
-      unsigned DestReg = 0;
-      switch (Res.first) {
-      default: break;
-      case X86::AX: DestReg = X86::AL; break;
-      case X86::DX: DestReg = X86::DL; break;
-      case X86::CX: DestReg = X86::CL; break;
-      case X86::BX: DestReg = X86::BL; break;
-      }
-      if (DestReg) {
-        Res.first = DestReg;
-        Res.second = &X86::GR8RegClass;
-      }
-    } else if (VT == MVT::i32 || VT == MVT::f32) {
-      unsigned DestReg = 0;
-      switch (Res.first) {
-      default: break;
-      case X86::AX: DestReg = X86::EAX; break;
-      case X86::DX: DestReg = X86::EDX; break;
-      case X86::CX: DestReg = X86::ECX; break;
-      case X86::BX: DestReg = X86::EBX; break;
-      case X86::SI: DestReg = X86::ESI; break;
-      case X86::DI: DestReg = X86::EDI; break;
-      case X86::BP: DestReg = X86::EBP; break;
-      case X86::SP: DestReg = X86::ESP; break;
-      }
-      if (DestReg) {
-        Res.first = DestReg;
-        Res.second = &X86::GR32RegClass;
-      }
-    } else if (VT == MVT::i64 || VT == MVT::f64) {
-      unsigned DestReg = 0;
-      switch (Res.first) {
-      default: break;
-      case X86::AX: DestReg = X86::RAX; break;
-      case X86::DX: DestReg = X86::RDX; break;
-      case X86::CX: DestReg = X86::RCX; break;
-      case X86::BX: DestReg = X86::RBX; break;
-      case X86::SI: DestReg = X86::RSI; break;
-      case X86::DI: DestReg = X86::RDI; break;
-      case X86::BP: DestReg = X86::RBP; break;
-      case X86::SP: DestReg = X86::RSP; break;
-      }
-      if (DestReg) {
-        Res.first = DestReg;
-        Res.second = &X86::GR64RegClass;
-      }
-    }
-  } else if (Res.second == &X86::FR32RegClass ||
-             Res.second == &X86::FR64RegClass ||
-             Res.second == &X86::VR128RegClass ||
-             Res.second == &X86::VR256RegClass ||
-             Res.second == &X86::FR32XRegClass ||
-             Res.second == &X86::FR64XRegClass ||
-             Res.second == &X86::VR128XRegClass ||
-             Res.second == &X86::VR256XRegClass ||
-             Res.second == &X86::VR512RegClass) {
+  // Get a matching integer of the correct size. i.e. "ax" with MVT::32 should
+  // return "eax". This should even work for things like getting 64bit integer
+  // registers when given an f64 type.
+  const TargetRegisterClass *Class = Res.second;
+  if (Class == &X86::GR8RegClass || Class == &X86::GR16RegClass ||
+      Class == &X86::GR32RegClass || Class == &X86::GR64RegClass) {
+    unsigned Size = VT.getSizeInBits();
+    MVT::SimpleValueType SimpleTy = Size == 1 || Size == 8 ? MVT::i8
+                                  : Size == 16 ? MVT::i16
+                                  : Size == 32 ? MVT::i32
+                                  : Size == 64 ? MVT::i64
+                                  : MVT::Other;
+    unsigned DestReg = getX86SubSuperRegisterOrZero(Res.first, SimpleTy);
+    if (DestReg > 0) {
+      Res.second = SimpleTy == MVT::i8 ? &X86::GR8RegClass
+                 : SimpleTy == MVT::i16 ? &X86::GR16RegClass
+                 : SimpleTy == MVT::i32 ? &X86::GR32RegClass
+                 : &X86::GR64RegClass;
+      assert(Res.second->contains(Res.first) && "Register in register class");
+    } else {
+      // No register found/type mismatch.
+      Res.first = 0;
+      Res.second = nullptr;
+    }
+  } else if (Class == &X86::FR32RegClass || Class == &X86::FR64RegClass ||
+             Class == &X86::VR128RegClass || Class == &X86::VR256RegClass ||
+             Class == &X86::FR32XRegClass || Class == &X86::FR64XRegClass ||
+             Class == &X86::VR128XRegClass || Class == &X86::VR256XRegClass ||
+             Class == &X86::VR512RegClass) {
     // Handle references to XMM physical registers that got mapped into the
     // wrong class.  This can happen with constraints like {xmm0} where the
     // target independent register mapper will just pick the first match it can
     // find, ignoring the required type.
 
     if (VT == MVT::f32 || VT == MVT::i32)
       Res.second = &X86::FR32RegClass;
     else if (VT == MVT::f64 || VT == MVT::i64)
       Res.second = &X86::FR64RegClass;
     else if (X86::VR128RegClass.hasType(VT))
       Res.second = &X86::VR128RegClass;
     else if (X86::VR256RegClass.hasType(VT))
       Res.second = &X86::VR256RegClass;
     else if (X86::VR512RegClass.hasType(VT))
       Res.second = &X86::VR512RegClass;
+    else {
+      // Type mismatch and not a clobber: Return an error;
+      Res.first = 0;
+      Res.second = nullptr;
+    }
   }
 
   return Res;
 }
 
 int X86TargetLowering::getScalingFactorCost(const AddrMode &AM,
                                             Type *Ty,
                                             unsigned AS) const {

llvm/trunk/lib/Target/X86/X86RegisterInfo.h

public:
   unsigned getFrameRegister(const MachineFunction &MF) const override;
   unsigned getPtrSizedFrameRegister(const MachineFunction &MF) const;
   unsigned getStackRegister() const { return StackPtr; }
   unsigned getBaseRegister() const { return BasePtr; }
   // FIXME: Move to FrameInfok
   unsigned getSlotSize() const { return SlotSize; }
 };
 
-// getX86SubSuperRegister - X86 utility function. It returns the sub or super
-// register of a specific X86 register.
-// e.g. getX86SubSuperRegister(X86::EAX, MVT::i16) return X86:AX
+/// Returns the sub or super register of a specific X86 register.
+/// e.g. getX86SubSuperRegister(X86::EAX, MVT::i16) returns X86::AX.
+/// Aborts on error.
 unsigned getX86SubSuperRegister(unsigned, MVT::SimpleValueType, bool High=false);
 
+/// Returns the sub or super register of a specific X86 register.
+/// Like getX86SubSuperRegister() but returns 0 on error.
+unsigned getX86SubSuperRegisterOrZero(unsigned, MVT::SimpleValueType,
+                                      bool High = false);
+
 //get512BitRegister - X86 utility - returns 512-bit super register
 unsigned get512BitSuperRegister(unsigned Reg);
 
 } // End llvm namespace
 
 #endif

llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp

X86RegisterInfo::getPtrSizedFrameRegister(const MachineFunction &MF) const {
   const X86Subtarget &Subtarget = MF.getSubtarget<X86Subtarget>();
   unsigned FrameReg = getFrameRegister(MF);
   if (Subtarget.isTarget64BitILP32())
     FrameReg = getX86SubSuperRegister(FrameReg, MVT::i32, false);
   return FrameReg;
 }
 
 namespace llvm {
-unsigned getX86SubSuperRegister(unsigned Reg, MVT::SimpleValueType VT,
+unsigned getX86SubSuperRegisterOrZero(unsigned Reg, MVT::SimpleValueType VT,
                                       bool High) {
   switch (VT) {
-  default: llvm_unreachable("Unexpected VT");
+  default: return 0;
   case MVT::i8:
     if (High) {
       switch (Reg) {
       default: return getX86SubSuperRegister(Reg, MVT::i64);
       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;
       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:
         return X86::DH;
       case X86::CH: case X86::CL: case X86::CX: case X86::ECX: case X86::RCX:
         return X86::CH;
       case X86::BH: case X86::BL: case X86::BX: case X86::EBX: case X86::RBX:
         return X86::BH;
       }
     } else {
       switch (Reg) {
-      default: llvm_unreachable("Unexpected register");
+      default: return 0;
       case X86::AH: case X86::AL: case X86::AX: case X86::EAX: case X86::RAX:
         return X86::AL;
       case X86::DH: case X86::DL: case X86::DX: case X86::EDX: case X86::RDX:
         return X86::DL;
       case X86::CH: case X86::CL: case X86::CX: case X86::ECX: case X86::RCX:
         return X86::CL;
       case X86::BH: case X86::BL: case X86::BX: case X86::EBX: case X86::RBX:
         return X86::BL;
if (High) {
       case X86::R14B: case X86::R14W: case X86::R14D: case X86::R14:
         return X86::R14B;
       case X86::R15B: case X86::R15W: case X86::R15D: case X86::R15:
         return X86::R15B;
       }
     }
   case MVT::i16:
     switch (Reg) {
-    default: llvm_unreachable("Unexpected register");
+    default: return 0;
     case X86::AH: case X86::AL: case X86::AX: case X86::EAX: case X86::RAX:
       return X86::AX;
     case X86::DH: case X86::DL: case X86::DX: case X86::EDX: case X86::RDX:
       return X86::DX;
     case X86::CH: case X86::CL: case X86::CX: case X86::ECX: case X86::RCX:
       return X86::CX;
     case X86::BH: case X86::BL: case X86::BX: case X86::EBX: case X86::RBX:
       return X86::BX;
case X86::R13B: case X86::R13W: case X86::R13D: case X86::R13:
       return X86::R13W;
     case X86::R14B: case X86::R14W: case X86::R14D: case X86::R14:
       return X86::R14W;
     case X86::R15B: case X86::R15W: case X86::R15D: case X86::R15:
       return X86::R15W;
     }
   case MVT::i32:
     switch (Reg) {
-    default: llvm_unreachable("Unexpected register");
+    default: return 0;
     case X86::AH: case X86::AL: case X86::AX: case X86::EAX: case X86::RAX:
       return X86::EAX;
     case X86::DH: case X86::DL: case X86::DX: case X86::EDX: case X86::RDX:
       return X86::EDX;
     case X86::CH: case X86::CL: case X86::CX: case X86::ECX: case X86::RCX:
       return X86::ECX;
     case X86::BH: case X86::BL: case X86::BX: case X86::EBX: case X86::RBX:
       return X86::EBX;
case X86::R13B: case X86::R13W: case X86::R13D: case X86::R13:
       return X86::R13D;
     case X86::R14B: case X86::R14W: case X86::R14D: case X86::R14:
       return X86::R14D;
     case X86::R15B: case X86::R15W: case X86::R15D: case X86::R15:
       return X86::R15D;
     }
   case MVT::i64:
     switch (Reg) {
-    default: llvm_unreachable("Unexpected register");
+    default: return 0;
     case X86::AH: case X86::AL: case X86::AX: case X86::EAX: case X86::RAX:
       return X86::RAX;
     case X86::DH: case X86::DL: case X86::DX: case X86::EDX: case X86::RDX:
       return X86::RDX;
     case X86::CH: case X86::CL: case X86::CX: case X86::ECX: case X86::RCX:
       return X86::RCX;
     case X86::BH: case X86::BL: case X86::BX: case X86::EBX: case X86::RBX:
       return X86::RBX;
case MVT::i64:
     case X86::R14B: case X86::R14W: case X86::R14D: case X86::R14:
       return X86::R14;
     case X86::R15B: case X86::R15W: case X86::R15D: case X86::R15:
       return X86::R15;
     }
   }
 }
 
+unsigned getX86SubSuperRegister(unsigned Reg, MVT::SimpleValueType VT,
+                                bool High) {
+  unsigned Res = getX86SubSuperRegisterOrZero(Reg, VT, High);
+  if (Res == 0)
+    llvm_unreachable("Unexpected register or VT");
+  return Res;
+}
+
 unsigned get512BitSuperRegister(unsigned Reg) {
   if (Reg >= X86::XMM0 && Reg <= X86::XMM31)
     return X86::ZMM0 + (Reg - X86::XMM0);
   if (Reg >= X86::YMM0 && Reg <= X86::YMM31)
     return X86::ZMM0 + (Reg - X86::YMM0);
   if (Reg >= X86::ZMM0 && Reg <= X86::ZMM31)
     return Reg;
   llvm_unreachable("Unexpected SIMD register");
 }
 
 }

llvm/trunk/test/CodeGen/X86/asm-mismatched-types.ll

+; RUN: llc -o - %s -no-integrated-as | FileCheck %s
+target triple = "x86_64--"
+
+; Allow to specify any of the 8/16/32/64 register names interchangeably in
+; constraints
+
+; Produced by C-programs like this:
+; void foo(int p) { register int reg __asm__("r8") = p;
+; __asm__ __volatile__("# REG: %0" : : "r" (reg)); }
+
+; CHECK-LABEL: reg64_as_32:
+; CHECK: # REG: %r8d
+define void @reg64_as_32(i32 %p) {
+  call void asm sideeffect "# REG: $0", "{r8}"(i32 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg64_as_32_float:
+; CHECK: # REG: %r8d
+define void @reg64_as_32_float(float %p) {
+  call void asm sideeffect "# REG: $0", "{r8}"(float %p)
+  ret void
+}
+
+; CHECK-LABEL: reg64_as_16:
+; CHECK: # REG: %r9w
+define void @reg64_as_16(i16 %p) {
+  call void asm sideeffect "# REG: $0", "{r9}"(i16 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg64_as_8:
+; CHECK: # REG: %bpl
+define void @reg64_as_8(i8 %p) {
+  call void asm sideeffect "# REG: $0", "{rbp}"(i8 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg32_as_16:
+; CHECK: # REG: %r15w
+define void @reg32_as_16(i16 %p) {
+  call void asm sideeffect "# REG: $0", "{r15d}"(i16 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg32_as_8:
+; CHECK: # REG: %r12b
+define void @reg32_as_8(i8 %p) {
+  call void asm sideeffect "# REG: $0", "{r12d}"(i8 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg16_as_8:
+; CHECK: # REG: %cl
+define void @reg16_as_8(i8 %p) {
+  call void asm sideeffect "# REG: $0", "{cx}"(i8 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg32_as_64:
+; CHECK: # REG: %rbp
+define void @reg32_as_64(i64 %p) {
+  call void asm sideeffect "# REG: $0", "{ebp}"(i64 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg32_as_64_float:
+; CHECK: # REG: %rbp
+define void @reg32_as_64_float(double %p) {
+  call void asm sideeffect "# REG: $0", "{ebp}"(double %p)
+  ret void
+}
+
+; CHECK-LABEL: reg16_as_64:
+; CHECK: # REG: %r13
+define void @reg16_as_64(i64 %p) {
+  call void asm sideeffect "# REG: $0", "{r13w}"(i64 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg16_as_64_float:
+; CHECK: # REG: %r13
+define void @reg16_as_64_float(double %p) {
+  call void asm sideeffect "# REG: $0", "{r13w}"(double %p)
+  ret void
+}
+
+; CHECK-LABEL: reg8_as_64:
+; CHECK: # REG: %rax
+define void @reg8_as_64(i64 %p) {
+  call void asm sideeffect "# REG: $0", "{al}"(i64 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg8_as_64_float:
+; CHECK: # REG: %rax
+define void @reg8_as_64_float(double %p) {
+  call void asm sideeffect "# REG: $0", "{al}"(double %p)
+  ret void
+}
+
+; CHECK-LABEL: reg16_as_32:
+; CHECK: # REG: %r11d
+define void @reg16_as_32(i32 %p) {
+  call void asm sideeffect "# REG: $0", "{r11w}"(i32 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg16_as_32_float:
+; CHECK: # REG: %r11d
+define void @reg16_as_32_float(float %p) {
+  call void asm sideeffect "# REG: $0", "{r11w}"(float %p)
+  ret void
+}
+
+; CHECK-LABEL: reg8_as_32:
+; CHECK: # REG: %r9d
+define void @reg8_as_32(i32 %p) {
+  call void asm sideeffect "# REG: $0", "{r9b}"(i32 %p)
+  ret void
+}
+
+; CHECK-LABEL: reg8_as_32_float:
+; CHECK: # REG: %r9d
+define void @reg8_as_32_float(float %p) {
+  call void asm sideeffect "# REG: $0", "{r9b}"(float %p)
+  ret void
+}
+
+; CHECK-LABEL: reg8_as_16:
+; CHECK: # REG: %di
+define void @reg8_as_16(i16 %p) {
+  call void asm sideeffect "# REG: $0", "{dil}"(i16 %p)
+  ret void
+}

llvm/trunk/test/CodeGen/X86/asm-reject-reg-type-mismatch.ll

+; RUN: not llc -o /dev/null %s 2>&1 | FileCheck %s
+target triple = "x86_64--"
+
+; CHECK: error: couldn't allocate output register for constraint '{ax}'
+define i128 @blup() {
+  %v = tail call i128 asm "", "={ax},0"(i128 0)
+  ret i128 %v
+}