[PATCH] D126138: [clang-tidy] Fix #55134 (regression introduced by 5da7c04)

2022-05-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.

I'm not a clang-tidy maintainer so I don't know if I have the authority to 
green-light this patch, but I'm fairly confident the fix is correct (it's 
restoring the previous usage of the SourceManager API in this loop prior to 
this refactor 
),
 and the tests capture the changed behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126138

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


[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

2022-05-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.
Herald added a project: All.

@iannisdezwart Hi :) Do you plan to continue to work on this? It seems like 
we're most of the way there!


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

https://reviews.llvm.org/D119077

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


[PATCH] D126274: [clangd] Handle '--' in QueryDriverDatabase

2022-05-23 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/1100,
a regression from D116721 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126274

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -50,6 +50,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -238,10 +239,17 @@
 tooling::CompileCommand &
 addSystemIncludes(tooling::CompileCommand &Cmd,
   llvm::ArrayRef SystemIncludes) {
+  std::vector ToAppend;
   for (llvm::StringRef Include : SystemIncludes) {
 // FIXME(kadircet): This doesn't work when we have "--driver-mode=cl"
-Cmd.CommandLine.push_back("-isystem");
-Cmd.CommandLine.push_back(Include.str());
+ToAppend.push_back("-isystem");
+ToAppend.push_back(Include.str());
+  }
+  if (!ToAppend.empty()) {
+// Just append when `--` isn't present.
+auto InsertAt = llvm::find(Cmd.CommandLine, "--");
+Cmd.CommandLine.insert(InsertAt, std::make_move_iterator(ToAppend.begin()),
+   std::make_move_iterator(ToAppend.end()));
   }
   return Cmd;
 }
@@ -254,7 +262,9 @@
   if (Arg == "-target" || Arg.startswith("--target="))
 return Cmd;
 }
-Cmd.CommandLine.push_back("--target=" + Target);
+// Just append when `--` isn't present.
+auto InsertAt = llvm::find(Cmd.CommandLine, "--");
+Cmd.CommandLine.insert(InsertAt, "--target=" + Target);
   }
   return Cmd;
 }


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -50,6 +50,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -238,10 +239,17 @@
 tooling::CompileCommand &
 addSystemIncludes(tooling::CompileCommand &Cmd,
   llvm::ArrayRef SystemIncludes) {
+  std::vector ToAppend;
   for (llvm::StringRef Include : SystemIncludes) {
 // FIXME(kadircet): This doesn't work when we have "--driver-mode=cl"
-Cmd.CommandLine.push_back("-isystem");
-Cmd.CommandLine.push_back(Include.str());
+ToAppend.push_back("-isystem");
+ToAppend.push_back(Include.str());
+  }
+  if (!ToAppend.empty()) {
+// Just append when `--` isn't present.
+auto InsertAt = llvm::find(Cmd.CommandLine, "--");
+Cmd.CommandLine.insert(InsertAt, std::make_move_iterator(ToAppend.begin()),
+   std::make_move_iterator(ToAppend.end()));
   }
   return Cmd;
 }
@@ -254,7 +262,9 @@
   if (Arg == "-target" || Arg.startswith("--target="))
 return Cmd;
 }
-Cmd.CommandLine.push_back("--target=" + Target);
+// Just append when `--` isn't present.
+auto InsertAt = llvm::find(Cmd.CommandLine, "--");
+Cmd.CommandLine.insert(InsertAt, "--target=" + Target);
   }
   return Cmd;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

2022-05-23 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 431589.
royjacobson added a comment.

Update the approach to use an AST property, and enable this for all language 
variants (not just CPP20).

There's still one test failing because OpenCL have got their own destructor 
thing going, I'll try to
see what I can do about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126194

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/class/class.dtor/p4.cpp
  clang/test/CXX/over/over.match/over.match.viable/p3.cpp

Index: clang/test/CXX/over/over.match/over.match.viable/p3.cpp
===
--- clang/test/CXX/over/over.match/over.match.viable/p3.cpp
+++ clang/test/CXX/over/over.match/over.match.viable/p3.cpp
@@ -49,7 +49,6 @@
   S(A) requires false;
   S(double) requires true;
   ~S() requires false;
-  // expected-note@-1 2{{because 'false' evaluated to false}}
   ~S() requires true;
   operator int() requires true;
   operator int() requires false;
@@ -58,11 +57,7 @@
 void bar() {
   WrapsStatics::foo(A{});
   S{1.}.foo(A{});
-  // expected-error@-1{{invalid reference to function '~S': constraints not satisfied}}
-  // Note - this behavior w.r.t. constrained dtors is a consequence of current
-  // wording, which does not invoke overload resolution when a dtor is called.
-  // P0848 is set to address this issue.
+
   S s = 1;
-  // expected-error@-1{{invalid reference to function '~S': constraints not satisfied}}
   int a = s;
 }
Index: clang/test/CXX/class/class.dtor/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.dtor/p4.cpp
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template 
+struct A {
+  ~A() = delete; // expected-note {{explicitly marked deleted}}
+  ~A() requires(N == 1) = delete; // expected-note {{explicitly marked deleted}}
+};
+
+template 
+struct B {
+  ~B() requires(N == 1) = delete; // expected-note {{explicitly marked deleted}}
+  virtual ~B() = delete; // expected-note {{explicitly marked deleted}}
+};
+
+template
+concept CO1 = N == 1;
+
+template
+concept CO2 = N > 0;
+
+template 
+struct C {
+  ~C() = delete; // expected-note {{explicitly marked deleted}}
+  ~C() requires(CO1) = delete;
+  ~C() requires(CO1 && CO2) = delete; // expected-note {{explicitly marked deleted}}
+};
+
+template 
+struct D {
+  ~D() requires(N != 0) = delete; // expected-note {{explicitly marked deleted}}
+  // expected-note@-1 {{candidate function has been explicitly deleted}}
+  // expected-note@-2 {{candidate function not viable: constraints not satisfied}}
+  // expected-note@-3 {{evaluated to false}}
+  ~D() requires(N == 1) = delete;
+  // expected-note@-1 {{candidate function has been explicitly deleted}}
+  // expected-note@-2 {{candidate function not viable: constraints not satisfied}}
+  // expected-note@-3 {{evaluated to false}}
+};
+
+template 
+concept Foo = requires (T t) {
+  { t.foo() };
+};
+
+template
+struct E {
+  void foo();
+  ~E();
+  ~E() requires Foo = delete;  // expected-note {{explicitly marked deleted}}
+};
+
+template struct A<1>;
+template struct A<2>;
+template struct B<1>;
+template struct B<2>;
+template struct C<1>;
+template struct C<2>;
+template struct D<0>;  // expected-error {{no viable destructor found for class 'D<0>'}} expected-note {{in instantiation of template}}
+template struct D<1>;  // expected-error {{destructor of class 'D<1>' is ambiguous}} expected-note {{in instantiation of template}}
+template struct D<2>;
+template struct E<1>;
+
+int main() {
+  A<1> a1; // expected-error {{attempt to use a deleted function}}
+  A<2> a2; // expected-error {{attempt to use a deleted function}}
+  B<1> b1; // expected-error {{attempt to use a deleted function}}
+  B<2> b2; // expected-error {{attempt to use a deleted function}}
+  C<1> c1; // expected-error {{attempt to use a deleted function}}
+  C<2> c2; // expected-error {{attempt to use a deleted function}}
+  D<0> d0;
+  D<1> d1;
+  D<2> d2; // expected-error {{attempt to use a deleted function}}
+  E<1> e1; // expected-error {{attempt to use a deleted function}}
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17755,6 +17755,62 @@
   AllIvarDecls.push_back(Ivar);
 }
 
+namespace {
+/// [class.dtor]p4:
+///   At the end of the definition of a class, overload resolution is
+///   performed among the prospective destructors declared in that class with
+///   an empty argument list to select the destructor for the class, also
+///   known as the selected destructor.
+///
+/// We do th

[PATCH] D126266: Mark the file entry invalid, until reread. Invalidate SLocEntry cache, readd it on reread. Do not use translateFile, because it pulls in parts of the pch.

2022-05-23 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a subscriber: cfe-commits.
v.g.vassilev added reviewers: rsmith, vsapsai.
v.g.vassilev added a comment.

Just to add that the `invalidateCache` is important for cling and clang-repl 
where we do something like:

clang-repl> #include "file_with_error.h"
// error is printed, we edit the file and include it again:
clang-repl> #include "file_with_error.h"

I am not sure if we can write such a test for clang-repl easily using the lit 
infrastructure...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126266

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


[PATCH] D126247: `readability-indentifier-naming` resolution order and examples

2022-05-23 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2742
 double   dValueDouble = 0.0;
 ULONGulValueUlong = 0;

Phabricator says there is no context available.  Did you generate this diff 
with `git diff -U99 main`?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2750
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can falling into several classifications. Bellow
+is a list of the classifications supported by ``readability-identifier-naming``

`fall`, not `falling`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2750
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can falling into several classifications. Bellow
+is a list of the classifications supported by ``readability-identifier-naming``

LegalizeAdulthood wrote:
> `fall`, not `falling`
`Below`, not `Bellow`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2756
+matched. This occurs when the semantics of the identifier match and there is a
+valid option for the classification present in the ``.clang-tidy`` file - e.g.,
+``readability-identifier-naming.AbstractClassCase``.

Does it have to be in the `.clang-tidy` file, or is it just a matter of setting 
options?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2772
+   - Class ``[class, struct]``
+   - Struct ``[class]``
+   - Union ``[union]``

Why is `Struct`  listed twice?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2786
+ - 
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``

I would have expected `ConstExprVariable` instead?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2857
+attempt disambiguation within the context of this document. For example,
+ identifiers that clang tidy is currently looking only at
+member variables.

Eugene.Zelenko wrote:
> Ditto.
`identifies`, not `identifiers`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2873
+classify the identifier in any other context. For example, in the 
+ semantic context, clang tidy will abort matching after failing
+to resolve the ``Parameter`` classification and a parameter will *not* be

`parameters` not `paramaters`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2873
+classify the identifier in any other context. For example, in the 
+ semantic context, clang tidy will abort matching after failing
+to resolve the ``Parameter`` classification and a parameter will *not* be

LegalizeAdulthood wrote:
> `parameters` not `paramaters`
Clang-tidy, not clang tidy



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2880
+
+The code snippet below[1]_ serves as an exhaustive example of various
+identifiers the ``readability-identifier-naming`` check is able to classify.

Should we be linking to ephemeral gists that can disappear?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3054
+// @param param Parameter
+int func(std::string *const str_ptr, const std::string &str, int 
*ptr_param, int param)
+{

This covers "const pointer", but what about "pointer to const"?
e.g. `std::string const *str_ptr` how is this handled?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3120
+// GlobalConstantPointer, GlobalConstant, Constant, GlobalPointer, 
GlobalVariable, Variable
+int *const global_const_ptr = nullptr;
+

Same, pointer to `const`?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3127-3129
+// StaticConstant does not actually trip for this declaration despite the 
documentation indicating that it should.
+// StaticConstant does not appear to trip for anything. Reading the code, 
it seems that StaticConstant logic is in the
+// wrong place and the conditions cannot be met.

Is this really a bug report disguised as a doc comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126247

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/

[PATCH] D126246: bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
usama54321 created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
This revision was not accepted when it landed; it landed in state "Draft".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG63ecb7dcc80d: bugfix in InfiniteLoopCheck to not print 
warnings for unevaluated loops (authored by usama54321).
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D126246?vs=431496&id=431572#toc

Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to 
use new check


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126246

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,36 +194,44 @@
   return nullptr;
 }
 
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
+  hasAncestor(expr(anyOf(
+  // `UnaryExprOrTypeTraitExpr` is unevaluated
+  // unless it's sizeof on VLA.
+  unaryExprOrTypeTraitExpr(
+  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
+  // `CXXTypeidExpr` is unevaluated unless it's
+  // applied to an expression of glvalue of
+  // polymorphic class type.
+  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+  // The controlling expression of
+  // `GenericSelectionExpr` is unevaluated.
+  genericSelectionExpr(
+  hasControllingExpr(hasDescendant(equalsNode(Exp,
+  cxxNoexceptExpr();
+}
+
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
  ASTContext &Context) {
-  return selectFirst(
+  return selectFirst(NodeID::value,
+   match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+  isUnevaluatedMatcher(Exp))
+ .bind(NodeID::value)),
+ Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(
  NodeID::value,
- match(
- findAll(
- expr(canResolveToExpr(equalsNode(Exp)),
-  anyOf(
-  // `Exp` is part of the underlying expression of
-  // decltype/typeof if it has an ancestor of
-  // typeLoc.
-  hasAncestor(typeLoc(unless(
-  hasAncestor(unaryExprOrTypeTraitExpr(),
-  hasAncestor(expr(anyOf(
-  // `UnaryExprOrTypeTraitExpr` is unevaluated
-  // unless it's sizeof on VLA.
-  unaryExprOrTypeTraitExpr(unless(sizeOfExpr(
-  hasArgumentOfType(variableArrayType(),
-  // `CXXTypeidExpr` is unevaluated unless it's
-  // applied to an expression of glvalue of
-  // polymorphic class type.
-  cxxTypeidExpr(
-  unless(isPotentiallyEvaluated())),
-  // The controlling expression of
-  // `GenericSelectionExpr` is unevaluated.
-  genericSelectionExpr(hasControllingExpr(
-  hasDescendant(equalsNode(Exp,
-  cxxNoexceptExpr())
- .bind(NodeID::value)),
- Stm, Context)) != nullptr;
+ match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
+   .bind(NodeID::value)),
+   Stm, Context)) != nullptr;
 }
 
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@
   }
   const Stmt *findPointe

[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
usama54321 marked an inline comment as done.
Closed by commit rG602682225ad6: bugfix in InfiniteLoopCheck to not print 
warnings for unevaluated loops (authored by usama54321).

Changed prior to commit:
  https://reviews.llvm.org/D126034?vs=431516&id=431571#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,7 +194,8 @@
   return nullptr;
 }
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ ASTContext &Context) {
   return selectFirst(
  NodeID::value,
  match(
@@ -225,6 +226,10 @@
  Stm, Context)) != nullptr;
 }
 
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}
+
 const Stmt *
 ExprMutationAnalyzer::findExprMutation(ArrayRef Matches) {
   return tryEachMatch(Matches, this, &ExprMutationAnalyzer::findMutation);
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ASTContext &Context);
 
 private:
   using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -650,3 +650,38 @@
   do {
   } while (1, (false) && val4 == 1);
 }
+
+void test_typeof() {
+  __typeof__({
+for (int i = 0; i < 10; ++i) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_infinite() {
+  __typeof__({
+for (int i = 0; i < 10;) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_while_infinite() {
+  __typeof__({
+int i = 0;
+while (i < 10) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_dowhile_infinite() {
+  __typeof__({
+int i = 0;
+do {
+
+} while (i < 10);
+0;
+  }) x;
+}
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -81,6 +81,22 @@
   return false;
 }
 
+bool isUnevaluated(const Decl *Func, const Stmt *LoopStmt, const Stmt *Cond,
+   ASTContext *Context) {
+  if (const auto *Exp = dyn_cast(Cond)) {
+if (const auto *ForLoop = dyn_cast(LoopStmt)) {
+  return (ForLoop->getInc() && ExprMutationAnalyzer::isUnevaluated(
+   Exp, *ForLoop->getInc(), *Context)) ||
+ (ForLoop->getBody() && ExprMutationAnalyzer::isUnevaluated(
+Exp, *ForLoop->getBody(), *Context)) ||
+ (ForLoop->getCond() && ExprMutationAnalyzer::isUnevaluated(
+Exp, *ForLoop->getCond(), *Context));
+}
+return ExprMutationAnalyzer::isUnevaluated(Exp, *LoopStmt, *Context);
+  }
+  return true;
+}
+
 /// Return whether at least one variable of `Cond` changed in `LoopStmt`.
 static bool isAtLeastOneCondVarChanged(const Decl *Func, const Stmt *LoopStmt,
const Stmt *Cond, ASTContext *Context) {
@@ -177,6 +193,9 @@
 }
   }
 
+  if (isUnevaluated(Func, LoopStmt, Cond, Result.Context))
+return;
+
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ca81abc - updated canResolveToExpr to accept both statements and expressions. Removed unnecessary code

2022-05-23 Thread usama hameed via cfe-commits

Author: usama hameed
Date: 2022-05-23T20:18:49-07:00
New Revision: ca81abcfd752e65c533825a5fadac19ce5a33578

URL: 
https://github.com/llvm/llvm-project/commit/ca81abcfd752e65c533825a5fadac19ce5a33578
DIFF: 
https://github.com/llvm/llvm-project/commit/ca81abcfd752e65c533825a5fadac19ce5a33578.diff

LOG: updated canResolveToExpr to accept both statements and expressions. 
Removed unnecessary code

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
clang/lib/Analysis/ExprMutationAnalyzer.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h 
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index a48b9735dfee..1ceef944fbc3 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -40,8 +40,6 @@ class ExprMutationAnalyzer {
   const Stmt *findPointeeMutation(const Decl *Dec);
   static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
 ASTContext &Context);
-  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
-ASTContext &Context);
 
 private:
   using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);

diff  --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp 
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 135327a3cfe5..e3bb902b1fe9 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -39,8 +39,13 @@ AST_MATCHER_P(Expr, maybeEvalCommaExpr, 
ast_matchers::internal::Matcher,
   return InnerMatcher.matches(*Result, Finder, Builder);
 }
 
-AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher,
+AST_MATCHER_P(Stmt, canResolveToExpr, ast_matchers::internal::Matcher,
   InnerMatcher) {
+  auto *Exp = dyn_cast(&Node);
+  if (!Exp) {
+return stmt().matches(Node, Finder, Builder);
+  }
+
   auto DerivedToBase = [](const ast_matchers::internal::Matcher &Inner) {
 return implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase),
   hasCastKind(CK_UncheckedDerivedToBase)),
@@ -71,7 +76,7 @@ AST_MATCHER_P(Expr, canResolveToExpr, 
ast_matchers::internal::Matcher,
  IgnoreDerivedToBase(ConditionalOperator),
  IgnoreDerivedToBase(ElvisOperator;
 
-  return ComplexMatcher.matches(Node, Finder, Builder);
+  return ComplexMatcher.matches(*Exp, Finder, Builder);
 }
 
 // Similar to 'hasAnyArgument', but does not work because 'InitListExpr' does
@@ -194,44 +199,36 @@ const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const 
Decl *Dec,
   return nullptr;
 }
 
-auto isUnevaluatedMatcher(const Stmt *Exp) {
-  return anyOf(
-  // `Exp` is part of the underlying expression of
-  // decltype/typeof if it has an ancestor of
-  // typeLoc.
-  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
-  hasAncestor(expr(anyOf(
-  // `UnaryExprOrTypeTraitExpr` is unevaluated
-  // unless it's sizeof on VLA.
-  unaryExprOrTypeTraitExpr(
-  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
-  // `CXXTypeidExpr` is unevaluated unless it's
-  // applied to an expression of glvalue of
-  // polymorphic class type.
-  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
-  // The controlling expression of
-  // `GenericSelectionExpr` is unevaluated.
-  genericSelectionExpr(
-  hasControllingExpr(hasDescendant(equalsNode(Exp,
-  cxxNoexceptExpr();
-}
-
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
- ASTContext &Context) {
-  return selectFirst(NodeID::value,
-   
match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
-  isUnevaluatedMatcher(Exp))
- .bind(NodeID::value)),
- Stm, Context)) != nullptr;
-}
-
 bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
  ASTContext &Context) {
   return selectFirst(
  NodeID::value,
- match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
-   .bind(NodeID::value)),
-   Stm, Context)) != nullptr;
+ match(
+ findAll(
+ stmt(canResolveToExpr(equalsNode(Exp)),
+  anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(
+   

[clang] 63ecb7d - bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread usama hameed via cfe-commits

Author: usama hameed
Date: 2022-05-23T20:18:49-07:00
New Revision: 63ecb7dcc80d17770461c8bf01bddeb2b795625b

URL: 
https://github.com/llvm/llvm-project/commit/63ecb7dcc80d17770461c8bf01bddeb2b795625b
DIFF: 
https://github.com/llvm/llvm-project/commit/63ecb7dcc80d17770461c8bf01bddeb2b795625b.diff

LOG: bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to 
use new check

Differential Revision: https://reviews.llvm.org/D126246

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
clang/lib/Analysis/ExprMutationAnalyzer.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index cb9bd7bb43f50..8815de220882e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -81,22 +81,6 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, 
const Stmt *LoopStmt,
   return false;
 }
 
-bool isUnevaluated(const Decl *Func, const Stmt *LoopStmt, const Stmt *Cond,
-   ASTContext *Context) {
-  if (const auto *Exp = dyn_cast(Cond)) {
-if (const auto *ForLoop = dyn_cast(LoopStmt)) {
-  return (ForLoop->getInc() && ExprMutationAnalyzer::isUnevaluated(
-   Exp, *ForLoop->getInc(), *Context)) ||
- (ForLoop->getBody() && ExprMutationAnalyzer::isUnevaluated(
-Exp, *ForLoop->getBody(), *Context)) ||
- (ForLoop->getCond() && ExprMutationAnalyzer::isUnevaluated(
-Exp, *ForLoop->getCond(), *Context));
-}
-return ExprMutationAnalyzer::isUnevaluated(Exp, *LoopStmt, *Context);
-  }
-  return true;
-}
-
 /// Return whether at least one variable of `Cond` changed in `LoopStmt`.
 static bool isAtLeastOneCondVarChanged(const Decl *Func, const Stmt *LoopStmt,
const Stmt *Cond, ASTContext *Context) {
@@ -193,7 +177,7 @@ void InfiniteLoopCheck::check(const 
MatchFinder::MatchResult &Result) {
 }
   }
 
-  if (isUnevaluated(Func, LoopStmt, Cond, Result.Context))
+  if (ExprMutationAnalyzer::isUnevaluated(LoopStmt, *LoopStmt, 
*Result.Context))
 return;
 
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))

diff  --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h 
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index 1b2b7ffcfb67a..a48b9735dfee5 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@ class ExprMutationAnalyzer {
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+ASTContext &Context);
   static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
 ASTContext &Context);
 

diff  --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp 
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 4d7decc1137c2..135327a3cfe54 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,36 +194,44 @@ const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const 
Decl *Dec,
   return nullptr;
 }
 
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
+  hasAncestor(expr(anyOf(
+  // `UnaryExprOrTypeTraitExpr` is unevaluated
+  // unless it's sizeof on VLA.
+  unaryExprOrTypeTraitExpr(
+  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
+  // `CXXTypeidExpr` is unevaluated unless it's
+  // applied to an expression of glvalue of
+  // polymorphic class type.
+  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+  // The controlling expression of
+  // `GenericSelectionExpr` is unevaluated.
+  genericSelectionExpr(
+  hasControllingExpr(hasDescendant(equalsNode(Exp,
+  cxxNoexceptExpr();
+}
+
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
  ASTContext &Context) {
-  return selectFirst(
+  return selectFirst(NodeID::value,
+   
match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+  isUnevaluatedMatcher(Exp))
+

[clang] 6026822 - bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread usama hameed via cfe-commits

Author: usama hameed
Date: 2022-05-23T20:18:48-07:00
New Revision: 602682225ad6c9135e84bbca3b91d5738712c64f

URL: 
https://github.com/llvm/llvm-project/commit/602682225ad6c9135e84bbca3b91d5738712c64f
DIFF: 
https://github.com/llvm/llvm-project/commit/602682225ad6c9135e84bbca3b91d5738712c64f.diff

LOG: bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

Differential Revision: https://reviews.llvm.org/D126034

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
clang/lib/Analysis/ExprMutationAnalyzer.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 3359a7f8932ed..cb9bd7bb43f50 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -81,6 +81,22 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, 
const Stmt *LoopStmt,
   return false;
 }
 
+bool isUnevaluated(const Decl *Func, const Stmt *LoopStmt, const Stmt *Cond,
+   ASTContext *Context) {
+  if (const auto *Exp = dyn_cast(Cond)) {
+if (const auto *ForLoop = dyn_cast(LoopStmt)) {
+  return (ForLoop->getInc() && ExprMutationAnalyzer::isUnevaluated(
+   Exp, *ForLoop->getInc(), *Context)) ||
+ (ForLoop->getBody() && ExprMutationAnalyzer::isUnevaluated(
+Exp, *ForLoop->getBody(), *Context)) ||
+ (ForLoop->getCond() && ExprMutationAnalyzer::isUnevaluated(
+Exp, *ForLoop->getCond(), *Context));
+}
+return ExprMutationAnalyzer::isUnevaluated(Exp, *LoopStmt, *Context);
+  }
+  return true;
+}
+
 /// Return whether at least one variable of `Cond` changed in `LoopStmt`.
 static bool isAtLeastOneCondVarChanged(const Decl *Func, const Stmt *LoopStmt,
const Stmt *Cond, ASTContext *Context) {
@@ -177,6 +193,9 @@ void InfiniteLoopCheck::check(const 
MatchFinder::MatchResult &Result) {
 }
   }
 
+  if (isUnevaluated(Func, LoopStmt, Cond, Result.Context))
+return;
+
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))
 return;
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
index 0074266ce4b87..a22f1bf06a534 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -650,3 +650,38 @@ void test_dependent_condition() {
   do {
   } while (1, (false) && val4 == 1);
 }
+
+void test_typeof() {
+  __typeof__({
+for (int i = 0; i < 10; ++i) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_infinite() {
+  __typeof__({
+for (int i = 0; i < 10;) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_while_infinite() {
+  __typeof__({
+int i = 0;
+while (i < 10) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_dowhile_infinite() {
+  __typeof__({
+int i = 0;
+do {
+
+} while (i < 10);
+0;
+  }) x;
+}

diff  --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h 
b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
index 9397c5df78abe..1b2b7ffcfb67a 100644
--- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@ class ExprMutationAnalyzer {
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ASTContext &Context);
 
 private:
   using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);

diff  --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp 
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index e9ff5e5e87658..4d7decc1137c2 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,7 +194,8 @@ const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl 
*Dec,
   return nullptr;
 }
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ ASTContext &Context) {
   return selectFirst(
  NodeID::value,
  match(
@@ -225,6 +226,10 @@ bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
  Stm, Context)) != nullptr;
 }
 
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}

[PATCH] D126192: [Driver] Support linking with lld for target AVR

2022-05-23 Thread Ben Shi via Phabricator via cfe-commits
benshi001 updated this revision to Diff 431565.

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

https://reviews.llvm.org/D126192

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld
  clang/test/Driver/Inputs/basic_avr_tree/usr/lib/avr/lib/ldscripts/avr5.xn
  clang/test/Driver/avr-toolchain.c

Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -53,8 +53,16 @@
 // LINKA-NOT: warning: {{.*}} data section address
 
 // RUN: %clang -### --target=avr --sysroot=%S/Inputs/ -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefixes=NOGCC %s
-// NOGCC: warning: no avr-gcc installation can be found on the system, cannot link standard libraries
+// NOGCC: error: invalid linker
 // NOGCC: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
 // NOGCC-NOT: warning: {{.*}} microcontroller
 // NOGCC-NOT: warning: {{.*}} avr-libc
 // NOGCC-NOT: warning: {{.*}} data section address
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -fuse-ld=avrld 2>&1 | FileCheck --check-prefix=NOLD %s
+// NOLD: error: invalid linker
+
+// RUN: %clang -### --target=avr --sysroot=%S/Inputs/basic_avr_tree -mmcu=atmega328 %s -fuse-ld=ld.lld 2>&1 | FileCheck --check-prefix=LLD %s
+// LLD: {{".*lld"}} {{.*}} "-e" "__vectors" "-T" {{".*avr5.x"}}
+// LLD-NOT: "avr-ld"
+// LLD-NOT: "-mavr5"
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -429,9 +429,20 @@
   // Compute information about the target AVR.
   std::string CPU = getCPUName(D, Args, getToolChain().getTriple());
   llvm::Optional FamilyName = GetMCUFamilyName(CPU);
+  llvm::Optional AVRLibcRoot = TC.findAVRLibcInstallation();
   llvm::Optional SectionAddressData = GetMCUSectionAddressData(CPU);
 
-  std::string Linker = getToolChain().GetProgramPath(getShortName());
+  // Compute the linker program path, and use GNU "avr-ld" as default.
+  const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
+  std::string Linker(A ? A->getValue() : getShortName());
+  if (!llvm::sys::fs::can_execute(Linker)) {
+std::string FullPath = getToolChain().GetProgramPath(Linker.c_str());
+if (!llvm::sys::fs::can_execute(FullPath))
+  D.Diag(diag::err_drv_invalid_linker_name) << Linker;
+else
+  Linker = FullPath;
+  }
+
   ArgStringList CmdArgs;
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
@@ -450,17 +461,11 @@
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
 if (!CPU.empty()) {
-Optional FamilyName = GetMCUFamilyName(CPU);
-Optional AVRLibcRoot = TC.findAVRLibcInstallation();
-
   if (!FamilyName) {
 // We do not have an entry for this CPU in the family
 // mapping table yet.
 D.Diag(diag::warn_drv_avr_family_linking_stdlibs_not_implemented)
 << CPU;
-  } else if (TC.getGCCInstallPath().empty()) {
-// We can not link since there is no avr-ld.
-D.Diag(diag::warn_drv_avr_gcc_not_found);
   } else if (!AVRLibcRoot) {
 // No avr-libc found and so no runtime linked.
 D.Diag(diag::warn_drv_avr_libc_not_found);
@@ -473,7 +478,6 @@
 LinkStdlib = true;
   }
 }
-
 if (!LinkStdlib)
   D.Diag(diag::warn_drv_avr_stdlib_not_linked);
   }
@@ -508,11 +512,28 @@
 
 CmdArgs.push_back("--end-group");
 
-// Specify the family name as the emulation mode to use.
-// This is almost always required because otherwise avr-ld
-// will assume 'avr2' and warn about the program being larger
-// than the bare minimum supports.
-CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+// Add specific options for different linkers.
+if (Linker.find("avr-ld") != std::string::npos) {
+  // Specify the family name as the emulation mode to use.
+  // This is almost always required because otherwise avr-ld
+  // will assume 'avr2' and warn about the program being larger
+  // than the bare minimum supports.
+  CmdArgs.push_back(Args.MakeArgString(std::string("-m") + *FamilyName));
+} else if (Linker.find("ld.lld") != std::string::npos) {
+  // We must explicitly specify `__vectors` as the entry for lld.
+  CmdArgs.push_back(Args.MakeArgString("-e"));
+  CmdArgs.push_back(Args.MakeArgString("__vectors"));
+  // We must explicitly spefify the linker script (for lld), which has
+  // already been integrated into avr-libc, and whose full path is
+  // expected to be $AVR-LIBC/lib/ldscripts/$FamilyName.x .
+  if (AVRLibcRoot && FamilyName) {
+std::string Prefix(*AVRLibcRoot + "/lib/ldscripts/");

[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:1293
   // that just takes the requested size.
-
-  FunctionDecl *OperatorNew = nullptr;
-  FunctionDecl *OperatorDelete = nullptr;
-  FunctionDecl *UnusedResult = nullptr;
-  bool PassAlignment = false;
-  SmallVector PlacementArgs;
-
+  //
   // [dcl.fct.def.coroutine]p9

erichkeane wrote:
> Extra comment line.
Oh, this is intended. I feel the format looks better with the blank line.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:1355
+  // We don't expect to call to global operator new with (size, p0, …, pn).
+  if (PromiseContainNew && !collectPlacementArgs(S, FD, Loc, PlacementArgs))
+return false;

erichkeane wrote:
> Can you explain how this works?  I'm not seeing what part of the collection 
> of placement args would prohibit the call you're talking about.
The new version of CWG issue 2585 says: "We shouldn't generate an allocation 
call in global scope with (std::size_t, p0, ..., pn)". Also it says that we 
wouldn't lookup into global scope if we find `operator new` in promise_type. It 
implies that we should only generate an allocation call in promise_type scope   
with (std::size_t, p0, ..., pn). So we should only collect placement arguments 
if we saw `operator new` in promise_type scope.

In other words, if we don't add the check, it would collect placement arguments 
all the way so it would be possible to generate an allocation call in global 
scope with (std::size_t, p0, ..., pn), which violates the update of CWG issue 
2585.



Comment at: clang/test/SemaCXX/coroutine-allocs2.cpp:2
+// Tests that we wouldn't generate an allocation call in global scope with 
(std::size_t, p0, ..., pn)
+// Although this test generates codes, it aims to test the semantics. So it is 
put here.
+// RUN: %clang_cc1 %s -std=c++20 -S -triple x86_64-unknown-linux-gnu 
-emit-llvm -disable-llvm-passes %s -o - | FileCheck %s

erichkeane wrote:
> I'm not a fan at all of having a FIleCheck-type-test in Sema, and this DOES 
> validate semantics.  Is there any way to get this to emit an error instead?  
> Perhaps declare the generated operator-new as 'deleted' and show that it 
> chooses THAT one instead by an error?
I tried to mark the allocation function as delete. But it doesn't work... And 
FileCheck is the only way I imaged. Another possible way might be to use 
FileCheck for dumped AST. But I feel it is worse since the ABI is more stable 
than the AST. (The format of AST is not guaranteed stable I thought.)


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

https://reviews.llvm.org/D126187

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


[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 431558.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D126187

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutine-allocs2.cpp

Index: clang/test/SemaCXX/coroutine-allocs2.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine-allocs2.cpp
@@ -0,0 +1,31 @@
+// Tests that we wouldn't generate an allocation call in global scope with (std::size_t, p0, ..., pn)
+// Although this test generates code, it aims to test the semantics. So it is put here.
+// RUN: %clang_cc1 %s -std=c++20 -S -triple x86_64-unknown-linux-gnu -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+#include "Inputs/std-coroutine.h"
+
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+}
+
+struct Allocator {};
+
+struct resumable {
+  struct promise_type {
+
+resumable get_return_object() { return {}; }
+auto initial_suspend() { return std::suspend_always(); }
+auto final_suspend() noexcept { return std::suspend_always(); }
+void unhandled_exception() {}
+void return_void(){};
+  };
+};
+
+void *operator new(std::size_t, void *);
+
+resumable f1(void *) {
+  co_return;
+}
+
+// CHECK: coro.alloc:
+// CHECK-NEXT: [[SIZE:%.+]] = call [[BITWIDTH:.+]] @llvm.coro.size.[[BITWIDTH]]()
+// CHECK-NEXT: call {{.*}} ptr @_Znwm([[BITWIDTH]] noundef [[SIZE]])
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1239,6 +1239,41 @@
   return true;
 }
 
+// Collect placement arguments for allocation function of coroutine FD.
+// Return true if we collect placement arguments succesfully. Return false,
+// otherwise.
+static bool collectPlacementArgs(Sema &S, FunctionDecl &FD, SourceLocation Loc,
+ SmallVectorImpl &PlacementArgs) {
+  if (auto *MD = dyn_cast(&FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+  ExprResult ThisExpr = S.ActOnCXXThis(Loc);
+  if (ThisExpr.isInvalid())
+return false;
+  ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
+  if (ThisExpr.isInvalid())
+return false;
+  PlacementArgs.push_back(ThisExpr.get());
+}
+  }
+
+  for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
+  continue;
+
+// Build a reference to the parameter.
+auto PDLoc = PD->getLocation();
+ExprResult PDRefExpr =
+S.BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
+   ExprValueKind::VK_LValue, PDLoc);
+if (PDRefExpr.isInvalid())
+  return false;
+
+PlacementArgs.push_back(PDRefExpr.get());
+  }
+
+  return true;
+}
+
 bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
   // Form and check allocation and deallocation calls.
   assert(!IsPromiseDependentType &&
@@ -1255,13 +1290,7 @@
   // allocated, followed by the coroutine function's arguments. If a matching
   // allocation function exists, use it. Otherwise, use an allocation function
   // that just takes the requested size.
-
-  FunctionDecl *OperatorNew = nullptr;
-  FunctionDecl *OperatorDelete = nullptr;
-  FunctionDecl *UnusedResult = nullptr;
-  bool PassAlignment = false;
-  SmallVector PlacementArgs;
-
+  //
   // [dcl.fct.def.coroutine]p9
   //   An implementation may need to allocate additional storage for a
   //   coroutine.
@@ -1288,31 +1317,12 @@
   // and p_i denotes the i-th function parameter otherwise. For a non-static
   // member function, q_1 is an lvalue that denotes *this; any other q_i is an
   // lvalue that denotes the parameter copy corresponding to p_i.
-  if (auto *MD = dyn_cast(&FD)) {
-if (MD->isInstance() && !isLambdaCallOperator(MD)) {
-  ExprResult ThisExpr = S.ActOnCXXThis(Loc);
-  if (ThisExpr.isInvalid())
-return false;
-  ThisExpr = S.CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
-  if (ThisExpr.isInvalid())
-return false;
-  PlacementArgs.push_back(ThisExpr.get());
-}
-  }
-  for (auto *PD : FD.parameters()) {
-if (PD->getType()->isDependentType())
-  continue;
 
-// Build a reference to the parameter.
-auto PDLoc = PD->getLocation();
-ExprResult PDRefExpr =
-S.BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
-   ExprValueKind::VK_LValue, PDLoc);
-if (PDRefExpr.isInvalid())
-  return false;
-
-PlacementArgs.push_back(PDRefExpr.get());
-  }
+  FunctionDecl *OperatorNew = nullptr;
+  FunctionDecl *OperatorDelete = nullptr;
+  FunctionDecl *UnusedResult = nullptr;
+  bool PassAlignment = false;
+  SmallVector PlacementArgs;
 
   bool PromiseContainNew = [this, &PromiseType]() -> bool {
 DeclarationName NewName =
@@ -1330,8 +1340,10 @@

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

2022-05-23 Thread Ryan Prichard via Phabricator via cfe-commits
rprichard added subscribers: craig.topper, rprichard.
rprichard added a comment.
Herald added a project: All.

Maybe this change is obsolete now that D59566  
is merged?


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

https://reviews.llvm.org/D29542

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


[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

2022-05-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

@HazardyKnusperkeks I think you know this better than any of us as you added 
the assertion to `setType()`. Does this look ok to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126132

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


[PATCH] D126258: [Clang] Avoid misleading 'conflicting types' diagnostic with no-prototype decls.

2022-05-23 Thread Cyndy Ishida via Phabricator via cfe-commits
cishida updated this revision to Diff 431543.
cishida added a comment.

Restrict using the canonical decl's source location to only when the prototype 
was inferred from previously seen decl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126258

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/prototype-redecls.c


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -12,6 +12,10 @@
 void blerp(short);  // expected-note {{previous}}
 void blerp(x) int x; {} // expected-error {{conflicting types for 'blerp'}}
 
+void foo(int); // expected-note {{previous}}
+void foo();
+void foo() {} // expected-error {{conflicting types for 'foo'}}
+
 void glerp(int);
 void glerp(x) short x; {} // Okay, promoted type is fine
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3911,6 +3911,8 @@
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
 Old->getNumParams() != New->getNumParams()) {
+  if (Old->hasInheritedPrototype())
+Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
   Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
   return true;


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -12,6 +12,10 @@
 void blerp(short);  // expected-note {{previous}}
 void blerp(x) int x; {} // expected-error {{conflicting types for 'blerp'}}
 
+void foo(int); // expected-note {{previous}}
+void foo();
+void foo() {} // expected-error {{conflicting types for 'foo'}}
+
 void glerp(int);
 void glerp(x) short x; {} // Okay, promoted type is fine
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3911,6 +3911,8 @@
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
 Old->getNumParams() != New->getNumParams()) {
+  if (Old->hasInheritedPrototype())
+Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
   Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

Whoops, no, this isn't an actual concern. Looks good then, please commit!




Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:234-236
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}

NoQ wrote:
> Shouldn't this one be removed now?
Nvm, it's the old interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 marked 2 inline comments as done.
jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:827
+def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
+  HelpText<"Pass  to the offload linker identified by ">, 
+  MetaVarName<" ">, Group;

tra wrote:
> The comment may need updating now.
Sure, I'll change it before I commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

Couple of nits. LGTM otherwise.




Comment at: clang/include/clang/Driver/Options.td:827
+def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
+  HelpText<"Pass  to the offload linker identified by ">, 
+  MetaVarName<" ">, Group;

The comment may need updating now.



Comment at: clang/include/clang/Driver/Options.td:828
+  HelpText<"Pass  to the offload linker identified by ">, 
+  MetaVarName<" ">, Group;
 def Xpreprocessor : Separate<["-"], "Xpreprocessor">, 
Group,

I think this is backwards. I think the first one here should be `` (the 
one joined with `-Xoffload-linker`), followd by ``


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:234-236
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}

Shouldn't this one be removed now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

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


[PATCH] D126258: [Clang] Avoid misleading 'conflicting types' diagnostic with no-prototype decls.

2022-05-23 Thread Cyndy Ishida via Phabricator via cfe-commits
cishida created this revision.
cishida added reviewers: jyknight, aaron.ballman.
Herald added a subscriber: ributzka.
Herald added a project: All.
cishida requested review of this revision.
Herald added a project: clang.

Clang has recently started diagnosing prototype redeclaration errors like 
rG385e7df33046 


This flagged legitimate issues in a codebase but was confusing to resolve 
because it actually conflicted with a function declaration from a system header 
and not from the one emitted with "note: ".

This patch updates the error handling to use the canonical declaration's source 
location instead to avoid misleading errors like the one described.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126258

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/prototype-redecls.c


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -12,6 +12,10 @@
 void blerp(short);  // expected-note {{previous}}
 void blerp(x) int x; {} // expected-error {{conflicting types for 'blerp'}}
 
+void foo(int); // expected-note {{previous}}
+void foo();
+void foo() {} // expected-error {{conflicting types for 'foo'}}
+
 void glerp(int);
 void glerp(x) short x; {} // Okay, promoted type is fine
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3911,6 +3911,7 @@
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
 Old->getNumParams() != New->getNumParams()) {
+  Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
   Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
   return true;


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -12,6 +12,10 @@
 void blerp(short);  // expected-note {{previous}}
 void blerp(x) int x; {} // expected-error {{conflicting types for 'blerp'}}
 
+void foo(int); // expected-note {{previous}}
+void foo();
+void foo() {} // expected-error {{conflicting types for 'foo'}}
+
 void glerp(int);
 void glerp(x) short x; {} // Okay, promoted type is fine
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3911,6 +3911,7 @@
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
 Old->getNumParams() != New->getNumParams()) {
+  Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
   Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

2022-05-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123319

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


[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
usama54321 updated this revision to Diff 431516.
usama54321 added a comment.

- updated canResolveToExpr to accept both statements and expressions. Removed 
unnecessary code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -39,8 +39,13 @@
   return InnerMatcher.matches(*Result, Finder, Builder);
 }
 
-AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher,
+AST_MATCHER_P(Stmt, canResolveToExpr, ast_matchers::internal::Matcher,
   InnerMatcher) {
+  auto *Exp = dyn_cast(&Node);
+  if (!Exp) {
+return stmt().matches(Node, Finder, Builder);
+  }
+
   auto DerivedToBase = [](const ast_matchers::internal::Matcher &Inner) {
 return implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase),
   hasCastKind(CK_UncheckedDerivedToBase)),
@@ -71,7 +76,7 @@
  IgnoreDerivedToBase(ConditionalOperator),
  IgnoreDerivedToBase(ElvisOperator;
 
-  return ComplexMatcher.matches(Node, Finder, Builder);
+  return ComplexMatcher.matches(*Exp, Finder, Builder);
 }
 
 // Similar to 'hasAnyArgument', but does not work because 'InitListExpr' does
@@ -194,12 +199,13 @@
   return nullptr;
 }
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
-  return selectFirst(
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(
  NodeID::value,
  match(
  findAll(
- expr(canResolveToExpr(equalsNode(Exp)),
+ stmt(canResolveToExpr(equalsNode(Exp)),
   anyOf(
   // `Exp` is part of the underlying expression of
   // decltype/typeof if it has an ancestor of
@@ -225,6 +231,10 @@
  Stm, Context)) != nullptr;
 }
 
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}
+
 const Stmt *
 ExprMutationAnalyzer::findExprMutation(ArrayRef Matches) {
   return tryEachMatch(Matches, this, &ExprMutationAnalyzer::findMutation);
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+ASTContext &Context);
 
 private:
   using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -650,3 +650,38 @@
   do {
   } while (1, (false) && val4 == 1);
 }
+
+void test_typeof() {
+  __typeof__({
+for (int i = 0; i < 10; ++i) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_infinite() {
+  __typeof__({
+for (int i = 0; i < 10;) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_while_infinite() {
+  __typeof__({
+int i = 0;
+while (i < 10) {
+}
+0;
+  }) x;
+}
+
+void test_typeof_dowhile_infinite() {
+  __typeof__({
+int i = 0;
+do {
+
+} while (i < 10);
+0;
+  }) x;
+}
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -177,6 +177,9 @@
 }
   }
 
+  if (ExprMutationAnalyzer::isUnevaluated(LoopStmt, *LoopStmt, *Result.Context))
+return;
+
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D126224#3531949 , @rnk wrote:

> I am reminded of the perennial problem of "optional" protobuf fields that, 
> when omitted, will cause production crashes.
>
> Do you think it would be less disruptive to synthesize a name? I believe the 
> string lives in a string pool, so naming all string literals 
> `` seems like it could be acceptable.

While the string would be deduplicated, the offset in the DIEs (or index in 
DWARFv5, plus offset in .debug_str_offsets) for each of these strings would 
not. But perhaps that would still be low-cost enough (if we were getting really 
fancy, we could bake that string offset/index into the abbreviation - though we 
probably don't have the infrastructure in LLVM's DWARF emission to do that 
super easily)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126224

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 431511.
jhuber6 added a comment.

Merging into a single argument and checking if the joined arg is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -108,6 +108,12 @@
   cl::desc("Argument to pass to the ptxas invocation"),
   cl::cat(ClangLinkerWrapperCategory));
 
+static cl::list
+LinkerArgs("device-linker", cl::ZeroOrMore,
+   cl::desc("Arguments to pass to the device linker invocation"),
+   cl::value_desc(" or ="),
+   cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt Verbose("v", cl::ZeroOrMore,
  cl::desc("Verbose output from tools"),
  cl::init(false),
@@ -226,6 +232,17 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
+// Forward user requested arguments to the device linking job.
+void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
+  for (StringRef Arg : LinkerArgs) {
+auto TripleAndValue = Arg.split('=');
+if (TripleAndValue.second.empty())
+  Args.push_back(TripleAndValue.first);
+else if (TripleAndValue.first == Triple)
+  Args.push_back(TripleAndValue.second);
+  }
+}
+
 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
@@ -531,6 +548,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
 return std::move(Err);
 
@@ -599,6 +617,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*LLDPath, CmdArgs))
 return std::move(Err);
 
@@ -676,6 +695,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
 return std::move(Err);
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -104,3 +104,9 @@
 // RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-LIBRARY %s
 
 // CHECK-LTO-LIBRARY: {{.*}}-lomptarget{{.*}}-lomptarget.devicertl
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -Xoffload-linker a -Xoffload-linker-nvptx64-nvidia-cuda b -Xoffload-linker-nvptx64 c \
+// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
+
+// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}-device-linker=nvptx64-nvidia-cuda=c{{.*}}--
Index: clang/test/Driver/linker-wrapper.c
===
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -80,3 +80,15 @@
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_52 {{.*}}.o
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
 // CUDA: fatbinary{{.*}}-64 --create {{.*}}.fatbin --image=profile=sm_52,file={{.*}}.out --image=profile=sm_70,file={{.*}}.out
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --dry-run --host-triple x86_64-unknown-linux-gnu -linker-path \
+// RUN:   /usr/bin/ld --device-linker=a --device-linker=nvptx64-nvidia-cuda=b -- \
+// RUN:   %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LINKER_ARGS
+
+// LINKER_ARGS: lld{{.*}}-flavor gnu --no-undefined -shared -o {{.*}}.out {{.*}}.o a
+// LINKER_ARGS: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o a b
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8390,6 +8390,20 @@
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
   const auto &LinkCommand = C.

[clang] 4f89ff3 - [test][clang] Move -O3 in command line

2022-05-23 Thread Vitaly Buka via cfe-commits

Author: Vitaly Buka
Date: 2022-05-23T15:57:14-07:00
New Revision: 4f89ff3fc71b0b2adfeb74b900e9a2a90ef80174

URL: 
https://github.com/llvm/llvm-project/commit/4f89ff3fc71b0b2adfeb74b900e9a2a90ef80174
DIFF: 
https://github.com/llvm/llvm-project/commit/4f89ff3fc71b0b2adfeb74b900e9a2a90ef80174.diff

LOG: [test][clang] Move -O3 in command line

Added: 


Modified: 
clang/test/CodeGen/sanitizer-module-constructor.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitizer-module-constructor.c 
b/clang/test/CodeGen/sanitizer-module-constructor.c
index 8c7f78dee9bfd..abc579c6fe23d 100644
--- a/clang/test/CodeGen/sanitizer-module-constructor.c
+++ b/clang/test/CodeGen/sanitizer-module-constructor.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=address -emit-llvm -O3 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=thread -emit-llvm -O3 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=memory -emit-llvm -O3 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=address -O3 -emit-llvm 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=thread -O3 -emit-llvm 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=memory -O3 -emit-llvm 
-fdebug-pass-manager -o - %s 2>&1 | FileCheck %s
 
 // This is regression test for PR42877
 



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


[PATCH] D126100: Add sanitizer-specific GlobalValue attributes.

2022-05-23 Thread Mitch Phillips via Phabricator via cfe-commits
hctim updated this revision to Diff 431508.
hctim added a comment.

Add an extra Global creation path from the frontend, and fix up a comment from 
Kirill on the other revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126100

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals-alias.cpp
  clang/test/CodeGen/asan-globals-odr.cpp
  clang/test/CodeGen/asan-static-odr.cpp
  clang/test/CodeGen/asan-strings.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLParser.h
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/GlobalValue.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/LLVMContextImpl.h

Index: llvm/lib/IR/LLVMContextImpl.h
===
--- llvm/lib/IR/LLVMContextImpl.h
+++ llvm/lib/IR/LLVMContextImpl.h
@@ -1503,6 +1503,9 @@
   /// Collection of per-GlobalValue partitions used in this context.
   DenseMap GlobalValuePartitions;
 
+  DenseMap
+  GlobalValueSanitizerMetadata;
+
   /// DiscriminatorTable - This table maps file:line locations to an
   /// integer representing the next DWARF path discriminator to assign to
   /// instructions in different blocks at the same location.
Index: llvm/lib/IR/Globals.cpp
===
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -67,6 +67,8 @@
   setDLLStorageClass(Src->getDLLStorageClass());
   setDSOLocal(Src->isDSOLocal());
   setPartition(Src->getPartition());
+  if (Src->hasSanitizerMetadata())
+setSanitizerMetadata(Src->getSanitizerMetadata());
 }
 
 void GlobalValue::removeFromParent() {
@@ -217,6 +219,18 @@
   HasPartition = !S.empty();
 }
 
+using SanitizerMetadata = GlobalValue::SanitizerMetadata;
+using GlobalSanitizer = GlobalValue::SanitizerMetadata::GlobalSanitizer;
+const SanitizerMetadata &GlobalValue::getSanitizerMetadata() const {
+  assert(hasSanitizerMetadata());
+  return getContext().pImpl->GlobalValueSanitizerMetadata[this];
+}
+
+void GlobalValue::setSanitizerMetadata(const SanitizerMetadata &Meta) {
+  getContext().pImpl->GlobalValueSanitizerMetadata[this] = Meta;
+  HasSanitizerMetadata = true;
+}
+
 StringRef GlobalObject::getSectionImpl() const {
   assert(hasSection());
   return getContext().pImpl->GlobalObjectSections[this];
Index: llvm/lib/IR/AsmWriter.cpp
===
--- llvm/lib/IR/AsmWriter.cpp
+++ llvm/lib/IR/AsmWriter.cpp
@@ -100,6 +100,9 @@
 using UseListOrderMap =
 DenseMap>>;
 
+using SanitizerMetadata = llvm::GlobalValue::SanitizerMetadata;
+using GlobalSanitizer = SanitizerMetadata::GlobalSanitizer;
+
 /// Look for a value that might be wrapped as metadata, e.g. a value in a
 /// metadata operand. Returns the input value as-is if it is not wrapped.
 static const Value *skipMetadataWrapper(const Value *V) {
@@ -3537,6 +3540,28 @@
 Out << '"';
   }
 
+  if (GV->hasSanitizerMetadata()) {
+SanitizerMetadata MD = GV->getSanitizerMetadata();
+if (MD.HasSanitizer(SanitizerMetadata::NoSanitize)) {
+  Out << ", no_sanitize";
+} else {
+  if (MD.HasSanitizer(SanitizerMetadata::Address))
+Out << ", sanitize_address";
+  if (MD.HasSanitizer(SanitizerMetadata::HWAddress))
+Out << ", sanitize_hwaddress";
+  if (MD.HasSanitizer(SanitizerMetadata::Memtag))
+Out << ", sanitize_address";
+  if (MD.HasSanitizer(SanitizerMetadata::NoAddress))
+Out << ", no_sanitize_address";
+  if (MD.HasSanitizer(SanitizerMetadata::NoHWAddress))
+Out << ", no_sanitize_hwaddress";
+  if (MD.HasSanitizer(SanitizerMetadata::NoMemtag))
+Out << ", no_sanitize_memtag";
+  if (MD.HasSanitizer(GlobalSanitizer::Address) && MD.IsDynInit)
+Out << ", sanitize_address_dyninit";
+}
+  }
+
   maybePrintComdat(Out, *GV);
   if (MaybeAlign A = GV->getAlign())
 Out << ", align " << A->value();
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1344,7 +1344,8 @@
 // GLOBALVAR: [strtab offset, strtab size, type, isconst, initid,
 // linkage, alignment, section, visibility, threadlocal,
 // unnamed_addr, externally_initialized, dllstorageclass,
-// comdat, attributes, DSO_Local]
+// comdat, attributes, DSO_Local, GlobalSanitizer::Sanitizer,
+// GlobalSanitizer::IsDynInit]
 Vals.push_back(addToStrtab(GV.getName()));
   

[PATCH] D126031: [libclang] add supporting for indexing/visiting C++ concepts

2022-05-23 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 431506.
arphaman added a comment.
Herald added a project: clang.

Fix the windows test failure (add -fno-delayed-template-parsing)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126031

Files:
  clang/include/clang-c/Index.h
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/Index/index-concept-kind.cpp
  clang/test/Index/index-concepts.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/tools/libclang/CXIndexDataConsumer.cpp
  clang/tools/libclang/CXIndexDataConsumer.h
  clang/tools/libclang/CursorVisitor.h

Index: clang/tools/libclang/CursorVisitor.h
===
--- clang/tools/libclang/CursorVisitor.h
+++ clang/tools/libclang/CursorVisitor.h
@@ -19,6 +19,10 @@
 class PreprocessingRecord;
 class ASTUnit;
 
+namespace concepts {
+class Requirement;
+}
+
 namespace cxcursor {
 
 class VisitorJob {
@@ -37,6 +41,8 @@
 MemberRefVisitKind,
 SizeOfPackExprPartsKind,
 LambdaExprPartsKind,
+ConceptSpecializationExprVisitKind,
+RequiresExprVisitKind,
 PostChildrenVisitKind
   };
 
@@ -242,6 +248,9 @@
   bool VisitStaticAssertDecl(StaticAssertDecl *D);
   bool VisitFriendDecl(FriendDecl *D);
   bool VisitDecompositionDecl(DecompositionDecl *D);
+  bool VisitConceptDecl(ConceptDecl *D);
+  bool VisitTypeConstraint(const TypeConstraint &TC);
+  bool VisitConceptRequirement(const concepts::Requirement &R);
 
   // Name visitor
   bool VisitDeclarationNameInfo(DeclarationNameInfo Name);
Index: clang/tools/libclang/CXIndexDataConsumer.h
===
--- clang/tools/libclang/CXIndexDataConsumer.h
+++ clang/tools/libclang/CXIndexDataConsumer.h
@@ -409,6 +409,8 @@
   bool handleFunctionTemplate(const FunctionTemplateDecl *D);
   bool handleTypeAliasTemplate(const TypeAliasTemplateDecl *D);
 
+  bool handleConcept(const ConceptDecl *D);
+
   bool handleReference(const NamedDecl *D, SourceLocation Loc, CXCursor Cursor,
const NamedDecl *Parent,
const DeclContext *DC,
Index: clang/tools/libclang/CXIndexDataConsumer.cpp
===
--- clang/tools/libclang/CXIndexDataConsumer.cpp
+++ clang/tools/libclang/CXIndexDataConsumer.cpp
@@ -146,6 +146,11 @@
 DataConsumer.importedModule(D);
 return true;
   }
+
+  bool VisitConceptDecl(const ConceptDecl *D) {
+DataConsumer.handleConcept(D);
+return true;
+  }
 };
 
 CXSymbolRole getSymbolRole(SymbolRoleSet Role) {
@@ -883,6 +888,12 @@
   return handleDecl(D, D->getLocation(), getCursor(D), DInfo);
 }
 
+bool CXIndexDataConsumer::handleConcept(const ConceptDecl *D) {
+  DeclInfo DInfo(/*isRedeclaration=*/!D->isCanonicalDecl(),
+ /*isDefinition=*/true, /*isContainer=*/false);
+  return handleDecl(D, D->getLocation(), getCursor(D), DInfo);
+}
+
 bool CXIndexDataConsumer::handleReference(const NamedDecl *D, SourceLocation Loc,
   CXCursor Cursor,
   const NamedDecl *Parent,
@@ -1249,7 +1260,6 @@
   case SymbolKind::TemplateTypeParm:
   case SymbolKind::TemplateTemplateParm:
   case SymbolKind::NonTypeTemplateParm:
-  case SymbolKind::Concept:
 return CXIdxEntity_Unexposed;
 
   case SymbolKind::Enum: return CXIdxEntity_Enum;
@@ -1289,6 +1299,8 @@
   case SymbolKind::Destructor: return CXIdxEntity_CXXDestructor;
   case SymbolKind::ConversionFunction: return CXIdxEntity_CXXConversionFunction;
   case SymbolKind::Parameter: return CXIdxEntity_Variable;
+  case SymbolKind::Concept:
+return CXIdxEntity_CXXConcept;
   }
   llvm_unreachable("invalid symbol kind");
 }
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -299,8 +299,6 @@
   case Stmt::BinaryConditionalOperatorClass:
   case Stmt::TypeTraitExprClass:
   case Stmt::CoawaitExprClass:
-  case Stmt::ConceptSpecializationExprClass:
-  case Stmt::RequiresExprClass:
   case Stmt::DependentCoawaitExprClass:
   case Stmt::CoyieldExprClass:
   case Stmt::CXXBindTemporaryExprClass:
@@ -637,6 +635,14 @@
 return getSelectorIdentifierCursor(SelectorIdIndex, C);
   }
 
+  case Stmt::ConceptSpecializationExprClass:
+K = CXCursor_ConceptSpecializationExpr;
+break;
+
+  case Stmt::RequiresExprClass:
+K = CXCursor_RequiresExpr;
+break;
+
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -761,10 +761,10 @@
 }
 
 bool CursorVisitor::VisitTemplateTyp

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:826
   Group;
+def Xoffload_linker : Separate<["-"], "Xoffload-linker">,
+  HelpText<"Pass  to the offload linker">, MetaVarName<"">,

tra wrote:
> This option still stands out as a sore thumb.
> 
> Could we fold it into the one below as `-Xoffload-linker-all` ?
> 
> Or, maybe make `Xoffload_linker_arg` use `"Xoffload-linker"` prefix and then 
> check that the first argument is either empty (which would meand "for all") 
> or "-".
> 
> Maybe we don't even need separate "-Xoffload_linker" option(s). I wonder if 
> it would make sense to extend the existing `-Xlinker` and use 
> `-Xlinker-` ?
> 
I don't think we could rework `-Xlinker` as it works by forwarding the 
arguments to the linker job, this requires some handling inside of Clang to 
format it properly. But I should definitely make it a single argument by just 
checking if the joined argument is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D124493: Move Sanitizer metadata to be on-GlobalValue.

2022-05-23 Thread Mitch Phillips via Phabricator via cfe-commits
hctim planned changes to this revision.
hctim added a comment.

Pulled out the IR-specific changes to D126100 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124493

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/include/clang/Driver/Options.td:826
   Group;
+def Xoffload_linker : Separate<["-"], "Xoffload-linker">,
+  HelpText<"Pass  to the offload linker">, MetaVarName<"">,

This option still stands out as a sore thumb.

Could we fold it into the one below as `-Xoffload-linker-all` ?

Or, maybe make `Xoffload_linker_arg` use `"Xoffload-linker"` prefix and then 
check that the first argument is either empty (which would meand "for all") or 
"-".

Maybe we don't even need separate "-Xoffload_linker" option(s). I wonder if it 
would make sense to extend the existing `-Xlinker` and use `-Xlinker-` ?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126247: `readability-indentifier-naming` resolution order and examples

2022-05-23 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2747
+Resolution order
+---
+

Please make length same as title.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2762
+ - InlineNamespace ``[inline namespace]``
+ - Namespace ```[namespace]``
+ - 

Extra back-tick.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2854
+Labels listed in ``<>`` brackets are semantic qualifiers and attempt to
+illustrate the semantic context within which clang-tidy resolves the
+classification. These are not formal semantic labels, rather labels which

`Clang-tidy`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2857
+attempt disambiguation within the context of this document. For example,
+ identifiers that clang tidy is currently looking only at
+member variables.

Ditto.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2862
+that particular classification. These must be present in the declaration for
+clang-tidy to match the classification.
+

Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126247

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


[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

2022-05-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Because of `TargetTriple.getOS() == llvm::Triple::Linux`, you need to guard the 
unittest under a similar check.

> Harbormaster completed remote builds in `B165047: Diff 430277.`

You can get Windows bot results in the link.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125862

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


[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

2022-05-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D126157#3531635 , @MyDeveloperDay 
wrote:

> LGTM too.. @owenpan this is one of my favourite features!!

I love it too! :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

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


[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:43-44
+ASTContext &Context);
+  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ASTContext &Context);
 

I suspect this may lead to subtle bugs when a `Stmt *` that actually points to 
an `Expr` at runtime would cause the first overload to be chosen.

Maybe teach `canResolveToExpr()` to accept an `Stmt` instead (with early 
return)?



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:197
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
-  return selectFirst(
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(

We're trying to avoid `auto` when it makes the code less readable i.e. when the 
type isn't obvious from the context 
(https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

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


[PATCH] D126031: [libclang] add supporting for indexing/visiting C++ concepts

2022-05-23 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

Yes, I'm going to check what's wrong with it.


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

https://reviews.llvm.org/D126031

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


[PATCH] D126247: `readability-indentifier-naming` resolution order and examples

2022-05-23 Thread Kazys Stepanas via Phabricator via cfe-commits
KazNX created this revision.
KazNX added reviewers: whisperity, clang-tools-extra.
KazNX added a project: clang-tools-extra.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
KazNX requested review of this revision.
Herald added a subscriber: cfe-commits.

Added extensive documentation to identify the order in which 
`readability-identifier-naming` attempts to resolve its classifications. This 
is followed by an exhaustive code example covering all identifiers commented 
with their appropriate classifications. This seeks to provide improved 
information for those seeking to define a particular naming standard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126247

Files:
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst

Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -2742,3 +2742,459 @@
 double   dValueDouble = 0.0;
 ULONGulValueUlong = 0;
 DWORDdwValueDword = 0;
+
+Resolution order
+---
+
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can falling into several classifications. Bellow
+is a list of the classifications supported by ``readability-identifier-naming``
+presented in the order in which the classifications are resolved. Some
+classifications appear multiple times as they can be checked in different
+contexts. The check returns as soon as the first valid classification is
+matched. This occurs when the semantics of the identifier match and there is a
+valid option for the classification present in the ``.clang-tidy`` file - e.g.,
+``readability-identifier-naming.AbstractClassCase``.
+
+ - Typedef  ``[typedef]``
+ - TypeAlias  ``[using Alias = ...]``
+ - InlineNamespace ``[inline namespace]``
+ - Namespace ```[namespace]``
+ - 
+   - ScopedEnumValue ``[enum class member]``
+   - EnumConstant  ``[enum member]``
+   - Constant
+   - Invalid
+ - 
+   - AbstractClass ``[class, struct, pure-virtual present]``
+   - Struct ``[struct]``
+   - Class ``[class, struct]``
+   - Struct ``[class]``
+   - Union ``[union]``
+   - Enum ``[enum, enum class]``
+   - Invalid
+ -  - does not cover ``[static, constexpr]``
+   - ``[const]``
+ - ConstantMember
+ - Constant
+   - PrivateMember ``[private]``
+   - ProtectedMember ``[protected]``
+   - PublicMember ``[public]``
+   - Member
+   - Invalid
+ - 
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``
+ - ConstantPointerParameter ``[*]``
+ - ConstantParameter
+ - Constant
+   - ParameterPack ``[...]``
+   - PointerParameter ``[*]``
+   - Parameter
+   - Invalid
+ - 
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``
+ - ClassConstant ``[const, static]``
+ - 
+   - GlobalConstantPointer ``[const *]``
+   - GlobalConstant ``[const]``
+ - StaticConstant ``[static, const]``
+ - 
+   - LocalConstantPointer ``[const *]``
+   - LocalConstant ``[const]``
+ - Constant ``[const]``
+   - 
+ - ClassMember ``[static]``
+   - 
+ - GlobalPointer ``[*]``
+ - GlobalVariable
+   - 
+ - StaticVariable ``[static]``
+ - LocalPointer ``[*]``
+ - LocalVariable
+   - 
+ - LocalVariable
+   - Variable
+ - 
+   - 
+   - ``[constexpr]``
+ - ConstexprMethod
+ - ConstexprFunction
+   - ClassMethod ``[static]``
+   - VirtualMethod ``[virtual]``
+   - PrivateMethod ``[private]``
+   - ProtectedMethod ``[protected]``
+   - PublicMethod ``[public]``
+   - Method
+   - Function
+   - Invalid
+ - 
+   - 
+   - ConstexprFunction ``[constexpr]``
+   - GlobalFunction ``[static method, static function, in any namespace]``
+   - Function
+   - Invalid
+ - 
+   - 
+ - TypeTemplateParameter
+ - TemplateParameter
+ - Invalid
+   - 
+ - ValueTemplateParameter
+ - TemplateParameter
+ - Invalid
+   - 
+ - TemplateTemplateParameter
+ - TemplateParameter
+ - Invalid
+ - Invalid
+
+Labels listed in ``<>`` brackets are semantic qualifiers and attempt to
+illustrate the semantic context within which clang-tidy resolves the
+classification. These are not formal semantic labels, rather labels which
+attempt disambiguation within the context of this document. For example,
+ identifiers that clang tidy is currently looking only at
+member variables.
+
+Items in ``[]`` brackets provide C/C++ keywords and/or decorations relevant to
+that particular classification. These must be present in the declaration for
+clang-tidy to match the classification.
+
+List nesting is used to collate classifications which share some semantic
+identification - either a logical grouping using ``<>`` or ``[]`` qualifiers -
+and are each validated within that same semantic context. That is, a

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D125904#3532545 , @jhuber6 wrote:

> clang a.c -c -o a-0.o // Has some externalized static variable.
> clang a.c -c -o a-1.o
> clang a-0.o a-1.o // Redefined symbol error

Ah. OK. This is a bit less of a concern. As long as we take compiler options 
into account it should work.

There are use cases when the same file is recompiled with different -DFOO 
macros. And that *is* supported by NVCC and, I think it is (or used to be?) 
broken in clang.
I think we currently end up renaming the source files for each compilation 
variant.

>> The fact that NVCC didn't always generate the same output **was** an issue 
>> when we were using it for CUDA compilation.
>> In general, "not supported by NVCC" is not quite applicable here, IMO. The 
>> goal here is to make clang work correctly.
>
> I feel like linking a file with itself is pretty uncommon, but in order to 
> support that we'd definitely need the CUID method so we can pass it to both 
> the host and device. I'm personally fine with this and the CUID living 
> together so if for whatever reason there's a symbol clash, the user can 
> specify a CUID to make it go away.

I agree that compiling and linking the same file is probably not very common 
and I can't think of practical use case for it. 
That said, I would consider compiling the same source with different 
preprocessor options to be a legitimate use case that we should support. 
Explicitly passing cuid would work as a workaround in those cases, so it's not 
a major issue if we can't make it work out of the box without explicit cuid.

> We also discussed the problem of non-static source trees which neither this 
> nor the current CUID would solve. As far as I can tell, this method would 
> work fine for like 99.99% of codes, but getting that last 0.01% would require 
> something like generating a UUID for each compilation job, which requires 
> intervention from the driver to set up offloading compilation properly. So 
> I'm not sure if it's the best trade-off.

Acknowledged. Creating globally-unique, yet consistent across all 
sub-compilations  ID based only the info available to individual subcompilation 
is probably hard-to-impossible to do.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6845
+}
+OS << llvm::format("%x", ID.getFile()) << llvm::format("%x", 
ID.getDevice())
+   << PLoc.getLine();

Considering that the file name may have arbitrary symbols in it, that may 
result in a symbol name that we can't really use.
I'd print a hash of the (finename+device+line) -- that would guarantee that we 
know there are no funky characters in the suffix.

I'm also not sure if the line number makes any difference here. There's no need 
to differentiate between symbols within the same TU within the same 
compilation, only across different compilations and for that filename+device 
should be sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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


[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

2022-05-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
 do {

curdeius wrote:
> owenpan wrote:
> > From 
> > https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
> >  it's unclear whether the braces should be removed when a block has a 
> > do-while loop as its single statement. Should I put the braces back and add 
> > handling of do-while for `RemoveBracesLLVM`?
> It's indeed not clear, I remember having paused over this code for a moment.
> I believe that the best course of action is to grep for both cases in the 
> codebase and see what's more frequent (and whether there's some sort of 
> consensus).
Good idea. I don't think there's any consensus specifically about this but will 
ask on discourse.llvm.org. To err on the conservative side, I will manually add 
the braces back for now.



Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+  if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = &Change;
-  else {
-if ((Change.StartOfBlockComment = LastBlockComment))
+  } else {
+if ((Change.StartOfBlockComment = LastBlockComment)) {
   Change.IndentationOffset =
   Change.StartOfTokenColumn -
   Change.StartOfBlockComment->StartOfTokenColumn;

curdeius wrote:
> owenpan wrote:
> > It would be nice to remove the braces of the `else` here. See 
> > https://github.com/llvm/llvm-project/issues/55663.
> +1. Would you fix it manually here then and fix it later?
I will leave it as is and use it as a test case for later. Implementing this 
would add more complexity to `RemoveBracesLLVM`, which is complex enough 
already. I will look into it in a few months as I will be away for the summer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

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


[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
usama54321 updated this revision to Diff 431497.
usama54321 added a comment.

Updating patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,35 +194,48 @@
   return nullptr;
 }
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
-  return selectFirst(
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
+  hasAncestor(expr(anyOf(
+  // `UnaryExprOrTypeTraitExpr` is unevaluated
+  // unless it's sizeof on VLA.
+  unaryExprOrTypeTraitExpr(
+  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
+  // `CXXTypeidExpr` is unevaluated unless it's
+  // applied to an expression of glvalue of
+  // polymorphic class type.
+  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+  // The controlling expression of
+  // `GenericSelectionExpr` is unevaluated.
+  genericSelectionExpr(
+  hasControllingExpr(hasDescendant(equalsNode(Exp,
+  cxxNoexceptExpr();
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(NodeID::value,
+   match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+  isUnevaluatedMatcher(Exp))
+ .bind(NodeID::value)),
+ Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(
  NodeID::value,
- match(
- findAll(
- expr(canResolveToExpr(equalsNode(Exp)),
-  anyOf(
-  // `Exp` is part of the underlying expression of
-  // decltype/typeof if it has an ancestor of
-  // typeLoc.
-  hasAncestor(typeLoc(unless(
-  hasAncestor(unaryExprOrTypeTraitExpr(),
-  hasAncestor(expr(anyOf(
-  // `UnaryExprOrTypeTraitExpr` is unevaluated
-  // unless it's sizeof on VLA.
-  unaryExprOrTypeTraitExpr(unless(sizeOfExpr(
-  hasArgumentOfType(variableArrayType(),
-  // `CXXTypeidExpr` is unevaluated unless it's
-  // applied to an expression of glvalue of
-  // polymorphic class type.
-  cxxTypeidExpr(
-  unless(isPotentiallyEvaluated())),
-  // The controlling expression of
-  // `GenericSelectionExpr` is unevaluated.
-  genericSelectionExpr(hasControllingExpr(
-  hasDescendant(equalsNode(Exp,
-  cxxNoexceptExpr())
- .bind(NodeID::value)),
- Stm, Context)) != nullptr;
+ match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
+   .bind(NodeID::value)),
+   Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
 }
 
 const Stmt *
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,10 @@
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+ASTContext &Context);
+  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
+ASTContext &

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

2022-05-23 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D125773#3525127 , @ilya-biryukov 
wrote:

> It looks like this particular change actually breaks standard compatibility 
> as we also use the same parsing action and don't build a module separately on 
> `import`.
>
>   [module.import]p5
>   A module-import-declaration that specifies a header-name H imports a 
> synthesized header unit, which is a
>   translation unit formed by applying phases 1 to 7 of translation (5.2) to 
> the source file or header nominated
>   by H, which shall not contain a module-declaration.

For the record, the intention is that (with local submodule visibility enabled) 
we behave "as if" we preprocessed separately, even if we do so using the same 
parsing action: we store various parts of the preprocessor state (most notably, 
the defined macros map) per-module, so that when we switch modules, we get the 
same preprocessing behavior as if we'd started from a clean slate. 
Fundamentally, Clang's model is that a single `ASTContext` and a single 
preprocessing action can cover multiple translation units, and that includes 
the possibility that a single preprocessor or `Sema` object can process 
multiple translation units, and we make this work by tracking which macros and 
declarations and similar should be visible where, based on which modules are 
currently imported. This is intended to be a conforming strategy for 
implementing C++20 header units.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125773

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 431491.
jhuber6 added a comment.

Go back to old joined method and also change the name to remote `_EQ`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -108,6 +108,12 @@
   cl::desc("Argument to pass to the ptxas invocation"),
   cl::cat(ClangLinkerWrapperCategory));
 
+static cl::list
+LinkerArgs("device-linker", cl::ZeroOrMore,
+   cl::desc("Arguments to pass to the device linker invocation"),
+   cl::value_desc(" or ="),
+   cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt Verbose("v", cl::ZeroOrMore,
  cl::desc("Verbose output from tools"),
  cl::init(false),
@@ -226,6 +232,17 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
+// Forward user requested arguments to the device linking job.
+void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
+  for (StringRef Arg : LinkerArgs) {
+auto TripleAndValue = Arg.split('=');
+if (TripleAndValue.second.empty())
+  Args.push_back(TripleAndValue.first);
+else if (TripleAndValue.first == Triple)
+  Args.push_back(TripleAndValue.second);
+  }
+}
+
 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
@@ -531,6 +548,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
 return std::move(Err);
 
@@ -599,6 +617,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*LLDPath, CmdArgs))
 return std::move(Err);
 
@@ -676,6 +695,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
 return std::move(Err);
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -104,3 +104,9 @@
 // RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-LIBRARY %s
 
 // CHECK-LTO-LIBRARY: {{.*}}-lomptarget{{.*}}-lomptarget.devicertl
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -Xoffload-linker a -Xoffload-linker-nvptx64-nvidia-cuda b -Xoffload-linker-nvptx64 c \
+// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
+
+// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}-device-linker=nvptx64-nvidia-cuda=c{{.*}}--
Index: clang/test/Driver/linker-wrapper.c
===
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -80,3 +80,15 @@
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_52 {{.*}}.o
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
 // CUDA: fatbinary{{.*}}-64 --create {{.*}}.fatbin --image=profile=sm_52,file={{.*}}.out --image=profile=sm_70,file={{.*}}.out
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --dry-run --host-triple x86_64-unknown-linux-gnu -linker-path \
+// RUN:   /usr/bin/ld --device-linker=a --device-linker=nvptx64-nvidia-cuda=b -- \
+// RUN:   %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LINKER_ARGS
+
+// LINKER_ARGS: lld{{.*}}-flavor gnu --no-undefined -shared -o {{.*}}.out {{.*}}.o a
+// LINKER_ARGS: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o a b
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8390,6 +8390,18 @@
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
   const auto &LinkCommand = C.g

[PATCH] D126034: [clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
usama54321 updated this revision to Diff 431490.
usama54321 added a comment.

Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to 
use new check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126034

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,36 +194,44 @@
   return nullptr;
 }
 
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
+  hasAncestor(expr(anyOf(
+  // `UnaryExprOrTypeTraitExpr` is unevaluated
+  // unless it's sizeof on VLA.
+  unaryExprOrTypeTraitExpr(
+  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
+  // `CXXTypeidExpr` is unevaluated unless it's
+  // applied to an expression of glvalue of
+  // polymorphic class type.
+  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+  // The controlling expression of
+  // `GenericSelectionExpr` is unevaluated.
+  genericSelectionExpr(
+  hasControllingExpr(hasDescendant(equalsNode(Exp,
+  cxxNoexceptExpr();
+}
+
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
  ASTContext &Context) {
-  return selectFirst(
+  return selectFirst(NodeID::value,
+   match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+  isUnevaluatedMatcher(Exp))
+ .bind(NodeID::value)),
+ Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(
  NodeID::value,
- match(
- findAll(
- expr(canResolveToExpr(equalsNode(Exp)),
-  anyOf(
-  // `Exp` is part of the underlying expression of
-  // decltype/typeof if it has an ancestor of
-  // typeLoc.
-  hasAncestor(typeLoc(unless(
-  hasAncestor(unaryExprOrTypeTraitExpr(),
-  hasAncestor(expr(anyOf(
-  // `UnaryExprOrTypeTraitExpr` is unevaluated
-  // unless it's sizeof on VLA.
-  unaryExprOrTypeTraitExpr(unless(sizeOfExpr(
-  hasArgumentOfType(variableArrayType(),
-  // `CXXTypeidExpr` is unevaluated unless it's
-  // applied to an expression of glvalue of
-  // polymorphic class type.
-  cxxTypeidExpr(
-  unless(isPotentiallyEvaluated())),
-  // The controlling expression of
-  // `GenericSelectionExpr` is unevaluated.
-  genericSelectionExpr(hasControllingExpr(
-  hasDescendant(equalsNode(Exp,
-  cxxNoexceptExpr())
- .bind(NodeID::value)),
- Stm, Context)) != nullptr;
+ match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
+   .bind(NodeID::value)),
+   Stm, Context)) != nullptr;
 }
 
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+ASTContext &Context);
   static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
 ASTContext &Context);
 
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
==

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

2022-05-23 Thread Tyler Chatow via Phabricator via cfe-commits
tchatow updated this revision to Diff 431489.
tchatow added a comment.

Added tests for non-empty enums


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125848

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3565,6 +3565,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3611,6 +3612,17 @@
"};",
EightIndent);
 
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
@@ -3658,7 +3670,19 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct [[nodiscard]] E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
+
+  verifyFormat("enum struct [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum struct [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
 }
 
 TEST_F(FormatTest, FormatsEnumClass) {
@@ -3675,7 +3699,19 @@
   verifyFormat("enum class X E {} d;");
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
+  verifyFormat("enum class [[nodiscard]] E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
+
+  verifyFormat("enum class [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum class [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
 }
 
 TEST_F(FormatTest, FormatsEnumTypes) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3360,11 +3360,18 @@
 
   while (FormatTok->Tok.getIdentifierInfo() ||
  FormatTok->isOneOf(tok::colon, tok::coloncolon, tok::less,
-tok::greater, tok::comma, tok::question)) {
+tok::greater, tok::comma, tok::question,
+tok::l_square, tok::r_square)) {
 nextToken();
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
+if (FormatTok->is(TT_AttributeSquare)) {
+  parseSquare();
+  // Consume the closing TT_AttributeSquare.
+  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+nextToken();
+}
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3565,6 +3565,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3611,6 +3612,17 @@
"};",
EightIndent);
 
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  ONE,\n"
+   "  TWO,\n"
+   "};");
+  verifyFormat("enum [[nodiscard]] E {\n"
+   "  // Comment 1\n"
+   "  ONE,\n"
+   "  // Comment 2\n"
+   "  TWO,\n"
+   "};");
+
   // Not enums.
   verifyFormat("enum X f() {\n"
"  a();\n"
@@ -3658,7 +3670,19 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct [[nodiscard]] E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
+
+  verifyFormat("enum

[PATCH] D126245: Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to use new check

2022-05-23 Thread Usama Hameed via Phabricator via cfe-commits
usama54321 created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
usama54321 requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126245

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,36 +194,44 @@
   return nullptr;
 }
 
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+  // `Exp` is part of the underlying expression of
+  // decltype/typeof if it has an ancestor of
+  // typeLoc.
+  hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr(),
+  hasAncestor(expr(anyOf(
+  // `UnaryExprOrTypeTraitExpr` is unevaluated
+  // unless it's sizeof on VLA.
+  unaryExprOrTypeTraitExpr(
+  unless(sizeOfExpr(hasArgumentOfType(variableArrayType(),
+  // `CXXTypeidExpr` is unevaluated unless it's
+  // applied to an expression of glvalue of
+  // polymorphic class type.
+  cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+  // The controlling expression of
+  // `GenericSelectionExpr` is unevaluated.
+  genericSelectionExpr(
+  hasControllingExpr(hasDescendant(equalsNode(Exp,
+  cxxNoexceptExpr();
+}
+
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
  ASTContext &Context) {
-  return selectFirst(
+  return selectFirst(NodeID::value,
+   match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+  isUnevaluatedMatcher(Exp))
+ .bind(NodeID::value)),
+ Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+ ASTContext &Context) {
+  return selectFirst(
  NodeID::value,
- match(
- findAll(
- expr(canResolveToExpr(equalsNode(Exp)),
-  anyOf(
-  // `Exp` is part of the underlying expression of
-  // decltype/typeof if it has an ancestor of
-  // typeLoc.
-  hasAncestor(typeLoc(unless(
-  hasAncestor(unaryExprOrTypeTraitExpr(),
-  hasAncestor(expr(anyOf(
-  // `UnaryExprOrTypeTraitExpr` is unevaluated
-  // unless it's sizeof on VLA.
-  unaryExprOrTypeTraitExpr(unless(sizeOfExpr(
-  hasArgumentOfType(variableArrayType(),
-  // `CXXTypeidExpr` is unevaluated unless it's
-  // applied to an expression of glvalue of
-  // polymorphic class type.
-  cxxTypeidExpr(
-  unless(isPotentiallyEvaluated())),
-  // The controlling expression of
-  // `GenericSelectionExpr` is unevaluated.
-  genericSelectionExpr(hasControllingExpr(
-  hasDescendant(equalsNode(Exp,
-  cxxNoexceptExpr())
- .bind(NodeID::value)),
- Stm, Context)) != nullptr;
+ match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
+   .bind(NodeID::value)),
+   Stm, Context)) != nullptr;
 }
 
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
Index: clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
===
--- clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
+++ clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+ASTContext &Context);
   static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
 ASTContext &Context);
 
Index: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
===

[PATCH] D123286: [Clang][OpenMP] Support for omp nothing

2022-05-23 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

LG


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

https://reviews.llvm.org/D123286

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


[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2022-05-23 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Parse/ParseOpenMP.cpp:3692-3694
+  if (Kind == llvm::omp::Clause::OMPC_fail) {
+Clause = ParseOpenMPFailClause(Clause);
+  }




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

https://reviews.llvm.org/D123235

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


[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D125904#3532492 , @tra wrote:

> This is a moderately serious issue. Some users care about the build 
> reproducibility. Recompiling the same sources and getting different results 
> will trigger all sorts of red flags that would need to be investigated in 
> order to make sure the build is not broken.

I mean this in the context that the following will not work

  clang a.c -c -o a-0.o // Has some externalized static variable.
  clang a.c -c -o a-1.o
  clang a-0.o a-1.o // Redefined symbol error

The build will be perfectly reproducible, the ID we append here is just 
`__static__` which should be the same in 
a static source tree. Though it might be annoying that the line number may 
change on white-space changes, so we could do without the line number at the 
end if that's an issue.

>> However, this is a very niche use-case and is not supported by Nvidia's CUDA 
>> compiler so it likely to be good enough.
>
> The fact that NVCC didn't always generate the same output **was** an issue 
> when we were using it for CUDA compilation.
> In general, "not supported by NVCC" is not quite applicable here, IMO. The 
> goal here is to make clang work correctly.

I feel like linking a file with itself is pretty uncommon, but in order to 
support that we'd definitely need the CUID method so we can pass it to both the 
host and device. I'm personally fine with this and the CUID living together so 
if for whatever reason there's a symbol clash, the user can specify a CUID to 
make it go away. We also discussed the problem of non-static source trees which 
neither this nor the current CUID would solve. As far as I can tell, this 
method would work fine for like 99.99% of codes, but getting that last 0.01% 
would require something like generating a UUID for each compilation job, which 
requires intervention from the driver to set up offloading compilation 
properly. So I'm not sure if it's the best trade-off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D126226#3532423 , @tra wrote:

> In D126226#3532257 , @MaskRay wrote:
>
>> It's better to avoid `JoinedAndSeparate` for new options. It is for `--xxx 
>> val` and `--xxxval` but not intended for the option this patch will add.
>
> I'm not sure I understand your argument. The two cases where I see 
> `JoinedAndSeparate` are used right now (`-Xarch_` and `-plugin-arg`) both are 
> using it for the purposes similar to this patch.
> I also do not quite see how `JoinedAndSeparate` is applicable to 
> `--xxxval`/`--xxx val`.
> Could you elaborate, please?
>
> In D126226#3532301 , @MaskRay wrote:
>
>> Consider something like `-Xoffload-linker-triple =`
>
> That could work.
>
> We keep running into the same old underlying issue that we do not have a good 
> way to name/reference specific parts of the compilation pipeline. -Xfoo used 
> to work OK for the linear 'standard' compilation pipeline, but these days 
> when compilation grew from a simple linear pipe it's no longer adequate and 
> we need to extend it.
>
> Speaking of triples. I think using triple as the selector is insufficient for 
> general offloading use. We may have offload variants that would use the same 
> triple, but would be compiled using their own pipeline. E.g. the GPU binaries 
> for sm_60 and sm_80 GPUs will use the same nvptx64 triple, but would 
> presumably be lined with different linker instances and may need different 
> options. My understanding is that AMDGPU has even more detailed offload 
> variants (same triple, same GPU arch, different features). I don't know 
> whether it's applicable to OpenMP, though. I think it is. IIRC OpenMP has a 
> way to specialize offload to particular GPU variant and that would probably 
> give you multiple offload targets, all with the same triple.

OK, please ignore my comments. I see `JoinedAndSeparate` that supports 2 
arguments. The relevant code is `llvm/lib/Option/Option.cpp:197`.
I hereby retreat my objection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

Unnamed variables are an oddity, sure; we've had to patch a downstream test or 
two that wasn't being careful enough.  But not providing a name is entirely 
defensible, and consumers should be willing to cope with DWARF that doesn't 
fully meet their expectations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126224

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D126226#3532471 , @jhuber6 wrote:

> In D126226#3532423 , @tra wrote:
>
>> We keep running into the same old underlying issue that we do not have a 
>> good way to name/reference specific parts of the compilation pipeline. -Xfoo 
>> used to work OK for the linear 'standard' compilation pipeline, but these 
>> days when compilation grew from a simple linear pipe it's no longer adequate 
>> and we need to extend it.
>
> Yeah, it's getting increasingly complicated to refer to certain portions of 
> the compilation toolchain as we start adding more complicated stuff. Just 
> recently I had a problem that I wanted to pass an `-Xclang` argument only to 
> the CUDA toolchain, and there's no way to do it as far as I can tell. It may 
> be worth revisiting this whole concept to support more arbitrary combinations.

`-Xarch_device` should do that for all device compilations, or you could use 
`-Xarch_sm_XX` if you need to pass it only to the compilation targeting `sm_XX`.

>> Speaking of triples. I think using triple as the selector is insufficient 
>> for general offloading use.
>
> Yes, it's not a truly generic solution. But I figured that just being able to 
> specify it for each "tool-chain" was sufficient for the use-case here and we 
> can expand it as needed. I added support for OpenMP to use `--offload-arch` 
> recently so we definitely use it. The OpenMP offloading GPU runtime library 
> is now built as a static library with `--offload-arch=` for all 32 supported 
> architectures currently, it works surprisingly well.

The comment was largely intended as a counterargument to @MaskRay 's proposal 
to hardcode triples into arguments. It's doable, but with ever continuing 
expanding set of offloading targets will be the source of unnecessary churn. It 
it were just triples, it would be fine, but our set is potentially a cartesian 
product of {triple, GPU variant} and both AMDGPU and nvptx have quite a few GPU 
variants they can target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

2022-05-23 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
 do {

owenpan wrote:
> From 
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
>  it's unclear whether the braces should be removed when a block has a 
> do-while loop as its single statement. Should I put the braces back and add 
> handling of do-while for `RemoveBracesLLVM`?
It's indeed not clear, I remember having paused over this code for a moment.
I believe that the best course of action is to grep for both cases in the 
codebase and see what's more frequent (and whether there's some sort of 
consensus).



Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+  if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = &Change;
-  else {
-if ((Change.StartOfBlockComment = LastBlockComment))
+  } else {
+if ((Change.StartOfBlockComment = LastBlockComment)) {
   Change.IndentationOffset =
   Change.StartOfTokenColumn -
   Change.StartOfBlockComment->StartOfTokenColumn;

owenpan wrote:
> It would be nice to remove the braces of the `else` here. See 
> https://github.com/llvm/llvm-project/issues/55663.
+1. Would you fix it manually here then and fix it later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

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


[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> The one downside to this is that we are no longer stable under multiple 
> compilations of the same file.

This is a moderately serious issue. Some users care about the build 
reproducibility. Recompiling the same sources and getting different results 
will trigger all sorts of red flags that would need to be investigated in order 
to make sure the build is not broken.

> However, this is a very niche use-case and is not supported by Nvidia's CUDA 
> compiler so it likely to be good enough.

The fact that NVCC didn't always generate the same output **was** an issue when 
we were using it for CUDA compilation.
In general, "not supported by NVCC" is not quite applicable here, IMO. The goal 
here is to make clang work correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125904

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


[PATCH] D126157: [clang-format][NFC] Insert/remove braces in clang/lib/Format/

2022-05-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
 do {

From 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
 it's unclear whether the braces should be removed when a block has a do-while 
loop as its single statement. Should I put the braces back and add handling of 
do-while for `RemoveBracesLLVM`?



Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+  if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
 LastBlockComment = &Change;
-  else {
-if ((Change.StartOfBlockComment = LastBlockComment))
+  } else {
+if ((Change.StartOfBlockComment = LastBlockComment)) {
   Change.IndentationOffset =
   Change.StartOfTokenColumn -
   Change.StartOfBlockComment->StartOfTokenColumn;

It would be nice to remove the braces of the `else` here. See 
https://github.com/llvm/llvm-project/issues/55663.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D126226#3532423 , @tra wrote:

> We keep running into the same old underlying issue that we do not have a good 
> way to name/reference specific parts of the compilation pipeline. -Xfoo used 
> to work OK for the linear 'standard' compilation pipeline, but these days 
> when compilation grew from a simple linear pipe it's no longer adequate and 
> we need to extend it.

Yeah, it's getting increasingly complicated to refer to certain portions of the 
compilation toolchain as we start adding more complicated stuff. Just recently 
I had a problem that I wanted to pass an `-Xclang` argument only to the CUDA 
toolchain, and there's no way to do it as far as I can tell. It may be worth 
revisiting this whole concept to support more arbitrary combinations.

> Speaking of triples. I think using triple as the selector is insufficient for 
> general offloading use. We may have offload variants that would use the same 
> triple, but would be compiled using their own pipeline. E.g. the GPU binaries 
> for sm_60 and sm_80 GPUs will use the same nvptx64 triple, but would 
> presumably be lined with different linker instances and may need different 
> options. My understanding is that AMDGPU has even more detailed offload 
> variants (same triple, same GPU arch, different features). I don't know 
> whether it's applicable to OpenMP, though. I think it is. IIRC OpenMP has a 
> way to specialize offload to particular GPU variant and that would probably 
> give you multiple offload targets, all with the same triple.

Yes, it's not a truly generic solution. But I figured that just being able to 
specify it for each "tool-chain" was sufficient for the use-case here and we 
can expand it as needed. I added support for OpenMP to use `--offload-arch` 
recently so we definitely use it. The OpenMP offloading GPU runtime library is 
now built as a static library with `--offload-arch=` for all 32 supported 
architectures currently, it works surprisingly well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

2022-05-23 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D126132#3531643 , @MyDeveloperDay 
wrote:

> LGTM  (sorry I've been slow on the reviews, my day job keeps getting in the 
> way ;-))

Np! It's really that @curdeius and @HazardyKnusperkeks are fast. :)




Comment at: clang/lib/Format/TokenAnnotator.cpp:1189
+  if (Tok->isNot(TT_LambdaArrow) && Tok->Previous &&
+  Tok->Previous->is(tok::kw_noexcept))
 Tok->setType(TT_TrailingReturnArrow);

Ironically, I forgot to add the braces! This was caught and fixed by 
`InsertBraces` in D126157, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126132

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


[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin

2022-05-23 Thread Michiel Derhaeg via Phabricator via cfe-commits
MichielDerhaeg marked an inline comment as done.
MichielDerhaeg added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4760-4762
 // reject options that shouldn't be supported in bitcode
 // also reject kernel/kext
 static const constexpr unsigned kBitcodeOptionIgnorelist[] = {

wanders wrote:
> This comment and variable name are not as accurate any longer.  Maybe moving 
> it inside `if (RawTriple.isOSDarwin())` makes it clearer.
Changed the comment to what I could derive from {D61627}.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125974

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


[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin

2022-05-23 Thread Michiel Derhaeg via Phabricator via cfe-commits
MichielDerhaeg updated this revision to Diff 431479.
MichielDerhaeg added a comment.
Herald added a reviewer: alexander-shaposhnikov.

- Render options for other platforms and test
- comment and propagate list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125974

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/embed-bitcode-elf.c

Index: clang/test/Driver/embed-bitcode-elf.c
===
--- /dev/null
+++ clang/test/Driver/embed-bitcode-elf.c
@@ -0,0 +1,13 @@
+// RUN: %clang -c -target aarch64-linux-android21 %s -fembed-bitcode -o %t.o \
+// RUN: -ffunction-sections -fdata-sections -fstack-protector-strong
+// RUN: llvm-readobj -S %t.o | FileCheck --check-prefix=CHECK %s
+// CHECK:   Name: .text.foo
+// CHECK:   Name: .llvmbc
+// CHECK:   Name: .llvmcmd
+//
+// RUN: llvm-objcopy %t.o --dump-section=.llvmcmd=%t.llvmcmd
+// RUN: FileCheck --check-prefix=CMD %s < %t.llvmcmd
+// CMD: ffunction-sections
+// CMD: fdata-sections
+
+int foo() { return 42; }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4757,54 +4757,91 @@
 // Render target options.
 TC.addClangTargetOptions(Args, CmdArgs, JA.getOffloadingDeviceKind());
 
-// reject options that shouldn't be supported in bitcode
-// also reject kernel/kext
-static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
-options::OPT_mkernel,
-options::OPT_fapple_kext,
-options::OPT_ffunction_sections,
-options::OPT_fno_function_sections,
-options::OPT_fdata_sections,
-options::OPT_fno_data_sections,
-options::OPT_fbasic_block_sections_EQ,
-options::OPT_funique_internal_linkage_names,
-options::OPT_fno_unique_internal_linkage_names,
-options::OPT_funique_section_names,
-options::OPT_fno_unique_section_names,
-options::OPT_funique_basic_block_section_names,
-options::OPT_fno_unique_basic_block_section_names,
-options::OPT_mrestrict_it,
-options::OPT_mno_restrict_it,
-options::OPT_mstackrealign,
-options::OPT_mno_stackrealign,
-options::OPT_mstack_alignment,
-options::OPT_mcmodel_EQ,
-options::OPT_mlong_calls,
-options::OPT_mno_long_calls,
-options::OPT_ggnu_pubnames,
-options::OPT_gdwarf_aranges,
-options::OPT_fdebug_types_section,
-options::OPT_fno_debug_types_section,
-options::OPT_fdwarf_directory_asm,
-options::OPT_fno_dwarf_directory_asm,
-options::OPT_mrelax_all,
-options::OPT_mno_relax_all,
-options::OPT_ftrap_function_EQ,
-options::OPT_ffixed_r9,
-options::OPT_mfix_cortex_a53_835769,
-options::OPT_mno_fix_cortex_a53_835769,
-options::OPT_ffixed_x18,
-options::OPT_mglobal_merge,
-options::OPT_mno_global_merge,
-options::OPT_mred_zone,
-options::OPT_mno_red_zone,
-options::OPT_Wa_COMMA,
-options::OPT_Xassembler,
-options::OPT_mllvm,
-};
-for (const auto &A : Args)
-  if (llvm::is_contained(kBitcodeOptionIgnorelist, A->getOption().getID()))
+for (const auto &A : Args) {
+  // For Apple platforms, the following options are disallowed to limit the
+  // ability to change the backend behaviour and ABI. This is to ensure the
+  // bitcode can be retargeted to certain architectures.
+  static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
+  options::OPT_mkernel,
+  options::OPT_fapple_kext,
+  options::OPT_ffunction_sections,
+  options::OPT_fno_function_sections,
+  options::OPT_fdata_sections,
+  options::OPT_fno_data_sections,
+  options::OPT_fbasic_block_sections_EQ,
+  options::OPT_funique_internal_linkage_names,
+  options::OPT_fno_unique_internal_linkage_names,
+  options::OPT_funique_section_names,
+  options::OPT_fno_unique_section_names,
+  options::OPT_funique_basic_block_section_names,
+  options::OPT_fno_unique_basic_block_section_names,
+  options::OPT_mrestrict_it,
+  options::OPT_mno_restrict_it,
+  options::OPT_mstackrealign,
+  options::OPT_mno_stackrealign,
+  options::OPT_mstack_alignment,
+  options::OPT_mcmodel_EQ,
+  options::OPT_mlong_calls,
+  options::OPT_mno_long_calls,
+  options::OPT_ggnu_pubnames,
+  options::OPT_gdwarf_aranges,
+  options::OPT_fdebug_types_section,
+  options::OPT_fno_debug_types_section,
+  options::OPT_fdwarf_directory_asm,
+  options::OPT_fno_dwarf_directory_asm,
+  options::OPT_mrelax_all,
+  op

Re: [clang] 7aa1fa0 - Reland "[dwarf] Emit a DIGlobalVariable for constant strings."

2022-05-23 Thread David Blaikie via cfe-commits
(when recommitting a patch it can be helpful to mention the revisions
of the previous commit/revert, the reason for the revert and what's
different in this version of the patch that addresses that issue (or
how was the issue otherwise addressed))

On Wed, May 18, 2022 at 1:59 PM Mitch Phillips via cfe-commits
 wrote:
>
>
> Author: Mitch Phillips
> Date: 2022-05-18T13:56:45-07:00
> New Revision: 7aa1fa0a0a07f7949d2d77c099aab43cf9b75a91
>
> URL: 
> https://github.com/llvm/llvm-project/commit/7aa1fa0a0a07f7949d2d77c099aab43cf9b75a91
> DIFF: 
> https://github.com/llvm/llvm-project/commit/7aa1fa0a0a07f7949d2d77c099aab43cf9b75a91.diff
>
> LOG: Reland "[dwarf] Emit a DIGlobalVariable for constant strings."
>
> An upcoming patch will extend llvm-symbolizer to provide the source line
> information for global variables. The goal is to move AddressSanitizer
> off of internal debug info for symbolization onto the DWARF standard
> (and doing a clean-up in the process). Currently, ASan reports the line
> information for constant strings if a memory safety bug happens around
> them. We want to keep this behaviour, so we need to emit debuginfo for
> these variables as well.
>
> Reviewed By: dblaikie, rnk, aprantl
>
> Differential Revision: https://reviews.llvm.org/D123534
>
> Added:
> clang/test/CodeGen/debug-info-variables.c
> llvm/test/DebugInfo/COFF/global-no-strings.ll
>
> Modified:
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/lib/CodeGen/CGDebugInfo.h
> clang/lib/CodeGen/CodeGenModule.cpp
> clang/test/VFS/external-names.c
> llvm/lib/AsmParser/LLParser.cpp
> llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
> llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>
> Removed:
> llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
>
>
> 
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index 3d73bfb8ce79..753427029441 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -5132,7 +5132,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
> Qualified) const {
>  return Name;
>codegenoptions::DebugTemplateNamesKind TemplateNamesKind =
>CGM.getCodeGenOpts().getDebugSimpleTemplateNames();
> -
> +
>if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
>  TemplateNamesKind = codegenoptions::DebugTemplateNamesKind::Full;
>
> @@ -5459,6 +5459,21 @@ void CGDebugInfo::EmitGlobalAlias(const 
> llvm::GlobalValue *GV,
>ImportedDeclCache[GD.getCanonicalDecl().getDecl()].reset(ImportDI);
>  }
>
> +void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
> +const StringLiteral *S) {
> +  SourceLocation Loc = S->getStrTokenLoc(0);
> +  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
> +  if (!PLoc.isValid())
> +return;
> +
> +  llvm::DIFile *File = getOrCreateFile(Loc);
> +  llvm::DIGlobalVariableExpression *Debug =
> +  DBuilder.createGlobalVariableExpression(
> +  nullptr, StringRef(), StringRef(), getOrCreateFile(Loc),
> +  getLineNumber(Loc), getOrCreateType(S->getType(), File), true);
> +  GV->addDebugInfo(Debug);
> +}
> +
>  llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
>if (!LexicalBlockStack.empty())
>  return LexicalBlockStack.back();
>
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.h 
> b/clang/lib/CodeGen/CGDebugInfo.h
> index 8984f3eb1d7a..38e3fa5b2fa9 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.h
> +++ b/clang/lib/CodeGen/CGDebugInfo.h
> @@ -533,6 +533,14 @@ class CGDebugInfo {
>/// Emit an @import declaration.
>void EmitImportDecl(const ImportDecl &ID);
>
> +  /// DebugInfo isn't attached to string literals by default. While certain
> +  /// aspects of debuginfo aren't useful for string literals (like a name), 
> it's
> +  /// nice to be able to symbolize the line and column information. This is
> +  /// especially useful for sanitizers, as it allows symbolization of
> +  /// heap-buffer-overflows on constant strings.
> +  void AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
> + const StringLiteral *S);
> +
>/// Emit C++ namespace alias.
>llvm::DIImportedEntity *EmitNamespaceAlias(const NamespaceAliasDecl &NA);
>
>
> diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
> b/clang/lib/CodeGen/CodeGenModule.cpp
> index f8bf210dc0e2..703cf4edf5f5 100644
> --- a/clang/lib/CodeGen/CodeGenModule.cpp
> +++ b/clang/lib/CodeGen/CodeGenModule.cpp
> @@ -5670,6 +5670,11 @@ 
> CodeGenModule::GetAddrOfConstantStringFromLiteral(const StringLiteral *S,
>}
>
>auto GV = GenerateStringLiteral(C, LT, *this, GlobalVariableName, 
> Alignment);
> +
> +  CGDebugInfo *DI = getModuleDebugInfo();
> +  if (DI && getCodeGenOpts().hasReducedDebugInfo())
> +DI->AddStringLiteralDebugInfo(GV, S);
> +
>if (Entry)
>  *Entr

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D126226#3532257 , @MaskRay wrote:

> It's better to avoid `JoinedAndSeparate` for new options. It is for `--xxx 
> val` and `--xxxval` but not intended for the option this patch will add.

I'm not sure I understand your argument. The two cases where I see 
`JoinedAndSeparate` are used right now (`-Xarch_` and `-plugin-arg`) both are 
using it for the purposes similar to this patch.
I also do not quite see how `JoinedAndSeparate` is applicable to 
`--xxxval`/`--xxx val`.
Could you elaborate, please?

In D126226#3532301 , @MaskRay wrote:

> Consider something like `-Xoffload-linker-triple =`

That could work.

We keep running into the same old underlying issue that we do not have a good 
way to name/reference specific parts of the compilation pipeline. -Xfoo used to 
work OK for the linear 'standard' compilation pipeline, but these days when 
compilation grew from a simple linear pipe it's no longer adequate and we need 
to extend it.

Speaking of triples. I think using triple as the selector is insufficient for 
general offloading use. We may have offload variants that would use the same 
triple, but would be compiled using their own pipeline. E.g. the GPU binaries 
for sm_60 and sm_80 GPUs will use the same nvptx64 triple, but would presumably 
be lined with different linker instances and may need different options. My 
understanding is that AMDGPU has even more detailed offload variants (same 
triple, same GPU arch, different features). I don't know whether it's 
applicable to OpenMP, though. I think it is. IIRC OpenMP has a way to 
specialize offload to particular GPU variant and that would probably give you 
multiple offload targets, all with the same triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 431475.
jhuber6 added a comment.

Updating to use @MaskRay's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -108,6 +108,12 @@
   cl::desc("Argument to pass to the ptxas invocation"),
   cl::cat(ClangLinkerWrapperCategory));
 
+static cl::list
+LinkerArgs("device-linker", cl::ZeroOrMore,
+   cl::desc("Arguments to pass to the device linker invocation"),
+   cl::value_desc(" or ="),
+   cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt Verbose("v", cl::ZeroOrMore,
  cl::desc("Verbose output from tools"),
  cl::init(false),
@@ -226,6 +232,17 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
+// Forward user requested arguments to the device linking job.
+void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
+  for (StringRef Arg : LinkerArgs) {
+auto TripleAndValue = Arg.split('=');
+if (TripleAndValue.second.empty())
+  Args.push_back(TripleAndValue.first);
+else if (TripleAndValue.first == Triple)
+  Args.push_back(TripleAndValue.second);
+  }
+}
+
 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
@@ -531,6 +548,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
 return std::move(Err);
 
@@ -599,6 +617,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*LLDPath, CmdArgs))
 return std::move(Err);
 
@@ -676,6 +695,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
 return std::move(Err);
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -104,3 +104,9 @@
 // RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-LIBRARY %s
 
 // CHECK-LTO-LIBRARY: {{.*}}-lomptarget{{.*}}-lomptarget.devicertl
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -Xoffload-linker a -Xoffload-linker nvptx64-nvidia-cuda=b \
+// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
+
+// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}--
Index: clang/test/Driver/linker-wrapper.c
===
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -80,3 +80,15 @@
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_52 {{.*}}.o
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
 // CUDA: fatbinary{{.*}}-64 --create {{.*}}.fatbin --image=profile=sm_52,file={{.*}}.out --image=profile=sm_70,file={{.*}}.out
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --dry-run --host-triple x86_64-unknown-linux-gnu -linker-path \
+// RUN:   /usr/bin/ld --device-linker=a --device-linker=nvptx64-nvidia-cuda=b -- \
+// RUN:   %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LINKER_ARGS
+
+// LINKER_ARGS: lld{{.*}}-flavor gnu --no-undefined -shared -o {{.*}}.out {{.*}}.o a
+// LINKER_ARGS: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o a b
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8390,6 +8390,12 @@
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
   const auto &LinkCommand = C.getJobs().getJobs().back();
 
+  // Forward -Xoffload-linker arguments to the device link job.
+  for 

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

2022-05-23 Thread Mitch Phillips 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 rGcead4eceb01b: [symbolizer] Parse DW_TAG_variable DIs to show 
line info for globals (authored by hctim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

Files:
  llvm/include/llvm/DebugInfo/DIContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/PDB/PDBContext.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/PDB/PDBContext.cpp
  llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp
  llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
  llvm/test/DebugInfo/Symbolize/ELF/data-command-symtab.yaml
  llvm/test/tools/llvm-symbolizer/data-location.yaml
  llvm/test/tools/llvm-symbolizer/data.s

Index: llvm/test/tools/llvm-symbolizer/data.s
===
--- llvm/test/tools/llvm-symbolizer/data.s
+++ llvm/test/tools/llvm-symbolizer/data.s
@@ -7,9 +7,12 @@
 
 # CHECK:  d1
 # CHECK-NEXT: 0 8
+# CHECK-NEXT: ??:?
 # CHECK-EMPTY:
 # CHECK-NEXT: d2
 # CHECK-NEXT: 8 4
+# CHECK-NEXT: ??:?
+# CHECK-EMPTY:
 
 d1:
 .quad 0x1122334455667788
Index: llvm/test/tools/llvm-symbolizer/data-location.yaml
===
--- /dev/null
+++ llvm/test/tools/llvm-symbolizer/data-location.yaml
@@ -0,0 +1,450 @@
+## Show that when "DATA" is used with an address, it forces the found location
+## to be symbolized as data, including the source information.
+
+# RUN: yaml2obj %s -o %t.so
+
+# RUN: llvm-symbolizer 'DATA 0x304d0' 'DATA 0x304d1' 'DATA 0x304d3' \
+# RUN:   'DATA 0x304c0' 'DATA 0x304c8' 'DATA 0x304d4' 'DATA 0x304dc' \
+# RUN:   'DATA 0x304d8' --obj=%t.so | FileCheck %s
+
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Check that lookups in the middle of the symbol are also resolved correctly.
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Now, the remainder of the symbols.
+# CHECK-NEXT: data_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:2
+# CHECK-EMPTY:
+# CHECK-NEXT: str
+# CHECK-NEXT: {{[0-9]+}} 8
+# CHECK-NEXT: /tmp/file.cpp:4
+# CHECK-EMPTY:
+# CHECK-NEXT: f()::function_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:8
+# CHECK-EMPTY:
+
+## Including the one that includes an addend.
+# CHECK-NEXT: alpha
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:12
+# CHECK-EMPTY:
+# CHECK-NEXT: beta
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:13
+# CHECK-EMPTY:
+
+## Ensure there's still a global that's offset-based.
+# RUN: llvm-dwarfdump --debug-info %t.so | FileCheck %s --check-prefix=OFFSET
+
+# OFFSET: DW_AT_location (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)
+
+
+## File below was generated using:
+##
+##   $ clang++ -g -O3 /tmp/file.cpp -shared -fuse-ld=lld -nostdlib \
+## -target aarch64-linux-gnuabi -mllvm -global-merge-ignore-single-use \
+## -o /tmp/file.so
+##
+##  With /tmp/file.cpp as:
+##1: int bss_global;
+##2: int data_global = 2;
+##3:
+##4: const char* str =
+##5: "12345678";
+##6:
+##7: int* f() {
+##8:   static int function_global;
+##9:   return &function_global;
+##   10: }
+##   11:
+##   12: static int alpha;
+##   13: static int beta;
+##   14: int *f(bool b) { return beta ? &alpha : β }
+##   15:
+##
+## ... then, one can get the offsets using `nm`, like:
+##   $ nm out.so | grep bss_global
+## 38fc B bss_global
+##
+## Note the use of the aarch64 target (with -nostdlib in order to allow linkage
+## without libraries for cross-compilation) as well as -O3 and
+## -global-merge-ignore-single-use. This is a specific combination that makes
+## the compiler emit the `alpha` global variable with a more complex
+## DW_AT_location than just a DW_OP_addr/DW_OP_addrx. In this instance, it
+## outputs a `DW_AT_location  (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)`.
+##
+## Ideally, this would be tested by invoking clang directly on a C source file,
+## but unfortunately there's no way to do that for LLVM tests. The other option
+## is to compile IR to an objfile, but llvm-symbolizer doesn't understand that
+## two symbols can have the same address in different sections. In the code
+## above, for example, we'd have bss_global at .bss+0x0, and data_global at
+## .data+0x0, and so the symbolizer would only print one of them. Hence, we have
+## the

[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth added a comment.

I'm going to try to summarize an offline discussion w/ @mcgrathr about this 
here:

There are some other considerations to think about w.r.t. emitting names for 
non-source language constructs, as would be the case here. In fact, DWARF 
already handles this case: the "this" pointer in C++. Despite the fact that it 
may not be directly used in the source program, it is tagged as artificial, and 
gets a name, so that programmers can reference it within the debugger.  
Debuggers don't know anything special about C++,  the compiler just emitted a  
DWARF variable that has the `DW_AT_name` of "this". They just reference the 
name, and allow users to access it as if it were another variable.

The point is that the `DW_AT_name` has a special meaning to debuggers, which 
may be problematic if we try to emit a name when this isn't really a source 
level construct and isn't something that should be accessed like another 
variable. Generating a name for these items makes them addressable, and is 
//probably// more problematic for downstream consumers of DWARF than omitting 
the name.  Generally, DWARF consumers probably shouldn't be relying on things 
like the name always being there. The standard is pretty clear that many fields 
are optional, i.e., they //may// be there or they //may not// be there.

It's probably fine to gate the new DWARF items behind a compiler flag if there 
is concern about incompatible consumers. It could even be on by default, but at 
least it would be possible to opt out of the new behavior. You may even be able 
to gate it based on the DWARF version. That isn't strictly correct, but is 
offered as a possible pragmatic convenience on the rationale that concern about 
incompatible consumers probably applies to "old" consumers and consumers new 
enough to handle v5 could be assumed new/well-maintained enough to either 
already handle, or quickly be fixed to handle, nameless `DW_TAG_variable`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126224

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

IIRC there is no built-in way supporting multiple (but fixed number of) values 
for an option (e.g. `-Xoffload-linker- `). In D105330 
 (llvm-nm option refactoring) I used a hack 
to support `-s __DATA __data`.
The multiple-value support for OptTable does not allow positional arguments 
after the option.

Consider something like `-Xoffload-linker-triple =`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

2022-05-23 Thread Mitch Phillips via Phabricator via cfe-commits
hctim updated this revision to Diff 431463.
hctim marked 4 inline comments as done.
hctim added a comment.

David's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

Files:
  llvm/include/llvm/DebugInfo/DIContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/PDB/PDBContext.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/PDB/PDBContext.cpp
  llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp
  llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
  llvm/test/DebugInfo/Symbolize/ELF/data-command-symtab.yaml
  llvm/test/tools/llvm-symbolizer/data-location.yaml
  llvm/test/tools/llvm-symbolizer/data.s

Index: llvm/test/tools/llvm-symbolizer/data.s
===
--- llvm/test/tools/llvm-symbolizer/data.s
+++ llvm/test/tools/llvm-symbolizer/data.s
@@ -7,9 +7,12 @@
 
 # CHECK:  d1
 # CHECK-NEXT: 0 8
+# CHECK-NEXT: ??:?
 # CHECK-EMPTY:
 # CHECK-NEXT: d2
 # CHECK-NEXT: 8 4
+# CHECK-NEXT: ??:?
+# CHECK-EMPTY:
 
 d1:
 .quad 0x1122334455667788
Index: llvm/test/tools/llvm-symbolizer/data-location.yaml
===
--- /dev/null
+++ llvm/test/tools/llvm-symbolizer/data-location.yaml
@@ -0,0 +1,450 @@
+## Show that when "DATA" is used with an address, it forces the found location
+## to be symbolized as data, including the source information.
+
+# RUN: yaml2obj %s -o %t.so
+
+# RUN: llvm-symbolizer 'DATA 0x304d0' 'DATA 0x304d1' 'DATA 0x304d3' \
+# RUN:   'DATA 0x304c0' 'DATA 0x304c8' 'DATA 0x304d4' 'DATA 0x304dc' \
+# RUN:   'DATA 0x304d8' --obj=%t.so | FileCheck %s
+
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Check that lookups in the middle of the symbol are also resolved correctly.
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Now, the remainder of the symbols.
+# CHECK-NEXT: data_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:2
+# CHECK-EMPTY:
+# CHECK-NEXT: str
+# CHECK-NEXT: {{[0-9]+}} 8
+# CHECK-NEXT: /tmp/file.cpp:4
+# CHECK-EMPTY:
+# CHECK-NEXT: f()::function_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:8
+# CHECK-EMPTY:
+
+## Including the one that includes an addend.
+# CHECK-NEXT: alpha
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:12
+# CHECK-EMPTY:
+# CHECK-NEXT: beta
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:13
+# CHECK-EMPTY:
+
+## Ensure there's still a global that's offset-based.
+# RUN: llvm-dwarfdump --debug-info %t.so | FileCheck %s --check-prefix=OFFSET
+
+# OFFSET: DW_AT_location (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)
+
+
+## File below was generated using:
+##
+##   $ clang++ -g -O3 /tmp/file.cpp -shared -fuse-ld=lld -nostdlib \
+## -target aarch64-linux-gnuabi -mllvm -global-merge-ignore-single-use \
+## -o /tmp/file.so
+##
+##  With /tmp/file.cpp as:
+##1: int bss_global;
+##2: int data_global = 2;
+##3:
+##4: const char* str =
+##5: "12345678";
+##6:
+##7: int* f() {
+##8:   static int function_global;
+##9:   return &function_global;
+##   10: }
+##   11:
+##   12: static int alpha;
+##   13: static int beta;
+##   14: int *f(bool b) { return beta ? &alpha : β }
+##   15:
+##
+## ... then, one can get the offsets using `nm`, like:
+##   $ nm out.so | grep bss_global
+## 38fc B bss_global
+##
+## Note the use of the aarch64 target (with -nostdlib in order to allow linkage
+## without libraries for cross-compilation) as well as -O3 and
+## -global-merge-ignore-single-use. This is a specific combination that makes
+## the compiler emit the `alpha` global variable with a more complex
+## DW_AT_location than just a DW_OP_addr/DW_OP_addrx. In this instance, it
+## outputs a `DW_AT_location  (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)`.
+##
+## Ideally, this would be tested by invoking clang directly on a C source file,
+## but unfortunately there's no way to do that for LLVM tests. The other option
+## is to compile IR to an objfile, but llvm-symbolizer doesn't understand that
+## two symbols can have the same address in different sections. In the code
+## above, for example, we'd have bss_global at .bss+0x0, and data_global at
+## .data+0x0, and so the symbolizer would only print one of them. Hence, we have
+## the ugly dso-to-yaml blob below.
+##
+## For now, constant strings don't have a debuginfo entry, and so can't be
+## symbolized co

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D126226#3532257 , @MaskRay wrote:

> It's better to avoid `JoinedAndSeparate` for new options. It is for `--xxx 
> val` and `--xxxval` but not intended for the option this patch will add.

So how should I pass these two arguments instead? I'm not sure if there's a 
good way to bind two arguments to a single command line arguments that's 
readable if we're not allowed to use this joined version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: StephenFan.

It's better to avoid `JoinedAndSeparate` for new options. It is for `--xxx val` 
and `--xxxval` but not intended for the option this patch will add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[clang] 217531f - [PS5] Make driver's PIC behavior match PS4

2022-05-23 Thread Paul Robinson via cfe-commits

Author: Paul Robinson
Date: 2022-05-23T12:50:22-07:00
New Revision: 217531f12b4b97dadb80c66ab97c71e57ae0adcf

URL: 
https://github.com/llvm/llvm-project/commit/217531f12b4b97dadb80c66ab97c71e57ae0adcf
DIFF: 
https://github.com/llvm/llvm-project/commit/217531f12b4b97dadb80c66ab97c71e57ae0adcf.diff

LOG: [PS5] Make driver's PIC behavior match PS4

The new test is a copy of the corresponding PS4 test, with the triple
etc updated, because there's currently no good way to make one lit test
"iterate" with multiple targets.

Added: 
clang/test/Driver/ps5-pic.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/CommonArgs.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 7961b006a9a00..3cc9565e6e8b2 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -546,8 +546,8 @@ def warn_drv_unable_to_find_directory_expected : Warning<
   "unable to find %0 directory, expected to be in '%1'">,
   InGroup, DefaultIgnore;
 
-def warn_drv_ps4_force_pic : Warning<
-  "option '%0' was ignored by the PS4 toolchain, using '-fPIC'">,
+def warn_drv_ps_force_pic : Warning<
+  "option '%0' was ignored by the %1 toolchain, using '-fPIC'">,
   InGroup;
 
 def warn_drv_ps_sdk_dir : Warning<

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 33f3df80a2a2e..45add8ad94a5f 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1286,23 +1286,24 @@ tools::ParsePICArgs(const ToolChain &ToolChain, const 
ArgList &Args) {
 O.matches(options::OPT_fPIE) || O.matches(options::OPT_fPIC);
   } else {
 PIE = PIC = false;
-if (EffectiveTriple.isPS4()) {
+if (EffectiveTriple.isPS()) {
   Arg *ModelArg = Args.getLastArg(options::OPT_mcmodel_EQ);
   StringRef Model = ModelArg ? ModelArg->getValue() : "";
   if (Model != "kernel") {
 PIC = true;
-ToolChain.getDriver().Diag(diag::warn_drv_ps4_force_pic)
-<< LastPICArg->getSpelling();
+ToolChain.getDriver().Diag(diag::warn_drv_ps_force_pic)
+<< LastPICArg->getSpelling()
+<< (EffectiveTriple.isPS4() ? "PS4" : "PS5");
   }
 }
   }
 }
   }
 
-  // Introduce a Darwin and PS4-specific hack. If the default is PIC, but the
-  // PIC level would've been set to level 1, force it back to level 2 PIC
+  // Introduce a Darwin and PS4/PS5-specific hack. If the default is PIC, but
+  // the PIC level would've been set to level 1, force it back to level 2 PIC
   // instead.
-  if (PIC && (Triple.isOSDarwin() || EffectiveTriple.isPS4()))
+  if (PIC && (Triple.isOSDarwin() || EffectiveTriple.isPS()))
 IsPICLevelTwo |= ToolChain.isPICDefault();
 
   // This kernel flags are a trump-card: they will disable PIC/PIE

diff  --git a/clang/test/Driver/ps5-pic.c b/clang/test/Driver/ps5-pic.c
new file mode 100644
index 0..0396122accf40
--- /dev/null
+++ b/clang/test/Driver/ps5-pic.c
@@ -0,0 +1,106 @@
+// REQUIRES: x86-registered-target
+
+// Test the driver's control over the PIC behavior for PS5 compiler.
+// These consist of tests of the relocation model flags and the
+// pic level flags passed to CC1.
+//
+// CHECK-NO-PIC: "-mrelocation-model" "static"
+// CHECK-NO-PIC-NOT: "-pic-level"
+// CHECK-NO-PIC-NOT: "-pic-is-pie"
+//
+// CHECK-DYNAMIC-NO-PIC2: unsupported option '-mdynamic-no-pic'
+// CHECK-DYNAMIC-NO-PIC2: "-mrelocation-model" "dynamic-no-pic"
+//
+// CHECK-PIC2: "-mrelocation-model" "pic"
+// CHECK-PIC2: "-pic-level" "2"
+//
+// CHECK-PIE2: "-mrelocation-model" "pic"
+// CHECK-PIE2: "-pic-is-pie"
+//
+// CHECK-NOPIC-IGNORED: using '-fPIC'
+// CHECK-NOPIC-IGNORED: "-mrelocation-model" "pic"
+// CHECK-NOPIC-IGNORED: "-pic-level" "2"
+//
+// CHECK-DIAG-PIC: option '-fno-PIC' was ignored by the PS5 toolchain, using 
'-fPIC'
+// CHECK-DIAG-PIE: option '-fno-PIE' was ignored by the PS5 toolchain, using 
'-fPIC'
+// CHECK-DIAG-pic: option '-fno-pic' was ignored by the PS5 toolchain, using 
'-fPIC'
+// CHECK-DIAG-pie: option '-fno-pie' was ignored by the PS5 toolchain, using 
'-fPIC'
+//
+// CHECK-STATIC-ERR: unsupported option '-static' for target 'PS5'
+
+// RUN: %clang -c %s -target x86_64-sie-ps5 -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target x86_64-sie-ps5 -fpic -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target x86_64-sie-ps5 -fPIC -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIC2
+// RUN: %clang -c %s -target x86_64-sie-ps5 -fpie -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PIE2
+// RUN: %clang -c %s -target x86_64-

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 431452.
jhuber6 added a comment.

Changing the `-Xoffload-linker=` to `-Xoffload-linker-`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -108,6 +108,12 @@
   cl::desc("Argument to pass to the ptxas invocation"),
   cl::cat(ClangLinkerWrapperCategory));
 
+static cl::list
+LinkerArgs("device-linker", cl::ZeroOrMore,
+   cl::desc("Arguments to pass to the device linker invocation"),
+   cl::value_desc(" or ="),
+   cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt Verbose("v", cl::ZeroOrMore,
  cl::desc("Verbose output from tools"),
  cl::init(false),
@@ -226,6 +232,17 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
+// Forward user requested arguments to the device linking job.
+void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
+  for (StringRef Arg : LinkerArgs) {
+auto TripleAndValue = Arg.split('=');
+if (TripleAndValue.second.empty())
+  Args.push_back(TripleAndValue.first);
+else if (TripleAndValue.first == Triple)
+  Args.push_back(TripleAndValue.second);
+  }
+}
+
 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
@@ -531,6 +548,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
 return std::move(Err);
 
@@ -599,6 +617,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*LLDPath, CmdArgs))
 return std::move(Err);
 
@@ -676,6 +695,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
 return std::move(Err);
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -104,3 +104,9 @@
 // RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-LIBRARY %s
 
 // CHECK-LTO-LIBRARY: {{.*}}-lomptarget{{.*}}-lomptarget.devicertl
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -Xoffload-linker a -Xoffload-linker-nvptx64-nvidia-cuda b -Xoffload-linker-nvptx64 c \
+// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
+
+// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}-device-linker=nvptx64-nvidia-cuda=c{{.*}}--
Index: clang/test/Driver/linker-wrapper.c
===
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -80,3 +80,15 @@
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_52 {{.*}}.o
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
 // CUDA: fatbinary{{.*}}-64 --create {{.*}}.fatbin --image=profile=sm_52,file={{.*}}.out --image=profile=sm_70,file={{.*}}.out
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --dry-run --host-triple x86_64-unknown-linux-gnu -linker-path \
+// RUN:   /usr/bin/ld --device-linker=a --device-linker=nvptx64-nvidia-cuda=b -- \
+// RUN:   %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LINKER_ARGS
+
+// LINKER_ARGS: lld{{.*}}-flavor gnu --no-undefined -shared -o {{.*}}.out {{.*}}.o a
+// LINKER_ARGS: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o a b
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8390,6 +8390,18 @@
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
   const auto &LinkCommand = C.getJobs().getJo

[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D126226#3532216 , @tra wrote:

> You do not need to hardcode it. The idea of `JoinedAndSeparate` is that an 
> option `foo` assepts two argumants, one glued to it and another following 
> after a whitespace.
> So, when you define an option `-Xoffload-linker-`, and then pass 
> `-Xoffload-linker-nvptx64=foo`, you will get `OPT_offload-linker__` with two 
> arguments. As an example see implementation of `plugin_arg` which deals with 
> the same kind of problem of passing arguments to an open-ended set of plugins.

I see, it's a little weird sinec the `-Xopenmp-target` option will be done 
different, but changing to this should just require switching out the `=` with 
`-`. I'll go ahead and do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126172: [clang] Fix comparison of TemplateArgument when they are of template kind

2022-05-23 Thread Robert Esclapez via Phabricator via cfe-commits
roberteg16 updated this revision to Diff 431451.
roberteg16 added a comment.

Fix wrongly moved clang-format disabling comment when trying to fix clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126172

Files:
  clang/lib/AST/TemplateBase.cpp
  clang/test/CXX/dcl/dcl.fct/p17.cpp


Index: clang/test/CXX/dcl/dcl.fct/p17.cpp
===
--- clang/test/CXX/dcl/dcl.fct/p17.cpp
+++ clang/test/CXX/dcl/dcl.fct/p17.cpp
@@ -257,4 +257,23 @@
   static_assert(is_same_v);
   // expected-error@-1{{no matching}}
   static_assert(is_same_v);
-}
+
+  template  struct S3 {};
+  // expected-note@-1 {{'S3' declared here}}
+  // expected-note@-2 {{template is declared here}}
+  // clang-format off
+  void f23(C2<::S3> auto);
+  // expected-error@-1 {{no template named 'S3' in the global namespace; did 
you mean simply 'S3'?}}
+  // expected-error@-2 {{use of class template '::S3' requires template 
arguments}}
+  // clang-format on
+  } // namespace constrained
+
+  template  struct S4 {};
+  // expected-note@-1 {{template is declared here}}
+
+  namespace constrained {
+  // clang-format off
+  void f24(C2<::S4> auto);
+  // expected-error@-1 {{use of class template '::S4' requires template 
arguments}}
+  // clang-format on
+  } // namespace constrained
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -370,7 +370,8 @@
 
   case Template:
   case TemplateExpansion:
-return TemplateArg.Name == Other.TemplateArg.Name &&
+return getAsTemplateOrTemplatePattern().getAsTemplateDecl() ==
+   Other.getAsTemplateOrTemplatePattern().getAsTemplateDecl() &&
TemplateArg.NumExpansions == Other.TemplateArg.NumExpansions;
 
   case Declaration:


Index: clang/test/CXX/dcl/dcl.fct/p17.cpp
===
--- clang/test/CXX/dcl/dcl.fct/p17.cpp
+++ clang/test/CXX/dcl/dcl.fct/p17.cpp
@@ -257,4 +257,23 @@
   static_assert(is_same_v);
   // expected-error@-1{{no matching}}
   static_assert(is_same_v);
-}
+
+  template  struct S3 {};
+  // expected-note@-1 {{'S3' declared here}}
+  // expected-note@-2 {{template is declared here}}
+  // clang-format off
+  void f23(C2<::S3> auto);
+  // expected-error@-1 {{no template named 'S3' in the global namespace; did you mean simply 'S3'?}}
+  // expected-error@-2 {{use of class template '::S3' requires template arguments}}
+  // clang-format on
+  } // namespace constrained
+
+  template  struct S4 {};
+  // expected-note@-1 {{template is declared here}}
+
+  namespace constrained {
+  // clang-format off
+  void f24(C2<::S4> auto);
+  // expected-error@-1 {{use of class template '::S4' requires template arguments}}
+  // clang-format on
+  } // namespace constrained
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -370,7 +370,8 @@
 
   case Template:
   case TemplateExpansion:
-return TemplateArg.Name == Other.TemplateArg.Name &&
+return getAsTemplateOrTemplatePattern().getAsTemplateDecl() ==
+   Other.getAsTemplateOrTemplatePattern().getAsTemplateDecl() &&
TemplateArg.NumExpansions == Other.TemplateArg.NumExpansions;
 
   case Declaration:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D126226#3532147 , @jhuber6 wrote:

> We already use this approach for the `-Xopenmp-target= ` option 
> that forwards arguments to the given toolchain, so that's what I was using as 
> a basis.

Yup. I've noticed that. It's unfortunate.

> I'm not sure if I want to hard-code the triple value into the argument 
> itself, since this could theoretically be used for any number of triples, 
> e.g. OpenMP offloading to `ppcle64` and `x86_64`, so it would get a little 
> messy. It's definitely a bit harder to read, but it's a bit of a special-case 
> option so it's to be expected I think.

You do not need to hardcode it. The idea of `JoinedAndSeparate` is that an 
option `foo` assepts two argumants, one glued to it and another following after 
a whitespace.
So, when you define an option `-Xoffload-linker-`, and then pass 
`-Xoffload-linker-nvptx64=foo`, you will get `OPT_offload-linker__` with two 
arguments. As an example see implementation of `plugin_arg` which deals with 
the same kind of problem of passing arguments to an open-ended set of plugins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D126226#3532127 , @tra wrote:

> `-Xoffload-linker= `
>
> The syntax is confusing. Normally only `triple` would be the argument for 
> `-Xoffload-linker` option. 
> Having both `-Xoffload-linker` and `-Xoffload-linker=` variants also looks 
> odd to me.
>
> In effect you're making `-Xoffload-linker=foo` a full option (as opposed to 
> it being an option `-Xoffload-linker= ` + argument `foo`) with a separate 
> argument that follows. I guess that might work, but it's a rather 
> unconventional use of command line parser, IMO.
>
> I think the main issue with this approach is that it makes the command line 
> hard to understand. When one sees `-Xsomething=a -b` it's impossible to tell 
> whether `-b` is a regular option or something to be passed to 
> `-Xsomething=a`. My assumption would be the former as `-Xsomething=` already 
> got its argument `a` and should have no business grabbing the next one.
>
> I think it would work better if the option could use a `-` or`_` for the 
> variant that passes the triple. E.g. `-Xoffload-linker-nvptx64=-foo`  or 
> `-Xoffload-linker-nvptx64 -foo` would be easily interpretable.

We already use this approach for the `-Xopenmp-targets= ` option 
that forwards arguments to the given toolchain, so that's what I was using as a 
basis. I'm not sure if I want to hard-code the triple value into the argument 
itself, since this could theoretically be used for any number of triples, e.g. 
OpenMP offloading to `ppcle64` and `x86_64`, so it would get a little messy. 
It's definitely a bit harder to read, but it's a bit of a special-case option 
so it's to be expected I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

`-Xoffload-linker= `

The syntax is confusing. Normally only `triple` would be the argument for 
`-Xoffload-linker` option. 
Having both `-Xoffload-linker` and `-Xoffload-linker=` variants also looks odd 
to me.

In effect you're making `-Xoffload-linker=foo` a full option (as opposed to it 
being an option `-Xoffload-linker= ` + argument `foo`) with a separate argument 
that follows. I guess that might work, but it's a rather unconventional use of 
command line parser, IMO.

I think the main issue with this approach is that it makes the command line 
hard to understand. When one sees `-Xsomething=a -b` it's impossible to tell 
whether `-b` is a regular option or something to be passed to `-Xsomething=a`. 
My assumption would be the former as `-Xsomething=` already got its argument 
`a` and should have no business grabbing the next one.

I think it would work better if the option could use a `-` or`_` for the 
variant that passes the triple. E.g. `-Xoffload-linker-nvptx64=-foo`  or 
`-Xoffload-linker-nvptx64 -foo` would be easily interpretable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

2022-05-23 Thread Balázs Benics via Phabricator via cfe-commits
steakhal planned changes to this revision.
steakhal added a comment.

In D126215#3531780 , @martong wrote:

>> We should support deprecated analyzer flags for at least one release. In 
>> this case I'm planning to drop this flag in clang-17
>
> Should we emit a warning for the user about the deprecation?

Definitely!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126215

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


[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi abandoned this revision.
akyrtzi added a comment.

This has been superseded by the above set of patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124687

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Mark Dewing via Phabricator via cfe-commits
markdewing accepted this revision.
markdewing added a comment.
This revision is now accepted and ready to land.

Works for passing libraries to nvlink.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126226

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


[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

2022-05-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

In D126137#3530777 , @kristof.beyls 
wrote:

> Therefore, I wonder if it wouldn't be better to name this -mharden-sls=retbr 
> for more consistency across architectures?

I think it's best to maintain compatibility with GCC; to do so otherwise might 
be surprising for users.

> Or is the indirect function call case not relevant for x86 (sorry - I'm not 
> up to speed on the details on the x86 side)?

Looks like GCC does not instrument indirect calls from what I can tell:

  $ cat x.c
  void bar(void (*x)(void)) {
x();
x();
  }
  $ gcc -mharden-sls=all x.c -c -O2
  $ llvm-objdump -dr x.o   
  ...
   :
 0: 53  pushq   %rbx
 1: 48 89 fbmovq%rdi, %rbx
 4: ff d7   callq   *%rdi
 6: 48 89 d8movq%rbx, %rax
 9: 5b  popq%rbx
 a: ff e0   jmpq*%rax
 c: cc  int3

so the indirect `call` instruction is not hardened. The indirect jmp (tail 
call) is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126137

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


[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

2022-05-23 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

Thanks for the patch!

> A tangential question is do we need these two options expect all?

For the Linux kernel's use, no. But I think it would extremely simple to 
implement. Instead of having one `SubtargetFeature` (`FeatureHardenSlsAll`), 
you'd have two (say `FeatureHardenSlsRet` and `FeatureHardenSlsInd`). Then the 
front-end decomposes `-mharden-sls=all` into BOTH `SubtargetFeatures`.  You've 
already implemented support for either in `X86AsmPrinter::emitBasicBlockEnd`. 
You condition could instead become something like `if (... && 
((Subtarget->hardenSlsRet() && I->getDesc().isReturn()) || 
(Subtarget->hardenSlsInd() && I->getDesc().isIndirectBranch(`. You'd also 
need the frontend to recognize the two additional values. Do you think that's 
doable @pengfei ?

---

Consider the following test case:

  void foo(void);
  
  void bar(void) {
void (*x)(void) = foo;
x();
  }

With this patch, `clang -mharden-sls=all x.c -c -O2` produces:

   :
 0: e9 00 00 00 00  jmp 0x5 
0001:  R_X86_64_PLT32   foo-0x4
 5: cc  int3

while `gcc -mharden-sls=all x.c -c -O2` produces

   :
 0: e9 00 00 00 00  jmp 0x5 
0001:  R_X86_64_PLT32   foo-0x4

So we pessimize tail calls. Please fix and add a test case for that. This might 
be an unintended side effect of using `isUnconditionalBranch`.

---

Relevant GCC commits:
c2e5c4feed32c808591b5278f680bbabe63eb225 ("x86: Generate INT3 for 
__builtin_eh_return")
53a643f8568067d7700a9f2facc8ba39974973d3 ("x86: Add 
-mharden-sls=[none|all|return|indirect-branch]")

The first seems to have some interaction between `-fcf-protection` and 
`__builtin_eh_return`. Is that something we need to handle?




Comment at: clang/docs/ReleaseNotes.rst:371
 
+- Support ``-mharden-sls=all`` for X86.
+

This should be updated if additional options are supported.

You should also update `clang/docs/ClangCommandLineReference.rst` I think.



Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:249-250
+StringRef Scope = A->getValue();
+if (Scope == "all")
+  Features.push_back("+harden-sls-all");
+  }

```
else
D.Diag(diag::err_invalid_sls_hardening)
<< Scope << A->getAsString(Args);
```
and add a test for a nonsensical value.



Comment at: clang/test/Driver/x86-target-features.c:308-309
+
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=all %s 
-### -o %t.o 2>&1 | FileCheck -check-prefix=SLS %s
+// RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mharden-sls=none %s 
-### -o %t.o 2>&1 | FileCheck -check-prefix=NO-SLS %s
+// SLS: "-target-feature" "+harden-sls-all"

Please add a test for `-mharden-sls=all -mharden-sls=none` to verify that the 
last value "wins."



Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:342-343
+  auto I = MBB.getLastNonDebugInstr();
+  if (I != MBB.end() && Subtarget->hardenSlsAll() &&
+  (I->getDesc().isUnconditionalBranch() || I->getDesc().isReturn())) {
+MCInst TmpInst;

Consider checking `Subtarget->hardenSlsAll()` first, since that's most unlikely 
to be unset.

```
if (Subtarget->hardenSlsAll()) {
  auto I = ...
}
```
I assume that's cheaper than finding the last non-debug instruction.



Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:343
+  if (I != MBB.end() && Subtarget->hardenSlsAll() &&
+  (I->getDesc().isUnconditionalBranch() || I->getDesc().isReturn())) {
+MCInst TmpInst;

Should we be using `MCInstrDesc::isIndirectBranch()` rather than 
`MCInstrDesc::isUnconditionalBranch()`?  The naming of the command line flag 
options seem to imply the former, not the latter.



Comment at: llvm/test/CodeGen/X86/speculation-hardening-sls.ll:91
+  ret i32 1
+}

Please add a test case for the extremely simple:
```
void bar(void (*x)(void)) {
  x();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126137

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


[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done.
akyrtzi added inline comments.



Comment at: clang/lib/Lex/DependencyDirectivesScanner.cpp:153
+
+  SmallVector CurDirToks;
+  SmallVector DirsWithToks;

jansvoboda11 wrote:
> Can you add a comment explaining the relationship between the members?
Done!



Comment at: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp:114
+  EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
+  EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
+  EXPECT_EQ(pp_eof, Directives[21].Kind);

jansvoboda11 wrote:
> What's the reason being these changes?
Originally the directive 'tokens' were:
```
   cxx_export_decl,
   cxx_module_decl,
   cxx_import_decl,
```

While all the other token kinds were representing one top-level directive, 
`cxx_export_decl` was different in that it was treated like a "modifier" for 
`cxx_module_decl` or `cxx_import_decl`. But this inconsistency did not affect 
anything since the preprocessor was reading the minimized sources and the 
`cxx_*` tokens were generally ignored.

After these changes the preprocessor reads sets of ("one top-level directive 
kind" + "array of tokens for the directive") and I thought it's a bit better to 
keep all the directive kinds consistent as representing "one top-level 
directive", thus I made the change for directive kinds to be:
```
  cxx_module_decl,
  cxx_import_decl,
  cxx_export_module_decl,
  cxx_export_import_decl,
```
in which case "export module" is treated as a separate top-level directive kind 
than "module".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125487

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


[PATCH] D126024: [MSVC, ARM64] Add __readx18 intrinsics

2022-05-23 Thread Stephen Long via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f1e64b54f59: [MSVC, ARM64] Add __readx18 intrinsics 
(authored by steplong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126024

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/arm64-microsoft-intrinsics.c

Index: clang/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -187,5 +187,64 @@
 // CHECK-MSVC: %[[DATA:.*]] = load i64, i64* %[[DATA_ADDR]], align 8
 // CHECK-MSVC: store i64 %[[DATA]], i64* %[[BITCAST_PTR]], align 1
 
+unsigned char check__readx18byte(unsigned long offset) {
+  return __readx18byte(offset);
+}
+
+// CHECK-MSVC: %[[OFFSET_ADDR:.*]] = alloca i32, align 4
+// CHECK-MSVC: store i32 %offset, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[X18:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2]])
+// CHECK-MSVC: %[[X18_AS_PTR:.*]] = inttoptr i64 %[[X18]] to i8*
+// CHECK-MSVC: %[[OFFSET:.*]] = load i32, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[ZEXT_OFFSET:.*]] = zext i32 %[[OFFSET]] to i64
+// CHECK-MSVC: %[[PTR:.*]] = getelementptr i8, i8* %[[X18_AS_PTR]], i64 %[[ZEXT_OFFSET]]
+// CHECK-MSVC: %[[RETVAL:.*]] = load i8, i8* %[[PTR]], align 1
+// CHECK-MSVC: ret i8 %[[RETVAL]]
+
+unsigned short check__readx18word(unsigned long offset) {
+  return __readx18word(offset);
+}
+
+// CHECK-MSVC: %[[OFFSET_ADDR:.*]] = alloca i32, align 4
+// CHECK-MSVC: store i32 %offset, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[X18:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2]])
+// CHECK-MSVC: %[[X18_AS_PTR:.*]] = inttoptr i64 %[[X18]] to i8*
+// CHECK-MSVC: %[[OFFSET:.*]] = load i32, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[ZEXT_OFFSET:.*]] = zext i32 %[[OFFSET]] to i64
+// CHECK-MSVC: %[[PTR:.*]] = getelementptr i8, i8* %[[X18_AS_PTR]], i64 %[[ZEXT_OFFSET]]
+// CHECK-MSVC: %[[BITCAST_PTR:.*]] = bitcast i8* %[[PTR]] to i16*
+// CHECK-MSVC: %[[RETVAL:.*]] = load i16, i16* %[[BITCAST_PTR]], align 1
+// CHECK-MSVC: ret i16 %[[RETVAL]]
+
+unsigned long check__readx18dword(unsigned long offset) {
+  return __readx18dword(offset);
+}
+
+// CHECK-MSVC: %[[OFFSET_ADDR:.*]] = alloca i32, align 4
+// CHECK-MSVC: store i32 %offset, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[X18:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2]])
+// CHECK-MSVC: %[[X18_AS_PTR:.*]] = inttoptr i64 %[[X18]] to i8*
+// CHECK-MSVC: %[[OFFSET:.*]] = load i32, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[ZEXT_OFFSET:.*]] = zext i32 %[[OFFSET]] to i64
+// CHECK-MSVC: %[[PTR:.*]] = getelementptr i8, i8* %[[X18_AS_PTR]], i64 %[[ZEXT_OFFSET]]
+// CHECK-MSVC: %[[BITCAST_PTR:.*]] = bitcast i8* %[[PTR]] to i32*
+// CHECK-MSVC: %[[RETVAL:.*]] = load i32, i32* %[[BITCAST_PTR]], align 1
+// CHECK-MSVC: ret i32 %[[RETVAL]]
+
+unsigned __int64 check__readx18qword(unsigned long offset) {
+  return __readx18qword(offset);
+}
+
+// CHECK-MSVC: %[[OFFSET_ADDR:.*]] = alloca i32, align 4
+// CHECK-MSVC: store i32 %offset, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[X18:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2]])
+// CHECK-MSVC: %[[X18_AS_PTR:.*]] = inttoptr i64 %[[X18]] to i8*
+// CHECK-MSVC: %[[OFFSET:.*]] = load i32, i32* %[[OFFSET_ADDR]], align 4
+// CHECK-MSVC: %[[ZEXT_OFFSET:.*]] = zext i32 %[[OFFSET]] to i64
+// CHECK-MSVC: %[[PTR:.*]] = getelementptr i8, i8* %[[X18_AS_PTR]], i64 %[[ZEXT_OFFSET]]
+// CHECK-MSVC: %[[BITCAST_PTR:.*]] = bitcast i8* %[[PTR]] to i64*
+// CHECK-MSVC: %[[RETVAL:.*]] = load i64, i64* %[[BITCAST_PTR]], align 1
+// CHECK-MSVC: ret i64 %[[RETVAL]]
+
 // CHECK-MSVC: ![[MD2]] = !{!"x18"}
 // CHECK-MSVC: ![[MD3]] = !{!"sp"}
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -567,6 +567,11 @@
 void __writex18word(unsigned long offset, unsigned short data);
 void __writex18dword(unsigned long offset, unsigned long data);
 void __writex18qword(unsigned long offset, unsigned __int64 data);
+
+unsigned char __readx18byte(unsigned long offset);
+unsigned short __readx18word(unsigned long offset);
+unsigned long __readx18dword(unsigned long offset);
+unsigned __int64 __readx18qword(unsigned long offset);
 #endif
 
 /**\
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -9977,6 +9977,30 @@
 return Store;
   }
 
+  if (BuiltinID == AArch64::BI__readx18byte ||
+  BuiltinID == AArch64::BI__readx18word ||
+   

[clang] 4f1e64b - [MSVC, ARM64] Add __readx18 intrinsics

2022-05-23 Thread Stephen Long via cfe-commits

Author: Stephen Long
Date: 2022-05-23T10:59:12-07:00
New Revision: 4f1e64b54f59dd4c303d04b62926f35bde5a2c79

URL: 
https://github.com/llvm/llvm-project/commit/4f1e64b54f59dd4c303d04b62926f35bde5a2c79
DIFF: 
https://github.com/llvm/llvm-project/commit/4f1e64b54f59dd4c303d04b62926f35bde5a2c79.diff

LOG: [MSVC, ARM64] Add __readx18 intrinsics

https://docs.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics?view=msvc-170

  unsigned char __readx18byte(unsigned long)
  unsigned short __readx18word(unsigned long)
  unsigned long __readx18dword(unsigned long)
  unsigned __int64 __readx18qword(unsigned long)

Given the lack of documentation of the intrinsics, we chose to align the offset 
with just
`CharUnits::One()` when calling `IRBuilderBase::CreateAlignedLoad()`

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D126024

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/intrin.h
clang/test/CodeGen/arm64-microsoft-intrinsics.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index a04b48dd128e3..65ab4fcced9ae 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -256,6 +256,11 @@ TARGET_HEADER_BUILTIN(__writex18word,  "vULiUs", "nh", 
"intrin.h", ALL_MS_LANGUA
 TARGET_HEADER_BUILTIN(__writex18dword, "vULiULi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__writex18qword, "vULiULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(__readx18byte,  "UcULi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(__readx18word,  "UsULi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(__readx18dword, "ULiULi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(__readx18qword, "ULLiULi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+
 #undef BUILTIN
 #undef LANGBUILTIN
 #undef TARGET_HEADER_BUILTIN

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 6168ba938db4f..bdc638299c4bd 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -9977,6 +9977,30 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
 return Store;
   }
 
+  if (BuiltinID == AArch64::BI__readx18byte ||
+  BuiltinID == AArch64::BI__readx18word ||
+  BuiltinID == AArch64::BI__readx18dword ||
+  BuiltinID == AArch64::BI__readx18qword) {
+llvm::Type *IntTy = ConvertType(E->getType());
+
+// Read x18 as i8*
+LLVMContext &Context = CGM.getLLVMContext();
+llvm::Metadata *Ops[] = {llvm::MDString::get(Context, "x18")};
+llvm::MDNode *RegName = llvm::MDNode::get(Context, Ops);
+llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
+llvm::Function *F =
+CGM.getIntrinsic(llvm::Intrinsic::read_register, {Int64Ty});
+llvm::Value *X18 = Builder.CreateCall(F, Metadata);
+X18 = Builder.CreateIntToPtr(X18, llvm::PointerType::get(Int8Ty, 0));
+
+// Load x18 + offset
+Value *Offset = Builder.CreateZExt(EmitScalarExpr(E->getArg(0)), Int64Ty);
+Value *Ptr = Builder.CreateGEP(Int8Ty, X18, Offset);
+Ptr = Builder.CreatePointerCast(Ptr, llvm::PointerType::get(IntTy, 0));
+LoadInst *Load = Builder.CreateAlignedLoad(IntTy, Ptr, CharUnits::One());
+return Load;
+  }
+
   // Handle MSVC intrinsics before argument evaluation to prevent double
   // evaluation.
   if (Optional MsvcIntId = translateAarch64ToMsvcIntrin(BuiltinID))

diff  --git a/clang/lib/Headers/intrin.h b/clang/lib/Headers/intrin.h
index dbc5159853ddf..de68b07491c6c 100644
--- a/clang/lib/Headers/intrin.h
+++ b/clang/lib/Headers/intrin.h
@@ -567,6 +567,11 @@ void __writex18byte(unsigned long offset, unsigned char 
data);
 void __writex18word(unsigned long offset, unsigned short data);
 void __writex18dword(unsigned long offset, unsigned long data);
 void __writex18qword(unsigned long offset, unsigned __int64 data);
+
+unsigned char __readx18byte(unsigned long offset);
+unsigned short __readx18word(unsigned long offset);
+unsigned long __readx18dword(unsigned long offset);
+unsigned __int64 __readx18qword(unsigned long offset);
 #endif
 
 
/**\

diff  --git a/clang/test/CodeGen/arm64-microsoft-intrinsics.c 
b/clang/test/CodeGen/arm64-microsoft-intrinsics.c
index a9b1d444553f5..1ad5233bb4c81 100644
--- a/clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ b/clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -187,5 +187,64 @@ void check__writex18qword(unsigned long offset, unsigned 
__int64 data) {
 // CHECK-MSVC: %[[DATA:.*]] = load i64, i64* %[[DATA_ADDR]], align 8
 // CHECK-MSVC: store i64 %[[DATA]], i64* %[[BITCAST_PTR]], align 1
 
+unsigned char check__readx18byte(

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 431429.
akyrtzi marked an inline comment as done.
akyrtzi added a comment.

Add documentation comments for a couple of fields of `Scanner` in 
`DependencyDirectivesScanner.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125487

Files:
  clang/include/clang/Lex/DependencyDirectivesScanner.h
  clang/include/clang/Lex/Lexer.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/Lexer/minimize_source_to_dependency_directives_invalid_macro_name.c
  clang/test/Lexer/minimize_source_to_dependency_directives_pragmas.c
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -204,51 +204,5 @@
   EXPECT_EQ(convert_to_slash(Deps[5]), "/root/symlink.h");
 }
 
-namespace dependencies {
-TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately1) {
-  auto VFS = llvm::makeIntrusiveRefCnt();
-  VFS->addFile("/mod.h", 0,
-   llvm::MemoryBuffer::getMemBuffer("#include \n"
-"// hi there!\n"));
-
-  DependencyScanningFilesystemSharedCache SharedCache;
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS);
-
-  DepFS.enableDirectivesScanningOfAllFiles(); // Let's be explicit for clarity.
-  auto StatusMinimized0 = DepFS.status("/mod.h");
-  DepFS.disableDirectivesScanning("/mod.h");
-  auto StatusFull1 = DepFS.status("/mod.h");
-
-  EXPECT_TRUE(StatusMinimized0);
-  EXPECT_TRUE(StatusFull1);
-  EXPECT_EQ(StatusMinimized0->getSize(), 17u);
-  EXPECT_EQ(StatusFull1->getSize(), 30u);
-  EXPECT_EQ(StatusMinimized0->getName(), StringRef("/mod.h"));
-  EXPECT_EQ(StatusFull1->getName(), StringRef("/mod.h"));
-}
-
-TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
-  auto VFS = llvm::makeIntrusiveRefCnt();
-  VFS->addFile("/mod.h", 0,
-   llvm::MemoryBuffer::getMemBuffer("#include \n"
-"// hi there!\n"));
-
-  DependencyScanningFilesystemSharedCache SharedCache;
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS);
-
-  DepFS.disableDirectivesScanning("/mod.h");
-  auto StatusFull0 = DepFS.status("/mod.h");
-  DepFS.enableDirectivesScanningOfAllFiles();
-  auto StatusMinimized1 = DepFS.status("/mod.h");
-
-  EXPECT_TRUE(StatusFull0);
-  EXPECT_TRUE(StatusMinimized1);
-  EXPECT_EQ(StatusFull0->getSize(), 30u);
-  EXPECT_EQ(StatusMinimized1->getSize(), 17u);
-  EXPECT_EQ(StatusFull0->getName(), StringRef("/mod.h"));
-  EXPECT_EQ(StatusMinimized1->getName(), StringRef("/mod.h"));
-}
-
-} // end namespace dependencies
 } // end namespace tooling
 } // end namespace clang
Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -14,39 +14,58 @@
 using namespace clang;
 using namespace clang::dependency_directives_scan;
 
-static bool minimizeSourceToDependencyDirectives(StringRef Input,
- SmallVectorImpl &Out) {
-  SmallVector Directives;
-  return scanSourceForDependencyDirectives(Input, Out, Directives);
+static bool minimizeSourceToDependencyDirectives(
+StringRef Input, SmallVectorImpl &Out,
+SmallVectorImpl &Tokens,
+SmallVectorImpl &Directives) {
+  Out.clear();
+  Tokens.clear();
+  Directives.clear();
+  if (scanSourceForDependencyDirectives(Input, Tokens, Directives))
+return true;
+
+  raw_svector_ostream OS(Out);
+  printDependencyDirectivesAsSource(Input, Directives, OS);
+  if (!Out.empty() && Out.back() != '\n')
+Out.push_back('\n');
+  Out.push_back('\0');
+  Out.pop_back();
+
+  return false;
 }
 
-static bool
-minimizeSourceToDependencyDirectives(StringRef Input,
- SmallVectorImpl &Out,
- SmallVectorImpl &Directives) {
-  return scanSourceForDependencyDirectives(Input, Out, Directives);
+static bool minimizeSourceToDependencyDirectives(StringRef Input,
+ SmallVectorImpl &Out) {
+  SmallVector Tokens;
+  SmallVector Directives;
+  return minimizeSourceToDependencyDirectives(Input, Out, Tokens, Directives);
 }
 
 namespace {
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Empty) {

[PATCH] D125555: [clang] Add __has_target_feature

2022-05-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:260
 
+``__has_target_feature``
+

yaxunl wrote:
> aaron.ballman wrote:
> > The first question that comes to mind for me is: why is `__has_feature` not 
> > sufficient?
> `__has_target_feature` accepts predefined target features for the specified 
> target triple and CPU. The names of target features are determined by the 
> target. They may overlap with clang features and cause ambiguity. Another 
> issue is that they usually contain '-', which makes them not valid 
> identifiers of C/C++, therefore the builtin macro has to accept a string 
> literal argument instead of identifier argument like `__has_feature`. 
Do we have to use a string literal? I am presuming there's an enumerated list 
of features for each target, and thus we can use an identifier argument (it 
might turn around and map that identifier back to a string literal, but that's 
fine). But I'm not certain I understand how a target feature will overlap with 
a clang feature and cause ambiguity.

My concerns really boil down to:

* It's already hard enough for users to know when to use `__has_extension` vs 
`__has_feature` and this is adding another option into the mix.
* There's no documentation about what constitutes a valid string literal for 
this feature and I don't know how a user would go about discovering the full 
enumerated list for any given target. Also, are these strings guaranteed to be 
stable (spelling doesn't change, strings aren't removed, etc)?
* (much smaller concern) A string literal may not be the most user-friendly way 
to surface the functionality (tools typically don't auto-complete string 
literals or do typo correction within them).



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

https://reviews.llvm.org/D12

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


[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

2022-05-23 Thread Nicolas van Kempen via Phabricator via cfe-commits
nicovank updated this revision to Diff 431426.
nicovank added a comment.

Fix formatting issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101471

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
@@ -10,15 +10,36 @@
 
 namespace std {
 template 
-class initializer_list
-{
+class initializer_list {
 public:
   initializer_list() noexcept {}
 };
 
+template 
+class pair {
+public:
+  pair() = default;
+  pair(const pair &) = default;
+  pair(pair &&) = default;
+
+  pair(const T1 &, const T2 &) {}
+  pair(T1 &&, T2 &&) {}
+
+  template 
+  pair(const pair &){};
+  template 
+  pair(pair &&){};
+};
+
 template 
 class vector {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   vector() = default;
   vector(initializer_list) {}
 
@@ -27,54 +48,230 @@
 
   template 
   void emplace_back(Args &&... args){};
+  template 
+  iterator emplace(const_iterator pos, Args &&...args){};
   ~vector();
 };
+
 template 
 class list {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  template 
+  iterator emplace(const_iterator pos, Args &&...args){};
   template 
   void emplace_back(Args &&... args){};
+  template 
+  void emplace_front(Args &&...args){};
   ~list();
 };
 
 template 
 class deque {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  template 
+  iterator emplace(const_iterator pos, Args &&...args){};
   template 
   void emplace_back(Args &&... args){};
+  template 
+  void emplace_front(Args &&...args){};
   ~deque();
 };
 
-template  struct remove_reference { using type = T; };
-template  struct remove_reference { using type = T; };
-template  struct remove_reference { using type = T; };
+template 
+class forward_list {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace_front(Args &&...args){};
+  template 
+  iterator emplace_after(const_iterator pos, Args &&...args){};
+};
 
-template  class pair {
+template 
+class set {
 public:
-  pair() = default;
-  pair(const pair &) = default;
-  pair(pair &&) = default;
+  using value_type = T;
 
-  pair(const T1 &, const T2 &) {}
-  pair(T1 &&, T2 &&) {}
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class map {
+public:
+  using value_type = std::pair;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class multiset {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class multimap {
+public:
+  using value_type = std::pair;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class unordered_set {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class unordered_map {
+public:
+  using value_type = std::pair;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template 
+  void emplace(Args &&...args){};
+  template 
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template 
+class unordered_multiset {
+public:
+  us

[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth added a subscriber: mcgrathr.
paulkirth added a comment.

@rnk the standard even has an example of this exact behavior, so I think it's 
hard to say its //wrong// for LLVM to do this, but that may be more gentle. I'm 
going to defer to more expert opinions here, however, and loop in @mcgrathr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126224

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


[PATCH] D126226: [OpenMP] Add `-Xoffload-linker` to forward input to the device linker

2022-05-23 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added reviewers: markdewing, jdoerfert, tianshilei1992, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.

We use the clang-linker-wrapper to perform device linking of embedded
offloading object files. This is done by generating those jobs inside of
the linker-wrapper itself. This patch adds an argument in Clang and the
linker-wrapper that allows users to forward input to the device linking
phase. This can either be done for every device linker, or for a
specific target triple. We use the `-Xoffload-linker ` and the
`-Xoffload-linker= ` syntax to accomplish this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126226

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -108,6 +108,12 @@
   cl::desc("Argument to pass to the ptxas invocation"),
   cl::cat(ClangLinkerWrapperCategory));
 
+static cl::list
+LinkerArgs("device-linker", cl::ZeroOrMore,
+   cl::desc("Arguments to pass to the device linker invocation"),
+   cl::value_desc(" or ="),
+   cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt Verbose("v", cl::ZeroOrMore,
  cl::desc("Verbose output from tools"),
  cl::init(false),
@@ -226,6 +232,17 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
+// Forward user requested arguments to the device linking job.
+void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
+  for (StringRef Arg : LinkerArgs) {
+auto TripleAndValue = Arg.split('=');
+if (TripleAndValue.second.empty())
+  Args.push_back(TripleAndValue.first);
+else if (TripleAndValue.first == Triple)
+  Args.push_back(TripleAndValue.second);
+  }
+}
+
 std::string getMainExecutable(const char *Name) {
   void *Ptr = (void *)(intptr_t)&getMainExecutable;
   auto COWPath = sys::fs::getMainExecutable(Name, Ptr);
@@ -531,6 +548,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
 return std::move(Err);
 
@@ -599,6 +617,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(*LLDPath, CmdArgs))
 return std::move(Err);
 
@@ -676,6 +695,7 @@
   for (StringRef Input : InputFiles)
 CmdArgs.push_back(Input);
 
+  renderXLinkerArgs(CmdArgs, TheTriple.getTriple());
   if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
 return std::move(Err);
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -104,3 +104,9 @@
 // RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-LIBRARY %s
 
 // CHECK-LTO-LIBRARY: {{.*}}-lomptarget{{.*}}-lomptarget.devicertl
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -Xoffload-linker a -Xoffload-linker=nvptx64-nvidia-cuda b -Xoffload-linker=nvptx64 c \
+// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
+
+// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}-device-linker=nvptx64-nvidia-cuda=c{{.*}}--
Index: clang/test/Driver/linker-wrapper.c
===
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -80,3 +80,15 @@
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_52 {{.*}}.o
 // CUDA: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
 // CUDA: fatbinary{{.*}}-64 --create {{.*}}.fatbin --image=profile=sm_52,file={{.*}}.out --image=profile=sm_70,file={{.*}}.out
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx908 \
+// RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --dry-run --host-triple x86_64-unknown-linux-gnu -linker-path \
+// RUN:   /usr/bin/ld --device-linker=a --device-linker=nvptx64-nv

[clang] dbd1ba2 - [PS5] Disable a test, same as PS4

2022-05-23 Thread Paul Robinson via cfe-commits

Author: Paul Robinson
Date: 2022-05-23T10:43:26-07:00
New Revision: dbd1ba28a3a435c87eb2a977028ea22e9aabf148

URL: 
https://github.com/llvm/llvm-project/commit/dbd1ba28a3a435c87eb2a977028ea22e9aabf148
DIFF: 
https://github.com/llvm/llvm-project/commit/dbd1ba28a3a435c87eb2a977028ea22e9aabf148.diff

LOG: [PS5] Disable a test, same as PS4

Added: 


Modified: 
clang/test/Driver/nostdincxx.cpp

Removed: 






  
Unicorn! · GitHub

  body {
background-color: #f1f1f1;
margin: 0;
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
  }

  .container { margin: 50px auto 40px auto; width: 600px; text-align: 
center; }

  a { color: #4183c4; text-decoration: none; }
  a:hover { text-decoration: underline; }

  h1 { letter-spacing: -1px; line-height: 60px; font-size: 60px; 
font-weight: 100; margin: 0px; text-shadow: 0 1px 0 #fff; }
  p { color: rgba(0, 0, 0, 0.5); margin: 10px 0 10px; font-size: 18px; 
font-weight: 200; line-height: 1.6em;}

  ul { list-style: none; margin: 25px 0; padding: 0; }
  li { display: table-cell; font-weight: bold; width: 1%; }

  .logo { display: inline-block; margin-top: 35px; }
  .logo-img-2x { display: none; }
  @media
  only screen and (-webkit-min-device-pixel-ratio: 2),
  only screen and (   min--moz-device-pixel-ratio: 2),
  only screen and ( -o-min-device-pixel-ratio: 2/1),
  only screen and (min-device-pixel-ratio: 2),
  only screen and (min-resolution: 192dpi),
  only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
  }

  #suggestions {
margin-top: 35px;
color: #ccc;
  }
  #suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
  }


  
  


  

  

  No server is currently available to service your 
request.
  Sorry about that. Please try refreshing and contact us if the problem 
persists.
  
https://github.com/contact";>Contact Support —
https://www.githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
  

  

  

  

  

  




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


[PATCH] D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang

2022-05-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D125970#3531673 , @JonChesterfield 
wrote:

> In D125970#3531645 , @aaron.ballman 
> wrote:
>
>> In D125970#3527685 , 
>> @JonChesterfield wrote:
>>
>>> If it was adding a calling convention, sure - caution warranted. There's no 
>>> llvm change here though, an existing CC is exposed to C++. No change to the 
>>> type system either.
>>
>> This is adding a user-facing calling convention to Clang and it changes the 
>> type system as a result. For example, lambda function pointer conversion 
>> operators sometimes are generated for each calling convention so that you 
>> can form a function pointer of the correct type (this might not be impacted 
>> by your change here); there's a specific number of bits for representing the 
>> enumeration of calling conventions and this uses one of those bits, etc.
>
> It slightly changes the type system of C++ code in that the calling 
> convention was previously only available in opencl / openmp etc. I was under 
> the impression that the compiler data representation cost of calling 
> conventions was in LLVM and thus pre-paid for the calling convention this 
> gives access to. There's the `enum CallingConv ` which has gained a field, I 
> didn't realise that was input into something of limited bitwidth.

Calling conventions are weird in that they have a fair amount of frontend AND 
backend work involved with them (though maybe this one is more backend than 
frontend as it doesn't seem to be doing much different in codegen). As for the 
bit-width thing, it mostly comes into play here: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/Type.h#L3678
 -- we try to pack as much information into as few bits as possible because 
`Type` overhead causes big problems (for example, it limits template 
instantiation depth due to memory overhead). We have *some* wiggle room left in 
that bit-field, and we have some ideas on how to improve the situation, but... 
nobody's done the refactoring work yet and each new calling convention we add 
brings us that much closer to the answer being "sorry, can't do it.".

>>> I'll propose a patch with some documentation for it if you wish, but it'll 
>>> just say "For ad hoc debugging of the amdgpu backend". Undocumented seems 
>>> to state that more clearly.
>>
>> I continue to question whether we want to support such a calling convention. 
>> This does not seem to be generally useful enough to warrant inclusion in 
>> Clang. The fact that you'd like to leave it undocumented as that's more 
>> clear for users is a pretty good indication that this calling convention 
>> doesn't meet the bar for an extension.
>
> Strictly speaking this lets people write a GPU kernel that can execute on 
> AMDGPU in freestanding C++.

That sounds generally useful, which is great!

> I happen to want to do that for testing LLVM in the immediate instance but 
> there's arguably wider applicability. However, it looks like how arguments 
> are represented in this calling convention has some strangeness (see 
> discussion with Sam above), particularly with regard to address spaces.

That sounds less great. :-( This suggests there may be another calling 
convention in the future which corrects those deficiencies.

> I can revert this patch if necessary, but it'll force me to continue trying 
> to test our compiler through the lens of opencl, and rules out programming 
> the hardware without the various specific language front ends. I think that 
> would be a sad loss.

My thinking is: I don't want you to revert and have no solution, because you 
have a problem to solve and this (presumably) solves it for you. But at the 
same time, I'd like us to be able to explore options to make sure that a 
calling convention is the best approach and that we're confident that we won't 
need additional calling conventions to fix corner cases in the future. For 
example, can the calling convention be inferred at codegen time in Clang or by 
an LLVM pass so that the FE doesn't need to expose an attribute through the 
type system?

Have you explored alternatives that don't require a user-facing attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125970

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


[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

I am reminded of the perennial problem of "optional" protobuf fields that, when 
omitted, will cause production crashes.

Do you think it would be less disruptive to synthesize a name? I believe the 
string lives in a string pool, so naming all string literals `` 
seems like it could be acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126224

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


[PATCH] D126224: Add DWARF string debug to clang release notes.

2022-05-23 Thread Mitch Phillips via Phabricator via cfe-commits
hctim created this revision.
hctim added reviewers: paulkirth, dblaikie.
Herald added a project: All.
hctim requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D12353  added inline strings to the DWARF info 
produced by clang. This
turns out to break some debugging software that assumes that a
DW_TAG_variable *must* come with a DW_AT_name. Add a release note to
broadcast this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126224

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -110,12 +110,12 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-- The builtin function __builtin_dump_struct would crash clang when the target 
+- The builtin function __builtin_dump_struct would crash clang when the target
   struct contains a bitfield. It now correctly handles bitfields.
   This fixes Issue `Issue 54462 
`_.
 - Statement expressions are now disabled in default arguments in general.
   This fixes Issue `Issue 53488 
`_.
-- According to `CWG 1394 `_ and 
+- According to `CWG 1394 `_ and
   `C++20 [dcl.fct.def.general]p2 
`_,
   Clang should not diagnose incomplete types in function definitions if the 
function body is "= delete;".
   This fixes Issue `Issue 52802 
`_.
@@ -440,6 +440,12 @@
 DWARF Support in Clang
 --
 
+- clang now adds DWARF information for inline strings in C/C++ programs,
+  allowing ``line:column`` symbolization of strings. Some debugging programs 
may
+  require updating, as this takes advantage of DWARF ``DW_TAG_variable``
+  structures *without* a ``DW_AT_name`` field, which is valid DWARF, but may
+  lead to assertion failures in some software.
+
 Arm and AArch64 Support in Clang
 
 
@@ -503,7 +509,7 @@
 
 - Added a new checker ``alpha.unix.cstring.UninitializedRead`` this will check 
for uninitialized reads
   from common memory copy/manipulation functions such as ``memcpy``, 
``mempcpy``, ``memmove``, ``memcmp``, `
-  `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. 
Although 
+  `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. 
Although
   this checker currently is in list of alpha checkers due to a false positive.
 
 .. _release-notes-ubsan:


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -110,12 +110,12 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-- The builtin function __builtin_dump_struct would crash clang when the target 
+- The builtin function __builtin_dump_struct would crash clang when the target
   struct contains a bitfield. It now correctly handles bitfields.
   This fixes Issue `Issue 54462 `_.
 - Statement expressions are now disabled in default arguments in general.
   This fixes Issue `Issue 53488 `_.
-- According to `CWG 1394 `_ and 
+- According to `CWG 1394 `_ and
   `C++20 [dcl.fct.def.general]p2 `_,
   Clang should not diagnose incomplete types in function definitions if the function body is "= delete;".
   This fixes Issue `Issue 52802 `_.
@@ -440,6 +440,12 @@
 DWARF Support in Clang
 --
 
+- clang now adds DWARF information for inline strings in C/C++ programs,
+  allowing ``line:column`` symbolization of strings. Some debugging programs may
+  require updating, as this takes advantage of DWARF ``DW_TAG_variable``
+  structures *without* a ``DW_AT_name`` field, which is valid DWARF, but may
+  lead to assertion failures in some software.
+
 Arm and AArch64 Support in Clang
 
 
@@ -503,7 +509,7 @@
 
 - Added a new checker ``alpha.unix.cstring.UninitializedRead`` this will check for uninitialized reads
   from common memory copy/manipulation functions such as ``memcpy``, ``mempcpy``, ``memmove``, ``memcmp``, `
-  `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``

[PATCH] D126223: [Clang][Sema] Fix a SemaType/VisitTagTypeLoc assertion

2022-05-23 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song created this revision.
yonghong-song added reviewers: compnerd, aaron.ballman.
Herald added a project: All.
yonghong-song requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Patch [1] added btf_type_tag attribute support. In particular,
in class TypeSpecLocFiller, it permitted to visit pointee's
of a PointerTypeLoc. This caused a behavior change outside
of the btf_type_tag area. Saleem Abdulrasool reported that
assertion would happen for the following code:

struct dispatch_object_s;
 void *_dispatch_queue_get_head(struct dispatch_object_s * volatile 
dq_items_head) {

  return (void *)(_Atomic __typeof__(dq_items_head) *)dq_items_head;

}

The following is what happened:

  ...   
  1. In Parse/ParseDecl.cpp, ParseTypeofSpecifier() is called to handle typeof
 specifier. DeclSpec is set to TST_typeofExpr.
  2. Later, in Sema/SemaType.cpp,
 TypeSpecLocFiller(S, S.Context, State, D.getDeclSpec()).Visit(CurrTL)
 is called.
  3. The specific types visited in the following order:
 VisitAtomicTypeLoc()
 VisitPointerTypeLoc()
 VisitElaboratedTypeLo()c
 VisitTagTypeLoc()

During VisitTagTypeLoc(), since DeclSpec is TST_typeofExpr, this will cause
assertion error in DS.getTypeSpecTypeNameLoc().

  clang: /home/yhs/work/llvm-project/clang/include/clang/Sema/DeclSpec.h:519:
clang::SourceLocation clang::DeclSpec::getTypeSpecTypeNameLoc() const:
Assertion `isDeclRep((TST) TypeSpecType) || TypeSpecType == TST_typename' 
failed.

If we remove the _Atomic in the above code like

struct dispatch_object_s;
 void *_dispatch_queue_get_head(struct dispatch_object_s * volatile 
dq_items_head) {

  return (void *)(_Atomic __typeof__(dq_items_head) *)dq_items_head;

}

The above step 3 will have VisitTypeOfExprTypeLoc() and no assertion happens.

To fix the issue, in VisitTagTypeLoc(), let us not do 
DS.getTypeSpecTypeNameLoc()
if the DeclSpec is TST_typeofExpr.

  [1] https://reviews.llvm.org/D99


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126223

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/atomic-typeof.c


Index: clang/test/Sema/atomic-typeof.c
===
--- /dev/null
+++ clang/test/Sema/atomic-typeof.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c11 -x c %s
+
+struct dispatch_object_s;
+void _dispatch_queue_get_head(struct dispatch_object_s * volatile 
dq_items_head) {
+  (_Atomic __typeof__(dq_items_head) *)dq_items_head;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6109,7 +6109,8 @@
 TL.setArgLocInfo(I, TemplateArgsInfo.arguments()[I].getLocInfo());
 }
 void VisitTagTypeLoc(TagTypeLoc TL) {
-  TL.setNameLoc(DS.getTypeSpecTypeNameLoc());
+  if (DS.getTypeSpecType() != DeclSpec::TST_typeofExpr)
+TL.setNameLoc(DS.getTypeSpecTypeNameLoc());
 }
 void VisitAtomicTypeLoc(AtomicTypeLoc TL) {
   // An AtomicTypeLoc can come from either an _Atomic(...) type specifier


Index: clang/test/Sema/atomic-typeof.c
===
--- /dev/null
+++ clang/test/Sema/atomic-typeof.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c11 -x c %s
+
+struct dispatch_object_s;
+void _dispatch_queue_get_head(struct dispatch_object_s * volatile dq_items_head) {
+  (_Atomic __typeof__(dq_items_head) *)dq_items_head;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6109,7 +6109,8 @@
 TL.setArgLocInfo(I, TemplateArgsInfo.arguments()[I].getLocInfo());
 }
 void VisitTagTypeLoc(TagTypeLoc TL) {
-  TL.setNameLoc(DS.getTypeSpecTypeNameLoc());
+  if (DS.getTypeSpecType() != DeclSpec::TST_typeofExpr)
+TL.setNameLoc(DS.getTypeSpecTypeNameLoc());
 }
 void VisitAtomicTypeLoc(AtomicTypeLoc TL) {
   // An AtomicTypeLoc can come from either an _Atomic(...) type specifier
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126172: [clang] Fix comparison of TemplateArgument when they are of template kind

2022-05-23 Thread Robert Esclapez via Phabricator via cfe-commits
roberteg16 updated this revision to Diff 431416.
roberteg16 added a comment.

Fix clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126172

Files:
  clang/lib/AST/TemplateBase.cpp
  clang/test/CXX/dcl/dcl.fct/p17.cpp


Index: clang/test/CXX/dcl/dcl.fct/p17.cpp
===
--- clang/test/CXX/dcl/dcl.fct/p17.cpp
+++ clang/test/CXX/dcl/dcl.fct/p17.cpp
@@ -257,4 +257,23 @@
   static_assert(is_same_v);
   // expected-error@-1{{no matching}}
   static_assert(is_same_v);
-}
+
+  template  struct S3 {};
+  // expected-note@-1 {{'S3' declared here}}
+  // expected-note@-2 {{template is declared here}}
+  void f23(C2<::S3> auto);
+  // clang-format off
+  // expected-error@-1 {{no template named 'S3' in the global namespace; did 
you mean simply 'S3'?}}
+  // expected-error@-2 {{use of class template '::S3' requires template 
arguments}}
+  // clang-format on
+  } // namespace constrained
+
+  template  struct S4 {};
+  // expected-note@-1 {{template is declared here}}
+
+  namespace constrained {
+  void f24(C2<::S4> auto);
+  // clang-format off
+  // expected-error@-1 {{use of class template '::S4' requires template 
arguments}}
+  // clang-format on
+  } // namespace constrained
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -370,7 +370,8 @@
 
   case Template:
   case TemplateExpansion:
-return TemplateArg.Name == Other.TemplateArg.Name &&
+return getAsTemplateOrTemplatePattern().getAsTemplateDecl() ==
+   Other.getAsTemplateOrTemplatePattern().getAsTemplateDecl() &&
TemplateArg.NumExpansions == Other.TemplateArg.NumExpansions;
 
   case Declaration:


Index: clang/test/CXX/dcl/dcl.fct/p17.cpp
===
--- clang/test/CXX/dcl/dcl.fct/p17.cpp
+++ clang/test/CXX/dcl/dcl.fct/p17.cpp
@@ -257,4 +257,23 @@
   static_assert(is_same_v);
   // expected-error@-1{{no matching}}
   static_assert(is_same_v);
-}
+
+  template  struct S3 {};
+  // expected-note@-1 {{'S3' declared here}}
+  // expected-note@-2 {{template is declared here}}
+  void f23(C2<::S3> auto);
+  // clang-format off
+  // expected-error@-1 {{no template named 'S3' in the global namespace; did you mean simply 'S3'?}}
+  // expected-error@-2 {{use of class template '::S3' requires template arguments}}
+  // clang-format on
+  } // namespace constrained
+
+  template  struct S4 {};
+  // expected-note@-1 {{template is declared here}}
+
+  namespace constrained {
+  void f24(C2<::S4> auto);
+  // clang-format off
+  // expected-error@-1 {{use of class template '::S4' requires template arguments}}
+  // clang-format on
+  } // namespace constrained
Index: clang/lib/AST/TemplateBase.cpp
===
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -370,7 +370,8 @@
 
   case Template:
   case TemplateExpansion:
-return TemplateArg.Name == Other.TemplateArg.Name &&
+return getAsTemplateOrTemplatePattern().getAsTemplateDecl() ==
+   Other.getAsTemplateOrTemplatePattern().getAsTemplateDecl() &&
TemplateArg.NumExpansions == Other.TemplateArg.NumExpansions;
 
   case Declaration:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D125272: [clang] Add -fcheck-new support

2022-05-23 Thread Pedro Falcato via Phabricator via cfe-commits
heatd added a comment.

In D125272#3515874 , @heatd wrote:

> In D125272#3504113 , @heatd wrote:
>
>> Adjusted the driver code to use addOptInFlag, adjusted the test, fixed the 
>> comment.
>
> Ping.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125272

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


[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

2022-05-23 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

> We should support deprecated analyzer flags for at least one release. In this 
> case I'm planning to drop this flag in clang-17

Should we emit a warning for the user about the deprecation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126215

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


[PATCH] D126216: [analyzer][NFC] Remove unused RegionStoreFeatures

2022-05-23 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126216

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


[PATCH] D123286: [Clang][OpenMP] Support for omp nothing

2022-05-23 Thread Sunil K via Phabricator via cfe-commits
koops added a comment.



In D123286#3527374 , @koops wrote:

> In D123286#3513797 , 
> @tianshilei1992 wrote:
>
>> Can we have test for right usage?
>
> I do not understand "test for right usage". From the specifications the only 
> right usage for "omp nothing" will be in the metadirective. If you have 
> meeting notes when "omp nothing" was framed please share it.

In gcc sources support "#pragma omp nothing" has been implemented. Test cases 
for it are in:

gcc/gcc/testsuite/c-c++-common/gomp/nothing-1.c
gcc/gcc/testsuite/c-c++-common/gomp/nothing-2.c

These 2 test cases indicate a similar kind of test cases as those which have 
been added in this support for "omp nothing". There does not seem to be a  test 
for right usage.


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

https://reviews.llvm.org/D123286

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


[PATCH] D126216: [analyzer][NFC] Remove unused RegionStoreFeatures

2022-05-23 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: NoQ, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal 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/D126216

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -317,29 +317,6 @@
   return removeBinding(BindingKey::Make(R, k));
 }
 
-//===--===//
-// Fine-grained control of RegionStoreManager.
-//===--===//
-
-namespace {
-struct minimal_features_tag {};
-struct maximal_features_tag {};
-
-class RegionStoreFeatures {
-  bool SupportsFields;
-public:
-  RegionStoreFeatures(minimal_features_tag) :
-SupportsFields(false) {}
-
-  RegionStoreFeatures(maximal_features_tag) :
-SupportsFields(true) {}
-
-  void enableFields(bool t) { SupportsFields = t; }
-
-  bool supportsFields() const { return SupportsFields; }
-};
-}
-
 //===--===//
 // Main RegionStore logic.
 //===--===//
@@ -349,8 +326,6 @@
 
 class RegionStoreManager : public StoreManager {
 public:
-  const RegionStoreFeatures Features;
-
   RegionBindings::Factory RBFactory;
   mutable ClusterBindings::Factory CBFactory;
 
@@ -377,16 +352,14 @@
 InvalidatedRegions *TopLevelRegions);
 
 public:
-  RegionStoreManager(ProgramStateManager& mgr, const RegionStoreFeatures &f)
-: StoreManager(mgr), Features(f),
-  RBFactory(mgr.getAllocator()), CBFactory(mgr.getAllocator()),
-  SmallStructLimit(0) {
+  RegionStoreManager(ProgramStateManager &mgr)
+  : StoreManager(mgr), RBFactory(mgr.getAllocator()),
+CBFactory(mgr.getAllocator()), SmallStructLimit(0) {
 ExprEngine &Eng = StateMgr.getOwningEngine();
 AnalyzerOptions &Options = Eng.getAnalysisManager().options;
 SmallStructLimit = Options.RegionStoreSmallStructLimit;
   }
 
-
   /// setImplicitDefaultValue - Set the default binding for the provided
   ///  MemRegion to the value implicitly defined for compound literals when
   ///  the value is not specified.
@@ -674,18 +647,9 @@
 
 std::unique_ptr
 ento::CreateRegionStoreManager(ProgramStateManager &StMgr) {
-  RegionStoreFeatures F = maximal_features_tag();
-  return std::make_unique(StMgr, F);
-}
-
-std::unique_ptr
-ento::CreateFieldsOnlyRegionStoreManager(ProgramStateManager &StMgr) {
-  RegionStoreFeatures F = minimal_features_tag();
-  F.enableFields(true);
-  return std::make_unique(StMgr, F);
+  return std::make_unique(StMgr);
 }
 
-
 //===--===//
 // Region Cluster analysis.
 //===--===//
@@ -2569,11 +2533,8 @@
 }
 
 RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B,
- const TypedValueRegion* R,
+ const TypedValueRegion *R,
  SVal V) {
-  if (!Features.supportsFields())
-return B;
-
   QualType T = R->getValueType();
   assert(T->isStructureOrClassType());
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -316,8 +316,6 @@
 // FIXME: Do we need to pass ProgramStateManager anymore?
 std::unique_ptr
 CreateRegionStoreManager(ProgramStateManager &StMgr);
-std::unique_ptr
-CreateFieldsOnlyRegionStoreManager(ProgramStateManager &StMgr);
 
 } // namespace ento
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

2022-05-23 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: NoQ, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I'm trying to remove unused options from the `Analyses.def` file, then merge 
the rest of the useful options into the `AnalyzerOptions.def`.
Then make sure one can set these by an `-analyzer-config XXX=YYY` style flag.
Then surface the `-analyzer-config` to the `clang` frontend;

After all of this, we can pursue the tablegen approach described 
https://discourse.llvm.org/t/rfc-tablegen-clang-static-analyzer-engine-options-for-better-documentation/61488

In this patch, I'm proposing flag deprecations.
We should support deprecated analyzer flags for at least one release. In this 
case I'm planning to drop this flag in `clang-17`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126215

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/Analyses.def
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -171,13 +171,7 @@
 }
 
 // Create the analyzer component creators.
-switch (Opts->AnalysisStoreOpt) {
-default:
-  llvm_unreachable("Unknown store manager.");
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATEFN)   \
-  case NAME##Model: CreateStoreMgr = CREATEFN; break;
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-}
+CreateStoreMgr = &CreateRegionStoreManager;
 
 switch (Opts->AnalysisConstraintsOpt) {
 default:
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -816,18 +816,6 @@
 #include "clang/Driver/Options.inc"
 #undef ANALYZER_OPTION_WITH_MARSHALLING
 
-  if (Opts.AnalysisStoreOpt != RegionStoreModel) {
-switch (Opts.AnalysisStoreOpt) {
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)   \
-  case NAME##Model:\
-GenerateArg(Args, OPT_analyzer_store, CMDFLAG, SA);\
-break;
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-default:
-  llvm_unreachable("Tried to generate unknown analysis store.");
-}
-  }
-
   if (Opts.AnalysisConstraintsOpt != RangeConstraintsModel) {
 switch (Opts.AnalysisConstraintsOpt) {
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN) \
@@ -915,21 +903,6 @@
 #include "clang/Driver/Options.inc"
 #undef ANALYZER_OPTION_WITH_MARSHALLING
 
-  if (Arg *A = Args.getLastArg(OPT_analyzer_store)) {
-StringRef Name = A->getValue();
-AnalysisStores Value = llvm::StringSwitch(Name)
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN) \
-  .Case(CMDFLAG, NAME##Model)
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-  .Default(NumStores);
-if (Value == NumStores) {
-  Diags.Report(diag::err_drv_invalid_value)
-<< A->getAsString(Args) << Name;
-} else {
-  Opts.AnalysisStoreOpt = Value;
-}
-  }
-
   if (Arg *A = Args.getLastArg(OPT_analyzer_constraints)) {
 StringRef Name = A->getValue();
 AnalysisConstraints Value = llvm::StringSwitch(Name)
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -40,13 +40,6 @@
 NumAnalyses
 };
 
-/// AnalysisStores - Set of available analysis store models.
-enum AnalysisStores {
-#define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN) NAME##Model,
-#include "clang/StaticAnalyzer/Core/Analyses.def"
-NumStores
-};
-
 /// AnalysisConstraints - Set of available constraint models.
 enum AnalysisConstraints {
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN) NAME##Model,
@@ -207,7 +200,6 @@
   /// A key-value table of use-specified configuration values.
   // TODO: This shouldn't be public.
   ConfigTable Config;
-  AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
   AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
Index: clang/include/clang/StaticAnalyzer/Core/Analys

  1   2   3   >