[PATCH] D154991: [FPEnv][TableGen] Add strictfp attribute to constrained intrinsics by default.

[Kevin P. Neal 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 rG91f886a40d3f: [FPEnv][TableGen] Add strictfp attribute to 
constrained intrinsics by default. (authored by kpn).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154991

Files:
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/Assembler/fp-intrinsics-attr.ll
  llvm/test/Verifier/fp-intrinsics-pass.ll
  llvm/utils/TableGen/CodeGenIntrinsics.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -388,6 +388,9 @@
   if (L->hasSideEffects != R->hasSideEffects)
 return R->hasSideEffects;
 
+  if (L->isStrictFP != R->isStrictFP)
+return R->isStrictFP;
+
   // Try to order by readonly/readnone attribute.
   uint32_t LK = L->ME.toIntValue();
   uint32_t RK = R->ME.toIntValue();
@@ -522,6 +525,8 @@
   OS << "  Attribute::get(C, Attribute::Convergent),\n";
 if (Intrinsic.isSpeculatable)
   OS << "  Attribute::get(C, Attribute::Speculatable),\n";
+if (Intrinsic.isStrictFP)
+  OS << "  Attribute::get(C, Attribute::StrictFP),\n";
 
 MemoryEffects ME = Intrinsic.ME;
 // TODO: IntrHasSideEffects should affect not only readnone intrinsics.
@@ -594,7 +599,8 @@
 Intrinsic.isNoReturn || Intrinsic.isNoCallback || Intrinsic.isNoSync ||
 Intrinsic.isNoFree || Intrinsic.isWillReturn || Intrinsic.isCold ||
 Intrinsic.isNoDuplicate || Intrinsic.isNoMerge ||
-Intrinsic.isConvergent || Intrinsic.isSpeculatable) {
+Intrinsic.isConvergent || Intrinsic.isSpeculatable ||
+Intrinsic.isStrictFP) {
   unsigned ID = UniqFnAttributes.find()->second;
   OS << "  AS[" << numAttrs++ << "] = {AttributeList::FunctionIndex, "
  << "getIntrinsicFnAttributeSet(C, " << ID << ")};\n";
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -103,6 +103,9 @@
   // True if the intrinsic is marked as speculatable.
   bool isSpeculatable;
 
+  // True if the intrinsic is marked as strictfp.
+  bool isStrictFP;
+
   enum ArgAttrKind {
 NoCapture,
 NoAlias,
Index: llvm/utils/TableGen/CodeGenIntrinsics.cpp
===
--- llvm/utils/TableGen/CodeGenIntrinsics.cpp
+++ llvm/utils/TableGen/CodeGenIntrinsics.cpp
@@ -74,6 +74,7 @@
   isConvergent = false;
   isSpeculatable = false;
   hasSideEffects = false;
+  isStrictFP = false;
 
   if (DefName.size() <= 4 || DefName.substr(0, 4) != "int_")
 PrintFatalError(DefLoc,
@@ -203,6 +204,8 @@
 isSpeculatable = true;
   else if (R->getName() == "IntrHasSideEffects")
 hasSideEffects = true;
+  else if (R->getName() == "IntrStrictFP")
+isStrictFP = true;
   else if (R->isSubClassOf("NoCapture")) {
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 addArgAttribute(ArgNo, NoCapture);
Index: llvm/test/Verifier/fp-intrinsics-pass.ll
===
--- llvm/test/Verifier/fp-intrinsics-pass.ll
+++ llvm/test/Verifier/fp-intrinsics-pass.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -passes=verify -S < %s 2>&1 | FileCheck %s
 
-declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata) #0
-declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #0
+declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata)
+declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata)
 
 ; Test that the verifier accepts legal code, and that the correct attributes are
 ; attached to the FP intrinsic. The attributes are checked at the bottom.
@@ -9,35 +9,34 @@
 ; CHECK: declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #[[ATTR]]
 ; Note: FP exceptions aren't usually caught through normal unwind mechanisms,
 ;   but we may want to revisit this for asynchronous exception handling.
-define double @f1(double %a, double %b) #0 {
+define double @f1(double %a, double %b) strictfp {
 ; CHECK-LABEL: define double @f1
-; CHECK-SAME: (double [[A:%.*]], double [[B:%.*]]) #[[ATTR1:[0-9]+]] {
+; CHECK-SAME: (double [[A:%.*]], double [[B:%.*]]) #[[STRICTFP:[0-9]+]] {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[FADD:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[A]], double [[B]], metadata !"round.dynamic", metadata !"fpexcept.strict") #[[ATTR1]]
+; CHECK-NEXT:[[FADD:%.*]] = call 

[PATCH] D154991: [FPEnv][TableGen] Add strictfp attribute to constrained intrinsics by default.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1102
 
-let IntrProperties = [IntrInaccessibleMemOnly, IntrWillReturn] in {
+/// IntrStrictFP - The intrinsic is allowed to be used in an alternate
+/// floating point environment.

Eventually I think it would be better if we had some sort of fpenv access 
property, such that we'll know which set of target intrinsics need handling and 
which aspects of the fpenv


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

https://reviews.llvm.org/D154991

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

[PATCH] D154991: [FPEnv][TableGen] Add strictfp attribute to constrained intrinsics by default.

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

kpn updated this revision to Diff 539204.
kpn added a comment.

Move test and change to round-tripping as requested.


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

https://reviews.llvm.org/D154991

Files:
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/Assembler/fp-intrinsics-attr.ll
  llvm/test/Verifier/fp-intrinsics-pass.ll
  llvm/utils/TableGen/CodeGenIntrinsics.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -388,6 +388,9 @@
   if (L->hasSideEffects != R->hasSideEffects)
 return R->hasSideEffects;
 
+  if (L->isStrictFP != R->isStrictFP)
+return R->isStrictFP;
+
   // Try to order by readonly/readnone attribute.
   uint32_t LK = L->ME.toIntValue();
   uint32_t RK = R->ME.toIntValue();
@@ -522,6 +525,8 @@
   OS << "  Attribute::get(C, Attribute::Convergent),\n";
 if (Intrinsic.isSpeculatable)
   OS << "  Attribute::get(C, Attribute::Speculatable),\n";
+if (Intrinsic.isStrictFP)
+  OS << "  Attribute::get(C, Attribute::StrictFP),\n";
 
 MemoryEffects ME = Intrinsic.ME;
 // TODO: IntrHasSideEffects should affect not only readnone intrinsics.
@@ -594,7 +599,8 @@
 Intrinsic.isNoReturn || Intrinsic.isNoCallback || Intrinsic.isNoSync ||
 Intrinsic.isNoFree || Intrinsic.isWillReturn || Intrinsic.isCold ||
 Intrinsic.isNoDuplicate || Intrinsic.isNoMerge ||
-Intrinsic.isConvergent || Intrinsic.isSpeculatable) {
+Intrinsic.isConvergent || Intrinsic.isSpeculatable ||
+Intrinsic.isStrictFP) {
   unsigned ID = UniqFnAttributes.find()->second;
   OS << "  AS[" << numAttrs++ << "] = {AttributeList::FunctionIndex, "
  << "getIntrinsicFnAttributeSet(C, " << ID << ")};\n";
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -103,6 +103,9 @@
   // True if the intrinsic is marked as speculatable.
   bool isSpeculatable;
 
+  // True if the intrinsic is marked as strictfp.
+  bool isStrictFP;
+
   enum ArgAttrKind {
 NoCapture,
 NoAlias,
Index: llvm/utils/TableGen/CodeGenIntrinsics.cpp
===
--- llvm/utils/TableGen/CodeGenIntrinsics.cpp
+++ llvm/utils/TableGen/CodeGenIntrinsics.cpp
@@ -74,6 +74,7 @@
   isConvergent = false;
   isSpeculatable = false;
   hasSideEffects = false;
+  isStrictFP = false;
 
   if (DefName.size() <= 4 || DefName.substr(0, 4) != "int_")
 PrintFatalError(DefLoc,
@@ -203,6 +204,8 @@
 isSpeculatable = true;
   else if (R->getName() == "IntrHasSideEffects")
 hasSideEffects = true;
+  else if (R->getName() == "IntrStrictFP")
+isStrictFP = true;
   else if (R->isSubClassOf("NoCapture")) {
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 addArgAttribute(ArgNo, NoCapture);
Index: llvm/test/Verifier/fp-intrinsics-pass.ll
===
--- llvm/test/Verifier/fp-intrinsics-pass.ll
+++ llvm/test/Verifier/fp-intrinsics-pass.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -passes=verify -S < %s 2>&1 | FileCheck %s
 
-declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata) #0
-declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #0
+declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata)
+declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata)
 
 ; Test that the verifier accepts legal code, and that the correct attributes are
 ; attached to the FP intrinsic. The attributes are checked at the bottom.
@@ -9,35 +9,34 @@
 ; CHECK: declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #[[ATTR]]
 ; Note: FP exceptions aren't usually caught through normal unwind mechanisms,
 ;   but we may want to revisit this for asynchronous exception handling.
-define double @f1(double %a, double %b) #0 {
+define double @f1(double %a, double %b) strictfp {
 ; CHECK-LABEL: define double @f1
-; CHECK-SAME: (double [[A:%.*]], double [[B:%.*]]) #[[ATTR1:[0-9]+]] {
+; CHECK-SAME: (double [[A:%.*]], double [[B:%.*]]) #[[STRICTFP:[0-9]+]] {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[FADD:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[A]], double [[B]], metadata !"round.dynamic", metadata !"fpexcept.strict") #[[ATTR1]]
+; CHECK-NEXT:[[FADD:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[A]], double [[B]], metadata !"round.dynamic", metadata !"fpexcept.strict")
 ; CHECK-NEXT:ret double [[FADD]]
 entry:
   %fadd 

[PATCH] D154991: [FPEnv][TableGen] Add strictfp attribute to constrained intrinsics by default.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/test/Feature/fp-intrinsics-attr.ll:1
+; RUN: opt -passes=verify -S < %s | FileCheck %s
+

Should move to test/Assembler and round trip through llvm-as and llvm-dis like 
other similar tests 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154991

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

[PATCH] D154991: [FPEnv][TableGen] Add strictfp attribute to constrained intrinsics by default.

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

kpn created this revision.
kpn added reviewers: pengfei, chapuni, akshaykhadse, craig.topper, arsenm.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
kpn requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wdng.
Herald added projects: clang, LLVM.

In D146869  @arsenm pointed out that the 
constrained intrinsics aren't getting the strictfp attribute by default. They 
should be, since they are required to have it anyway.

TableGen did not know about this attribute until now. This patch adds strictfp 
to TableGen, and it uses it on all of the constrained intrinsics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154991

Files:
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/Feature/fp-intrinsics-attr.ll
  llvm/test/Verifier/fp-intrinsics-pass.ll
  llvm/utils/TableGen/CodeGenIntrinsics.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -388,6 +388,9 @@
   if (L->hasSideEffects != R->hasSideEffects)
 return R->hasSideEffects;
 
+  if (L->isStrictFP != R->isStrictFP)
+return R->isStrictFP;
+
   // Try to order by readonly/readnone attribute.
   uint32_t LK = L->ME.toIntValue();
   uint32_t RK = R->ME.toIntValue();
@@ -522,6 +525,8 @@
   OS << "  Attribute::get(C, Attribute::Convergent),\n";
 if (Intrinsic.isSpeculatable)
   OS << "  Attribute::get(C, Attribute::Speculatable),\n";
+if (Intrinsic.isStrictFP)
+  OS << "  Attribute::get(C, Attribute::StrictFP),\n";
 
 MemoryEffects ME = Intrinsic.ME;
 // TODO: IntrHasSideEffects should affect not only readnone intrinsics.
@@ -594,7 +599,8 @@
 Intrinsic.isNoReturn || Intrinsic.isNoCallback || Intrinsic.isNoSync ||
 Intrinsic.isNoFree || Intrinsic.isWillReturn || Intrinsic.isCold ||
 Intrinsic.isNoDuplicate || Intrinsic.isNoMerge ||
-Intrinsic.isConvergent || Intrinsic.isSpeculatable) {
+Intrinsic.isConvergent || Intrinsic.isSpeculatable ||
+Intrinsic.isStrictFP) {
   unsigned ID = UniqFnAttributes.find()->second;
   OS << "  AS[" << numAttrs++ << "] = {AttributeList::FunctionIndex, "
  << "getIntrinsicFnAttributeSet(C, " << ID << ")};\n";
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -103,6 +103,9 @@
   // True if the intrinsic is marked as speculatable.
   bool isSpeculatable;
 
+  // True if the intrinsic is marked as strictfp.
+  bool isStrictFP;
+
   enum ArgAttrKind {
 NoCapture,
 NoAlias,
Index: llvm/utils/TableGen/CodeGenIntrinsics.cpp
===
--- llvm/utils/TableGen/CodeGenIntrinsics.cpp
+++ llvm/utils/TableGen/CodeGenIntrinsics.cpp
@@ -74,6 +74,7 @@
   isConvergent = false;
   isSpeculatable = false;
   hasSideEffects = false;
+  isStrictFP = false;
 
   if (DefName.size() <= 4 || DefName.substr(0, 4) != "int_")
 PrintFatalError(DefLoc,
@@ -203,6 +204,8 @@
 isSpeculatable = true;
   else if (R->getName() == "IntrHasSideEffects")
 hasSideEffects = true;
+  else if (R->getName() == "IntrStrictFP")
+isStrictFP = true;
   else if (R->isSubClassOf("NoCapture")) {
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 addArgAttribute(ArgNo, NoCapture);
Index: llvm/test/Verifier/fp-intrinsics-pass.ll
===
--- llvm/test/Verifier/fp-intrinsics-pass.ll
+++ llvm/test/Verifier/fp-intrinsics-pass.ll
@@ -1,7 +1,7 @@
 ; RUN: opt -passes=verify -S < %s 2>&1 | FileCheck %s
 
-declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata) #0
-declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #0
+declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata)
+declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata)
 
 ; Test that the verifier accepts legal code, and that the correct attributes are
 ; attached to the FP intrinsic. The attributes are checked at the bottom.
@@ -9,35 +9,34 @@
 ; CHECK: declare double @llvm.experimental.constrained.sqrt.f64(double, metadata, metadata) #[[ATTR]]
 ; Note: FP exceptions aren't usually caught through normal unwind mechanisms,
 ;   but we may want to revisit this for asynchronous exception handling.
-define double @f1(double %a, double %b) #0 {
+define double @f1(double %a, double %b) strictfp {
 ; CHECK-LABEL: define double @f1
-; CHECK-SAME: (double [[A:%.*]], double [[B:%.*]])