[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
jcranmer-intel added a reviewer: dcoughlin.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch improves Clang call graph analysis by adding in expressions that are 
not found in regular function bodies, such as default arguments or C++ member 
initializers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65453

Files:
  clang/lib/Analysis/CallGraph.cpp
  clang/test/Analysis/cxx-callgraph.cpp


Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | 
FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -103,6 +103,16 @@
 VisitChildren(E);
   }
 
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -167,13 +177,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {


Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -103,6 +103,16 @@
 VisitChildren(E);
   }
 
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -167,13 +177,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

No, I do not have commit access, so if you could commit it, it would be greatly 
appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65453



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

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

The test has been passing for me. What error are you seeing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65453



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

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Okay, I see the issue now. I originally developed this patch on a fork with a 
whole lot of extra changes, and that fork included some extra modifications to 
the callgraph that I had missed: 
https://github.com/intel/llvm/commit/971fecdc316930c0c1c79283d1094ee4c4ca41e0#diff-cae4e2b4043cd0f49ce29e77de22a5a5.
 I'll merge the callgraph-related changes in that patch back onto a clean patch 
for upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65453



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

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 215462.
jcranmer-intel added a comment.

I've rolled the relevant call graph analysis changes from the prior commit into 
this updated patch.


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

https://reviews.llvm.org/D65453

Files:
  clang/include/clang/Analysis/CallGraph.h
  clang/lib/Analysis/CallGraph.cpp
  clang/test/Analysis/cxx-callgraph.cpp

Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -79,6 +79,40 @@
 VisitChildren(CE);
   }
 
+  void VisitLambdaExpr(LambdaExpr *LE) {
+if (CXXMethodDecl *MD = LE->getCallOperator())
+  G->VisitFunctionDecl(MD);
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (FunctionDecl *FD = E->getOperatorNew())
+  addCalledDecl(FD);
+VisitChildren(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+CXXConstructorDecl *Ctor = E->getConstructor();
+if (FunctionDecl *Def = Ctor->getDefinition())
+  addCalledDecl(Def);
+const auto *ConstructedType = Ctor->getParent();
+if (ConstructedType->hasUserDeclaredDestructor()) {
+  CXXDestructorDecl *Dtor = ConstructedType->getDestructor();
+  if (FunctionDecl *Def = Dtor->getDefinition())
+addCalledDecl(Def);
+}
+VisitChildren(E);
+  }
+
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -143,13 +177,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
Index: clang/include/clang/Analysis/CallGraph.h
===
--- clang/include/clang/Analysis/CallGraph.h
+++ clang/include/clang/Analysis/CallGraph.h
@@ -131,6 +131,7 @@
 
   bool shouldWalkTypesOfTypeLocs() const { return false; }
   bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
 
 private:
   /// Add the given declaration to the call graph.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65453: [analyzer] Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 215999.
jcranmer-intel added a comment.

I think there are use cases for having a callgraph that errs on the side of 
adding edges that might not exist, but I'm happy enough to leave that for a 
later patch.

This does remind me that we need a real clang IR that we can leverage for this 
sort of thing.


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

https://reviews.llvm.org/D65453

Files:
  clang/include/clang/Analysis/CallGraph.h
  clang/lib/Analysis/CallGraph.cpp
  clang/test/Analysis/cxx-callgraph.cpp

Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -79,6 +79,34 @@
 VisitChildren(CE);
   }
 
+  void VisitLambdaExpr(LambdaExpr *LE) {
+if (CXXMethodDecl *MD = LE->getCallOperator())
+  G->VisitFunctionDecl(MD);
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (FunctionDecl *FD = E->getOperatorNew())
+  addCalledDecl(FD);
+VisitChildren(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+CXXConstructorDecl *Ctor = E->getConstructor();
+if (FunctionDecl *Def = Ctor->getDefinition())
+  addCalledDecl(Def);
+VisitChildren(E);
+  }
+
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -143,13 +171,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
Index: clang/include/clang/Analysis/CallGraph.h
===
--- clang/include/clang/Analysis/CallGraph.h
+++ clang/include/clang/Analysis/CallGraph.h
@@ -131,6 +131,7 @@
 
   bool shouldWalkTypesOfTypeLocs() const { return false; }
   bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
 
 private:
   /// Add the given declaration to the call graph.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132056: [HLSL] Restrict to supported targets

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel accepted this revision.
jcranmer-intel added a comment.
This revision is now accepted and ready to land.

I don't necessarily know clang-specific code to have that valuable an opinion 
here, but I don't see anything wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132056

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Some general comments on the documentation here:

One of the lesser-known issues of `-ffast-math` is the fact that it (or 
`-funsafe-math-optimizations`) causes `crtfastmath.o` to be linked, which adds 
a static constructor that sets the FTZ/DAZ bits in MXCSR, affecting not only 
the current compilation unit but all static and shared libraries included in 
the program. This behavior deserves to be mentioned somewhere in the 
documentation. Additionally, the effects of fast-math/unsafe-math-optimizations 
on fdenormal-fp-math{32} should be noted. I realize this is orthogonal to the 
actual change you're doing here, but while we're improving the documentation of 
`-f[no-]fast-math`, we should probably fix this missing documentation at the 
same time.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2778
+  StringRef LastSeenFfpContractOption;
+  StringRef LastSeenFfastMathOption;
   if (!JA.isDeviceOffloading(Action::OFK_Cuda) &&

Given that this will only be set to "" or "fast", it probably makes sense to 
make for this be a `bool SeenFfastMathOption` variable instead.


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

https://reviews.llvm.org/D123630

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/docs/UsersManual.rst:1430
+
+   * ``-ffp-contract=on``
+

You can add `-fdenormal-fp-math=ieee` here.



Comment at: clang/docs/UsersManual.rst:1453-1455
+   Note: ``DenormalFPMath`` and ``DenormalFP32Math`` are set by default to IEEE
+   (no flush) for ``-fno-fast-math``, ``-fno-unsafe-math-optimizations``, and
+   any setting of ``fp-model``. Clang does enable flush-to-zero when

You can replace this text with saying that `-fno-fast-math` implies 
`-fdenormal-fp-math=ieee`. No need to directly mention `DenormalFPMath`; 
instead relate it to the other command line flags that are documented.



Comment at: clang/docs/UsersManual.rst:1455-1458
+   any setting of ``fp-model``. Clang does enable flush-to-zero when
+   ``-fast=math`` or ``-funsafe-math-optimzations`` are used, when it is able 
to
+   find the ``cftfastmath.o``. This will affect not only the current 
compilation
+   but all static and shared libraries included in the program.

`crtfastmath.o` should probably be mentioned in a separate section, like the "A 
note about ..." sections, with the text in `-fno-fast-math` only mentioning 
that it causes code not to be linked with `crtfastmath.o`.


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

https://reviews.llvm.org/D123630

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel accepted this revision.
jcranmer-intel added a comment.
This revision is now accepted and ready to land.

I'm happy with these changes. I'll let Aaron have one last crack at the wording 
of the documentation, in case there's any minor editorial stuff he'd like to 
see cleaned up.


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

https://reviews.llvm.org/D123630

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

So while trying to review this patch, I've discovered there's an annoying 
incompatibility between C and C++ here, in that C and C++ specify different 
rules on how to choose between `_Float64`, `double`, and `long double` if all 
are `binary64` (C says `_Float64` > `long double` > `double`, C++ says `long 
double` > `_Float64` > `double`), and I don't have enough knowledge of clang 
internal implementation to know if the changes being done can capture the 
different semantics there. (It's also not entirely clear to me that the 
incompatibility between C and C++ was intentional in the first place; it looks 
like the paper author intentionally chose the ordering for C++ and argued for 
the change to C, but the CFP working group soundly rejected the change, and 
personally I agree with the CFP decision here over C++).




Comment at: clang/lib/AST/ASTContext.cpp:136-138
+// another floating point type.
+constexpr std::array, 5>
+CXX23FloatingPointConversionRankMap = {

I don't like having a large table here of results. It just doesn't scale; if 
you take into account all of the putative types that might be supported, you've 
got 3 basic types + 4 interchange formats + 3 decimal types + 1 IEEE extended 
type + bfloat + 3 IBM hex floats, and that's just things already supported by 
LLVM or that I know someone's working on.

I think after special casing float/double/long double, you can handle all the 
other types by simply using a mixture of `fltSemantics::isRepresentableBy` and 
a subrank preference list.

In the event that support for `_Float32` and `_Float64` are added, this table 
will no longer be target-independent. We have a few targets where `double` is 
binary32 and several targets where `long double` is either binary32 or binary64 
(and others where it's neither). I think it's better to start writing this 
method in a way that can handle this mapping be target-dependent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

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

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Apologies for the drive-by comment, but I happened to be searching for TBAA 
reviews after lamenting the current documentation and this popped up.

> Agreed, strict aliasing violations are already a problem with the current 
> level of TBAA support and we regularly see mis-compiles when new 
> optimizations get enabled due to violations in projects. Improving precision 
> of TBAA annotation is likely to expose more violations But there are cases 
> where the additional guarantees could make a difference and other compiler 
> implementations make use of them, so I think it would be worthwhile to work 
> towards improving TBAA. I think we've also been through other disruptive 
> transitions, like more aggressively adding `mustprogress`, but those were 
> probably on a somewhat smaller scale.
>
> Maybe we should try improve our tooling to detect violations in parallel to 
> the effort in this patch? I am planning on trying to resurrect the 
> type-sanitizer patches. I've also been playing around with a simple tool 
> that's inserting assertions to check 2 pointers are not equal if TBAA claims 
> they are no alias. The latter seems to surface a few violations in code in 
> `llvm-test-suite` and also in tablegen. That is even without the current 
> patch applied.

I think better tooling for TBAA in general is needed, although I'm not sure if 
this is a "in parallel" or "as a prerequisite step." One of the things I 
started doing myself is building a DOT output for TBAA metadata so that it's 
easier to understand the TBAA type tree without having to stare too hard at all 
of the metadata numbers. I don't want to derail this patch with too much 
discussion, but since it's slightly relevant here, one more comment:




Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:200-203
+  StringRef Name =
+  cast(
+  ScalarMD->getOperand(CodeGenOpts.NewStructPathTBAA ? 2 : 0))
+  ->getString();

Seeing this code reminds me that in a few other contexts, it would be useful to 
have access to the TBAA wrapper helpers in 
`llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp`, so it would be worthwhile at 
some point to start fronting those types into header files to simplify some of 
the queries, especially in cases where you want to support both old and new 
TBAA formats. I'd imagine your tbaa-checker tool would also want access to this 
information as well, for example; I know the helper I started writing wanted it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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

[PATCH] D119289: [Clang] Add lowering for _C complex arithmetic to complex intrinsics.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
Herald added subscribers: dexonsmith, dang, pengfei.
jcranmer-intel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This adds a new guard flag, -fuse-complex-intrinsics, that is defaulted to on
for the x86/x86-64 platforms, as they can be lowered to complex intrinsics
through the entire pipeline correctly. Other platforms could be added, but it
requires auditing their support correctly in the backend, and I don't have as
much familiarity with their ABIs as I do for x86/x86-64. Help would be
appreciated.

Future patches will build on top of this support to enable more complex limited
range support, but only when lowering to intrinsics.

Depends on D119288 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119289

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/complex-intrinsics.c

Index: clang/test/CodeGen/complex-intrinsics.c
===
--- /dev/null
+++ clang/test/CodeGen/complex-intrinsics.c
@@ -0,0 +1,188 @@
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-pc-win64 -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple i686-unknown-unknown -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple spir -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -fuse-complex-intrinsics -o - | FileCheck %s --check-prefix=INTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-pc-win64 -fuse-complex-intrinsics -o - | FileCheck %s --check-prefix=INTRIN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -fuse-complex-intrinsics -DT=int -o - | FileCheck %s --check-prefix=INT
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -DT=int -o - | FileCheck %s --check-prefix=INT
+
+// Check defaults for intrinsics:
+// RUN: %clang %s -O0 -S -emit-llvm -target x86_64-unknown-unknown -o - | FileCheck %s --check-prefix=INTRIN
+// RUN: %clang %s -O0 -S -emit-llvm -target x86_64-unknown-unknown -fuse-complex-intrinsics -o - | FileCheck %s --check-prefix=INTRIN
+// RUN: %clang %s -O0 -S -emit-llvm -target x86_64-unknown-unknown -fno-use-complex-intrinsics -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang %s -O0 -S -emit-llvm -target aarch64-unknown-unknown -o - | FileCheck %s --check-prefix=NOINTRIN
+// RUN: %clang %s -O0 -S -emit-llvm -target aarch64-unknown-unknown -fuse-complex-intrinsics -o - | FileCheck %s --check-prefix=INTRIN
+// RUN: %clang %s -O0 -S -emit-llvm -target aarch64-unknown-unknown -fno-use-complex-intrinsics -o - | FileCheck %s --check-prefix=NOINTRIN
+
+#ifndef T
+#  define T float
+#endif
+
+T check_var;
+// INTRIN: @check_var = {{.*}}global [[T:[a-z0-9]+]]
+// NOINTRIN: @check_var = {{.*}}global [[T:[a-z0-9]+]]
+// INT: @check_var = {{.*}}global [[T:i[0-9]+]]
+
+T _Complex add_rc(T a, T _Complex b) {
+  // INTRIN-LABEL: @add_rc(
+  // INTRIN-COUNT-1: fadd [[T]]
+  // INTRIN: ret
+  // NOINTRIN-LABEL: @add_rc(
+  // NOINTRIN-COUNT-1: fadd [[T]]
+  // NOINTRIN: ret
+  // INT-LABEL: @add_rc(
+  // INT-COUNT-1: add [[T]]
+  // INT: ret
+  return a + b;
+}
+
+T _Complex add_cr(T _Complex a, T b) {
+  // INTRIN-LABEL: @add_cr(
+  // INTRIN-COUNT-1: fadd [[T]]
+  // INTRIN: ret
+  // NOINTRIN-LABEL: @add_cr(
+  // NOINTRIN-COUNT-1: fadd [[T]]
+  // NOINTRIN: ret
+  // INT-LABEL: @add_cr(
+  // INT-COUNT-1: add [[T]]
+  // INT: ret
+  return a + b;
+}
+
+T _Complex add_cc(T _Complex a, T _Complex b) {
+  // INTRIN-LABEL: @add_cc(
+  // INTRIN-COUNT-2: fadd [[T]]
+  // INTRIN: ret
+  // NOINTRIN-LABEL: @add_cc(
+  // NOINTRIN-COUNT-2: fadd [[T]]
+  // NOINTRIN: ret
+  // INT-LABEL: @add_cc(
+  // INT-COUNT-2: add [[T]]
+  // INT: ret
+  return a + b;
+}
+
+T _Complex sub_rc(T a, T _Complex b) {
+  // INTRIN-LABEL: @sub_rc(
+  // INTRIN: fsub [[T]]
+  // INTRIN: fneg [[T]]
+  // INTRIN: ret
+  // NOINTRIN-LABEL: @sub_rc(
+  // NOINTRIN: fsub [[T]]
+  // NOINTRIN: fneg [[T]]
+  // NOINTRIN: ret
+  // INT-LABEL: @sub_rc(
+  // INT-COUNT-2: sub [[T]]
+  // INT: ret
+  return a - b;
+}
+
+T _Complex sub_cr(T _Complex a, T b) {
+  // INTRIN-LABEL: @sub_cr(
+  // INTRIN: fsub [[T]]
+  // INTRIN-NOT: fsub [[T]]
+  // INTRIN: ret
+  // NOINTRIN-LABEL: @sub_cr(
+  // NOINTRIN: fsub [[T]]
+  // NOINTRIN-NOT: fsub [[T]]
+  // NOINTRIN: ret
+  // INT-LABEL: @sub_cr(
+  // INT-COUNT-2: sub [[T]]
+  // INT: ret
+  return a - b;
+}
+
+T _Complex sub_cc(T _Complex a, T _Comp

[PATCH] D119290: [Clang] Add support for -fcx-limited-range, -fcx-fortran-rules options.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
Herald added a subscriber: dang.
jcranmer-intel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These are built on top of, and require, the complex intrinsics definitions to
work with. They are intended to be similar in operation to the gcc command line
options.

Depends on D119289 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119290

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/complex-range-flags.c

Index: clang/test/Driver/complex-range-flags.c
===
--- /dev/null
+++ clang/test/Driver/complex-range-flags.c
@@ -0,0 +1,21 @@
+// Test that the tri-valued complex range argument gets set appropriately.
+
+// REQUIRES: clang-driver
+
+// CHECK-FULL-RANGE: "-cc1"
+// CHECK-FULL-RANGE: "-fcx-range=full"
+
+// CHECK-LIMITED-RANGE: "-cc1"
+// CHECK-LIMITED-RANGE: "-fcx-range=limited"
+
+// CHECK-NONAN-RANGE: "-cc1"
+// CHECK-NONAN-RANGE: "-fcx-range=nonan"
+
+// RUN: %clang -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FULL-RANGE %s
+
+// RUN: %clang -### -c -fcx-limited-range %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LIMITED-RANGE %s
+
+// RUN: %clang -### -c -fcx-fortran-rules %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NONAN-RANGE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2696,6 +2696,19 @@
 // overriden by ffp-exception-behavior?
   bool RoundingFPMath = false;
   bool RoundingMathPresent = false; // Is rounding-math in args?
+  LangOptions::ComplexRangeKind CxRangeArg =
+LangOptions::CX_Full;
+  StringRef RulingRangeArg = "";
+  auto setCxRangeArg = [&CxRangeArg, &RulingRangeArg, &D, &Args](
+  LangOptions::ComplexRangeKind Kind, StringRef ArgName) {
+if (!RulingRangeArg.empty() && Kind != CxRangeArg) {
+D.Diag(clang::diag::warn_drv_overriding_flag_option)
+   << Args.MakeArgString(RulingRangeArg)
+   << Args.MakeArgString(ArgName);
+}
+RulingRangeArg = ArgName;
+CxRangeArg = Kind;
+  };
   // -ffp-model values: strict, fast, precise
   StringRef FPModel = "";
   // -ffp-exception-behavior options: strict, maytrap, ignore
@@ -2942,6 +2955,7 @@
   RoundingFPMath = false;
   // If fast-math is set then set the fp-contract mode to fast.
   FPContract = "fast";
+  CxRangeArg = LangOptions::CX_NoNan;
   break;
 case options::OPT_fno_fast_math:
   HonorINFs = true;
@@ -2966,6 +2980,19 @@
   << "-ffp-contract=on";
 }
   break;
+
+case options::OPT_fcx_limited_range:
+  setCxRangeArg(LangOptions::CX_Limited, "fcx-limited-range");
+  break;
+case options::OPT_fnocx_limited_range:
+  setCxRangeArg(LangOptions::CX_Full, "fcx-limited-range");
+  break;
+case options::OPT_fcx_fortran_rules:
+  setCxRangeArg(LangOptions::CX_NoNan, "fcx-fortran-rules");
+  break;
+case options::OPT_fnocx_fortran_rules:
+  setCxRangeArg(LangOptions::CX_Full, "fcx-fortran-rules");
+  break;
 }
 if (StrictFPModel) {
   // If -ffp-model=strict has been specified on command line but
@@ -3103,6 +3130,18 @@
DefaultUseComplexIntrinsics)) {
 CmdArgs.push_back("-fuse-complex-intrinsics");
   }
+
+  switch (CxRangeArg) {
+  case LangOptions::CX_Limited:
+CmdArgs.push_back("-fcx-range=limited");
+break;
+  case LangOptions::CX_NoNan:
+CmdArgs.push_back("-fcx-range=nonan");
+break;
+  case LangOptions::CX_Full:
+CmdArgs.push_back("-fcx-range=full");
+break;
+  }
 }
 
 static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1847,6 +1847,16 @@
   "Determines whether to lower _Complex operations to LLVM complex "
   "intrinsics">,
   NegFlag>;
+def fcx_limited_range : Flag<["-"], "fcx-limited-range">, Group;
+def fnocx_limited_range : Flag<["-"], "fnocx-limited-range">, Group;
+def fcx_fortran_rules : Flag<["-"], "fcx-fortran-rules">, Group;
+def fnocx_fortran_rules : Flag<["-"], "fnocx-fortran-rules">, Group;
+def cx_rangeEQ : Joined<["-"], "fcx-range=">, Group, Flags<[CC1Option]>,
+  HelpText<"Specifies the behavior of complex multiplication and division">,
+  Values<"limited,nonan,full">,
+  NormalizedValuesScope<"LangOptions">,
+  NormalizedValues<["CX_Limited", "CX_NoNan", "CX_Full"]>,
+  MarshallingInfoEnum, "CX_Full">;
 
 def ffor_scope : Flag<["-"], "ffor-scope">, Group;
 def fno_for_scope : Flag

[PATCH] D119291: [Clang] Add support for STDC CX_LIMITED_RANGE pragma.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
Herald added a subscriber: dexonsmith.
jcranmer-intel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This pragma is defined in the C specification. Notably, the C specification
makes the "DEFAULT" value identical to "OFF" for this pragma, unlike other
floating-point pragmas which are undefined behavior. This may be surprising to
some users.

This patch builds on, and requires, the complex intrinsics to make the pragmas
work properly.

Depends on D119290 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119291

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/cx-limited-range-pragma.c

Index: clang/test/CodeGen/cx-limited-range-pragma.c
===
--- /dev/null
+++ clang/test/CodeGen/cx-limited-range-pragma.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -fuse-complex-intrinsics -fcx-range=limited -o - | FileCheck %s --check-prefixes=CHECK-COMMON,CHECK-LIMITED
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -fuse-complex-intrinsics -fcx-range=nonan -o - | FileCheck %s --check-prefixes=CHECK-COMMON,CHECK-NO-NAN
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -fuse-complex-intrinsics -fcx-range=full -o - | FileCheck %s --check-prefixes=CHECK-COMMON,CHECK-FULL
+
+_Complex float range_limited(_Complex float a, _Complex float b) {
+// CHECK-COMMON: @range_limited
+// CHECK-COMMON: call{{.*}}complex.fmul{{.*}} #[[LIMITED:[0-9]+]]
+// CHECK-COMMON: call{{.*}}complex.fdiv{{.*}} #[[LIMITED]]
+#pragma STDC CX_LIMITED_RANGE ON
+  return a * b + a / b;
+}
+
+_Complex float range_full(_Complex float a, _Complex float b) {
+// CHECK-COMMON: @range_full
+// CHECK-COMMON: call{{.*}}complex.fmul{{.*}} #[[FULL:[0-9]+]]
+// CHECK-COMMON: call{{.*}}complex.fdiv{{.*}} #[[FULL]]
+#pragma STDC CX_LIMITED_RANGE OFF
+  return a * b + a / b;
+}
+
+_Complex float range_default(_Complex float a, _Complex float b) {
+// CHECK-LIMITED: @range_default
+// CHECK-LIMITED: call{{.*}}complex.fmul{{.*}} #[[LIMITED]]
+// CHECK-LIMITED: call{{.*}}complex.fdiv{{.*}} #[[LIMITED]]
+// CHECK-NO-NAN: @range_default
+// CHECK-NO-NAN: call{{.*}}complex.fmul{{.*}} #[[LIMITED]]
+// CHECK-NO-NAN: call{{.*}}complex.fdiv{{.*}} #[[NONAN:[0-9]+]]
+// CHECK-FULL: @range_default
+// CHECK-FULL: call{{.*}}complex.fmul{{.*}} #[[FULL]]
+// CHECK-FULL: call{{.*}}complex.fdiv{{.*}} #[[FULL]]
+  return a * b + a / b;
+}
+
+_Complex float range_scoped(_Complex float a, _Complex float b) {
+// CHECK-COMMON: @range_scoped
+// CHECK-COMMON: call{{.*}}complex.fmul{{.*}} #[[FULL]]
+// CHECK-COMMON: call{{.*}}complex.fdiv{{.*}} #[[FULL]]
+#pragma STDC CX_LIMITED_RANGE OFF
+  _Complex float res = a * b;
+  {
+#pragma STDC CX_LIMITED_RANGE DEFAULT
+res += a / b;
+  }
+  return res;
+}
+
+// CHECK-COMMON: attributes #[[LIMITED]] = { "complex-range"="limited" }
+// CHECK-COMMON: attributes #[[FULL]] = { "complex-range"="full" }
+// CHECK-NO-NAN: attributes #[[NONAN]] = { "complex-range"="no-nan" }
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1165,6 +1165,14 @@
   CurFPFeatures = NewFPFeatures.applyOverrides(LO);
 }
 
+void Sema::ActOnPragmaComplexLimitedRange(SourceLocation Loc,
+  LangOptions::ComplexRangeKind Range) {
+  FPOptionsOverride NewFPFeatures = CurFPFeatureOverrides();
+  NewFPFeatures.setComplexRangeOverride(Range);
+  FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
+  CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
+}
+
 void Sema::ActOnPragmaFPExceptions(SourceLocation Loc,
LangOptions::FPExceptionModeKind FPE) {
   setExceptionMode(Loc, FPE);
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -791,6 +791,9 @@
   case tok::annot_pragma_fenv_round:
 HandlePragmaFEnvRound();
 return nullptr;
+  case tok::annot_pragma_cx_limited_range:
+HandlePragmaComplexLimitedRange();
+return nullptr;
   case tok::annot_pragma_float_control:
 HandlePragmaFloatControl();
 return nullptr;
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -388,6 +388,12 @@
 ConsumeAnnotationToken();
 return StmtError();
 
+  case tok::annot_pragma_cx_limited_range:
+Prohibit

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D136568#3878104 , @Izaron wrote:

> The online documentation 
> (https://en.cppreference.com/w/cpp/numeric/math/ilogb) says:
>
>   1. If the correct result is greater than INT_MAX or smaller than INT_MIN, 
> FE_INVALID is raised.
>   2. If arg is ±0, ±∞, or NaN, FE_INVALID is raised.
>   3. In all other cases, the result is exact (FE_INEXACT is never raised) and 
> the current rounding mode is ignored
>
> The first point seemingly never occur, because llvm's `ilogb` return type is 
> `int`.
> The second point is handled as expected (`APFloatTest.cpp` checks it)

FWIW, I would be slightly wary of relying on cppreference as definitive for 
niche semantic issues like this, although it is correct in this case. C2x is 
actually pretty well-organized in Annex F, and enumerates most of the corner 
cases specifically in every function.




Comment at: clang/lib/AST/ExprConstant.cpp:12452
+int Ilogb;
+if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St))
+  return false;

`long double` is `ppc_fp128` on at least some PPC targets, and while I'm not 
entirely certain of what `ilogb` properly returns in the corner cases of the 
`ppc_fp128`, I'm not entirely confident that the implementation of `APFloat` is 
correct in those cases. I'd like someone with PPC background to comment in here.



Comment at: clang/test/Sema/constant-builtins-ilogb.cpp:13
+static_assert(!__builtin_constant_p(FUNC(T(Inf;\
+static_assert(!__builtin_constant_p(FUNC(T(NegInf;
+

Test of smallest subnormal and largest finite number would also be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136568

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

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12452
+int Ilogb;
+if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St))
+  return false;

Izaron wrote:
> aaron.ballman wrote:
> > jcranmer-intel wrote:
> > > `long double` is `ppc_fp128` on at least some PPC targets, and while I'm 
> > > not entirely certain of what `ilogb` properly returns in the corner cases 
> > > of the `ppc_fp128`, I'm not entirely confident that the implementation of 
> > > `APFloat` is correct in those cases. I'd like someone with PPC background 
> > > to comment in here.
> > Paging @hubert.reinterpretcast for help finding a good person to comment on 
> > the PPC questions.
> @jcranmer-intel constexpr evaluation is quite machine-/target-independent. 
> Clang evaluates it based on its **internal** representation of float 
> variables.
> 
> [[ 
> https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/include/llvm/ADT/APFloat.h#L1256
>  | int ilogb ]] uses `Arg.getIEEE()`, that [[ 
> https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/include/llvm/ADT/APFloat.h#L819-L825
>  | returns Clang's internal float representation ]].
> 
> Whichever float semantics is being used, [[ 
> https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/llvm/lib/Support/APFloat.cpp#L54-L61
>  | minExponent and maxExponent are representable as APFloatBase::ExponentType 
> ]], which is `int32_t`:
> ```
> /// A signed type to represent a floating point numbers unbiased exponent.
> typedef int32_t ExponentType;
> ```
> 
> We already use `int ilogb` in some constexpr evaluation code: [[ 
> https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/clang/lib/AST/ExprConstant.cpp#L14592
>  | link ]], it is working correct because it is working on Clang's float 
> representations.
> We already use `int ilogb` in some constexpr evaluation code: [[ 
> https://github.com/llvm/llvm-project/blob/2e5bf4da99a2f8d3d4bb4f1a4d1ed968a01e8f02/clang/lib/AST/ExprConstant.cpp#L14592
>  | link ]], it is working correct because it is working on Clang's float 
> representations.

`APFloat::getIEEE()`, if I'm following it correctly, only returns the details 
of the high double in `ppc_fp128` floats, and I'm not sufficiently well-versed 
in the `ppc_fp128` format to establish whether or not the low double comes into 
play here. glibc seems to think that the low double comes into play in at least 
one case: 
https://github.com/bminor/glibc/blob/30891f35fa7da832b66d80d0807610df361851f3/sysdeps/ieee754/ldbl-128ibm/e_ilogbl.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136568

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

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/test/Sema/constant-builtins-ilogb.cpp:53-63
+// assert smallest subnormal and largest finite numbers
+static_assert(__builtin_ilogbf(1.40129846E-45f) == -149);
+static_assert(__builtin_ilogbf(3.40282347E+38f) == 127);
+
+static_assert(__builtin_ilogb(4.9406564584124654E-324) == -1074);
+static_assert(__builtin_ilogb(1.7976931348623157E+308) == 1023);
+

You can simplify at least the float, double, and especially long double tests 
using something like this:

```
static_assert(__builtin_ilogbf(__FLT_MIN__) == __FLT_MIN_EXP__ + 1);
static_assert(__builtin_ilogbf(__FLT_MAX__) == __FLT_MAX_EXP__ - 1);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136568

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

[PATCH] D134407: [FPEnv] Remove inaccurate comments regarding signaling NaN for isless

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel accepted this revision.
jcranmer-intel added a comment.
This revision is now accepted and ready to land.

This looks fine to me, but as clang is not normally my wheelhouse, I'd still 
prefer to see someone else approve this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134407

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

[PATCH] D134369: [Clang] Support constexpr builtin fmax

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D134369#3821949 , @efriedma wrote:

> I think __builtin_fmax can raise a floating-point exception; in that case, it 
> wouldn't be constant, I think?  Not sure how consistent we are about handling 
> that sort of thing in constant evaluation at the moment.

https://eel.is/c++draft/library.c#3 says that a floating-point exception other 
than `FE_INEXACT` causes it to not be a constant expression, if I have constant 
expression wording right. `FE_INVALID` can be raised if the inputs are 
signalling NaNs, but even if we're claiming implementation of Annex F, it's 
okay to treat sNaN as qNaN unless we're claiming `FE_SNANS_ALWAYS_SIGNAL` (this 
is new in C2x, I think). We shouldn't be claiming that except maybe if we're 
using `-fp-model=strict`. I don't think `#pragma STDC FENV_ACCESS ON` bears any 
relevance here, but now I'm wondering what needs to happen with that for 
regular floating-point operations that may trigger exceptions.




Comment at: clang/lib/AST/ExprConstant.cpp:14033-14034
+!EvaluateFloat(E->getArg(1), RHS, Info))
+  return false;
+if (Result.isNaN() || RHS > Result)
+  Result = RHS;

If I'm reading APFloat's source correctly, this doesn't suffice to consistently 
make __builtin_fmax(±0.0, ±0.0) return +0.0 if one of the zeroes is positive. 
(We're not required to return +0.0 in this case, but most libm implementations 
seem to endeavor to return +0.0 in this situation, even if the LLVM intrinsic 
we lower to doesn't).



Comment at: clang/test/Sema/constant-builtins-fmax.cpp:35-39
+#define FMAX_TEST_BOTH_ZERO(T, FUNC)   \
+static_assert(0.0 == FUNC(0.0, 0.0));  \
+static_assert(0.0 == FUNC(-0.0, 0.0)); \
+static_assert(0.0 == FUNC(0.0, -0.0)); \
+static_assert(0.0 == FUNC(-0.0, -0.0));

These tests aren't covering what the sign of zero is in these cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134369

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Some extra constexpr tests that may be interesting:

- Constant expressions that produce floating point exceptions other than 
`FE_INEXACT`.
- Subnormal values as operands, as well as operations with normal operands that 
produce denormal values (i.e., check for DAZ/FTZ, respectively.)
- qNaN arithmetic
- sNaN arithmetic
- different NaN payloads (this can also catch out differences caused by host 
hardware: there's disagreement as to which NaN payload is used in an fadd, some 
processors prefer the first one, others prefer the second one).
- converting infinity, NaN, numbers outside INT_MAX range to integers
- converting integers to floating-point outside the latter's range--that's only 
possible with `unsigned __int128` and `float` or `uint32_t` and `half` though

Some of these tests relate to things that you haven't implemented yet--and 
that's fine--but it's worth keeping in mind for future patches.




Comment at: clang/lib/AST/Interp/Floating.h:27-29
+  template  struct Repr;
+  template <> struct Repr<32> { using Type = float; };
+  template <> struct Repr<64> { using Type = double; };

aaron.ballman wrote:
> Er, how will this extend to `long double` where the number of bits is rather 
> more difficult?
Or `half` and `bfloat`, which are both 16-bit floating-point types?



Comment at: clang/lib/AST/Interp/Floating.h:31-33
+  // The primitive representing the Floating.
+  using ReprT = typename Repr::Type;
+  ReprT V;

My gut reaction is that, for floating-point types, you're probably better off 
using `APFloat` directly rather than trying to use the float/double platform 
types directly. Directly using platform types for computation means you 
potentially have changes based on the how clang itself was compiled--especially 
if someone decides to compile clang with `-ffast-math` and now all your 
constexpr stuff is executing in FTZ/DAZ mode. `APFloat` may be slower, but it 
also provides more protection from floating-point chicanery.



Comment at: clang/lib/AST/Interp/Floating.h:82
+  ComparisonCategoryResult compare(const Floating &RHS) const {
+return Compare(V, RHS.V);
+  }

This is supposed to be capable of returning unordered if `*this` or `RHS` is a 
NaN, right?



Comment at: clang/lib/AST/Interp/Floating.h:106
+  // ---
+
+  static bool add(Floating A, Floating B, unsigned OpBits, Floating *R) {

tbaeder wrote:
> The operations here don't do overflow or UB handling.
They also don't support rounding mode. (constexpr evaluation arguably should at 
least support `FENV_ROUND`)


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

https://reviews.llvm.org/D134859

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:27-29
+  template  struct Repr;
+  template <> struct Repr<32> { using Type = float; };
+  template <> struct Repr<64> { using Type = double; };

tbaeder wrote:
> jcranmer-intel wrote:
> > aaron.ballman wrote:
> > > Er, how will this extend to `long double` where the number of bits is 
> > > rather more difficult?
> > Or `half` and `bfloat`, which are both 16-bit floating-point types?
> I have spent some time with this today and tried to simply always use 
> `APFloat` instead of a primitive type. Unfortunately that doesn't work 
> because what we put on the stack is not the `Floating` (or `Integral`), but 
> the underlying primitive type. So even if we do the final math (in `::add`, 
> etc) via `APFloat`, we need something we can serialize to `char[]` so we can 
> put it on the stack. Do you think that would work?
I don't know enough about the structure of the bytecode interpreter here to say 
for sure, but this smells to me like you're baking in an assumption that every 
primitive target type has a corresponding primitive type on the host. This 
assumption just doesn't hold when it comes to floating point (only two of the 
seven types, `float` and `double`, are generally portable, and even then, there 
be dragons in some corner cases).

If you do need to continue down this route, there are two requirements that 
should be upheld:
* The representation shouldn't assume that the underlying primitive type exists 
on host (bfloat16 and float128 are better test cases here).
* Conversion to/from host primitive types shouldn't be easy to accidentally do.

(Worth repeating again that bit size is insufficient to distinguish floating 
point types: `bfloat` and `half` are both 16-bit, PPC `long double` and IEEE 
754 quad precision are both 128-bit, and x86 `long double` is 80 bits stored as 
96 bits on 32-bit and 128 bits on 64-bit.)


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

https://reviews.llvm.org/D134859

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:27-29
+  template  struct Repr;
+  template <> struct Repr<32> { using Type = float; };
+  template <> struct Repr<64> { using Type = double; };

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > jcranmer-intel wrote:
> > > > > tbaeder wrote:
> > > > > > jcranmer-intel wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > Er, how will this extend to `long double` where the number of 
> > > > > > > > bits is rather more difficult?
> > > > > > > Or `half` and `bfloat`, which are both 16-bit floating-point 
> > > > > > > types?
> > > > > > I have spent some time with this today and tried to simply always 
> > > > > > use `APFloat` instead of a primitive type. Unfortunately that 
> > > > > > doesn't work because what we put on the stack is not the `Floating` 
> > > > > > (or `Integral`), but the underlying primitive type. So even if we 
> > > > > > do the final math (in `::add`, etc) via `APFloat`, we need 
> > > > > > something we can serialize to `char[]` so we can put it on the 
> > > > > > stack. Do you think that would work?
> > > > > I don't know enough about the structure of the bytecode interpreter 
> > > > > here to say for sure, but this smells to me like you're baking in an 
> > > > > assumption that every primitive target type has a corresponding 
> > > > > primitive type on the host. This assumption just doesn't hold when it 
> > > > > comes to floating point (only two of the seven types, `float` and 
> > > > > `double`, are generally portable, and even then, there be dragons in 
> > > > > some corner cases).
> > > > > 
> > > > > If you do need to continue down this route, there are two 
> > > > > requirements that should be upheld:
> > > > > * The representation shouldn't assume that the underlying primitive 
> > > > > type exists on host (bfloat16 and float128 are better test cases 
> > > > > here).
> > > > > * Conversion to/from host primitive types shouldn't be easy to 
> > > > > accidentally do.
> > > > > 
> > > > > (Worth repeating again that bit size is insufficient to distinguish 
> > > > > floating point types: `bfloat` and `half` are both 16-bit, PPC `long 
> > > > > double` and IEEE 754 quad precision are both 128-bit, and x86 `long 
> > > > > double` is 80 bits stored as 96 bits on 32-bit and 128 bits on 
> > > > > 64-bit.)
> > > > Well, is there a way to convert an APFloat to a char[] that would work 
> > > > instead of going to float/double and storing that? The only thing I see 
> > > > in the docs is `convertToHexString()` (and the docs don't mention 
> > > > whether the conversion is lossy). If not, do you think adding such a 
> > > > conversion to `APFloat` and its various implementations is the better 
> > > > way forward?
> > > Let's avoid serializing the floats to strings so that we can parse the 
> > > string to turn it back into a float later; that's going to have poor 
> > > performance even if we do get all the corner cases correct regarding 
> > > things like rounding, etc.
> > > 
> > > `APFloat` does not have any sort of serialization functionality beyond 
> > > through strings representing the value. I think you'd have to invent such 
> > > an interface.
> > Do you know who I might talk to regrading such an interface, both the 
> > implementation as well as general feasibility?
> I think there may be at least two ways to do this: use an `APFloat` and put 
> the serialization interfaces there, or use an `APValue` and put the 
> serialization interfaces there.
> 
> Because `APFloat` is an ADT in LLVM, I think it should probably go up on 
> Discourse for broader discussion. @chandlerc is still listed as the code 
> owner for ADTs but he's not been active in quite some time. Instead, I would 
> recommend talking to @dblaikie (he's got a good eye for ADT work in general) 
> and @foad, @RKSimon, and @sepavloff as folks who have recently been touching 
> `APFloat`.
> 
> `APValue` is a Clang-specific class, and it already has some amount of 
> serialization support, it seems 
> (https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/APValue.h#L54).
>  From a quick look, it seems we're already using `APValue` in a reasonable 
> number of places in the interpreter, so it might make sense to use this 
> object consistently to represent all values in the new interpreter?
Going through `APInt` is already possible in `APFloat`, and that might have 
some methods to go to char arrays.


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

https://reviews.llvm.org/D134859

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:33-34
+  /// Primitive representing limits.
+  // static constexpr auto Min = std::numeric_limits::min();
+  // static constexpr auto Max = std::numeric_limits::max();
+

tbaeder wrote:
> This is currently commented out, but I //think// I can get the semantics of 
> the `APFloat` and ask its semantics for min/max values.
`APFloat::get{Largest,Smallest}` will do the trick.



Comment at: clang/lib/AST/Interp/Floating.h:89
+  bool isMin() const { return false; } // TODO
+  bool isMinusOne() const { return F == APFloat(-1.0f); }
+  bool isNan() const { return F.isNaN(); }

FYI, `operator==` on `APFloat` requires the two types to have the same 
semantics. Probably the fastest way to check if it's -1 is either `ilogb(F) == 
0 && F.isNegative()` or `F.isExactlyValue(-1.0)`.



Comment at: clang/lib/AST/Interp/Interp.cpp:421
 
+bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem) {
+  Floating F = S.Stk.pop();

FWIW, `const llvm::fltSemantics &` is the usual way it's used.


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

https://reviews.llvm.org/D134859

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

[PATCH] D134369: [Clang] Support constexpr builtin fmax

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:14029-14030
+  case Builtin::BI__builtin_fmaxf16:
+  case Builtin::BI__builtin_fmaxf128: {
+APFloat RHS(0.);
+if (!EvaluateFloat(E->getArg(0), Result, Info) ||

I think you should add a comment saying `// TODO: Handle sNaN`, just so that we 
remember to revisit this later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134369

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
Herald added subscribers: ldrumm, wenlei, ThomasRaoux, arphaman, Anastasia, 
yaxunl.
Herald added a project: All.
jcranmer-intel requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,35 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in the SPIR-V type name.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -1,16 +1,16 @@
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=X86
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple amdgcn-amd-amdhsa %s -o - | FileCheck %s --check-prefix=AMDGCN
 __kernel void testPipe( pipe int test )
 {
 int s = sizeof(test);
 // X86: store %opencl.pipe_ro_t* %test, %opencl.pipe_ro_t** %test.addr, align 8
 // X86: store i32 8, i32* %s, align 4
-// SPIR: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 4
-// SPIR: store i32 4, i32* %s, align 4
-// SPIR64: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 8
-// SPIR64: store i32 8, i32* %s, align 4
+// SPIR: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 4
+// SPIR: store i32 4, ptr %s, align 4
+// SPIR64: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 8
+// SPIR64: store i32 8, ptr %s, align 4
 // AMDGCN: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)* addrspace(5)* %test.addr, 

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Most of the testing for this change has been in conjunction with the changes in 
the SPIRV-LLVM-Translator repository here: 
https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/1799. I haven't 
updated the in-tree experimental target to support the target-extension types, 
but I did want to provide more documentation as to how these types work in the 
existing LLVM documentation for the SPIR-V backend.

There's still a few issues with this patch. First, I've made the use of these 
types unconditional for the SPIR-V target--it may be more appropriate to make 
these triggered off of a hidden option defaulted to off for now, or maybe based 
on whether or not opaque pointers are enabled. The other issue is the 
cast_image.cl test which... does an illegal operation in SPIR-V, and looking at 
the history of the test, doesn't seem to really test what it's claiming to test 
(https://reviews.llvm.org/D22927 was the revision it was adding tests for). I 
disabled the test for now for SPIR-V out of ignorance of which way to go about 
for fixing this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:40
+static llvm::Type *getSPIRVType(llvm::LLVMContext &Ctx, StringRef BaseType,
+StringRef OpenCLName, StringRef ReadSuffix) {
+  SmallVector IntParams = {0, 0, 0, 0, 0, 0};

bader wrote:
> I believe this can be done at "compile time" (i.e. during the clang build, 
> not clang run).
> Can we have a pre-computed map from an OpenCL built-in type to a SPIR-V type?
> Another option is compile-time evaluated function. This should be possible, 
> right?
> 
> If I get it right, here we take a string representation of an OpenCL image 
> type and process it at runtime, which seems to be unnecessary as we have 
> pre-defined (by the spec) set of the types.
I can definitely switch the read suffix to use a compile-time enum, since there 
are only 3 cases (plus, it's a static assert). Making the openCL name to int 
param conversion be a compile-time constant might be doable with some tricks, 
but I'll have to think about it for a little bit. It's a little harder because 
we're taking a string to 6 array values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.






Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:80
+
+  if (useSPIRVTargetExtType(CGM)) {
+switch (cast(T)->getKind()) {

Anastasia wrote:
> Perhaps it's best to split into separate functions and reflect in naming what 
> they correspond to.
I've decided to pull this out into a new subclass of CGOpenCLRuntime that's 
target specific.



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:100
+#define INTEL_SUBGROUP_AVC_TYPE(Name, Id)  
\
+case BuiltinType::OCLIntelSubgroupAVC##Id: 
\
+  return llvm::TargetExtType::get(Ctx, "spirv.Avc" #Id "INTEL");

Anastasia wrote:
> Why does this need special handling?
The `EXT_OPAQUE_TYPE` macro doesn't map cleanly to the SPIR-V OpType* 
names--the parameters you'd get would be `intel_sub_group_avc_mce_payload_t, 
OCLIntelSubgroupAVCMcePayload, cl_intel_device_side_avc_motion_estimation` 
whereas the SPIR-V type name is supposed to be `AvcMcePayloadINTEL`. Using the 
`INTEL_SUBGROUP_AVC_TYPE` instead allows me to construct the correct SPIR-V 
type name without having to manually write out each case statement, or adjust 
macros used in many, many more places. (As it is, I need to rename 4 of the 
types).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 487960.
jcranmer-intel added a comment.

Fix some of the code review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,35 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in the SPIR-V instruction.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -1,16 +1,16 @@
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=X86
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple amdgcn-amd-amdhsa %s -o - | FileCheck %s --check-prefix=AMDGCN
 __kernel void testPipe( pipe int test )
 {
 int s = sizeof(test);
 // X86: store %opencl.pipe_ro_t* %test, %opencl.pipe_ro_t** %test.addr, align 8
 // X86: store i32 8, i32* %s, align 4
-// SPIR: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 4
-// SPIR: store i32 4, i32* %s, align 4
-// SPIR64: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 8
-// SPIR64: store i32 8, i32* %s, align 4
+// SPIR: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 4
+// SPIR: store i32 4, ptr %s, align 4
+// SPIR64: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 8
+// SPIR64: store i32 8, ptr %s, align 4
 // AMDGCN: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)* addrspace(5)* %test.addr, align 8
 // AMDGCN: store i32 8, i32 addrspace(5)* %s, 

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Interp.cpp:487
+  if (S.inConstantContext())
+return true;
+

Not sure I understand the conditions that cause `S.inConstantContext()` to be 
true, which gives me some cause for concern. Additionally, there's no tests 
covering the checks in the function.



Comment at: clang/lib/AST/Interp/Opcodes.td:502
+  let Types = [AluTypeClass];
+  let Args = [ArgFltSemantics];
+  let HasGroup = 1;

Integer-to-floating point conversion is dependent on rounding mode--consider 
`(float)UINT_MAX`.


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

https://reviews.llvm.org/D134859

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:502
+  let Types = [AluTypeClass];
+  let Args = [ArgFltSemantics];
+  let HasGroup = 1;

tbaeder wrote:
> sepavloff wrote:
> > tbaeder wrote:
> > > jcranmer-intel wrote:
> > > > Integer-to-floating point conversion is dependent on rounding 
> > > > mode--consider `(float)UINT_MAX`.
> > > This test succeeds here, whether I use `-frounding-math` or not:
> > > 
> > > ```
> > > constexpr float f = (float)4294967295;
> > > static_assert(f == (float)4.2949673E+9);
> > > ```
> > > How can I test this behavior?
> > You can use `#pragma STDC FENV_ROUND`. See 
> > `clang/test/AST/const-fpfeatures.c` as an example.
> Why does it work here to compare both `f` and `f2` to the same value? 
> https://godbolt.org/z/zdsf1sK7r
`FENV_ROUND` is a new feature of C2x, so it doesn't look like it's properly 
handled in gcc or clang yet. (It's for sure not handled in clang--the code to 
convert a floating-point literal into an `APFloat` hardcodes default rounding 
mode in its call.)

Dynamic rounding mode shows you the difference: https://godbolt.org/z/856xM8KTh


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

https://reviews.llvm.org/D134859

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Looking at the attribute logic here, there is conceptual room for both a 
`dynamic` and an `unknown` mode (i.e., you get a top and a bottom value), but I 
don't think there is value in distinguishing between them, so I'm fine with 
keeping just a `dynamic`.

I didn't bother to look at the clang changes, and my only comments are some 
minor ones around documentation:




Comment at: llvm/docs/LangRef.rst:2166-2167
+
+If the mode is ``"dynamic"``, transformations which depend on the
+behavior of denormal values should not be performed.
+

I feel like the description of this mode should mention that whether or not 
denormals are flushed is derived from the dynamic state of the FP environment.



Comment at: llvm/docs/LangRef.rst:2174-2178
If the input mode is ``"preserve-sign"``, or ``"positive-zero"``, a
floating-point operation must treat any input denormal value as
zero. In some situations, if an instruction does not respect this
mode, the input may need to be converted to 0 as if by
``@llvm.canonicalize`` during lowering for correctness.

This isn't your fault, but I noticed when reading the LangRef online that this 
paragraph has a slightly-different indentation that causes most of this 
attribute's documentation to gain an extra level of indentation.


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

https://reviews.llvm.org/D142907

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

[PATCH] D141899: [IR][X86] Remove X86AMX type in LLVM IR instead of target extension

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

From some of the verifier checks and tests, it looks like `target("x86.amx")` 
would also require some new type properties, to express its unsuitability for 
alloca-and-friends, as well as non-intrinsic arguments.

The spelling change needs to be release noted, and I would like there to be 
some mention of the the type in documentation going forward, but it seems that 
X86 doesn't have a target-specific documentation page yet.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5338
+if (TargetExtTy->getName() == "x86.AMX")
+  return true;
+  return false;

nikic wrote:
> Probably worthwhile to add an overload like `Type::isTargetExtTy(StringRef)`?
+1 to a method for this pattern.



Comment at: llvm/include/llvm-c/Core.h:168
   LLVMBFloatTypeKind,/**< 16 bit brain floating point type */
-  LLVMX86_AMXTypeKind,   /**< X86 AMX */
   LLVMTargetExtTypeKind, /**< Target extension type */

Removing this enum value changes the ABI. I don't think we've removed a type 
before, but with analogy to the opcode enum above, we should probably 
explicitly enumerate the type kinds and leave a hole for where 
`LLVMX86_AMXTypeKind` used to be.



Comment at: llvm/include/llvm-c/Core.h:1555
- */
-LLVMTypeRef LLVMX86AMXTypeInContext(LLVMContextRef C);
-

Retaining the existing LLVM-C methods is possible without much maintenance 
overhead, so we should do so.



Comment at: llvm/lib/IR/Type.cpp:843
+  ArrayType::get(FixedVectorType::get(Type::getIntNTy(C, 32), 16), 16),
+  TargetExtType::HasZeroInit, TargetExtType::CanBeGlobal);
+  }

If I'm following the verifier rules for `x86_amx` correctly, these are not in 
fact true for a `target("x86.amx")` type.



Comment at: llvm/test/Verifier/x86_amx1.ll:2-3
-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
-
-@GV = dso_local global [10 x x86_amx] zeroinitializer, align 16
-; CHECK: invalid array element type

It should be possible to retain these verifier tests even if `x86_amx` is moved 
to a target extension type, although the messages may need to change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141899

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

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D127579#3585553 , @bader wrote:

> And in addition to that ISA defines types, which are not natively supported 
> by LLVM IR e.g. image. To represent those types clang in OpenCL language mode 
> emits a pointer to an opaque structure with special name like 
> opencl. (e.g. opencl.image2d_t). All ISA types, which are 
> defined that way look the same with type-less pointers.
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/OpenCLImageTypes.def

One of the important facts about these types is that the SPIR-V specification 
doesn't let you actually cast between these types and other types such as 
integers. (Which is an issue because I've just come across a testcase where, in 
opaque pointer mode, an OpenCL event type is being used in a `ptrtoint` to 
store to a variable as an `i64`, which I can't legally translate to SPIR-V.) Of 
course, this probably means that these types need to have their representation 
in LLVM changed entirely, but I haven't yet done the legwork to experiment in 
that mode.

I haven't yet tested whether or not this patch is sufficient to support all of 
the use cases I need for type scavenging for SPIR-V, but one of the things I do 
see is that there's no support for return type information, which may be a bit 
of an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127579

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

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D127579#3588607 , @iliya-diyachkov 
wrote:

> I think the return type information can be added in the next version of the 
> patch, however it should be attached to function declarations (not 
> definitions as it's done now), right? Do you think, declarations also require 
> information about argument types?

Function declarations are more useful to get information than function 
definitions. A lot of the failures I'm currently seeing in my side of things 
appear to be due to not getting pointer types right when calling OpenCL/SYCL 
builtins causing interesting things to happen later on.

There's another approach I've considered taking, which is rather than using a 
metadata-based approach, emitting `elementtype` for any pointer-valued 
arguments [it doesn't solve return values, though]. This does require more 
changes to LLVM to let such attributes work on non-intrinsic functions, but it 
may end up working smoother than trying to reparse C types as LLVM types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127579

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

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
Herald added a subscriber: ormris.
Herald added a project: All.
jcranmer-intel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127221

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/print-pipeline-passes.c


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a fire to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: BitcodeWriterPass
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -96,6 +96,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 }
 
 namespace {
@@ -958,6 +959,17 @@
 break;
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a fire to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: BitcodeWriterPass
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -96,6 +96,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 }
 
 namespace {
@@ -958,6 +959,17 @@
 break;
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155394: [clang][Interp] Implement __builtin_fpclassify

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/test/AST/Interp/builtin-functions.cpp:121-130
+  char classify_nan [__builtin_fpclassify(+1, -1, -1, -1, -1, 
__builtin_nan(""))];
+  char classify_snan[__builtin_fpclassify(+1, -1, -1, -1, -1, 
__builtin_nans(""))];
+  char classify_inf [__builtin_fpclassify(-1, +1, -1, -1, -1, 
__builtin_inf())];
+  char classify_neg_inf [__builtin_fpclassify(-1, +1, -1, -1, -1, 
-__builtin_inf())];
+  char classify_normal  [__builtin_fpclassify(-1, -1, +1, -1, -1, 1.539)];
+  char classify_normal2 [__builtin_fpclassify(-1, -1, +1, -1, -1, 1e-307)];
+  char classify_denorm  [__builtin_fpclassify(-1, -1, -1, +1, -1, 1e-308)];

aaron.ballman wrote:
> One thing we should test is that promotion does not happen on the final 
> argument (converting it from `float` to `double`) -- @jcranmer-intel can you 
> think of a good way to test that in this case?
A subnormal `float` would be a normal `double`, so using `1.0e-38f` should 
classify as a denormal and not a normal numal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155394

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

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D155546#4510691 , @aaron.ballman 
wrote:

> It's not yet clear to me what happens when any of these functions encounter a 
> signaling NaN at compile time. CC @hubert.reinterpretcast @jcranmer-intel 
> @rsmith

`fmin(sNaN, x)` signals FE_INVALID, which would be a compile-time error per 
[library.c]p3 (every FP exception save FE_INEXACT is a compile-time error). 
Except sNaN raising FE_INVALID is only a //recommended// practice, so it's 
really unclear what the C++ standard attempts to say on this matter.

Treating sNaN as always signaling FE_INVALID is probably the safer option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155546

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

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73
+  constexpr float f1 = __builtin_fmin(1.0, 2.0f);
+  static_assert(f1 == 1.0f, "");
+
+  constexpr float min = __builtin_fmin(__builtin_nan(""), 1);
+  static_assert(min == 1, "");
+  constexpr float min2 = __builtin_fmin(1, __builtin_nan(""));
+  static_assert(min2 == 1, "");

tbaeder wrote:
> aaron.ballman wrote:
> > Can you add a test using `__builtin_nans` to show that it results in an 
> > invalid constant expression because of the `FE_INVALID` signal?
> It doesn't currently result in an invalid constant expression in clang (both 
> new and current interpreter). Where should that signal occur? Or do I need to 
> check for signaling nans whenever I compute a floating value?
Most, but not all, floating-point operations with an sNaN signal an exception. 
The complete list of exceptions is, I believe:

- C2x 7.12.3 classification macros (e.g., `isnan`, `fpclassify`)
- totalorder, totalordermag
- fneg, fabs, copysign, "copy" (basically anything that could do an SSA copy of 
the value)
- conversion to/from strings, maybe (IEEE 754 has some "should"s in here)

The best place to do the checking for sNaNs is where you're handling the inputs 
for a function.

(As a brief aside, C++23 only discusses making FP exceptions compile-time 
errors for calling C library functions, not regular floating-point exceptions.)


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

https://reviews.llvm.org/D155546

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

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/test/AST/Interp/builtin-functions.cpp:65-73
+  constexpr float f1 = __builtin_fmin(1.0, 2.0f);
+  static_assert(f1 == 1.0f, "");
+
+  constexpr float min = __builtin_fmin(__builtin_nan(""), 1);
+  static_assert(min == 1, "");
+  constexpr float min2 = __builtin_fmin(1, __builtin_nan(""));
+  static_assert(min2 == 1, "");

aaron.ballman wrote:
> jcranmer-intel wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > Can you add a test using `__builtin_nans` to show that it results in an 
> > > > invalid constant expression because of the `FE_INVALID` signal?
> > > It doesn't currently result in an invalid constant expression in clang 
> > > (both new and current interpreter). Where should that signal occur? Or do 
> > > I need to check for signaling nans whenever I compute a floating value?
> > Most, but not all, floating-point operations with an sNaN signal an 
> > exception. The complete list of exceptions is, I believe:
> > 
> > - C2x 7.12.3 classification macros (e.g., `isnan`, `fpclassify`)
> > - totalorder, totalordermag
> > - fneg, fabs, copysign, "copy" (basically anything that could do an SSA 
> > copy of the value)
> > - conversion to/from strings, maybe (IEEE 754 has some "should"s in here)
> > 
> > The best place to do the checking for sNaNs is where you're handling the 
> > inputs for a function.
> > 
> > (As a brief aside, C++23 only discusses making FP exceptions compile-time 
> > errors for calling C library functions, not regular floating-point 
> > exceptions.)
> That's the way that I would approach it (to check for a signaling NaN any 
> time you need its value) and I think that's the same as the suggestion from 
> @jcranmer-intel.
> not regular floating-point exceptions.

That should say "regular floating-point operations" (e.g., `a + b`).


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

https://reviews.llvm.org/D155546

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

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319
+if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half))
   PromoteType = Context.DoubleTy;

aaron.ballman wrote:
> Hmmm... the existing code seems wrong to me because it's not paying any 
> attention to `FLT_EVAL_METHOD`, but I think it probably should? CC 
> @jcranmer-intel @zahiraam for opinions.
> 
> Actually, I wonder if the correct approach here is to split 
> `Sema::DefaultArgumentPromotion()` up so that we can calculate what the 
> default argument promoted type is of the expression independent of performing 
> the actual promotion, and call the promotion type calculation logic here?
C23 6.5.2.2p6 [draft N3096] says "trailing arguments that have type `float` are 
promoted to `double`". `FLT_EVAL_METHOD` shouldn't need to apply here, since 
it's largely about reflecting that things like x87 using 80-bit precision 
internally, and not actual argument passing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

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

[PATCH] D156506: [clang][Interp] Check floating results for NaNs

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

You definitely don't want these rules to apply to all qNaNs. It's when an input 
operand is an sNaN for many operations. Note that the result of an operation 
with an sNaN as input (and FP result type) is a qNaN output, and the only times 
that you get an sNaN output are the times when you never signal (I think), so 
checking the result type is incorrect.




Comment at: clang/lib/AST/Interp/Interp.cpp:534-539
   if ((Status & APFloat::opStatus::opInvalidOp) &&
   FPO.getExceptionMode() != LangOptions::FPE_Ignore) {
 // There is no usefully definable result.
 S.FFDiag(E);
 return false;
   }

A further note is that sNaNs signal `FE_INVALID` when used, so sNaN should 
generally fall into this if statement in particular.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156506

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

[PATCH] D156506: [clang][Interp] Check floating results for NaNs

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Interp.cpp:503
+  //   If during the evaluation of an expression, the result is not
+  //   mathematically defined [...], the behavior is undefined.
+  // FIXME: C++ rules require us to not conform to IEEE 754 here.

tbaeder wrote:
> @jcranmer-intel Doesn't this comment (which I've coped from 
> `ExprConstant.cpp`) contradict what you said about not checking the result?
Immediately following that in the specification is this:
> [Note 3: Treatment of division by zero, forming a remainder using a zero 
> divisor, and all floating-point exceptions varies among machines, and is 
> sometimes adjustable by a library function. — end note]

The current C++ specification is rather clear about its unclarity on 
floating-point. Also note that IEEE 754 defines floating-point data as 
consisting of {-inf, ..., -0} union {+0, ..., +inf} union {NaN}. So NaN is 
arguable to be a well-defined mathematical result, if you consider that 
floating-point types don't model real numbers but an approximation of real 
numbers (just as unsigned integers model not integers but integers mod 2^N).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156506

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

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 556476.
jcranmer-intel added a comment.

Rebase to trunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/print-pipeline-passes.c


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: BitcodeWriterPass
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: BitcodeWriterPass
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102
+outs() << "\n";
+return;
+  }

aeubanks wrote:
> I wouldn't return here, doesn't seem right that we'll skip running the opt 
> pipeline but continue with compilation. we should either bail out entirely  
> of producing any output files (which would probably require code changes 
> elsewhere), or run everything as normal, not do something weird where we 
> don't run the optimization pipeline but still output files
This is basically doing the same thing that `opt -print-pipeline-passes` is 
doing: 
https://github.com/llvm/llvm-project/blob/d1b418f55263ec48d14f220ad020d55f126cfcab/llvm/tools/opt/NewPMDriver.cpp#L500-L524.

In the case of emitting LLVM IR or bitcode, this logic is sufficient to not 
emit any output. In the case of .s or .o files, it looks like I have to also 
bail out of running `RunCodegenPipeline`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 556483.
jcranmer-intel added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/print-pipeline-passes.c


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should 
also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 556574.
jcranmer-intel added a comment.

Replace verify with annotation-remarks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/print-pipeline-passes.c


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: annotation-remarks
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should 
also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: annotation-remarks
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel marked 5 inline comments as done.
jcranmer-intel added inline comments.



Comment at: clang/test/CodeGen/print-pipeline-passes.c:8
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}

aeubanks wrote:
> aeubanks wrote:
> > I believe this will fail in non-assert builds
> since we don't run the verifier in non-assert builds
Apparently verify is added by clang unless `-disable-llvm-verifier` is present 
on the command line, but I've switched it to annotation-remarks nevertheless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

[Joshua Cranmer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jcranmer-intel marked an inline comment as done.
Closed by commit rGbf4923710333: [Clang] Enable -print-pipeline-passes in 
clang. (authored by jcranmer-intel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/print-pipeline-passes.c


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
-mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: annotation-remarks
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should 
also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");


Index: clang/test/CodeGen/print-pipeline-passes.c
===
--- /dev/null
+++ clang/test/CodeGen/print-pipeline-passes.c
@@ -0,0 +1,9 @@
+// Test that -print-pipeline-passes works in Clang
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null -mllvm -print-pipeline-passes -O0 %s 2>&1 | FileCheck %s
+
+// Don't try to check all passes, just a few to make sure that something is
+// actually printed.
+// CHECK: always-inline
+// CHECK-SAME: annotation-remarks
+void Foo(void) {}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 
 namespace llvm {
 extern cl::opt DebugInfoCorrelate;
+extern cl::opt PrintPipelinePasses;
 
 // Experiment to move sanitizers earlier.
 static cl::opt ClSanitizeOnOptimizerEarlyEP(
@@ -1090,6 +1091,17 @@
   TheModule->addModuleFlag(Module::Error, "UnifiedLTO", uint32_t(1));
   }
 
+  // Print a textual, '-passes=' compatible, representation of pipeline if
+  // requested.
+  if (PrintPipelinePasses) {
+MPM.printPipeline(outs(), [&PIC](StringRef ClassName) {
+  auto PassName = PIC.getPassNameForClassName(ClassName);
+  return PassName.empty() ? ClassName : PassName;
+});
+outs() << "\n";
+return;
+  }
+
   // Now that we have all of the passes ready, run them.
   {
 PrettyStackTraceString CrashInfo("Optimizer");
@@ -1127,6 +1139,13 @@
 return;
   }
 
+  // If -print-pipeline-passes is requested, don't run the legacy pass manager.
+  // FIXME: when codegen is switched to use the new pass manager, it should also
+  // emit pass names here.
+  if (PrintPipelinePasses) {
+return;
+  }
+
   {
 PrettyStackTraceString CrashInfo("Code generation");
 llvm::TimeTraceScope TimeScope("CodeGenPasses");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Friendly review ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 501594.
jcranmer-intel added a comment.

Documentation tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,36 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in their `corresponding
+SPIR-V instruction `_.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -7,9 +7,9 @@
 int s = sizeof(test);
 // X86: store ptr %test, ptr %test.addr, align 8
 // X86: store i32 8, ptr %s, align 4
-// SPIR: store ptr addrspace(1) %test, ptr %test.addr, align 4
+// SPIR: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 4
 // SPIR: store i32 4, ptr %s, align 4
-// SPIR64: store ptr addrspace(1) %test, ptr %test.addr, align 8
+// SPIR64: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 8
 // SPIR64: store i32 8, ptr %s, align 4
 // AMDGCN: store ptr addrspace(1) %test, ptr addrspace(5) %test.addr, align 8
 // AMDGCN: store i32 8, ptr addrspace(5) %s, align 4
Index: clang/test/CodeGenOpenCL/sampler.cl
===
--- clang/test/CodeGenOpenCL/sampler.cl
+++ clang/test/CodeGenOpenCL/sampler.cl
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -no-opaque-pointers %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=clc++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1 %s -cl-std=clc++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986
+  // for more details).
+  SmallVector IntParams = {0, 0, 0, 0, 0, 0};
+

Anastasia wrote:
> I think the list initialization doesn't do what you are trying to achieve 
> with `SmallVector` as it appends the elements. You should probably just be 
> using constructor with size i.e. IntParams(6)?
The list initialization is filling in 6 0 elements, which is intentional.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10992
+  if (OpenCLName.startswith("image2d"))
+IntParams[0] = 1; // 1D
+  else if (OpenCLName.startswith("image3d"))

Anastasia wrote:
> Ok, is the order of parameters defined or documented somewhere? Would it make 
> sense to create some kind of a local enum map containing the indices and use 
> enum members instead of constants to improve readability/maintenance?
They're documented in the link given in the first comment in this method: 
https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpTypeImage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D145150: clang: Emit nofpclass(nan inf) for -ffinite-math-only

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2212
+static llvm::FPClassTest getNoFPClassTestMask(const LangOptions &LangOpts) {
+  // TODO: Handle -fno-signaling-nans
+  llvm::FPClassTest Mask = llvm::fcNone;

Clang doesn't have support for -f[no-]signaling-nans yet, but the gcc 
documentation for the option states:

> Compile code assuming that IEEE signaling NaNs may generate user-visible 
> traps during floating-point operations.  Setting this option disables 
> optimizations that may change the number of exceptions visible with signaling 
> NaNs.  This option implies -ftrapping-math.

This strikes me as saying that sNaNs are treated as qNaN (akin to the `nsz` 
fast-math flag) rather than saying that it's UB to have `sNaN` as a value, 
thus, I don't think it makes sense for -fno-signaling-nans to translate into a 
`nofpclass` attribute.


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

https://reviews.llvm.org/D145150

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Friendly review ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

I'm studiously ignoring the Clang and LLVM codegen changes here, but otherwise, 
I think the direction of this change is generally good.




Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1377-1378
 llvm_unreachable("unknown denormal mode");
-return Operand;
+  case DenormalMode::Dynamic:
+return nullptr;
   case DenormalMode::IEEE:

You should change the doxygen documentation to indicate that this method 
returns nullptr if the denormal mode is dynamic.

Ditto for ConstantFoldFPInstOperands.


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

https://reviews.llvm.org/D142907

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

[PATCH] D145150: clang: Emit nofpclass(nan inf) for -ffinite-math-only

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

I'm generally okay with the approach of this patch. I'm not sufficiently 
well-versed in the clang codegen side of things to know if this covers all of 
the bases, and I'd appreciate someone who is familiar with that side of things 
to approve this patch.


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

https://reviews.llvm.org/D145150

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer 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 rGbcad161db3e6: [Clang][SPIR-V] Emit target extension types 
for OpenCL types on SPIR-V. (authored by jcranmer-intel).

Changed prior to commit:
  https://reviews.llvm.org/D141008?vs=501594&id=504768#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,36 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in their `corresponding
+SPIR-V instruction `_.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -7,9 +7,9 @@
 int s = sizeof(test);
 // X86: store ptr %test, ptr %test.addr, align 8
 // X86: store i32 8, ptr %s, align 4
-// SPIR: store ptr addrspace(1) %test, ptr %test.addr, align 4
+// SPIR: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 4
 // SPIR: store i32 4, ptr %s, align 4
-// SPIR64: store ptr addrspace(1) %test, ptr %test.addr, align 8
+// SPIR64: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 8
 // SPIR64: store i32 8, ptr %s, align 4
 // AMDGCN: store ptr addrspace(1) %test, ptr addrspace(5) %test.addr, align 8
 // AMDGCN: store i32 8, ptr addrspace(5) %s, align 4
Index: clang/test/CodeGenOpenCL/sampler.cl
===
--- clang/test/CodeGenOpenCL/sampler.cl
+++ clang/test/CodeGenOpenCL/sampler.cl
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -no-opaque-pointers %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=clc++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 495866.
jcranmer-intel marked 2 inline comments as done.
jcranmer-intel added a comment.
Herald added a subscriber: jdoerfert.

This updates code, and rebases tests on top of trunk.

Note: test issues still haven't been fixed, will fix that likely in the next
revision of the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,36 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in their `corresponding
+SPIR-V instruction `_.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -1,16 +1,16 @@
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=X86
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
-// RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR
+// RUN: %clang_cc1 -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple spir64-unknown-unknown %s -o - | FileCheck %s --check-prefix=SPIR64
 // RUN: %clang_cc1 -no-opaque-pointers -x cl -O0 -cl-std=CL2.0 -emit-llvm -triple amdgcn-amd-amdhsa %s -o - | FileCheck %s --check-prefix=AMDGCN
 __kernel void testPipe( pipe int test )
 {
 int s = sizeof(test);
 // X86: store %opencl.pipe_ro_t* %test, %opencl.pipe_ro_t** %test.addr, align 8
 // X86: store i32 8, i32* %s, align 4
-// SPIR: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 4
-// SPIR: store i32 4, i32* %s, align 4
-// SPIR64: store %opencl.pipe_ro_t addrspace(1)* %test, %opencl.pipe_ro_t addrspace(1)** %test.addr, align 8
-// SPIR64: store i32 8, i32* %s, align 4
+// SPIR: store target("spirv.Pipe",

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel marked an inline comment as done.
jcranmer-intel added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2260
 
-return CGF.CGM.getNullPointer(cast(ConvertType(DestTy)),
-  DestTy);
+// The type may be a target extension type instead of a pointer type
+// (e.g., OpenCL types mapped for SPIR-V). In the former case, emit a

Anastasia wrote:
> Ok, yet this looks strange to me... do you have an example that hits this 
> code?
> 
> At some point we added `CK_ZeroToOCLOpaqueType`   so I wonder if we 
> should be using this instead of `CK_NullToPointer` here i.e. ideally clang 
> should not assume too early how type are mapped into target specific 
> representation.
> 
I thought I had a test case that caused this code to be executed it, but after 
removing it and trying out the entire testsuite with it disabled, it never 
fired. So these changes are indeed unnecessary.



Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:200
+
+  // Choose the dimension of the image--this corresponds to the Dim parameter,
+  // so (e.g.) a 2D image has value 1, not 2.

Anastasia wrote:
> Any reason for this? Can we create a `constexpr` map or enum type that would 
> contain those numbers instead of using hard coded ones scattered around?
There are constants in the SPIR-V backend that could conceivably be reused 
(generated from tablegen), but there's no guarantee that we're being compiled 
with the LLVM SPIR-V backend enabled, which makes it hard to reuse them.

I can add some more comments to explicitly give the references that specify 
what the values mean, but I'm not entirely certain it's worth building enums 
just for this one function to use.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:239
+  // extension types.
+  if (getTriple().isSPIRV() || getTriple().isSPIR())
+OpenCLRuntime.reset(new CGSpirVOpenCLRuntime(*this));

Anastasia wrote:
> Do we want to change old SPIR representation or keep it as is? It seems that 
> SPIR spec defined them as LLVM's opaque pointer types... but I appreciate 
> that for maintenance purposes it's easier to keep those in sync.
From what I can observe, SPIR and SPIRV seem to be used interchangeably as far 
as the toolchains are concerned--anything targetting SPIR ends up going through 
the SPIRV toolchain. This is why I've triggered it for both of the toolchains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

Not entirely sure where the best place to effect this (I think somewhere in the 
clang driver code?), but on further reflection, it feels like strict fp-model 
in clang should set the denormal mode to dynamic.


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

https://reviews.llvm.org/D142907

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

In D142907#4132430 , @arsenm wrote:

> I was thinking of changing the default in general to dynamic. I was going to 
> at least change the strictfp default in a follow up

I had the same thought too, but I reflected a little further that the default 
fp model implying that the environment being in the default state means we can 
assume the FTZ/DAZ are also in a default (IEEE) state.

In D142907#4132543 , @kpn wrote:

> What's the plan for tying this to strictfp? Because I don't it should be tied 
> to cases where we use the constrained intrinsics but the exceptions are 
> ignored and the default rounding is in stated. Those instructions are 
> supposed to behave the same as the non-constrained instructions. So keying 
> off the presence of the strictfp attribute on the function definition, or the 
> (equivalent) presence of constrained intrinsics, would be too simple.

The way I see it, `strictfp` is an assertion that every FP instruction has a 
dependency on the FP environment, which is largely orthogonal to the 
`denormal-mode` attribute asserting that the FTZ/DAZ bits in the FP environment 
have a particular value. The constrained intrinsics also have the ability to 
assert some properties of the FP environment (specifically, rounding mode and 
exception behavior) on individual instructions. By not adding any metadata to 
constrained intrinsics at the same time, we don't get the ability to set the 
denormal-mode on a per-instruction basis-but I don't think there's much value 
to be gained by doing so (giving that we already have it at a per-function 
level).

> Would we get different denormal behavior with a clang flag vs using a #pragma 
> at the top of a source file? That seems surprising as well.

One of the consequences of having so many different ways of controlling 
compiler FP environment assumptions is that there's a crazy amount of 
interactions to consider. But I think there is ultimately a workable solution 
for the clang frontend to generate interactions that make sense.


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

https://reviews.llvm.org/D142907

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

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel updated this revision to Diff 498453.
jcranmer-intel marked 5 inline comments as done.
jcranmer-intel added a comment.

Mostly test fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/OpenCLExtensionTypes.def
  clang/lib/CodeGen/CGOpenCLRuntime.cpp
  clang/lib/CodeGen/CGOpenCLRuntime.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/test/CodeGenOpenCL/cast_image.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
  clang/test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  clang/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/test/CodeGenOpenCL/sampler.cl
  clang/test/Index/pipe-size.cl
  llvm/docs/SPIRVUsage.rst

Index: llvm/docs/SPIRVUsage.rst
===
--- llvm/docs/SPIRVUsage.rst
+++ llvm/docs/SPIRVUsage.rst
@@ -75,3 +75,36 @@
 Example:
 
 ``-target spirv64v1.0`` can be used to compile for SPIR-V version 1.0 with 64-bit pointer width.
+
+.. _spirv-types:
+
+Representing special types in SPIR-V
+
+
+SPIR-V specifies several kinds of opaque types. These types are represented
+using target extension types. These types are represented as follows:
+
+  .. table:: SPIR-V Opaque Types
+
+ == == =
+ SPIR-V TypeLLVM type name LLVM type arguments
+ == == =
+ OpTypeImage``spirv.Image``sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeSampler  ``spirv.Sampler``  (none)
+ OpTypeSampledImage ``spirv.SampledImage`` sampled type, dimensionality, depth, arrayed, MS, sampled, image format, access qualifier
+ OpTypeEvent``spirv.Event``(none)
+ OpTypeDeviceEvent  ``spirv.DeviceEvent``  (none)
+ OpTypeReserveId``spirv.ReserveId``(none)
+ OpTypeQueue``spirv.Queue``(none)
+ OpTypePipe ``spirv.Pipe`` access qualifier
+ OpTypePipeStorage  ``spirv.PipeStorage``  (none)
+ == == =
+
+All integer arguments take the same value as they do in their `corresponding
+SPIR-V instruction `_.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
+SPIR-V IR as ``target("spirv.Image", void, 1, 1, 0, 0, 0, 0, 0)``, with its
+dimensionality parameter as ``1`` meaning 2D. Sampled image types include the
+parameters of its underlying image type, so that a sampled image for the
+previous type has the representation
+``target("spirv.SampledImage, void, 1, 1, 0, 0, 0, 0, 0)``.
Index: clang/test/Index/pipe-size.cl
===
--- clang/test/Index/pipe-size.cl
+++ clang/test/Index/pipe-size.cl
@@ -7,9 +7,9 @@
 int s = sizeof(test);
 // X86: store ptr %test, ptr %test.addr, align 8
 // X86: store i32 8, ptr %s, align 4
-// SPIR: store ptr addrspace(1) %test, ptr %test.addr, align 4
+// SPIR: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 4
 // SPIR: store i32 4, ptr %s, align 4
-// SPIR64: store ptr addrspace(1) %test, ptr %test.addr, align 8
+// SPIR64: store target("spirv.Pipe", 0) %test, ptr %test.addr, align 8
 // SPIR64: store i32 8, ptr %s, align 4
 // AMDGCN: store ptr addrspace(1) %test, ptr addrspace(5) %test.addr, align 8
 // AMDGCN: store i32 8, ptr addrspace(5) %s, align 4
Index: clang/test/CodeGenOpenCL/sampler.cl
===
--- clang/test/CodeGenOpenCL/sampler.cl
+++ clang/test/CodeGenOpenCL/sampler.cl
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -no-opaque-pointers %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers %s -cl-std=clc++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=CHECK-COMMON,CHECK-SPIR %s
+// RUN: %clang_cc1 %s -cl-std=clc++ -emit-llvm -triple spir-unknown-unknown -o - -O0 | FileCheck --check-prefixes=

[PATCH] D141008: [Clang][SPIR-V] Emit target extension types for OpenCL types on SPIR-V.

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/test/CodeGenOpenCL/opencl_types.cl:2
 // RUN: %clang_cc1 -no-opaque-pointers -cl-std=CL2.0 %s -triple 
"spir-unknown-unknown" -emit-llvm -o - -O0 | FileCheck %s 
--check-prefixes=CHECK-COM,CHECK-SPIR
 // RUN: %clang_cc1 -no-opaque-pointers -cl-std=CL2.0 %s -triple 
"amdgcn--amdhsa" -emit-llvm -o - -O0 | FileCheck %s 
--check-prefixes=CHECK-COM,CHECK-AMDGCN
 

yaxunl wrote:
> need a non-spir target
This file checks AMD already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141008

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

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added a comment.

I don't normally handle name mangling, so I can't comment too much here, but I 
will note that Itanium ABI is planning on using DF16b for `std::bfloat16_t`: 
https://github.com/itanium-cxx-abi/cxx-abi/pull/147


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139608

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