[Driver, CodeGen] pass through and apply -fassociative-math

[spatel]

Summary

There are 2 parts to getting the -fassociative-math command-line flag translated to LLVM FMF:

1. In the driver/frontend, we accept the flag and its 'no' inverse and deal with the interactions with other flags like -ffast-math. This was mostly already done - we just needed to pass the flag on as a codegen option. The test file is complicated because there are many potential combinations of flags here.

2. In codegen, we map the option to FMF in the IR builder. This is simple code and corresponding test.

For the motivating example from PR27372:

float foo(float a, float x) { return ((a + x) - x); }

$ ./clang -O2 27372.c -S -o - -ffast-math  -fno-associative-math -emit-llvm  | egrep 'fadd|fsub'
  %add = fadd nnan ninf nsz arcp contract float %0, %1
  %sub = fsub nnan ninf nsz arcp contract float %add, %2

So 'reassoc' is off as expected (and so is the new 'afn' but that's a different patch). This case now works as expected end-to-end although the underlying logic is still wrong:

$ ./clang  -O2 27372.c -S -o - -ffast-math  -fno-associative-math | grep xmm
	addss	%xmm1, %xmm0
	subss	%xmm1, %xmm0

We're not done because the case where 'reassoc' is set is ignored by optimizer passes. Example:

$ ./clang  -O2 27372.c -S -o - -fassociative-math -emit-llvm  | grep fadd
  %add = fadd reassoc float %0, %1

$ ./clang -O2  27372.c -S -o - -fassociative-math | grep xmm
	addss	%xmm1, %xmm0
	subss	%xmm1, %xmm0

[spatel]

I just reviewed the gcc docs:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

"[-fassociative-math] requires that both -fno-signed-zeros and -fno-trapping-math be in effect."

If we want to match that behavior, I need to change this patch to light up 'nsz' and the no-trapping-math function attribute.

[wristow]

In D39812#919687, @spatel wrote:

I just reviewed the gcc docs:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

"[-fassociative-math] requires that both -fno-signed-zeros and -fno-trapping-math be in effect."

If we want to match that behavior, I need to change this patch to light up 'nsz' and the no-trapping-math function attribute.

Yes, I think we want to match that behavior. But by "light up" 'nsz' and the no-trapping-math attribute, do you mean automatically turn them on when '-fassociative-math' is specified? I'd think it should be the other way around: Suppress the effect of '-fassociative-math' unless both '-fno-signed-zeros' and '-fno-trapping-math' are also specified.

[spatel]

In D39812#919699, @wristow wrote:

Yes, I think we want to match that behavior. But by "light up" 'nsz' and the no-trapping-math attribute, do you mean automatically turn them on when '-fassociative-math' is specified? I'd think it should be the other way around: Suppress the effect of '-fassociative-math' unless both '-fno-signed-zeros' and '-fno-trapping-math' are also specified.

Yes, my reading of the gcc docs was off. The code appears to match your understanding:
https://godbolt.org/g/Vau3cv

So we'll restrict the effect of -fassociative-math based on the presence of the other flags...more spaghetti needed. :)

[spatel]

Patch updated:
Match gcc's handling of -fassociative-math with -fno-signed-zeros and -fno-trapping-math. I've created a new codegen option (-mreassociate) that maps directly to LLVM's 'reassoc' flag, so we limit the mix-and-match logic of these flags to the driver/frontend.

So now we have:

$ ./clang  27372.c -S -o -  -fassociative-math -O2 -emit-llvm | egrep 'fadd|fsub'
  %add = fadd float %a, %x
  %sub = fsub float %add, %x
$ ./clang  27372.c -S -o -  -fassociative-math -fno-signed-zeros -O2 -emit-llvm | egrep 'fadd|fsub'
  %add = fadd nsz float %a, %x
  %sub = fsub nsz float %add, %x
$ ./clang  27372.c -S -o -  -fassociative-math -fno-signed-zeros -fno-trapping-math -O2 -emit-llvm | egrep 'fadd|fsub'
  %add = fadd reassoc nsz float %a, %x
  %sub = fsub reassoc nsz float %add, %x

[spatel]

Updated again - the last upload was missing the driver's test file.

[spatel]

Ping.

[spatel]

Ping * 2.

[spatel]

Ping * 3.

[spatel]

Ping * 4.

[hfinkel]

LGTM

include/clang/Driver/Options.td

HelpText<"Strip (or keep only, if negative) a given number of path components "
            "when emitting check metadata.">;
 
 } // end -f[no-]sanitize* flags
 
 def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">,
   Group<f_Group>;
 def fno_unsafe_math_optimizations : Flag<["-"], "fno-unsafe-math-optimizations">,
   Group<f_Group>;
-def fassociative_math : Flag<["-"], "fassociative-math">, Group<f_Group>;
+def fassociative_math : Flag<["-"], "fassociative-math">, Group<f_Group>, Flags<[CC1Option]>;
 def fno_associative_math : Flag<["-"], "fno-associative-math">, Group<f_Group>;
 def freciprocal_math :
   Flag<["-"], "freciprocal-math">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Allow division operations to be reassociated">;
 def fno_reciprocal_math : Flag<["-"], "fno-reciprocal-math">, Group<f_Group>;
 def ffinite_math_only : Flag<["-"], "ffinite-math-only">, Group<f_Group>, Flags<[CC1Option]>;
 def fno_finite_math_only : Flag<["-"], "fno-finite-math-only">, Group<f_Group>;
 def fsigned_zeros : Flag<["-"], "fsigned-zeros">, Group<f_Group>;

include/clang/Frontend/CodeGenOptions.def

CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
                                        ///< enabled.
 CODEGENOPT(NoExecStack       , 1, 0) ///< Set when -Wa,--noexecstack is enabled.
 CODEGENOPT(FatalWarnings     , 1, 0) ///< Set when -Wa,--fatal-warnings is
                                      ///< enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is enabled.
 CODEGENOPT(NoInfsFPMath      , 1, 0) ///< Assume FP arguments, results not +-Inf.
 CODEGENOPT(NoSignedZeros     , 1, 0) ///< Allow ignoring the signedness of FP zero
+CODEGENOPT(AssociativeMath   , 1, 0) ///< Allow reassociation of FP math ops
 CODEGENOPT(ReciprocalMath    , 1, 0) ///< Allow FP divisions to be reassociated.
 CODEGENOPT(NoTrappingMath    , 1, 0) ///< Set when -fno-trapping-math is enabled.
 CODEGENOPT(NoNaNsFPMath      , 1, 0) ///< Assume FP arguments, results not NaN.
 CODEGENOPT(FlushDenorm       , 1, 0) ///< Allow FP denorm numbers to be flushed to zero
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
 CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
 /// \brief Method of Objective-C dispatch to use.
 ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)

lib/CodeGen/CodeGenFunction.cpp

if (CGM.getCodeGenOpts().NoNaNsFPMath) {
     FMF.setNoNaNs();
   }
   if (CGM.getCodeGenOpts().NoSignedZeros) {
     FMF.setNoSignedZeros();
   }
   if (CGM.getCodeGenOpts().ReciprocalMath) {
     FMF.setAllowReciprocal();
   }
+  if (CGM.getCodeGenOpts().AssociativeMath) {
+    FMF.setAllowReassoc();
+  }
   Builder.setFastMathFlags(FMF);
 }
 
 CodeGenFunction::~CodeGenFunction() {
   assert(LifetimeExtendedCleanupStack.empty() && "failed to emit a cleanup");
 
   // If there are any unclaimed block infos, go ahead and destroy them
   // now.  This can happen if IR-gen gets clever and skips evaluating

lib/Driver/ToolChains/Clang.cpp

static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
 
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
       !TrappingMath)
     CmdArgs.push_back("-menable-unsafe-fp-math");
 
   if (!SignedZeros)
     CmdArgs.push_back("-fno-signed-zeros");
 
+  if (AssociativeMath)
+    CmdArgs.push_back("-fassociative-math");
+
   if (ReciprocalMath)
     CmdArgs.push_back("-freciprocal-math");
 
   if (!TrappingMath)
     CmdArgs.push_back("-fno-trapping-math");
 
   if (!DenormalFPMath.empty())
     CmdArgs.push_back(

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.NoNaNsFPMath = (Args.hasArg(OPT_menable_no_nans) ||
                        Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
                        Args.hasArg(OPT_cl_finite_math_only) ||
                        Args.hasArg(OPT_cl_fast_relaxed_math));
   Opts.NoSignedZeros = (Args.hasArg(OPT_fno_signed_zeros) ||
                         Args.hasArg(OPT_cl_no_signed_zeros) ||
                         Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
                         Args.hasArg(OPT_cl_fast_relaxed_math));
+  Opts.AssociativeMath = Args.hasArg(OPT_fassociative_math);
   Opts.FlushDenorm = Args.hasArg(OPT_cl_denorms_are_zero);
   Opts.CorrectlyRoundedDivSqrt =
       Args.hasArg(OPT_cl_fp32_correctly_rounded_divide_sqrt);
   Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
   Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
   Opts.BackendOptions = Args.getAllArgValues(OPT_backend_option);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);

test/CodeGen/finite-math.c

 // RUN: %clang_cc1 -ffinite-math-only -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=FINITE
 // RUN: %clang_cc1 -fno-signed-zeros -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK  -check-prefix=NSZ
 // RUN: %clang_cc1 -freciprocal-math -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK  -check-prefix=RECIP
+// RUN: %clang_cc1 -fassociative-math -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK  -check-prefix=ASSOC
 
 float f0, f1, f2;
 
 void foo(void) {
   // CHECK-LABEL: define {{.*}}void @foo()
 
   // FINITE: fadd nnan ninf
   // NSZ: fadd nsz
   // RECIP: fadd arcp
+  // ASSOC: fadd reassoc
   f0 = f1 + f2;
 
   // CHECK: ret
 }
 

test/Driver/fast-math.c

 // RUN: %clang -### -fno-fast-math -fno-math-errno -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 //
 // RUN: %clang -### -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN:     -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
 // CHECK-UNSAFE-MATH: "-cc1"
 // CHECK-UNSAFE-MATH: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH: "-fassociative-math"
 //
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN:     -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH-UNSAFE-MATH %s
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-UNSAFE-MATH: "-menable-unsafe-fp-math"
-//
+// CHECK-NO-FAST-MATH-UNSAFE-MATH: "-fassociative-math"
+
+// The 2nd -fno-fast-math overrides -fassociative-math.
+
 // RUN: %clang -### -fno-fast-math -fno-math-errno -fassociative-math -freciprocal-math \
 // RUN:     -fno-fast-math -fno-signed-zeros -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH-NO-FAST-MATH %s
 // CHECK-UNSAFE-MATH-NO-FAST-MATH: "-cc1"
 // CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-UNSAFE-MATH-NO-FAST-MATH-NOT: "-fassociative-math"
 //
 // Check that various umbrella flags also enable these frontend options.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-INFS %s
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NANS %s
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
 // RUN: %clang -### -ffinite-math-only -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-INFS %s
 // RUN: %clang -### -ffinite-math-only -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NANS %s
 // RUN: %clang -### -funsafe-math-optimizations -fno-math-errno -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
 //
 // One umbrella flag is *really* weird and also changes the semantics of the
 // program by adding a special preprocessor macro. Check that the frontend flag
 // modeling this semantic change is provided. Also check that the flag is not
-// present if any of the optimization is disabled.
+// present if any of the optimizations are disabled.
 // RUN: %clang -### -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -ffinite-math-only \
 // RUN:     -fno-math-errno -ffp-contract=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // RUN: %clang -### -fno-honor-infinities -fno-honor-nans -fno-math-errno \
 // RUN:     -fassociative-math -freciprocal-math -fno-signed-zeros \
 // RUN:     -fno-trapping-math -ffp-contract=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
 // CHECK-FAST-MATH: "-cc1"
 // CHECK-FAST-MATH: "-ffast-math"
 // CHECK-FAST-MATH: "-ffinite-math-only"
 //
 // RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fno-finite-math-only -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fno-unsafe-math-optimizations -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fmath-errno -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -fno-associative-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH --check-prefix=CHECK-ASSOC-MATH %s
 // CHECK-NO-FAST-MATH: "-cc1"
 // CHECK-NO-FAST-MATH-NOT: "-ffast-math"
+// CHECK-ASSOC-MATH-NOT: "-fassociative-math"
 //
 // Check various means of disabling these flags, including disabling them after
 // they've been enabled via an umbrella flag.
 // RUN: %clang -### -fno-honor-infinities -fhonor-infinities -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NO-INFS %s
 // RUN: %clang -### -ffinite-math-only -fhonor-infinities -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NO-INFS %s
 // RUN: %clang -### -ffinite-math-only -fno-finite-math-only -c %s 2>&1 \

 // RUN: %clang -### -ffast-math -fhonor-nans -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NO-NANS %s
 // RUN: %clang -### -ffast-math -fno-finite-math-only -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-NO-NANS %s
 // CHECK-NO-NO-NANS: "-cc1"
 // CHECK-NO-NO-NANS-NOT: "-menable-no-nans"
 // CHECK-NO-NO-NANS-NOT: "-ffinite-math-only"
 // CHECK-NO-NO-NANS: "-o"
-//
+
+// A later inverted option overrides an earlier option.
+
 // RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
 // RUN:     -fno-trapping-math -fno-associative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN:     -fno-trapping-math -fno-reciprocal-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN:     -fno-trapping-math -fsigned-zeros -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
-// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
-// RUN:     -fno-trapping-math -ftrapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -funsafe-math-optimizations -fno-associative-math -c %s \
 // RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -funsafe-math-optimizations -fno-reciprocal-math -c %s \
 // RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -fsigned-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -fno-unsafe-math-optimizations \
 // RUN:     -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fno-associative-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // RUN: %clang -### -ffast-math -fno-reciprocal-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fsigned-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -ftrapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fno-unsafe-math-optimizations -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+
 // CHECK-NO-UNSAFE-MATH: "-cc1"
 // CHECK-NO-UNSAFE-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-NO_UNSAFE-MATH-NOT: "-fassociative-math"
 // CHECK-NO-UNSAFE-MATH: "-o"
-//
+
+// In these runs, associative math is allowed even though the unsafe umbrella option is not deployed.
+
+// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
+// RUN:     -fno-trapping-math -fno-reciprocal-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASSOC-NO-UNSAFE-MATH %s
+
+// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
+// RUN:     -fno-trapping-math -fsigned-zeros -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASSOC-NO-UNSAFE-MATH %s
+
+// RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
+// RUN:     -fno-trapping-math -ftrapping-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ASSOC-NO-UNSAFE-MATH %s
+
+// CHECK-ASSOC-NO-UNSAFE-MATH: "-cc1"
+// CHECK-ASSOC-NO-UNSAFE-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-ASSOC-NO_UNSAFE-MATH: "-fassociative-math"
+// CHECK-ASSOC-NO-UNSAFE-MATH-NOT: "-menable-unsafe-fp-math"
+// CHECK-ASSOC-NO-UNSAFE-MATH: "-o"
+
 // RUN: %clang -### -ftrapping-math -fno-trapping-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-TRAPPING-MATH %s
 // CHECK-NO-TRAPPING-MATH: "-fno-trapping-math"