[FPEnv] Fix complex operations in strictfp mode

[sepavloff]

Summary

Operations on floating-point complex data had incorrect FP attributes
in strictfp mode, because IRBuilder object was not synchronized with AST
node attributes.

[kpn]

Are we testing _Complex multiply or subtraction anywhere? I have a vague memory of multiply not working correctly.

[arsenm]

clang/test/CodeGen/complex-strictfp.c

8–10

So the tests are wrong but should no longer be fixed?

[kpn]

clang/test/CodeGen/complex-strictfp.c

8–10

The tests were wrong and are now correct. He left the comment to tell the future that if either "fpexcept.maytrap" or "round.tonearest" reappear then it means a regression in clang.

How about changing "all' to "any"? And change "wrong" to "clang bugs"? As in 'Any cases of "fpexcept.maytrap" in this test are clang bugs.'?

[sepavloff]

Updated comment in test as proposed by Kevin

[sepavloff]

In D141765#4059214, @kpn wrote:

Are we testing _Complex multiply or subtraction anywhere? I have a vague memory of multiply not working correctly.

There is a test clang/test/CodeGen/complex-math.c that checks complex operations including subtraction and multiplication. The check for multiplication is not thorough however. And only default FP environment is checked.

IIRC spec2017 contains a test that uses C99 complex numbers. If there were issues, I think they would be reported.

[sepavloff]

Use --implicit-check-not in test

[rjmccall]

Seems right to me.

[sepavloff]

Thanks!

clang/lib/CodeGen/CGExprComplex.cpp

llvm::Constant *Null =
                        llvm::Constant::getNullValue(CGF.ConvertType(Elem));
     return ComplexPairTy(Null, Null);
   }
 
   struct BinOpInfo {
     ComplexPairTy LHS;
     ComplexPairTy RHS;
     QualType Ty;  // Computation Type.
+    FPOptions FPFeatures;
   };
 
   BinOpInfo EmitBinOps(const BinaryOperator *E,
                        QualType PromotionTy = QualType());
   ComplexPairTy EmitPromoted(const Expr *E, QualType PromotionTy);
   ComplexPairTy EmitPromotedComplexOperand(const Expr *E, QualType PromotionTy);
   LValue EmitCompoundAssignLValue(const CompoundAssignOperator *E,
                                   ComplexPairTy (ComplexExprEmitter::*Func)
ComplexPairTy ComplexExprEmitter::VisitUnaryNot(const UnaryOperator *E) {
 
   return ComplexPairTy(Op.first, ResI);
 }
 
 ComplexPairTy ComplexExprEmitter::EmitBinAdd(const BinOpInfo &Op) {
   llvm::Value *ResR, *ResI;
 
   if (Op.LHS.first->getType()->isFloatingPointTy()) {
+    CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Op.FPFeatures);
     ResR = Builder.CreateFAdd(Op.LHS.first,  Op.RHS.first,  "add.r");
     if (Op.LHS.second && Op.RHS.second)
       ResI = Builder.CreateFAdd(Op.LHS.second, Op.RHS.second, "add.i");
     else
       ResI = Op.LHS.second ? Op.LHS.second : Op.RHS.second;
     assert(ResI && "Only one operand may be real!");
   } else {
     ResR = Builder.CreateAdd(Op.LHS.first,  Op.RHS.first,  "add.r");
     assert(Op.LHS.second && Op.RHS.second &&
            "Both operands of integer complex operators must be complex!");
     ResI = Builder.CreateAdd(Op.LHS.second, Op.RHS.second, "add.i");
   }
   return ComplexPairTy(ResR, ResI);
 }
 
 ComplexPairTy ComplexExprEmitter::EmitBinSub(const BinOpInfo &Op) {
   llvm::Value *ResR, *ResI;
   if (Op.LHS.first->getType()->isFloatingPointTy()) {
+    CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Op.FPFeatures);
     ResR = Builder.CreateFSub(Op.LHS.first, Op.RHS.first, "sub.r");
     if (Op.LHS.second && Op.RHS.second)
       ResI = Builder.CreateFSub(Op.LHS.second, Op.RHS.second, "sub.i");
     else
       ResI = Op.LHS.second ? Op.LHS.second
                            : Builder.CreateFNeg(Op.RHS.second, "sub.i");
     assert(ResI && "Only one operand may be real!");
   } else {
if (Op.LHS.first->getType()->isFloatingPointTy()) {
     // The general formulation is:
     // (a + ib) * (c + id) = (a * c - b * d) + i(a * d + b * c)
     //
     // But we can fold away components which would be zero due to a real
     // operand according to C11 Annex G.5.1p2.
     // FIXME: C11 also provides for imaginary types which would allow folding
     // still more of this within the type system.
 
+    CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Op.FPFeatures);
     if (Op.LHS.second && Op.RHS.second) {
       // If both operands are complex, emit the core math directly, and then
       // test for NaNs. If we find NaNs in the result, we delegate to a libcall
       // to carefully re-compute the correct infinity representation if
       // possible. The expectation is that the presence of NaNs here is
       // *extremely* rare, and so the cost of the libcall is almost irrelevant.
       // This is good, because the libcall re-computes the core multiplication
       // exactly the same as we do here and re-tests for NaNs in order to be
ComplexPairTy ComplexExprEmitter::EmitBinDiv(const BinOpInfo &Op) {
   if (LHSr->getType()->isFloatingPointTy()) {
     // If we have a complex operand on the RHS and FastMath is not allowed, we
     // delegate to a libcall to handle all of the complexities and minimize
     // underflow/overflow cases. When FastMath is allowed we construct the
     // divide inline using the same algorithm as for integer operands.
     //
     // FIXME: We would be able to avoid the libcall in many places if we
     // supported imaginary types in addition to complex types.
+    CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Op.FPFeatures);
     if (RHSi && !CGF.getLangOpts().FastMath) {
       BinOpInfo LibCallOp = Op;
       // If LHS was a real, supply a null imaginary part.
       if (!LHSi)
         LibCallOp.LHS.second = llvm::Constant::getNullValue(LHSr->getType());
 
       switch (LHSr->getType()->getTypeID()) {
       default:
ComplexExprEmitter::EmitBinOps(const BinaryOperator *E,
   BinOpInfo Ops;
 
   Ops.LHS = EmitPromotedComplexOperand(E->getLHS(), PromotionType);
   Ops.RHS = EmitPromotedComplexOperand(E->getRHS(), PromotionType);
   if (!PromotionType.isNull())
     Ops.Ty = PromotionType;
   else
     Ops.Ty = E->getType();
+  Ops.FPFeatures = E->getFPFeaturesInEffect(CGF.getLangOpts());
   return Ops;
 }
 
 
 LValue ComplexExprEmitter::
 EmitCompoundAssignLValue(const CompoundAssignOperator *E,
           ComplexPairTy (ComplexExprEmitter::*Func)(const BinOpInfo&),
                          RValue &Val) {
   TestAndClearIgnoreReal();
   TestAndClearIgnoreImag();
   QualType LHSTy = E->getLHS()->getType();
   if (const AtomicType *AT = LHSTy->getAs<AtomicType>())
     LHSTy = AT->getValueType();
 
-  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
   BinOpInfo OpInfo;
+  OpInfo.FPFeatures = E->getFPFeaturesInEffect(CGF.getLangOpts());
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
 
   // Load the RHS and LHS operands.
   // __block variables need to have the rhs evaluated first, plus this should
   // improve codegen a little.
   QualType PromotionTypeCR;
   PromotionTypeCR = getPromotionType(E->getComputationResultType());
   if (PromotionTypeCR.isNull())
     PromotionTypeCR = E->getComputationResultType();

clang/test/CodeGen/complex-strictfp.c

 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck --implicit-check-not=fpexcept.maytrap --implicit-check-not=round.tonearest %s
 
 
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
 // Include rounding metadata in the testing.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
-// FIXME: All cases of "round.tonearest" in this test are wrong.
+// Any cases of "fpexcept.maytrap" in this test are clang bugs.
+// Any cases of "round.tonearest" in this test are clang bugs.
 
 #pragma float_control(except, on)
 #pragma STDC FENV_ROUND FE_UPWARD
 
 _Complex double g1, g2;
 _Complex float cf;
 double D;
 
-// CHECK-LABEL: define {{[^@]+}}@test3a(
+// CHECK-LABEL: @test3a(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr @D, align 8
 // CHECK-NEXT:    [[CF_REAL:%.*]] = load float, ptr @cf, align 4
 // CHECK-NEXT:    [[CF_IMAG:%.*]] = load float, ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    [[CONV:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_REAL]], metadata !"fpexcept.strict") #[[ATTR2:[0-9]+]]
 // CHECK-NEXT:    [[CONV1:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_IMAG]], metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[CONV]], double [[TMP0]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CONV2:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double [[ADD_R]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CONV3:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double [[CONV1]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store float [[CONV2]], ptr @cf, align 4
 // CHECK-NEXT:    store float [[CONV3]], ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    ret void
 //
 void test3a(void) {
   cf += D;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3b(
+// CHECK-LABEL: @test3b(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[CF_REAL:%.*]] = load float, ptr @cf, align 4
 // CHECK-NEXT:    [[CF_IMAG:%.*]] = load float, ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    [[CONV:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_REAL]], metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CONV1:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_IMAG]], metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr @D, align 8
 // CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[TMP0]], double [[CONV]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store double [[ADD_R]], ptr @D, align 8
 // CHECK-NEXT:    ret void
 //
 void test3b(void) {
   D += cf;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3c(
+// CHECK-LABEL: @test3c(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:    [[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:    [[CF_REAL:%.*]] = load float, ptr @cf, align 4
 // CHECK-NEXT:    [[CF_IMAG:%.*]] = load float, ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    [[CONV:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_REAL]], metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CONV1:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float [[CF_IMAG]], metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CALL:%.*]] = call { double, double } @__divdc3(double noundef [[CONV]], double noundef [[CONV1]], double noundef [[G1_REAL]], double noundef [[G1_IMAG]]) #[[ATTR3:[0-9]+]]
 // CHECK-NEXT:    [[TMP0:%.*]] = extractvalue { double, double } [[CALL]], 0
 // CHECK-NEXT:    [[TMP1:%.*]] = extractvalue { double, double } [[CALL]], 1
 // CHECK-NEXT:    [[CONV2:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double [[TMP0]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    [[CONV3:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double [[TMP1]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store float [[CONV2]], ptr @cf, align 4
 // CHECK-NEXT:    store float [[CONV3]], ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    ret void
 //
 void test3c(void) {
   cf /= g1;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3d(
+// CHECK-LABEL: @test3d(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:    [[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr @D, align 8
-// CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[G1_REAL]], double [[TMP0]], metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR2]]
+// CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[G1_REAL]], double [[TMP0]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store double [[ADD_R]], ptr @g1, align 8
 // CHECK-NEXT:    store double [[G1_IMAG]], ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:    ret void
 //
 void test3d(void) {
   g1 = g1 + D;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3e(
+// CHECK-LABEL: @test3e(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr @D, align 8
 // CHECK-NEXT:    [[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:    [[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
-// CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[TMP0]], double [[G1_REAL]], metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR2]]
+// CHECK-NEXT:    [[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[TMP0]], double [[G1_REAL]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store double [[ADD_R]], ptr @g1, align 8
 // CHECK-NEXT:    store double [[G1_IMAG]], ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:    ret void
 //
 void test3e(void) {
   g1 = D + g1;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t1(
+// CHECK-LABEL: @t1(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[CONV:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double 4.000000e+00, metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store float [[CONV]], ptr @cf, align 4
 // CHECK-NEXT:    ret void
 //
 void t1(void) {
   (__real__ cf) = 4.0;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t2(
+// CHECK-LABEL: @t2(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[CONV:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double 4.000000e+00, metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    store float [[CONV]], ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
 // CHECK-NEXT:    ret void
 //
 void t2(void) {
   (__imag__ cf) = 4.0;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t91(
+// CHECK-LABEL: @t91(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[C:%.*]] = alloca [0 x i8], align 1
 // CHECK-NEXT:    br i1 false, label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
 // CHECK:       cond.true:
 // CHECK-NEXT:    [[CONV:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float 2.000000e+00, metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    br label [[COND_END:%.*]]
 // CHECK:       cond.false:
 // CHECK-NEXT:    [[CONV1:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float 2.000000e+00, metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    br label [[COND_END]]
 // CHECK:       cond.end:
 // CHECK-NEXT:    [[COND_R:%.*]] = phi double [ [[CONV]], [[COND_TRUE]] ], [ [[CONV1]], [[COND_FALSE]] ]
 // CHECK-NEXT:    [[COND_I:%.*]] = phi double [ 0.000000e+00, [[COND_TRUE]] ], [ 0.000000e+00, [[COND_FALSE]] ]
 // CHECK-NEXT:    ret void
 //
 void t91(void) {
   // Check for proper type promotion of conditional expression
   char c[(int)(sizeof(typeof((0 ? 2.0f : (_Complex double) 2.0f))) - sizeof(_Complex double))];
   // Check for proper codegen
   (0 ? 2.0f : (_Complex double) 2.0f);
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t92(
+// CHECK-LABEL: @t92(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[C:%.*]] = alloca [0 x i8], align 1
 // CHECK-NEXT:    br i1 false, label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
 // CHECK:       cond.true:
 // CHECK-NEXT:    [[CONV:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float 2.000000e+00, metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:    br label [[COND_END:%.*]]
 // CHECK:       cond.false:
 // CHECK-NEXT:    [[CONV1:%.*]] = call double @llvm.experimental.constrained.fpext.f64.f32(float 2.000000e+00, metadata !"fpexcept.strict") #[[ATTR2]]