Re: [PATCH] D24481: make “#pragma STDC FP_CONTRACT” on by default
scanon accepted this revision. scanon added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D24481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24481: make “#pragma STDC FP_CONTRACT” on by default
Without digging into them yet, these are almost caused by overly-sensitive tests that are erroneously expecting bit-exact results. - Steve Sent from my iPhone > On Sep 23, 2016, at 4:42 PM, Renato Golin wrote: > > rengolin added a subscriber: rengolin. > rengolin added a comment. > > Folks, this commit has broken both AArch64 test-suite buildbots: > > http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/3162 > > http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/10449 > > I have reverted in r282289, let me know if you need help testing on AArch64. > > > Repository: > rL LLVM > > https://reviews.llvm.org/D24481 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24481: make “#pragma STDC FP_CONTRACT” on by default
That's not a great long-term solution for obvious reasons, but I think it's a great quick fix. Sent from my iPhone > On Sep 23, 2016, at 5:53 PM, Hal Finkel wrote: > > We currently have logic in the test suite that sets -ffp-contract=off on > PowerPC (because the default for GCC and other compilers on PowerPC/Linux > systems is essentially -ffp-contract=fast). We might just want to do this now > for all platforms. > > -Hal > > - Original Message - >> From: "Steve Canon" >> To: reviews+d24481+public+c0b8d50a92298...@reviews.llvm.org >> Cc: "a skolnik" , clatt...@apple.com, >> hfin...@anl.gov, "yaxun liu" , >> cfe-commits@lists.llvm.org >> Sent: Friday, September 23, 2016 4:47:32 PM >> Subject: Re: [PATCH] D24481: make “#pragma STDC FP_CONTRACT” on by default >> >> Without digging into them yet, these are almost caused by >> overly-sensitive tests that are erroneously expecting bit-exact >> results. >> >> - Steve >> >> Sent from my iPhone >> >>> On Sep 23, 2016, at 4:42 PM, Renato Golin >>> wrote: >>> >>> rengolin added a subscriber: rengolin. >>> rengolin added a comment. >>> >>> Folks, this commit has broken both AArch64 test-suite buildbots: >>> >>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/3162 >>> >>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/10449 >>> >>> I have reverted in r282289, let me know if you need help testing on >>> AArch64. >>> >>> >>> Repository: >>> rL LLVM >>> >>> https://reviews.llvm.org/D24481 >>> >>> >>> >> > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14200: Make FP_CONTRACT ON default.
scanon updated this revision to Diff 39205.
scanon added a comment.
Additionally test contraction of compound assignment expressions.
http://reviews.llvm.org/D14200
Files:
include/clang/Basic/LangOptions.def
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/aarch64-neon-fma.c
test/CodeGen/aarch64-scalar-fma.c
test/CodeGen/fp-contract-pragma.cpp
test/Driver/clang_f_opts.c
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -36,8 +36,10 @@
// RUN: %clang -### -S -ffp-contract=fast %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-FAST-CHECK %s
// RUN: %clang -### -S -ffast-math %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-FAST-CHECK %s
// RUN: %clang -### -S -ffp-contract=off %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-OFF-CHECK %s
+// RUN: %clang -### -S -ffp-contract=on %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-ON-CHECK %s
// FP-CONTRACT-FAST-CHECK: -ffp-contract=fast
// FP-CONTRACT-OFF-CHECK: -ffp-contract=off
+// FP-CONTRACT-ON-CHECK: -ffp-contract=on
// RUN: %clang -### -S -funroll-loops %s 2>&1 | FileCheck -check-prefix=CHECK-UNROLL-LOOPS %s
// RUN: %clang -### -S -fno-unroll-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-UNROLL-LOOPS %s
Index: test/CodeGen/fp-contract-pragma.cpp
===
--- test/CodeGen/fp-contract-pragma.cpp
+++ test/CodeGen/fp-contract-pragma.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+#pragma STDC FP_CONTRACT OFF
+
// Is FP_CONTRACT is honored in a simple case?
float fp_contract_1(float a, float b, float c) {
// CHECK: _Z13fp_contract_1fff
Index: test/CodeGen/aarch64-scalar-fma.c
===
--- test/CodeGen/aarch64-scalar-fma.c
+++ test/CodeGen/aarch64-scalar-fma.c
@@ -0,0 +1,177 @@
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=fast -S -o - %s | FileCheck -check-prefix=CHECK-FAST -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=on -S -o - %s | FileCheck -check-prefix=CHECK-ON -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=off -S -o - %s | FileCheck -check-prefix=CHECK-OFF -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -S -o - %s | FileCheck -check-prefix=CHECK-ON -check-prefix=CHECK-ALL %s
+// REQUIRES: aarch64-registered-target
+
+float test1(float x, float y, float z) {
+ return x*y + z;
+ // CHECK-ALL-LABEL: test1:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test2(double x, double y, double z) {
+ z -= x*y;
+ return z;
+ // CHECK-ALL-LABEL: test2:
+ // CHECK-FAST: fmsub
+ // CHECK-ON: fmsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+float test3(float x, float y, float z) {
+ float tmp = x*y;
+ return tmp + z;
+ // CHECK-ALL-LABEL: test3:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test4(double x, double y, double z) {
+ double tmp = x*y;
+ return tmp - z;
+ // CHECK-ALL-LABEL: test4:
+ // CHECK-FAST: fnmsub
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+#pragma STDC FP_CONTRACT ON
+
+float test5(float x, float y, float z) {
+ return x*y + z;
+ // CHECK-ALL-LABEL: test5:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test6(double x, double y, double z) {
+ z -= x*y;
+ return z;
+ // CHECK-ALL-LABEL: test6:
+ // CHECK-FAST: fmsub
+ // CHECK-ON: fmsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+float test7(float x, float y, float z) {
+ float tmp = x*y;
+ return tmp + z;
+ // CHECK-ALL-LABEL: test7:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test8(double x, double y, double z) {
+ double tmp = x*y;
+ return tmp - z;
+ // CHECK-ALL-LABEL: test8:
+ // CHECK-FAST: fnmsub
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+#pragma STDC FP_CONTRACT OFF
+
+float test9(float x, float y, float z) {
+ return x*y + z;
+ // CHECK-ALL-LABEL: test9:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test10(double x, double y, double z) {
+ z -= x*y;
+ return z;
+ // CHECK-ALL-LABEL: test10:
+ // CHECK-FAST: fmsub
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+float test11(float x, float y, float z) {
+ float tmp = x*y;
+ return tmp + z;
+ // CHECK-ALL-LABEL: test11:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fadd
+ // CHECK-OFF:
Re: [PATCH] D20341: [CUDA] Enable fusing FP ops for CUDA by default.
scanon added a comment. `-ffp-contract=on` obeys the semantics of C's FP_CONTRACT pragma. In particular, it will not fuse: float m = x*y; float a = m + z; Whereas you probably want that to fuse for your purposes. `-ffp-contract=fast` seems more in line with your needs. http://reviews.llvm.org/D20341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
scanon added a comment. I agree with Marshall. Aside from the points he raises, this approach looks fine. http://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18639: Use __builtin_isnan/isinf/isfinite in complex
scanon added a comment. I am not the right person to review the C++ template details, but everything else seems OK to me. https://reviews.llvm.org/D18639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14200: Make FP_CONTRACT ON default.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL253269: Make FP_CONTRACT ON the default. (authored by
scanon).
Changed prior to commit:
http://reviews.llvm.org/D14200?vs=39205&id=40351#toc
Repository:
rL LLVM
http://reviews.llvm.org/D14200
Files:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/aarch64-neon-fma.c
cfe/trunk/test/CodeGen/aarch64-scalar-fma.c
cfe/trunk/test/CodeGen/fp-contract-pragma.cpp
cfe/trunk/test/Driver/clang_f_opts.c
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1336,7 +1336,6 @@
Opts.ZVector = 0;
Opts.CXXOperatorNames = 1;
Opts.LaxVectorConversions = 0;
-Opts.DefaultFPContract = 1;
Opts.NativeHalfType = 1;
}
Index: cfe/trunk/include/clang/Basic/LangOptions.def
===
--- cfe/trunk/include/clang/Basic/LangOptions.def
+++ cfe/trunk/include/clang/Basic/LangOptions.def
@@ -187,7 +187,7 @@
BENIGN_LANGOPT(SpellChecking , 1, 1, "spell-checking")
LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating point constants as single precision constants")
LANGOPT(FastRelaxedMath , 1, 0, "OpenCL fast relaxed math")
-LANGOPT(DefaultFPContract , 1, 0, "FP_CONTRACT")
+LANGOPT(DefaultFPContract , 1, 1, "FP_CONTRACT")
LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field type alignment")
LANGOPT(HexagonQdsp6Compat , 1, 0, "hexagon-qdsp6 backward compatibility")
LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting")
Index: cfe/trunk/test/Driver/clang_f_opts.c
===
--- cfe/trunk/test/Driver/clang_f_opts.c
+++ cfe/trunk/test/Driver/clang_f_opts.c
@@ -36,8 +36,10 @@
// RUN: %clang -### -S -ffp-contract=fast %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-FAST-CHECK %s
// RUN: %clang -### -S -ffast-math %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-FAST-CHECK %s
// RUN: %clang -### -S -ffp-contract=off %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-OFF-CHECK %s
+// RUN: %clang -### -S -ffp-contract=on %s 2>&1 | FileCheck -check-prefix=FP-CONTRACT-ON-CHECK %s
// FP-CONTRACT-FAST-CHECK: -ffp-contract=fast
// FP-CONTRACT-OFF-CHECK: -ffp-contract=off
+// FP-CONTRACT-ON-CHECK: -ffp-contract=on
// RUN: %clang -### -S -funroll-loops %s 2>&1 | FileCheck -check-prefix=CHECK-UNROLL-LOOPS %s
// RUN: %clang -### -S -fno-unroll-loops %s 2>&1 | FileCheck -check-prefix=CHECK-NO-UNROLL-LOOPS %s
Index: cfe/trunk/test/CodeGen/fp-contract-pragma.cpp
===
--- cfe/trunk/test/CodeGen/fp-contract-pragma.cpp
+++ cfe/trunk/test/CodeGen/fp-contract-pragma.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+#pragma STDC FP_CONTRACT OFF
+
// Is FP_CONTRACT is honored in a simple case?
float fp_contract_1(float a, float b, float c) {
// CHECK: _Z13fp_contract_1fff
Index: cfe/trunk/test/CodeGen/aarch64-scalar-fma.c
===
--- cfe/trunk/test/CodeGen/aarch64-scalar-fma.c
+++ cfe/trunk/test/CodeGen/aarch64-scalar-fma.c
@@ -0,0 +1,177 @@
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=fast -S -o - %s | FileCheck -check-prefix=CHECK-FAST -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=on -S -o - %s | FileCheck -check-prefix=CHECK-ON -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -ffp-contract=off -S -o - %s | FileCheck -check-prefix=CHECK-OFF -check-prefix=CHECK-ALL %s
+// RUN: %clang_cc1 -triple=aarch64-unknown -Os -S -o - %s | FileCheck -check-prefix=CHECK-ON -check-prefix=CHECK-ALL %s
+// REQUIRES: aarch64-registered-target
+
+float test1(float x, float y, float z) {
+ return x*y + z;
+ // CHECK-ALL-LABEL: test1:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test2(double x, double y, double z) {
+ z -= x*y;
+ return z;
+ // CHECK-ALL-LABEL: test2:
+ // CHECK-FAST: fmsub
+ // CHECK-ON: fmsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+float test3(float x, float y, float z) {
+ float tmp = x*y;
+ return tmp + z;
+ // CHECK-ALL-LABEL: test3:
+ // CHECK-FAST: fmadd
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fadd
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fadd
+}
+
+double test4(double x, double y, double z) {
+ double tmp = x*y;
+ return tmp - z;
+ // CHECK-ALL-LABEL: test4:
+ // CHECK-FAST: fnmsub
+ // CHECK-ON: fmul
+ // CHECK-ON-NEXT: fsub
+ // CHECK-OFF: fmul
+ // CHECK-OFF-NEXT: fsub
+}
+
+#pragma STDC FP_CONTRACT ON
+
+float test5(float x, float y, float z) {
+ return x*y + z;
+ //
[PATCH] D14891: Replace assert with early-out in tryEmitFMulAdd
scanon created this revision.
scanon added reviewers: hfinkel, resistor, lhames.
scanon added a subscriber: cfe-commits.
r253269 exposed a pre-existing issue in tryEmitFMulAdd, where we would assert
if a fusable operation had operands with multiple uses (we didn't see it
previously because FP_CONTRACT was off by default). Fix this issue by changing
the assert into an early-out where we simply won't try to fuse.
Once this is done, we can try again at enabling FP_CONTRACT ON as a default.
http://reviews.llvm.org/D14891
Files:
lib/CodeGen/CGExprScalar.cpp
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2563,22 +2563,19 @@
if (CGF.CGM.getCodeGenOpts().getFPContractMode() != CodeGenOptions::FPC_On)
return nullptr;
- // We have a potentially fusable op. Look for a mul on one of the operands.
+ // We have a potentially fusable op. Look for a mul on one of the operands
+ // that does not have any other uses.
if (llvm::BinaryOperator* LHSBinOp = dyn_cast(op.LHS))
{
-if (LHSBinOp->getOpcode() == llvm::Instruction::FMul) {
- assert(LHSBinOp->getNumUses() == 0 &&
- "Operations with multiple uses shouldn't be contracted.");
+if (LHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+LHSBinOp->hasNUses(0))
return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
-}
- } else if (llvm::BinaryOperator* RHSBinOp =
- dyn_cast(op.RHS)) {
-if (RHSBinOp->getOpcode() == llvm::Instruction::FMul) {
- assert(RHSBinOp->getNumUses() == 0 &&
- "Operations with multiple uses shouldn't be contracted.");
+ }
+ if (llvm::BinaryOperator* RHSBinOp = dyn_cast(op.RHS))
{
+if (RHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+RHSBinOp->hasNUses(0))
return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
-}
}
-
+
return nullptr;
}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2563,22 +2563,19 @@
if (CGF.CGM.getCodeGenOpts().getFPContractMode() != CodeGenOptions::FPC_On)
return nullptr;
- // We have a potentially fusable op. Look for a mul on one of the operands.
+ // We have a potentially fusable op. Look for a mul on one of the operands
+ // that does not have any other uses.
if (llvm::BinaryOperator* LHSBinOp = dyn_cast(op.LHS)) {
-if (LHSBinOp->getOpcode() == llvm::Instruction::FMul) {
- assert(LHSBinOp->getNumUses() == 0 &&
- "Operations with multiple uses shouldn't be contracted.");
+if (LHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+LHSBinOp->hasNUses(0))
return buildFMulAdd(LHSBinOp, op.RHS, CGF, Builder, false, isSub);
-}
- } else if (llvm::BinaryOperator* RHSBinOp =
- dyn_cast(op.RHS)) {
-if (RHSBinOp->getOpcode() == llvm::Instruction::FMul) {
- assert(RHSBinOp->getNumUses() == 0 &&
- "Operations with multiple uses shouldn't be contracted.");
+ }
+ if (llvm::BinaryOperator* RHSBinOp = dyn_cast(op.RHS)) {
+if (RHSBinOp->getOpcode() == llvm::Instruction::FMul &&
+RHSBinOp->hasNUses(0))
return buildFMulAdd(RHSBinOp, op.LHS, CGF, Builder, isSub, false);
-}
}
-
+
return nullptr;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14891: Replace assert with early-out in tryEmitFMulAdd
scanon added a comment. Friendly ping. http://reviews.llvm.org/D14891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15165: change an assert when generating fmuladd to an ordinary 'if' check (PR25719)
scanon added a comment. This is mostly http://reviews.llvm.org/D14891 with a test case added, but http://reviews.llvm.org/D14891 also fixed a second very minor issue: that the "else if" should just be "if". Also, majnemer made a few style suggestions there that it would be nice to adopt. Either way, we should merge the two patches. This can be the canonical one if you want to update it. http://reviews.llvm.org/D15165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15165: change an assert when generating fmuladd to an ordinary 'if' check (PR25719)
scanon added a comment. LGTM. http://reviews.llvm.org/D15165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14891: Replace assert with early-out in tryEmitFMulAdd
scanon abandoned this revision. scanon added a comment. Abandoned for http://reviews.llvm.org/D15165. http://reviews.llvm.org/D14891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
