[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
This revision was automatically updated to reflect the committed changes.
Closed by commit rG827be690dce1: [clang] FastMathFlags.allowContract should be
initialized only from FPFeatures. (authored by mibintc).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
Files:
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/constrained-math-builtins.c
clang/test/CodeGen/fp-contract-on-pragma.cpp
clang/test/CodeGen/fp-contract-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-class.cpp
clang/test/CodeGen/fp-floatcontrol-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-stack.cpp
clang/test/CodeGenOpenCL/relaxed-fpmath.cl
clang/test/CodeGenOpenCL/single-precision-constant.cl
Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | FileCheck %s
float fn(float f) {
- // CHECK: tail call contract float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
+ // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
return f*2. + 1.;
}
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -8,12 +8,12 @@
float spscalardiv(float a, float b) {
// CHECK: @spscalardiv(
- // NORMAL: fdiv contract float
+ // NORMAL: fdiv float
// FAST: fdiv fast float
- // FINITE: fdiv nnan ninf contract float
- // UNSAFE: fdiv nnan nsz contract float
- // MAD: fdiv contract float
- // NOSIGNED: fdiv nsz contract float
+ // FINITE: fdiv nnan ninf float
+ // UNSAFE: fdiv nnan nsz float
+ // MAD: fdiv float
+ // NOSIGNED: fdiv nsz float
return a / b;
}
// CHECK: attributes
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -9,7 +9,7 @@
float fun_default FUN(1)
//CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
// Note that backend wants constrained intrinsics used
@@ -37,7 +37,7 @@
//CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if NOHONOR
-//CHECK-NOHONOR: nnan ninf contract float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
+//CHECK-NOHONOR: nnan ninf float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if FAST
//Not possible to enable float_control(except) in FAST mode.
@@ -49,13 +49,13 @@
float exc_pop FUN(5)
//CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
//CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
#endif
#if FAST
//CHECK-FAST: fmul fast float
@@ -66,13 +66,13 @@
float exc_off FUN(5)
//CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
-//CHECK-DEBSTRICT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: call float @llvm.fmuladd{{.*}}
#endif
#if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
#endif
#if FAST
//CHECK-FAST: fmul fast float
@@ -83,30 +83,30 @@
float precise_on FUN(3)
//CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
-//CHECK-DEBSTRICT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: float {{.*}}llvm.fmuladd{{.*}}
#endif
#if NOHONOR
// If precise is pushed then all fast-math should be off!
-//CHECK-NOHONOR: call contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call float {{.*}}llvm.fmuladd{{.*}}
#endif
#if FAST
-//CHECK-FAST: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-FAST: float {{.*}}llvm.fmuladd{{.*}}
#endif
#pragma float_control(pop)
float precise_pop FUN(3)
//CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: con
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. No, go ahead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
michele.scandale added a comment. This looks good to me. @rjmccall do you have any more feedback? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
Anastasia added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
mibintc wrote:
> mibintc wrote:
> > rjmccall wrote:
> > > michele.scandale wrote:
> > > > mibintc wrote:
> > > > > mibintc wrote:
> > > > > > rjmccall wrote:
> > > > > > > mibintc wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > mibintc wrote:
> > > > > > > > > > I changed this because the FAST version of this test
> > > > > > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast'
> > > > > > > > > > attribute on the instruction dump. All the LLVM FMF bits
> > > > > > > > > > must be set for the fast attribute print. By default, the
> > > > > > > > > > value for OpenCL is ffp-contract=on
> > > > > > > > > Is there an overall behavior change for OpenCL across these
> > > > > > > > > patches?
> > > > > > > > I think the answer to your question is no. Here is more
> > > > > > > > details: OpenCL sets the default behavior to ffp-contract=on.
> > > > > > > > In https://reviews.llvm.org/D72841 I added the function
> > > > > > > > UpdateFastMathFlags, that function set the llvm
> > > > > > > > FMF.allowContract bit to be ( ffp-contract=on or
> > > > > > > > ffp-contract=fast). This patch just drops the check on
> > > > > > > > ffp-contract=on. I didn't wind back to see how the llvm fast
> > > > > > > > attribute was being set for this [opencl] test case originally.
> > > > > > > Well, to what extent are there (including this patch) overall
> > > > > > > test changes for the OpenCL tests, and what are tthey?
> > > > > > In the #pragma float_control patch https://reviews.llvm.org/D72841,
> > > > > > I changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the
> > > > > > non-FAST cases to show the contract bit. Also 1 line in
> > > > > > single-precision-constant.cl for the same reason, to show the
> > > > > > contract bit. This patch undoes those test changes. I'll do more
> > > > > > investigation to understand why the fast bit isn't being set in the
> > > > > > FAST case in relaxed-fpmath.cl without the change to
> > > > > > CompilerInovcaton
> > > > > Prior to the patch for #pragma float_control, the llvm.FastMathFlags
> > > > > was initialized from LangArgs.FastMath in the CodeGenFunction
> > > > > constructor approximately line 74, and the FMF values in IRBuilder
> > > > > were never changed--For the test
> > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl with option
> > > > > -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath =
> > > > > Args.hasArg(OPT_ffast_math) ||
> > > > > Args.hasArg(OPT_cl_fast_relaxed_math)) If FastMath is on, then all
> > > > > the llvm.FMF flags are set.
> > > > >
> > > > > The #pragma float_control patch does change the IRBuilder settings in
> > > > > the course of visiting the Expr nodes, using the information in the
> > > > > Expr nodes, but the initial setting of FPFeatures from the command
> > > > > line overlooked the fact that FastMath, and therefore
> > > > > ffp-contract=fast, is enabled.
> > > > Prior to D72841 compiling something with `-ffast-math
> > > > -ffp-contract={on,off}` was still producing `fast` as fast-math flags
> > > > on the instructions for the same reason.
> > > > The clang driver does not consider the contraction mode for passing
> > > > `-ffast-math` to CC1, which is consistent with the GCC behavior (I
> > > > checked if the `__FAST_MATH__` is defined if I compile with
> > > > `-ffast-math -ffp-contract=off`).
> > > > According to this, the code in `CodeGenFunction`:
> > > > ```
> > > > if (LangOpts.FastMath)
> > > > FMF.setFast();
> > > > ```
> > > > is not correct.
> > > >
> > > > The OpenCL option `-cl-fast-relaxed-math` should be equivalent to
> > > > `-ffast-math` for the user. I don't know what the interaction between
> > > > `-cl-fast-relaxed-math` and `-ffp-contract={on, off,fast}`.
> > > >
> > > > If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the
> > > > following might work:
> > > > ```
> > > > if (Args.hasArg(OPTS_cl_fast_relaxed_math) &&
> > > > !Arg.hasArg(OPT_ffp_contractEQ))
> > > > Opts.setDefaultFPContractMode)LangOptions::FPM_Fast);
> > > > ```
> > > Okay, thanks for the investigation, I agree that that all makes sense.
> > > I'm not sure what it would mean to enable fast math but disable
> > > contraction — that combination doesn't seem useful.
> > >
> > > I'm fine with going forward with the OpenCL changes as they are, but you
> > > might want to specifically reach out to the OpenCL people and let them
> > > know what's going on.
> > @arsenm Matt, I'm making this change to CompilerInvocation to fix a bug
> > that was introduced by https://reviews.llvm.org/D72841 I see that you
> > contribute to OpenCL will you please let interested folks know
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc added a subscriber: arsenm.
mibintc added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
rjmccall wrote:
> michele.scandale wrote:
> > mibintc wrote:
> > > mibintc wrote:
> > > > rjmccall wrote:
> > > > > mibintc wrote:
> > > > > > rjmccall wrote:
> > > > > > > mibintc wrote:
> > > > > > > > I changed this because the FAST version of this test
> > > > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast'
> > > > > > > > attribute on the instruction dump. All the LLVM FMF bits must
> > > > > > > > be set for the fast attribute print. By default, the value for
> > > > > > > > OpenCL is ffp-contract=on
> > > > > > > Is there an overall behavior change for OpenCL across these
> > > > > > > patches?
> > > > > > I think the answer to your question is no. Here is more details:
> > > > > > OpenCL sets the default behavior to ffp-contract=on. In
> > > > > > https://reviews.llvm.org/D72841 I added the function
> > > > > > UpdateFastMathFlags, that function set the llvm FMF.allowContract
> > > > > > bit to be ( ffp-contract=on or ffp-contract=fast). This patch just
> > > > > > drops the check on ffp-contract=on. I didn't wind back to see how
> > > > > > the llvm fast attribute was being set for this [opencl] test case
> > > > > > originally.
> > > > > Well, to what extent are there (including this patch) overall test
> > > > > changes for the OpenCL tests, and what are tthey?
> > > > In the #pragma float_control patch https://reviews.llvm.org/D72841, I
> > > > changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST
> > > > cases to show the contract bit. Also 1 line in
> > > > single-precision-constant.cl for the same reason, to show the contract
> > > > bit. This patch undoes those test changes. I'll do more investigation
> > > > to understand why the fast bit isn't being set in the FAST case in
> > > > relaxed-fpmath.cl without the change to CompilerInovcaton
> > > Prior to the patch for #pragma float_control, the llvm.FastMathFlags was
> > > initialized from LangArgs.FastMath in the CodeGenFunction constructor
> > > approximately line 74, and the FMF values in IRBuilder were never
> > > changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl with
> > > option -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath =
> > > Args.hasArg(OPT_ffast_math) ||
> > > Args.hasArg(OPT_cl_fast_relaxed_math)) If FastMath is on, then all the
> > > llvm.FMF flags are set.
> > >
> > > The #pragma float_control patch does change the IRBuilder settings in the
> > > course of visiting the Expr nodes, using the information in the Expr
> > > nodes, but the initial setting of FPFeatures from the command line
> > > overlooked the fact that FastMath, and therefore ffp-contract=fast, is
> > > enabled.
> > Prior to D72841 compiling something with `-ffast-math
> > -ffp-contract={on,off}` was still producing `fast` as fast-math flags on
> > the instructions for the same reason.
> > The clang driver does not consider the contraction mode for passing
> > `-ffast-math` to CC1, which is consistent with the GCC behavior (I checked
> > if the `__FAST_MATH__` is defined if I compile with `-ffast-math
> > -ffp-contract=off`).
> > According to this, the code in `CodeGenFunction`:
> > ```
> > if (LangOpts.FastMath)
> > FMF.setFast();
> > ```
> > is not correct.
> >
> > The OpenCL option `-cl-fast-relaxed-math` should be equivalent to
> > `-ffast-math` for the user. I don't know what the interaction between
> > `-cl-fast-relaxed-math` and `-ffp-contract={on, off,fast}`.
> >
> > If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the
> > following might work:
> > ```
> > if (Args.hasArg(OPTS_cl_fast_relaxed_math) &&
> > !Arg.hasArg(OPT_ffp_contractEQ))
> > Opts.setDefaultFPContractMode)LangOptions::FPM_Fast);
> > ```
> Okay, thanks for the investigation, I agree that that all makes sense. I'm
> not sure what it would mean to enable fast math but disable contraction —
> that combination doesn't seem useful.
>
> I'm fine with going forward with the OpenCL changes as they are, but you
> might want to specifically reach out to the OpenCL people and let them know
> what's going on.
@arsenm Matt, I'm making this change to CompilerInvocation to fix a bug that
was introduced by https://reviews.llvm.org/D72841 I see that you contribute to
OpenCL will you please let interested folks know about this change?
D72841 erroneously set the llvm.FMF.contract bit to true whether the
contraction status was "allow contraction across statements" or "allow
contraction within statements". This patch corrects that error to set
llvm.FMF.contract bit only when contraction status is "allow contraction across
statements". D72841 mis-captur
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc updated this revision to Diff 264241.
mibintc marked an inline comment as done.
mibintc added a comment.
This is the same as the previous patch, except I removed the fix for pragma
push-pop that John said should be committed separately
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
Files:
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/constrained-math-builtins.c
clang/test/CodeGen/fp-contract-on-pragma.cpp
clang/test/CodeGen/fp-contract-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-class.cpp
clang/test/CodeGen/fp-floatcontrol-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-stack.cpp
clang/test/CodeGenOpenCL/relaxed-fpmath.cl
clang/test/CodeGenOpenCL/single-precision-constant.cl
Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | FileCheck %s
float fn(float f) {
- // CHECK: tail call contract float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
+ // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
return f*2. + 1.;
}
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -8,12 +8,12 @@
float spscalardiv(float a, float b) {
// CHECK: @spscalardiv(
- // NORMAL: fdiv contract float
+ // NORMAL: fdiv float
// FAST: fdiv fast float
- // FINITE: fdiv nnan ninf contract float
- // UNSAFE: fdiv nnan nsz contract float
- // MAD: fdiv contract float
- // NOSIGNED: fdiv nsz contract float
+ // FINITE: fdiv nnan ninf float
+ // UNSAFE: fdiv nnan nsz float
+ // MAD: fdiv float
+ // NOSIGNED: fdiv nsz float
return a / b;
}
// CHECK: attributes
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -9,7 +9,7 @@
float fun_default FUN(1)
//CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
// Note that backend wants constrained intrinsics used
@@ -37,7 +37,7 @@
//CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if NOHONOR
-//CHECK-NOHONOR: nnan ninf contract float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
+//CHECK-NOHONOR: nnan ninf float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if FAST
//Not possible to enable float_control(except) in FAST mode.
@@ -49,13 +49,13 @@
float exc_pop FUN(5)
//CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
//CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
#endif
#if FAST
//CHECK-FAST: fmul fast float
@@ -66,13 +66,13 @@
float exc_off FUN(5)
//CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
-//CHECK-DEBSTRICT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: call float @llvm.fmuladd{{.*}}
#endif
#if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
#endif
#if FAST
//CHECK-FAST: fmul fast float
@@ -83,30 +83,30 @@
float precise_on FUN(3)
//CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
-//CHECK-DEBSTRICT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: float {{.*}}llvm.fmuladd{{.*}}
#endif
#if NOHONOR
// If precise is pushed then all fast-math should be off!
-//CHECK-NOHONOR: call contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call float {{.*}}llvm.fmuladd{{.*}}
#endif
#if FAST
-//CHECK-FAST: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-FAST: float {{.*}}llvm.fmuladd{{.*}}
#endif
#pragma float_control(pop)
float precise_pop FUN(3)
//CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc marked an inline comment as done.
mibintc added a subscriber: Anastasia.
mibintc added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
mibintc wrote:
> rjmccall wrote:
> > michele.scandale wrote:
> > > mibintc wrote:
> > > > mibintc wrote:
> > > > > rjmccall wrote:
> > > > > > mibintc wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > mibintc wrote:
> > > > > > > > > I changed this because the FAST version of this test
> > > > > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast'
> > > > > > > > > attribute on the instruction dump. All the LLVM FMF bits
> > > > > > > > > must be set for the fast attribute print. By default, the
> > > > > > > > > value for OpenCL is ffp-contract=on
> > > > > > > > Is there an overall behavior change for OpenCL across these
> > > > > > > > patches?
> > > > > > > I think the answer to your question is no. Here is more details:
> > > > > > > OpenCL sets the default behavior to ffp-contract=on. In
> > > > > > > https://reviews.llvm.org/D72841 I added the function
> > > > > > > UpdateFastMathFlags, that function set the llvm FMF.allowContract
> > > > > > > bit to be ( ffp-contract=on or ffp-contract=fast). This patch
> > > > > > > just drops the check on ffp-contract=on. I didn't wind back to
> > > > > > > see how the llvm fast attribute was being set for this [opencl]
> > > > > > > test case originally.
> > > > > > Well, to what extent are there (including this patch) overall test
> > > > > > changes for the OpenCL tests, and what are tthey?
> > > > > In the #pragma float_control patch https://reviews.llvm.org/D72841, I
> > > > > changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST
> > > > > cases to show the contract bit. Also 1 line in
> > > > > single-precision-constant.cl for the same reason, to show the
> > > > > contract bit. This patch undoes those test changes. I'll do more
> > > > > investigation to understand why the fast bit isn't being set in the
> > > > > FAST case in relaxed-fpmath.cl without the change to CompilerInovcaton
> > > > Prior to the patch for #pragma float_control, the llvm.FastMathFlags
> > > > was initialized from LangArgs.FastMath in the CodeGenFunction
> > > > constructor approximately line 74, and the FMF values in IRBuilder were
> > > > never changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl
> > > > with option -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath =
> > > > Args.hasArg(OPT_ffast_math) ||
> > > > Args.hasArg(OPT_cl_fast_relaxed_math)) If FastMath is on, then all the
> > > > llvm.FMF flags are set.
> > > >
> > > > The #pragma float_control patch does change the IRBuilder settings in
> > > > the course of visiting the Expr nodes, using the information in the
> > > > Expr nodes, but the initial setting of FPFeatures from the command line
> > > > overlooked the fact that FastMath, and therefore ffp-contract=fast, is
> > > > enabled.
> > > Prior to D72841 compiling something with `-ffast-math
> > > -ffp-contract={on,off}` was still producing `fast` as fast-math flags on
> > > the instructions for the same reason.
> > > The clang driver does not consider the contraction mode for passing
> > > `-ffast-math` to CC1, which is consistent with the GCC behavior (I
> > > checked if the `__FAST_MATH__` is defined if I compile with `-ffast-math
> > > -ffp-contract=off`).
> > > According to this, the code in `CodeGenFunction`:
> > > ```
> > > if (LangOpts.FastMath)
> > > FMF.setFast();
> > > ```
> > > is not correct.
> > >
> > > The OpenCL option `-cl-fast-relaxed-math` should be equivalent to
> > > `-ffast-math` for the user. I don't know what the interaction between
> > > `-cl-fast-relaxed-math` and `-ffp-contract={on, off,fast}`.
> > >
> > > If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the
> > > following might work:
> > > ```
> > > if (Args.hasArg(OPTS_cl_fast_relaxed_math) &&
> > > !Arg.hasArg(OPT_ffp_contractEQ))
> > > Opts.setDefaultFPContractMode)LangOptions::FPM_Fast);
> > > ```
> > Okay, thanks for the investigation, I agree that that all makes sense. I'm
> > not sure what it would mean to enable fast math but disable contraction —
> > that combination doesn't seem useful.
> >
> > I'm fine with going forward with the OpenCL changes as they are, but you
> > might want to specifically reach out to the OpenCL people and let them know
> > what's going on.
> @arsenm Matt, I'm making this change to CompilerInvocation to fix a bug that
> was introduced by https://reviews.llvm.org/D72841 I see that you contribute
> to OpenCL will you please let interested folks know about this change?
> D72841 erroneously set the llvm.FMF.contract bit to true whether the
> contraction status was "allow
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
rjmccall added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
michele.scandale wrote:
> mibintc wrote:
> > mibintc wrote:
> > > rjmccall wrote:
> > > > mibintc wrote:
> > > > > rjmccall wrote:
> > > > > > mibintc wrote:
> > > > > > > I changed this because the FAST version of this test
> > > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast'
> > > > > > > attribute on the instruction dump. All the LLVM FMF bits must be
> > > > > > > set for the fast attribute print. By default, the value for
> > > > > > > OpenCL is ffp-contract=on
> > > > > > Is there an overall behavior change for OpenCL across these patches?
> > > > > I think the answer to your question is no. Here is more details:
> > > > > OpenCL sets the default behavior to ffp-contract=on. In
> > > > > https://reviews.llvm.org/D72841 I added the function
> > > > > UpdateFastMathFlags, that function set the llvm FMF.allowContract bit
> > > > > to be ( ffp-contract=on or ffp-contract=fast). This patch just drops
> > > > > the check on ffp-contract=on. I didn't wind back to see how the
> > > > > llvm fast attribute was being set for this [opencl] test case
> > > > > originally.
> > > > Well, to what extent are there (including this patch) overall test
> > > > changes for the OpenCL tests, and what are tthey?
> > > In the #pragma float_control patch https://reviews.llvm.org/D72841, I
> > > changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST cases
> > > to show the contract bit. Also 1 line in single-precision-constant.cl
> > > for the same reason, to show the contract bit. This patch undoes those
> > > test changes. I'll do more investigation to understand why the fast bit
> > > isn't being set in the FAST case in relaxed-fpmath.cl without the change
> > > to CompilerInovcaton
> > Prior to the patch for #pragma float_control, the llvm.FastMathFlags was
> > initialized from LangArgs.FastMath in the CodeGenFunction constructor
> > approximately line 74, and the FMF values in IRBuilder were never
> > changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl with
> > option -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath =
> > Args.hasArg(OPT_ffast_math) || Args.hasArg(OPT_cl_fast_relaxed_math))
> > If FastMath is on, then all the llvm.FMF flags are set.
> >
> > The #pragma float_control patch does change the IRBuilder settings in the
> > course of visiting the Expr nodes, using the information in the Expr nodes,
> > but the initial setting of FPFeatures from the command line overlooked the
> > fact that FastMath, and therefore ffp-contract=fast, is enabled.
> Prior to D72841 compiling something with `-ffast-math -ffp-contract={on,off}`
> was still producing `fast` as fast-math flags on the instructions for the
> same reason.
> The clang driver does not consider the contraction mode for passing
> `-ffast-math` to CC1, which is consistent with the GCC behavior (I checked if
> the `__FAST_MATH__` is defined if I compile with `-ffast-math
> -ffp-contract=off`).
> According to this, the code in `CodeGenFunction`:
> ```
> if (LangOpts.FastMath)
> FMF.setFast();
> ```
> is not correct.
>
> The OpenCL option `-cl-fast-relaxed-math` should be equivalent to
> `-ffast-math` for the user. I don't know what the interaction between
> `-cl-fast-relaxed-math` and `-ffp-contract={on, off,fast}`.
>
> If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the
> following might work:
> ```
> if (Args.hasArg(OPTS_cl_fast_relaxed_math) && !Arg.hasArg(OPT_ffp_contractEQ))
> Opts.setDefaultFPContractMode)LangOptions::FPM_Fast);
> ```
Okay, thanks for the investigation, I agree that that all makes sense. I'm not
sure what it would mean to enable fast math but disable contraction — that
combination doesn't seem useful.
I'm fine with going forward with the OpenCL changes as they are, but you might
want to specifically reach out to the OpenCL people and let them know what's
going on.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
michele.scandale added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
mibintc wrote:
> mibintc wrote:
> > rjmccall wrote:
> > > mibintc wrote:
> > > > rjmccall wrote:
> > > > > mibintc wrote:
> > > > > > I changed this because the FAST version of this test
> > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast'
> > > > > > attribute on the instruction dump. All the LLVM FMF bits must be
> > > > > > set for the fast attribute print. By default, the value for OpenCL
> > > > > > is ffp-contract=on
> > > > > Is there an overall behavior change for OpenCL across these patches?
> > > > I think the answer to your question is no. Here is more details:
> > > > OpenCL sets the default behavior to ffp-contract=on. In
> > > > https://reviews.llvm.org/D72841 I added the function
> > > > UpdateFastMathFlags, that function set the llvm FMF.allowContract bit
> > > > to be ( ffp-contract=on or ffp-contract=fast). This patch just drops
> > > > the check on ffp-contract=on. I didn't wind back to see how the llvm
> > > > fast attribute was being set for this [opencl] test case originally.
> > > Well, to what extent are there (including this patch) overall test
> > > changes for the OpenCL tests, and what are tthey?
> > In the #pragma float_control patch https://reviews.llvm.org/D72841, I
> > changed 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST cases to
> > show the contract bit. Also 1 line in single-precision-constant.cl for the
> > same reason, to show the contract bit. This patch undoes those test
> > changes. I'll do more investigation to understand why the fast bit isn't
> > being set in the FAST case in relaxed-fpmath.cl without the change to
> > CompilerInovcaton
> Prior to the patch for #pragma float_control, the llvm.FastMathFlags was
> initialized from LangArgs.FastMath in the CodeGenFunction constructor
> approximately line 74, and the FMF values in IRBuilder were never
> changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl with option
> -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath =
> Args.hasArg(OPT_ffast_math) || Args.hasArg(OPT_cl_fast_relaxed_math))
> If FastMath is on, then all the llvm.FMF flags are set.
>
> The #pragma float_control patch does change the IRBuilder settings in the
> course of visiting the Expr nodes, using the information in the Expr nodes,
> but the initial setting of FPFeatures from the command line overlooked the
> fact that FastMath, and therefore ffp-contract=fast, is enabled.
Prior to D72841 compiling something with `-ffast-math -ffp-contract={on,off}`
was still producing `fast` as fast-math flags on the instructions for the same
reason.
The clang driver does not consider the contraction mode for passing
`-ffast-math` to CC1, which is consistent with the GCC behavior (I checked if
the `__FAST_MATH__` is defined if I compile with `-ffast-math
-ffp-contract=off`).
According to this, the code in `CodeGenFunction`:
```
if (LangOpts.FastMath)
FMF.setFast();
```
is not correct.
The OpenCL option `-cl-fast-relaxed-math` should be equivalent to `-ffast-math`
for the user. I don't know what the interaction between `-cl-fast-relaxed-math`
and `-ffp-contract={on, off,fast}`.
If we need to keep `-cl-fast-relaxed-math` a CC1 option, I guess the following
might work:
```
if (Args.hasArg(OPTS_cl_fast_relaxed_math) && !Arg.hasArg(OPT_ffp_contractEQ))
Opts.setDefaultFPContractMode)LangOptions::FPM_Fast);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc marked an inline comment as done. mibintc added a comment. reply about the incorrect setting of 'fast' during OpenCL compilation with option -cl-fast-relaxed-math Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943 + if (Opts.FastRelaxedMath) +Opts.setDefaultFPContractMode(LangOptions::FPM_Fast); Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat); mibintc wrote: > rjmccall wrote: > > mibintc wrote: > > > rjmccall wrote: > > > > mibintc wrote: > > > > > I changed this because the FAST version of this test > > > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute > > > > > on the instruction dump. All the LLVM FMF bits must be set for the > > > > > fast attribute print. By default, the value for OpenCL is > > > > > ffp-contract=on > > > > Is there an overall behavior change for OpenCL across these patches? > > > I think the answer to your question is no. Here is more details: OpenCL > > > sets the default behavior to ffp-contract=on. In > > > https://reviews.llvm.org/D72841 I added the function UpdateFastMathFlags, > > > that function set the llvm FMF.allowContract bit to be ( ffp-contract=on > > > or ffp-contract=fast). This patch just drops the check on > > > ffp-contract=on. I didn't wind back to see how the llvm fast attribute > > > was being set for this [opencl] test case originally. > > Well, to what extent are there (including this patch) overall test changes > > for the OpenCL tests, and what are tthey? > In the #pragma float_control patch https://reviews.llvm.org/D72841, I changed > 2 CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST cases to show the > contract bit. Also 1 line in single-precision-constant.cl for the same > reason, to show the contract bit. This patch undoes those test changes. I'll > do more investigation to understand why the fast bit isn't being set in the > FAST case in relaxed-fpmath.cl without the change to CompilerInovcaton Prior to the patch for #pragma float_control, the llvm.FastMathFlags was initialized from LangArgs.FastMath in the CodeGenFunction constructor approximately line 74, and the FMF values in IRBuilder were never changed--For the test clang/test/CodeGenOpenCL/relaxed-fpmath.cl with option -cl-fast-relaxed-math. (In ParseLangArgs, Opts.FastMath = Args.hasArg(OPT_ffast_math) || Args.hasArg(OPT_cl_fast_relaxed_math)) If FastMath is on, then all the llvm.FMF flags are set. The #pragma float_control patch does change the IRBuilder settings in the course of visiting the Expr nodes, using the information in the Expr nodes, but the initial setting of FPFeatures from the command line overlooked the fact that FastMath, and therefore ffp-contract=fast, is enabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc marked 2 inline comments as done.
mibintc added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
rjmccall wrote:
> mibintc wrote:
> > rjmccall wrote:
> > > mibintc wrote:
> > > > I changed this because the FAST version of this test
> > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute
> > > > on the instruction dump. All the LLVM FMF bits must be set for the
> > > > fast attribute print. By default, the value for OpenCL is
> > > > ffp-contract=on
> > > Is there an overall behavior change for OpenCL across these patches?
> > I think the answer to your question is no. Here is more details: OpenCL
> > sets the default behavior to ffp-contract=on. In
> > https://reviews.llvm.org/D72841 I added the function UpdateFastMathFlags,
> > that function set the llvm FMF.allowContract bit to be ( ffp-contract=on or
> > ffp-contract=fast). This patch just drops the check on ffp-contract=on.
> > I didn't wind back to see how the llvm fast attribute was being set for
> > this [opencl] test case originally.
> Well, to what extent are there (including this patch) overall test changes
> for the OpenCL tests, and what are tthey?
In the #pragma float_control patch https://reviews.llvm.org/D72841, I changed 2
CodeGen/OpenCL tests: relaxed-fp-math.cl in the non-FAST cases to show the
contract bit. Also 1 line in single-precision-constant.cl for the same reason,
to show the contract bit. This patch undoes those test changes. I'll do more
investigation to understand why the fast bit isn't being set in the FAST case
in relaxed-fpmath.cl without the change to CompilerInovcaton
Comment at: clang/lib/Sema/SemaAttr.cpp:460
case PFC_Push:
-Action = Sema::PSK_Push_Set;
-FpPragmaStack.Act(Loc, Action, StringRef(),
NewFPFeatures.getAsOpaqueInt());
+if (FpPragmaStack.Stack.empty()) {
+ FpPragmaStack.Act(Loc, Sema::PSK_Set, StringRef(),
rjmccall wrote:
> mibintc wrote:
> > When i was adding a test, I realized that pragma float_control(push) then
> > pop wasn't working as expected. If the stack is empty, which is most of the
> > time, first need to push the current fp features onto the stack so they can
> > be restored at the pop
> It seems that the way `PragmaStack` is supposed to be used is that we're
> supposed to be using its `CurrentValue` instead of having a separate
> `CurFPFeatures`. But mostly it looks like there's a lot of functionality
> associated with `PragmaStack` that we aren't using at all because this is a
> substantially simpler mode; we could really just be using a
> `SmallVector`.
>
> Anyway, I guess this fix works, although it should be done in a separate
> patch.
I'll submit it in a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
rjmccall added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943 + if (Opts.FastRelaxedMath) +Opts.setDefaultFPContractMode(LangOptions::FPM_Fast); Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat); mibintc wrote: > rjmccall wrote: > > mibintc wrote: > > > I changed this because the FAST version of this test > > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on > > > the instruction dump. All the LLVM FMF bits must be set for the fast > > > attribute print. By default, the value for OpenCL is ffp-contract=on > > Is there an overall behavior change for OpenCL across these patches? > I think the answer to your question is no. Here is more details: OpenCL sets > the default behavior to ffp-contract=on. In https://reviews.llvm.org/D72841 > I added the function UpdateFastMathFlags, that function set the llvm > FMF.allowContract bit to be ( ffp-contract=on or ffp-contract=fast). This > patch just drops the check on ffp-contract=on. I didn't wind back to see > how the llvm fast attribute was being set for this [opencl] test case > originally. Well, to what extent are there (including this patch) overall test changes for the OpenCL tests, and what are tthey? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943 + if (Opts.FastRelaxedMath) +Opts.setDefaultFPContractMode(LangOptions::FPM_Fast); Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat); rjmccall wrote: > mibintc wrote: > > I changed this because the FAST version of this test > > clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on > > the instruction dump. All the LLVM FMF bits must be set for the fast > > attribute print. By default, the value for OpenCL is ffp-contract=on > Is there an overall behavior change for OpenCL across these patches? I think the answer to your question is no. Here is more details: OpenCL sets the default behavior to ffp-contract=on. In https://reviews.llvm.org/D72841 I added the function UpdateFastMathFlags, that function set the llvm FMF.allowContract bit to be ( ffp-contract=on or ffp-contract=fast). This patch just drops the check on ffp-contract=on. I didn't wind back to see how the llvm fast attribute was being set for this [opencl] test case originally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79903/new/ https://reviews.llvm.org/D79903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
rjmccall added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
mibintc wrote:
> I changed this because the FAST version of this test
> clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on the
> instruction dump. All the LLVM FMF bits must be set for the fast attribute
> print. By default, the value for OpenCL is ffp-contract=on
Is there an overall behavior change for OpenCL across these patches?
Comment at: clang/lib/Sema/SemaAttr.cpp:460
case PFC_Push:
-Action = Sema::PSK_Push_Set;
-FpPragmaStack.Act(Loc, Action, StringRef(),
NewFPFeatures.getAsOpaqueInt());
+if (FpPragmaStack.Stack.empty()) {
+ FpPragmaStack.Act(Loc, Sema::PSK_Set, StringRef(),
mibintc wrote:
> When i was adding a test, I realized that pragma float_control(push) then pop
> wasn't working as expected. If the stack is empty, which is most of the time,
> first need to push the current fp features onto the stack so they can be
> restored at the pop
It seems that the way `PragmaStack` is supposed to be used is that we're
supposed to be using its `CurrentValue` instead of having a separate
`CurFPFeatures`. But mostly it looks like there's a lot of functionality
associated with `PragmaStack` that we aren't using at all because this is a
substantially simpler mode; we could really just be using a
`SmallVector`.
Anyway, I guess this fix works, although it should be done in a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc marked 3 inline comments as done.
mibintc added a comment.
added some inline explanation
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
I changed this because the FAST version of this test
clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on the
instruction dump. All the LLVM FMF bits must be set for the fast attribute
print. By default, the value for OpenCL is ffp-contract=on
Comment at: clang/lib/Sema/SemaAttr.cpp:460
case PFC_Push:
-Action = Sema::PSK_Push_Set;
-FpPragmaStack.Act(Loc, Action, StringRef(),
NewFPFeatures.getAsOpaqueInt());
+if (FpPragmaStack.Stack.empty()) {
+ FpPragmaStack.Act(Loc, Sema::PSK_Set, StringRef(),
When i was adding a test, I realized that pragma float_control(push) then pop
wasn't working as expected. If the stack is empty, which is most of the time,
first need to push the current fp features onto the stack so they can be
restored at the pop
Comment at: clang/test/CodeGen/constrained-math-builtins.c:157
- // CHECK: call contract float @llvm.experimental.constrained.fmuladd.f32
+ // CHECK: call float @llvm.experimental.constrained.fmuladd.f32
// CHECK: fneg
most of the test changes here are just a revert of the test changes from the
original patch for float_control
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79903/new/
https://reviews.llvm.org/D79903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement
mibintc created this revision.
mibintc added reviewers: rjmccall, scanon.
Herald added a project: clang.
mibintc marked 3 inline comments as done.
mibintc added a comment.
added some inline explanation
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2943
+ if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
I changed this because the FAST version of this test
clang/test/CodeGenOpenCL/relaxed-fpmath.cl wants the 'fast' attribute on the
instruction dump. All the LLVM FMF bits must be set for the fast attribute
print. By default, the value for OpenCL is ffp-contract=on
Comment at: clang/lib/Sema/SemaAttr.cpp:460
case PFC_Push:
-Action = Sema::PSK_Push_Set;
-FpPragmaStack.Act(Loc, Action, StringRef(),
NewFPFeatures.getAsOpaqueInt());
+if (FpPragmaStack.Stack.empty()) {
+ FpPragmaStack.Act(Loc, Sema::PSK_Set, StringRef(),
When i was adding a test, I realized that pragma float_control(push) then pop
wasn't working as expected. If the stack is empty, which is most of the time,
first need to push the current fp features onto the stack so they can be
restored at the pop
Comment at: clang/test/CodeGen/constrained-math-builtins.c:157
- // CHECK: call contract float @llvm.experimental.constrained.fmuladd.f32
+ // CHECK: call float @llvm.experimental.constrained.fmuladd.f32
// CHECK: fneg
most of the test changes here are just a revert of the test changes from the
original patch for float_control
Previously, the IRBuilder.FMF.allowContract was initialized to true if either
(ffp-contract=fast or ffp-contract=on) ; with this patch that bit will only be
set if ffp-contract=fast. This problem was pointed out by michele.scandale in
a reply to https://reviews.llvm.org/D72841
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79903
Files:
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaAttr.cpp
clang/test/CodeGen/constrained-math-builtins.c
clang/test/CodeGen/fp-contract-on-pragma.cpp
clang/test/CodeGen/fp-contract-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-class.cpp
clang/test/CodeGen/fp-floatcontrol-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-stack.cpp
clang/test/CodeGenOpenCL/relaxed-fpmath.cl
clang/test/CodeGenOpenCL/single-precision-constant.cl
Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | FileCheck %s
float fn(float f) {
- // CHECK: tail call contract float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
+ // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
return f*2. + 1.;
}
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -8,12 +8,12 @@
float spscalardiv(float a, float b) {
// CHECK: @spscalardiv(
- // NORMAL: fdiv contract float
+ // NORMAL: fdiv float
// FAST: fdiv fast float
- // FINITE: fdiv nnan ninf contract float
- // UNSAFE: fdiv nnan nsz contract float
- // MAD: fdiv contract float
- // NOSIGNED: fdiv nsz contract float
+ // FINITE: fdiv nnan ninf float
+ // UNSAFE: fdiv nnan nsz float
+ // MAD: fdiv float
+ // NOSIGNED: fdiv nsz float
return a / b;
}
// CHECK: attributes
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -9,7 +9,7 @@
float fun_default FUN(1)
//CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
// Note that backend wants constrained intrinsics used
@@ -37,7 +37,7 @@
//CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if NOHONOR
-//CHECK-NOHONOR: nnan ninf contract float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
+//CHECK-NOHONOR: nnan ninf float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
#endif
#if FAST
//Not possible to enable float_control(except) in FAST mode.
@@ -49,13 +49,13 @@
float exc_pop FUN(5)
//CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
#if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
#endif
#if EBSTRICT
/
