[X86][ASM] Fixed assertion crash with inline asm (PR14393)

[kbelochapka]

Summary

Fixed assertion crash when inline assembler input parameter has floating point constraint but requires conversion from anything but floating point entity.

[RKSimon]

Please don't forget to include llvm-commits as a subscriber otherwise your patch is likely to be missed

[RKSimon]

The test files need to be simplified as much as possible - many of the attributes (e.g. all the debug srcloc) and meta data can be dropped and the remaining moved into the command line.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

286

(style) You should be able remove most braces, also cast/dyn_cast calls can use auto:

if (auto *I = dyn_cast_or_null<Instruction>(V)) {
  if (auto *CI = dyn_cast<CallInst>(I))
    if (isa<InlineAsm>(CI->getCalledValue()))
      return Ctx.emitError(CI, ErrMsg + ", in an __asm__");
  return Ctx.emitError(I, ErrMsg);
}

465

ValueVT = PartVT?

469

newline

490–491

Add assertion message

598

ValueVT =PartVT?

test/CodeGen/X86/asm-inconsistent-operand-constraints-fpreg-integer.ll

17

drop this

19

Remove these and add to the command line as -mtriple=x86_64-unknown-linux-gnu

test/CodeGen/X86/asm-inconsistent-operand-constraints-fpreg-vector.ll

27

drop this

29

Remove these and add to the command line as -mtriple=x86_64-unknown-linux-gnu

[kbelochapka]

limit showing error message only to the cases that require copy of non floating point datum into MVT::f80 container.

[kbelochapka]

Simon, can you please take a second look at this one when you have a chance.

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

static void diagnosePossiblyInvalidConstraint(LLVMContext &Ctx, const Value *V,
   const char *AsmError = ", possible invalid constraint for vector type";
   if (const CallInst *CI = dyn_cast<CallInst>(I))
     if (isa<InlineAsm>(CI->getCalledValue()))
       return Ctx.emitError(I, ErrMsg + AsmError);
 
   return Ctx.emitError(I, ErrMsg);
 }
 
+static void emitInvalidConstraintError(LLVMContext &Ctx, const Value *V,
+                                       const StringRef &ErrMsg) {
+  if (auto *I = dyn_cast_or_null<Instruction>(V)) {
+    if (auto *CI = dyn_cast<CallInst>(I))
+      if (isa<InlineAsm>(CI->getCalledValue()))
+        return Ctx.emitError(CI, ErrMsg + ", in an __asm__");
+    return Ctx.emitError(I, ErrMsg);
+  }
+  return Ctx.emitError(ErrMsg);
+}
+
 /// getCopyFromPartsVector - Create a value that contains the specified legal
 /// parts combined into the value they represent.  If the parts combine to a
 /// type larger than ValueVT then AssertOp can be used to specify whether the
 /// extra bits are known to be zero (ISD::AssertZext) or sign extended from
 /// ValueVT (ISD::AssertSext).
 static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL,
                                       const SDValue *Parts, unsigned NumParts,
                                       MVT PartVT, EVT ValueVT, const Value *V,
static void getCopyToParts(SelectionDAG &DAG, const SDLoc &DL, SDValue Val,
   assert(!ValueVT.isVector() && "Vector case handled elsewhere");
   EVT PartEVT = PartVT;
   if (PartEVT == ValueVT) {
     assert(NumParts == 1 && "No-op copy with multiple parts!");
     Parts[0] = Val;
     return;
   }
 
+  if (NumParts == 1) {
+    if ((PartVT == MVT::f80) && (!ValueVT.isFloatingPoint())) {
+      // Alternative is to use ISD::SINT_TO_FP instead of generating an error.
+      Val = DAG.getUNDEF(PartVT);
+      emitInvalidConstraintError(*DAG.getContext(), V,
+                                 "Inconsistent operand constraints");
+      Parts[0] = Val;
+      return;
+    }
+  }
   if (NumParts * PartBits > ValueVT.getSizeInBits()) {
     // If the parts cover more bits than the value has, promote the value.
     if (PartVT.isFloatingPoint() && ValueVT.isFloatingPoint()) {
       assert(NumParts == 1 && "Do not know what to promote to!");
       Val = DAG.getNode(ISD::FP_EXTEND, DL, PartVT, Val);
     } else {
       if (ValueVT.isFloatingPoint()) {
         // FP values need to be bitcast, then extended if they are being put
         // into a larger container.
         ValueVT = EVT::getIntegerVT(*DAG.getContext(),  ValueVT.getSizeInBits());
         Val = DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
       }
       assert((PartVT.isInteger() || PartVT == MVT::x86mmx) &&
              ValueVT.isInteger() &&
              "Unknown mismatch!");
       ValueVT = EVT::getIntegerVT(*DAG.getContext(), NumParts * PartBits);
       Val = DAG.getNode(ExtendKind, DL, ValueVT, Val);
       if (PartVT == MVT::x86mmx)
         Val = DAG.getNode(ISD::BITCAST, DL, PartVT, Val);
     }
   } else if (PartBits == ValueVT.getSizeInBits()) {
-    // Different types of the same size.
-    assert(NumParts == 1 && PartEVT != ValueVT);
+    // Same type or different types of the same size.
+    assert(NumParts == 1);
     Val = DAG.getNode(ISD::BITCAST, DL, PartVT, Val);
   } else if (NumParts * PartBits < ValueVT.getSizeInBits()) {
+    if (PartVT.isFloatingPoint() && ValueVT.isFloatingPoint()) {
+      assert(NumParts == 1 && "Do not know what to truncate to!");
+      Val = DAG.getFPExtendOrRound(Val, DL, PartVT);
+    } else {
+      if (ValueVT.isFloatingPoint()) {
+        // FP values need to be bitcast, then truncated if they are being put
+        // into a smaller container.
+        ValueVT = EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits());
+        Val = DAG.getNode(ISD::BITCAST, DL, ValueVT, Val);
+      }
       // If the parts cover less bits than value has, truncate the value.
       assert((PartVT.isInteger() || PartVT == MVT::x86mmx) &&
-           ValueVT.isInteger() &&
-           "Unknown mismatch!");
+             ValueVT.isInteger() && "Unknown mismatch!");
       ValueVT = EVT::getIntegerVT(*DAG.getContext(), NumParts * PartBits);
       Val = DAG.getNode(ISD::TRUNCATE, DL, ValueVT, Val);
       if (PartVT == MVT::x86mmx)
         Val = DAG.getNode(ISD::BITCAST, DL, PartVT, Val);
     }
+  }
 
   // The value may have changed - recompute ValueVT.
   ValueVT = Val.getValueType();
   assert(NumParts * PartBits == ValueVT.getSizeInBits() &&
          "Failed to tile the value with PartVT!");
 
   if (NumParts == 1) {
     if (PartEVT != ValueVT) {
static void getCopyToPartsVector(SelectionDAG &DAG, const SDLoc &DL,
                                  MVT PartVT, const Value *V,
                                  bool IsABIRegCopy) {
 
   EVT ValueVT = Val.getValueType();
   assert(ValueVT.isVector() && "Not a vector");
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
 
   if (NumParts == 1) {
+    if (PartVT == MVT::f80) {
+      if (!ValueVT.isFloatingPoint() ||
+          (PartVT.getSizeInBits() != ValueVT.getSizeInBits())) {
+        Val = DAG.getUNDEF(PartVT);
+        emitInvalidConstraintError(*DAG.getContext(), V,
+                                   "Inconsistent operand constraints");
+        Parts[0] = Val;
+        return;
+      }
+    }
     EVT PartEVT = PartVT;
     if (PartEVT == ValueVT) {
       // Nothing to do.
     } else if (PartVT.getSizeInBits() == ValueVT.getSizeInBits()) {
       // Bitconvert vector->vector case.
       Val = DAG.getNode(ISD::BITCAST, DL, PartVT, Val);
     } else if (PartVT.isVector() &&
                PartEVT.getVectorElementType() == ValueVT.getVectorElementType() &&

test/CodeGen/X86/asm-inconsistent-operand-constraints-fpreg-integer.ll

+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -O=0 < %s 2>&1 | FileCheck %s --check-prefix=CHECK
+
+; void test54(short s, int i, long l, long long ll, float f, double d, long double ld) {
+;    int b = 0;
+;  __asm__("" : "=r"(b) : "f"(f));
+;  __asm__("" : "=r"(b) : "f"(d));
+;  __asm__("" : "=r"(b) : "f"(ld));
+;  __asm__("" : "=r"(b) : "f"(s));
+;  __asm__("" : "=r"(b) : "f"(i));
+;  __asm__("" : "=r"(b) : "f"(l));
+;  __asm__("" : "=r"(b) : "f"(ll));
+;}
+
+; ModuleID = 'asm-inconsistent-operand-constraints-fpreg-integer.c'
+
+; CHECK:  error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define void @test54(i16 signext %s, i32 %i, i64 %l, i64 %ll, float %f, double %d, x86_fp80 %ld) #0 {
+entry:
+  %s.addr = alloca i16, align 2
+  %i.addr = alloca i32, align 4
+  %l.addr = alloca i64, align 8
+  %ll.addr = alloca i64, align 8
+  %f.addr = alloca float, align 4
+  %d.addr = alloca double, align 8
+  %ld.addr = alloca x86_fp80, align 16
+  %b = alloca i32, align 4
+  store i16 %s, i16* %s.addr, align 2
+  store i32 %i, i32* %i.addr, align 4
+  store i64 %l, i64* %l.addr, align 8
+  store i64 %ll, i64* %ll.addr, align 8
+  store float %f, float* %f.addr, align 4
+  store double %d, double* %d.addr, align 8
+  store x86_fp80 %ld, x86_fp80* %ld.addr, align 16
+  store i32 0, i32* %b, align 4
+  %0 = load float, float* %f.addr, align 4
+  %1 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(float %0) #1, !srcloc !2
+  store i32 %1, i32* %b, align 4
+  %2 = load double, double* %d.addr, align 8
+  %3 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(double %2) #1, !srcloc !3
+  store i32 %3, i32* %b, align 4
+  %4 = load x86_fp80, x86_fp80* %ld.addr, align 16
+  %5 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(x86_fp80 %4) #1, !srcloc !4
+  store i32 %5, i32* %b, align 4
+  %6 = load i16, i16* %s.addr, align 2
+  %7 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(i16 %6) #1, !srcloc !5
+  store i32 %7, i32* %b, align 4
+  %8 = load i32, i32* %i.addr, align 4
+  %9 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(i32 %8) #1, !srcloc !6
+  store i32 %9, i32* %b, align 4
+  %10 = load i64, i64* %l.addr, align 8
+  %11 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(i64 %10) #1, !srcloc !7
+  store i32 %11, i32* %b, align 4
+  %12 = load i64, i64* %ll.addr, align 8
+  %13 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(i64 %12) #1, !srcloc !8
+  store i32 %13, i32* %b, align 4
+  ret void
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 5.0.0 (trunk 306467)"}
+!2 = !{i32 113}
+!3 = !{i32 147}
+!4 = !{i32 181}
+!5 = !{i32 217}
+!6 = !{i32 251}
+!7 = !{i32 285}
+!8 = !{i32 319}

test/CodeGen/X86/asm-inconsistent-operand-constraints-fpreg-vector.ll

+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -O=0 < %s 2>&1 | FileCheck %s --check-prefix=CHECK
+
+; typedef long long __m128 __attribute__((__vector_size__(16)));
+; typedef short int __m64 __attribute__((__vector_size__(8)));
+;
+; typedef double __m128d __attribute__((__vector_size__(16)));
+; typedef long long __m128i __attribute__((__vector_size__(16)));
+;
+; typedef double __v2df __attribute__ ((__vector_size__ (16)));
+; typedef long long __v2di __attribute__ ((__vector_size__ (16)));
+; typedef short __v8hi __attribute__((__vector_size__(16)));
+; typedef char __v16qi __attribute__((__vector_size__(16)));
+; 
+; void test51(__m64 m64, __m128i m128, __m128d m128d, __v2df v2df, __v2di v2di, __v8hi v8hi, __v16qi v16qi ) {
+;   int b = 0;
+;   __asm__("" : "=r"(b) : "f"(m64));
+;   __asm__("" : "=r"(b) : "f"(m128));
+;   __asm__("" : "=r"(b) : "f"(m128d));
+;   __asm__("" : "=r"(b) : "f"(v2df));
+;   __asm__("" : "=r"(b) : "f"(v2di));
+;   __asm__("" : "=r"(b) : "f"(v8hi));
+;   __asm__("" : "=r"(b) : "f"(v16qi));
+; }
+
+; ModuleID = 'asm-inconsistent-operand-constraints-fpreg-vector.c'
+
+; CHECK:  error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+; CHECK-NEXT: error: Inconsistent operand constraints, in an __asm__ at line {{[0-9]*}}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define void @test51(double %m64.coerce, <2 x i64> %m128, <2 x double> %m128d, <2 x double> %v2df, <2 x i64> %v2di, <8 x i16> %v8hi, <16 x i8> %v16qi) #0 {
+entry:
+  %m64 = alloca <4 x i16>, align 8
+  %m64.addr = alloca <4 x i16>, align 8
+  %m128.addr = alloca <2 x i64>, align 16
+  %m128d.addr = alloca <2 x double>, align 16
+  %v2df.addr = alloca <2 x double>, align 16
+  %v2di.addr = alloca <2 x i64>, align 16
+  %v8hi.addr = alloca <8 x i16>, align 16
+  %v16qi.addr = alloca <16 x i8>, align 16
+  %b = alloca i32, align 4
+  %0 = bitcast <4 x i16>* %m64 to double*
+  store double %m64.coerce, double* %0, align 8
+  %m641 = load <4 x i16>, <4 x i16>* %m64, align 8
+  store <4 x i16> %m641, <4 x i16>* %m64.addr, align 8
+  store <2 x i64> %m128, <2 x i64>* %m128.addr, align 16
+  store <2 x double> %m128d, <2 x double>* %m128d.addr, align 16
+  store <2 x double> %v2df, <2 x double>* %v2df.addr, align 16
+  store <2 x i64> %v2di, <2 x i64>* %v2di.addr, align 16
+  store <8 x i16> %v8hi, <8 x i16>* %v8hi.addr, align 16
+  store <16 x i8> %v16qi, <16 x i8>* %v16qi.addr, align 16
+  store i32 0, i32* %b, align 4
+  %1 = load <4 x i16>, <4 x i16>* %m64.addr, align 8
+  %2 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<4 x i16> %1) #1, !srcloc !2
+  store i32 %2, i32* %b, align 4
+  %3 = load <2 x i64>, <2 x i64>* %m128.addr, align 16
+  %4 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<2 x i64> %3) #1, !srcloc !3
+  store i32 %4, i32* %b, align 4
+  %5 = load <2 x double>, <2 x double>* %m128d.addr, align 16
+  %6 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<2 x double> %5) #1, !srcloc !4
+  store i32 %6, i32* %b, align 4
+  %7 = load <2 x double>, <2 x double>* %v2df.addr, align 16
+  %8 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<2 x double> %7) #1, !srcloc !5
+  store i32 %8, i32* %b, align 4
+  %9 = load <2 x i64>, <2 x i64>* %v2di.addr, align 16
+  %10 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<2 x i64> %9) #1, !srcloc !6
+  store i32 %10, i32* %b, align 4
+  %11 = load <8 x i16>, <8 x i16>* %v8hi.addr, align 16
+  %12 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<8 x i16> %11) #1, !srcloc !7
+  store i32 %12, i32* %b, align 4
+  %13 = load <16 x i8>, <16 x i8>* %v16qi.addr, align 16
+  %14 = call i32 asm "", "=r,f,~{dirflag},~{fpsr},~{flags}"(<16 x i8> %13) #1, !srcloc !8
+  store i32 %14, i32* %b, align 4
+  ret void
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 5.0.0 (trunk 306467)"}
+!2 = !{i32 631}
+!3 = !{i32 667}
+!4 = !{i32 704}
+!5 = !{i32 742}
+!6 = !{i32 779}
+!7 = !{i32 816}
+!8 = !{i32 853}