Always honor fp-contract pragmas on PlayStation

[wjristow]

The pragma:
#pragma clang fp contract (off)

can be used to disable FMA, but when cross-statement FMA is enabled in fast fp contract mode (via -ffast-math, for example), the effect of that pragma is suppressed.

To have Clang honor that pragma in 'fast' fp contract mode in C/C++, an additional switch:
-ffp-contract=fast-honor-pragmas

must be applied. On PlayStation, we want to always honor that pragma, without requiring the additional switch to be specified. This commit does that.

[llvmbot]

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: None (wjristow)

Changes

The pragma:
#pragma clang fp contract (off)

can be used to disable FMA, but when cross-statement FMA is enabled in fast fp contract mode (via -ffast-math, for example), the effect of that pragma is suppressed.

To have Clang honor that pragma in 'fast' fp contract mode in C/C++, an additional switch:
-ffp-contract=fast-honor-pragmas

must be applied. On PlayStation, we want to always honor that pragma, without requiring the additional switch to be specified. This commit does that.

---

Full diff: https://github.com/llvm/llvm-project/pull/162549.diff

2 Files Affected:

• (modified) clang/lib/CodeGen/BackendUtil.cpp (+5-1)
• (added) clang/test/CodeGen/X86/fma-fast-pragma.cpp (+53)

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 64f1917739e12..52bf702cdadb0 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -396,7 +396,11 @@ static bool initTargetOptions(const CompilerInstance &CI,
     Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
     break;
   case LangOptions::FPM_Fast:
-    Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
+    // We always honor fp-contract pragmas for PlayStation.
+    if (CI.getASTContext().getTargetInfo().getTriple().isPS())
+      Options.AllowFPOpFusion = llvm::FPOpFusion::Standard;
+    else
+      Options.AllowFPOpFusion = llvm::FPOpFusion::Fast;
     break;
   }
 
diff --git a/clang/test/CodeGen/X86/fma-fast-pragma.cpp b/clang/test/CodeGen/X86/fma-fast-pragma.cpp
new file mode 100644
index 0000000000000..9d38e245b5f64
--- /dev/null
+++ b/clang/test/CodeGen/X86/fma-fast-pragma.cpp
@@ -0,0 +1,53 @@
+// REQUIRES: x86-registered-target
+
+// With the pragma in place, generic targets leave FMA enabled unless the
+// switch '-ffp-contract=fast-honor-pragmas' is used to disable it; whereas
+// for PlayStation, the pragma is always honored, so FMA is disabled even in
+// plain 'fast' mode:
+// RUN: %clang_cc1 -S -triple x86_64-unknown-unknown -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-YES-FMA %s
+// RUN: %clang_cc1 -S -triple x86_64-unknown-unknown -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast-honor-pragmas -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-FMA %s
+// RUN: %clang_cc1 -S -triple x86_64-unknown-unknown -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast -ffp-contract=fast-honor-pragmas -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-FMA %s
+// RUN: %clang_cc1 -S -triple x86_64-sie-ps5 -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-FMA %s
+// RUN: %clang_cc1 -S -triple x86_64-sie-ps5 -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast-honor-pragmas -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-NO-FMA %s
+//
+// With the pragma suppressed, FMA happens in 'fast' or 'fast-honor-pragmas'
+// modes (for generic targets and for PlayStation):
+// RUN: %clang_cc1 -S -DSUPPRESS_PRAGMA \
+// RUN:   -triple x86_64-unknown-unknown -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-YES-FMA %s
+// RUN: %clang_cc1 -S -DSUPPRESS_PRAGMA \
+// RUN:   -triple x86_64-unknown-unknown -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast-honor-pragmas -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-YES-FMA %s
+// RUN: %clang_cc1 -S -DSUPPRESS_PRAGMA \
+// RUN:   -triple x86_64-sie-ps5 -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-YES-FMA %s
+// RUN: %clang_cc1 -S -DSUPPRESS_PRAGMA \
+// RUN:   -triple x86_64-sie-ps5 -target-feature +fma \
+// RUN:   -O2 -ffp-contract=fast-honor-pragmas -o - %s | \
+// RUN:   FileCheck --check-prefix=CHECK-YES-FMA %s
+//
+float compute(float a, float b, float c) {
+#if !defined(SUPPRESS_PRAGMA)
+#pragma clang fp contract (off)
+#endif
+  float product = a * b;
+  return product + c;
+}
+
+// CHECK-NO-FMA: vmulss
+// CHECK-NO-FMA-NEXT: vaddss
+
+// CHECK-YES-FMA: vfmadd213ss

[wjristow]

The test-case of this patch generates X86 assembly code, rather than the usual approach of generating generic IR. The reason I did that is because the impact of the change isn't visible in the IR. That's due to a bug, which I've just filed: #162550

If this patch is accepted, we will want to update the documentation of -ffp-contract=<value>:
https://clang.llvm.org/docs/UsersManual.html#cmdoption-ffp-contract

For reference, here are some points of discussion about the -ffp-contract=fast-honor-pragmas concept:
https://discourse.llvm.org/t/fp-contract-fast-and-pragmas/58529
https://discourse.llvm.org/t/rfc-honor-pragmas-with-ffp-contract-fast/80797/14

[andykaylor]

I continue to believe that ignoring pragmas when fast-math is enabled is a bug, and I'd like to see a more general change such as I proposed in https://github.com/llvm/llvm-project/pull/105746.

[wjristow]

I continue to believe that ignoring pragmas when fast-math is enabled is a bug, and I'd like to see a more general change such as I proposed in https://github.com/llvm/llvm-project/pull/105746.

FWIW, that makes good sense to me. I admit I only looked at that PR of yours carefully today, as I was preparing this one.

[efriedma-quic]

Maybe time to revisit #105746; the situation in the backends looks a lot better. Looks like Arm and Mips still have some problematic isel patterns; everything else looks okay at first glance?

We could blacklist specific backends which still have issues.

[wjristow]

Maybe time to revisit #105746; the situation in the backends looks a lot better. Looks like Arm and Mips still have some problematic isel patterns; everything else looks okay at first glance?

We could blacklist specific backends which still have issues.

That sounds good to me. Like Andy, I also view the current behavior as a bug. And I view this PR of mine as "fix that bug for PlayStation", whereas I view #105746 as "fix that bug, for all". So I'd be happy to have that done.

---

And as an aside, if we do Andy's full fix, then the bug I reported as related to this (#162550) becomes moot. That bug is essentially: In LTO mode the pragma is always honored. Or more directly, the switch -ffp-contract=fast-honor-pragmas is completely ineffective in LTO mode; so if that feature is removed, there is nothing there to be fixed.