[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-15 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

In D71467#1820589 , @rjmccall wrote:

> I think I have a slight preference for the second option, where there's a 
> single method that does all the work for the two cases.


OK, now checked in as 870137d 
 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-14 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

> Well, just like for all the other FP builder methods, you can use the 
> setIsFPConstrained method on the builder object to switch between strict and 
> non-strict mode. Does this not suffice, or is there anything particular about 
> the comparisons that would require anything extra?

Ah, sorry, I forgot that `IsFPConstrained` is about whether we emit the 
intrinsics at all and not whether the intrinsics are currently recording real 
constraints.

I think I have a slight preference for the second option, where there's a 
single method that does all the work for the two cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-14 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

In D71467#1817939 , @rjmccall wrote:

> Is this approach going to work with scope-local strictness?  We need a way to 
> do a comparison that has the non-strict properties but appears in a function 
> that enables strictness elsewhere.


Well, just like for all the other FP builder methods, you can use the 
setIsFPConstrained method on the builder object to switch between strict and 
non-strict mode.   Does this not suffice, or is there anything particular about 
the comparisons that would require anything extra?

> Please document the difference between these two methods.

OK, checked in header file comments as 6aca3e8 
.

> Can you make a helper method for the common code in the non-constrained paths 
> here?

Would you prefer something like

  private:
Value *CreateFCmpHelper(CmpInst::Predicate P, Value *LHS, Value *RHS,
const Twine &Name, MDNode *FPMathTag) {
  if (auto *LC = dyn_cast(LHS))
if (auto *RC = dyn_cast(RHS))
  return Insert(Folder.CreateFCmp(P, LC, RC), Name);
  return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), 
Name);
}
  
  public:
Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
  const Twine &Name = "", MDNode *FPMathTag = nullptr) {
  if (IsFPConstrained)
return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
  P, LHS, RHS, Name);
  
  return CreateFCmpHelper(P, LHS, RHS, Name, FPMathTag);
}
[...]

or rather something like:

  private:
Value *CreateFCmpHelper(CmpInst::Predicate P, Value *LHS, Value *RHS,
bool IsSignaling, const Twine &Name, MDNode 
*FPMathTag) {
  if (IsFPConstrained)
return CreateConstrainedFPCmp(IsSignaling ? 
Intrinsic::experimental_constrained_fcmps
  : 
Intrinsic::experimental_constrained_fcmp,
  P, LHS, RHS, Name);
  
  if (auto *LC = dyn_cast(LHS))
if (auto *RC = dyn_cast(RHS))
  return Insert(Folder.CreateFCmp(P, LC, RC), Name);
  return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), 
Name);
}
  
  public:
Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
  const Twine &Name = "", MDNode *FPMathTag = nullptr) {
  return CreateFCmpHelper(P, LHS, RHS, false, Name, FPMathTag);
}
[...]

or maybe simply have CreateFCmpS call CreateFCmp directly in the non-strict 
case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-13 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

Is this approach going to work with scope-local strictness?  We need a way to 
do a comparison that has the non-strict properties but appears in a function 
that enables strictness elsewhere.




Comment at: llvm/include/llvm/IR/IRBuilder.h:2342
 return Insert(Folder.CreateFCmp(P, LC, RC), Name);
 return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
   }

Can you make a helper method for the common code in the non-constrained paths 
here?

Please document the difference between these two methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-10 Thread Ulrich Weigand via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG76e9c2a9870e: [FPEnv] Generate constrained FP comparisons 
from clang (authored by uweigand).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fpconstrained-cmp-double.c
  clang/test/CodeGen/fpconstrained-cmp-float.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -1195,6 +1195,18 @@
 return MetadataAsValue::get(Context, ExceptMDS);
   }
 
+  Value *getConstrainedFPPredicate(CmpInst::Predicate Predicate) {
+assert(CmpInst::isFPPredicate(Predicate) &&
+   Predicate != CmpInst::FCMP_FALSE &&
+   Predicate != CmpInst::FCMP_TRUE &&
+   "Invalid constrained FP comparison predicate!");
+
+StringRef PredicateStr = CmpInst::getPredicateName(Predicate);
+auto *PredicateMDS = MDString::get(Context, PredicateStr);
+
+return MetadataAsValue::get(Context, PredicateMDS);
+  }
+
 public:
   Value *CreateAdd(Value *LHS, Value *RHS, const Twine &Name = "",
bool HasNUW = false, bool HasNSW = false) {
@@ -2351,12 +2363,41 @@
 
   Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
 const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
+P, LHS, RHS, Name);
+
+if (auto *LC = dyn_cast(LHS))
+  if (auto *RC = dyn_cast(RHS))
+return Insert(Folder.CreateFCmp(P, LC, RC), Name);
+return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
+  }
+
+  Value *CreateFCmpS(CmpInst::Predicate P, Value *LHS, Value *RHS,
+ const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmps,
+P, LHS, RHS, Name);
+
 if (auto *LC = dyn_cast(LHS))
   if (auto *RC = dyn_cast(RHS))
 return Insert(Folder.CreateFCmp(P, LC, RC), Name);
 return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
   }
 
+  CallInst *CreateConstrainedFPCmp(
+  Intrinsic::ID ID, CmpInst::Predicate P, Value *L, Value *R,
+  const Twine &Name = "",
+  Optional Except = None) {
+Value *PredicateV = getConstrainedFPPredicate(P);
+Value *ExceptV = getConstrainedFPExcept(Except);
+
+CallInst *C = CreateIntrinsic(ID, {L->getType()},
+  {L, R, PredicateV, ExceptV}, nullptr, Name);
+setConstrainedFPCallAttr(C);
+return C;
+  }
+
   //======//
   // Instruction creation methods: Other Instructions
   //======//
Index: clang/test/CodeGen/fpconstrained-cmp-float.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained-cmp-float.c
@@ -0,0 +1,151 @@
+// RUN: %clang_cc1 -ffp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=FCMP
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=EXCEPT
+// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=MAYTRAP
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=IGNORE
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=EXCEPT
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=MAYTRAP
+
+_Bool QuietEqual(float f1, float f2) {
+  // CHECK-LABEL: define {{.*}}i1 @QuietEqual(float %f1, float %f2)
+
+  // FCMP: fcmp oeq float %{{.*}}, %{{.*}}
+  // IGNORE: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.ignore")
+  // EXCEPT: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.strict")
+  // MAYTRAP: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.maytrap")
+  return f1 == f2;
+
+  // CHECK: ret
+}
+
+_Bool QuietNotEqual(float f1, float f2) {
+  // CHECK-LABEL: define {{.*}}i1 @QuietNotEqual(float %f1, float %f2)
+
+  // FCMP: fcmp une float %{{.*}}, %{{.*}}
+  // IGNORE: call i1 @llvm.experimental.constrained.fcmp.f32

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-09 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2020-01-08 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

Ping again.


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-18 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

Ping?


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

In D71467#1786192 , @erichkeane wrote:

> __builtin_fpclassify/isfinite/isinf/isinf_sign/isnan/isnormal/signbit are all 
> implemented the same as the OTHER ones, except there is a strange fixup step 
> in SEMA that removes the float->double cast.  It is IMO the wrong way to do 
> it.
>
> I don't think it would modify the IR at all or the AST, but I'm also working 
> on removing that hack (which is what I meant by the fp-classification type 
> ones).
>
> I hope the work I've done already is sufficient to unblock this patch.


Yes, this patch is no longer blocked, thanks!

What I was trying to say is that there is a fundamental difference between the 
comparison builtins like isless, isgreater, etc. and the classification 
builtins like isinf, isnan, etc.

The former **should** result in comparison instructions being generated, the 
only difference between the builtin and a regular "<" operator is that the 
builtin emits a quiet compare while the operator emits a signaling compare in 
strict mode.

However, the latter (classification macros) should not actually emit **any** 
comparison instructions in strict mode, because the classification macros may 
never trap, but all comparison instructions do.  So the basic idea of 
implementing e.g. isinf(x) as "fabs(x) == infinity"  (like the comment in 
CGBuiltin.cpp currently says) is fundamentally wrong in strict mode.


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

In D71467#1786188 , @uweigand wrote:

> In D71467#1785943 , @erichkeane 
> wrote:
>
> > I did the compare operators that didn't work right, and will do a separate 
> > patch for the fp-classification type ones: 
> > f02d6dd6c7afc08f871a623c0411f2d77ed6acf8 
> > 
>
>
> Thanks!  Now I'm getting the correct output for the float test cases as well, 
> and I've added them to the patch.
>
> As to fp-classification, I think there is an additional complication here:  
> according to IEEE and the proposed C2x standard, these builtins should 
> **never** raise any exception, not even when receiving a signaling NaN as 
> input.  Strictly speaking, this means that they cannot possibly be 
> implemented in terms of any comparison operation.
>
> Now, on SystemZ (and many other platforms, I think) there are in fact 
> specialized instructions that will implement the required semantics without 
> raising any exceptions, but there seems to be no way to represent those at 
> the LLVM IR level.  We'll probably need some extensions here (some new 
> IR-level builtins?) ...
>
> (But I'd say that problem is unrelated to this patch, so I'd prefer to 
> decouple that problem from the question of whether **this** patch is the 
> right solution for comparisons.)


__builtin_fpclassify/isfinite/isinf/isinf_sign/isnan/isnormal/signbit are all 
implemented the same as the OTHER ones, except there is a strange fixup step in 
SEMA that removes the float->double cast.  It is IMO the wrong way to do it.

I don't think it would modify the IR at all or the AST, but I'm also working on 
removing that hack (which is what I meant by the fp-classification type ones).

I hope the work I've done already is sufficient to unblock this patch.


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand added a comment.

In D71467#1785943 , @erichkeane wrote:

> I did the compare operators that didn't work right, and will do a separate 
> patch for the fp-classification type ones: 
> f02d6dd6c7afc08f871a623c0411f2d77ed6acf8 
> 


Thanks!  Now I'm getting the correct output for the float test cases as well, 
and I've added them to the patch.

As to fp-classification, I think there is an additional complication here:  
according to IEEE and the proposed C2x standard, these builtins should 
**never** raise any exception, not even when receiving a signaling NaN as 
input.  Strictly speaking, this means that they cannot possibly be implemented 
in terms of any comparison operation.

Now, on SystemZ (and many other platforms, I think) there are in fact 
specialized instructions that will implement the required semantics without 
raising any exceptions, but there seems to be no way to represent those at the 
LLVM IR level.  We'll probably need some extensions here (some new IR-level 
builtins?) ...

(But I'd say that problem is unrelated to this patch, so I'd prefer to decouple 
that problem from the question of whether **this** patch is the right solution 
for comparisons.)


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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand updated this revision to Diff 234096.
uweigand added a comment.

Added float (f32) test cases.


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

https://reviews.llvm.org/D71467

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fpconstrained-cmp-double.c
  clang/test/CodeGen/fpconstrained-cmp-float.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -1161,6 +1161,18 @@
 return MetadataAsValue::get(Context, ExceptMDS);
   }
 
+  Value *getConstrainedFPPredicate(CmpInst::Predicate Predicate) {
+assert(CmpInst::isFPPredicate(Predicate) &&
+   Predicate != CmpInst::FCMP_FALSE &&
+   Predicate != CmpInst::FCMP_TRUE &&
+   "Invalid constrained FP comparison predicate!");
+
+StringRef PredicateStr = CmpInst::getPredicateName(Predicate);
+auto *PredicateMDS = MDString::get(Context, PredicateStr);
+
+return MetadataAsValue::get(Context, PredicateMDS);
+  }
+
 public:
   Value *CreateAdd(Value *LHS, Value *RHS, const Twine &Name = "",
bool HasNUW = false, bool HasNSW = false) {
@@ -2308,12 +2320,41 @@
 
   Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
 const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
+P, LHS, RHS, Name);
+
+if (auto *LC = dyn_cast(LHS))
+  if (auto *RC = dyn_cast(RHS))
+return Insert(Folder.CreateFCmp(P, LC, RC), Name);
+return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
+  }
+
+  Value *CreateFCmpS(CmpInst::Predicate P, Value *LHS, Value *RHS,
+ const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmps,
+P, LHS, RHS, Name);
+
 if (auto *LC = dyn_cast(LHS))
   if (auto *RC = dyn_cast(RHS))
 return Insert(Folder.CreateFCmp(P, LC, RC), Name);
 return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
   }
 
+  CallInst *CreateConstrainedFPCmp(
+  Intrinsic::ID ID, CmpInst::Predicate P, Value *L, Value *R,
+  const Twine &Name = "",
+  Optional Except = None) {
+Value *PredicateV = getConstrainedFPPredicate(P);
+Value *ExceptV = getConstrainedFPExcept(Except);
+
+CallInst *C = CreateIntrinsic(ID, {L->getType()},
+  {L, R, PredicateV, ExceptV}, nullptr, Name);
+setConstrainedFPCallAttr(C);
+return C;
+  }
+
   //======//
   // Instruction creation methods: Other Instructions
   //======//
Index: clang/test/CodeGen/fpconstrained-cmp-float.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained-cmp-float.c
@@ -0,0 +1,151 @@
+// RUN: %clang_cc1 -ffp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=FCMP
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=EXCEPT
+// RUN: %clang_cc1 -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=MAYTRAP
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=IGNORE
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=EXCEPT
+// RUN: %clang_cc1 -frounding-math -ffp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK -check-prefix=MAYTRAP
+
+_Bool QuietEqual(float f1, float f2) {
+  // CHECK-LABEL: define {{.*}}i1 @QuietEqual(float %f1, float %f2)
+
+  // FCMP: fcmp oeq float %{{.*}}, %{{.*}}
+  // IGNORE: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.ignore")
+  // EXCEPT: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.strict")
+  // MAYTRAP: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"oeq", metadata !"fpexcept.maytrap")
+  return f1 == f2;
+
+  // CHECK: ret
+}
+
+_Bool QuietNotEqual(float f1, float f2) {
+  // CHECK-LABEL: define {{.*}}i1 @QuietNotEqual(float %f1, float %f2)
+
+  // FCMP: fcmp une float %{{.*}}, %{{.*}}
+  // IGNORE: call i1 @llvm.experimental.constrained.fcmp.f32(float %{{.*}}, float %{{.*}}, metadata !"une", metadata !"fpexcept.ignore")
+  // EXCEPT: call i1 @llvm.experimental.con

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-16 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

In D71467#1784398 , @erichkeane wrote:

> In D71467#1784338 , @craig.topper 
> wrote:
>
> > In D71467#1784286 , @rjmccall 
> > wrote:
> >
> > > The bug with `__builtin_isless` should be a really easy fix; the builtin 
> > > just needs to be flagged as having custom type-checking, and then we need 
> > > to make sure we do appropriate promotions on the arguments (but we 
> > > probably do).
> >
> >
> > I think I convinced @erichkeane to look at it on Monday.
>
>
> Everything but fpclassify is pretty trivial, I just needed to write a test 
> but needed to go home. I'll commit that Monday when I get to it. Fpclassify 
> will take a touch longer since the int arguments need to be dealt with, but 
> that shouldn't be more than a little work.


I did the compare operators that didn't work right, and will do a separate 
patch for the fp-classification type ones: 
f02d6dd6c7afc08f871a623c0411f2d77ed6acf8 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-13 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

In D71467#1784338 , @craig.topper 
wrote:

> In D71467#1784286 , @rjmccall wrote:
>
> > The bug with `__builtin_isless` should be a really easy fix; the builtin 
> > just needs to be flagged as having custom type-checking, and then we need 
> > to make sure we do appropriate promotions on the arguments (but we probably 
> > do).
>
>
> I think I convinced @erichkeane to look at it on Monday.


Everything but fpclassify is pretty trivial, I just needed to write a test but 
needed to go home. I'll commit that Monday when I get to it. Fpclassify will 
take a touch longer since the int arguments need to be dealt with, but that 
shouldn't be more than a little work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-13 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper added a subscriber: erichkeane.
craig.topper added a comment.

In D71467#1784286 , @rjmccall wrote:

> The bug with `__builtin_isless` should be a really easy fix; the builtin just 
> needs to be flagged as having custom type-checking, and then we need to make 
> sure we do appropriate promotions on the arguments (but we probably do).


I think I convinced @erichkeane to look at it on Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-13 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

The bug with `__builtin_isless` should be a really easy fix; the builtin just 
needs to be flagged as having custom type-checking, and then we need to make 
sure we do appropriate promotions on the arguments (but we probably do).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71467



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

2019-12-13 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand created this revision.
uweigand added reviewers: kpn, andrew.w.kaylor, craig.topper, cameron.mcinally, 
RKSimon, spatel, rjmccall, rsmith.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Update the IRBuilder to generate constrained FP comparisons in CreateFCmp when 
IsFPConstrained is true, similar to the other places in the IRBuilder.

Also, add a new CreateFCmpS to emit **signaling** FP comparisons, and use it in 
clang where comparisons are supposed to be signaling (currently, only when 
emitting code for the <, <=, >, >= operators).  Most other places are supposed 
to emit quiet comparisons, including the equality comparisons, the various 
builtins like isless, and uses of floating-point values in boolean contexts.  A 
few places that I haven't touched may need some extra thought (e.g. are 
comparisons implicitly generated to implement sanitizer checks supposed to be 
signaling?).

I've noticed two potential problems while implementing this:

- There is currently no way to add fast-math flags to a constrained FP 
comparison, since this is implemented as an intrinsic call that returns a 
boolean type, and FMF are only allowed for calls returning a floating-point 
type.  However, given the discussion around 
https://bugs.llvm.org/show_bug.cgi?id=42179, it seems that FCmp itself really 
shouldn't have any FMF either, so this is probably OK.

- Using builtins like __builtin_isless on a "float" type will implicitly 
convert the float argument to a double; apparently this is because the builtin 
is declared as having a variable argument list?   In any case, that means that 
even though a quiet comparison is generated, the semantics still isn't correct 
for quiet NaNs, as that implicit conversion will already signal an exception.  
This probably needs to be fixed, but I guess that can be done as a separate 
patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71467

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/fpconstrained-cmp.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -1161,6 +1161,18 @@
 return MetadataAsValue::get(Context, ExceptMDS);
   }
 
+  Value *getConstrainedFPPredicate(CmpInst::Predicate Predicate) {
+assert(CmpInst::isFPPredicate(Predicate) &&
+   Predicate != CmpInst::FCMP_FALSE &&
+   Predicate != CmpInst::FCMP_TRUE &&
+   "Invalid constrained FP comparison predicate!");
+
+StringRef PredicateStr = CmpInst::getPredicateName(Predicate);
+auto *PredicateMDS = MDString::get(Context, PredicateStr);
+
+return MetadataAsValue::get(Context, PredicateMDS);
+  }
+
 public:
   Value *CreateAdd(Value *LHS, Value *RHS, const Twine &Name = "",
bool HasNUW = false, bool HasNSW = false) {
@@ -2308,12 +2320,41 @@
 
   Value *CreateFCmp(CmpInst::Predicate P, Value *LHS, Value *RHS,
 const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmp,
+P, LHS, RHS, Name);
+
+if (auto *LC = dyn_cast(LHS))
+  if (auto *RC = dyn_cast(RHS))
+return Insert(Folder.CreateFCmp(P, LC, RC), Name);
+return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
+  }
+
+  Value *CreateFCmpS(CmpInst::Predicate P, Value *LHS, Value *RHS,
+ const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+if (IsFPConstrained)
+  return CreateConstrainedFPCmp(Intrinsic::experimental_constrained_fcmps,
+P, LHS, RHS, Name);
+
 if (auto *LC = dyn_cast(LHS))
   if (auto *RC = dyn_cast(RHS))
 return Insert(Folder.CreateFCmp(P, LC, RC), Name);
 return Insert(setFPAttrs(new FCmpInst(P, LHS, RHS), FPMathTag, FMF), Name);
   }
 
+  CallInst *CreateConstrainedFPCmp(
+  Intrinsic::ID ID, CmpInst::Predicate P, Value *L, Value *R,
+  const Twine &Name = "",
+  Optional Except = None) {
+Value *PredicateV = getConstrainedFPPredicate(P);
+Value *ExceptV = getConstrainedFPExcept(Except);
+
+CallInst *C = CreateIntrinsic(ID, {L->getType()},
+  {L, R, PredicateV, ExceptV}, nullptr, Name);
+setConstrainedFPCallAttr(C);
+return C;
+  }
+
   //======//
   // Instruction creation methods: Other Instructions
   //======//
Index: clang/test/CodeGen/fpconstrained-cmp.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained-cmp.c
@@ -0,0 +1,151 @@
+// RUN: %clang_cc1 -ffp-exception-behavior=ignore -emit-llvm -o - %s