r226915 - Process the -fno-signed-zeros optimization flag (PR20870)

Richard Smith richard at metafoo.co.uk
Tue Apr 7 18:36:42 PDT 2015


On Tue, Apr 7, 2015 at 5:59 PM, Tom Stellard <tom at stellard.net> wrote:

> On Tue, Apr 07, 2015 at 05:45:20PM -0700, Richard Smith wrote:
> > On Tue, Apr 7, 2015 at 4:36 PM, Tom Stellard <tom at stellard.net> wrote:
> >
> > > On Tue, Apr 07, 2015 at 03:11:44PM -0700, Richard Smith wrote:
> > > > On Tue, Apr 7, 2015 at 3:05 PM, Tom Stellard <tom at stellard.net>
> wrote:
> > > >
> > > > > On Fri, Jan 23, 2015 at 04:40:50PM -0000, Sanjay Patel wrote:
> > > > > > Author: spatel
> > > > > > Date: Fri Jan 23 10:40:50 2015
> > > > > > New Revision: 226915
> > > > > >
> > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=226915&view=rev
> > > > > > Log:
> > > > > > Process the -fno-signed-zeros optimization flag (PR20870)
> > > > > >
> > > > > > The driver currently accepts but ignores the -fno-signed-zeros
> flag.
> > > > > > This patch passes the flag through and enables 'nsz'
> fast-math-flag
> > > > > > generation in IR.
> > > > > >
> > > > > > The existing OpenCL flag for the same functionality is made into
> an
> > > > > > alias here. It may be removed in a subsequent patch.
> > > > > >
> > > > >
> > > > > What is the reason for removing this option?  This will break
> OpenCL
> > > > > implementations that use clang.
> > > >
> > > >
> > > > We do not provide any stability guarantees for Clang's -cc1
> interface in
> > > > general. Are these implementations running -cc1 directly, or passing
> this
> > > > flag via -Xclang? If the latter, we should add a proper (stable)
> > > > driver-level flag for it.
> > > >
> > >
> > > I understand there are no stability guarantees, but regardless I feel
> > > like it's useful to keep it around as an alias since it is part of
> > > the OpenCL spec.  Is there a high cost involved in keeping it?
> > >
> >
> > It's not really reasonable to expect Clang's -cc1 to implement the flags
> > required by the OpenCL spec. Your OpenCL driver should be recognizing
> those
> > flags and handling them in whatever manner is appropriate.
> >
>
> What's the  difference between -cc1 flags and 'proper driver-level flags' ?


The driver-level flags are the ones in Options.td; these are accepted by
the GCC-compatible 'clang' driver -- though it sounds like this isn't the
right thing for clover's case, since it seems you're not using the clang
driver.

> In the OpenCL implementation I'm working on (clover), we are using the
> > > C++ API, so the argument is being passed via:
> > > clang::CompilerInvocation::CreateFromArgs()
> >
> >
> > Could you instead set NoSignedZeros directly on the CodeGenOptions of the
> > CompilerInvocation after calling CreateFromArgs?
>
> I could do this, but I'm wondering if there still might be a way to add
> the OpenCL flags to clang.  It is still useful if you want to do offline
> compilation.


We could keep these options as a convenience. However, we generally desire
to keep the -cc1 interface minimal, as essentially a serialized form of the
options structs that CompilerInvocation::CreateFromArgs initializes. As
such, having two different flags that do the same thing doesn't make a lot
of sense. Users are generally not expected to directly supply arguments to
-cc1 invocations, so the benefit seems questionable.

-Tom
> >
> > -Tom
> > >
> > >
> > >
> > > > -Tom
> > > > >
> > > > > > This should resolve bug 20870 (
> > > > > http://llvm.org/bugs/show_bug.cgi?id=20870 );
> > > > > > patches for the optimizer were checked in at:
> > > > > >
> http://llvm.org/viewvc/llvm-project?view=revision&revision=225050
> > > > > >
> http://llvm.org/viewvc/llvm-project?view=revision&revision=224583
> > > > > >
> > > > > > Differential Revision: http://reviews.llvm.org/D6873
> > > > > >
> > > > > >
> > > > > > Modified:
> > > > > >     cfe/trunk/include/clang/Driver/CC1Options.td
> > > > > >     cfe/trunk/include/clang/Driver/Options.td
> > > > > >     cfe/trunk/lib/Driver/Tools.cpp
> > > > > >     cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> > > > > >     cfe/trunk/test/CodeGen/finite-math.c
> > > > > >     cfe/trunk/test/Driver/fast-math.c
> > > > > >
> > > > > > Modified: cfe/trunk/include/clang/Driver/CC1Options.td
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
> > > > > > +++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Jan 23
> 10:40:50
> > > 2015
> > > > > > @@ -580,7 +580,7 @@ def cl_finite_math_only : Flag<["-"], "c
> > > > > >    HelpText<"OpenCL only. Allow floating-point optimizations that
> > > assume
> > > > > arguments and results are not NaNs or +-Inf.">;
> > > > > >  def cl_kernel_arg_info : Flag<["-"], "cl-kernel-arg-info">,
> > > > > >    HelpText<"OpenCL only. Generate kernel argument metadata.">;
> > > > > > -def cl_no_signed_zeros : Flag<["-"], "cl-no-signed-zeros">,
> > > > > > +def : Flag<["-"], "cl-no-signed-zeros">,
> Alias<fno_signed_zeros>,
> > > > > >    HelpText<"OpenCL only. Allow optimizations to ignore the
> > > signedness
> > > > > of the floating-point zero.">;
> > > > > >  def cl_unsafe_math_optimizations : Flag<["-"],
> > > > > "cl-unsafe-math-optimizations">,
> > > > > >    HelpText<"OpenCL only. Allow unsafe floating-point
> optimizations.
> > > > > Also implies -cl-no-signed-zeros and -cl-mad-enable">;
> > > > > >
> > > > > > Modified: cfe/trunk/include/clang/Driver/Options.td
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/include/clang/Driver/Options.td (original)
> > > > > > +++ cfe/trunk/include/clang/Driver/Options.td Fri Jan 23 10:40:50
> > > 2015
> > > > > > @@ -566,7 +566,9 @@ def fno_reciprocal_math : Flag<["-"], "f
> > > > > >  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>;
> > > > > > -def fno_signed_zeros : Flag<["-"], "fno-signed-zeros">,
> > > Group<f_Group>;
> > > > > > +def fno_signed_zeros :
> > > > > > +  Flag<["-"], "fno-signed-zeros">, Group<f_Group>,
> > > Flags<[CC1Option]>,
> > > > > > +  HelpText<"Allow optimizations that ignore the sign of floating
> > > point
> > > > > zeros">;
> > > > > >  def fhonor_nans : Flag<["-"], "fhonor-nans">, Group<f_Group>;
> > > > > >  def fno_honor_nans : Flag<["-"], "fno-honor-nans">,
> Group<f_Group>;
> > > > > >  def fhonor_infinities : Flag<["-"], "fhonor-infinities">,
> > > > > Group<f_Group>;
> > > > > >
> > > > > > Modified: cfe/trunk/lib/Driver/Tools.cpp
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/lib/Driver/Tools.cpp (original)
> > > > > > +++ cfe/trunk/lib/Driver/Tools.cpp Fri Jan 23 10:40:50 2015
> > > > > > @@ -2955,6 +2955,8 @@ void Clang::ConstructJob(Compilation &C,
> > > > > >        !TrappingMath)
> > > > > >      CmdArgs.push_back("-menable-unsafe-fp-math");
> > > > > >
> > > > > > +  if (!SignedZeros)
> > > > > > +    CmdArgs.push_back("-fno-signed-zeros");
> > > > > >
> > > > > >    // Validate and pass through -fp-contract option.
> > > > > >    if (Arg *A = Args.getLastArg(options::OPT_ffast_math,
> > > > > FastMathAliasOption,
> > > > > >
> > > > > > Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
> > > > > > +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Jan 23
> 10:40:50
> > > > > 2015
> > > > > > @@ -441,7 +441,7 @@ static bool ParseCodeGenArgs(CodeGenOpti
> > > > > >
> > >  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_cl_no_signed_zeros);
> > > > > > +  Opts.NoSignedZeros = Args.hasArg(OPT_fno_signed_zeros);
> > > > > >    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);
> > > > > >
> > > > > > Modified: cfe/trunk/test/CodeGen/finite-math.c
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/finite-math.c?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/test/CodeGen/finite-math.c (original)
> > > > > > +++ cfe/trunk/test/CodeGen/finite-math.c Fri Jan 23 10:40:50 2015
> > > > > > @@ -1,11 +1,15 @@
> > > > > > -// RUN: %clang_cc1 -ffinite-math-only -emit-llvm -o - %s |
> > > FileCheck %s
> > > > > > +// 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
> > > > > > +
> > > > > >  float f0, f1, f2;
> > > > > >
> > > > > >  void foo(void) {
> > > > > >    // CHECK-LABEL: define void @foo()
> > > > > >
> > > > > > -  // CHECK: fadd nnan ninf
> > > > > > +  // FINITE: fadd nnan ninf
> > > > > > +  // NSZ: fadd nsz
> > > > > >    f0 = f1 + f2;
> > > > > >
> > > > > >    // CHECK: ret
> > > > > >  }
> > > > > > +
> > > > > >
> > > > > > Modified: cfe/trunk/test/Driver/fast-math.c
> > > > > > URL:
> > > > >
> > >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fast-math.c?rev=226915&r1=226914&r2=226915&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- cfe/trunk/test/Driver/fast-math.c (original)
> > > > > > +++ cfe/trunk/test/Driver/fast-math.c Fri Jan 23 10:40:50 2015
> > > > > > @@ -25,6 +25,21 @@
> > > > > >  // CHECK-NO-INFS-NO-FAST-MATH: "-cc1"
> > > > > >  // CHECK-NO-INFS-NO-FAST-MATH-NOT: "-menable-no-infs"
> > > > > >  //
> > > > > > +// RUN: %clang -### -fno-signed-zeros -c %s 2>&1 \
> > > > > > +// RUN:   | FileCheck --check-prefix=CHECK-NO-SIGNED-ZEROS %s
> > > > > > +// CHECK-NO-SIGNED-ZEROS: "-cc1"
> > > > > > +// CHECK-NO-SIGNED-ZEROS: "-fno-signed-zeros"
> > > > > > +//
> > > > > > +// RUN: %clang -### -fno-fast-math -fno-signed-zeros -c %s 2>&1
> \
> > > > > > +// RUN:   | FileCheck
> > > --check-prefix=CHECK-NO-FAST-MATH-NO-SIGNED-ZEROS
> > > > > %s
> > > > > > +// CHECK-NO-FAST-MATH-NO-SIGNED-ZEROS: "-cc1"
> > > > > > +// CHECK-NO-FAST-MATH-NO-SIGNED-ZEROS: "-fno-signed-zeros"
> > > > > > +//
> > > > > > +// RUN: %clang -### -fno-signed-zeros -fno-fast-math -c %s 2>&1
> \
> > > > > > +// RUN:   | FileCheck
> > > --check-prefix=CHECK-NO-SIGNED-ZEROS-NO-FAST-MATH
> > > > > %s
> > > > > > +// CHECK-NO-SIGNED-ZEROS-NO-FAST-MATH: "-cc1"
> > > > > > +// CHECK-NO-SIGNED-ZEROS-NO-FAST-MATH-NOT: "-fno-signed-zeros"
> > > > > > +//
> > > > > >  // RUN: %clang -### -fno-honor-nans -c %s 2>&1 \
> > > > > >  // RUN:   | FileCheck --check-prefix=CHECK-NO-NANS %s
> > > > > >  // CHECK-NO-NANS: "-cc1"
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > cfe-commits mailing list
> > > > > > cfe-commits at cs.uiuc.edu
> > > > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > > > > _______________________________________________
> > > > > cfe-commits mailing list
> > > > > cfe-commits at cs.uiuc.edu
> > > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > > > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150407/d1646979/attachment.html>


More information about the cfe-commits mailing list