[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-10 Thread Bjorn Pettersson via Phabricator via cfe-commits 
bjope added a comment.

I have no objections about doing this revert.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-10 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam added a comment.

In D145704#4184584 , @aaron.ballman 
wrote:

> LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the 
> next ~4 hours, feel free to land.

Great! Thanks @aaron.ballman  and @tstellar.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-10 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the next 
~4 hours, feel free to land.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Tom Stellard via Phabricator via cfe-commits 
tstellar added a comment.

We're holding -rc4 until this is merged, so it would be great if we could merge 
it on Friday.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D145704#4182364 , @aaron.ballman 
wrote:

> This variant comes with a .patch file added to the PR; that should be deleted 
> in the next round. Aside from that, the changes LGTM, but I'd like to give 
> precommit CI a chance to complete and some time for the folks impacted by the 
> changes to weigh in before accepting.

Heh you caught the .patch file and already fixed it, thanks!


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D145704#4182341 , @zahiraam wrote:

> In D145704#4182268 , @aaron.ballman 
> wrote:
>
>> Thank you for working on this! Because this is fixing an ABI break, if we 
>> can land this in time for the release candidate I think we should try to add 
>> this to 16.0 to limit damage. CC @tstellar for his opinion.
>>
>> This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5 
>> ; that 
>> commit changed `clang/lib/Sema/Sema.cpp` but this commit does not back those 
>> changes out. Is that intentional?
>
> Yes that makes sense because the changes in Sema were moved to 
> lib/lex/Preprocessor.cpp  in this patch: https://reviews.llvm.org/D128814

Ah, thank you for the explanation, that makes sense.

This variant comes with a .patch file added to the PR; that should be deleted 
in the next round. Aside from that, the changes LGTM, but I'd like to give 
precommit CI a chance to complete and some time for the folks impacted by the 
changes to weigh in before accepting.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam updated this revision to Diff 503865.

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

https://reviews.llvm.org/D145704

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/X86/fexcess-precision.c
  clang/test/CodeGen/eval-method-fast-math.cpp
  clang/test/Preprocessor/flt_eval_macro.cpp

Index: clang/test/Preprocessor/flt_eval_macro.cpp
===
--- clang/test/Preprocessor/flt_eval_macro.cpp
+++ clang/test/Preprocessor/flt_eval_macro.cpp
@@ -17,7 +17,7 @@
 // RUN:   %s -o - | FileCheck %s  -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple=x86_64-apple-macos13.0 -ffast-math \
-// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-MINUS-ONE -strict-whitespace
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 %s -o - \
 // RUN:   | FileCheck %s  -strict-whitespace
@@ -64,7 +64,6 @@
 
 int foo() {
   // CHECK: #define Name "One"
-  // CHECK-MINUS-ONE: #define Name "MinusOne"
   // EXT: #define Name "Three"
   return Name;
 }
Index: clang/test/CodeGen/eval-method-fast-math.cpp
===
--- clang/test/CodeGen/eval-method-fast-math.cpp
+++ /dev/null
@@ -1,117 +0,0 @@
-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
-// RUN: | FileCheck %s -check-prefixes=CHECK
-
-// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
-
-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
-// RUN: -ffast-math -triple x86_64-linux-gnu \
-// RUN: -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
-// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-float a = 1.0f, b = 2.0f, c = 3.0f;
-#pragma float_control(precise, off)
-float res2 = a + b + c;
-int val3 = __FLT_EVAL_METHOD__;
-#pragma float_control(precise, on)
-float res3 = a + b + c;
-int val4 = __FLT_EVAL_METHOD__;
-
-// CHECK: @val3 = global i32 -1
-// CHECK: @val4 = global i32 0
-
-// CHECK-EXT: @val3 = global i32 -1
-// CHECK-EXT: @val4 = global i32 2
-
-// CHECK-FAST: @val3 = global i32 -1
-// CHECK-FAST: @val4 = global i32 -1
-
-float res;
-int add(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-int add_precise(float a, float b, float c) {
-#pragma float_control(precise, on)
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-#pragma float_control(precise, on)
-int add_precise_1(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int add_not_precise(float a, float b, float c) {
-  // Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-// Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-int add_not_precise_1(float a, float b, float c) {
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int getFPEvalMethod() {
-  // CHECK: ret i32 0
-  return __FLT_EVAL_METHOD__;
-}
-
-float res1;
-int whatever(float a, float b, float c) {
-#pragma float_control(precise, off)
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: store float {{.*}}, ptr
-  // CHECK: store i32 -1
-  // CHECK: store i32 0
-  // CHECK: ret i32 -1
-  res1 = a + b + c;
-  int val1 = __FLT_EVAL_METHOD__;
-  {
-#pragma float_control(precise, on)
-int val2 = __FLT_EVAL_METHOD__;
-  }
-  return __FLT_EVAL_METHOD__;
-}
Index: clang/test/CodeGen/X86/fexcess-precision.c
===
--- clang/test/CodeGen/X86/fexcess-precision.c
+++ clang/test/CodeGen/X86/fexcess-precision.c
@@ -380,7 +380,7 @@
 //
 // CHECK-UNSAFE-LABEL: @getFEM(
 // CHECK-UNSAFE-NEXT:  entry:
-// CHECK-UNSAFE-NEXT:ret i32 -1

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam added a comment.

In D145704#4182268 , @aaron.ballman 
wrote:

> Thank you for working on this! Because this is fixing an ABI break, if we can 
> land this in time for the release candidate I think we should try to add this 
> to 16.0 to limit damage. CC @tstellar for his opinion.
>
> This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5 
> ; that 
> commit changed `clang/lib/Sema/Sema.cpp` but this commit does not back those 
> changes out. Is that intentional?

Yes that makes sense because the changes in Sema were moved to 
lib/lex/Preprocessor.cpp  in this patch: https://reviews.llvm.org/D128814


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a subscriber: tstellar.
aaron.ballman added a comment.

Thank you for working on this! Because this is fixing an ABI break, if we can 
land this in time for the release candidate I think we should try to add this 
to 16.0 to limit damage. CC @tstellar for his opinion.

This doesn't seem to fully revert bbf0d1932a3c1be970ed8a580e51edf571b80fd5 
; that 
commit changed `clang/lib/Sema/Sema.cpp` but this commit does not back those 
changes out. Is that intentional?




Comment at: clang/test/Preprocessor/flt_eval_macro.cpp:66-68
   // CHECK: #define Name "One"
   // CHECK-MINUS-ONE: #define Name "MinusOne"
   // EXT: #define Name "Three"

This line is no longer needed because it will never be checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam added subscribers: Wilco1, kito-cheng, bjope.
zahiraam added a comment.

Tagging @davemgreen, @kito-cheng, @bjope and @Wilco1 for awareness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

2023-03-09 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam created this revision.
zahiraam added reviewers: andrew.w.kaylor, aaron.ballman.
Herald added a subscriber: pengfei.
Herald added a project: All.
zahiraam requested review of this revision.
Herald added a project: clang.

Setting __FLT_EVAL_METHOD__ to -1 with fast-math will set 
__GLIBC_FLT_EVAL_METHOD to 2 and long double ends up being used for
float_t and double_t. This creates some ABI breakage with various C libraries.

See details here: https://github.com/llvm/llvm-project/issues/60781

This reverts commit bbf0d1932a3c1be970ed8a580e51edf571b80fd5 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145704

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/X86/fexcess-precision.c
  clang/test/CodeGen/eval-method-fast-math.cpp
  clang/test/Preprocessor/flt_eval_macro.cpp

Index: clang/test/Preprocessor/flt_eval_macro.cpp
===
--- clang/test/Preprocessor/flt_eval_macro.cpp
+++ clang/test/Preprocessor/flt_eval_macro.cpp
@@ -17,7 +17,7 @@
 // RUN:   %s -o - | FileCheck %s  -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple=x86_64-apple-macos13.0 -ffast-math \
-// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-MINUS-ONE -strict-whitespace
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK -strict-whitespace
 
 // RUN: %clang_cc1 -E -dM -triple i386-pc-windows -target-cpu pentium4 %s -o - \
 // RUN:   | FileCheck %s  -strict-whitespace
Index: clang/test/CodeGen/eval-method-fast-math.cpp
===
--- clang/test/CodeGen/eval-method-fast-math.cpp
+++ /dev/null
@@ -1,117 +0,0 @@
-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -triple x86_64-linux-gnu -emit-llvm -o - %s  \
-// RUN: | FileCheck %s -check-prefixes=CHECK
-
-// RUN: %clang_cc1 -triple i386--linux -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-EXT
-
-// RUN: %clang_cc1 -fexperimental-strict-floating-point  \
-// RUN: -mreassociate -freciprocal-math -ffp-contract=fast \
-// RUN: -ffast-math -triple x86_64-linux-gnu \
-// RUN: -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-// RUN: %clang_cc1 -triple i386--linux -mreassociate -freciprocal-math \
-// RUN: -ffp-contract=fast -ffast-math -emit-llvm -o - %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK-FAST
-
-float a = 1.0f, b = 2.0f, c = 3.0f;
-#pragma float_control(precise, off)
-float res2 = a + b + c;
-int val3 = __FLT_EVAL_METHOD__;
-#pragma float_control(precise, on)
-float res3 = a + b + c;
-int val4 = __FLT_EVAL_METHOD__;
-
-// CHECK: @val3 = global i32 -1
-// CHECK: @val4 = global i32 0
-
-// CHECK-EXT: @val3 = global i32 -1
-// CHECK-EXT: @val4 = global i32 2
-
-// CHECK-FAST: @val3 = global i32 -1
-// CHECK-FAST: @val4 = global i32 -1
-
-float res;
-int add(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-int add_precise(float a, float b, float c) {
-#pragma float_control(precise, on)
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-#pragma float_control(precise, on)
-int add_precise_1(float a, float b, float c) {
-  // CHECK: fadd float
-  // CHECK: load float, ptr
-  // CHECK: fadd float
-  // CHECK: store float
-  // CHECK: ret i32 0
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int add_not_precise(float a, float b, float c) {
-  // Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-
-#pragma float_control(push)
-// Fast-math is enabled with this pragma.
-#pragma float_control(precise, off)
-int add_not_precise_1(float a, float b, float c) {
-  // CHECK: fadd fast float
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: float {{.*}}, ptr
-  // CHECK: ret i32 -1
-  res = a + b + c;
-  return __FLT_EVAL_METHOD__;
-}
-#pragma float_control(pop)
-
-int getFPEvalMethod() {
-  // CHECK: ret i32 0
-  return __FLT_EVAL_METHOD__;
-}
-
-float res1;
-int whatever(float a, float b, float c) {
-#pragma float_control(precise, off)
-  // CHECK: load float, ptr
-  // CHECK: fadd fast float
-  // CHECK: store float {{.*}}, ptr
-  // CHECK: store i32 -1
-  // CHECK: store i32 0
-  // CHECK: ret i32 -1
-  res1 = a + b + c;
-  int val1 = __FLT_EVAL_METHOD__;
-  {
-#pragma float_control(precise, on)
-int