[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-15 Thread Warren Ristow via Phabricator via cfe-commits
wristow added a comment.

Thanks @zahiraam !  The updated ReleaseNote LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 387337.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,11 +205,16 @@
 
 Floating Point Support in Clang
 ---
-- The -ffp-model=precise now implies -ffp-contract=on rather than
-  -ffp-contract=fast, and the documentation of these features has been
-  clarified. Previously, the documentation claimed that -ffp-model=precise was
-  the default, but this was incorrect because the precise model implied
-  -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
+- The default setting of FP contraction (FMA) is now -ffp-contract=on (for
+  languages other than CUDA/HIP) even when optimization is off. Previously,
+  the default behavior was equivalent to -ffp-contract=off (-ffp-contract
+  was not set).
+  Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
+  rather than -ffp-contract=fast, and the documentation of these features has
+  been clarified. Previously, the documentation claimed that -ffp-model=precise
+  was the default, but this was incorrect because the precise model implied
+  -ffp-contract=fast, wheras the (now corrected) default behavior is
+  -ffp-contract=on.
   -ffp-model=precise is now exactly the default mode of the compiler.
 
 Internal API Changes


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,11 +205,16 @@
 
 Floating Point Support in Clang
 ---
-- The -ffp-model=precise now implies -ffp-contract=on rather than
-  -ffp-contract=fast, and the documentation of these features has been
-  clarified. Previously, the documentation claimed that -ffp-model=precise was
-  the default, but this was incorrect because the precise model implied
-  -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
+- The default setting of FP contraction (FMA) is now -ffp-contract=on (for
+  languages other than CUDA/HIP) even when optimization is off. Previously,
+  the default behavior was equivalent to -ffp-contract=off (-ffp-contract
+  was not set).
+  Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
+  rather than -ffp-contract=fast, and the documentation of these features has
+  been clarified. Previously, the documentation claimed that -ffp-model=precise
+  was the default, but this was incorrect because the precise model implied
+  -ffp-contract=fast, wheras the (now corrected) default behavior is
+  -ffp-contract=on.
   -ffp-model=precise is now exactly the default mode of the compiler.
 
 Internal API Changes
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-15 Thread Warren Ristow via Phabricator via cfe-commits
wristow added a comment.

In D107994#3131268 , @zahiraam wrote:

> In D107994#3130494 , @wristow wrote:
>
>> The Release Note change here says:
>>
>>   Floating Point Support in Clang
>>   ---
>>   - The -ffp-model=precise now implies -ffp-contract=on rather than
>> -ffp-contract=fast, and the documentation of these features has been
>> clarified. Previously, the documentation claimed that -ffp-model=precise 
>> was
>> the default, but this was incorrect because the precise model implied
>> -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
>> -ffp-model=precise is now exactly the default mode of the compiler.
>>
>> Unless I'm missing something, there is a related change here that I think 
>> should be more overtly noted (given the discussions in this review, I 
>> //think// this additional change is understood/expected, but I'm surprised 
>> it's not pointed out explicitly -- so maybe I'm misunderstanding).
>>
>> Specifically, this commit explicitly sets `-ffp-contract=on` in the default 
>> mode (which is what the documentation said, and continues to say).  But 
>> previously, there wasn't //any// explicit setting of `-ffp-contract` by 
>> default (and I think that lack of an explicit setting, was equivalent to 
>> `-ffp-contract=off`).
>> ...
>
> @wristow Are you suggesting a change of wording in the ReleaseNotes?

Yes @zahiraam, either a modification to what was written, or an additional 
separate point.  The critical user-visible change from this commit (AFAICS) is 
that previously the default behavior was `-ffp-contract=off`, whereas now the 
default behavior is `-ffp-contract=on`.  The ReleaseNote as written implies 
that the FMA situation //was// `-ffp-contract=fast`, and it has been changed to 
`-ffp-contract=on`, and either of those modes would result in FMA being done 
for cases like:

  float foo(float a, float b, float c) {
return a * b + c;
  }

In short, the current ReleaseNote implies that FMA would be used by default, 
for cases like the above (because `-ffp-contract=on` was the default) -- but 
this isn't true.  (The current ReleaseNote also clarifies that if 
`-ffp-model=precise` were explicitly specified, then it previously would set 
`-ffp-contract=fast` (which was not what the documentation said) and now it 
sets `-ffp-contract=on` (which //is// what the documentation says) -- this 
aspect is correct.)

I think the ReleaseNote should clarify that the result of this change is that 
previously, the default behavior was equivalent to `-ffp-contract=off` (and so 
no FMA would be done by default), but this commit makes the default behavior 
`-ffp-contract=on` (so FMA is enabled by default, even at `-O0`).  The 
documentation indicates that the default is `-ffp-contract=on`, so this change 
makes the compiler behavior consistent with the documentation, with respect to 
the `-ffp-contact` switch.  It also makes it consistent with the documentation 
for the `-ffp-model` switch.

A possible new wording is below (maybe this is too verbose, so making it more 
concise is fine too, from my POV):

  Floating Point Support in Clang
  ---
  - The default setting of FP contraction (FMA) is now -ffp-contract=on (for
languages other than CUDA/HIP).  This is consistent with the documentation.
Previously, the default behavior was equivalent to -ffp-contract=off, so
the old behavior was that FMA would not be used by default for code like:
float foo(float a, float b, float c) {
  return a * b + c;
}
Whereas now, FMA will be used in the above code, even when optimization
is off.
  
Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
rather than -ffp-contract=fast, and the documentation of these features has
been clarified.  Previously, the documentation claimed that
-ffp-model=precise was the default, but this was incorrect because the
precise model implied -ffp-contract=fast, whereas the (now corrected)
default behavior is -ffp-contract=on.
-ffp-model=precise is now exactly the default mode of the compiler.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-15 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

In D107994#3130494 , @wristow wrote:

> The Release Note change here says:
>
>   Floating Point Support in Clang
>   ---
>   - The -ffp-model=precise now implies -ffp-contract=on rather than
> -ffp-contract=fast, and the documentation of these features has been
> clarified. Previously, the documentation claimed that -ffp-model=precise 
> was
> the default, but this was incorrect because the precise model implied
> -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
> -ffp-model=precise is now exactly the default mode of the compiler.
>
> Unless I'm missing something, there is a related change here that I think 
> should be more overtly noted (given the discussions in this review, I 
> //think// this additional change is understood/expected, but I'm surprised 
> it's not pointed out explicitly -- so maybe I'm misunderstanding).
>
> Specifically, this commit explicitly sets `-ffp-contract=on` in the default 
> mode (which is what the documentation said, and continues to say).  But 
> previously, there wasn't //any// explicit setting of `-ffp-contract` by 
> default (and I think that lack of an explicit setting, was equivalent to 
> `-ffp-contract=off`).
>
> So with this commit, we now enable FMA by default (even at `-O0`). Noting the 
> semantic change that FMA is now being enabled by default seems sensible.
>
> Succinctly, in terms of the Release Note, the documentation claims that 
> `-ffp-contract=on` is the default, but in fact the behavior //was// as though 
> `-ffp-contract=off` was the default.  The default now really is 
> `-ffp-contract=on`.
>
> __
>
> Also, I see a relatively minor point about `-ffp-contract` in the Users 
> Manual.  It says that setting `-ffast-math` implies `-ffp-contract=fast`, and 
> it says that setting `-ffp-model=fast` "Behaves identically to specifying 
> both `-ffast-math` and `ffp-contract=fast`".  But that's redundant, since 
> `-ffast-math` already implies  `-ffp-contract=fast`.  That is, `-ffast-math` 
> and `-ffp-model=fast` are equivalent.

@wristow Are you suggesting a change of wording in the ReleaseNotes?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-14 Thread Warren Ristow via Phabricator via cfe-commits
wristow added a comment.

The Release Note change here says:

  Floating Point Support in Clang
  ---
  - The -ffp-model=precise now implies -ffp-contract=on rather than
-ffp-contract=fast, and the documentation of these features has been
clarified. Previously, the documentation claimed that -ffp-model=precise was
the default, but this was incorrect because the precise model implied
-ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
-ffp-model=precise is now exactly the default mode of the compiler.

Unless I'm missing something, there is a related change here that I think 
should be more overtly noted (given the discussions in this review, I //think// 
this additional change is understood/expected, but I'm surprised it's not 
pointed out explicitly -- so maybe I'm misunderstanding).

Specifically, this commit explicitly sets `-ffp-contract=on` in the default 
mode (which is what the documentation said, and continues to say).  But 
previously, there wasn't //any// explicit setting of `-ffp-contract` by default 
(and I think that lack of an explicit setting, was equivalent to 
`-ffp-contract=off`).

So with this commit, we now enable FMA by default (even at `-O0`). Noting the 
semantic change that FMA is now being enabled by default seems sensible.

Succinctly, in terms of the Release Note, the documentation claims that 
`-ffp-contract=on` is the default, but in fact the behavior //was// as though 
`-ffp-contract=off` was the default.  The default now really is 
`-ffp-contract=on`.

__

Also, I see a relatively minor point about `-ffp-contract` in the Users Manual. 
 It says that setting `-ffast-math` implies `-ffp-contract=fast`, and it says 
that setting `-ffp-model=fast` "Behaves identically to specifying both 
`-ffast-math` and `ffp-contract=fast`".  But that's redundant, since 
`-ffast-math` already implies  `-ffp-contract=fast`.  That is, `-ffast-math` 
and `-ffp-model=fast` are equivalent.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-11 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor closed this revision.
andrew.w.kaylor added a comment.

Closed by commit by rGf04e387055e495e3e14570087d68e93593cf2918 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-10 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.

lgtm


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

In D107994#3115589 , @thakis wrote:

> This fixes tests on macOS, but I don't know if the space is there 
> intentionally to check for presence of any attributes:
>
>   % git diff
>   diff --git a/clang/test/CodeGen/ffp-contract-option.c 
> b/clang/test/CodeGen/ffp-contract-option.c
>   index 04d23d5f90b7..327722f56f4e 100644
>   --- a/clang/test/CodeGen/ffp-contract-option.c
>   +++ b/clang/test/CodeGen/ffp-contract-option.c
>   @@ -63,7 +63,7 @@
>// RUN: --check-prefixes=CHECK,CHECK-FPC-ON
>   
>float mymuladd(float x, float y, float z) {
>   -  // CHECK: define {{.*}} float @mymuladd(float noundef %x, float noundef 
> %y, float noundef %z)
>   +  // CHECK: define{{.*}} float @mymuladd(float noundef %x, float noundef 
> %y, float noundef %z)
>  return x * y + z;
>  // expected-warning{{overriding '-ffp-contract=fast' option with 
> '-ffp-contract=on'}}
>   
>   diff --git a/clang/test/CodeGen/ffp-model.c b/clang/test/CodeGen/ffp-model.c
>   index ee0eedb2718f..98d31106a119 100644
>   --- a/clang/test/CodeGen/ffp-model.c
>   +++ b/clang/test/CodeGen/ffp-model.c
>   @@ -19,7 +19,7 @@
>// RUN: --check-prefixes CHECK,CHECK-FAST1
>   
>float mymuladd(float x, float y, float z) {
>   -  // CHECK: define {{.*}} float @mymuladd(float noundef %x, float noundef 
> %y, float noundef %z)
>   +  // CHECK: define{{.*}} float @mymuladd(float noundef %x, float noundef 
> %y, float noundef %z)
>  return x * y + z;
>  // CHECK-FAST: fmul fast float
>
> If this is the right fix, please land it. Else I'll revert again in 30 min or 
> so; bots have been broken for hours by now.






Comment at: clang/test/CodeGen/ffp-model.c:4
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace
+

zahiraam wrote:
> andrew.w.kaylor wrote:
> > Why did you add the strict-whitespace option?
> Because on some targets, the IR generated for the define is "define 
> {{.*}}float @mymuladd" and on and some others it is 
> "define {{.*}} float @mymuladd" (space between {{.*}} and float.
I was trying to fix the issue with the define function check. 
With this run command:

// RUN: %clang -S -emit-llvm -fno-fast-math -emit-llvm -target 
x86_64-apple-macosx10.11 %s -o -
I get this IR:

define float @mymuladd(float noundef %x, float noundef %y, float noundef %z)

This wouldn't be represented by

// CHECK: define {{.*}} float @mymuladd

but by

// CHECK: define{{.*}} float @mymuladd

or

// CHECK: define {{.*}}float @mymuladd 
But I get replacing with // CHECK: define{{.*}} float @mymuladd
Should solve the issue. Right?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 386279.
zahiraam marked 2 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/CodeGen/ffp-model.c
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/Driver/fp-model.c
  clang/test/Misc/ffp-contract.c

Index: clang/test/Misc/ffp-contract.c
===
--- /dev/null
+++ clang/test/Misc/ffp-contract.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=aarch64-apple-darwin \
+// RUN: -S -o - %s | FileCheck --check-prefix=CHECK-FMADD %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+  // CHECK-FMADD: fmadd
+  float x = a * b;
+  float y = x + c;
+  return y;
+}
Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -99,7 +99,7 @@
 // RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"
+// CHECK-FPM-PRECISE: "-ffp-contract=on"
 // CHECK-FPM-PRECISE: "-fno-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -2,11 +2,11 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4
Index: clang/test/CodeGen/ffp-model.c
===
--- /dev/null
+++ clang/test/CodeGen/ffp-model.c
@@ -0,0 +1,48 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -S -emit-llvm -ffp-model=fast -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-FAST
+
+// RUN: %clang -S -emit-llvm -ffp-model=precise %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-PRECISE
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict %s -o - \
+// RUN: -target x86_64 | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict -ffast-math \
+// RUN: -target x86_64 %s -o - | FileCheck %s \
+// RUN: 

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments.



Comment at: clang/test/CodeGen/ffp-model.c:4
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace
+

andrew.w.kaylor wrote:
> Why did you add the strict-whitespace option?
Because on some targets, the IR generated for the define is "define {{.*}}float 
@mymuladd" and on and some others it is 
"define {{.*}} float @mymuladd" (space between {{.*}} and float.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 386124.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/CodeGen/ffp-model.c
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/Driver/fp-model.c
  clang/test/Misc/ffp-contract.c

Index: clang/test/Misc/ffp-contract.c
===
--- /dev/null
+++ clang/test/Misc/ffp-contract.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=aarch64-apple-darwin \
+// RUN: -S -o - %s | FileCheck --check-prefix=CHECK-FMADD %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+  // CHECK-FMADD: fmadd
+  float x = a * b;
+  float y = x + c;
+  return y;
+}
Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -99,7 +99,7 @@
 // RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"
+// CHECK-FPM-PRECISE: "-ffp-contract=on"
 // CHECK-FPM-PRECISE: "-fno-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -2,11 +2,11 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4
Index: clang/test/CodeGen/ffp-model.c
===
--- /dev/null
+++ clang/test/CodeGen/ffp-model.c
@@ -0,0 +1,52 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -S -emit-llvm -ffp-model=fast -emit-llvm %s -o - \
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=precise %s -o - \
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-PRECISE --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict %s -o - \
+// RUN: -target x86_64 | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-STRICT --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict -ffast-math \
+// RUN: -target 

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-09 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor added inline comments.



Comment at: clang/test/CodeGen/ffp-model.c:4
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace
+

Why did you add the strict-whitespace option?



Comment at: clang/test/CodeGen/ffp-model.c:10
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict %s -o - \
+// RUN: | FileCheck %s \

This is still going to have problems with targets that don't support strictfp 
unless you add an explicit target to the run line.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 386014.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/CodeGen/ffp-model.c
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/Driver/fp-model.c
  clang/test/Misc/ffp-contract.c

Index: clang/test/Misc/ffp-contract.c
===
--- /dev/null
+++ clang/test/Misc/ffp-contract.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -O3 -ffp-contract=fast -triple=aarch64-apple-darwin \
+// RUN: -S -o - %s | FileCheck --check-prefix=CHECK-FMADD %s
+// REQUIRES: aarch64-registered-target
+
+float fma_test1(float a, float b, float c) {
+  // CHECK-FMADD: fmadd
+  float x = a * b;
+  float y = x + c;
+  return y;
+}
Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -99,7 +99,7 @@
 // RUN: %clang -### -nostdinc -ffp-model=precise -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FPM-PRECISE %s
 // CHECK-FPM-PRECISE: "-cc1"
-// CHECK-FPM-PRECISE: "-ffp-contract=fast"
+// CHECK-FPM-PRECISE: "-ffp-contract=on"
 // CHECK-FPM-PRECISE: "-fno-rounding-math"
 
 // RUN: %clang -### -nostdinc -ffp-model=strict -c %s 2>&1 \
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -2,11 +2,11 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN: -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
Index: clang/test/CodeGen/ppc-emmintrin.c
===
--- clang/test/CodeGen/ppc-emmintrin.c
+++ clang/test/CodeGen/ppc-emmintrin.c
@@ -2,9 +2,9 @@
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:  -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN:  -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
 // RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
-// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN:   -ffp-contract=off -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
 
 // CHECK-BE-DAG: @_mm_movemask_pd.perm_mask = internal constant <4 x i32> , align 16
 // CHECK-BE-DAG: @_mm_shuffle_epi32.permute_selectors = internal constant [4 x i32] [i32 66051, i32 67438087, i32 134810123, i32 202182159], align 4
Index: clang/test/CodeGen/ffp-model.c
===
--- /dev/null
+++ clang/test/CodeGen/ffp-model.c
@@ -0,0 +1,52 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -S -emit-llvm -ffp-model=fast -emit-llvm %s -o - \
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=precise %s -o - \
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-PRECISE --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict %s -o - \
+// RUN: | FileCheck %s \
+// RUN: --check-prefixes=CHECK,CHECK-STRICT --strict-whitespace
+
+// RUN: %clang -S -emit-llvm -ffp-model=strict -ffast-math \
+// RUN: %s -o - | FileCheck %s \

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

2021-11-08 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This fixes tests on macOS, but I don't know if the space is there intentionally 
to check for presence of any attributes:

  % git diff
  diff --git a/clang/test/CodeGen/ffp-contract-option.c 
b/clang/test/CodeGen/ffp-contract-option.c
  index 04d23d5f90b7..327722f56f4e 100644
  --- a/clang/test/CodeGen/ffp-contract-option.c
  +++ b/clang/test/CodeGen/ffp-contract-option.c
  @@ -63,7 +63,7 @@
   // RUN: --check-prefixes=CHECK,CHECK-FPC-ON
  
   float mymuladd(float x, float y, float z) {
  -  // CHECK: define {{.*}} float @mymuladd(float noundef %x, float noundef 
%y, float noundef %z)
  +  // CHECK: define{{.*}} float @mymuladd(float noundef %x, float noundef %y, 
float noundef %z)
 return x * y + z;
 // expected-warning{{overriding '-ffp-contract=fast' option with 
'-ffp-contract=on'}}
  
  diff --git a/clang/test/CodeGen/ffp-model.c b/clang/test/CodeGen/ffp-model.c
  index ee0eedb2718f..98d31106a119 100644
  --- a/clang/test/CodeGen/ffp-model.c
  +++ b/clang/test/CodeGen/ffp-model.c
  @@ -19,7 +19,7 @@
   // RUN: --check-prefixes CHECK,CHECK-FAST1
  
   float mymuladd(float x, float y, float z) {
  -  // CHECK: define {{.*}} float @mymuladd(float noundef %x, float noundef 
%y, float noundef %z)
  +  // CHECK: define{{.*}} float @mymuladd(float noundef %x, float noundef %y, 
float noundef %z)
 return x * y + z;
 // CHECK-FAST: fmul fast float

If this is the right fix, please land it. Else I'll revert again in 30 min or 
so; bots have been broken for hours by now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107994/new/

https://reviews.llvm.org/D107994

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits