[FPEnv][WIP] Verify non-mixing of strictfp instructions and regular FP instructions

[kpn]

Summary

This patch, which should be applied on _top_ of D68233, adds checking for illegal mixing of strictfp and regular FP instructions. Because regular FP instructions lack the ordering constraints of strictfp the mixing of them in a single function is forbidden.

This is not ready to commit as it needs polish. It will also probably cause test failures that need to be addressed first. I'm opening this ticket now to facilitate finding those test failures.

llvm/lib/IR/Verifier.cpp

class Verifier : public InstVisitor<Verifier>, VerifierSupport {
   /// Whether we've seen a call to @llvm.localescape in this function
   /// already.
   bool SawFrameEscape;
 
   /// Whether or not we've seen constrained floating-point intrinsics
   /// in a function.
   bool HasConstrainedFP;
 
+  /// Whether or not we've seen normal floating-point instructions in
+  /// a function.
+  bool HasUnconstrainedFP;
+
   /// Whether the current function has a DISubprogram attached to it.
   bool HasDebugInfo = false;
 
   /// The current source language.
   dwarf::SourceLanguage CurrentSourceLang = dwarf::DW_LANG_lo_user;
 
   /// Whether source was present on the first DIFile encountered in each CU.
   DenseMap<const DICompileUnit *, bool> HasSourceDebugInfo;
for (const auto &Pair : SiblingFuncletInfo) {
     Active.clear();
   }
 }
 
 // visitFunction - Verify that a function is ok.
 //
 void Verifier::visitFunction(const Function &F) {
   HasConstrainedFP = false;
+  HasUnconstrainedFP = false;
 
   visitGlobalValue(F);
 
   // Check function arguments.
   FunctionType *FT = F.getFunctionType();
   unsigned NumArgs = F.arg_size();
 
   Assert(&Context == &F.getContext(),
void Verifier::visitFunction(const Function &F) {
 
   // Verify that the function has the correct attributes if a constrained
   // floating-point value was seen.
   if (HasConstrainedFP)
     Assert (F.hasFnAttribute(Attribute::StrictFP),
             "Constrained floating point requires function attribute strictfp!",
             &F);
 
+  if (HasUnconstrainedFP && HasConstrainedFP) // XXX different why?
+    CheckFailed(
+      "Constrained and normal floating-point cannot be mixed in a function!");
+
   auto *N = F.getSubprogram();
   HasDebugInfo = (N != nullptr);
   if (!HasDebugInfo)
     return;
 
   // Check that all !dbg attachments lead to back to N.
   //
   // FIXME: Check this incrementally while visiting !dbg attachments.
void Verifier::visitFPTruncInst(FPTruncInst &I) {
   unsigned DestBitSize = DestTy->getScalarSizeInBits();
 
   Assert(SrcTy->isFPOrFPVectorTy(), "FPTrunc only operates on FP", &I);
   Assert(DestTy->isFPOrFPVectorTy(), "FPTrunc only produces an FP", &I);
   Assert(SrcTy->isVectorTy() == DestTy->isVectorTy(),
          "fptrunc source and destination must both be a vector or neither", &I);
   Assert(SrcBitSize > DestBitSize, "DestTy too big for FPTrunc", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitFPExtInst(FPExtInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
   // Get the size of the types in bits, we'll need this later
   unsigned SrcBitSize = SrcTy->getScalarSizeInBits();
   unsigned DestBitSize = DestTy->getScalarSizeInBits();
 
   Assert(SrcTy->isFPOrFPVectorTy(), "FPExt only operates on FP", &I);
   Assert(DestTy->isFPOrFPVectorTy(), "FPExt only produces an FP", &I);
   Assert(SrcTy->isVectorTy() == DestTy->isVectorTy(),
          "fpext source and destination must both be a vector or neither", &I);
   Assert(SrcBitSize < DestBitSize, "DestTy too small for FPExt", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitUIToFPInst(UIToFPInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
   bool SrcVec = SrcTy->isVectorTy();
   bool DstVec = DestTy->isVectorTy();
 
   Assert(SrcVec == DstVec,
          "UIToFP source and dest must both be vector or scalar", &I);
   Assert(SrcTy->isIntOrIntVectorTy(),
          "UIToFP source must be integer or integer vector", &I);
   Assert(DestTy->isFPOrFPVectorTy(), "UIToFP result must be FP or FP vector",
          &I);
 
   if (SrcVec && DstVec)
     Assert(cast<VectorType>(SrcTy)->getElementCount() ==
                cast<VectorType>(DestTy)->getElementCount(),
            "UIToFP source and dest vector length mismatch", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitSIToFPInst(SIToFPInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
   bool SrcVec = SrcTy->isVectorTy();
   bool DstVec = DestTy->isVectorTy();
 
   Assert(SrcVec == DstVec,
          "SIToFP source and dest must both be vector or scalar", &I);
   Assert(SrcTy->isIntOrIntVectorTy(),
          "SIToFP source must be integer or integer vector", &I);
   Assert(DestTy->isFPOrFPVectorTy(), "SIToFP result must be FP or FP vector",
          &I);
 
   if (SrcVec && DstVec)
     Assert(cast<VectorType>(SrcTy)->getElementCount() ==
                cast<VectorType>(DestTy)->getElementCount(),
            "SIToFP source and dest vector length mismatch", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitFPToUIInst(FPToUIInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
   bool SrcVec = SrcTy->isVectorTy();
   bool DstVec = DestTy->isVectorTy();
 
   Assert(SrcVec == DstVec,
          "FPToUI source and dest must both be vector or scalar", &I);
   Assert(SrcTy->isFPOrFPVectorTy(), "FPToUI source must be FP or FP vector",
          &I);
   Assert(DestTy->isIntOrIntVectorTy(),
          "FPToUI result must be integer or integer vector", &I);
 
   if (SrcVec && DstVec)
     Assert(cast<VectorType>(SrcTy)->getElementCount() ==
                cast<VectorType>(DestTy)->getElementCount(),
            "FPToUI source and dest vector length mismatch", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitFPToSIInst(FPToSIInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
   bool SrcVec = SrcTy->isVectorTy();
   bool DstVec = DestTy->isVectorTy();
 
   Assert(SrcVec == DstVec,
          "FPToSI source and dest must both be vector or scalar", &I);
   Assert(SrcTy->isFPOrFPVectorTy(), "FPToSI source must be FP or FP vector",
          &I);
   Assert(DestTy->isIntOrIntVectorTy(),
          "FPToSI result must be integer or integer vector", &I);
 
   if (SrcVec && DstVec)
     Assert(cast<VectorType>(SrcTy)->getElementCount() ==
                cast<VectorType>(DestTy)->getElementCount(),
            "FPToSI source and dest vector length mismatch", &I);
 
+  HasUnconstrainedFP = true;
+
   visitInstruction(I);
 }
 
 void Verifier::visitPtrToIntInst(PtrToIntInst &I) {
   // Get the source and destination types
   Type *SrcTy = I.getOperand(0)->getType();
   Type *DestTy = I.getType();
 
Assert(U.getType() == U.getOperand(0)->getType(),
          "Unary operators must have same type for"
          "operands and result!",
          &U);
 
   switch (U.getOpcode()) {
   // Check that floating-point arithmetic operators are only used with
   // floating-point operands.
   case Instruction::FNeg:
+    // No constrained version, so no need to set any flags here.
     Assert(U.getType()->isFPOrFPVectorTy(),
            "FNeg operator only works with float types!", &U);
     break;
   default:
     llvm_unreachable("Unknown UnaryOperator opcode!");
   }
 
   visitInstruction(U);
case Instruction::FRem:
     Assert(B.getType()->isFPOrFPVectorTy(),
            "Floating-point arithmetic operators only work with "
            "floating-point types!",
            &B);
     Assert(B.getType() == B.getOperand(0)->getType(),
            "Floating-point arithmetic operators must have same type "
            "for operands and result!",
            &B);
+    HasUnconstrainedFP = true;
     break;
   // Check that logical operators are only used with integral operands.
   case Instruction::And:
   case Instruction::Or:
   case Instruction::Xor:
     Assert(B.getType()->isIntOrIntVectorTy(),
            "Logical operators only work with integral types!", &B);
     Assert(B.getType() == B.getOperand(0)->getType(),
Assert(Op0Ty == Op1Ty,
          "Both operands to FCmp instruction are not of the same type!", &FC);
   // Check that the operands are the right type
   Assert(Op0Ty->isFPOrFPVectorTy(),
          "Invalid operand types for FCmp instruction", &FC);
   // Check that the predicate is valid.
   Assert(FC.isFPPredicate(),
          "Invalid predicate in FCmp instruction!", &FC);
 
+  HasUnconstrainedFP = true; // XXX what about select?
+
   visitInstruction(FC);
 }
 
 void Verifier::visitExtractElementInst(ExtractElementInst &EI) {
   Assert(
       ExtractElementInst::isValidOperands(EI.getOperand(0), EI.getOperand(1)),
       "Invalid extractelement operands!", &EI);
   visitInstruction(EI);

llvm/test/Verifier/fp-intrinsics.ll

 ; RUN: opt -verify -S < %s 2>&1 | FileCheck --check-prefix=CHECK1 %s
 ; RUN: sed -e s/.T2:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK2 %s
 ; RUN: sed -e s/.T3:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK3 %s
 ; RUN: sed -e s/.T4:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK4 %s
 ; RUN: sed -e s/.T5:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK5 %s
 ; RUN: sed -e s/.T6:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK6 %s
 ; RUN: sed -e s/.T7:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK7 %s
 ; RUN: sed -e s/.T7I:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK7I %s
 ; RUN: sed -e s/.T8:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK8 %s
+; RUN: sed -e s/.T9:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK9 %s
+; RUN: sed -e s/.T10:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK10 %s
+; RUN: sed -e s/.T11:// %s | not opt -verify -disable-output 2>&1 | FileCheck --check-prefix=CHECK11 %s
+; RUN: sed -e s/.T12:// %s | opt -verify -disable-output 2>&1
 
 ; Common declarations used for all runs.
 declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata)
 declare float @llvm.experimental.constrained.fadd.f32(float, float, metadata, metadata)
 declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata)
+declare float @llvm.experimental.constrained.fptrunc.f32.f64(double, metadata, metadata)
+declare double @llvm.experimental.constrained.fpext.f64.f32(float, metadata)
 
 ; Test that the verifier accepts legal code, and that the correct attributes are
 ; attached to the FP intrinsic.
 ; CHECK1: declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata) #[[ATTR:[0-9]+]]
 ; CHECK1: declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #[[ATTR]]
 ; CHECK1: attributes #[[ATTR]] = { inaccessiblememonly nofree nosync nounwind willreturn }
 ; Note: FP exceptions aren't usually caught through normal unwind mechanisms,
 ;       but we may want to revisit this for asynchronous exception handling.

 ;T6:   ret double %fadd
 ;T6: }
 
 ; Test for mismatched function and function call attributes
 ; CHECK7: Functions and their contained calls and invokes must match in use of attribute strictfp!
 ;T7: declare double @llvm.sqrt.f64(double)
 ;T7: define double @f7(double %a) #0 {
 ;T7: entry:
-;T7:   %sqrt = call double @llvm.sqrt.f64(double %a)
-;T7:   ret double %sqrt
+;T7:   %fadd = call double @llvm.experimental.constrained.sqrt.f64(
+;T7:                                           double %a,
+;T7:                                           metadata !"round.dynamic",
+;T7:                                           metadata !"fpexcept.strict") #0
+;T7:   ret double %fadd
 ;T7: }
 
 ; Test for mismatched function and (invoke) function call attributes
 ; CHECK7I: Functions and their contained calls and invokes must match in use of attribute strictfp!
 ;T7I: declare double @doublefoo(double)
 ;T7I: define void @f7i(double %a) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) #0 {
 ;T7I: entry:
 ;T7I:   invoke void @doublefoo(%a)
 ;T7I:   to label %conta unwind label %contb
-;T7I: 
+;T7I:
 ;T7I: conta:
 ;T7I:   ret void
-;T7I: 
+;T7I:
 ;T7I: contb:
 ;T7I:   %0 = landingpad { i8*, i32 }
 ;T7I:           filter [0 x i8*] zeroinitializer
 ;T7I:   ret void
 ;T7I: }
 
 ; Test for mismatched function and function call attributes
 ; CHECK8: Functions and their contained calls and invokes must match in use of attribute strictfp!
 ;T8: define double @f8(double %a) {
 ;T8: entry:
-;T8:   %fadd = call double @llvm.experimental.constrained.sqrt.f64(
-;T8:                                           double %a,
-;T8:                                           metadata !"round.dynamic",
-;T8:                                           metadata !"fpexcept.strict") #0
-;T8:   ret double %fadd
+;T8:   %fadd = call double @llvm.experimental.constrained.fadd.f64(
+;T8:                                         double %a, double %b,
+;T8:                                         metadata !"round.dynamic",
+;T8:                                         metadata !"fpexcept.strict") #0
+;T8:   %fadd2 = fadd double %fadd, %c
+;T8:
+;T8:   ret double %fadd2
 ;T8: }
 
+; Test for illegal mixing of constrained and non-constrained fp math.
+; Test that the order of the instructions does not affect the test.
+; CHECK9: Constrained and normal floating-point cannot be mixed in a function!
+;T9: define double @f9(double %a, double %b, double %c) #0 {
+;T9: entry:
+;T9:   %fadd = fadd double %a, %c
+;T9:
+;T9:   %fadd2 = call double @llvm.experimental.constrained.fadd.f64(
+;T9:                                         double %fadd, double %b,
+;T9:                                         metadata !"round.dynamic",
+;T9:                                         metadata !"fpexcept.strict") #0
+;T9:
+;T9:   ret double %fadd2
+;T9: }
+
+; Test for illegal mixing of constrained and non-constrained fp math.
+; Test that truncating instructions are properly tested.
+; CHECK10: Constrained and normal floating-point cannot be mixed in a function!
+;T10: define float @f10(double %a, float %b) #0 {
+;T10: entry:
+;T10:   %fpval = fptrunc double %a to float;
+;T10:
+;T10:   %fpval2 = call float @llvm.experimental.constrained.fadd.f32(
+;T10:                                         float %fpval, float %b,
+;T10:                                         metadata !"round.dynamic",
+;T10:                                         metadata !"fpexcept.strict") #0
+;T10:
+;T10:   ret float %fpval2
+;T10: }
+
+; Test for illegal mixing of constrained and non-constrained fp math.
+; Test that extending intructions are properly tested.
+; CHECK11: Constrained and normal floating-point cannot be mixed in a function!
+;T11: define double @f11(float %a, double %b) #0 {
+;T11: entry:
+;T11:   %fpval = fpext float %a to double;
+;T11:
+;T11:   %fpval2 = call double @llvm.experimental.constrained.fadd.f64(
+;T11:                                         double %fpval, double %b,
+;T11:                                         metadata !"round.dynamic",
+;T11:                                         metadata !"fpexcept.strict") #0
+;T11:
+;T11:   ret double %fpval2
+;T11: }
+
+; Test for illegal mixing of constrained and non-constrained fp math.
+; Test that fneg operations are NOT tested.
+; CHECK12: xxx
+;T12: define double @f12(double %a, double %b) #0 {
+;T12: entry:
+;T12:   %fneg = fneg double %a
+;T12:
+;T12:   %fneg2 = call double @llvm.experimental.constrained.fadd.f64(
+;T12:                                         double %fneg, double %b,
+;T12:                                         metadata !"round.dynamic",
+;T12:                                         metadata !"fpexcept.strict") #0
+;T12:
+;T12:   ret double %fneg2
+;T12: }
+
+
 attributes #0 = { strictfp }