[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00a62547da7e: Stop traping on sNaN in __builtin_isnan 
(authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/X86/strictfp_builtins.c
  clang/test/CodeGen/aarch64-strictfp-builtins.c
  clang/test/CodeGen/strictfp_builtins.c
  clang/test/CodeGen/strictfp_fpclassify.c
  llvm/include/llvm/ADT/APFloat.h
  llvm/include/llvm/IR/Type.h

Index: llvm/include/llvm/IR/Type.h
===
--- llvm/include/llvm/IR/Type.h
+++ llvm/include/llvm/IR/Type.h
@@ -308,6 +308,10 @@
   /// ppc long double), this method returns -1.
   int getFPMantissaWidth() const;
 
+  /// Return whether the type is IEEE compatible, as defined by the eponymous
+  /// method in APFloat.
+  bool isIEEE() const { return APFloat::getZero(getFltSemantics()).isIEEE(); }
+
   /// If this is a vector type, return the element type, otherwise return
   /// 'this'.
   inline Type *getScalarType() const {
Index: llvm/include/llvm/ADT/APFloat.h
===
--- llvm/include/llvm/ADT/APFloat.h
+++ llvm/include/llvm/ADT/APFloat.h
@@ -1218,6 +1218,7 @@
   bool isSmallest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isSmallest()); }
   bool isLargest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isLargest()); }
   bool isInteger() const { APFLOAT_DISPATCH_ON_SEMANTICS(isInteger()); }
+  bool isIEEE() const { return usesLayout(getSemantics()); }
 
   APFloat =(const APFloat ) = default;
   APFloat =(APFloat &) = default;
Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -92,17 +92,38 @@
   return;
 }
 
-// CHECK-LABEL: @test_isnan(
+// CHECK-LABEL: @test_float_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:store float [[F:%.*]], float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load float, float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast float [[TMP0]] to i32
+// CHECK-NEXT:[[ABS:%.*]] = and i32 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i32 [[#%u,0x7F80]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i32 [[TMP1]], 31
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[ISNAN]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_float_isnan(float f) {
+  P(isnan, (f));
+
+  return;
+}
+
+// CHECK-LABEL: @test_double_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[D_ADDR:%.*]] = alloca double, align 8
 // CHECK-NEXT:store double [[D:%.*]], double* [[D_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, double* [[D_ADDR]], align 8
-// CHECK-NEXT:[[CMP:%.*]] = call i1 @llvm.experimental.constrained.fcmp.f64(double [[TMP0]], double [[TMP0]], metadata !"uno", metadata !"fpexcept.strict") [[ATTR4]]
-// CHECK-NEXT:[[TMP1:%.*]] = zext i1 [[CMP]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[TMP1]]) [[ATTR4]]
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast double [[TMP0]] to i64
+// CHECK-NEXT:[[ABS:%.*]] = and i64 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i64 [[#%u,0x7FF0]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i64 [[TMP1]], 63
+// CHECK-NEXT:[[RES:%.*]] = trunc i64 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.5, i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
-void test_isnan(double d) {
+void test_double_isnan(double d) {
   P(isnan, (d));
 
   return;
@@ -120,7 +141,7 @@
 // CHECK-NEXT:[[AND:%.*]] = and i1 [[ISEQ]], [[ISINF]]
 // CHECK-NEXT:[[AND1:%.*]] = and i1 [[AND]], [[ISNORMAL]]
 // CHECK-NEXT:[[TMP2:%.*]] = zext i1 [[AND1]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.5, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.6, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
 void test_isnormal(double d) {
Index: clang/test/CodeGen/aarch64-strictfp-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-strictfp-builtins.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -o - -triple arm64-none-linux-gnu | FileCheck %s
+
+// Test that the constrained intrinsics are picking up the 

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Kevin P. Neal via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre added inline comments.



Comment at: clang/test/CodeGen/aarch64-strictfp-builtins.c:1
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap 
-fexperimental-strict-floating-point -o - -triple arm64-none-linux-gnu | 
FileCheck %s
+

AArch64 is not StrictFP enabled so we need this option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 321554.
thopre added a comment.

Add AArch64 testcase and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/X86/strictfp_builtins.c
  clang/test/CodeGen/aarch64-strictfp-builtins.c
  clang/test/CodeGen/strictfp_builtins.c
  clang/test/CodeGen/strictfp_fpclassify.c
  llvm/include/llvm/ADT/APFloat.h
  llvm/include/llvm/IR/Type.h

Index: llvm/include/llvm/IR/Type.h
===
--- llvm/include/llvm/IR/Type.h
+++ llvm/include/llvm/IR/Type.h
@@ -308,6 +308,10 @@
   /// ppc long double), this method returns -1.
   int getFPMantissaWidth() const;
 
+  /// Return whether the type is IEEE compatible, as defined by the eponymous
+  /// method in APFloat.
+  bool isIEEE() const { return APFloat::getZero(getFltSemantics()).isIEEE(); }
+
   /// If this is a vector type, return the element type, otherwise return
   /// 'this'.
   inline Type *getScalarType() const {
Index: llvm/include/llvm/ADT/APFloat.h
===
--- llvm/include/llvm/ADT/APFloat.h
+++ llvm/include/llvm/ADT/APFloat.h
@@ -1218,6 +1218,7 @@
   bool isSmallest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isSmallest()); }
   bool isLargest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isLargest()); }
   bool isInteger() const { APFLOAT_DISPATCH_ON_SEMANTICS(isInteger()); }
+  bool isIEEE() const { return usesLayout(getSemantics()); }
 
   APFloat =(const APFloat ) = default;
   APFloat =(APFloat &) = default;
Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -92,17 +92,38 @@
   return;
 }
 
-// CHECK-LABEL: @test_isnan(
+// CHECK-LABEL: @test_float_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:store float [[F:%.*]], float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load float, float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast float [[TMP0]] to i32
+// CHECK-NEXT:[[ABS:%.*]] = and i32 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i32 [[#%u,0x7F80]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i32 [[TMP1]], 31
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[ISNAN]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_float_isnan(float f) {
+  P(isnan, (f));
+
+  return;
+}
+
+// CHECK-LABEL: @test_double_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[D_ADDR:%.*]] = alloca double, align 8
 // CHECK-NEXT:store double [[D:%.*]], double* [[D_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, double* [[D_ADDR]], align 8
-// CHECK-NEXT:[[CMP:%.*]] = call i1 @llvm.experimental.constrained.fcmp.f64(double [[TMP0]], double [[TMP0]], metadata !"uno", metadata !"fpexcept.strict") [[ATTR4]]
-// CHECK-NEXT:[[TMP1:%.*]] = zext i1 [[CMP]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[TMP1]]) [[ATTR4]]
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast double [[TMP0]] to i64
+// CHECK-NEXT:[[ABS:%.*]] = and i64 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i64 [[#%u,0x7FF0]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i64 [[TMP1]], 63
+// CHECK-NEXT:[[RES:%.*]] = trunc i64 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.5, i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
-void test_isnan(double d) {
+void test_double_isnan(double d) {
   P(isnan, (d));
 
   return;
@@ -120,7 +141,7 @@
 // CHECK-NEXT:[[AND:%.*]] = and i1 [[ISEQ]], [[ISINF]]
 // CHECK-NEXT:[[AND1:%.*]] = and i1 [[AND]], [[ISNORMAL]]
 // CHECK-NEXT:[[TMP2:%.*]] = zext i1 [[AND1]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.5, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.6, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
 void test_isnormal(double d) {
Index: clang/test/CodeGen/aarch64-strictfp-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-strictfp-builtins.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -fexperimental-strict-floating-point -o - -triple arm64-none-linux-gnu | FileCheck %s
+
+// Test that the constrained intrinsics are picking up the exception
+// metadata from the AST instead of the global default from the command line.
+
+#pragma float_control(except, on)
+

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

In D95948#2542884 , @mibintc wrote:

> Should we add tests for mlong-double-64, -80, -128?

Assuming Ty->getScalarSizeInBits() returns 64, 80, or 128 in those cases, I 
think it's good enough to just add an AArch64 (or some other 128-bit long 
double host) and call it a day. We're testing isnan(), not that command line 
option, after all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Should we add tests for mlong-double-64, -80, -128?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

This looks like a definite step forward. Thank you!

Can you do an AArch64 test case showing long double? Right now there's no 
128-bit test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3011
+Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
+if (bitsize > 32)

thopre wrote:
> mibintc wrote:
> > the comment at line 3001 doesn't show the lshr or the compare to zero
> This logical right shift will move the sign bit to bit0 and set all other 
> bits to 0. The result will be 1 (true) if sign bit is set (sub is negative) 
> or 0 (false) otherwise. That matches the comment. Would you like me to add a 
> comment here to make it more explicit?
No, that's OK, you don't need to do that. thanks.  I'd like to see what other 
reviewers say, thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre updated this revision to Diff 321107.
thopre marked 2 inline comments as done.
thopre added a comment.

- Fix order of sub operands in comments and use fully parenthesized expression
- Explain how LShr is equivalent to < 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/X86/strictfp_builtins.c
  clang/test/CodeGen/strictfp_builtins.c
  clang/test/CodeGen/strictfp_fpclassify.c
  llvm/include/llvm/ADT/APFloat.h
  llvm/include/llvm/IR/Type.h

Index: llvm/include/llvm/IR/Type.h
===
--- llvm/include/llvm/IR/Type.h
+++ llvm/include/llvm/IR/Type.h
@@ -308,6 +308,10 @@
   /// ppc long double), this method returns -1.
   int getFPMantissaWidth() const;
 
+  /// Return whether the type is IEEE compatible, as defined by the eponymous
+  /// method in APFloat.
+  bool isIEEE() const { return APFloat::getZero(getFltSemantics()).isIEEE(); }
+
   /// If this is a vector type, return the element type, otherwise return
   /// 'this'.
   inline Type *getScalarType() const {
Index: llvm/include/llvm/ADT/APFloat.h
===
--- llvm/include/llvm/ADT/APFloat.h
+++ llvm/include/llvm/ADT/APFloat.h
@@ -1218,6 +1218,7 @@
   bool isSmallest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isSmallest()); }
   bool isLargest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isLargest()); }
   bool isInteger() const { APFLOAT_DISPATCH_ON_SEMANTICS(isInteger()); }
+  bool isIEEE() const { return usesLayout(getSemantics()); }
 
   APFloat =(const APFloat ) = default;
   APFloat =(APFloat &) = default;
Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -92,17 +92,38 @@
   return;
 }
 
-// CHECK-LABEL: @test_isnan(
+// CHECK-LABEL: @test_float_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:store float [[F:%.*]], float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load float, float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast float [[TMP0]] to i32
+// CHECK-NEXT:[[ABS:%.*]] = and i32 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i32 [[#%u,0x7F80]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i32 [[TMP1]], 31
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[ISNAN]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_float_isnan(float f) {
+  P(isnan, (f));
+
+  return;
+}
+
+// CHECK-LABEL: @test_double_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[D_ADDR:%.*]] = alloca double, align 8
 // CHECK-NEXT:store double [[D:%.*]], double* [[D_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, double* [[D_ADDR]], align 8
-// CHECK-NEXT:[[CMP:%.*]] = call i1 @llvm.experimental.constrained.fcmp.f64(double [[TMP0]], double [[TMP0]], metadata !"uno", metadata !"fpexcept.strict") [[ATTR4]]
-// CHECK-NEXT:[[TMP1:%.*]] = zext i1 [[CMP]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[TMP1]]) [[ATTR4]]
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast double [[TMP0]] to i64
+// CHECK-NEXT:[[ABS:%.*]] = and i64 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i64 [[#%u,0x7FF0]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i64 [[TMP1]], 63
+// CHECK-NEXT:[[RES:%.*]] = trunc i64 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.5, i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
-void test_isnan(double d) {
+void test_double_isnan(double d) {
   P(isnan, (d));
 
   return;
@@ -120,7 +141,7 @@
 // CHECK-NEXT:[[AND:%.*]] = and i1 [[ISEQ]], [[ISINF]]
 // CHECK-NEXT:[[AND1:%.*]] = and i1 [[AND]], [[ISNORMAL]]
 // CHECK-NEXT:[[TMP2:%.*]] = zext i1 [[AND1]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.5, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.6, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
 void test_isnormal(double d) {
Index: clang/test/CodeGen/X86/strictfp_builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/strictfp_builtins.c
@@ -0,0 +1,46 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -o - -triple x86_64-unknown-unknown | FileCheck %s
+
+// Test that the constrained intrinsics are picking up the exception
+// 

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre marked 2 inline comments as done.
thopre added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2995
+Builder.getDefaultConstrainedExcept() == fp::ebIgnore ||
+!Ty->isIEEE()) {
+  V = Builder.CreateFCmpUNO(V, V, "cmp");

I'm not too sure if that's enough to check isIEEE here. x86 extended precision 
behaves a bit different to IEEE single and double precision, esp. wrt. infinite 
and NaN. I've decided to ignore NaN values that are deprecated since the 8087.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3001
+// NaN has all exp bits set and a non zero significand. Therefore:
+// isnan(V) == ((abs(V) & exp mask) - exp mask < 0)
+unsigned bitsize = Ty->getScalarSizeInBits();

mibintc wrote:
> Are you using a reference (e.g. existing implementation) for this rewrite, or 
> is this invention? If a reference can you please tell me what it is.  The 
> expression you have written here doesn't match the codegen below. I don't see 
> the comparison to zero. Can you provide full parenthesization--the compare to 
> zero is lower than subtract?
I've noticed glibc isnan implementation does not trigger a trap so I've looked 
at the assembly it generates for the float case. This is the generalized 
version for double and long double as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre marked 2 inline comments as done.
thopre added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2988
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
-// FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
 Value *V = EmitScalarExpr(E->getArg(0));

mibintc wrote:
> What about all the other FIXME related to strictfp, are we going to pick them 
> off piecemeal? It would be nice to have a holistic approach, it would be more 
> clear, and less duplicated code. For example, presumably the test at line 
> 2991 is common.
I do want to do isinf and other similar builtin but I would prefer to have a 
review of this to see if there's anything wrong with my approach before I work 
on other builtins. Once I do I'll factor things out as needed.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3010
+Value *Sub =
+Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));

mibintc wrote:
> compared to the comment above at line 3001, lhs and rhs are swapped in the sub
My bad, the comment needs to be fixed.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3011
+Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
+if (bitsize > 32)

mibintc wrote:
> the comment at line 3001 doesn't show the lshr or the compare to zero
This logical right shift will move the sign bit to bit0 and set all other bits 
to 0. The result will be 1 (true) if sign bit is set (sub is negative) or 0 
(false) otherwise. That matches the comment. Would you like me to add a comment 
here to make it more explicit?



Comment at: clang/test/CodeGen/X86/strictfp_builtins.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -o - -triple 
x86_64-unknown-unknown | FileCheck %s

mibintc wrote:
> Why did you put long double into this new test case instead of putting it 
> with the float and double in strictfp_builtins?
long double is implemented differently for each target. x86 uses an 80 bit 
extended precision format, AFAIK AArch64 uses a 128bit IEEE format, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Melanie Blower via Phabricator via cfe-commits]

mibintc added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2988
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
-// FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
 Value *V = EmitScalarExpr(E->getArg(0));

What about all the other FIXME related to strictfp, are we going to pick them 
off piecemeal? It would be nice to have a holistic approach, it would be more 
clear, and less duplicated code. For example, presumably the test at line 2991 
is common.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3001
+// NaN has all exp bits set and a non zero significand. Therefore:
+// isnan(V) == ((abs(V) & exp mask) - exp mask < 0)
+unsigned bitsize = Ty->getScalarSizeInBits();

Are you using a reference (e.g. existing implementation) for this rewrite, or 
is this invention? If a reference can you please tell me what it is.  The 
expression you have written here doesn't match the codegen below. I don't see 
the comparison to zero. Can you provide full parenthesization--the compare to 
zero is lower than subtract?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3010
+Value *Sub =
+Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));

compared to the comment above at line 3001, lhs and rhs are swapped in the sub



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3011
+Builder.CreateSub(llvm::ConstantInt::get(IntTy, ExpMask), AbsV);
+V = Builder.CreateLShr(Sub, llvm::ConstantInt::get(IntTy, bitsize - 1));
+if (bitsize > 32)

the comment at line 3001 doesn't show the lshr or the compare to zero



Comment at: clang/test/CodeGen/X86/strictfp_builtins.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -emit-llvm -ffp-exception-behavior=maytrap -o - -triple 
x86_64-unknown-unknown | FileCheck %s

Why did you put long double into this new test case instead of putting it with 
the float and double in strictfp_builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95948

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

[PATCH] D95948: Stop traping on sNaN in __builtin_isnan

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre created this revision.
thopre added reviewers: efriedma, kpn, mibintc, sepavloff, rjmccall.
Herald added subscribers: dexonsmith, pengfei.
thopre requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.

__builtin_isnan currently generates a floating-point compare operation
which triggers a trap when faced with a signaling NaN in StrictFP mode.
This commit uses integer operations instead to not generate any trap in
such a case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95948

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/X86/strictfp_builtins.c
  clang/test/CodeGen/strictfp_builtins.c
  clang/test/CodeGen/strictfp_fpclassify.c
  llvm/include/llvm/ADT/APFloat.h
  llvm/include/llvm/IR/Type.h

Index: llvm/include/llvm/IR/Type.h
===
--- llvm/include/llvm/IR/Type.h
+++ llvm/include/llvm/IR/Type.h
@@ -308,6 +308,10 @@
   /// ppc long double), this method returns -1.
   int getFPMantissaWidth() const;
 
+  /// Return whether the type is IEEE compatible, as defined by the eponymous
+  /// method in APFloat.
+  bool isIEEE() const { return APFloat::getZero(getFltSemantics()).isIEEE(); }
+
   /// If this is a vector type, return the element type, otherwise return
   /// 'this'.
   inline Type *getScalarType() const {
Index: llvm/include/llvm/ADT/APFloat.h
===
--- llvm/include/llvm/ADT/APFloat.h
+++ llvm/include/llvm/ADT/APFloat.h
@@ -1218,6 +1218,7 @@
   bool isSmallest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isSmallest()); }
   bool isLargest() const { APFLOAT_DISPATCH_ON_SEMANTICS(isLargest()); }
   bool isInteger() const { APFLOAT_DISPATCH_ON_SEMANTICS(isInteger()); }
+  bool isIEEE() const { return usesLayout(getSemantics()); }
 
   APFloat =(const APFloat ) = default;
   APFloat =(APFloat &) = default;
Index: clang/test/CodeGen/strictfp_builtins.c
===
--- clang/test/CodeGen/strictfp_builtins.c
+++ clang/test/CodeGen/strictfp_builtins.c
@@ -92,17 +92,38 @@
   return;
 }
 
-// CHECK-LABEL: @test_isnan(
+// CHECK-LABEL: @test_float_isnan(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[F_ADDR:%.*]] = alloca float, align 4
+// CHECK-NEXT:store float [[F:%.*]], float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load float, float* [[F_ADDR]], align 4
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast float [[TMP0]] to i32
+// CHECK-NEXT:[[ABS:%.*]] = and i32 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i32 [[#%u,0x7F80]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i32 [[TMP1]], 31
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[ISNAN]]) [[ATTR4]]
+// CHECK-NEXT:ret void
+//
+void test_float_isnan(float f) {
+  P(isnan, (f));
+
+  return;
+}
+
+// CHECK-LABEL: @test_double_isnan(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[D_ADDR:%.*]] = alloca double, align 8
 // CHECK-NEXT:store double [[D:%.*]], double* [[D_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, double* [[D_ADDR]], align 8
-// CHECK-NEXT:[[CMP:%.*]] = call i1 @llvm.experimental.constrained.fcmp.f64(double [[TMP0]], double [[TMP0]], metadata !"uno", metadata !"fpexcept.strict") [[ATTR4]]
-// CHECK-NEXT:[[TMP1:%.*]] = zext i1 [[CMP]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.4, i64 0, i64 0), i32 [[TMP1]]) [[ATTR4]]
+// CHECK-NEXT:[[BITCAST:%.*]] = bitcast double [[TMP0]] to i64
+// CHECK-NEXT:[[ABS:%.*]] = and i64 [[BITCAST]], [[#%u,0x7FFF]]
+// CHECK-NEXT:[[TMP1:%.*]] = sub i64 [[#%u,0x7FF0]], [[ABS]]
+// CHECK-NEXT:[[ISNAN:%.*]] = lshr i64 [[TMP1]], 63
+// CHECK-NEXT:[[RES:%.*]] = trunc i64 [[ISNAN]] to i32
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([9 x i8], [9 x i8]* @.str.5, i64 0, i64 0), i32 [[RES]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
-void test_isnan(double d) {
+void test_double_isnan(double d) {
   P(isnan, (d));
 
   return;
@@ -120,7 +141,7 @@
 // CHECK-NEXT:[[AND:%.*]] = and i1 [[ISEQ]], [[ISINF]]
 // CHECK-NEXT:[[AND1:%.*]] = and i1 [[AND]], [[ISNORMAL]]
 // CHECK-NEXT:[[TMP2:%.*]] = zext i1 [[AND1]] to i32
-// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.5, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
+// CHECK-NEXT:call void @p(i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.6, i64 0, i64 0), i32 [[TMP2]]) [[ATTR4]]
 // CHECK-NEXT:ret void
 //
 void test_isnormal(double d) {
Index: clang/test/CodeGen/X86/strictfp_builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/strictfp_builtins.c
@@ -0,0 +1,46 @@
+// NOTE: Assertions have been autogenerated by