[Driver, CodeGen] rename options to disable an FP cast optimization

[spatel]

Summary

As suggested in the post-commit thread for rL331056, we should match these clang options with the established vocabulary of the corresponding sanitizer option. Also, the use of 'strict' is well-known for these kinds of knobs, and we can improve the descriptive text in the docs.

So this intends to match the logic of D46135 but only change the words. I'll post the matching LLVM patch next and link it to this.

[spatel]

Patch updated:
I missed a spot in the release notes - forgot to delete 'workaround' when listing the new options.

[lebedev.ri]

Overall makes sense to me.

docs/ReleaseNotes.rst

94

- in a casted-to integer type
+ in destination integer type

?

96

s/event/case/ ?

docs/UsersManual.rst

1260–1264

same

1262

same

include/clang/Driver/Options.td

1033

Maybe add HelpText<""> to both of them, just to explain what they are,
specify that the first is the default, and the second relaxes language rules to help broken code.

test/CodeGen/no-junk-ftrunc.c

7

It wouldn't hurt to duplicate this run-line, and explicitly pass -fno-strict-float-cast-overflow.

[spatel]

Patch updated:

1. Improved language in docs.
2. Added help text for clang options.
3. Added driver test with -fstrict-float-cast-overflow specified explicitly.

I'll leave this up for further review in case there are other suggestions.

[rsmith]

docs/ReleaseNotes.rst

96

Add "By default" to the start of this sentence.

[spatel]

Patch updated:
Added "By default" to the description in the user manual and release notes to make it clearer how this behaves.

docs/ReleaseNotes.rst

- Clang's handling of the GCC ``packed`` class attribute in C++ has been fixed
   Clang 7 and earlier versions. The old behavior can be restored by setting
   ``-fclang-abi-compat`` to ``6`` or earlier.
 
 - ...
 
 New Compiler Flags
 ------------------
 
-- :option:`-ffp-cast-overflow-workaround` and
-  :option:`-fno-fp-cast-overflow-workaround`
-  enable (disable) a workaround for code that casts floating-point values to
-  integers and back to floating-point. If the floating-point value is not
-  representable in the intermediate integer type, the code is incorrect
-  according to the language standard. This flag will attempt to generate code
-  as if the result of an overflowing conversion matches the overflowing behavior
-  of a target's native float-to-int conversion instructions.
+- :option:`-fstrict-float-cast-overflow-workaround` and
+  :option:`-fno-strict-float-cast-overflow-workaround` -
+   When a floating-point value is not representable in a casted-to integer
+   type, the code has undefined behavior according to the language standard.
+   Clang will not guarantee any particular result in that event. With the
+   'no-strict' option, Clang attempts to match the overflowing behavior of
+   the target's native float-to-int conversion instructions.
 
 - ...
 
 Deprecated Compiler Flags
 -------------------------
 
 The following options are deprecated and ignored. They will be removed in
 future versions of Clang.

docs/UsersManual.rst

.. option:: -fdenormal-fp-math=[values]
 
    Select which denormal numbers the code is permitted to require.
 
    Valid values are: ``ieee``, ``preserve-sign``, and ``positive-zero``,
    which correspond to IEEE 754 denormal numbers, the sign of a
    flushed-to-zero number is preserved in the sign of 0, denormals are
    flushed to positive zero, respectively.
 
-.. option:: -f[no-]fp-cast-overflow-workaround
+.. option:: -f[no-]strict-float-cast-overflow
 
-   Enable a workaround for code that casts floating-point values to 
-   integers and back to floating-point. If the floating-point value 
-   is not representable in the intermediate integer type, the code is
-   incorrect according to the language standard. This flag will attempt 
-   to generate code as if the result of an overflowing conversion matches
-   the overflowing behavior of a target's native float-to-int conversion
-   instructions.
+   When a floating-point value is not representable in a casted-to integer 
+   type, the code has undefined behavior according to the language standard.
+   Clang will not guarantee any particular result in that event. With the
+   'no-strict' option, Clang attempts to match the overflowing behavior of 
+   the target's native float-to-int conversion instructions.
 
 .. option:: -fwhole-program-vtables
 
    Enable whole-program vtable optimizations, such as single-implementation
    devirtualization and virtual constant propagation, for classes with
    :doc:`hidden LTO visibility <LTOVisibility>`. Requires ``-flto``.
 
 .. option:: -fno-assume-sane-operator-new

include/clang/Driver/Options.td

 def : Flag<["-"], "fhonor-infinites">, Alias<fhonor_infinities>;
 def : Flag<["-"], "fno-honor-infinites">, Alias<fno_honor_infinities>;
 def ftrapping_math : Flag<["-"], "ftrapping-math">, Group<f_Group>, Flags<[CC1Option]>;
 def fno_trapping_math : Flag<["-"], "fno-trapping-math">, Group<f_Group>, Flags<[CC1Option]>;
 def ffp_contract : Joined<["-"], "ffp-contract=">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Form fused FP ops (e.g. FMAs): fast (everywhere)"
   " | on (according to FP_CONTRACT pragma, default) | off (never fuse)">, Values<"fast,on,off">;
 
-def ffp_cast_overflow_workaround : Flag<["-"],
-  "ffp-cast-overflow-workaround">, Group<f_Group>, Flags<[CC1Option]>;
-def fno_fp_cast_overflow_workaround : Flag<["-"],
-  "fno-fp-cast-overflow-workaround">, Group<f_Group>, Flags<[CC1Option]>;
+def fstrict_float_cast_overflow : Flag<["-"],
+  "fstrict-float-cast-overflow">, Group<f_Group>, Flags<[CC1Option]>;
+def fno_strict_float_cast_overflow : Flag<["-"],
+  "fno-strict-float-cast-overflow">, Group<f_Group>, Flags<[CC1Option]>;
 
 def ffor_scope : Flag<["-"], "ffor-scope">, Group<f_Group>;
 def fno_for_scope : Flag<["-"], "fno-for-scope">, Group<f_Group>;
 
 def frewrite_includes : Flag<["-"], "frewrite-includes">, Group<f_Group>,
   Flags<[CC1Option]>;
 def fno_rewrite_includes : Flag<["-"], "fno-rewrite-includes">, Group<f_Group>;
 

include/clang/Frontend/CodeGenOptions.def

 CODEGENOPT(NoSignedZeros     , 1, 0) ///< Allow ignoring the signedness of FP zero
 CODEGENOPT(Reassociate       , 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
 
-/// Disable a float-to-int-to-float cast optimization. This attempts to generate
-/// code as if the result of an overflowing conversion matches the overflowing
-/// behavior of a target's native float-to-int conversion instructions.
-CODEGENOPT(FPCastOverflowWorkaround, 1, 0)
+/// When false, this attempts to generate code as if the result of an
+/// overflowing conversion matches the overflowing behavior of a target's native
+/// float-to-int conversion instructions.
+CODEGENOPT(StrictFloatCastOverflow, 1, 1)
 
 CODEGENOPT(UniformWGSize     , 1, 0) ///< -cl-uniform-work-group-size
 CODEGENOPT(NoZeroInitializedInBSS , 1, 0) ///< -fno-zero-initialized-in-bss.
 /// \brief Method of Objective-C dispatch to use.
 ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)
 CODEGENOPT(OmitLeafFramePointer , 1, 0) ///< Set when -momit-leaf-frame-pointer is
                                         ///< enabled.
 

lib/CodeGen/CGCall.cpp

FuncAttrs.addAttribute("less-precise-fpmad",
                            llvm::toStringRef(CodeGenOpts.LessPreciseFPMAD));
 
     if (!CodeGenOpts.FPDenormalMode.empty())
       FuncAttrs.addAttribute("denormal-fp-math", CodeGenOpts.FPDenormalMode);
 
     FuncAttrs.addAttribute("no-trapping-math",
                            llvm::toStringRef(CodeGenOpts.NoTrappingMath));
 
-    if (CodeGenOpts.FPCastOverflowWorkaround)
-      FuncAttrs.addAttribute("fp-cast-overflow-workaround", "true");
+    // Strict (compliant) code is the default, so only add this attribute to
+    // indicate that we are trying to workaround a problem case.
+    if (!CodeGenOpts.StrictFloatCastOverflow)
+      FuncAttrs.addAttribute("strict-float-cast-overflow", "false");
 
     // TODO: Are these all needed?
     // unsafe/inf/nan/nsz are handled by instruction-level FastMathFlags.
     FuncAttrs.addAttribute("no-infs-fp-math",
                            llvm::toStringRef(CodeGenOpts.NoInfsFPMath));
     FuncAttrs.addAttribute("no-nans-fp-math",
                            llvm::toStringRef(CodeGenOpts.NoNaNsFPMath));
     FuncAttrs.addAttribute("unsafe-fp-math",

lib/Driver/ToolChains/Clang.cpp

if (!HonorINFs && !HonorNaNs)
     CmdArgs.push_back("-ffinite-math-only");
 
   if (const Arg *A = Args.getLastArg(options::OPT_mfpmath_EQ)) {
     CmdArgs.push_back("-mfpmath");
     CmdArgs.push_back(A->getValue());
   }
 
   // Disable a codegen optimization for floating-point casts.
-  if (Args.hasFlag(options::OPT_ffp_cast_overflow_workaround,
-                   options::OPT_fno_fp_cast_overflow_workaround, false))
-    CmdArgs.push_back("-ffp-cast-overflow-workaround");
+  if (Args.hasFlag(options::OPT_fno_strict_float_cast_overflow,
+                   options::OPT_fstrict_float_cast_overflow, false))
+    CmdArgs.push_back("-fno-strict-float-cast-overflow");
 }
 
 static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
                                   const llvm::Triple &Triple,
                                   const InputInfo &Input) {
   // Enable region store model by default.
   CmdArgs.push_back("-analyzer-store=region");
 

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.FlushDenorm = Args.hasArg(OPT_cl_denorms_are_zero);
   Opts.CorrectlyRoundedDivSqrt =
       Args.hasArg(OPT_cl_fp32_correctly_rounded_divide_sqrt);
   Opts.UniformWGSize =
       Args.hasArg(OPT_cl_uniform_work_group_size);
   Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
   Opts.ReciprocalMath = Args.hasArg(OPT_freciprocal_math);
   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);
-  Opts.FPCastOverflowWorkaround = Args.hasArg(OPT_ffp_cast_overflow_workaround);
+  Opts.StrictFloatCastOverflow =
+      !Args.hasArg(OPT_fno_strict_float_cast_overflow);
 
   Opts.NoZeroInitializedInBSS = Args.hasArg(OPT_mno_zero_initialized_in_bss);
   Opts.NumRegisterParameters = getLastArgIntValue(Args, OPT_mregparm, 0, Diags);
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.EnableSegmentedStacks = Args.hasArg(OPT_split_stacks);
   Opts.RelaxAll = Args.hasArg(OPT_mrelax_all);
   Opts.IncrementalLinkerCompatible =

test/CodeGen/no-junk-ftrunc.c

-// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -S -fno-strict-float-cast-overflow %s -emit-llvm -o - | FileCheck %s
 // CHECK-LABEL: main
-// CHECK: attributes #0 = {{.*}}"fp-cast-overflow-workaround"="true"{{.*}}
+// CHECK: attributes #0 = {{.*}}"strict-float-cast-overflow"="false"{{.*}}
 
 // The workaround attribute is not applied by default.
 
 // RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s --check-prefix=DEFAULT
 // DEFAULT-LABEL: main
-// DEFAULT-NOT: fp-cast-overflow-workaround
+// DEFAULT-NOT: strict-float-cast-overflow
 
 int main() {
   return 0;
 }
 

test/Driver/fast-math.c

 // CHECK-NO-REASSOC-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"
 
 // This isn't fast-math, but the option is handled in the same place as other FP params.
-// Last option wins, and the flag is *not* passed by default. 
+// Last option wins, and strict behavior is assumed by default. 
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND %s
 // CHECK-FPOV-WORKAROUND: "-cc1"
-// CHECK-FPOV-WORKAROUND: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND: "-fno-strict-float-cast-overflow"
 
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPOV-WORKAROUND-DEFAULT %s
 // CHECK-FPOV-WORKAROUND-DEFAULT: "-cc1"
-// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-FPOV-WORKAROUND-DEFAULT-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND %s
 // CHECK-NO-FPOV-WORKAROUND: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-NOT: "strict-float-cast-overflow"
 
-// RUN: %clang -### -ffp-cast-overflow-workaround -fno-fp-cast-overflow-workaround -c %s 2>&1 \
+// RUN: %clang -### -fno-strict-float-cast-overflow -fstrict-float-cast-overflow -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FPOV-WORKAROUND-OVERRIDE %s
 // CHECK-NO-FPOV-WORKAROUND-OVERRIDE: "-cc1"
-// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "-ffp-cast-overflow-workaround"
+// CHECK-NO-FPOV-WORKAROUND-OVERRIDE-NOT: "strict-float-cast-overflow"