[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7d7c448df9a: Fix `unsafe-fp-math` attribute emission. 
(authored by michele.scandale).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fp-function-attrs.cpp
  clang/test/CodeGen/func-attr.c

Index: clang/test/CodeGen/func-attr.c
===
--- clang/test/CodeGen/func-attr.c
+++ clang/test/CodeGen/func-attr.c
@@ -1,12 +1,28 @@
-// RUN: %clang -c -target x86_64 -ffast-math \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
 
-// RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=on -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=off -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
 
 float foo(float a, float b) {
   return a+b;
 }
 
-// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[FAST_ATTRS:#[0-9]+]]
-// CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" {{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} "unsafe-fp-math"="true"
+// CHECK:  define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[ATTRS:#[0-9]+]]
+// CHECK:  attributes [[ATTRS]] = {
+// CHECK-SAME:   "approx-func-fp-math"="true"
+// CHECK-SAME:   "no-signed-zeros-fp-math"="true"
+// CHECK-SAME:   "no-trapping-math"="true"
+// CHECK-UNSAFE-SAME:"unsafe-fp-math"="true"
+// CHECK-NOUNSAFE-NOT:   "unsafe-fp-math"="true"
+// CHECK-SAME: }
Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- clang/test/CodeGen/fp-function-attrs.cpp
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast-honor-pragmas -emit-llvm -o - %s | FileCheck %s
 
 float test_default(float a, float b, float c) {
   float tmp = a;
@@ -35,10 +36,23 @@
   return tmp;
 }
 
-// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
 // CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
 // CHECK: fadd fast float {{%.+}}, {{%.+}}
 
+float test_contract_on_pragma(float a, float b, float c) {
+  float tmp = a * b;
+  {
+#pragma clang fp contract(on)
+tmp += c;
+  }
+  return tmp;
+}
+
+// CHECK: define{{.*}} float @_Z23test_contract_on_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
+// CHECK: fmul fast float {{%.+}}, {{%.+}}
+// CHECK: fadd reassoc nnan ninf nsz arcp afn float {{%.+}}, {{%.+}}
+
 // CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
 // CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
-// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NO_UNSAFE_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -173,10 +173,11 @@
   mergeFnAttrValue("no-infs-fp-math", FPFeatures.getNoHonorInfs());
   mergeFnAttrValue("no-nans-fp-math", FPFeatures.getNoHonorNaNs());
   mergeFnAttrValue("no-signed-zeros-fp-math", FPFeatures.getNoSignedZero());
-  

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 474828.
michele.scandale added a comment.

Rebase on top of compiler driver changes for `-funsafe-math-optimizations`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fp-function-attrs.cpp
  clang/test/CodeGen/func-attr.c

Index: clang/test/CodeGen/func-attr.c
===
--- clang/test/CodeGen/func-attr.c
+++ clang/test/CodeGen/func-attr.c
@@ -1,12 +1,28 @@
-// RUN: %clang -c -target x86_64 -ffast-math \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
 
-// RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=on -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=off -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
 
 float foo(float a, float b) {
   return a+b;
 }
 
-// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[FAST_ATTRS:#[0-9]+]]
-// CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" {{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} "unsafe-fp-math"="true"
+// CHECK:  define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[ATTRS:#[0-9]+]]
+// CHECK:  attributes [[ATTRS]] = {
+// CHECK-SAME:   "approx-func-fp-math"="true"
+// CHECK-SAME:   "no-signed-zeros-fp-math"="true"
+// CHECK-SAME:   "no-trapping-math"="true"
+// CHECK-UNSAFE-SAME:"unsafe-fp-math"="true"
+// CHECK-NOUNSAFE-NOT:   "unsafe-fp-math"="true"
+// CHECK-SAME: }
Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- clang/test/CodeGen/fp-function-attrs.cpp
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast-honor-pragmas -emit-llvm -o - %s | FileCheck %s
 
 float test_default(float a, float b, float c) {
   float tmp = a;
@@ -35,10 +36,23 @@
   return tmp;
 }
 
-// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
 // CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
 // CHECK: fadd fast float {{%.+}}, {{%.+}}
 
+float test_contract_on_pragma(float a, float b, float c) {
+  float tmp = a * b;
+  {
+#pragma clang fp contract(on)
+tmp += c;
+  }
+  return tmp;
+}
+
+// CHECK: define{{.*}} float @_Z23test_contract_on_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
+// CHECK: fmul fast float {{%.+}}, {{%.+}}
+// CHECK: fadd reassoc nnan ninf nsz arcp afn float {{%.+}}, {{%.+}}
+
 // CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
 // CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
-// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NO_UNSAFE_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -173,10 +173,11 @@
   mergeFnAttrValue("no-infs-fp-math", FPFeatures.getNoHonorInfs());
   mergeFnAttrValue("no-nans-fp-math", FPFeatures.getNoHonorNaNs());
   mergeFnAttrValue("no-signed-zeros-fp-math", FPFeatures.getNoSignedZero());
-  

[PATCH] D137578: Fix 'unsafe-math-optimizations' flag dependencies

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale accepted this revision.
michele.scandale added a comment.
This revision is now accepted and ready to land.

This looks good for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137578

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D136786#3903646 , @zahiraam wrote:

> The changes in this patch look good to me. 
> @michele.scandale please make sure not to drop the driver changes that we 
> agreed upon in this patch. Thanks.

I started looking at the change needed to have `unsafe-math => 
fp-contract=fast`.

If I look how `-ffast-math` behave today, I see that in the driver code 
`-ffast-math` changes the state for `FPContract` 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3032),
 but then the condition for which `-ffast-math` is forwarded to the CC1 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3149)
 doesn't consider `FPContract`. This seems related to the fact that GCC does 
emit `__FAST_MATH__` even if the contraction mode is not fast.
From a quick look the `FastMath` language options is used mainly to guard some 
macro definition and a codegen path for complex floating point values, so this 
seems ok in practice.

It seems intended that the semantic of `-ffast-math` for the CC1 is different 
than the semantic of `-ffast-math` for the compiler driver. Based on this, I'd 
expect a similar solution for `-funsafe-math-optimizations`, i.e. 
`-funsafe-math-optimizations => -ffp-contract=fast` only at the compiler driver 
level.  Does this sound good?

If so, then the driver change for the `-funsafe-math-optimizations -> 
-ffp-contract=fast` could be done separately without affecting the code here.

> I talked to @andrew.w.kaylor offline: I was thinking that it might be 
> necessary to make the two driver changes we talked about, before merging this 
> patch. But if the tests pass then I think it's okay to implement the driver 
> changes in an upcoming patch.

In this patch the only suboptimal test change is the change to 
`clang/test/CodeGenOpenCL/relaxed-fpmath.cl`, where despite the presence of 
`-cl-unsafe-math-optimizations` the `"unsafe-fp-math"="true"` function 
attributes is not generated due to the contraction mode not being fast.
My understanding is that `-cl-fast-relaxed-math` should be equivalent to 
`-ffast-math`, and `-cl-unsafe-math-optimizations` should be equivalent to 
`-funsafe-math-optimizations` from the user perspective.

From what I see the `-cl-*` options are simply forwarded as-is to the CC1, and 
this seems to be the desired behavior for the compiler driver w.r.t. OpenCL 
specific options. From what I see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L3803
 implements `-cl-fast-relaxed-math => -ffast-math`, but this solution doesn't 
play well if on the same command line you also have `-ffp-contract=VAL` as the 
relative order of the options is not taken into account. I'd expect a similar 
change for the `-cl-unsafe-math-optimizations` case.

I can either put a `TODO` comment in the 
`clang/test/CodeGenOpenCL/relaxed-fpmath.cl` test, and make the changes with 
the driver changes for `-funsafe-math-optimizations`, otherwise the change for 
the `-cl-unsafe-math-optimizations` options needs to be done in this patch.

Any preference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale marked an inline comment as done.
michele.scandale added inline comments.



Comment at: clang/test/CodeGen/func-attr.c:5
 // RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: -fno-math-errno -ffp-contract=fast -emit-llvm -S -o - %s | FileCheck 
%s --check-prefixes=CHECK,CHECK-UNSAFE
+

michele.scandale wrote:
> See comment about `MathErrno` in `CGCall.cpp`
Replaced command line with `%clang_cc1` to avoid depending on compiler driver 
behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:181
+  FPFeatures.allowFPContractAcrossStatement());
 }
 

zahiraam wrote:
> Shouldn't then this also check for FPFeatures.getFPContractMode() ?
This should be covered by `FPFeatures.allowFPContractAcrossStatement()` which 
is basically `getFPContractMode() == LangOptions::FPM_Fast`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D136786#3896341 , @zahiraam wrote:

>> I'm going to ignore fast-math right now, because I think the current 
>> handling is mostly OK, and this review is about unsafe-fp-math. The 
>> unsafe-fp-math case is a little simpler in concept, but I think we've got 
>> more problems with it.
>>
>> Another fundamental principle I'd like to declare here is that 
>> LangOpts.UnsafeFPMath, TargetOptions.UnsafeFPMath, and the "unsafe-fp-math" 
>> function attribute should all mean exactly the same thing. The function 
>> attribute has a different scope, so it might not always have the same value 
>> as the other two, but it should at least mean the same thing.
>>
>> My understanding is this:
>>
>>   unsafe-fp-math = exception_behavior(ignore) +
>>fenv_access(off) +
>>no_signed_zeros(on) +
>>allow_reciprocal(on) +
>>allow_approximate_fns(on) +
>>allow_reassociation(on) +
>>fp_contract(fast)
>>
>> The first two of those settings are the default behavior. The others can be 
>> turned on by individual command line options. If all of those semantic modes 
>> are in the states above, then we are in "unsafe-fp-math" mode. That's my 
>> proposal.
>
> Agree. This is not currently the case.  -funsafe-math-optimizations should 
> set the FPContract=fast the same way it's done for -ffast-math. I think it 
> makes sense to fix this in this patch. @michele.scandale WDYT?

(*) I understand that Clang has a different definition of `-ffast-math` 
compared to the GCC one (i.e. in Clang `-ffast-math` implies 
`-ffp-contract=fast`), and that this can be considered an indication that there 
isn't a strong desire to be compatible with GCC. If that's the generally 
accepted direction, I won't oppose it.

Given that we all agreed that we should be using only low level options inside 
the compiler, I would keep changes to the compiler driver separate from this IR 
code generation change, and the two should be completely independent.

>> There are a couple of things we need to talk about here:
>>
>> 1. Currently, the Driver/ToolChains/Clang.cpp RenderFloatingPointOptions() 
>> function is looking for !MathErrno before deciding to pass 
>> "-funsafe-math-optimizations" to the front end, so LangOpts.UnsafeFPMath 
>> will not be set unless math-errno support is off. However, the 
>> RenderFloatingPointOptions() handling of -funsafe-math-optimizations does 
>> not change the MathErrno setting. Clearly, there's a bug here one way or the 
>> other. In GCC -funsafe-math-optimizations does not affect the math errno 
>> handling, so I think that's the correct behavior.
>
> Yep!   This can be fixed in an upcoming patch. @michele.scandale Do you want 
> to do it? If not, I can.

I can prepare a patch for that, and if there is a general consensus about (*) 
then we can have a single change that fixes the compiler driver code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D136786#3893485 , @andrew.w.kaylor 
wrote:

> My understanding is this:
>
>   unsafe-fp-math = exception_behavior(ignore) +
>fenv_access(off) +
>no_signed_zeros(on) +
>allow_reciprocal(on) +
>allow_approximate_fns(on) +
>allow_reassociation(on) +
>fp_contract(fast)
>
> The first two of those settings are the default behavior. The others can be 
> turned on by individual command line options. If all of those semantic modes 
> are in the states above, then we are in "unsafe-fp-math" mode. That's my 
> proposal.

I've updated the patch to use the low level options as you suggested. In the 
current version of the patch I'm not currently checking explicitly 
`exception_behavior(ignore) &&  fenv_access(off)` given that the other modes of 
these properties seems to work only if overall there is the "precise" profile. 
I don't know if that's acceptable, or if instead the CC1 shouldn't make any 
assumption about the compiler driver doing the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 471801.
michele.scandale added a comment.

Use low level properties to set `"unsafe-fp-math"="true"`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fp-function-attrs.cpp
  clang/test/CodeGen/func-attr.c
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -51,7 +51,7 @@
 // UNSAFE-NOT: "no-infs-fp-math"
 // UNSAFE-NOT: "no-nans-fp-math"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
-// UNSAFE: "unsafe-fp-math"="true"
+// UNSAFE-NOT: "unsafe-fp-math"="true"
 
 // MAD: "less-precise-fpmad"="true"
 // MAD-NOT: "no-infs-fp-math"
Index: clang/test/CodeGen/func-attr.c
===
--- clang/test/CodeGen/func-attr.c
+++ clang/test/CodeGen/func-attr.c
@@ -1,12 +1,28 @@
-// RUN: %clang -c -target x86_64 -ffast-math \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
 
-// RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=fast -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=on -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -funsafe-math-optimizations \
+// RUN: -ffp-contract=off -emit-llvm -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
 
 float foo(float a, float b) {
   return a+b;
 }
 
-// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[FAST_ATTRS:#[0-9]+]]
-// CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" {{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} "unsafe-fp-math"="true"
+// CHECK:  define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[ATTRS:#[0-9]+]]
+// CHECK:  attributes [[ATTRS]] = {
+// CHECK-SAME:   "approx-func-fp-math"="true"
+// CHECK-SAME:   "no-signed-zeros-fp-math"="true"
+// CHECK-SAME:   "no-trapping-math"="true"
+// CHECK-UNSAFE-SAME:"unsafe-fp-math"="true"
+// CHECK-NOUNSAFE-NOT:   "unsafe-fp-math"="true"
+// CHECK-SAME: }
Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- clang/test/CodeGen/fp-function-attrs.cpp
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast-honor-pragmas -emit-llvm -o - %s | FileCheck %s
 
 float test_default(float a, float b, float c) {
   float tmp = a;
@@ -35,10 +36,23 @@
   return tmp;
 }
 
-// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
 // CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
 // CHECK: fadd fast float {{%.+}}, {{%.+}}
 
+float test_contract_on_pragma(float a, float b, float c) {
+  float tmp = a * b;
+  {
+#pragma clang fp contract(on)
+tmp += c;
+  }
+  return tmp;
+}
+
+// CHECK: define{{.*}} float @_Z23test_contract_on_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]]
+// CHECK: fmul fast float {{%.+}}, {{%.+}}
+// CHECK: fadd reassoc nnan ninf nsz arcp afn float {{%.+}}, {{%.+}}
+
 // CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
 // CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
-// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NO_UNSAFE_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" 

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D136786#3893485 , @andrew.w.kaylor 
wrote:

> As a first principle, I'd like to clarify a "big picture" item that I was 
> trying to get at in my earlier comment. I'd like to be able to reason about 
> this from the starting point of individual semantic modes. There is a set of 
> modes defined in the clang user's manual 
> (https://clang.llvm.org/docs/UsersManual.html#controlling-floating-point-behavior).
>  I think this is reasonably complete:
>
>   exception_behavior
>   fenv_access
>   rounding_mode
>   fp-contract
>   denormal_fp_math
>   denormal_fp_math_32
>   support_math_errno
>   no_honor_nans
>   no_honor_infinities
>   no_signed_zeros
>   allow_reciprocal
>   allow_approximate_fns
>   allow_reassociation
>
> These are the modes from clang's perspective. They get communicated to the 
> back end in different ways. The backend handling of this isn't nearly as 
> consistent as I'd like, but that's a different problem. I think these basic 
> modes can be thought of as language-independent and should be used as the 
> basic building blocks for reasoning about floating point behavior across all 
> LLVM-based compilers.
>
> On top of these modes, we have two concepts "fast-math" and 
> "unsafe-math-optimizations" which are essentially composites of one or more 
> of the above semantic modes. I say "essentially" because up to this point 
> there has been some fuzzy handling of that. I'd like to say they are 
> absolutely composites of the above modes, and I think we can make it so, 
> starting here.
>
> If we agree that fast-math and unsafe-math-optimizations/unsafe-fp-math are 
> composites of some of the other modes, then we can apply a symmetry property 
> that I think will be helpful in the problem we're trying to solve here and 
> will lead to better reasoning about these settings in the future.
>
> I am proposing that passing "-ffast-math" should have *exactly* the same 
> effects as passing all of the individual command line options that are 
> implied by -ffast-math. Likewise, passing "-funsafe-math-optimizations" 
> should have *exactly* the same effects as passing all of the individual 
> options that are implied by -funsafe-math-optimizations. And, of course, any 
> combination of options that turn various states on and off can be strung 
> together and we can just evaluate the final state those options leave us in 
> to see if we are in the "fast-math" state or the "unsafe-fp-math" state.

Yes, I agree that this should be the long term goal.

> I'm going to ignore fast-math right now, because I think the current handling 
> is mostly OK, and this review is about unsafe-fp-math. The unsafe-fp-math 
> case is a little simpler in concept, but I think we've got more problems with 
> it.
>
> Another fundamental principle I'd like to declare here is that 
> LangOpts.UnsafeFPMath, TargetOptions.UnsafeFPMath, and the "unsafe-fp-math" 
> function attribute should all mean exactly the same thing. The function 
> attribute has a different scope, so it might not always have the same value 
> as the other two, but it should at least mean the same thing.
>
> My understanding is this:
>
>   unsafe-fp-math = exception_behavior(ignore) +
>fenv_access(off) +
>no_signed_zeros(on) +
>allow_reciprocal(on) +
>allow_approximate_fns(on) +
>allow_reassociation(on) +
>fp_contract(fast)
>
> The first two of those settings are the default behavior. The others can be 
> turned on by individual command line options. If all of those semantic modes 
> are in the states above, then we are in "unsafe-fp-math" mode. That's my 
> proposal.
>
> There are a couple of things we need to talk about here:
>
> 1. Currently, the Driver/ToolChains/Clang.cpp RenderFloatingPointOptions() 
> function is looking for !MathErrno before deciding to pass 
> "-funsafe-math-optimizations" to the front end, so LangOpts.UnsafeFPMath will 
> not be set unless math-errno support is off. However, the 
> RenderFloatingPointOptions() handling of -funsafe-math-optimizations does not 
> change the MathErrno setting. Clearly, there's a bug here one way or the 
> other. In GCC -funsafe-math-optimizations does not affect the math errno 
> handling, so I think that's the correct behavior.

I agree there is a bug. My resolution for this would be to remove the 
`!MathErrno` from:

  if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
  ApproxFunc && !TrappingMath)
CmdArgs.push_back("-funsafe-math-optimizations");

so that we match GCC behavior and definition.

> 2. Currently, the Driver/ToolChains/Clang.cpp RenderFloatingPointOptions() 
> function does not consider the FPContract setting when deciding whether to 
> pass "-funsafe-math-optimizations" to the front end or set the fp-contract 
> value when handling the 

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865
   FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
-  LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
-  LangOpts.NoSignedZero && LangOpts.ApproxFunc)) &&
-LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)
+if (LangOpts.UnsafeFPMath &&
+(LangOpts.getDefaultFPContractMode() ==

andrew.w.kaylor wrote:
> michele.scandale wrote:
> > michele.scandale wrote:
> > > zahiraam wrote:
> > > > zahiraam wrote:
> > > > > michele.scandale wrote:
> > > > > > michele.scandale wrote:
> > > > > > > I've found quite confusing the `(FastMath || (!FastMath && ... 
> > > > > > > ))`.
> > > > > > > 
> > > > > > > Using directly `UnsafeFPMath` seems more compact, however it also 
> > > > > > > causes to taking into account the value for `MathErrno` -- which 
> > > > > > > it might be not relevant.
> > > > > > > 
> > > > > > > If `MathErrno` shouldn't affect this, then I would rewrite this 
> > > > > > > as:
> > > > > > > ```
> > > > > > > if (LangOpts.AllowFPReassoc && LangOpts.AllowRecip &&
> > > > > > >  LangOpts.NoSignedZero && LangOpts.ApproxFunc &&
> > > > > > >  (LangOpts.getDefaultFPContractMode() ==
> > > > > > >  LangOptions::FPModeKind::FPM_Fast ||
> > > > > > >  LangOpts.getDefaultFPContractMode() ==
> > > > > > >  LangOptions::FPModeKind::FPM_FastHonorPragmas))
> > > > > > >   FuncAttrs.addAttribute("unsafe-fp-math", "true");
> > > > > > > ```
> > > > > > > so that only the relevant options are considered and there is no 
> > > > > > > need to think about what is implied by `FastMath`.
> > > > > > I do wonder if in 
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089-L3091
> > > > > >  is actually correct to have `!MathErrno`. In the GCC documentation 
> > > > > > of `-funsafe-math-optimizations` I don't see any connection to the 
> > > > > > `-fmath-errno` or `-fno-math-errno` options.
> > > > > Interesting! Using on the command line 'funsafe-math-optimization' 
> > > > > implies 'math-errno'. But 'funsafe-math-optimizations' is triggered 
> > > > > when this is true:
> > > > >  !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&   
> > > > >ApproxFunc && !TrappingMath
> > > > > 
> > > > > See here: 
> > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089
> > > > >  
> > > > When the ‘funsafe-math-optimizations’ is used on the command line 
> > > > LangOpts.UnsafeFP is ‘0’. 
> > > > The function attribute ‘funnsafe-fp-math’ is not set. Is that what’s 
> > > > wanted? I would have thought that we want the attribute enabled for 
> > > > unsafe mode.
> > > > 
> > > > If we call 
> > > > SpecialOperations = AllowFPReassoc && NoSignedZero && AllowRecip && 
> > > > ApproxFunc && !RoundingMath.
> > > > 
> > > > ‘ffast-math’ = !MathErrno && FiniteMathOnly && SpecialOperations
> > > > ‘funsafe-math-optimizations’ = MathErrno && !FiniteMathOnly && 
> > > > SpecialOperations 
> > > > (it's true that there is no mention of MathErrno in the GCC doc, but 
> > > > the default is fmath-errno so presumably using this option on the 
> > > > command line implies MathErrno).
> > > > 
> > > > 
> > > > The function attribute UnsafeFPMath is enabled for 
> > > > (‘ffast-math’ || ‘funsafe-math-optimization’). 
> > > > That will lead to this condition: 
> > > > (SpecialOperations  && MathErrNo && "-ffast-math" && 
> > > > -ffp-contract=fast") || (SpecialOperations && "-fmath-errno" && 
> > > > "-ffp-contract=on") .
> > > > 
> > > > 
> > > The compiler driver has a default value for `MathErrno` which depends on 
> > > the toolchain properties. When `-funsafe-math-optimizations` is processed 
> > > `MathErrno` is not affected, but then the compiler driver specify 
> > > `-funsafe-math-optimizations` to the CC1 command line only if `MathErrno` 
> > > is false.
> > > The way the compiler driver mutate the floating point options state when 
> > > processing `-funsafe-math-optimizations` seems to match the GCC 
> > > documentation, but then the condition for the forwarding expect something 
> > > more.
> > > It is not clear to me if the intention is to match GCC documented 
> > > behavior, or if instead the Clang definition of 
> > > `-funsafe-math-optimizations` implies `-fno-math-errno`.
> > >When the ‘funsafe-math-optimizations’ is used on the command line 
> > >LangOpts.UnsafeFP is ‘0’. 
> > The function attribute ‘funnsafe-fp-math’ is not set. Is that what’s 
> > wanted? I would have thought that we want the attribute enabled for unsafe 
> > mode.
> > 
> > I believe this is caused by `MathErrno` state in the compiler driver being 
> > not affected when processing `-funsafe-math-optimizations`, but considered 
> > for the 

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865
   FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
-  LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
-  LangOpts.NoSignedZero && LangOpts.ApproxFunc)) &&
-LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)
+if (LangOpts.UnsafeFPMath &&
+(LangOpts.getDefaultFPContractMode() ==

zahiraam wrote:
> zahiraam wrote:
> > michele.scandale wrote:
> > > michele.scandale wrote:
> > > > I've found quite confusing the `(FastMath || (!FastMath && ... ))`.
> > > > 
> > > > Using directly `UnsafeFPMath` seems more compact, however it also 
> > > > causes to taking into account the value for `MathErrno` -- which it 
> > > > might be not relevant.
> > > > 
> > > > If `MathErrno` shouldn't affect this, then I would rewrite this as:
> > > > ```
> > > > if (LangOpts.AllowFPReassoc && LangOpts.AllowRecip &&
> > > >  LangOpts.NoSignedZero && LangOpts.ApproxFunc &&
> > > >  (LangOpts.getDefaultFPContractMode() ==
> > > >  LangOptions::FPModeKind::FPM_Fast ||
> > > >  LangOpts.getDefaultFPContractMode() ==
> > > >  LangOptions::FPModeKind::FPM_FastHonorPragmas))
> > > >   FuncAttrs.addAttribute("unsafe-fp-math", "true");
> > > > ```
> > > > so that only the relevant options are considered and there is no need 
> > > > to think about what is implied by `FastMath`.
> > > I do wonder if in 
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089-L3091
> > >  is actually correct to have `!MathErrno`. In the GCC documentation of 
> > > `-funsafe-math-optimizations` I don't see any connection to the 
> > > `-fmath-errno` or `-fno-math-errno` options.
> > Interesting! Using on the command line 'funsafe-math-optimization' implies 
> > 'math-errno'. But 'funsafe-math-optimizations' is triggered when this is 
> > true:
> >  !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&  
> > ApproxFunc && !TrappingMath
> > 
> > See here: 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089
> >  
> When the ‘funsafe-math-optimizations’ is used on the command line 
> LangOpts.UnsafeFP is ‘0’. 
> The function attribute ‘funnsafe-fp-math’ is not set. Is that what’s wanted? 
> I would have thought that we want the attribute enabled for unsafe mode.
> 
> If we call 
> SpecialOperations = AllowFPReassoc && NoSignedZero && AllowRecip && 
> ApproxFunc && !RoundingMath.
> 
> ‘ffast-math’ = !MathErrno && FiniteMathOnly && SpecialOperations
> ‘funsafe-math-optimizations’ = MathErrno && !FiniteMathOnly && 
> SpecialOperations 
> (it's true that there is no mention of MathErrno in the GCC doc, but the 
> default is fmath-errno so presumably using this option on the command line 
> implies MathErrno).
> 
> 
> The function attribute UnsafeFPMath is enabled for 
> (‘ffast-math’ || ‘funsafe-math-optimization’). 
> That will lead to this condition: 
> (SpecialOperations  && MathErrNo && "-ffast-math" && -ffp-contract=fast") || 
> (SpecialOperations && "-fmath-errno" && "-ffp-contract=on") .
> 
> 
The compiler driver has a default value for `MathErrno` which depends on the 
toolchain properties. When `-funsafe-math-optimizations` is processed 
`MathErrno` is not affected, but then the compiler driver specify 
`-funsafe-math-optimizations` to the CC1 command line only if `MathErrno` is 
false.
The way the compiler driver mutate the floating point options state when 
processing `-funsafe-math-optimizations` seems to match the GCC documentation, 
but then the condition for the forwarding expect something more.
It is not clear to me if the intention is to match GCC documented behavior, or 
if instead the Clang definition of `-funsafe-math-optimizations` implies 
`-fno-math-errno`.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865
   FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
-  LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
-  LangOpts.NoSignedZero && LangOpts.ApproxFunc)) &&
-LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)
+if (LangOpts.UnsafeFPMath &&
+(LangOpts.getDefaultFPContractMode() ==

michele.scandale wrote:
> zahiraam wrote:
> > zahiraam wrote:
> > > michele.scandale wrote:
> > > > michele.scandale wrote:
> > > > > I've found quite confusing the `(FastMath || (!FastMath && ... ))`.
> > > > > 
> > > > > Using directly `UnsafeFPMath` seems more compact, however it also 
> > > > > causes to taking into account the value for `MathErrno` -- which it 
> > > > > might be not relevant.
> > > > > 
> > > > > If `MathErrno` shouldn't affect this, then I would 

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865
   FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
-  LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
-  LangOpts.NoSignedZero && LangOpts.ApproxFunc)) &&
-LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)
+if (LangOpts.UnsafeFPMath &&
+(LangOpts.getDefaultFPContractMode() ==

michele.scandale wrote:
> I've found quite confusing the `(FastMath || (!FastMath && ... ))`.
> 
> Using directly `UnsafeFPMath` seems more compact, however it also causes to 
> taking into account the value for `MathErrno` -- which it might be not 
> relevant.
> 
> If `MathErrno` shouldn't affect this, then I would rewrite this as:
> ```
> if (LangOpts.AllowFPReassoc && LangOpts.AllowRecip &&
>  LangOpts.NoSignedZero && LangOpts.ApproxFunc &&
>  (LangOpts.getDefaultFPContractMode() ==
>  LangOptions::FPModeKind::FPM_Fast ||
>  LangOpts.getDefaultFPContractMode() ==
>  LangOptions::FPModeKind::FPM_FastHonorPragmas))
>   FuncAttrs.addAttribute("unsafe-fp-math", "true");
> ```
> so that only the relevant options are considered and there is no need to 
> think about what is implied by `FastMath`.
I do wonder if in 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L3089-L3091
 is actually correct to have `!MathErrno`. In the GCC documentation of 
`-funsafe-math-optimizations` I don't see any connection to the `-fmath-errno` 
or `-fno-math-errno` options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865
   FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
-  LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
-  LangOpts.NoSignedZero && LangOpts.ApproxFunc)) &&
-LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)
+if (LangOpts.UnsafeFPMath &&
+(LangOpts.getDefaultFPContractMode() ==

I've found quite confusing the `(FastMath || (!FastMath && ... ))`.

Using directly `UnsafeFPMath` seems more compact, however it also causes to 
taking into account the value for `MathErrno` -- which it might be not relevant.

If `MathErrno` shouldn't affect this, then I would rewrite this as:
```
if (LangOpts.AllowFPReassoc && LangOpts.AllowRecip &&
 LangOpts.NoSignedZero && LangOpts.ApproxFunc &&
 (LangOpts.getDefaultFPContractMode() ==
 LangOptions::FPModeKind::FPM_Fast ||
 LangOpts.getDefaultFPContractMode() ==
 LangOptions::FPModeKind::FPM_FastHonorPragmas))
  FuncAttrs.addAttribute("unsafe-fp-math", "true");
```
so that only the relevant options are considered and there is no need to think 
about what is implied by `FastMath`.



Comment at: clang/test/CodeGen/func-attr.c:5
 // RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: -fno-math-errno -ffp-contract=fast -emit-llvm -S -o - %s | FileCheck 
%s --check-prefixes=CHECK,CHECK-UNSAFE
+

See comment about `MathErrno` in `CGCall.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136786

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

[PATCH] D136786: Fix `unsafe-fp-math` attribute emission.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: zahiraam, aaron.ballman, andrew.w.kaylor, 
rjmccall, efriedma, fhahn.
Herald added a subscriber: ormris.
Herald added a project: All.
michele.scandale requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The conditions for which Clang emits the `unsafe-fp-math` function
attribute has been modified as part of
`84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7`.
In the backend code generators `"unsafe-fp-math"="true"` enable floating
point contraction for the whole function.
The intent of the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7`
was to prevent backend code generators performing contractions when that
is not expected.
However the change is inaccurate and incomplete because it allows
`unsafe-fp-math` to be set also when only in-statement contraction is
allowed.

Consider the following example

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

and compile it with the command line

  clang -fno-math-errno -funsafe-math-optimizations -ffp-contract=on \
-O2 -mavx512f -S -o -

The resulting assembly has a `vfmadd213ss` instruction which corresponds
to a fused multiply-add. From the user perspective there shouldn't be
any contraction because the multiplication and the addition are not in
the same statement.

The optimized IR is:

  define float @test(float noundef %a, float noundef %b, float noundef %c) #0 {
%mul = fmul reassoc nsz arcp afn float %b, %a
%add = fadd reassoc nsz arcp afn float %mul, %c
ret float %add
  }
  
  attributes #0 = {
[...]
"no-signed-zeros-fp-math"="true"
"no-trapping-math"="true"
[...]
"unsafe-fp-math"="true"
  }

The `"unsafe-fp-math"="true"` function attribute allows the backend code
generator to perform `(fadd (fmul a, b), c) -> (fmadd a, b, c)`.

In the current IR representation there is no way to determine the
statement boundaries from the original source code.
Because of this for in-statement only contraction the generated IR
doesn't have instructions with the `contract` fast-math flag and
`llvm.fmuladd` is being used to represent contractions opportunities
that occur within a single statement.
Therefore `"unsafe-fp-math"="true"` can only be emitted when contraction
across statements is allowed.

Moreover the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7` doesn't
take into account that the floating point math function attributes can
be refined during IR code generation of a function to handle the cases
where the floating point math options are modified within a compound
statement via pragmas (see `CGFPOptionsRAII`).
For consistency `unsafe-fp-math` needs to be disabled if the contraction
mode for any scope/operation is not `fast`.
Similarly for consistency reason the initialization of `UnsafeFPMath` of
in `TargetOptions` for the backend code generation should take into
account the contraction mode as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136786

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fp-function-attrs.cpp
  clang/test/CodeGen/func-attr.c
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -51,7 +51,7 @@
 // UNSAFE-NOT: "no-infs-fp-math"
 // UNSAFE-NOT: "no-nans-fp-math"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
-// UNSAFE: "unsafe-fp-math"="true"
+// UNSAFE-NOT: "unsafe-fp-math"="true"
 
 // MAD: "less-precise-fpmad"="true"
 // MAD-NOT: "no-infs-fp-math"
Index: clang/test/CodeGen/func-attr.c
===
--- clang/test/CodeGen/func-attr.c
+++ clang/test/CodeGen/func-attr.c
@@ -1,12 +1,21 @@
 // RUN: %clang -c -target x86_64 -ffast-math \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
 
 // RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
-// RUN: -emit-llvm -S -o - %s | FileCheck %s
+// RUN: -fno-math-errno -ffp-contract=fast -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE
+
+// RUN: %clang -c -target x86_64 -funsafe-math-optimizations \
+// RUN: -fno-math-errno -ffp-contract=on -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE
 
 float foo(float a, float b) {
   return a+b;
 }
 
-// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[FAST_ATTRS:#[0-9]+]]
-// CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" {{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} "unsafe-fp-math"="true"
+// CHECK:  define{{.*}} float @foo(float noundef %{{.*}}, float noundef 

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869
+if ((LangOpts.FastMath ||
+ !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip 
&&
+ !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero &&
+ LangOpts.ApproxFunc) &&
+LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)

zahiraam wrote:
> michele.scandale wrote:
> > If I look at the clang driver code, `-funsafe-math-optimizations` is 
> > specified on the CC1 command line if
> > ```
> > !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && 
> > ApproxFunc && !TrappingMath
> > ```
> > is true.
> > This means that it shouldn't matter the value of the floating point 
> > contraction, or whether infs/nans are honored or not.
> > 
> > Was there another issue that this specific part of the change addresses?
> This is to make sure that if we are using -ffast-math or 
> -funsafe-math-optimization along with -ffp-contract=off, fma instructions are 
> not generated.  In this case the function shouldn't be marked with the 
> unsafe-fp-math attribute.
> This is to make sure that if we are using -ffast-math or 
> -funsafe-math-optimization along with -ffp-contract=off, fma instructions are 
> not generated.  In this case the function shouldn't be marked with the 
> unsafe-fp-math attribute.

If I understand correctly this is because the function attribute 
`unsafe-fp-math` from backends perspective allow any kind of contraction?
If so, shouldn't then this be valid only when the fp-contraction mode is 
`fast`, given that `on` means that only in-statement contraction is allowed, 
and clang handles that by generating `llvm.fmuladd` to lower expressions like 
`a * b + c` in given statement?

What about the `!LangOpts.FiniteMathOnly` part? I'd would think this shouldn't 
be checked at all as there are separate function attributes for infs and nans.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869
+if ((LangOpts.FastMath ||
+ !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip 
&&
+ !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero &&
+ LangOpts.ApproxFunc) &&
+LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)

michele.scandale wrote:
> zahiraam wrote:
> > michele.scandale wrote:
> > > If I look at the clang driver code, `-funsafe-math-optimizations` is 
> > > specified on the CC1 command line if
> > > ```
> > > !MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && 
> > > ApproxFunc && !TrappingMath
> > > ```
> > > is true.
> > > This means that it shouldn't matter the value of the floating point 
> > > contraction, or whether infs/nans are honored or not.
> > > 
> > > Was there another issue that this specific part of the change addresses?
> > This is to make sure that if we are using -ffast-math or 
> > -funsafe-math-optimization along with -ffp-contract=off, fma instructions 
> > are not generated.  In this case the function shouldn't be marked with the 
> > unsafe-fp-math attribute.
> > This is to make sure that if we are using -ffast-math or 
> > -funsafe-math-optimization along with -ffp-contract=off, fma instructions 
> > are not generated.  In this case the function shouldn't be marked with the 
> > unsafe-fp-math attribute.
> 
> If I understand correctly this is because the function attribute 
> `unsafe-fp-math` from backends perspective allow any kind of contraction?
> If so, shouldn't then this be valid only when the fp-contraction mode is 
> `fast`, given that `on` means that only in-statement contraction is allowed, 
> and clang handles that by generating `llvm.fmuladd` to lower expressions like 
> `a * b + c` in given statement?
> 
> What about the `!LangOpts.FiniteMathOnly` part? I'd would think this 
> shouldn't be checked at all as there are separate function attributes for 
> infs and nans.
> This is to make sure that if we are using -ffast-math or 
> -funsafe-math-optimization along with -ffp-contract=off, fma instructions are 
> not generated.  In this case the function shouldn't be marked with the 
> unsafe-fp-math attribute.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135097

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1865-1869
+if ((LangOpts.FastMath ||
+ !LangOpts.FastMath && LangOpts.AllowFPReassoc && LangOpts.AllowRecip 
&&
+ !LangOpts.FiniteMathOnly && LangOpts.NoSignedZero &&
+ LangOpts.ApproxFunc) &&
+LangOpts.getDefaultFPContractMode() != 
LangOptions::FPModeKind::FPM_Off)

If I look at the clang driver code, `-funsafe-math-optimizations` is specified 
on the CC1 command line if
```
!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros && ApproxFunc 
&& !TrappingMath
```
is true.
This means that it shouldn't matter the value of the floating point 
contraction, or whether infs/nans are honored or not.

Was there another issue that this specific part of the change addresses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135097

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

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc49cde6467f9: [Sema] Fix `ExtVectorElementExpr` tree 
transform for the `isArrow` case. (authored by michele.scandale).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/instantiate-clang.cpp


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2814,20 +2814,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-  /*FirstQualifierInScope*/ 
nullptr,
-  NameInfo,
-  /* TemplateArgs */ nullptr,
-  /*S*/ nullptr);
+return getSema().BuildMemberReferenceExpr(
+Base, Base->getType(), OpLoc, IsArrow, SS, SourceLocation(),
+/*FirstQualifierInScope*/ nullptr, NameInfo,
+/* TemplateArgs */ nullptr,
+/*S*/ nullptr);
   }
 
   /// Build a new initializer list expression.
@@ -11424,9 +11422,9 @@
   // FIXME: Bad source location
   SourceLocation FakeOperatorLoc =
   SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
-  return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
-  E->getAccessorLoc(),
-  E->getAccessor());
+  return getDerived().RebuildExtVectorElementExpr(
+  Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+  E->getAccessor());
 }
 
 template


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2814,20 +2814,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ 

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D131698#3848469 , 
@michele.scandale wrote:

> @rjmccall would you be able to land this on my behalf? Thanks in advance.

I got commit access. I will take care of landing the change. Thanks anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

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

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

@rjmccall would you be able to land this on my behalf? Thanks in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

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

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 466686.
michele.scandale added a comment.

Final rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/instantiate-clang.cpp


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2814,20 +2814,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-  /*FirstQualifierInScope*/ 
nullptr,
-  NameInfo,
-  /* TemplateArgs */ nullptr,
-  /*S*/ nullptr);
+return getSema().BuildMemberReferenceExpr(
+Base, Base->getType(), OpLoc, IsArrow, SS, SourceLocation(),
+/*FirstQualifierInScope*/ nullptr, NameInfo,
+/* TemplateArgs */ nullptr,
+/*S*/ nullptr);
   }
 
   /// Build a new initializer list expression.
@@ -11424,9 +11422,9 @@
   // FIXME: Bad source location
   SourceLocation FakeOperatorLoc =
   SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
-  return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
-  E->getAccessorLoc(),
-  E->getAccessor());
+  return getDerived().RebuildExtVectorElementExpr(
+  Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+  E->getAccessor());
 }
 
 template


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2814,20 +2814,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-  

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

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

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 463727.
michele.scandale added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/instantiate-clang.cpp


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2778,20 +2778,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-  /*FirstQualifierInScope*/ 
nullptr,
-  NameInfo,
-  /* TemplateArgs */ nullptr,
-  /*S*/ nullptr);
+return getSema().BuildMemberReferenceExpr(
+Base, Base->getType(), OpLoc, IsArrow, SS, SourceLocation(),
+/*FirstQualifierInScope*/ nullptr, NameInfo,
+/* TemplateArgs */ nullptr,
+/*S*/ nullptr);
   }
 
   /// Build a new initializer list expression.
@@ -11388,9 +11386,9 @@
   // FIXME: Bad source location
   SourceLocation FakeOperatorLoc =
   SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
-  return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
-  E->getAccessorLoc(),
-  E->getAccessor());
+  return getDerived().RebuildExtVectorElementExpr(
+  Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+  E->getAccessor());
 }
 
 template


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2778,20 +2778,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-   

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

I can see failures related to this change in a downstream version of clang 
where the default `ClangABICompat` value is not `Latest` (after the followup 
https://reviews.llvm.org/rGda6187f566b7881cb8350621aea9bd582de569b9).
My understanding is that with `-fclang-abi-compat=14` we should get the 
behavior prior to this change. If my understanding is correct, then I'd think 
that such failure shouldn't occur.

These are the tests that I see failing with new diagnostic messages `class 
template partial specialization is not more specialized than the primary 
template` being raised:

  Clang :: CXX/temp/temp.decls/temp.class.spec/p8-0x.cpp
  Clang :: CXX/temp/temp.decls/temp.variadic/example-bind.cpp
  Clang :: CXX/temp/temp.decls/temp.variadic/example-tuple.cpp
  Clang :: CXX/temp/temp.decls/temp.variadic/injected-class-name.cpp
  Clang :: CXX/temp/temp.decls/temp.variadic/metafunctions.cpp
  Clang :: CXX/temp/temp.fct.spec/temp.arg.explicit/p3-0x.cpp
  Clang :: CXX/temp/temp.fct.spec/temp.arg.explicit/p9-0x.cpp
  Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
  Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p21.cpp
  Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp
  Clang :: CXX/temp/temp.param/p11-0x.cpp
  Clang :: SemaCXX/coreturn-exp-namespace.cpp
  Clang :: SemaCXX/coreturn.cpp
  Clang :: SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
  Clang :: SemaCXX/coroutine-unhandled_exception-warning.cpp
  Clang :: SemaCXX/coroutines-exp-namespace.cpp
  Clang :: SemaCXX/coroutines.cpp
  Clang :: SemaCXX/crash-lambda-12645424.cpp
  Clang :: SemaCXX/discrim-union.cpp
  Clang :: SemaTemplate/attributes.cpp
  Clang :: SemaTemplate/class-template-spec.cpp
  Clang :: SemaTemplate/deduction.cpp
  Clang :: SemaTemplate/pack-deduction.cpp
  Clang :: SemaTemplate/temp_arg_nontype_cxx1z.cpp
  Clang :: SemaTemplate/temp_arg_template_cxx1z.cpp

I've reproduced this locally with the following patch to inject 
`-fclang-abi-compat=14` on the command line executed for those tests:

  diff --git a/clang/test/CXX/temp/lit.local.cfg 
b/clang/test/CXX/temp/lit.local.cfg
  new file mode 100644
  index ..23d23bb6c46e
  --- /dev/null
  +++ b/clang/test/CXX/temp/lit.local.cfg
  @@ -0,0 +1,3 @@
  +config.substitutions.insert(0,
  +  ('%clang_cc1',
  +   '%clang_cc1 -fclang-abi-compat=14.0'))
  diff --git a/clang/test/SemaCXX/lit.local.cfg 
b/clang/test/SemaCXX/lit.local.cfg
  new file mode 100644
  index ..23d23bb6c46e
  --- /dev/null
  +++ b/clang/test/SemaCXX/lit.local.cfg
  @@ -0,0 +1,3 @@
  +config.substitutions.insert(0,
  +  ('%clang_cc1',
  +   '%clang_cc1 -fclang-abi-compat=14.0'))
  diff --git a/clang/test/SemaTemplate/lit.local.cfg 
b/clang/test/SemaTemplate/lit.local.cfg
  new file mode 100644
  index ..23d23bb6c46e
  --- /dev/null
  +++ b/clang/test/SemaTemplate/lit.local.cfg
  @@ -0,0 +1,3 @@
  +config.substitutions.insert(0,
  +  ('%clang_cc1',
  +   '%clang_cc1 -fclang-abi-compat=14.0'))

(Note that by adding the `lit.local.cfg` file abovementioned there will also be 
the failure of `SemaCXX/class-layout.cpp`, but that can be ignored as for some 
command lines the expected behavior is equivalent to pass 
`-fclang-abi-compat=latest`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745

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

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 451911.
michele.scandale added a comment.

Clang-format fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131698

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/instantiate-clang.cpp


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2779,20 +2779,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-  /*FirstQualifierInScope*/ 
nullptr,
-  NameInfo,
-  /* TemplateArgs */ nullptr,
-  /*S*/ nullptr);
+return getSema().BuildMemberReferenceExpr(
+Base, Base->getType(), OpLoc, IsArrow, SS, SourceLocation(),
+/*FirstQualifierInScope*/ nullptr, NameInfo,
+/* TemplateArgs */ nullptr,
+/*S*/ nullptr);
   }
 
   /// Build a new initializer list expression.
@@ -11385,9 +11383,9 @@
   // FIXME: Bad source location
   SourceLocation FakeOperatorLoc =
   SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
-  return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
-  E->getAccessorLoc(),
-  E->getAccessor());
+  return getDerived().RebuildExtVectorElementExpr(
+  Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+  E->getAccessor());
 }
 
 template


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2779,20 +2779,18 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
-  SS, SourceLocation(),
-

[PATCH] D131698: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: rsmith, doug.gregor, rjmccall.
Herald added a project: All.
michele.scandale requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Make sure we propagate the value for `IsArrow` to
`RebuildExtVectorElementExpr` in order to be able to handle correctly
vector component accesses where the base value is a pointer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131698

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/instantiate-clang.cpp


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2779,15 +2779,15 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
 return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
+  OpLoc, IsArrow,
   SS, SourceLocation(),
   /*FirstQualifierInScope*/ 
nullptr,
   NameInfo,
@@ -11385,9 +11385,9 @@
   // FIXME: Bad source location
   SourceLocation FakeOperatorLoc =
   SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
-  return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
-  E->getAccessorLoc(),
-  E->getAccessor());
+  return getDerived().RebuildExtVectorElementExpr(
+  Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+  E->getAccessor());
 }
 
 template


Index: clang/test/SemaTemplate/instantiate-clang.cpp
===
--- clang/test/SemaTemplate/instantiate-clang.cpp
+++ clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@
 template struct ExtVectorAccess0;
 template struct ExtVectorAccess0;
 
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+  void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+  }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
 
 template
 struct ShuffleVector0 {
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -2779,15 +2779,15 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildExtVectorElementExpr(Expr *Base,
-   SourceLocation OpLoc,
-   SourceLocation AccessorLoc,
-   IdentifierInfo ) {
+  ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo ) {
 
 CXXScopeSpec SS;
 DeclarationNameInfo NameInfo(, AccessorLoc);
 return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
-  OpLoc, /*IsArrow*/ false,
+  OpLoc, IsArrow,
   SS, SourceLocation(),
   

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D83454#2145086 , @michele.scandale 
wrote:

> In D83454#2144386 , @JDevlieghere 
> wrote:
>
> > In D83454#2142719 , 
> > @michele.scandale wrote:
> >
> > > I tested locally the standalone build of Clang with D83426 
> > > .
> > >  I just want to make sure that we all agree this is right way moving 
> > > forward.
> >
> >
> > Yep, with the target exported this is the right thing to do.
>
>
> Great.
>
> I don't have commit rights. Could someone land this on my behalf?


Anybody?

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D83454#2144386 , @JDevlieghere 
wrote:

> In D83454#2142719 , 
> @michele.scandale wrote:
>
> > I tested locally the standalone build of Clang with D83426 
> > .
> >  I just want to make sure that we all agree this is right way moving 
> > forward.
>
>
> Yep, with the target exported this is the right thing to do.


Great.

I don't have commit rights. Could someone land this on my behalf?

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

I tested locally the standalone build of Clang with D83426 
.
I just want to make sure that we all agree this is right way moving forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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

[PATCH] D83426: Unbreak Clang standalone build.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

Thanks.

Would you be able to land this on my behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83426



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

[PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: chandlerc, beanz, zturner.
Herald added subscribers: lldb-commits, cfe-commits, MaskRay, aheejin, 
arichardson, sbc100, mgorny, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLDB.

The `intrinsics_gen` target exists in the CMake exports since r309389
(see LLVMConfig.cmake.in), hence projects can depend on `intrinsics_gen`
even it they are built separately from LLVM.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83454

Files:
  clang/CMakeLists.txt
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/Frontend/CMakeLists.txt
  clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
  clang/tools/clang-import-test/CMakeLists.txt
  clang/tools/clang-offload-bundler/CMakeLists.txt
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  lld/COFF/CMakeLists.txt
  lld/Common/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/MinGW/CMakeLists.txt
  lld/lib/Core/CMakeLists.txt
  lld/wasm/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt

Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/CMakeLists.txt
@@ -1,8 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
-
 add_lldb_library(lldbPluginRenderScriptRuntime PLUGIN
   RenderScriptRuntime.cpp
   RenderScriptExpressionOpts.cpp
@@ -10,7 +5,7 @@
   RenderScriptScriptGroup.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbBreakpoint
Index: lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
===
--- lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
+++ lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lldb_library(lldbPluginExpressionParserClang
   ASTResultSynthesizer.cpp
   ASTStructExtractor.cpp
@@ -29,7 +25,7 @@
   NameSearchContext.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbCore
Index: lldb/source/Expression/CMakeLists.txt
===
--- lldb/source/Expression/CMakeLists.txt
+++ lldb/source/Expression/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLDB_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lldb_library(lldbExpression
   DiagnosticManager.cpp
   DWARFExpression.cpp
@@ -18,7 +14,7 @@
   UtilityFunction.cpp
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
 
   LINK_LIBS
 lldbCore
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -64,7 +64,7 @@
 # some of these generated headers. This approach is copied from Clang's main
 # CMakeLists.txt, so it should kept in sync the code in Clang which was added
 # in llvm-svn 308844.
-if(LLVM_ENABLE_MODULES AND NOT LLDB_BUILT_STANDALONE)
+if(LLVM_ENABLE_MODULES)
   list(APPEND LLVM_COMMON_DEPENDS intrinsics_gen)
 endif()
 
Index: lld/wasm/CMakeLists.txt
===
--- lld/wasm/CMakeLists.txt
+++ lld/wasm/CMakeLists.txt
@@ -2,10 +2,6 @@
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(WasmOptionsTableGen)
 
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldWasm
   Driver.cpp
   InputChunks.cpp
@@ -37,5 +33,5 @@
 
   DEPENDS
   WasmOptionsTableGen
-  ${tablegen_deps}
+  intrinsics_gen
   )
Index: lld/lib/Core/CMakeLists.txt
===
--- lld/lib/Core/CMakeLists.txt
+++ lld/lib/Core/CMakeLists.txt
@@ -1,7 +1,3 @@
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldCore
   DefinedAtom.cpp
   Error.cpp
@@ -24,5 +20,5 @@
   ${LLVM_PTHREAD_LIB}
 
   DEPENDS
-  ${tablegen_deps}
+  intrinsics_gen
   )
Index: lld/MinGW/CMakeLists.txt
===
--- lld/MinGW/CMakeLists.txt
+++ lld/MinGW/CMakeLists.txt
@@ -2,10 +2,6 @@
 tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(MinGWOptionsTableGen)
 
-if(NOT LLD_BUILT_STANDALONE)
-  set(tablegen_deps intrinsics_gen)
-endif()
-
 add_lld_library(lldMinGW
   Driver.cpp
 
@@ -19,5 +15,5 @@
 
   DEPENDS
   MinGWOptionsTableGen
-  ${tablegen_deps}
+  intrinsics_gen
 )
Index: lld/ELF/CMakeLists.txt

[PATCH] D83426: Unbreak Clang standalone build.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added a reviewer: chandlerc.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Add missing C++ language standard setup for Clang standalone build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83426

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -9,6 +9,10 @@
 if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   project(Clang)
 
+  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD_REQUIRED YES)
+  set(CMAKE_CXX_EXTENSIONS NO)
+
   # Rely on llvm-config.
   set(CONFIG_OUTPUT)
   if(LLVM_CONFIG)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -9,6 +9,10 @@
 if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   project(Clang)
 
+  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD_REQUIRED YES)
+  set(CMAKE_CXX_EXTENSIONS NO)
+
   # Rely on llvm-config.
   set(CONFIG_OUTPUT)
   if(LLVM_CONFIG)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

Uhm.. it looks like it is not needed anymore. In the `LLVMConfig.cmake` that 
will be installed a `intrinsics_gen` and `omp_gen` custom targets are created 
for exactly the purpose of allowing out-of-tree or standalone builds to freely 
depend on them.
The Clang code where `intrinsics_gen` is conditionally added as a dependency is 
from 2014, while the change in `LLVMConfig.cmake.in` is from 2017.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D83414: Unbreak Clang standalone build.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: clementval, simon_tatham, chandlerc.
Herald added subscribers: cfe-commits, sstefan1, martong, arphaman, mgorny.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Having the `omp_gen` dependency added unconditionally to Clang libraries
breaks the Clang standalone build. This dependency should be added only
if `CLANG_BUILT_STANDALONE` is false, similarly to the `intrinsics_gen`
case.
Moreover the C++ standard must be set properly as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83414

Files:
  clang/CMakeLists.txt
  clang/lib/ARCMigrate/CMakeLists.txt
  clang/lib/AST/CMakeLists.txt
  clang/lib/ASTMatchers/CMakeLists.txt
  clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Frontend/Rewrite/CMakeLists.txt
  clang/lib/Index/CMakeLists.txt
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Serialization/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
  clang/lib/Tooling/ASTDiff/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Transformer/CMakeLists.txt

Index: clang/lib/Tooling/Transformer/CMakeLists.txt
===
--- clang/lib/Tooling/Transformer/CMakeLists.txt
+++ clang/lib/Tooling/Transformer/CMakeLists.txt
@@ -19,7 +19,4 @@
   clangLex
   clangToolingCore
   clangToolingRefactoring
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Tooling/Syntax/CMakeLists.txt
===
--- clang/lib/Tooling/Syntax/CMakeLists.txt
+++ clang/lib/Tooling/Syntax/CMakeLists.txt
@@ -15,7 +15,4 @@
   clangFrontend
   clangLex
   clangToolingCore
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Tooling/Refactoring/CMakeLists.txt
===
--- clang/lib/Tooling/Refactoring/CMakeLists.txt
+++ clang/lib/Tooling/Refactoring/CMakeLists.txt
@@ -22,7 +22,4 @@
   clangLex
   clangRewrite
   clangToolingCore
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Tooling/CMakeLists.txt
===
--- clang/lib/Tooling/CMakeLists.txt
+++ clang/lib/Tooling/CMakeLists.txt
@@ -31,7 +31,6 @@
 
   DEPENDS
   ClangDriverOptions
-  omp_gen
 
   LINK_LIBS
   clangAST
Index: clang/lib/Tooling/ASTDiff/CMakeLists.txt
===
--- clang/lib/Tooling/ASTDiff/CMakeLists.txt
+++ clang/lib/Tooling/ASTDiff/CMakeLists.txt
@@ -8,7 +8,4 @@
   clangBasic
   clangAST
   clangLex
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
@@ -23,7 +23,4 @@
   clangLex
   clangStaticAnalyzerCheckers
   clangStaticAnalyzerCore
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/StaticAnalyzer/Core/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -61,8 +61,5 @@
   clangLex
   clangRewrite
   clangToolingCore
-
-  DEPENDS
-  omp_gen
   )
 
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -135,7 +135,4 @@
   clangBasic
   clangLex
   clangStaticAnalyzerCore
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Serialization/CMakeLists.txt
===
--- clang/lib/Serialization/CMakeLists.txt
+++ clang/lib/Serialization/CMakeLists.txt
@@ -30,7 +30,4 @@
   clangBasic
   clangLex
   clangSema
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Sema/CMakeLists.txt
===
--- clang/lib/Sema/CMakeLists.txt
+++ clang/lib/Sema/CMakeLists.txt
@@ -72,7 +72,6 @@
 
   DEPENDS
   ClangOpenCLBuiltinsImpl
-  omp_gen
 
   LINK_LIBS
   clangAST
Index: clang/lib/Parse/CMakeLists.txt
===
--- clang/lib/Parse/CMakeLists.txt
+++ clang/lib/Parse/CMakeLists.txt
@@ -27,7 +27,4 @@
   clangBasic
   clangLex
   clangSema
-
-  DEPENDS
-  omp_gen
   )
Index: clang/lib/Index/CMakeLists.txt
===
--- clang/lib/Index/CMakeLists.txt
+++ clang/lib/Index/CMakeLists.txt
@@ -27,7 +27,4 @@
   clangRewrite
   clangSerialization
   clangToolingCore
-
-  DEPENDS
-  omp_gen
   )

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D82659#2136999 , @clementval wrote:

> Looks good but just one question ... When clang is built as standalone it 
> does not build the OpenMP part inside Clang? I haven't seen any code to avoid 
> compiling the OpenMP parsing and semantic checking inside clang.


I don't think there is a way to avoid compiling the OpenMP support in Clang. 
The standalone build is just building the content of the `clang` directory as a 
separate CMake project reusing the an already built LLVM -- therefore the 
`libLLVMFrontendOpenMP` as well as the `OMP.h.inc` would have been generated 
already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D82659: Fix missing build dependency on omp_gen.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

Why `omp_gen` is now a dependency of `clang-tablegen-targets` rather than being 
in the `LLVM_COMMON_DEPENDS` list like `clang-tablegen-targets`?

Moreover I've noticed that with the recent changes where  `omp_gen` has been 
added as a dependency in several libraries, this was done unconditionally 
breaking the Clang standalone build.
For the same issue `intrinsics_gen` is added only if `CLANG_BUILT_STANDALONE ` 
is false.

At this point I think that something like:

  # All targets below may depend on all tablegen'd files.
  get_property(CLANG_TABLEGEN_TARGETS GLOBAL PROPERTY CLANG_TABLEGEN_TARGETS)
  add_custom_target(clang-tablegen-targets DEPENDS ${CLANG_TABLEGEN_TARGETS})
  set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "Misc")
  list(APPEND LLVM_COMMON_DEPENDS clang-tablegen-targets)
  if(NOT CLANG_BUILT_STANDALONE)
list(APPEND LLVM_COMMON_DEPENDS omg_gen)
  endif()

would fix all the issues, and it would allow removing the explicit dependencies 
added to each clang library.

Is there any issue with my reasoning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82659



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

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

As far as I can tell the failures reported shouldn't be caused by this change.

John, would you be able to land this on my behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462



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

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 269417.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -704,6 +704,18 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII();
+
+  private:
+CodeGenFunction 
+FPOptions OldFPFeatures;
+Optional FMFGuard;
+  };
+  FPOptions CurFPFeatures;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -65,13 +65,14 @@
 : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
   Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
   CGBuilderInserterTy(this)),
-  SanOpts(CGM.getLangOpts().Sanitize), DebugInfo(CGM.getModuleDebugInfo()),
-  PGO(cgm), ShouldEmitLifetimeMarkers(shouldEmitLifetimeMarkers(
-CGM.getCodeGenOpts(), CGM.getLangOpts())) {
+  SanOpts(CGM.getLangOpts().Sanitize), CurFPFeatures(CGM.getLangOpts()),
+  DebugInfo(CGM.getModuleDebugInfo()), PGO(cgm),
+  ShouldEmitLifetimeMarkers(
+  shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
   if (!suppressNewContext)
 CGM.getCXXABI().getMangleContext().startNewFunction();
 
-  SetFastMathFlags(FPOptions(CGM.getLangOpts()));
+  SetFastMathFlags(CurFPFeatures);
   SetFPModel();
 }
 
@@ -132,6 +133,51 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGF(CGF), OldFPFeatures(CGF.CurFPFeatures) {
+  CGF.CurFPFeatures = FPFeatures;
+
+  if (OldFPFeatures == FPFeatures)
+return;
+
+  FMFGuard.emplace(CGF.Builder);
+
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == 

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 269387.
michele.scandale added a comment.

Store `CGF` in RAII object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -704,6 +704,18 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII();
+
+  private:
+CodeGenFunction 
+FPOptions OldFPFeatures;
+Optional FMFGuard;
+  };
+  FPOptions CurFPFeatures;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -65,13 +65,14 @@
 : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
   Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
   CGBuilderInserterTy(this)),
-  SanOpts(CGM.getLangOpts().Sanitize), DebugInfo(CGM.getModuleDebugInfo()),
-  PGO(cgm), ShouldEmitLifetimeMarkers(shouldEmitLifetimeMarkers(
-CGM.getCodeGenOpts(), CGM.getLangOpts())) {
+  SanOpts(CGM.getLangOpts().Sanitize), CurFPFeatures(CGM.getLangOpts()),
+  DebugInfo(CGM.getModuleDebugInfo()), PGO(cgm),
+  ShouldEmitLifetimeMarkers(
+  shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
   if (!suppressNewContext)
 CGM.getCXXABI().getMangleContext().startNewFunction();
 
-  SetFastMathFlags(FPOptions(CGM.getLangOpts()));
+  SetFastMathFlags(CurFPFeatures);
   SetFPModel();
 }
 
@@ -132,6 +133,51 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGF(CGF), OldFPFeatures(CGF.CurFPFeatures) {
+  CGF.CurFPFeatures = FPFeatures;
+
+  if (OldFPFeatures == FPFeatures)
+return;
+
+  FMFGuard.emplace(CGF.Builder);
+
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 269381.
michele.scandale added a comment.

Modify IR function attribute directly.
RAII object modifies IR builder state and function attributes only if needed.
Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -704,6 +704,18 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII();
+
+  private:
+FPOptions 
+FPOptions OldFPFeatures;
+Optional FMFGuard;
+  };
+  FPOptions CurFPFeatures;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -65,13 +65,14 @@
 : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
   Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
   CGBuilderInserterTy(this)),
-  SanOpts(CGM.getLangOpts().Sanitize), DebugInfo(CGM.getModuleDebugInfo()),
-  PGO(cgm), ShouldEmitLifetimeMarkers(shouldEmitLifetimeMarkers(
-CGM.getCodeGenOpts(), CGM.getLangOpts())) {
+  SanOpts(CGM.getLangOpts().Sanitize), CurFPFeatures(CGM.getLangOpts()),
+  DebugInfo(CGM.getModuleDebugInfo()), PGO(cgm),
+  ShouldEmitLifetimeMarkers(
+  shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
   if (!suppressNewContext)
 CGM.getCXXABI().getMangleContext().startNewFunction();
 
-  SetFastMathFlags(FPOptions(CGM.getLangOpts()));
+  SetFastMathFlags(CurFPFeatures);
   SetFPModel();
 }
 
@@ -132,6 +133,51 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CurFPFeatures(CGF.CurFPFeatures), OldFPFeatures(CurFPFeatures) {
+  CGF.CurFPFeatures = FPFeatures;
+
+  if (OldFPFeatures == FPFeatures)
+return;
+
+  FMFGuard.emplace(CGF.Builder);
+
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D80462#2077569 , @rjmccall wrote:

> Can we do this conservatively only if something is changed within the 
> function definition, and otherwise respect the global settings?


I'm not sure I understand what you mean. The initial state is already the 
global settings coming from the language options (see constructor of 
`CodeGenFunction`).
Then whenever we process a unary/binary floating point operator we consider the 
local FPOptions and adjust the values of the function attributes accordingly.

Maybe instead of having an explicit state (the booleans) in `CodeGenFunction` 
tracking the values for these function attributes, I could simply update the 
function attributes directly. I guess the extra cost is negligible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462



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

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 268667.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -704,6 +704,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
+ "FPConstrained should be enabled on entire function");
+
+  CGF.FnAttrNoInfsFPMath &= FPFeatures.noHonorInfs();
+  CGF.FnAttrNoNaNsFPMath &= FPFeatures.noHonorNaNs();
+  CGF.FnAttrNoSignedZerosFPMath &= FPFeatures.noSignedZeros();
+  CGF.FnAttrUnsafeFPMath &=
+  

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267907.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -678,6 +678,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
+ "FPConstrained should be enabled on entire function");
+
+  CGF.FnAttrNoInfsFPMath &= FPFeatures.noHonorInfs();
+  CGF.FnAttrNoNaNsFPMath &= FPFeatures.noHonorNaNs();
+  CGF.FnAttrNoSignedZerosFPMath &= FPFeatures.noSignedZeros();
+  CGF.FnAttrUnsafeFPMath &=
+  

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267783.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -678,6 +678,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
+ "FPConstrained should be enabled on entire function");
+
+  CGF.FnAttrNoInfsFPMath &= FPFeatures.noHonorInfs();
+  CGF.FnAttrNoNaNsFPMath &= FPFeatures.noHonorNaNs();
+  CGF.FnAttrNoSignedZerosFPMath &= FPFeatures.noSignedZeros();
+  CGF.FnAttrUnsafeFPMath &=
+  

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

Thanks John.

Would you be able to land this on my behalf?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267401.
michele.scandale added a comment.

Revert last change about `-ffast-math` imply "fast" contraction mode by default 
in CC1.
Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCUDA/library-builtin.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -11,7 +11,7 @@
   // NORMAL: fdiv float
   // FAST: fdiv fast float
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float
   // NOSIGNED: fdiv nsz float
   return a / b;
@@ -38,7 +38,7 @@
 
 // UNSAFE: "less-precise-fpmad"="true"
 // UNSAFE: "no-infs-fp-math"="false"
-// UNSAFE: "no-nans-fp-math"="true"
+// UNSAFE: "no-nans-fp-math"="false"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
 // UNSAFE: "unsafe-fp-math"="true"
 
Index: clang/test/CodeGenCUDA/library-builtin.cu
===
--- clang/test/CodeGenCUDA/library-builtin.cu
+++ clang/test/CodeGenCUDA/library-builtin.cu
@@ -10,7 +10,7 @@
 
 // logf() should be calling itself recursively as we don't have any standard
 // library on device side.
-// DEVICE: call float @logf(float
+// DEVICE: call contract float @logf(float
 extern "C" __attribute__((device)) float logf(float __x) { return logf(__x); }
 
 // NOTE: this case is to illustrate the expected differences in behavior between
@@ -18,5 +18,5 @@
 // library.
 //
 // Host is assumed to have standard library, so logf() calls LLVM intrinsic.
-// HOST: call float @llvm.log.f32(float
+// HOST: call contract float @llvm.log.f32(float
 extern "C" float logf(float __x) { return logf(__x); }
Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -10,7 +10,7 @@
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -8,17 +8,17 @@
 void test_sqrt(float a0, double a1, long double a2) {
   // CHECK-YES: call float @sqrtf
   // CHECK-NO: call float @llvm.sqrt.f32(float
-  // CHECK-FAST: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
   // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
-  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call reassoc nsz arcp afn x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
Index: clang/test/CodeGen/fp-options-to-fast-math-flags.c
===
--- /dev/null
+++ clang/test/CodeGen/fp-options-to-fast-math-flags.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck -check-prefix CHECK-PRECISE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-nans -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-NANS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-infs -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-INFS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffinite-math-only -emit-llvm -o - %s | FileCheck -check-prefix CHECK-FINITE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fno-signed-zeros -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-SIGNED-ZEROS %s
+// RUN: %clang_cc1 -triple 

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale requested review of this revision.
michele.scandale added a comment.

I've just realized it might be incorrect to have the CC1 option `-ffast-math` 
changing the default contraction mode. The clang driver generates `-ffast-math` 
based on conditions that do not involve the contraction mode state at all:

  // -ffast-math enables the __FAST_MATH__ preprocessor macro, but check for the
  // individual features enabled by -ffast-math instead of the option itself as
  // that's consistent with gcc's behaviour.
  if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath &&
  ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
CmdArgs.push_back("-ffast-math");
if (FPModel.equals("fast")) {
  if (FPContract.equals("fast"))
// All set, do nothing.
;
  else if (FPContract.empty())
// Enable -ffp-contract=fast
CmdArgs.push_back(Args.MakeArgString("-ffp-contract=fast"));
  else
D.Diag(clang::diag::warn_drv_overriding_flag_option)
  << "-ffp-model=fast"
  << Args.MakeArgString("-ffp-contract=" + FPContract);
}
  }

For example the running the following `clang -### -funsafe-math-optimizations 
-ffinite-math-only -x c -` lead to a CC1 command line without any 
`-ffp-contract=` option relying on the fact that the default value for the 
contraction mode in the compiler is OFF.

I will revert the modification on this aspect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267124.
michele.scandale added a comment.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Fixup more tests with now redundant `-ffp-contract=fast` option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/fast-math.c
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCUDA/library-builtin.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl
  clang/test/Headers/nvptx_device_math_sin.c
  clang/test/Headers/nvptx_device_math_sin.cpp
  clang/test/Parser/fp-floatcontrol-syntax.cpp

Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -22,7 +22,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fdenormal-fp-math=preserve-sign,preserve-sign -ftrapping-math -fsyntax-only %s -DDEFAULT -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ffp-contract=fast -DPRECISE -verify
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ftrapping-math -ffp-contract=off -frounding-math -ffp-exception-behavior=strict -DSTRICT -verify
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -menable-no-infs -menable-no-nans -menable-unsafe-fp-math -fno-signed-zeros -mreassociate -freciprocal-math -ffp-contract=fast -ffast-math -ffinite-math-only -fsyntax-only %s -DFAST -verify
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only %s -ffast-math -DFAST -verify
 double a = 0.0;
 double b = 1.0;
 
Index: clang/test/Headers/nvptx_device_math_sin.cpp
===
--- clang/test/Headers/nvptx_device_math_sin.cpp
+++ clang/test/Headers/nvptx_device_math_sin.cpp
@@ -1,8 +1,8 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix=SLOW
-// RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -ffast-math -ffp-contract=fast
-// RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -ffast-math -ffp-contract=fast | FileCheck %s --check-prefix=FAST
+// RUN: %clang_cc1 -x c++ -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc -ffast-math
+// RUN: %clang_cc1 -x c++ -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - -ffast-math | FileCheck %s --check-prefix=FAST
 // expected-no-diagnostics
 
 #include 
Index: clang/test/Headers/nvptx_device_math_sin.c
===
--- clang/test/Headers/nvptx_device_math_sin.c
+++ clang/test/Headers/nvptx_device_math_sin.c
@@ -1,8 +1,8 @@
 // REQUIRES: nvptx-registered-target
 // RUN: %clang_cc1 -x c -internal-isystem %S/Inputs/include -fopenmp -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
 // RUN: %clang_cc1 -x c -include __clang_openmp_device_functions.h -internal-isystem %S/../../lib/Headers/openmp_wrappers -internal-isystem %S/Inputs/include -fopenmp -triple nvptx64-nvidia-cuda 

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267125.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -678,6 +678,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
+ "FPConstrained should be enabled on entire function");
+
+  CGF.FnAttrNoInfsFPMath &= FPFeatures.noHonorInfs();
+  CGF.FnAttrNoNaNsFPMath &= FPFeatures.noHonorNaNs();
+  CGF.FnAttrNoSignedZerosFPMath &= FPFeatures.noSignedZeros();
+  CGF.FnAttrUnsafeFPMath &=
+  FPFeatures.allowAssociativeMath() && 

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267122.
michele.scandale added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -678,6 +678,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == llvm::fp::ebIgnore &&
+   NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) &&
+ "FPConstrained should be enabled on entire function");
+
+  CGF.FnAttrNoInfsFPMath &= FPFeatures.noHonorInfs();
+  CGF.FnAttrNoNaNsFPMath &= FPFeatures.noHonorNaNs();
+  CGF.FnAttrNoSignedZerosFPMath &= FPFeatures.noSignedZeros();
+  CGF.FnAttrUnsafeFPMath &=
+  

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 267121.
michele.scandale added a comment.

Rebase + `-ffast-math` change the default contraction mode to fast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fast-math.c
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCUDA/library-builtin.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -11,7 +11,7 @@
   // NORMAL: fdiv float
   // FAST: fdiv fast float
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float
   // NOSIGNED: fdiv nsz float
   return a / b;
@@ -38,7 +38,7 @@
 
 // UNSAFE: "less-precise-fpmad"="true"
 // UNSAFE: "no-infs-fp-math"="false"
-// UNSAFE: "no-nans-fp-math"="true"
+// UNSAFE: "no-nans-fp-math"="false"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
 // UNSAFE: "unsafe-fp-math"="true"
 
Index: clang/test/CodeGenCUDA/library-builtin.cu
===
--- clang/test/CodeGenCUDA/library-builtin.cu
+++ clang/test/CodeGenCUDA/library-builtin.cu
@@ -10,7 +10,7 @@
 
 // logf() should be calling itself recursively as we don't have any standard
 // library on device side.
-// DEVICE: call float @logf(float
+// DEVICE: call contract float @logf(float
 extern "C" __attribute__((device)) float logf(float __x) { return logf(__x); }
 
 // NOTE: this case is to illustrate the expected differences in behavior between
@@ -18,5 +18,5 @@
 // library.
 //
 // Host is assumed to have standard library, so logf() calls LLVM intrinsic.
-// HOST: call float @llvm.log.f32(float
+// HOST: call contract float @llvm.log.f32(float
 extern "C" float logf(float __x) { return logf(__x); }
Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -10,7 +10,7 @@
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -8,17 +8,17 @@
 void test_sqrt(float a0, double a1, long double a2) {
   // CHECK-YES: call float @sqrtf
   // CHECK-NO: call float @llvm.sqrt.f32(float
-  // CHECK-FAST: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
   // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
-  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call reassoc nsz arcp afn x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
Index: clang/test/CodeGen/fpconstrained.cpp
===
--- clang/test/CodeGen/fpconstrained.cpp
+++ clang/test/CodeGen/fpconstrained.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -x c++ -ftrapping-math -fexceptions -fcxx-exceptions -frounding-math -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=FPMODELSTRICT
 // RUN: %clang_cc1 -x c++ -ffp-contract=fast -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck %s -check-prefix=PRECISE
 // RUN: %clang_cc1 -x c++ -ffast-math -fexceptions -fcxx-exceptions -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
-// RUN: %clang_cc1 -x c++ -ffast-math -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck %s -check-prefix=FASTNOCONTRACT
+// RUN: 

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D80315#2061164 , @rjmccall wrote:

> In D80315#2059160 , 
> @michele.scandale wrote:
>
> > In D80315#2058914 , @rjmccall 
> > wrote:
> >
> > > In D80315#2058735 , 
> > > @michele.scandale wrote:
> > >
> > > > In D80315#2058549 , @rjmccall 
> > > > wrote:
> > > >
> > > > > The code cleanups all seems reasonable.  The actual changes in 
> > > > > code-generation changes are because we were failing to set these 
> > > > > reliably?
> > > >
> > > >
> > > > Most of them yes.
> > > >
> > > > In the CUDA test we there is now `contract` because we honor the 
> > > > default contraction mode that for CUDA is set to fast.
> > >
> > >
> > > Right, and we weren't honoring that mode before?
> >
> >
> > Not in the setup of base fast-math flags inside `CodeGenFunction`. However 
> > when emitting code for expression with `FPOptions` stored in the AST nodes 
> > then `contract` was set correctly.
>
>
> Okay, that seems justifiable.
>
> >>> In `clang/test/CodeGen/complex-math.c` I've added `-ffp-contract=fast` to 
> >>> the command line option because `-ffast-math` at the CC1 level does not 
> >>> change the default contraction mode.
> >> 
> >> Oh, I see, makes sense.  So there was inconsistent treatment of the 
> >> options, where one thing observed it but others didn't, and that's been 
> >> fixed now.
> > 
> > Do you think we should handle `-ffast-math` as `-cl-fast-relaxed-math`, 
> > i.e. it implies the default contraction mode being "fast"?
>
> I'm actually surprised it doesn't.  I can't imagine why someone enabling fast 
> math would want contraction to be disabled.


Just to be clear the clang driver does the right thing.
If you run `clang -ffast-math` the CC1 invocation has both `-ffast-math` and 
`-ffp-contract=fast` (and other options as well)

Here specifically I'm just considering the behavior of `clang -cc1 -ffast-math`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D80315#2058914 , @rjmccall wrote:

> In D80315#2058735 , 
> @michele.scandale wrote:
>
> > In D80315#2058549 , @rjmccall 
> > wrote:
> >
> > > The code cleanups all seems reasonable.  The actual changes in 
> > > code-generation changes are because we were failing to set these reliably?
> >
> >
> > Most of them yes.
> >
> > In the CUDA test we there is now `contract` because we honor the default 
> > contraction mode that for CUDA is set to fast.
>
>
> Right, and we weren't honoring that mode before?


Not in the setup of base fast-math flags inside `CodeGenFunction`. However when 
emitting code for expression with `FPOptions` stored in the AST nodes then 
`contract` was set correctly.

> 
> 
>> In `clang/test/CodeGen/complex-math.c` I've added `-ffp-contract=fast` to 
>> the command line option because `-ffast-math` at the CC1 level does not 
>> change the default contraction mode.
> 
> Oh, I see, makes sense.  So there was inconsistent treatment of the options, 
> where one thing observed it but others didn't, and that's been fixed now.

Do you think we should handle `-ffast-math` as `-cl-fast-relaxed-math`, i.e. it 
implies the default contraction mode being "fast"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D80315#2058549 , @rjmccall wrote:

> The code cleanups all seems reasonable.  The actual changes in 
> code-generation changes are because we were failing to set these reliably?


Most of them yes.

In the CUDA test we there is now `contract` because we honor the default 
contraction mode that for CUDA is set to fast.

In `clang/test/CodeGen/complex-math.c` I've added `-ffp-contract=fast` to the 
command line option because `-ffast-math` at the CC1 level does not change the 
default contraction mode. We might want to treat `-ffast-math` similarly to 
`-cl-fast-relaxed-math`, i.e. it implies by default the "fast" contraction 
mode. Before this change there behavior was accidentally the same as 
"-ffast-math changes the default contraction mode" because there was:

  if (CGM.getLangOpts().FastMath)
FMF.setFast()

and so the bit for `AllowContract` was enabled in the fast-math flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80462: Fix floating point math function attributes definition.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: rjmccall, mibintc.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With the support for changing the state of floating point optimizations
within a code block, we need to make sure that the function attributes
related to floating point math optimization are set in a conservative
way w.r.t. the body of the function to prevent illegal transformations
in the backends where such attributes are consumed.

Depends on D80315 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80462

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/fp-function-attrs.cpp

Index: clang/test/CodeGen/fp-function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-function-attrs.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s
+
+float test_default(float a, float b, float c) {
+  float tmp = a;
+  tmp += b;
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z12test_defaultfff(float %a, float %b, float %c) [[FAST_ATTRS:#[0-9]+]]
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_precise_on_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma float_control(precise, on)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z22test_precise_on_pragmafff(float %a, float %b, float %c) [[PRECISE_ATTRS:#[0-9]+]]
+// CHECK: fadd float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+float test_reassociate_off_pragma(float a, float b, float c) {
+  float tmp = a;
+  {
+#pragma clang fp reassociate(off)
+tmp += b;
+  }
+  tmp += c;
+  return tmp;
+}
+
+// CHECK: define float @_Z27test_reassociate_off_pragmafff(float %a, float %b, float %c) [[NOREASSOC_ATTRS:#[0-9]+]]
+// CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}}
+// CHECK: fadd fast float {{%.+}}, {{%.+}}
+
+// CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} }
+// CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} }
+// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} }
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -675,6 +675,20 @@
 return DominatingValue::save(*this, value);
   }
 
+  class CGFPOptionsRAII : public CGBuilderTy::FastMathFlagGuard {
+  public:
+CGFPOptionsRAII(CodeGenFunction , FPOptions FPFeatures);
+~CGFPOptionsRAII() = default;
+  };
+
+  /// State for the floating point function attributes. At the end of the
+  /// codegen of a function they will have the strictest configuration required
+  /// by the statements in the function itself.
+  bool FnAttrNoInfsFPMath;
+  bool FnAttrNoNaNsFPMath;
+  bool FnAttrNoSignedZerosFPMath;
+  bool FnAttrUnsafeFPMath;
+
 public:
   /// ObjCEHValueStack - Stack of Objective-C exception values, used for
   /// rethrows.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -73,6 +73,11 @@
 
   SetFastMathFlags(FPOptions(CGM.getLangOpts()));
   SetFPModel();
+
+  FnAttrNoInfsFPMath = CGM.getLangOpts().NoHonorInfs;
+  FnAttrNoNaNsFPMath = CGM.getLangOpts().NoHonorNaNs;
+  FnAttrNoSignedZerosFPMath = CGM.getLangOpts().NoSignedZero;
+  FnAttrUnsafeFPMath = CGM.getLangOpts().UnsafeFPMath;
 }
 
 CodeGenFunction::~CodeGenFunction() {
@@ -132,6 +137,32 @@
   Builder.setFastMathFlags(FMF);
 }
 
+CodeGenFunction::CGFPOptionsRAII::CGFPOptionsRAII(CodeGenFunction ,
+  FPOptions FPFeatures)
+: CGBuilderTy::FastMathFlagGuard(CGF.Builder) {
+  auto NewRoundingBehavior = FPFeatures.getRoundingMode();
+  CGF.Builder.setDefaultConstrainedRounding(NewRoundingBehavior);
+  auto NewExceptionBehavior =
+  ToConstrainedExceptMD(FPFeatures.getExceptionMode());
+  CGF.Builder.setDefaultConstrainedExcept(NewExceptionBehavior);
+
+  CGF.SetFastMathFlags(FPFeatures);
+
+  assert((CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() ||
+  isa(CGF.CurFuncDecl) ||
+  isa(CGF.CurFuncDecl) ||
+  (NewExceptionBehavior == 

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 265405.
michele.scandale added a comment.

Fix formatting issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCUDA/library-builtin.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -11,7 +11,7 @@
   // NORMAL: fdiv float
   // FAST: fdiv fast float
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float
   // NOSIGNED: fdiv nsz float
   return a / b;
@@ -38,7 +38,7 @@
 
 // UNSAFE: "less-precise-fpmad"="true"
 // UNSAFE: "no-infs-fp-math"="false"
-// UNSAFE: "no-nans-fp-math"="true"
+// UNSAFE: "no-nans-fp-math"="false"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
 // UNSAFE: "unsafe-fp-math"="true"
 
Index: clang/test/CodeGenCUDA/library-builtin.cu
===
--- clang/test/CodeGenCUDA/library-builtin.cu
+++ clang/test/CodeGenCUDA/library-builtin.cu
@@ -10,7 +10,7 @@
 
 // logf() should be calling itself recursively as we don't have any standard
 // library on device side.
-// DEVICE: call float @logf(float
+// DEVICE: call contract float @logf(float
 extern "C" __attribute__((device)) float logf(float __x) { return logf(__x); }
 
 // NOTE: this case is to illustrate the expected differences in behavior between
@@ -18,5 +18,5 @@
 // library.
 //
 // Host is assumed to have standard library, so logf() calls LLVM intrinsic.
-// HOST: call float @llvm.log.f32(float
+// HOST: call contract float @llvm.log.f32(float
 extern "C" float logf(float __x) { return logf(__x); }
Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -10,7 +10,7 @@
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -8,17 +8,17 @@
 void test_sqrt(float a0, double a1, long double a2) {
   // CHECK-YES: call float @sqrtf
   // CHECK-NO: call float @llvm.sqrt.f32(float
-  // CHECK-FAST: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
   // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
-  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call reassoc nsz arcp afn x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
Index: clang/test/CodeGen/fp-options-to-fast-math-flags.c
===
--- /dev/null
+++ clang/test/CodeGen/fp-options-to-fast-math-flags.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck -check-prefix CHECK-PRECISE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-nans -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-NANS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-infs -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-INFS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffinite-math-only -emit-llvm -o - %s | FileCheck -check-prefix CHECK-FINITE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fno-signed-zeros -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-SIGNED-ZEROS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -mreassociate -emit-llvm -o - %s | FileCheck -check-prefix 

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 265333.
michele.scandale added a comment.

Fix 'clang/test/CodeGenCUDA/library-builtin.cu'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCUDA/library-builtin.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -11,7 +11,7 @@
   // NORMAL: fdiv float
   // FAST: fdiv fast float
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float
   // NOSIGNED: fdiv nsz float
   return a / b;
@@ -38,7 +38,7 @@
 
 // UNSAFE: "less-precise-fpmad"="true"
 // UNSAFE: "no-infs-fp-math"="false"
-// UNSAFE: "no-nans-fp-math"="true"
+// UNSAFE: "no-nans-fp-math"="false"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
 // UNSAFE: "unsafe-fp-math"="true"
 
Index: clang/test/CodeGenCUDA/library-builtin.cu
===
--- clang/test/CodeGenCUDA/library-builtin.cu
+++ clang/test/CodeGenCUDA/library-builtin.cu
@@ -10,7 +10,7 @@
 
 // logf() should be calling itself recursively as we don't have any standard
 // library on device side.
-// DEVICE: call float @logf(float
+// DEVICE: call contract float @logf(float
 extern "C" __attribute__((device)) float logf(float __x) { return logf(__x); }
 
 // NOTE: this case is to illustrate the expected differences in behavior between
@@ -18,5 +18,5 @@
 // library.
 //
 // Host is assumed to have standard library, so logf() calls LLVM intrinsic.
-// HOST: call float @llvm.log.f32(float
+// HOST: call contract float @llvm.log.f32(float
 extern "C" float logf(float __x) { return logf(__x); }
Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -10,7 +10,7 @@
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -8,17 +8,17 @@
 void test_sqrt(float a0, double a1, long double a2) {
   // CHECK-YES: call float @sqrtf
   // CHECK-NO: call float @llvm.sqrt.f32(float
-  // CHECK-FAST: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);
 
   // CHECK-YES: call x86_fp80 @sqrtl
   // CHECK-NO: call x86_fp80 @llvm.sqrt.f80(x86_fp80
-  // CHECK-FAST: call x86_fp80 @llvm.sqrt.f80(x86_fp80
+  // CHECK-FAST: call reassoc nsz arcp afn x86_fp80 @llvm.sqrt.f80(x86_fp80
   long double l2 = sqrtl(a2);
 }
 
Index: clang/test/CodeGen/fp-options-to-fast-math-flags.c
===
--- /dev/null
+++ clang/test/CodeGen/fp-options-to-fast-math-flags.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck -check-prefix CHECK-PRECISE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-nans -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-NANS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -menable-no-infs -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-INFS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffinite-math-only -emit-llvm -o - %s | FileCheck -check-prefix CHECK-FINITE %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fno-signed-zeros -emit-llvm -o - %s | FileCheck -check-prefix CHECK-NO-SIGNED-ZEROS %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -mreassociate -emit-llvm -o - %s | 

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale marked an inline comment as done.
michele.scandale added inline comments.



Comment at: clang/test/CodeGenOpenCL/relaxed-fpmath.cl:14
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float

This change is based on the following:
* `-cl-fast-relaxed-math` = `-cl-unsafe-math-optimizations` + 
`-cl-finite-math-only` 
* the GCC option `-funsafe-math-optimizations` and 
`-cl-unsafe-math-optimizations` are described with very similar wording and 
from the GCC description states explicitly mention that no signed zeros, 
reassociation and reciprocals are enabled, but there is no mention to assuming 
that NaNs do not exist.

See 
https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/clBuildProgram.html
 and https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale marked 2 inline comments as done.
michele.scandale added inline comments.



Comment at: clang/test/CodeGen/libcalls.c:11-21
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);

For CUDA the default FP contract mode is `fast`, therefore the `contract` FMF 
is emitted.



Comment at: clang/test/CodeGenCUDA/builtins-amdgcn.cu:13
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* 
@_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* 
@_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__

For CUDA the default FP contract mode is `fast`, therefore the `contract` FMF 
is emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80315



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

[PATCH] D80315: Fix CC1 command line options mapping into fast-math flags.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: rjmccall, mibintc, Anastasia.
Herald added subscribers: cfe-commits, jvesely.
Herald added a project: clang.
michele.scandale marked an inline comment as done.
michele.scandale added inline comments.
michele.scandale marked 2 inline comments as done.



Comment at: clang/test/CodeGen/libcalls.c:11-21
+  // CHECK-FAST: call reassoc nsz arcp afn float @llvm.sqrt.f32(float
   float l0 = sqrtf(a0);
 
   // CHECK-YES: call double @sqrt
   // CHECK-NO: call double @llvm.sqrt.f64(double
-  // CHECK-FAST: call double @llvm.sqrt.f64(double
+  // CHECK-FAST: call reassoc nsz arcp afn double @llvm.sqrt.f64(double
   double l1 = sqrt(a1);

For CUDA the default FP contract mode is `fast`, therefore the `contract` FMF 
is emitted.



Comment at: clang/test/CodeGenCUDA/builtins-amdgcn.cu:13
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* 
@_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* 
@_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__

For CUDA the default FP contract mode is `fast`, therefore the `contract` FMF 
is emitted.



Comment at: clang/test/CodeGenOpenCL/relaxed-fpmath.cl:14
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float

This change is based on the following:
* `-cl-fast-relaxed-math` = `-cl-unsafe-math-optimizations` + 
`-cl-finite-math-only` 
* the GCC option `-funsafe-math-optimizations` and 
`-cl-unsafe-math-optimizations` are described with very similar wording and 
from the GCC description states explicitly mention that no signed zeros, 
reassociation and reciprocals are enabled, but there is no mention to assuming 
that NaNs do not exist.

See 
https://www.khronos.org/registry/OpenCL/sdk/1.2/docs/man/xhtml/clBuildProgram.html
 and https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html


This fixes the mapping between CC1 command line options to the
properties in `LangOptions` describing the floating point optimizations
configuration.
The default fast-math flags for the IR builder are now derived from such
properties to avoid inconsistencies.
Finally some of the `CodeGenOptions` floating point optimizations
properties have been removed since they are now exist in `LangOptions`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80315

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/builtins-nvptx-ptx60.cu
  clang/test/CodeGen/complex-math.c
  clang/test/CodeGen/fp-options-to-fast-math-flags.c
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl

Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -11,7 +11,7 @@
   // NORMAL: fdiv float
   // FAST: fdiv fast float
   // FINITE: fdiv nnan ninf float
-  // UNSAFE: fdiv nnan nsz float
+  // UNSAFE: fdiv reassoc nsz arcp afn float
   // MAD: fdiv float
   // NOSIGNED: fdiv nsz float
   return a / b;
@@ -38,7 +38,7 @@
 
 // UNSAFE: "less-precise-fpmad"="true"
 // UNSAFE: "no-infs-fp-math"="false"
-// UNSAFE: "no-nans-fp-math"="true"
+// UNSAFE: "no-nans-fp-math"="false"
 // UNSAFE: "no-signed-zeros-fp-math"="true"
 // UNSAFE: "unsafe-fp-math"="true"
 
Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -10,7 +10,7 @@
 }
 
 // CHECK-LABEL: @_Z12test_ds_fmaxf(
-// CHECK: call float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
+// CHECK: call contract float @llvm.amdgcn.ds.fmax(float addrspace(3)* @_ZZ12test_ds_fmaxfE6shared, float %{{[^,]*}}, i32 0, i32 0, i1 false)
 __global__
 void test_ds_fmax(float src) {
   __shared__ float shared;
Index: clang/test/CodeGen/libcalls.c
===
--- clang/test/CodeGen/libcalls.c
+++ clang/test/CodeGen/libcalls.c
@@ -8,17 +8,17 @@
 void test_sqrt(float a0, double a1, long double a2) {
   // CHECK-YES: call float @sqrtf
   // CHECK-NO: call float @llvm.sqrt.f32(float
-  // CHECK-FAST: call float @llvm.sqrt.f32(float
+  // CHECK-FAST: call reassoc nsz arcp afn float 

[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement

[Michele Scandale via Phabricator via cfe-commits]

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] D72841: Add support for pragma float_control, to control precision and exception behavior at the source level

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() ||
+   FPFeatures.allowFPContractWithinStatement());
 }

mibintc wrote:
> mibintc wrote:
> > michele.scandale wrote:
> > > I'm not convinced it correct to set `contract` when 
> > > `allowFPContractWithinStatement` return true. Can someone clarify this?
> > > 
> > > If I compile the following example with `-ffp-contract=on`:
> > > ```
> > > float test1(float a, float b, float c) {
> > >   float x = a * b;
> > >   return x + c;
> > > }
> > > 
> > > float test2(float a, float b, float c) {
> > >   return a * b + c;
> > > }
> > > ```
> > > 
> > > Before this change the generated code was:
> > > ```
> > > define float @test1(float %a, float %b, float %c) {
> > >   %0 = fmul float %a, %b
> > >   %1 = fadd float %0, %c
> > >   ret float %1
> > > }
> > > 
> > > define float @test2(float %a, float %b, float %c) {
> > >   %0 = call float @llvm.fmuladd.f32(float %a, float%b, float %c)
> > >   ret float %0
> > > }
> > > ```
> > > 
> > > And my understanding is that the in-statement contraction is implemented 
> > > by emitting the `llvm.fmuladd` call that a backend might decide to 
> > > implement as `fmul + fadd` or as `fma`.
> > > 
> > > With this change the generated code is:
> > > ```
> > > define float @test1(float %a, float %b, float %c) {
> > >   %0 = fmul contract float %a, %b
> > >   %1 = fadd contract float %0, %c
> > >   ret float %1
> > > }
> > > 
> > > define float @test2(float %a, float %b, float %c) {
> > >   %0 = call contract float @llvm.fmuladd.f32(float %a, float%b, float %c)
> > >   ret float %0
> > > }
> > > ```
> > > and it seems to me that in `test1` (where multiple statements where 
> > > explicitly used) the optimizer is now allowed to perform the contraction, 
> > > violating the original program semantic where only "in-statement" 
> > > contraction was allowed.
> > Thanks @michele.scandale i will work on a patch for this
> @michele.scandale I posted a patch for 'contract' here, 
> https://reviews.llvm.org/D79903 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



___
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 via Phabricator via cfe-commits]

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] D72841: Add support for pragma float_control, to control precision and exception behavior at the source level

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D72841#2030707 , @rjmccall wrote:

> IIUC, the way within-statement contraction is supposed to work is that there 
> are supposed to be blocking intrinsics inserted at various places.  I don't 
> remember the details, though, or if anyone's thought about how it interacts 
> with cross-statement contraction, which this pragma permits within the same 
> function (and could occur with inlining regardless).


Prior to this change `contract` was never generated in the case of in-statement 
contraction only, instead clang was emitting `llvm.fmuladd` to inform the 
backend that only those were eligible for contraction. From a correctness 
perspective I think this was perfectly fine.

Currently I don't see any logic to generate "blocking intrinsics" (I guess to 
define a region around the instructions emitted for the given statement). Until 
such mechanism is in place, I think that generating the `contract` fast-math 
flag  also for in-statement contraction is wrong because it breaks the original 
program semantic.

Am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D72841: Add support for pragma float_control, to control precision and exception behavior at the source level

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:396-401
+allow_reassoc(LangOpts.FastMath || LangOpts.AllowFPReassoc),
+no_nans(LangOpts.FastMath || LangOpts.NoHonorNaNs),
+no_infs(LangOpts.FastMath || LangOpts.NoHonorInfs),
+no_signed_zeros(LangOpts.FastMath || LangOpts.NoSignedZero),
+allow_reciprocal(LangOpts.FastMath || LangOpts.AllowRecip),
+approx_func(LangOpts.FastMath || LangOpts.ApproxFunc) {}

Same comment on `LangOpts.FastMath || ` as the one for `CompilerInvocation.cpp`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3190-3196
+  Opts.AllowFPReassoc = Opts.FastMath || CGOpts.Reassociate;
+  Opts.NoHonorNaNs =
+  Opts.FastMath || CGOpts.NoNaNsFPMath || Opts.FiniteMathOnly;
+  Opts.NoHonorInfs =
+  Opts.FastMath || CGOpts.NoInfsFPMath || Opts.FiniteMathOnly;
+  Opts.NoSignedZero = Opts.FastMath || CGOpts.NoSignedZeros;
+  Opts.AllowRecip = Opts.FastMath || CGOpts.ReciprocalMath;

Why do we need `Opts.FastMath || ` here? The code in the compiler driver 
`clang/lib/Driver/ToolChains/Clang.cpp` 
(https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Clang.cpp#L2510)
 already takes care of generating the right flags for the CC1 to configure the 
floating point rules.

Moreover, if we ignore what the compiler driver does, the fact that 
`Args.hasArg(OPT_ffast_math)` is not considered in the definition of the 
codegen options such as `NoInfsFPMath`, `NoNaNsFPMath`, `NoSignedZeros`, 
`Reassociate`, so the you have already two distinct options for the same 
abstract property that might not match.

I think that at the CC1 level the reasoning should be done in terms of the fine 
grain options, and let the compiler driver makes life easy for the users -- 
i.e. `LangOpts.FastMath` should just control whether the macro `__FAST_MATH__` 
is defined or not.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D72841: Add support for pragma float_control, to control precision and exception behavior at the source level

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:227
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() ||
+   FPFeatures.allowFPContractWithinStatement());
 }

I'm not convinced it correct to set `contract` when 
`allowFPContractWithinStatement` return true. Can someone clarify this?

If I compile the following example with `-ffp-contract=on`:
```
float test1(float a, float b, float c) {
  float x = a * b;
  return x + c;
}

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

Before this change the generated code was:
```
define float @test1(float %a, float %b, float %c) {
  %0 = fmul float %a, %b
  %1 = fadd float %0, %c
  ret float %1
}

define float @test2(float %a, float %b, float %c) {
  %0 = call float @llvm.fmuladd.f32(float %a, float%b, float %c)
  ret float %0
}
```

And my understanding is that the in-statement contraction is implemented by 
emitting the `llvm.fmuladd` call that a backend might decide to implement as 
`fmul + fadd` or as `fma`.

With this change the generated code is:
```
define float @test1(float %a, float %b, float %c) {
  %0 = fmul contract float %a, %b
  %1 = fadd contract float %0, %c
  ret float %1
}

define float @test2(float %a, float %b, float %c) {
  %0 = call contract float @llvm.fmuladd.f32(float %a, float%b, float %c)
  ret float %0
}
```
and it seems to me that in `test1` (where multiple statements where explicitly 
used) the optimizer is now allowed to perform the contraction, violating the 
original program semantic where only "in-statement" contraction was allowed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 246139.
michele.scandale added a comment.
Herald added a subscriber: jfb.

Tests for `__attribute__(())` syntax + attribute arguments + fix for `_Atomic` 
qualifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp
  clang/test/Parser/cxx-ambig-decl-expr.cpp
  clang/test/Parser/cxx-attributes.cpp

Index: clang/test/Parser/cxx-attributes.cpp
===
--- clang/test/Parser/cxx-attributes.cpp
+++ clang/test/Parser/cxx-attributes.cpp
@@ -22,3 +22,15 @@
 }
 
 __attribute((typename)) int x; // expected-warning {{unknown attribute 'typename' ignored}}
+
+void fn() {
+  void (*__attribute__((attr)) fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*__attribute__((attrA)) *__attribute__((attrB)) fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&__attribute__((attr)) fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&__attribute__((attr)) fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+
+  int i[5];
+  int (*__attribute__((attr(i[1]))) pi);  // expected-warning{{unknown attribute 'attr' ignored}}
+  pi = [0];
+}
Index: clang/test/Parser/cxx-ambig-decl-expr.cpp
===
--- clang/test/Parser/cxx-ambig-decl-expr.cpp
+++ clang/test/Parser/cxx-ambig-decl-expr.cpp
@@ -38,4 +38,7 @@
   // These are array declarations.
   int(x[(1,1)]); // expected-error {{redefinition}}
   int(x[true ? 1,1 : 1]); // expected-error {{redefinition}}
+
+  int (*_Atomic atomic_ptr_to_int);
+  *atomic_ptr_to_int = 42;
 }
Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,15 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+
+  int i[5];
+  int (*[[attr(i[1])]] pi);  // expected-warning{{unknown attribute 'attr' ignored}}
+  pi = [0];
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -186,21 +186,8 @@
 ConsumeToken();
 
 // Skip attributes.
-while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
-   tok::kw_alignas)) {
-  if (Tok.is(tok::l_square)) {
-ConsumeBracket();
-if (!SkipUntil(tok::r_square))
-  return TPResult::Error;
-  } else {
-ConsumeToken();
-if (Tok.isNot(tok::l_paren))
-  return TPResult::Error;
-ConsumeParen();
-if (!SkipUntil(tok::r_paren))
-  return TPResult::Error;
-  }
-}
+if (!TrySkipAttributes())
+  return TPResult::Error;
 
 if (TryAnnotateOptionalCXXScopeToken())
   return TPResult::Error;
@@ -781,6 +768,32 @@
   return CAK_NotAttributeSpecifier;
 }
 
+bool Parser::TrySkipAttributes() {
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (Tok.isNot(tok::l_square))
+return false;
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square) || Tok.isNot(tok::r_square))
+return false;
+  // Note that explicitly checking for `[[` and `]]` allows to fail as
+  // expected in the case of the Objective-C message send syntax.
+  ConsumeBracket();
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return false;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return false;
+}
+  }
+
+  return true;
+}
+
 Parser::TPResult Parser::TryParsePtrOperatorSeq() {
   while (true) {
 if (TryAnnotateOptionalCXXScopeToken(true))
@@ -790,9 +803,14 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  if (!TrySkipAttributes())
+return TPResult::Error;
+
   while 

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 246095.
michele.scandale added a comment.

Fixed comment wording + rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp

Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -186,21 +186,8 @@
 ConsumeToken();
 
 // Skip attributes.
-while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
-   tok::kw_alignas)) {
-  if (Tok.is(tok::l_square)) {
-ConsumeBracket();
-if (!SkipUntil(tok::r_square))
-  return TPResult::Error;
-  } else {
-ConsumeToken();
-if (Tok.isNot(tok::l_paren))
-  return TPResult::Error;
-ConsumeParen();
-if (!SkipUntil(tok::r_paren))
-  return TPResult::Error;
-  }
-}
+if (!TrySkipAttributes())
+  return TPResult::Error;
 
 if (TryAnnotateOptionalCXXScopeToken())
   return TPResult::Error;
@@ -781,6 +768,32 @@
   return CAK_NotAttributeSpecifier;
 }
 
+bool Parser::TrySkipAttributes() {
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (Tok.isNot(tok::l_square))
+return false;
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square) || Tok.isNot(tok::r_square))
+return false;
+  // Note that explicitly checking for `[[` and `]]` allows to fail as
+  // expected in the case of the Objective-C message send syntax.
+  ConsumeBracket();
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return false;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return false;
+}
+  }
+
+  return true;
+}
+
 Parser::TPResult Parser::TryParsePtrOperatorSeq() {
   while (true) {
 if (TryAnnotateOptionalCXXScopeToken(true))
@@ -790,6 +803,11 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  if (!TrySkipAttributes())
+return TPResult::Error;
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2441,6 +2441,10 @@
   TPResult TryParseBracketDeclarator();
   TPResult TryConsumeDeclarationSpecifier();
 
+  /// Try to skip a possibly empty sequence of 'attribute-specifier's without
+  /// full validation of the syntactic structure of attributes.
+  bool TrySkipAttributes();
+
 public:
   TypeResult ParseTypeName(SourceRange *Range = nullptr,
DeclaratorContext Context
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

@rsmith: is this fine for you as well?

If this is ready for landing, I would appreciate if somebody can land this on 
my behalf since I do not have commit rights. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643



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

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale marked an inline comment as done.
michele.scandale added a comment.

Assuming this is fine, I would need someone landing this change on my behalf 
since I do not have commit rights. Thanks!




Comment at: clang/lib/Parse/ParseTentative.cpp:780
+  if (!SkipUntil(tok::r_square) || Tok.isNot(tok::r_square))
+return false;
+  ConsumeBracket();

rjmccall wrote:
> Okay, great.  The check for adjacent squares at the end makes sure this 
> doesn't catch an ObjC message send, in case this is ever used in a context 
> where there's attribute/expression ambiguity; that might be worth a comment.  
> Otherwise LGTM.
Comment added. Let me know if the wording is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643



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

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 245073.
michele.scandale added a comment.

Add comment about ObjC message send being rejected + rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp

Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -186,21 +186,8 @@
 ConsumeToken();
 
 // Skip attributes.
-while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
-   tok::kw_alignas)) {
-  if (Tok.is(tok::l_square)) {
-ConsumeBracket();
-if (!SkipUntil(tok::r_square))
-  return TPResult::Error;
-  } else {
-ConsumeToken();
-if (Tok.isNot(tok::l_paren))
-  return TPResult::Error;
-ConsumeParen();
-if (!SkipUntil(tok::r_paren))
-  return TPResult::Error;
-  }
-}
+if (!TrySkipAttributes())
+  return TPResult::Error;
 
 if (TryAnnotateOptionalCXXScopeToken())
   return TPResult::Error;
@@ -781,6 +768,32 @@
   return CAK_NotAttributeSpecifier;
 }
 
+bool Parser::TrySkipAttributes() {
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (Tok.isNot(tok::l_square))
+return false;
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square) || Tok.isNot(tok::r_square))
+return false;
+  // Note that explicitly checking for `[[` and `]]` allows to fail as
+  // expected in the case of the Objective-C message send syntax.
+  ConsumeBracket();
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return false;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return false;
+}
+  }
+
+  return true;
+}
+
 Parser::TPResult Parser::TryParsePtrOperatorSeq() {
   while (true) {
 if (TryAnnotateOptionalCXXScopeToken(true))
@@ -790,6 +803,11 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  if (!TrySkipAttributes())
+return TPResult::Error;
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2441,6 +2441,10 @@
   TPResult TryParseBracketDeclarator();
   TPResult TryConsumeDeclarationSpecifier();
 
+  /// Try to skip a possibly empty sequence of 'attribute-specifier' without of
+  /// full validation of the syntactic structure of attributes.
+  bool TrySkipAttributes();
+
 public:
   TypeResult ParseTypeName(SourceRange *Range = nullptr,
DeclaratorContext Context
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale marked an inline comment as done.
michele.scandale added inline comments.



Comment at: clang/lib/Parse/ParseTentative.cpp:798
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square))

rjmccall wrote:
> Do you want to check for double-brackets here?
The copy of the code I was replicating here wasn't doing that. But it makes 
sense to check at least that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643



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

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 245068.
michele.scandale added a comment.

Check for double brackets for C++11 style attributes, and outline the loop to 
skip attributes in `TrySkipAttributes` to fix also the other use in 
`TryConsumeDeclarationSpecifier`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp

Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -186,21 +186,8 @@
 ConsumeToken();
 
 // Skip attributes.
-while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
-   tok::kw_alignas)) {
-  if (Tok.is(tok::l_square)) {
-ConsumeBracket();
-if (!SkipUntil(tok::r_square))
-  return TPResult::Error;
-  } else {
-ConsumeToken();
-if (Tok.isNot(tok::l_paren))
-  return TPResult::Error;
-ConsumeParen();
-if (!SkipUntil(tok::r_paren))
-  return TPResult::Error;
-  }
-}
+if (!TrySkipAttributes())
+  return TPResult::Error;
 
 if (TryAnnotateOptionalCXXScopeToken())
   return TPResult::Error;
@@ -781,6 +768,30 @@
   return CAK_NotAttributeSpecifier;
 }
 
+bool Parser::TrySkipAttributes() {
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (Tok.isNot(tok::l_square))
+return false;
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square) || Tok.isNot(tok::r_square))
+return false;
+  ConsumeBracket();
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return false;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return false;
+}
+  }
+
+  return true;
+}
+
 Parser::TPResult Parser::TryParsePtrOperatorSeq() {
   while (true) {
 if (TryAnnotateOptionalCXXScopeToken(true))
@@ -790,6 +801,11 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  if (!TrySkipAttributes())
+return TPResult::Error;
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2441,6 +2441,10 @@
   TPResult TryParseBracketDeclarator();
   TPResult TryConsumeDeclarationSpecifier();
 
+  /// Try to skip a possibly empty sequence of 'attribute-specifier' without of
+  /// full validation of the syntactic structure of attributes.
+  bool TrySkipAttributes();
+
 public:
   TypeResult ParseTypeName(SourceRange *Range = nullptr,
DeclaratorContext Context
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale created this revision.
michele.scandale added reviewers: rjmccall, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
michele.scandale updated this revision to Diff 244756.
michele.scandale added a comment.

Missed GNU style attributes


The syntax rules for `ptr-operator` allow attributes after `*`, `&`,
`&&`, therefore we should be able to parse the following:

  void fn() {
  void (*[[attr]] x)() = 
  void (&[[attr]] y)() = fn;
  void (&&[[attr]] z)() = fn;
  }

However the current logic in `TryParsePtrOperatorSeq` does not consider
the presence of attributes leading to unexpected parsing errors.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74643

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp


Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 
'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // 
expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown 
attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 
'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 
'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -790,6 +790,24 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, 
tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square))
+return TPResult::Error;
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return TPResult::Error;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return TPResult::Error;
+}
+  }
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))


Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -790,6 +790,24 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square))
+return TPResult::Error;
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return TPResult::Error;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return TPResult::Error;
+}
+  }
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74643: [Parse] Consider attributes in `TryParsePtrOperatorSeq`.

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale updated this revision to Diff 244756.
michele.scandale added a comment.

Missed GNU style attributes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74643

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/CXX/dcl.decl/p4-0x.cpp


Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 
'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // 
expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown 
attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 
'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 
'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -790,6 +790,24 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, 
tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square))
+return TPResult::Error;
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return TPResult::Error;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return TPResult::Error;
+}
+  }
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))


Index: clang/test/CXX/dcl.decl/p4-0x.cpp
===
--- clang/test/CXX/dcl.decl/p4-0x.cpp
+++ clang/test/CXX/dcl.decl/p4-0x.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 struct X {
   void f() &;
@@ -7,3 +6,11 @@
 };
 
 void (X::*pmf)() & = ::f;
+
+void fn() {
+  void (*[[attr]] fn_ptr)() =  // expected-warning{{unknown attribute 'attr' ignored}}
+  void (*[[attrA]] *[[attrB]] fn_ptr_ptr)() = _ptr; // expected-warning{{unknown attribute 'attrA' ignored}} expected-warning{{unknown attribute 'attrB' ignored}}
+
+  void (&[[attr]] fn_lref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+  void (&&[[attr]] fn_rref)() = fn; // expected-warning{{unknown attribute 'attr' ignored}}
+}
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -790,6 +790,24 @@
 (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::star))) {
   // ptr-operator
   ConsumeAnyToken();
+
+  // Skip attributes.
+  while (Tok.isOneOf(tok::l_square, tok::kw___attribute, tok::kw___declspec,
+ tok::kw_alignas)) {
+if (Tok.is(tok::l_square)) {
+  ConsumeBracket();
+  if (!SkipUntil(tok::r_square))
+return TPResult::Error;
+} else {
+  ConsumeToken();
+  if (Tok.isNot(tok::l_paren))
+return TPResult::Error;
+  ConsumeParen();
+  if (!SkipUntil(tok::r_paren))
+return TPResult::Error;
+}
+  }
+
   while (Tok.isOneOf(tok::kw_const, tok::kw_volatile, tok::kw_restrict,
  tok::kw__Nonnull, tok::kw__Nullable,
  tok::kw__Null_unspecified))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

In D62731#1775046 , @mibintc wrote:

> In D62731#1773854 , 
> @michele.scandale wrote:
>
> > I've noticed you removed the change for `CompilerInvocation.cpp` about the 
> > initialization of the codegen option `NoTrappingMath`. Was that an accident?
>
>
> I checked the old and new version of the patch and it seems like 
> initialization of NoTrappingMath is unchanged, the definition of the option 
> has it default to 0, and CompilerInvocation.cpp sets it like this in 
> ParseLangArgs, was it something else you were looking at?
>
>   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);


In the driver code you don't always pass `-fno-trapping-math`, therefore when 
when the compiler setup the CodeGen options in `ParseCodeGenArgs` you will end 
up executing `Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);` hence 
you will have that `Opt.NoTrappingMath = false`. This is inconsistent with the 
state of the compiler driver where no-trapping-math is enabled default.

If you want to keep the default of the CC1 different than the default of the 
compiler driver, that's fine to me, but in that case the compiler driver needs 
to pass `-fno-trapping-math` to the CC1.
If we want the same new default, then the logic in `ParseCodeGenArgs` must be 
updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added a comment.

I've noticed you removed the change for `CompilerInvocation.cpp` about the 
initialization of the codegen option `NoTrappingMath`. Was that an accident?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2345
+<< "-ffp-contract=fast";
+optID = options::OPT_ffp_contract;
+FPModel = Val;

mibintc wrote:
> michele.scandale wrote:
> > Here the state of the floating point options seems unchanged except for 
> > `FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
> > expect the state of the floating point options to match the one of 
> > `-fno-fast-math` except for `FPContract` which you want to be set to "fast".
> > 
> > I think you might need to replicate the reset for all the option here as 
> > well, so at this point I don't know how much worth is to use the optID 
> > reset trick for the "fast" case only.
> @michele.scandale Thanks for your helpful review, I think I fixed the things 
> that you remarked on. I also added a test case for the assertion fail that 
> you saw.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2337
+<< "-ffp-contract=fast";
+optID = options::OPT_ffast_math;
+FPModel = Val;

Here it seems you are changing `optID` to `OPT_ffast_math` to reuse the logic 
specified below for that case to reset the state of the floating point options.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2345
+<< "-ffp-contract=fast";
+optID = options::OPT_ffp_contract;
+FPModel = Val;

Here the state of the floating point options seems unchanged except for 
`FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would 
expect the state of the floating point options to match the one of 
`-fno-fast-math` except for `FPContract` which you want to be set to "fast".

I think you might need to replicate the reset for all the option here as well, 
so at this point I don't know how much worth is to use the optID reset trick 
for the "fast" case only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

mibintc wrote:
> michele.scandale wrote:
> > With this change if I run `clang -### -ffast-math test.c` I don't see 
> > `-fno-trapping-math` passed to the CC1. 
> > This is changing the changes the value of the function level attribute 
> > "no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
> > Is this an intended change?
> > 
> > Moreover since with this patch the default value for trapping math changed, 
> > the "no-trapping-math" function level attribute is incorrect also for 
> > default case.
> Before this patch, ftrapping-math was added to the Driver and also a 
> bitfield, ``NoTrappingFPMath`` was created in the LLVM to describe the state 
> of trapping-math, but otherwise that bit wasn't consulted and the option had 
> no effect.  Gcc describes ftrapping-math as the default, but in llvm by 
> default floating point exceptions are masked and this corresponds to the 
> floating point Constrained Intrinsics having exception behavior set to 
> ignored.  This patch changed the llvm constructor to set the trapping bit to 
> "no trap".  In fact I'd like to get rid of the ``NoTrappingFPMath`` bitfield 
> since it's not being used, but I didn't make that change at this point. 
> 
> If I remember correctly, there are a bunch of driver tests that failed if 
> fno-trapping-math is output to cc1. I'd have to reconstruct the details.  
> Since fno-trapping-math is the default, it isn't passed through on the cc1 
> command line: the Clang.cpp driver doesn't pass through the positive and 
> negative for each existing option.
> 
> Thanks for pointing out the line in CGCall.cpp, it seems the CodeGenOpts 
> aren't getting set up perfectly I'll fix that in CompilerInvocation.cpp; I 
> don't see anything setting trapping-math as part of function level attribute, 
> @michele.scandale  did I overlook that/can you point out where that is?
I guess you are referring to the code in `TargetMachine.cpp` where the function 
level attributes are used to reset the `TargetOptions` state whenever we 
initiate the backend codegen for a given function. Considering that the 
trapping math option as stated in the documentation did not have any effect, 
I'm not surprised to see not many uses. The only one I can see is in 
`llvm/lib/Target/ARM/ARMAsmPrinter.cpp : 687` where the function level 
attribute affects the emission of some ARM specific attributes.

My only concern was that the change of the default value for trapping math was 
not propagated entirely causing this function level attribute to be initialized 
incorrectly.
Fixing the logic in `CompilerInvocation.cpp` considering the change of default 
it is fine for me.

Given that `ffp-exception-behavior={ignore,maytrap,strict}` supersedes 
`-f{,no-}trapping-math` I would expect long term to see the internal state of 
the compiler frontend to only care about the new state `FPExceptionBehavior` 
for both language and code generation options. And I guess the same would apply 
to the backend stage as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Michele Scandale via Phabricator via cfe-commits]

michele.scandale added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2408
+case options::OPT_frounding_math:
+  // The default setting for frounding-math is True and ffast-math
+  // sets fno-rounding-math, but we only want to use constrained

Isn't the default `-fno-rounding-math`?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2416
+  RoundingFPMath = false;
+  RoundingMathPresent = false;
+  break;

Shouldn't this be set to `true` similarly to what you do for 
`TrappingMathPresent ` to track whether there is an explicit option related to 
rounding math?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2574
+// FP Exception Behavior is also set to strict
+assert(FPExceptionBehavior.equals("strict"));
+CmdArgs.push_back("-ftrapping-math");

Running `clang -### -ftrapping-math -ffp-exception-behavior=ignore` lead to 
this assertion to fail. As far as I can see `TrappingMath` is not changed in 
the case FPExceptionBehavior is "ignore" or "maytrap".
Clearly in the "ignore" case it should be safe to just set `TrappingMath` to 
false, but I'm not sure about the "maytrap" case.
It seems that `-ffp-exception-behavior` is more general than 
`-f{,no-}trapping-math`, so it seems natural to me to see `ftrapping-math` and 
`foo-trapping-math` as aliases for `ffp-exception-behavior=strict` and 
`ffp-exception-behavior=ignore` respectively. If we agree on this, then I would 
expect the reasoning inside the compiler only in terms of `FPExceptionBehavior`.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2576
+CmdArgs.push_back("-ftrapping-math");
+  } else if (TrappingMathPresent)
 CmdArgs.push_back("-fno-trapping-math");

With this change if I run `clang -### -ffast-math test.c` I don't see 
`-fno-trapping-math` passed to the CC1. 
This is changing the changes the value of the function level attribute 
"no-trapping-math" (see lib/CodeGen/CGCall.cpp : 1747).
Is this an intended change?

Moreover since with this patch the default value for trapping math changed, the 
"no-trapping-math" function level attribute is incorrect also for default case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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