[PATCH] D114859: [clang-format] Add better support for co-routinues

[MyDeveloperDay 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 rG57b95aed2a04: [clang-format] Add better support for 
co-routinues (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114859

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/TokenAnnotator.cpp
  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
@@ -22724,6 +22724,65 @@
   verifyFormat("export", Style);
 }
 
+TEST_F(FormatTest, CoroutineForCoawait) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("for co_await (auto x : range())\n  ;");
+  verifyFormat("for (auto i : arr) {\n"
+   "}",
+   Style);
+  verifyFormat("for co_await (auto i : arr) {\n"
+   "}",
+   Style);
+  verifyFormat("for co_await (auto i : foo(T{})) {\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTest, CoroutineCoAwait) {
+  verifyFormat("int x = co_await foo();");
+  verifyFormat("int x = (co_await foo());");
+  verifyFormat("co_await (42);");
+  verifyFormat("void operator co_await(int);");
+  verifyFormat("void operator co_await(a);");
+  verifyFormat("co_await a;");
+  verifyFormat("co_await missing_await_resume{};");
+  verifyFormat("co_await a; // comment");
+  verifyFormat("void test0() { co_await a; }");
+  verifyFormat("co_await co_await co_await foo();");
+  verifyFormat("co_await foo().bar();");
+  verifyFormat("co_await [this]() -> Task { co_return x; }");
+  verifyFormat("co_await [this](int a, int b) -> Task { co_return co_await "
+   "foo(); }(x, y);");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  verifyFormat("co_await [this](int a, int b) -> Task {\n"
+   "  co_return co_await foo();\n"
+   "}(x, y);",
+   Style);
+  verifyFormat("co_await;");
+}
+
+TEST_F(FormatTest, CoroutineCoYield) {
+  verifyFormat("int x = co_yield foo();");
+  verifyFormat("int x = (co_yield foo());");
+  verifyFormat("co_yield (42);");
+  verifyFormat("co_yield {42};");
+  verifyFormat("co_yield 42;");
+  verifyFormat("co_yield n++;");
+  verifyFormat("co_yield ++n;");
+  verifyFormat("co_yield;");
+}
+
+TEST_F(FormatTest, CoroutineCoReturn) {
+  verifyFormat("co_return (42);");
+  verifyFormat("co_return;");
+  verifyFormat("co_return {};");
+  verifyFormat("co_return x;");
+  verifyFormat("co_return co_await foo();");
+  verifyFormat("co_return co_yield foo();");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2399,6 +2399,8 @@
   if (Style.Language == FormatStyle::LK_JavaScript &&
   FormatTok->is(Keywords.kw_await))
 nextToken();
+  if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
+nextToken();
   if (FormatTok->Tok.is(tok::l_paren))
 parseParens();
   if (FormatTok->Tok.is(tok::l_brace)) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1004,6 +1004,8 @@
 if (CurrentToken && CurrentToken->is(Keywords.kw_await))
   next();
   }
+  if (Style.isCpp() && CurrentToken && CurrentToken->is(tok::kw_co_await))
+next();
   Contexts.back().ColonIsForRangeExpr = true;
   next();
   if (!parseParens())
@@ -2952,6 +2954,14 @@
   if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
 return false;
 
+  // operator co_await(x)
+  if (Right.is(tok::l_paren) && Left.is(tok::kw_co_await) && Left.Previous &&
+  Left.Previous->is(tok::kw_operator))
+return false;
+  // co_await (x), co_yield (x), co_return (x)
+  if (Left.isOneOf(tok::kw_co_await, tok::kw_co_yield, tok::kw_co_return) &&
+  Right.isNot(tok::semi))
+return true;
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -265,7 +265,7 @@
   space before parentheses. The custom options can be set using
   ``SpaceBeforeParensOptions``.
 
-- Improved Cpp20 Modules support.
+- Improved C++20 Modules and Coroutines support.
 
 libclang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org

[clang] 57b95ae - [clang-format] Add better support for co-routinues

[via cfe-commits]

Author: mydeveloperday
Date: 2021-12-02T08:06:43Z
New Revision: 57b95aed2a04f00dc84c4c4926c506528081196d

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

LOG: [clang-format] Add better support for co-routinues

Responding to a Discord call to help {D113977} and heavily inspired by the 
unlanded {D34225} add some support to help coroutinues from not being formatted 
from

```for co_await(auto elt : seq)```

to

```
for
co_await(auto elt : seq)
```

Because of the dominance of clang-format in the C++ community, I don't think we 
should make it the blocker that prevents users from embracing the newer parts 
of the standard because we butcher the layout of some of the new constucts.

Reviewed By: HazardyKnusperkeks, Quuxplusone, ChuanqiXu

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 20e99fbf2e136..149d0e169389f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -265,7 +265,7 @@ clang-format
   space before parentheses. The custom options can be set using
   ``SpaceBeforeParensOptions``.
 
-- Improved Cpp20 Modules support.
+- Improved C++20 Modules and Coroutines support.
 
 libclang
 

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 22faf54b017da..a94d8cdc3b041 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1004,6 +1004,8 @@ class AnnotatingParser {
 if (CurrentToken && CurrentToken->is(Keywords.kw_await))
   next();
   }
+  if (Style.isCpp() && CurrentToken && CurrentToken->is(tok::kw_co_await))
+next();
   Contexts.back().ColonIsForRangeExpr = true;
   next();
   if (!parseParens())
@@ -2952,6 +2954,14 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
   if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
 return false;
 
+  // operator co_await(x)
+  if (Right.is(tok::l_paren) && Left.is(tok::kw_co_await) && Left.Previous &&
+  Left.Previous->is(tok::kw_operator))
+return false;
+  // co_await (x), co_yield (x), co_return (x)
+  if (Left.isOneOf(tok::kw_co_await, tok::kw_co_yield, tok::kw_co_return) &&
+  Right.isNot(tok::semi))
+return true;
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index a7e0422e3146c..5b9fe267aae60 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2399,6 +2399,8 @@ void UnwrappedLineParser::parseForOrWhileLoop() {
   if (Style.Language == FormatStyle::LK_JavaScript &&
   FormatTok->is(Keywords.kw_await))
 nextToken();
+  if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
+nextToken();
   if (FormatTok->Tok.is(tok::l_paren))
 parseParens();
   if (FormatTok->Tok.is(tok::l_brace)) {

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 9b6607b46d93a..bda5f7019416e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22724,6 +22724,65 @@ TEST_F(FormatTest, Cpp20ModulesSupport) {
   verifyFormat("export", Style);
 }
 
+TEST_F(FormatTest, CoroutineForCoawait) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat("for co_await (auto x : range())\n  ;");
+  verifyFormat("for (auto i : arr) {\n"
+   "}",
+   Style);
+  verifyFormat("for co_await (auto i : arr) {\n"
+   "}",
+   Style);
+  verifyFormat("for co_await (auto i : foo(T{})) {\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTest, CoroutineCoAwait) {
+  verifyFormat("int x = co_await foo();");
+  verifyFormat("int x = (co_await foo());");
+  verifyFormat("co_await (42);");
+  verifyFormat("void operator co_await(int);");
+  verifyFormat("void operator co_await(a);");
+  verifyFormat("co_await a;");
+  verifyFormat("co_await missing_await_resume{};");
+  verifyFormat("co_await a; // comment");
+  verifyFormat("void test0() { co_await a; }");
+  verifyFormat("co_await co_await co_await foo();");
+  verifyFormat("co_await foo().bar();");
+  verifyFormat("co_await [this]() -> Task { co_return x; }");
+  verifyFormat("co_await [this](int a, int b) -> Task { co_return co_await "
+   "foo(); }(x, y);");
+
+  FormatStyle Style = getLLVMStyle();
+  Style.ColumnLimit = 40;
+  ver

[PATCH] D114864: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:131
+  const auto *MostRecent = RD->getMostRecentDecl();
+  if (SM.isInMainFile(MostRecent->getBeginLoc())) {
+if (const auto *Definition = RD->getDefinition())

nit: I'd just inline `RD->getMostRecentDecl()` here.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:42
   },
+  {
+  "class ^X {}; class ^X;",

might be worth having a fixme comment here (around we should ignore all 
re-decls when we can see a def)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114864

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

[PATCH] D114864: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 391249.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114864

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,18 @@
   "class ^X;",
   "X *y;",
   },
+  // FIXME(kirillbobyrev): When definition is available, we don't need to
+  // mark forward declarations as used.
+  {
+  "class ^X {}; class ^X;",
+  "X *y;",
+  },
+  // We already have forward declaration in the main file, the other
+  // non-definition declarations are not needed.
+  {
+  "class ^X {}; class X;",
+  "class X; X *y;",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
 class ReferencedLocationCrawler
 : public RecursiveASTVisitor {
 public:
-  ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {}
+  ReferencedLocationCrawler(ReferencedLocations &Result,
+const SourceManager &SM)
+  : Result(Result), SM(SM) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 add(DRE->getDecl());
@@ -120,6 +122,17 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
+// If we see a declaration in the mainfile, skip all non-definition decls.
+// We only do this for classes because forward declarations are common and
+// we don't want to include every header that forward-declares the symbol
+// because they're often unrelated.
+if (const auto *RD = llvm::dyn_cast(D)) {
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
+if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
+  Result.insert(Definition->getLocation());
+return;
+  }
+}
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());
   }
@@ -128,6 +141,7 @@
 
   ReferencedLocations &Result;
   llvm::DenseSet Visited;
+  const SourceManager &SM;
 };
 
 // Given a set of referenced FileIDs, determines all the potentially-referenced
@@ -253,7 +267,7 @@
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
   trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
-  ReferencedLocationCrawler Crawler(Result);
+  ReferencedLocationCrawler Crawler(Result, AST.getSourceManager());
   Crawler.TraverseAST(AST.getASTContext());
   findReferencedMacros(AST, Result);
   return Result;


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,18 @@
   "class ^X;",
   "X *y;",
   },
+  // FIXME(kirillbobyrev): When definition is available, we don't need to
+  // mark forward declarations as used.
+  {
+  "class ^X {}; class ^X;",
+  "X *y;",
+  },
+  // We already have forward declaration in the main file, the other
+  // non-definition declarations are not needed.
+  {
+  "class ^X {}; class X;",
+  "class X; X *y;",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
 class ReferencedLocationCrawler
 : public RecursiveASTVisitor {
 public:
-  ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {}
+  ReferencedLocationCrawler(ReferencedLocations &Result,
+const SourceManager &SM)
+  : Result(Result), SM(SM) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 add(DRE->getDecl());
@@ -120,6 +122,17 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
+// If we see a declaration in the mainfile, skip all non-definition decls.
+// We only do this for classes because forward declarations are common and
+// we don't want to include every header that forward-declares the symbol
+// becaus

[clang-tools-extra] d3a4ef3 - [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-12-02T10:26:08+01:00
New Revision: d3a4ef35685c6aaad74294b678e77e1b75af1918

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

LOG: [clangd] IncludeClenaer: Don't mark forward declarations of a class if 
it's declared in the main file

This will mark more headers that are unrelated to used symbol but contain its
forawrd declaration. E.g. the following are examples of headers forward
declaring `llvm::StringRef`:

- clang/include/clang/Basic/Cuda.h
- llvm/include/llvm/Support/SHA256.h
- llvm/include/llvm/Support/TrigramIndex.h
- llvm/include/llvm/Support/RandomNumberGenerator.
- ... and more (~50 in total)

This patch is a reduced version of D112707 which was controversial.

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 33964d9f83ce0..76d5d626b9f88 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@ namespace {
 class ReferencedLocationCrawler
 : public RecursiveASTVisitor {
 public:
-  ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {}
+  ReferencedLocationCrawler(ReferencedLocations &Result,
+const SourceManager &SM)
+  : Result(Result), SM(SM) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 add(DRE->getDecl());
@@ -120,6 +122,17 @@ class ReferencedLocationCrawler
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
+// If we see a declaration in the mainfile, skip all non-definition decls.
+// We only do this for classes because forward declarations are common and
+// we don't want to include every header that forward-declares the symbol
+// because they're often unrelated.
+if (const auto *RD = llvm::dyn_cast(D)) {
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
+if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
+  Result.insert(Definition->getLocation());
+return;
+  }
+}
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());
   }
@@ -128,6 +141,7 @@ class ReferencedLocationCrawler
 
   ReferencedLocations &Result;
   llvm::DenseSet Visited;
+  const SourceManager &SM;
 };
 
 // Given a set of referenced FileIDs, determines all the potentially-referenced
@@ -253,7 +267,7 @@ FileID headerResponsible(FileID ID, const SourceManager &SM,
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
   trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
-  ReferencedLocationCrawler Crawler(Result);
+  ReferencedLocationCrawler Crawler(Result, AST.getSourceManager());
   Crawler.TraverseAST(AST.getASTContext());
   findReferencedMacros(AST, Result);
   return Result;

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index f72791da4030a..ddc0a99d58356 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,18 @@ TEST(IncludeCleaner, ReferencedLocations) {
   "class ^X;",
   "X *y;",
   },
+  // FIXME(kirillbobyrev): When definition is available, we don't need to
+  // mark forward declarations as used.
+  {
+  "class ^X {}; class ^X;",
+  "X *y;",
+  },
+  // We already have forward declaration in the main file, the other
+  // non-definition declarations are not needed.
+  {
+  "class ^X {}; class X;",
+  "class X; X *y;",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",



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

[PATCH] D114864: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file

[Kirill Bobyrev 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 rGd3a4ef35685c: [clangd] IncludeClenaer: Don't mark 
forward declarations of a class if it's… (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114864

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,18 @@
   "class ^X;",
   "X *y;",
   },
+  // FIXME(kirillbobyrev): When definition is available, we don't need to
+  // mark forward declarations as used.
+  {
+  "class ^X {}; class ^X;",
+  "X *y;",
+  },
+  // We already have forward declaration in the main file, the other
+  // non-definition declarations are not needed.
+  {
+  "class ^X {}; class X;",
+  "class X; X *y;",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
 class ReferencedLocationCrawler
 : public RecursiveASTVisitor {
 public:
-  ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {}
+  ReferencedLocationCrawler(ReferencedLocations &Result,
+const SourceManager &SM)
+  : Result(Result), SM(SM) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 add(DRE->getDecl());
@@ -120,6 +122,17 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
+// If we see a declaration in the mainfile, skip all non-definition decls.
+// We only do this for classes because forward declarations are common and
+// we don't want to include every header that forward-declares the symbol
+// because they're often unrelated.
+if (const auto *RD = llvm::dyn_cast(D)) {
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
+if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
+  Result.insert(Definition->getLocation());
+return;
+  }
+}
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());
   }
@@ -128,6 +141,7 @@
 
   ReferencedLocations &Result;
   llvm::DenseSet Visited;
+  const SourceManager &SM;
 };
 
 // Given a set of referenced FileIDs, determines all the potentially-referenced
@@ -253,7 +267,7 @@
 ReferencedLocations findReferencedLocations(ParsedAST &AST) {
   trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
-  ReferencedLocationCrawler Crawler(Result);
+  ReferencedLocationCrawler Crawler(Result, AST.getSourceManager());
   Crawler.TraverseAST(AST.getASTContext());
   findReferencedMacros(AST, Result);
   return Result;


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,18 @@
   "class ^X;",
   "X *y;",
   },
+  // FIXME(kirillbobyrev): When definition is available, we don't need to
+  // mark forward declarations as used.
+  {
+  "class ^X {}; class ^X;",
+  "X *y;",
+  },
+  // We already have forward declaration in the main file, the other
+  // non-definition declarations are not needed.
+  {
+  "class ^X {}; class X;",
+  "class X; X *y;",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
 class ReferencedLocationCrawler
 : public RecursiveASTVisitor {
 public:
-  ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {}
+  ReferencedLocationCrawler(ReferencedLocations &Result,
+const SourceManager &SM)
+  : Result(Result), SM(SM) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
 add(DRE->getDecl());
@@ -120,6 +122,17 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
+// If we see a declaration in the mainfile, skip all non-definition decls.
+// We only do this for classes because forwar

[PATCH] D114937: [PowerPC] [Clang] Fix alignment adjustment of single-elemented float128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf created this revision.
qiucf added reviewers: jsji, nemanjai, rjmccall, shchenz, PowerPC, 
hubert.reinterpretcast.
Herald added a subscriber: kbarton.
qiucf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This does similar thing to D91596 , but fixes 
single element 128-bit float type like `struct { long double x; }`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114937

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ppc64le-varargs-f128.c


Index: clang/test/CodeGen/ppc64le-varargs-f128.c
===
--- clang/test/CodeGen/ppc64le-varargs-f128.c
+++ clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -107,12 +107,16 @@
 // IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
 // IEEE: call void @llvm.va_start(i8* %[[AP1]])
 // IEEE: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]]
-// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = getelementptr inbounds i8, i8* %[[CUR]], i64 
16
+// IEEE: %[[P0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %[[P0]], 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = getelementptr inbounds i8, i8* %[[ALIGN]], 
i64 16
 // IEEE: store i8* %[[V0]], i8** %[[AP]], align 8
-// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to %struct.ldbl128_s*
+// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to %struct.ldbl128_s*
 // IEEE: %[[V2:[0-9a-zA-Z_.]+]] = bitcast %struct.ldbl128_s* 
%[[TMP:[0-9a-zA-Z_.]+]] to i8*
 // IEEE: %[[V3:[0-9a-zA-Z_.]+]] = bitcast %struct.ldbl128_s* %[[V1]] to i8*
-// IEEE: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 %[[V2]], i8* align 
8 %[[V3]], i64 16, i1 false)
+// IEEE: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 16 %[[V2]], i8* align 
16 %[[V3]], i64 16, i1 false)
 // IEEE: %[[COERCE:[0-9a-zA-Z_.]+]] = getelementptr inbounds 
%struct.ldbl128_s, %struct.ldbl128_s* %[[TMP]], i32 0, i32 0
 // IEEE: %[[V4:[0-9a-zA-Z_.]+]] = load fp128, fp128* %[[COERCE]], align 16
 // IEEE: call void @foo_ls(fp128 inreg %[[V4]])
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5083,13 +5083,16 @@
   if (const ComplexType *CTy = Ty->getAs())
 Ty = CTy->getElementType();
 
+  auto FloatUsesVector = [this](QualType Ty){
+return Ty->isRealFloatingType() && &getContext().getFloatTypeSemantics(
+   Ty) == &llvm::APFloat::IEEEquad();
+  };
+
   // Only vector types of size 16 bytes need alignment (larger types are
   // passed via reference, smaller types are not aligned).
   if (Ty->isVectorType()) {
 return CharUnits::fromQuantity(getContext().getTypeSize(Ty) == 128 ? 16 : 
8);
-  } else if (Ty->isRealFloatingType() &&
- &getContext().getFloatTypeSemantics(Ty) ==
- &llvm::APFloat::IEEEquad()) {
+  } else if (FloatUsesVector(Ty)) {
 // According to ABI document section 'Optional Save Areas': If extended
 // precision floating-point values in IEEE BINARY 128 QUADRUPLE PRECISION
 // format are supported, map them to a single quadword, quadword aligned.
@@ -5116,7 +5119,9 @@
 
   // With special case aggregates, only vector base types need alignment.
   if (AlignAsType) {
-return CharUnits::fromQuantity(AlignAsType->isVectorType() ? 16 : 8);
+bool UsesVector = AlignAsType->isVectorType() ||
+  FloatUsesVector(QualType(AlignAsType, 0));
+return CharUnits::fromQuantity(UsesVector ? 16 : 8);
   }
 
   // Otherwise, we only need alignment for any aggregate type that


Index: clang/test/CodeGen/ppc64le-varargs-f128.c
===
--- clang/test/CodeGen/ppc64le-varargs-f128.c
+++ clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -107,12 +107,16 @@
 // IEEE: %[[AP1:[0-9a-zA-Z_.]+]] = bitcast i8** %[[AP:[0-9a-zA-Z_.]+]] to i8*
 // IEEE: call void @llvm.va_start(i8* %[[AP1]])
 // IEEE: %[[CUR:[0-9a-zA-Z_.]+]] = load i8*, i8** %[[AP]]
-// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = getelementptr inbounds i8, i8* %[[CUR]], i64 16
+// IEEE: %[[P0:[0-9a-zA-Z_.]+]] = ptrtoint i8* %[[CUR]] to i64
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %[[P0]], 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[ALIGN:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[V0:[0-9a-zA-Z_.]+]] = getelementptr inbounds i8, i8* %[[ALIGN]], i64 16
 // IEEE: store i8* %[[V0]], i8** %[[AP]], align 8
-// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = bitcast i8* %[[CUR]] to %struct.ldbl128_s*
+// IEEE: %[[V1:[0-9a-zA-Z_.]+]] = bitcast i8* %[[ALIGN]] to %struct.ldbl128_s*
 // IEEE: %[[V2:[0-9a-zA-Z_.]+]] = bitcast %struct.ldbl128_s* %[[TMP:[0-9a-zA-Z_.]+]] to i8

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added a reviewer: steakhal.
Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: Szelethus.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Move the SymExpr simplification fixpoint logic into SValBuilder.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114938

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -21,6 +21,35 @@
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
+
+  // With one `simplifySValImpl` call, a compound symbols might collapse to
+  // simpler symbol tree that is still possible to further simplify. Thus, we
+  // do the simplification on a new symbol tree until we reach the simplest
+  // form, i.e. the fixpoint.
+  // Consider the following symbol `(b * b) * b * b` which has this tree:
+  //   *
+  //  / \
+  // *   b
+  ///  \
+  //   /b
+  // (b * b)
+  // Now, if the `b * b == 1` new constraint is added then during the first
+  // iteration we have the following transformations:
+  //   *  *
+  //  / \/ \
+  // *   b -->  b   b
+  ///  \
+  //   /b
+  //  1
+  // We need another iteration to reach the final result `1`.
+  SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
+
+  // Recursively descends into symbolic expressions and replaces symbols
+  // with their known values (in the sense of the getKnownValue() method).
+  // We traverse the symbol tree and query the constraint values for the
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValImpl(ProgramStateRef State, SVal V);
+
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
 ProgramStateManager &stateMgr)
@@ -40,8 +69,6 @@
   ///  (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
 
-  /// Recursively descends into symbolic expressions and replaces symbols
-  /// with their known values (in the sense of the getKnownValue() method).
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
 
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
@@ -1105,7 +1132,21 @@
   return nullptr;
 }
 
+SVal SimpleSValBuilder::simplifyUntilFixpoint(ProgramStateRef State, SVal Val) {
+  SVal SimplifiedVal = simplifySValImpl(State, Val);
+  // Do the simplification until we can.
+  while (SimplifiedVal != Val) {
+Val = SimplifiedVal;
+SimplifiedVal = simplifySValImpl(State, Val);
+  }
+  return SimplifiedVal;
+}
+
 SVal SimpleSValBuilder::simplifySVal(ProgramStateRef State, SVal V) {
+  return simplifyUntilFixpoint(State, V);
+}
+
+SVal SimpleSValBuilder::simplifySValImpl(ProgramStateRef State, SVal V) {
   // For now, this function tries to constant-fold symbols inside a
   // nonloc::SymbolVal, and does nothing else. More simplifications should
   // be possible, such as constant-folding an index in an ElementRegion.
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2191,42 +2191,6 @@
  Constraint->getMaxValue(), true);
 }
 
-// Simplify the given symbol with the help of the SValBuilder. In
-// SValBuilder::symplifySval, we traverse the symbol tree and query the
-// constraint values for the sub-trees and if a value is a constant we do the
-// constant folding. Compound symbols might collapse to simpler symbol tree
-// that is still possible to further simplify. Thus, we do the simplification on
-// a new symbol tree until we reach the simplest form, i.e. the fixpoint.
-//
-// Consider the following symbol `(b * b) * b * b` which has this tree:
-//   *
-//  / \
-// *   b
-///  \
-//   /b
-// (b * b)
-// Now, if the `b * b == 1` new constraint is added then during the first
-// iteration we have the following transformations:
-//   *  *
-//  / \/ \
-// *   b -->  b   b
-///  \
-//   /b
-//  1
-// We need another iteration to reach the final result `1`.
-LLVM_NODISCARD
-static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
-  const SymbolRef Sym) {
-  SVal Val = SVB.makeSymbolVal(Sym);
-  S

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Measurement results are attached, I think there is no relevant difference.

F20847935: another_fixpoint_before_merge_2.png 


F20847949: stats.html 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:544-546
+  // Set to true for attributes that support parameter pack expansion in its
+  // arguments.
+  bit ParmExpansionArgsSupport = 0;

aaron.ballman wrote:
> From a design perspective, I think this is a property of the parameters of an 
> attribute rather than of an attribute itself. So I'd rather not add a bit to 
> the `Attr` class, but instead modify the `Argument` class. There are a few 
> approaches to that which I can think of, and I'm not certain which is the 
> best approach.
> 
> 1) Introduce `ExpressionPackArgument` and attributes can opt into using it if 
> they want to support packs.
> 2) Add this bit to `VariadicExprArgument` and attributes can opt into using 
> it if they want to support packs.
> 3) Change `VariadicExprArgument` to also mean "allows packs" and all 
> attributes using this automatically support packs.
> 
> I think that my conservative preference is for #2, but my intuition is that 
> #3 is where we probably ultimately want to get to.
> 
> There are definitely some open questions with this approach that we'll have 
> to figure out:
> 
> * Can you mix packs with variadics in the same attribute?
> * Can you have two pack arguments in the same attribute?
> * Does the tablegen design also seem likely to work when we want to introduce 
> type packs instead of value packs?
> 
> I think the safest bet is to be conservative and not allow mixing packs with 
> variadics, and not allowing multiple packs. We should be able to diagnose 
> that situation from ClangAttrEmitter.cpp to help attribute authors out. 
> However, it'd be worth adding a FIXME comment to that diagnostic logic asking 
> whether we want to relax the behavior at some point. But if you feel strongly 
> that we should support these situations initially, I wouldn't be opposed.
Having `ParmExpansionArgsSupport` in `Attr` would allow attributes to populate 
with parameter packs across argument boundaries. It seems more permissive, but 
frankly I have no attachment to it, so I am okay with tying it to `Argument` 
instead.

Of the mentioned options, I agree #2 and #3 are the preferred paths. For 
simplicity I prefer #2, though I will have to figure out how best to add a 
check on individual arguments, which @erichkeane's suggestion to limit the 
attributes to only allowing parameter packs in variadic arguments as the last 
argument would help with. On the other hand, the population logic for most of 
the relevant attributes are explicitly defined in 
`clang/lib/Sema/SemaDeclAttr.cpp` (which will have to be changed with #3) so 
being more permissive - like the current version of the patch is - gives more 
freedom to the attributes.

> Does the tablegen design also seem likely to work when we want to introduce 
> type packs instead of value packs?

I am not sure about how tablegen would take it, but the parser doesn't 
currently parse more than a single type argument, so I don't see a need for 
allowing type packs at the moment.

> I think the safest bet is to be conservative and not allow mixing packs with 
> variadics, and not allowing multiple packs. We should be able to diagnose 
> that situation from ClangAttrEmitter.cpp to help attribute authors out. 
> However, it'd be worth adding a FIXME comment to that diagnostic logic asking 
> whether we want to relax the behavior at some point. But if you feel strongly 
> that we should support these situations initially, I wouldn't be opposed.

If we limit packs to `VariadicExprArgument` - like `annotate` does in the 
current state of this patch - having multiple packs in the same argument is 
simple to handle. Both are represented as `Expr` and the variadic argument will 
simply contain two expressions until the templates are instantiated and the 
packs are expanded. This is also a feature I would like to keep, though it 
should be possible to work around the limitation if we conclude it can't stay.




Comment at: clang/lib/Parse/ParseDecl.cpp:447
+  }
+  ArgExpr = Actions.ActOnPackExpansion(ArgExpr.get(), ConsumeToken());
+}

erichkeane wrote:
> Do you have a test for something that isn't a pack followed by an ellipsis?  
> What is the behavior there?   I'm also concerned that there doesn't seem to 
> be anything here that handles complex-pack expansions (like folds), can you 
> test that as well?
> 
> Also, why is this 'if' not up on line 429 (that is, outside of this 'else'?). 
>  I believe ParseAssignmentExpression is going to muck-up the tokens, so I'm 
> not particularly sure what is going to happen here?
> Do you have a test for something that isn't a pack followed by an ellipsis? 
> What is the behavior there? I'm also concerned that there doesn't seem to be 
> anything here that handles complex-pack expansions (like folds), can you test 
> that as well?

Good question! I would expect it to fail 

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:447
+  }
+  ArgExpr = Actions.ActOnPackExpansion(ArgExpr.get(), ConsumeToken());
+}

steffenlarsen wrote:
> erichkeane wrote:
> > Do you have a test for something that isn't a pack followed by an ellipsis? 
> >  What is the behavior there?   I'm also concerned that there doesn't seem 
> > to be anything here that handles complex-pack expansions (like folds), can 
> > you test that as well?
> > 
> > Also, why is this 'if' not up on line 429 (that is, outside of this 
> > 'else'?).  I believe ParseAssignmentExpression is going to muck-up the 
> > tokens, so I'm not particularly sure what is going to happen here?
> > Do you have a test for something that isn't a pack followed by an ellipsis? 
> > What is the behavior there? I'm also concerned that there doesn't seem to 
> > be anything here that handles complex-pack expansions (like folds), can you 
> > test that as well?
> 
> Good question! I would expect it to fail in kind during template 
> instantiation, or earlier if it is not an expression, but I will test it out 
> once we converge on a new solution.
> 
> > Also, why is this 'if' not up on line 429 (that is, outside of this 
> > 'else'?). I believe ParseAssignmentExpression is going to muck-up the 
> > tokens, so I'm not particularly sure what is going to happen here?
> 
> I originally tried having the `ActOnPackExpansion` earlier, but it did indeed 
> cause trouble. However, if my understanding of the previous two branches is 
> correct, they are:
> 
> 
>   # Type argument. This could be consuming the ellipsis as well, though I 
> suspect it needs another path given that it does not seem to produce any 
> expressions. Note however that it currently seems to stop argument parsing 
> after reading the first type (confirmed by the FIXME) so parameter pack logic 
> for this branch would probably be more suitable once it actually allows 
> attributes to take multiple types.
>   # Identifier argument. Correct me if I am wrong, but I don't think this can 
> be populated by a parameter pack.
> 
> I could add the diagnostic to these if ellipsis is found, but if my 
> assumptions are correct it doesn't make much sense to expand these branches 
> beyond that.
> I'm also concerned that there doesn't seem to be anything here that handles 
> complex-pack expansions (like folds), can you test that as well?

Sorry, I forgot to address this. I have not tested it, but I would expect 
complex-pack expansions like folds to be at expression-level. I will make sure 
to test this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114439

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

[PATCH] D113896: [clangd] cleanup of header guard names

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/PathMapping.h:2
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHMAPPING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHMAPPING_H
+

Please move the header guard after the comment like we do in the other files.



Comment at: clang-tools-extra/clangd/PathMapping.h:72
+
+#endif

Please add a closing comment (`// LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHMAPPING_H`)



Comment at: clang-tools-extra/clangd/unittests/LSPClient.h:1
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H

Please move the header guard after the comment like we do in the other files.



Comment at: clang-tools-extra/clangd/unittests/LSPClient.h:87
+
+#endif

Also needs a closing comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[clang] 69deb13 - Use cc/c++ instead of gcc/g++ on FreeBSD.

[Frederic Cambus via cfe-commits]

Author: Frederic Cambus
Date: 2021-12-02T11:52:40+01:00
New Revision: 69deb1371fd4cd2fbf37a82cbc21df79c6d51c70

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

LOG: Use cc/c++ instead of gcc/g++ on FreeBSD.

All supported FreeBSD platforms do not have GCC in base anymore.

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

Added: 


Modified: 
clang/tools/scan-build/libexec/ccc-analyzer

Removed: 




diff  --git a/clang/tools/scan-build/libexec/ccc-analyzer 
b/clang/tools/scan-build/libexec/ccc-analyzer
index 2a589a3a0ef5e..35b7a27126c58 100755
--- a/clang/tools/scan-build/libexec/ccc-analyzer
+++ b/clang/tools/scan-build/libexec/ccc-analyzer
@@ -80,7 +80,7 @@ if (`uname -s` =~ m/Darwin/) {
   if (-x "/usr/bin/xcrun") {
 $UseXCRUN = 1;
   }
-} elsif (`uname -s` =~ m/OpenBSD/) {
+} elsif (`uname -s` =~ m/(FreeBSD|OpenBSD)/) {
   $DefaultCCompiler = 'cc';
   $DefaultCXXCompiler = 'c++';
 } else {



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

[PATCH] D114530: [clang][scan-build] Use cc/c++ instead of gcc/g++ on FreeBSD.

[Frederic Cambus via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69deb1371fd4: Use cc/c++ instead of gcc/g++ on FreeBSD. 
(authored by fcambus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114530

Files:
  clang/tools/scan-build/libexec/ccc-analyzer


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -80,7 +80,7 @@
   if (-x "/usr/bin/xcrun") {
 $UseXCRUN = 1;
   }
-} elsif (`uname -s` =~ m/OpenBSD/) {
+} elsif (`uname -s` =~ m/(FreeBSD|OpenBSD)/) {
   $DefaultCCompiler = 'cc';
   $DefaultCXXCompiler = 'c++';
 } else {


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -80,7 +80,7 @@
   if (-x "/usr/bin/xcrun") {
 $UseXCRUN = 1;
   }
-} elsif (`uname -s` =~ m/OpenBSD/) {
+} elsif (`uname -s` =~ m/(FreeBSD|OpenBSD)/) {
   $DefaultCCompiler = 'cc';
   $DefaultCXXCompiler = 'c++';
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Given this is a minor change that only affects users of 
disable_sanitizer_instrumentation, I'm inclined towards landing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

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

[PATCH] D113896: [clangd] cleanup of header guard names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

LG apart from closing comments, some of the files have it and some don't (apart 
from the newly introduced ones, it didn't add it to the files that didn't have 
a closing comment, but updated the ones that had).
The code from the tidy check looks like it should be handling this case, can 
you check what's going on?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[PATCH] D108301: [MSP430][Clang] Update hard-coded MCU data

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:223-224
+  MCUData LoadedMCUData;
+  if (const Arg *MCUArg = Args.getLastArg(options::OPT_mmcu_EQ))
+LoadedMCUData = getMCUData(MCUArg->getValue());
+

It might be worth moving this under `hwmult` option check, so we do not need to 
load MCU data again if the option is properly specificed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108301

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

[clang] 3b44264 - [VE][Clang][NFC] Disable VE toolchain tests on Windows

[Simon Moll via cfe-commits]

Author: Simon Moll
Date: 2021-12-02T13:33:46+01:00
New Revision: 3b442644e278b53e351addf349c75b995b10f029

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

LOG: [VE][Clang][NFC] Disable VE toolchain tests on Windows

VE hardware is unsupported under Windows. Disable the clang VE toolchain
tests here. Tests breaking because of non-POSIX path separators.

Added: 


Modified: 
clang/test/Driver/ve-toolchain.c
clang/test/Driver/ve-toolchain.cpp

Removed: 




diff  --git a/clang/test/Driver/ve-toolchain.c 
b/clang/test/Driver/ve-toolchain.c
index eb12fdffa7f6b..fc6a0cf88765c 100644
--- a/clang/test/Driver/ve-toolchain.c
+++ b/clang/test/Driver/ve-toolchain.c
@@ -1,5 +1,6 @@
 /// Check the behavior of toolchain for NEC Aurora VE
 /// REQUIRES: ve-registered-target
+/// UNSUPPORTED: system-windows
 
 
///-
 /// Checking dwarf-version

diff  --git a/clang/test/Driver/ve-toolchain.cpp 
b/clang/test/Driver/ve-toolchain.cpp
index 6567f9606b754..bb02f7e63abd4 100644
--- a/clang/test/Driver/ve-toolchain.cpp
+++ b/clang/test/Driver/ve-toolchain.cpp
@@ -1,5 +1,6 @@
 /// Check the behavior of toolchain for NEC Aurora VE
 /// REQUIRES: ve-registered-target
+/// UNSUPPORTED: system-windows
 
 
///-
 /// Checking dwarf-version



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

[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

@sammccall

There is one part of your comment that I couldn't understand, could you please 
clarify what you mean bi this?

> we can't skip based on a definition (main file or class) that can't stand 
> alone, usually because of qualifiers. e.g. void foo::bar() {}.

FWIW if we decided to only deal with `RecordDecl` I don't know if your comment 
refers to them or it's some other context. An example for what you mean would 
be appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112707

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

[PATCH] D110618: [HIPSPV][2/4] Add HIPSPV tool chain

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D110618#3152939 , @linjamaki wrote:

> In D110618#3148501 , @Anastasia 
> wrote:
>
>> Could you please clarify the interface to SPIRV-LLVM-Translator tool, 
>> specifically:
>>
>> - Does clang lookup the path to the translator or assume any default path?
>
> HIPSPV primarily relies on the system’s PATH for locating the translator.
>
> Additionally, the translator is sought in the same directory where the Clang 
> driver is installed in. This is rather a side-effect than a feature. The 
> directory is added to Clang’s internal search paths for looking up the 
> `clang-offload-bundler` tool required by the HIPSPV tool chain.
>
>> - Is there any diagnostic provided if the translator not installed/found?
>
> Clang displays the following standard error if the translator is not found:
>
>   error: unable to execute command: Executable "llvm-spirv" doesn't exist!
>
>
>
>> - How does clang synchronize with the translator versions i.e. some LLVM IR 
>> might not be ingested by certain version of the translator. Would this 
>> results in the translator ICE or error in the translator, or is it something 
>> that can be diagnosed early by clang or during clang build/installation?
>
> A version mismatch may result in an error in the translator. This is a known 
> issue and a reason we want to switch to the SPIR-V backend when it lands on 
> the LLVM - **the translator is meant to be used temporarily by the HIPSPV**. 
> The synchronization could be improved by having the Clang to seek a SPIR-V 
> Translator binary that is named with the LLVM version it has been built 
> against - e.g. “llvm-spirv-14” for the next LLVM release.
>
> AFAIK, Clang’s infrastructure does not support early diagnosis on external 
> tools. The diagnosis during Clang’s build and installation probably won’t 
> matter much if the Clang is distributed as binary to other systems - the 
> destination system might not have the required translator version.

Alternatively, we could see if translator could emit a diagnostic by looking at 
the clang version emitted in metadata... I presume this might not be too hard 
to implement...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110618

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

[PATCH] D112110: [OpenCL] queue_t and ndrange_t can't be defined in program scope.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.
Herald added a subscriber: Naghasan.



Comment at: clang/test/SemaOpenCL/invalid-types.cl:10
+
+typedef image2d_t test;
+global test qtt; // expected-error {{the '__global ndrange_t' type cannot be 
used to declare a program scope variable}}

Did you mean to use `ndrange_t` or `queue_t` here? Otherwise, it doesn't seem 
that your commit is related to the image types...


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

https://reviews.llvm.org/D112110

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

[PATCH] D112626: Convert float to double on __builtin_dump_struct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2090-2094
+// Variadic functions expect the caller to promote float to double.
+if (CanonicalType == Context.FloatTy) {
+  FieldPtr =
+  CGF.Builder.CreateFPExt(FieldPtr, CGF.ConvertType(Context.DoubleTy));
+}

rjmccall wrote:
> aaron.ballman wrote:
> > This change is an improvement as far as it goes, but I think we might be 
> > missing other floating-point promotions here. For example, `__fp16` fields 
> > also seem to be unusable: https://godbolt.org/z/z3a45f9YE
> > 
> > Also, we don't seem to handle the integer promotions at all (but still get 
> > correct results there), so I think we're getting the correct behavior there 
> > by chance rather than by design. Oh, yeah, note the differences here: 
> > https://godbolt.org/z/f13eq3668
> > 
> > ```
> > foo:
> >   ...
> >   %7 = load i8, i8* %4, align 1, !dbg !217
> >   %8 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], [6 
> > x i8]* @2, i32 0, i32 0), i8 %7), !dbg !217
> >   ...
> > 
> > bar:
> >   ...
> >   %2 = load i8, i8* %1, align 1, !dbg !222
> >   %3 = zext i8 %2 to i32, !dbg !222
> >   %4 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([5 x i8], [5 
> > x i8]* @.str, i64 0, i64 0), i32 %3), !dbg !223
> >   ...
> > ```
> > I think we should probably fix all of the promotion problems at once rather 
> > than piecemeal.
> > 
> It's actually really annoying that this logic has to be duplicated in IRGen 
> instead of being able to take advantage of the existing promotion logic in 
> Sema.  Can we just generate a helper function in Sema and somehow link it to 
> the builtin call?
> 
> Um.  Also, the `static` local DenseMap in the code above this is totally 
> unacceptable and should not have been committed.  Clang is supposed to be 
> embeddable as a library and should not be using global mutable variables.
> It's actually really annoying that this logic has to be duplicated in IRGen 
> instead of being able to take advantage of the existing promotion logic in 
> Sema. Can we just generate a helper function in Sema and somehow link it to 
> the builtin call?

That seems worth a shot but I think it is not something that needs to happen 
for this patch (that could be a rather heavy lift to ask someone who just 
joined the community) unless the basic fix starts getting out of hand.

> Um. Also, the static local DenseMap in the code above this is totally 
> unacceptable and should not have been committed. Clang is supposed to be 
> embeddable as a library and should not be using global mutable variables.

We do that with some degree of frequency already (command line arguments using 
`llvm::cl::opt` are all global mutable variables, such as 
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CoverageMappingGen.cpp#L34),
 but yeah, fixing that up would also not be a bad idea, but orthogonal to this 
patch IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112626

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

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a9487df73e9: Add new clang-tidy check for 
string_view(nullptr) (authored by CJ-Johnson, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1102 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using size_t = long long;
+using nullptr_t = decltype(nullptr);
+
+template 
+T &&declval();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+
+  basic_string_view(const CharT *);
+
+  // Not present in C++17 and C++20:
+  // basic_string_view(std::nullptr_t);
+
+  basic_string_view(const CharT *, size_t);
+
+  basic_string_view(const basic_string_view &);
+
+  basic_string_view &operator=(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>,
+   basic_string_view);
+template 
+bool operator<(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>,
+basic_string_view);
+template 
+bool operator<=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>,
+   basic_string_view);
+template 
+bool operator>(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>,
+basic_string_view);
+template 
+bool operator>=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>,
+basic_string_view);
+template 
+bool operator==(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>,
+basic_string_view);
+template 
+bool operator!=(basic_string_view,
+type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;
+
+(void)(std::string_view({})) /* a5 */; // Default `const CharT*`
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a5 */;
+  }
+
+  // Temporary Object
+  {
+(void)(std::string_view{nullptr}) /* a6 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a6 */;
+
+(void)(std::string_view{(nullptr)}) /* a7 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a7 */;
+
+(void)(std::string_view{{nullptr}}) /* a8 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: construct

[clang-tools-extra] 6a9487d - Add new clang-tidy check for string_view(nullptr)

[Yitzhak Mandelbaum via cfe-commits]

Author: CJ Johnson
Date: 2021-12-02T13:25:28Z
New Revision: 6a9487df73e917c4faf5e060f2bb33c6ade3f967

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

LOG: Add new clang-tidy check for string_view(nullptr)

Checks for various ways that the `const CharT*` constructor of 
`std::basic_string_view` can be passed a null argument and replaces them with 
the default constructor in most cases. For the comparison operators, braced 
initializer list does not compile so instead a call to `.empty()` or the empty 
string literal are used, where appropriate.

This prevents code from invoking behavior which is unconditionally undefined. 
The single-argument `const CharT*` constructor does not check for the null case 
before dereferencing its input. The standard is slated to add an 
explicitly-deleted overload to catch some of these cases: wg21.link/p2166

https://reviews.llvm.org/D114823 is a companion change to prevent duplicate 
warnings from the `bugprone-string-constructor` check.

Reviewed By: ymandel

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

Added: 
clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 9f0e6e3d7335d..82a807eaa1714 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -50,6 +50,7 @@
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
+#include "StringviewNullptrCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousIncludeCheck.h"
 #include "SuspiciousMemoryComparisonCheck.h"
@@ -157,6 +158,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-string-integer-assignment");
 CheckFactories.registerCheck(
 "bugprone-string-literal-with-embedded-nul");
+CheckFactories.registerCheck(
+"bugprone-stringview-nullptr");
 CheckFactories.registerCheck(
 "bugprone-suspicious-enum-usage");
 CheckFactories.registerCheck(

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 6bb9ed947439a..c63b16131fac7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -45,6 +45,7 @@ add_clang_library(clangTidyBugproneModule
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
+  StringviewNullptrCheck.cpp
   SuspiciousEnumUsageCheck.cpp
   SuspiciousIncludeCheck.cpp
   SuspiciousMemoryComparisonCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
new file mode 100644
index 0..1f7dc48bbe8fb
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -0,0 +1,191 @@
+//===--- StringviewNullptrCheck.cpp - clang-tidy 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "StringviewNullptrCheck.h"
+#include "../utils/TransformerClangTidyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
+#include "clang/Tooling/Transformer/Stencil.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+using namespace ::clang::ast_matchers;
+using namespace ::clang::transformer;
+
+namespace {
+AST_MATCHER_P(InitListExpr, initCountIs, unsigned, N) {
+  return Node.getNumInits() == N;
+}
+} // namespace
+
+RewriteRule StringviewNullptrCheckImpl() {
+  auto construction_warning =
+  cat("constructing basic_string_view from null is undefined; replace with 
"
+  "the default constructor");
+  auto ass

[PATCH] D114809: Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D114809#3165214 , @aaron.ballman 
wrote:

> LGTM! I had no idea...
>
> (As a follow-up question, should we be removing the `using` declarations 
> starting around line 141 or so? Many of them are not used, and if the 
> documentation dumper can't look through typedefs and gets the documentation 
> wrong, perhaps we want to discourage using them entirely?)

Aaron -- at least in our codebase, those are used a fair amount. What about 
updating the script to understand those typedefs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114809

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

[clang] 4e9e2f2 - Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

[Yitzhak Mandelbaum via cfe-commits]

Author: James King
Date: 2021-12-02T13:28:05Z
New Revision: 4e9e2f24178077d7055c0f667d13f887fef508d5

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

LOG: Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

Updates the return types of these matchers' definitions to use
`internal::Matcher` instead of `LambdaCaptureMatcher`. This
ensures that they are categorized as traversal matchers, instead of narrowing
matchers.

Reviewed By: ymandel, tdl-g, aaron.ballman

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

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index 8f310d42be1c9..9e71608a86250 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -4550,35 +4550,6 @@ Narrowing Matchers
 
 
 
-MatcherLambdaExpr>forEachLambdaCaptureLambdaCaptureMatcher
 InnerMatcher
-Matches each 
lambda capture in a lambda expression.
-
-Given
-  int main() {
-int x, y;
-float z;
-auto f = [=]() { return x + y + z; };
-  }
-lambdaExpr(forEachLambdaCapture(
-lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
-will trigger two matches, binding for 'x' and 'y' respectively.
-
-
-
-MatcherLambdaExpr>hasAnyCaptureLambdaCaptureMatcher 
InnerMatcher
-Matches any capture 
in a lambda expression.
-
-Given
-  void foo() {
-int t = 5;
-auto f = [=](){ return t; };
-  }
-lambdaExpr(hasAnyCapture(lambdaCapture())) and
-lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
-  both match `[=](){ return t; }`.
-
-
-
 MatcherMemberExpr>isArrow
 Matches member expressions 
that are called with '->' as opposed
 to '.'.
@@ -8405,6 +8376,35 @@ AST Traversal Matchers
 
 
 
+MatcherLambdaExpr>forEachLambdaCaptureMatcherLambdaCapture>
 InnerMatcher
+Matches each 
lambda capture in a lambda expression.
+
+Given
+  int main() {
+int x, y;
+float z;
+auto f = [=]() { return x + y + z; };
+  }
+lambdaExpr(forEachLambdaCapture(
+lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
+will trigger two matches, binding for 'x' and 'y' respectively.
+
+
+
+MatcherLambdaExpr>hasAnyCaptureMatcherLambdaCapture>
 InnerMatcher
+Matches any capture 
in a lambda expression.
+
+Given
+  void foo() {
+int t = 5;
+auto f = [=](){ return t; };
+  }
+lambdaExpr(hasAnyCapture(lambdaCapture())) and
+lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
+  both match `[=](){ return t; }`.
+
+
+
 MatcherMemberExpr>hasDeclarationMatcherDecl>  
InnerMatcher
 Matches a node if 
the declaration associated with that node
 matches the given matcher.

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index d6e5b215462b2..5221d05477d0e 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4227,8 +4227,8 @@ AST_MATCHER(VarDecl, isInitCapture) { return 
Node.isInitCapture(); }
 /// lambdaExpr(forEachLambdaCapture(
 /// lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
 /// will trigger two matches, binding for 'x' and 'y' respectively.
-AST_MATCHER_P(LambdaExpr, forEachLambdaCapture, LambdaCaptureMatcher,
-  InnerMatcher) {
+AST_MATCHER_P(LambdaExpr, forEachLambdaCapture,
+  internal::Matcher, InnerMatcher) {
   BoundNodesTreeBuilder Result;
   bool Matched = false;
   for (const auto &Capture : Node.captures()) {
@@ -4655,7 +4655,8 @@ extern const 
internal::VariadicAllOfMatcher lambdaCapture;
 /// lambdaExpr(hasAnyCapture(lambdaCapture())) and
 /// lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
 ///   both match `[=](){ return t; }`.
-AST_MATCHER_P(LambdaExpr, hasAnyCapture, LambdaCaptureMatcher, InnerMatcher) {
+AST_MATCHER_P(LambdaExpr, hasAnyCapture, internal::Matcher,
+  InnerMatcher) {
   for (const LambdaCapture &Capture : Node.captures()) {
 clang::ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
 if (InnerMatcher.matches(Capture, Fin

[PATCH] D114809: Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

[Yitzhak Mandelbaum 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 rG4e9e2f241780: Fix documentation for `forEachLambdaCapture` 
and `hasAnyCapture` (authored by jcking1034, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114809

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4227,8 +4227,8 @@
 /// lambdaExpr(forEachLambdaCapture(
 /// lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
 /// will trigger two matches, binding for 'x' and 'y' respectively.
-AST_MATCHER_P(LambdaExpr, forEachLambdaCapture, LambdaCaptureMatcher,
-  InnerMatcher) {
+AST_MATCHER_P(LambdaExpr, forEachLambdaCapture,
+  internal::Matcher, InnerMatcher) {
   BoundNodesTreeBuilder Result;
   bool Matched = false;
   for (const auto &Capture : Node.captures()) {
@@ -4655,7 +4655,8 @@
 /// lambdaExpr(hasAnyCapture(lambdaCapture())) and
 /// lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
 ///   both match `[=](){ return t; }`.
-AST_MATCHER_P(LambdaExpr, hasAnyCapture, LambdaCaptureMatcher, InnerMatcher) {
+AST_MATCHER_P(LambdaExpr, hasAnyCapture, internal::Matcher,
+  InnerMatcher) {
   for (const LambdaCapture &Capture : Node.captures()) {
 clang::ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
 if (InnerMatcher.matches(Capture, Finder, &Result)) {
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4550,35 +4550,6 @@
 
 
 
-MatcherLambdaExpr>forEachLambdaCaptureLambdaCaptureMatcher InnerMatcher
-Matches each lambda capture in a lambda expression.
-
-Given
-  int main() {
-int x, y;
-float z;
-auto f = [=]() { return x + y + z; };
-  }
-lambdaExpr(forEachLambdaCapture(
-lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
-will trigger two matches, binding for 'x' and 'y' respectively.
-
-
-
-MatcherLambdaExpr>hasAnyCaptureLambdaCaptureMatcher InnerMatcher
-Matches any capture in a lambda expression.
-
-Given
-  void foo() {
-int t = 5;
-auto f = [=](){ return t; };
-  }
-lambdaExpr(hasAnyCapture(lambdaCapture())) and
-lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
-  both match `[=](){ return t; }`.
-
-
-
 MatcherMemberExpr>isArrow
 Matches member expressions that are called with '->' as opposed
 to '.'.
@@ -8405,6 +8376,35 @@
 
 
 
+MatcherLambdaExpr>forEachLambdaCaptureMatcherLambdaCapture> InnerMatcher
+Matches each lambda capture in a lambda expression.
+
+Given
+  int main() {
+int x, y;
+float z;
+auto f = [=]() { return x + y + z; };
+  }
+lambdaExpr(forEachLambdaCapture(
+lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
+will trigger two matches, binding for 'x' and 'y' respectively.
+
+
+
+MatcherLambdaExpr>hasAnyCaptureMatcherLambdaCapture> InnerMatcher
+Matches any capture in a lambda expression.
+
+Given
+  void foo() {
+int t = 5;
+auto f = [=](){ return t; };
+  }
+lambdaExpr(hasAnyCapture(lambdaCapture())) and
+lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("t")
+  both match `[=](){ return t; }`.
+
+
+
 MatcherMemberExpr>hasDeclarationMatcherDecl>  InnerMatcher
 Matches a node if the declaration associated with that node
 matches the given matcher.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114621: [clangd] Show parameters for construct.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks LGTM!
let me know if I should land this for you.




Comment at: clang-tools-extra/clangd/Hover.cpp:1054
   // editor, as they might be long.
-  if (ReturnType) {
+  if (ReturnType)
 // For functions we display signature in a list form, e.g.:

can you add braces around here?



Comment at: clang-tools-extra/clangd/Hover.cpp:1074
+  if (Type && !ReturnType && !Parameters)
+// Don't print Type after Parameters or ReturnType
+Output.addParagraph().appendText("Type: ").appendCode(*Type);

can you move the comment above the if and also include `as this will just 
duplicate the information`.



Comment at: clang-tools-extra/clangd/Hover.cpp:1065
 
+  if (Parameters && !Parameters->empty()) {
+Output.addParagraph().appendText("Parameters: ");

lh123 wrote:
> kadircet wrote:
> > it's a subtle invariant that we only have parameters for functions (which 
> > has a return type) or constructor/destructor/conversion-operators (which 
> > doesn't have either a type or a return type).
> > 
> > I think we should assert on `Type` not being present here, as otherwise we 
> > would probably duplicate parameters in both places. can you also add a 
> > condition around `!Type` and have a comment saying ` // Don't print 
> > parameters after Type, as they're likely to be mentioned there.`
> We cannot assert here because the `function type` has both `ReturnType`, 
> `Type`, and `Parameters`. I think we only need to not print `Type` when 
> `ReturnType` or `Parameters` are present.
sorry by asserting i meant having it inside the check. this LG, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114621

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

[clang] f4d3cb4 - [HIPSPV] Add CUDA->SPIR-V address space mapping

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2021-12-02T13:34:27Z
New Revision: f4d3cb4ca833d0b165d199e78ed8f1d59a8032eb

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

LOG: [HIPSPV] Add CUDA->SPIR-V address space mapping

Add mapping for CUDA address spaces for HIP to SPIR-V
translation. This change allows HIP device code to be
emitted as valid SPIR-V by mapping unqualified pointers
to generic address space and by mapping __device__ and
__shared__ AS to their equivalent AS in SPIR-V
(CrossWorkgroup and Workgroup, respectively).

Cuda's __constant__ AS is handled specially. In HIP
unqualified pointers (aka "flat" pointers) can point to
__constant__ objects. Mapping this AS to ConstantMemory
would produce to illegal address space casts to
generic AS. Therefore, __constant__ AS is mapped to
CrossWorkgroup.

Patch by linjamaki (Henry Linjamäki)!

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

Added: 
clang/test/CodeGenHIP/hipspv-addr-spaces.cpp

Modified: 
clang/lib/Basic/Targets/SPIR.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index 704b1843dfedb..8cf18b6c20f12 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -56,9 +56,14 @@ static const unsigned SPIRDefIsGenMap[] = {
 0, // opencl_generic
 0, // opencl_global_device
 0, // opencl_global_host
-0, // cuda_device
-0, // cuda_constant
-0, // cuda_shared
+// cuda_* address space mapping is intended for HIPSPV (HIP to SPIR-V
+// translation). This mapping is enabled when the language mode is HIP.
+1, // cuda_device
+// cuda_constant pointer can be casted to default/"flat" pointer, but in
+// SPIR-V casts between constant and generic pointers are not allowed. For
+// this reason cuda_constant is mapped to SPIR-V CrossWorkgroup.
+1, // cuda_constant
+3, // cuda_shared
 1, // sycl_global
 5, // sycl_global_device
 6, // sycl_global_host
@@ -74,6 +79,8 @@ class LLVM_LIBRARY_VISIBILITY BaseSPIRTargetInfo : public 
TargetInfo {
 protected:
   BaseSPIRTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
+assert((Triple.isSPIR() || Triple.isSPIRV()) &&
+   "Invalid architecture for SPIR or SPIR-V.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR(-V) target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -137,11 +144,16 @@ class LLVM_LIBRARY_VISIBILITY BaseSPIRTargetInfo : public 
TargetInfo {
 // FIXME: SYCL specification considers unannotated pointers and references
 // to be pointing to the generic address space. See section 5.9.3 of
 // SYCL 2020 specification.
-// Currently, there is no way of representing SYCL's default address space
-// language semantic along with the semantics of embedded C's default
-// address space in the same address space map. Hence the map needs to be
-// reset to allow mapping to the desired value of 'Default' entry for SYCL.
-setAddressSpaceMap(/*DefaultIsGeneric=*/Opts.SYCLIsDevice);
+// Currently, there is no way of representing SYCL's and HIP's default
+// address space language semantic along with the semantics of embedded C's
+// default address space in the same address space map. Hence the map needs
+// to be reset to allow mapping to the desired value of 'Default' entry for
+// SYCL and HIP.
+setAddressSpaceMap(
+/*DefaultIsGeneric=*/Opts.SYCLIsDevice ||
+// The address mapping from HIP language for device code is only 
defined
+// for SPIR-V.
+(getTriple().isSPIRV() && Opts.HIP && Opts.CUDAIsDevice));
   }
 
   void setSupportedOpenCLOpts() override {
@@ -159,6 +171,7 @@ class LLVM_LIBRARY_VISIBILITY SPIRTargetInfo : public 
BaseSPIRTargetInfo {
 public:
   SPIRTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
   : BaseSPIRTargetInfo(Triple, Opts) {
+assert(Triple.isSPIR() && "Invalid architecture for SPIR.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -177,6 +190,8 @@ class LLVM_LIBRARY_VISIBILITY SPIR32TargetInfo : public 
SPIRTargetInfo {
 public:
   SPIR32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
   : SPIRTargetInfo(Triple, Opts) {
+assert(Triple.getArch() == llvm::Triple::spir &&
+   "Invalid architecture for 32-bit SPIR.");
 PointerWidth = PointerAlign = 32;
 SizeType = TargetInfo::UnsignedInt;
 PtrDiffType = IntPtrType = TargetInfo::SignedInt;
@@ -192,6 +207,8 @@ class LLVM_LIBRARY_VISIBILITY SPI

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4d3cb4ca833: [HIPSPV] Add CUDA->SPIR-V address space 
mapping (authored by Anastasia).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

Files:
  clang/lib/Basic/Targets/SPIR.h
  clang/test/CodeGenHIP/hipspv-addr-spaces.cpp

Index: clang/test/CodeGenHIP/hipspv-addr-spaces.cpp
===
--- /dev/null
+++ clang/test/CodeGenHIP/hipspv-addr-spaces.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple spirv64 -x hip -emit-llvm -fcuda-is-device \
+// RUN:   -o - %s | FileCheck %s
+
+#define __device__ __attribute__((device))
+#define __shared__ __attribute__((shared))
+#define __constant__ __attribute__((constant))
+
+// CHECK: %struct.foo_t = type { i32, i32 addrspace(4)* }
+
+// CHECK: @d ={{.*}} addrspace(1) externally_initialized global
+__device__ int d;
+
+// CHECK: @c ={{.*}} addrspace(1) externally_initialized global
+__constant__ int c;
+
+// CHECK: @s ={{.*}} addrspace(3) global
+__shared__ int s;
+
+// CHECK: @foo ={{.*}} addrspace(1) externally_initialized global %struct.foo_t
+__device__ struct foo_t {
+  int i;
+  int* pi;
+} foo;
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z3barPi(i32 addrspace(4)*
+__device__ int* bar(int *x) {
+  return x;
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_dv()
+__device__ int* baz_d() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @d to i32 addrspace(4)*
+  return &d;
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_cv()
+__device__ int* baz_c() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @c to i32 addrspace(4)*
+  return &c;
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_sv()
+__device__ int* baz_s() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(3)* @s to i32 addrspace(4)*
+  return &s;
+}
Index: clang/lib/Basic/Targets/SPIR.h
===
--- clang/lib/Basic/Targets/SPIR.h
+++ clang/lib/Basic/Targets/SPIR.h
@@ -56,9 +56,14 @@
 0, // opencl_generic
 0, // opencl_global_device
 0, // opencl_global_host
-0, // cuda_device
-0, // cuda_constant
-0, // cuda_shared
+// cuda_* address space mapping is intended for HIPSPV (HIP to SPIR-V
+// translation). This mapping is enabled when the language mode is HIP.
+1, // cuda_device
+// cuda_constant pointer can be casted to default/"flat" pointer, but in
+// SPIR-V casts between constant and generic pointers are not allowed. For
+// this reason cuda_constant is mapped to SPIR-V CrossWorkgroup.
+1, // cuda_constant
+3, // cuda_shared
 1, // sycl_global
 5, // sycl_global_device
 6, // sycl_global_host
@@ -74,6 +79,8 @@
 protected:
   BaseSPIRTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
+assert((Triple.isSPIR() || Triple.isSPIRV()) &&
+   "Invalid architecture for SPIR or SPIR-V.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR(-V) target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -137,11 +144,16 @@
 // FIXME: SYCL specification considers unannotated pointers and references
 // to be pointing to the generic address space. See section 5.9.3 of
 // SYCL 2020 specification.
-// Currently, there is no way of representing SYCL's default address space
-// language semantic along with the semantics of embedded C's default
-// address space in the same address space map. Hence the map needs to be
-// reset to allow mapping to the desired value of 'Default' entry for SYCL.
-setAddressSpaceMap(/*DefaultIsGeneric=*/Opts.SYCLIsDevice);
+// Currently, there is no way of representing SYCL's and HIP's default
+// address space language semantic along with the semantics of embedded C's
+// default address space in the same address space map. Hence the map needs
+// to be reset to allow mapping to the desired value of 'Default' entry for
+// SYCL and HIP.
+setAddressSpaceMap(
+/*DefaultIsGeneric=*/Opts.SYCLIsDevice ||
+// The address mapping from HIP language for device code is only defined
+// for SPIR-V.
+(getTriple().isSPIRV() && Opts.HIP && Opts.CUDAIsDevice));
   }
 
   void setSupportedOpenCLOpts() override {
@@ -159,6 +171,7 @@
 public:
   SPIRTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
   : BaseSPIRTargetInfo(Triple, Opts) {
+assert(Triple.isSPIR() && "Invalid architecture for SPIR.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -177,6 +1

[PATCH] D114809: Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114809#3166602 , @ymandel wrote:

> In D114809#3165214 , @aaron.ballman 
> wrote:
>
>> LGTM! I had no idea...
>>
>> (As a follow-up question, should we be removing the `using` declarations 
>> starting around line 141 or so? Many of them are not used, and if the 
>> documentation dumper can't look through typedefs and gets the documentation 
>> wrong, perhaps we want to discourage using them entirely?)
>
> Aaron -- at least in our codebase, those are used a fair amount. What about 
> updating the script to understand those typedefs?

That would also be an effectively solution! I was assuming it would be 
difficult to update the script.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114809

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

[PATCH] D110155: [OpenCL] Allow optional __generic in __remove_address_space utility

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks


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

https://reviews.llvm.org/D110155

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

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This makes IncludeCleaner more useful in the presense of a large number of
forward declarations. If the definition is already in the Translation Unit and
visible to the Main File, forward declarations have no effect.

The original patch D112707  was split in two: 
D114864  and this one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114949

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,13 +51,20 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
   "Integer x;",
   },
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
+  "namespace ns { struct X; struct ^X {}; }",
   "using ns::X;",
   },
   {
@@ -88,8 +95,8 @@
   },
   // Redecls
   {
-  "class ^X; class ^X{}; class ^X;",
-  "X *y;",
+  "void ^foo(); void ^foo() {} void ^foo();",
+  "void bar() { foo(); }",
   },
   // Constructor
   {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -122,13 +122,33 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
-// If we see a declaration in the mainfile, skip all non-definition decls.
-// We only do this for classes because forward declarations are common and
-// we don't want to include every header that forward-declares the symbol
-// because they're often unrelated.
+// Handle RecordDecls separately: classes and structs are often
+// forward-declared and we don't want to include every header that
+// forward-declares the symbol because they're often unrelated. Also, if
+// definition is available, we can safely skip forward declarations because
+// they have no effect. The only exception is symbols that require their
+// original declaration because of the scope specifier, e.g. nested
+// classes:
+//
+// Foo.h:
+// class Foo { class Bar; }
+//
+// Foo.cpp
+// class Foo::Bar {};
+//
+// Here, Foo::Bar definition will require the original declaration inside
+// Foo, but it will be available because we will still mark Foo definition
+// as used.
 if (const auto *RD = llvm::dyn_cast(D)) {
-  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
-if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
+  const auto *Definition = RD->getDefinition();
+  if (Definition) {
+Result.insert(Definition->getLocation());
+return;
+  }
+  // If we see a declaration in the mainfile, skip all non-definition
+  // decls.
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation())) {
+if (Definition)
   Result.insert(Definition->getLocation());
 return;
   }


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't

[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D112707#3166535 , @kbobyrev wrote:

> @sammccall
>
> There is one part of your comment that I couldn't understand, could you 
> please clarify what you mean bi this?
>
>> we can't skip based on a definition (main file or class) that can't stand 
>> alone, usually because of qualifiers. e.g. void foo::bar() {}.
>
> FWIW if we decided to only deal with `RecordDecl` I don't know if your 
> comment refers to them or it's some other context. An example for what you 
> mean would be appreciated!

Yeah I don't think this applies to classes.
The following is valid but you can't drop the (include of) first declaration:

  namespace a { void foo(); }
  void a::foo();

However classes cannot be forward-declared with qualifiers in this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112707

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

[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D112707#3166703 , @sammccall wrote:

> In D112707#3166535 , @kbobyrev 
> wrote:
>
>> @sammccall
>>
>> There is one part of your comment that I couldn't understand, could you 
>> please clarify what you mean bi this?
>>
>>> we can't skip based on a definition (main file or class) that can't stand 
>>> alone, usually because of qualifiers. e.g. void foo::bar() {}.
>>
>> FWIW if we decided to only deal with `RecordDecl` I don't know if your 
>> comment refers to them or it's some other context. An example for what you 
>> mean would be appreciated!
>
> Yeah I don't think this applies to classes.
> The following is valid but you can't drop the (include of) first declaration:
>
>   namespace a { void foo(); }
>   void a::foo();
>
> However classes cannot be forward-declared with qualifiers in this way.

I see, thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112707

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

[PATCH] D113896: [clangd] cleanup of header guard names

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel updated this revision to Diff 391301.
kuhnel marked 4 inline comments as done.
kuhnel added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/ExpectedTypes.h
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/HeaderSourceSwitch.h
  clang-tools-extra/clangd/HeuristicResolver.h
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/InlayHints.h
  clang-tools-extra/clangd/PathMapping.h
  clang-tools-extra/clangd/URI.h
  clang-tools-extra/clangd/index/BackgroundIndexLoader.h
  clang-tools-extra/clangd/index/BackgroundRebuild.h
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/IndexAction.h
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/SymbolLocation.h
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/clangd/index/remote/Client.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/Annotations.h
  clang-tools-extra/clangd/unittests/LSPClient.h
  clang-tools-extra/clangd/unittests/Matchers.h
  clang-tools-extra/clangd/unittests/SyncAPI.h
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestIndex.h
  clang-tools-extra/clangd/unittests/TestScheme.h
  clang-tools-extra/clangd/unittests/TestTU.h
  clang-tools-extra/clangd/unittests/TestWorkspace.h
  clang-tools-extra/clangd/unittests/support/TestTracer.h
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 
 #include "TestTU.h"
 #include "index/Index.h"
Index: clang-tools-extra/clangd/unittests/support/TestTracer.h
===
--- clang-tools-extra/clangd/unittests/support/TestTracer.h
+++ clang-tools-extra/clangd/unittests/support/TestTracer.h
@@ -10,8 +10,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
 
 #include "support/Trace.h"
 #include "llvm/ADT/StringMap.h"
Index: clang-tools-extra/clangd/unittests/TestWorkspace.h
===
--- clang-tools-extra/clangd/unittests/TestWorkspace.h
+++ clang-tools-extra/clangd/unittests/TestWorkspace.h
@@ -13,8 +13,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
 
 #include "TestFS.h"
 #include "TestTU.h"
@@ -56,4 +56,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -14,8 +14,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
@@ -104,4 +104,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
Index: clang-tools-extra/clangd/unittests/TestIndex.h
=

[PATCH] D113896: [clangd] cleanup of header guard names

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

@kadircet looks like clang-tidy does not detect missing comments in `#endif`. 
It does complain about missing comments for namespaces. E.g. in 
`Serialization.h` the `#endif` comment is missing, but without a finding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[PATCH] D114952: [clang-cl] Define _MSVC_LANG for -std=c++2b

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: hans.
thakis requested review of this revision.

This matches the value that msvc v19.29 VS16.11 uses for
_MSVC_LANG with /std:c++latest.


https://reviews.llvm.org/D114952

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/predefined-win-macros.c


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -45,9 +45,14 @@
 // CHECK-MS-NOT: GXX
 
 // RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
-// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2A
-// CHECK-MS-CPP2A: #define _MSC_VER 1900
-// CHECK-MS-CPP2A: #define _MSVC_LANG 202002L
+// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP20
+// CHECK-MS-CPP20: #define _MSC_VER 1900
+// CHECK-MS-CPP20: #define _MSVC_LANG 202002L
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
+// RUN: -fms-compatibility-version=19.00 -std=c++2b -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2B
+// CHECK-MS-CPP2B: #define _MSC_VER 1900
+// CHECK-MS-CPP2B: #define _MSVC_LANG 202004L
 
 // RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -181,7 +181,9 @@
   Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
 
 if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
-  if (Opts.CPlusPlus20)
+  if (Opts.CPlusPlus2b)
+Builder.defineMacro("_MSVC_LANG", "202004L");
+  else if (Opts.CPlusPlus20)
 Builder.defineMacro("_MSVC_LANG", "202002L");
   else if (Opts.CPlusPlus17)
 Builder.defineMacro("_MSVC_LANG", "201703L");


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -45,9 +45,14 @@
 // CHECK-MS-NOT: GXX
 
 // RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions -fms-compatibility \
-// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP2A
-// CHECK-MS-CPP2A: #define _MSC_VER 1900
-// CHECK-MS-CPP2A: #define _MSVC_LANG 202002L
+// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP20
+// CHECK-MS-CPP20: #define _MSC_VER 1900
+// CHECK-MS-CPP20: #define _MSVC_LANG 202002L
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions -fms-compatibility \
+// RUN: -fms-compatibility-version=19.00 -std=c++2b -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP2B
+// CHECK-MS-CPP2B: #define _MSC_VER 1900
+// CHECK-MS-CPP2B: #define _MSVC_LANG 202004L
 
 // RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -181,7 +181,9 @@
   Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
 
 if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
-  if (Opts.CPlusPlus20)
+  if (Opts.CPlusPlus2b)
+Builder.defineMacro("_MSVC_LANG", "202004L");
+  else if (Opts.CPlusPlus20)
 Builder.defineMacro("_MSVC_LANG", "202002L");
   else if (Opts.CPlusPlus17)
 Builder.defineMacro("_MSVC_LANG", "201703L");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114952: [clang-cl] Define _MSVC_LANG for -std=c++2b

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(for reference: https://godbolt.org/z/W6n6jqz4Y)


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

https://reviews.llvm.org/D114952

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

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I don't think there is any time concern with this patch. It's NFC. You should 
retitle accordingly.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:51
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValImpl(ProgramStateRef State, SVal V);
+

What about calling this `simplifySValOnce()`



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1137
+  SVal SimplifiedVal = simplifySValImpl(State, Val);
+  // Do the simplification until we can.
+  while (SimplifiedVal != Val) {

You can remove this comment. Not really useful anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

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

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D114887#3165415 , @martong wrote:

> In D114887#3165397 , @steakhal 
> wrote:
>
>> Also, please provide the coverage of the new test case (only excercising the 
>> new test case not the whole test file) I really want to make sure that it 
>> will cover the loop as it supposed to.
>>
>> Oh, I just now see that there is no test. Now I'm really courious.
>
> There is a test case in D103317  (with the 
> so many `b`s) which covers the loop. That patch is the dependent patch of the 
> reverted patch.

I'm still curious to see which lines are covered by tests.
Pragmatically, this patch changes some behavior, thus we need to know what 
lines become uncovered due to this change.
I suspect it won't change much, but better be on the safe side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D114713: [AArch64][SVE][NEON] Add NEON-SVE-Bridge intrinsics

[Matt Devereau via Phabricator via cfe-commits]

MattDevereau marked an inline comment as done.
MattDevereau added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1325
   setOperationAction(ISD::MLOAD, VT, Custom);
+  setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom);
 }

paulwalker-arm wrote:
> MattDevereau wrote:
> > paulwalker-arm wrote:
> > > Can you extract this into its own patch as it's really not relevant to 
> > > the rest of the patch and is currently missing tests.  Presumably 
> > > `llvm/test/CodeGen/AArch64/sve-insert-vector.ll` needs updating?
> > i've been adding some tests to assert this block of code. i've got tests 
> > for `insert(vscale x n x bfloat, n x bfloat, idx)` and `insert(vscale x n x 
> > bfloat, vscale x n x bfloat, idx)`.
> > the n = 4 and n = 8 tests are fine, but n = 2 for `insert(vscale x 2 x 
> > bfloat, 2 x bfloat, idx)`  fails an assertion. i've had a quick poke around 
> > but haven't seen an obvious reason why its failing, should I worry about 
> > this and spend more time on it or just submit the tests i've already got 
> > for `4bf16` and `8bf16`?
> Obviously it would be nice for all combinations to work but that's not 
> something you have to fix if it's not directly affecting what you need.
> 
> I've checked and it seems `2 x half` doesn't work out of the box either so it 
> sounds reasonable to me for your new `bfloat` handling to mirror the existing 
> supported `half` use cases only.
Resolved this in 4244f95cc6ce73ab38fbb91929a0888309f3ca8d


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

https://reviews.llvm.org/D114713

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

[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:151
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation())) {
+if (Definition)
   Result.insert(Definition->getLocation());

if we made it here, we already know definition wasn't visible in the current TU.

maybe just something like:

```
// Special case RecordDecls, as it is common for them to be forward declared 
multiple times.
// The most common two cases are:
// - definition available in TU, only mark that one as usage. rest is likely to 
be unnecessary. this might result in false positives when an internal 
definition is visible.
// - there's a forward declaration in the main file, no need for other redecls.
if (auto *RD = ...) {
  if (auto *Def) {
insert that;
return;
  }
  if (mostRecentInMainFile) {
return;
  }
}
```

If we really want to keep a note around nested decls, we might say something 
like:
`Forward decls/definitions of nested recorddecls can only be nested inside 
other recorddecls, and the definition for outer recorddecl has to be visible, 
and we always preserve definitions.`

But I don't think all the additional information provides much value, rather 
creates confusion and forces one to think more about the details.



Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:67
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
+  "namespace ns { struct X; struct ^X {}; }",
   "using ns::X;",

you mind changing these into functions as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Jay Foad via Phabricator via cfe-commits]

foad created this revision.
foad added reviewers: arsenm, rampitec, critson, yaxunl, b-sumner.
Herald added subscribers: kerbowa, hiraditya, t-tye, tpr, dstuttard, nhaehnle, 
jvesely, kzhuravl.
foad requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wdng.
Herald added projects: clang, LLVM.

The ray_origin, ray_dir and ray_inv_dir arguments should all be vec3 to
match how the hardware instruction works.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114957

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll

Index: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
===
--- llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
+++ llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
@@ -3,15 +3,15 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX1030 %s
 ; RUN: not --crash llc -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefix=ERR %s
 
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(uint node_ptr, float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(uint node_ptr, float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(ulong node_ptr, float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(ulong node_ptr, float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(uint node_ptr, float ray_extent, float3 ray_origin, float3 ray_dir, float3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(uint node_ptr, float ray_extent, float3 ray_origin, half3 ray_dir, half3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(ulong node_ptr, float ray_extent, float3 ray_origin, float3 ray_dir, float3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(ulong node_ptr, float ray_extent, float3 ray_origin, half3 ray_dir, half3 ray_inv_dir, uint4 texture_descr)
 
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32, float, <4 x float>, <4 x float>, <4 x float>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(i32, float, <4 x float>, <4 x half>, <4 x half>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(i64, float, <4 x float>, <4 x float>, <4 x float>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(i64, float, <4 x float>, <4 x half>, <4 x half>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32, float, <3 x float>, <3 x float>, <3 x float>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(i32, float, <3 x float>, <3 x half>, <3 x half>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(i64, float, <3 x float>, <3 x float>, <3 x float>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(i64, float, <3 x float>, <3 x half>, <3 x half>, <4 x i32>)
 
 ; ERR: in function image_bvh_intersect_ray{{.*}}intrinsic not supported on subtarget
 ; Arguments are flattened to represent the actual VGPR_A layout, so we have no
@@ -23,43 +23,43 @@
 ; GCN-NEXT:s_waitcnt vmcnt(0)
 ; GCN-NEXT:; return to shader part epilog
 main_body:
-  %ray_origin0 = insertelement <4 x float> undef, float %ray_origin_x, i32 0
-  %ray_origin1 = insertelement <4 x float> %ray_origin0, float %ray_origin_y, i32 1
-  %ray_origin = insertelement <4 x float> %ray_origin1, float %ray_origin_z, i32 2
-  %ray_dir0 = insertelement <4 x float> undef, float %ray_dir_x, i32 0
-  %ray_dir1 = insertelement <4 x float> %ray_dir0, float %ray_dir_y, i32 1
-  %ray_dir = insertelement <4 x float> %ray_dir1, float %ray_dir_z, i32 2
-  %ray_inv_dir0 = insertelement <4 x float> undef, float %ray_inv_dir_x, i32 0
-  %ray_inv_dir1 = insertelement <4 x float> %ray_inv_dir0, float %ray_inv_dir_y, i32 1
-  %ray_inv_dir = insertelement <4 x float> %ray_inv_dir1, float %ray_inv_dir_z, i32 2
-  %v = call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32 %node_ptr, float %ray_extent, <4 x float> %ray_origin, <4 x float> %ray_dir, <4 x float> %ray_inv_dir, <4 x i32> %tdescr)
+  %ray_origin0 = insertelement <3 x float> undef, float %ray_origin_x, i32 0
+  %ray_or

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

This is a flag-day change to the signatures of the LLVM intrinsics and the 
OpenCL builtins. Is that OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114952: [clang-cl] Define _MSVC_LANG for -std=c++2b

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D114952

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Greg Rodgers via Phabricator via cfe-commits]

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

this is ok as is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[clang-tools-extra] c006ea6 - [clang-tidy] Fix build broken by commit 6a9487df73e917c4faf5e060f2bb33c6ade3f967 (D113148)

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-12-02T14:41:00Z
New Revision: c006ea6bde234153abadc8927557da9b8e0dc029

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

LOG: [clang-tidy] Fix build broken by commit 
6a9487df73e917c4faf5e060f2bb33c6ade3f967 (D113148)

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index c63b16131fac7..d1dbaec8358ad 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -83,4 +83,5 @@ clang_target_link_libraries(clangTidyBugproneModule
   clangBasic
   clangLex
   clangTooling
+  clangTransformer
   )



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

[PATCH] D114446: [clang] Fix wrong -Wunused-local-typedef warning if use is a dependent qialified identifier

[Kristina Bessonova via Phabricator via cfe-commits]

krisb updated this revision to Diff 391312.
krisb added a comment.

Simplified diagnostic condition in 
TemplateDeclInstantiator::InstantiateTypedefNameDecl().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114446

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T &item) {
+   using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+   item.a_type_t::f();
+}
+
+void foo() {
+   handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,10 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs.
+  if (!Typedef->isInvalidDecl() && (!RD || RD->isLocalClass()))
+SemaRef.DiagnoseUnusedDecl(Typedef);
+
   return Typedef;
 }
 
@@ -1907,8 +1911,6 @@
 LocalInstantiations.perform();
   }
 
-  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
-
   if (IsInjectedClassName)
 assert(Record->isInjectedClassName() && "Broken injected-class-name");
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1794,16 +1794,19 @@
 
   // Except for labels, we only care about unused decls that are local to
   // functions.
-  bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
-  if (const auto *R = dyn_cast(D->getDeclContext()))
-// For dependent types, the diagnostic is deferred.
-WithinFunction =
-WithinFunction || (R->isLocalClass() && !R->isDependentType());
+  auto *Context = D->getDeclContext();
+  bool WithinFunction = Context->isFunctionOrMethod();
+  if (const auto *R = dyn_cast(Context))
+WithinFunction = WithinFunction || R->isLocalClass();
   if (!WithinFunction)
 return false;
 
-  if (isa(D))
+  if (const auto *TD = dyn_cast(D)) {
+// Defer warnings for typedefs within a dependent context.
+if (Context->isDependentContext())
+  return false;
 return true;
+  }
 
   // White-list anything that isn't a local variable.
   if (!isa(D) || isa(D) || isa(D))


Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -255,5 +255,20 @@
 }
 } // TypedefInLocalClassOfTemplateClassMember
 
+namespace TypedefInDependentQualifiedIdentifier {
+struct A { void f() const {} };
+
+template 
+void handler(const T &item) {
+	using a_type_t = A; // no-diag
+  using b_type_t = A; // expected-warning {{unused type alias 'b_type_t'}}
+	item.a_type_t::f();
+}
+
+void foo() {
+	handler(A());
+}
+} // TypedefInDependentQualifiedIdentifier
+
 // This should not disable any warnings:
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -955,6 +955,10 @@
   Typedef->setAccess(D->getAccess());
   Typedef->setReferenced(D->isReferenced());
 
+  // Diagnose unused local typedefs.
+  if (!Typedef->isInvalidDecl() && (!RD || RD->isLocalClass()))
+SemaRef.DiagnoseUnusedDecl(Typedef);
+
   return Typedef;
 }
 
@@ -1907,8 +1911,6 @@
 LocalInstantiations.perform();
   }
 
-  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
-
   if (IsInjectedClassName)
 assert(Record->isInjectedClassName() && "Broken injected-class-name");
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1794,16 +1794,19 @@
 
   // Except for labels, we only care about unused decls that are local to
   // functions.
-  bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
-  if (const auto *R = dyn_cast(D->getDeclContext()))
-// For dependent types, the diagnostic is deferred.
-WithinFunction =
-Wit

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D114957#3166817 , @foad wrote:

> This is a flag-day change to the signatures of the LLVM intrinsics and the 
> OpenCL builtins. Is that OK?

This breaks users' code. If we have to do this, at least let clang emit a 
pre-defined macro e.g. __amdgcn_bvh_use_vec3__=1 so that users can make their 
code work before and after the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D114957#3166858 , @yaxunl wrote:

> In D114957#3166817 , @foad wrote:
>
>> This is a flag-day change to the signatures of the LLVM intrinsics and the 
>> OpenCL builtins. Is that OK?
>
> This breaks users' code. If we have to do this, at least let clang emit a 
> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
> their code work before and after the change.

I do not think it's worth introducing a macro for this. Are there actually C 
users of these builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 391316.
junaire added a comment.

I didn't know if I understand Aaron's words right, but I think I can update the 
patch first...


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

https://reviews.llvm.org/D114688

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -36,3 +36,10 @@
   static_assert(!is_const::value);
   static_assert(!is_const::value);
 }
+
+void test_builtin_elementwise_ceil() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -135,3 +135,24 @@
   c1 = __builtin_elementwise_min(c1, c2);
   // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was '_Complex float')}}
 }
+
+void test_builtin_elementwise_ceil(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_ceil(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_ceil();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_ceil(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_ceil(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_ceil(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_ceil(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
Index: clang/test/CodeGen/builtins-elementwise-math.c
===
--- clang/test/CodeGen/builtins-elementwise-math.c
+++ clang/test/CodeGen/builtins-elementwise-math.c
@@ -189,3 +189,20 @@
   // CHECK-NEXT: call i32 @llvm.smin.i32(i32 [[IAS1]], i32 [[B]])
   int_as_one = __builtin_elementwise_min(int_as_one, b);
 }
+
+void test_builtin_elementwise_ceil(float f1, float f2, double d1, double d2,
+   float4 vf1, float4 vf2, si8 vi1, si8 vi2,
+   long long int i1, long long int i2, short si) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_ceil(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.ceil.f32(float [[F1]])
+  f2 = __builtin_elementwise_ceil(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.ceil.f64(double [[D1]])
+  d2 = __builtin_elementwise_ceil(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_ceil(vf1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2098,10 +2098,54 @@
 break;
   }
 
-  case Builtin::BI__builtin_elementwise_abs:
-if (SemaBuiltinElementwiseMathOneArg(TheCall))
+  // __builtin_elementwise_abs estricts the element type to signed integers or
+  // floating point types only.
+  case Builtin::BI__builtin_elementwise_abs: {
+if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
+  return ExprError();
+
+auto CheckArgType = [this](CallExpr *TheCall) -> bool {
+  QualType ArgTy = TheCall->getArg(0)->getType();
+  SourceLocation ArgLoc = TheCall->getArg(0)->getBeginLoc();
+
+  QualType EltTy = ArgTy;
+  if (auto *VecTy = EltTy->getAs())
+EltTy = VecTy->getElementType();
+  if (EltTy->isUnsignedIntegerType())
+return Diag(ArgLoc, diag::err_builtin_invalid_arg_type)
+   << 1 << /* signed integer or float ty*/ 3 << ArgTy;
+  return false;
+};
+
+if (CheckArgType(TheCall))
+  return ExprError();
+break;
+  }
+
+  // __builtin_elementwise_abs estricts the element type to floating point
+  // types only.
+  case Builtin::BI__builtin_elementwise_ceil: {
+if (PrepareBuiltinElementwiseMathOneArgCal

[PATCH] D113638: [xray] Add support for hexagon architecture

[Sid Manning via Phabricator via cfe-commits]

sidneym added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113638

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

[PATCH] D111566: [SYCL] Fix function pointer address space

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

Thanks for the reviews @dylanmckay and @rjmccall ! I agree that moving the 
logic for functions pointers to `getTargetAddressSpace`  makes sense. However, 
I'm not sure what the consequences are, since that increases the impact of this 
change quite a bit. I'm not sure if I will have the time to deal with any 
issues that arise before I go on vacation for Christmas. I'll take a quick look 
sometime this week, and hopefully its a simple enough change. If not, I can do 
it in a follow-up PR as suggested above, unless someone objects.


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

https://reviews.llvm.org/D111566

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

[PATCH] D64008: [RISCV] Avoid save-restore target feature warning

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, frasercrmck, evandro, luismarques, sameer.abuasal, s.egerton, MaskRay.

For historical documentation purposes:

I don't think this ever actually fully worked. Whilst it stops passing the 
feature explicitly here, and thus means no target feature is passed when you 
don't specify either driver option, -m(no-)save-restore is still in 
m_riscv_Features_Group and so the later call to handleTargetFeaturesGroup still 
automatically adds [+-]save-restore if you specify one of the options on the 
command line, meaning -mno-save-restore still warned in Clang 9 and 10 despite 
having this patch. Clang 11 stopped warning as the feature was finally 
implemented by then.

https://godbolt.org/z/Gqv3GG6n4


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64008

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D114957#3166861 , @arsenm wrote:

> In D114957#3166858 , @yaxunl wrote:
>
>> In D114957#3166817 , @foad wrote:
>>
>>> This is a flag-day change to the signatures of the LLVM intrinsics and the 
>>> OpenCL builtins. Is that OK?
>>
>> This breaks users' code. If we have to do this, at least let clang emit a 
>> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
>> their code work before and after the change.
>
> I do not think it's worth introducing a macro for this. Are there actually C 
> users of these builtins?

Yes we have users who use these clang builtins. We have received quite a few 
complaints about making breaking API changes without a way to detect them in 
the program.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D112410: [SPIR-V] Add a toolchain for SPIR-V in clang

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh requested changes to this revision.
svenvh added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Driver/Driver.cpp:3728
 
+  // Linking separate translation units for SPIR-V is not supported yet.
+  // It can be done either by LLVM IR linking before conversion of the final

FIXME? (assuming this is something we want to address eventually)



Comment at: clang/test/Driver/spirv-toolchain.cl:10
+// SPV64-SAME: "-o" [[BC:".*bc"]]
+// SPV64: {{".*llvm-spirv.*"}} [[BC]] "-o" {{".*o"}}
+

Any reason to not just check for `llvm-spirv{{.*}}`, for consistency with the 
clang check above?



Comment at: clang/test/Misc/warning-flags.c:21
 
-CHECK: Warnings without flags (67):
+CHECK: Warnings without flags (68):
 

The comment above says: "The list of warnings below should NEVER grow.", and 
the current patch violates that.  You'll need to add a warning group to the new 
warning.


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

https://reviews.llvm.org/D112410

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

In D114957#3166882 , @yaxunl wrote:

> In D114957#3166861 , @arsenm wrote:
>
>> In D114957#3166858 , @yaxunl wrote:
>>
>>> In D114957#3166817 , @foad wrote:
>>>
 This is a flag-day change to the signatures of the LLVM intrinsics and the 
 OpenCL builtins. Is that OK?
>>>
>>> This breaks users' code. If we have to do this, at least let clang emit a 
>>> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
>>> their code work before and after the change.
>>
>> I do not think it's worth introducing a macro for this. Are there actually C 
>> users of these builtins?
>
> Yes we have users who use these clang builtins. We have received quite a few 
> complaints about making breaking API changes without a way to detect them in 
> the program.

But builtins are not part of the documented API and we have advised developers 
using them that they are subject to change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

In D114957#3166858 , @yaxunl wrote:

> In D114957#3166817 , @foad wrote:
>
>> This is a flag-day change to the signatures of the LLVM intrinsics and the 
>> OpenCL builtins. Is that OK?
>
> This breaks users' code. If we have to do this, at least let clang emit a 
> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
> their code work before and after the change.

I don't know anything about OpenCL macros. Is it good enough to put this in 
`AMDGPUTargetInfo::getTargetDefines`:

  if (Opts.OpenCL)
Builder.defineMacro("__amdgcn_bvh_use_vec3__");

Does it need tests, documentation, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

In D114957#3166936 , @foad wrote:

> In D114957#3166858 , @yaxunl wrote:
>
>> In D114957#3166817 , @foad wrote:
>>
>>> This is a flag-day change to the signatures of the LLVM intrinsics and the 
>>> OpenCL builtins. Is that OK?
>>
>> This breaks users' code. If we have to do this, at least let clang emit a 
>> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
>> their code work before and after the change.
>
> I don't know anything about OpenCL macros. Is it good enough to put this in 
> `AMDGPUTargetInfo::getTargetDefines`:
>
>   if (Opts.OpenCL)
> Builder.defineMacro("__amdgcn_bvh_use_vec3__");
>
> Does it need tests, documentation, etc?

But how long would that be carried?  And then deprecated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114885: Use VersionTuple for parsing versions in Triple, fixing issues that caused the original change to be reverted. This makes it possible to distinguish between "16" and "16.0" after pars

[James Farrell via Phabricator via cfe-commits]

jamesfarrell updated this revision to Diff 391323.
jamesfarrell added a comment.

Don't use the preprocessor to hide test cases, so we can at least tell if it 
compiles when testing on platforms other than Apple and AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114885

Files:
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Sema/attr-availability-android.c
  clang/test/Sema/attr-availability.c
  clang/test/Sema/availability-guard-format.mm
  clang/test/SemaObjC/attr-availability.m
  clang/test/SemaObjC/property-deprecated-warning.m
  clang/test/SemaObjC/unguarded-availability-maccatalyst.m
  clang/test/SemaObjC/unguarded-availability.m
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/unittests/ADT/TripleTest.cpp
  llvm/unittests/Support/Host.cpp

Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -366,7 +366,6 @@
   }
 }
 
-#if defined(__APPLE__) || defined(_AIX)
 static bool runAndGetCommandOutput(
 const char *ExePath, ArrayRef argv,
 std::unique_ptr &Buffer, off_t &Size) {
@@ -406,12 +405,9 @@
   // disabled.
   (void) runAndGetCommandOutput;
 }
-#endif
 
-#if defined(__APPLE__)
 TEST_F(HostTest, getMacOSHostVersion) {
-  using namespace llvm::sys;
-  llvm::Triple HostTriple(getProcessTriple());
+  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 return;
 
@@ -420,34 +416,32 @@
   std::unique_ptr Buffer;
   off_t Size;
   ASSERT_EQ(runAndGetCommandOutput(SwVersPath, argv, Buffer, Size), true);
-  StringRef SystemVersion(Buffer.get(), Size);
+  StringRef SystemVersionStr(Buffer.get(), Size);
 
   // Ensure that the two versions match.
-  unsigned SystemMajor, SystemMinor, SystemMicro;
-  ASSERT_EQ(llvm::Triple((Twine("x86_64-apple-macos") + SystemVersion))
-.getMacOSXVersion(SystemMajor, SystemMinor, SystemMicro),
+  VersionTuple SystemVersion;
+  ASSERT_EQ(llvm::Triple((Twine("x86_64-apple-macos") + SystemVersionStr))
+.getMacOSXVersion(SystemVersion),
 true);
-  unsigned HostMajor, HostMinor, HostMicro;
-  ASSERT_EQ(HostTriple.getMacOSXVersion(HostMajor, HostMinor, HostMicro), true);
+  VersionTuple HostVersion;
+  ASSERT_EQ(HostTriple.getMacOSXVersion(HostVersion), true);
 
-  if (SystemMajor > 10) {
+  if (SystemVersion.getMajor() > 10) {
 // Don't compare the 'Minor' and 'Micro' versions, as they're always '0' for
 // the 'Darwin' triples on 11.x.
-ASSERT_EQ(SystemMajor, HostMajor);
+ASSERT_EQ(SystemVersion.getMajor(), HostVersion.getMajor());
   } else {
 // Don't compare the 'Micro' version, as it's always '0' for the 'Darwin'
 // triples.
-ASSERT_EQ(std::tie(SystemMajor, SystemMinor), std::tie(HostMajor, HostMinor));
+ASSERT_EQ(SystemVersion.getMajor(), HostVersion.getMajor());
+ASSERT_EQ(SystemVersion.getMinor(), HostVersion.getMinor());
   }
 }
-#endif
 
-#if defined(_AIX)
 TEST_F(HostTest, AIXVersionDetect) {
-  using namespace llvm::sys;
-
-  llvm::Triple HostTriple(getProcessTriple());
-  ASSERT_EQ(HostTriple.getOS(), Triple::AIX);
+  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
+  if (HostTriple.getOS() != Triple::AIX)
+return;
 
   llvm::Triple ConfiguredHostTriple(LLVM_HOST_TRIPLE);
   ASSERT_EQ(ConfiguredHostTriple.getOS(), Triple::AIX);
@@ -457,22 +451,21 @@
   std::unique_ptr Buffer;
   off_t Size;
   ASSERT_EQ(runAndGetCommandOutput(ExePath, argv, Buffer, Size), true);
-  StringRef SystemVersion(Buffer.get(), Size);
+  StringRef SystemVersionStr(Buffer.get(), Size);
 
-  unsigned SystemMajor, SystemMinor, SystemMicro;
-  llvm::Triple((Twine("powerpc-ibm-aix") + SystemVersion))
-  .getOSVersion(SystemMajor, SystemMinor, SystemMicro);
+  VersionTuple SystemVersion =
+  llvm::Triple((Twine("powerpc-ibm-aix") + SystemVersionStr))
+  .getOSVersion();
 
   // Ensure that the host triple version (major) and release (minor) numbers,
   // unless explicitly configured, match with those of the current system.
   if (!ConfiguredHostTriple.getOSMajorVersion()) {
-unsigned HostMajor, HostMinor, HostMicro;
-HostTriple.getOSVersion(HostMajor, HostMinor, HostMicro);
-ASSERT_EQ(std::tie(SystemMajor, SystemMinor),
-  std::tie(HostMajor, HostMinor));
+VersionTuple HostVersion = HostTriple.getOSVersion();
+ASSERT_E

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

In D114957#3166948 , @b-sumner wrote:

> In D114957#3166936 , @foad wrote:
>
>> In D114957#3166858 , @yaxunl wrote:
>>
>>> In D114957#3166817 , @foad wrote:
>>>
 This is a flag-day change to the signatures of the LLVM intrinsics and the 
 OpenCL builtins. Is that OK?
>>>
>>> This breaks users' code. If we have to do this, at least let clang emit a 
>>> pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
>>> their code work before and after the change.
>>
>> I don't know anything about OpenCL macros. Is it good enough to put this in 
>> `AMDGPUTargetInfo::getTargetDefines`:
>>
>>   if (Opts.OpenCL)
>> Builder.defineMacro("__amdgcn_bvh_use_vec3__");
>>
>> Does it need tests, documentation, etc?
>
> But how long would that be carried?  And then deprecated?

Then do you think the patch is OK as-is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In D114413#3160044 , @peixin wrote:

> In D114413#3159095 , @Meinersbur 
> wrote:
>
>> In D114413#3154936 , @peixin wrote:
>>
>>> 
>
> `getPreheader()` was in `OMPIRBuilder.cpp` before you rebase in your last 
> update here. That's why I let you rebase since I failed to git apply your 
> last patch in main branch. It's not important now. Please forget about that.

D114368  (which this patch depends on) moves 
`getPreheder()` to the `.cpp` files (because it has become more than a simple 
getter)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[clang] 89d645d - [Clang] Fix LTO pipeline test after 770a50b28c00211f9a.

[Florian Hahn via cfe-commits]

Author: Florian Hahn
Date: 2021-12-02T15:42:52Z
New Revision: 89d645dd3a60cd5bb3cc9a78ad17d3b063cc98bf

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

LOG: [Clang] Fix LTO pipeline test after 770a50b28c00211f9a.

Added: 


Modified: 
clang/test/CodeGen/lto-newpm-pipeline.c

Removed: 




diff  --git a/clang/test/CodeGen/lto-newpm-pipeline.c 
b/clang/test/CodeGen/lto-newpm-pipeline.c
index 8119c79425ef7..ff8d921151397 100644
--- a/clang/test/CodeGen/lto-newpm-pipeline.c
+++ b/clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,7 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: AnnotationRemarksPass
 // CHECK-FULL-O0-NEXT: Running pass: VerifierPass
 // CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
@@ -42,6 +43,7 @@
 // CHECK-THIN-O0: Running pass: CoroCleanupPass
 // CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: AnnotationRemarksPass
 // CHECK-THIN-O0-NEXT: Running pass: VerifierPass
 // CHECK-THIN-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass



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

[PATCH] D113896: [clangd] cleanup of header guard names

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Also, please mention the

In D113896#3166732 , @kuhnel wrote:

> @kadircet looks like clang-tidy does not detect missing comments in `#endif`. 
> It does complain about missing comments for namespaces. E.g. in 
> `Serialization.h` the `#endif` comment is missing, but without a finding.

This looks to be a deliberate decision: 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h#L32

Looking at the code under `clang/include`, it looks like most headers don't 
have the `#endif // COMMENT`, so this might be correct. However, the style 
guide is rather vague and doesn't mention this at all: 
https://llvm.org/docs/CodingStandards.html#header-guard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D114957#3166974 , @foad wrote:

> In D114957#3166948 , @b-sumner 
> wrote:
>
>> In D114957#3166936 , @foad wrote:
>>
>>> In D114957#3166858 , @yaxunl 
>>> wrote:
>>>
 In D114957#3166817 , @foad wrote:

> This is a flag-day change to the signatures of the LLVM intrinsics and 
> the OpenCL builtins. Is that OK?

 This breaks users' code. If we have to do this, at least let clang emit a 
 pre-defined macro e.g. `__amdgcn_bvh_use_vec3__`=1 so that users can make 
 their code work before and after the change.
>>>
>>> I don't know anything about OpenCL macros. Is it good enough to put this in 
>>> `AMDGPUTargetInfo::getTargetDefines`:
>>>
>>>   if (Opts.OpenCL)
>>> Builder.defineMacro("__amdgcn_bvh_use_vec3__");
>>>
>>> Does it need tests, documentation, etc?
>>
>> But how long would that be carried?  And then deprecated?
>
> Then do you think the patch is OK as-is?

Let's discuss with the users and see whether the macro is needed or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[clang-tools-extra] 5bd643d - [clangd] cleanup of header guard names

[Christian Kühnel via cfe-commits]

Author: Christian Kühnel
Date: 2021-12-02T15:58:35Z
New Revision: 5bd643d31d11e96bcae833025b241681370e527c

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

LOG: [clangd] cleanup of header guard names

Renaming header guards to match the LLVM convention.
This patch was created by automatically applying the fixes from
clang-tidy.

I've removed the [NFC]  tag from the title, as we're adding header guards in 
some files and thus might trigger behavior changes.

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

Added: 


Modified: 
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/ExpectedTypes.h
clang-tools-extra/clangd/FindTarget.h
clang-tools-extra/clangd/HeaderSourceSwitch.h
clang-tools-extra/clangd/HeuristicResolver.h
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/IncludeFixer.h
clang-tools-extra/clangd/InlayHints.h
clang-tools-extra/clangd/PathMapping.h
clang-tools-extra/clangd/URI.h
clang-tools-extra/clangd/index/BackgroundIndexLoader.h
clang-tools-extra/clangd/index/BackgroundRebuild.h
clang-tools-extra/clangd/index/CanonicalIncludes.h
clang-tools-extra/clangd/index/IndexAction.h
clang-tools-extra/clangd/index/ProjectAware.h
clang-tools-extra/clangd/index/Serialization.h
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/index/SymbolLocation.h
clang-tools-extra/clangd/index/SymbolOrigin.h
clang-tools-extra/clangd/index/dex/Token.h
clang-tools-extra/clangd/index/dex/Trigram.h
clang-tools-extra/clangd/index/remote/Client.h
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/Annotations.h
clang-tools-extra/clangd/unittests/LSPClient.h
clang-tools-extra/clangd/unittests/Matchers.h
clang-tools-extra/clangd/unittests/SyncAPI.h
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestIndex.h
clang-tools-extra/clangd/unittests/TestTU.h
clang-tools-extra/clangd/unittests/TestWorkspace.h
clang-tools-extra/clangd/unittests/support/TestTracer.h
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

Removed: 
clang-tools-extra/clangd/unittests/TestScheme.h



diff  --git a/clang-tools-extra/clangd/CollectMacros.h 
b/clang-tools-extra/clangd/CollectMacros.h
index 2167ebe2e3560..f8df9f9a20cc4 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTMACROS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTMACROS_H
 
 #include "AST.h"
 #include "Protocol.h"
@@ -114,4 +114,4 @@ collectPragmaMarksCallback(const SourceManager &, 
std::vector &Out);
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTEDMACROS_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_COLLECTMACROS_H

diff  --git a/clang-tools-extra/clangd/ExpectedTypes.h 
b/clang-tools-extra/clangd/ExpectedTypes.h
index 36b7cce170234..873b106b91d17 100644
--- a/clang-tools-extra/clangd/ExpectedTypes.h
+++ b/clang-tools-extra/clangd/ExpectedTypes.h
@@ -15,8 +15,8 @@
 // the index. Similar types (such as `int` and `long`) are folded together,
 // forming equivalence classes with the same encoding.
 
//===--===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_EXPECTED_TYPES_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_EXPECTED_TYPES_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_EXPECTEDTYPES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_EXPECTEDTYPES_H
 
 #include "clang/AST/Type.h"
 #include "llvm/ADT/StringRef.h"

diff  --git a/clang-tools-extra/clangd/FindTarget.h 
b/clang-tools-extra/clangd/FindTarget.h
index 3e4cf8b0c9a0a..756fe1d6458f9 100644
--- a/clang-tools-extra/clangd/FindTarget.h
+++ b/clang-tools-extra/clangd/FindTarget.h
@@ -19,8 +19,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_FINDTARGET_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_FINDTARGET_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDTARGET_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDTARGET_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
@@ -220,4 +220,4 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, 
DeclRelationSet);
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_FINDTARGET_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_FINDTARGET_H

diff  --git a/clang-tools-e

[PATCH] D113896: [clangd] cleanup of header guard names

[Christian Kühnel via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5bd643d31d11: [clangd] cleanup of header guard names 
(authored by kuhnel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

Files:
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/ExpectedTypes.h
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/HeaderSourceSwitch.h
  clang-tools-extra/clangd/HeuristicResolver.h
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/IncludeFixer.h
  clang-tools-extra/clangd/InlayHints.h
  clang-tools-extra/clangd/PathMapping.h
  clang-tools-extra/clangd/URI.h
  clang-tools-extra/clangd/index/BackgroundIndexLoader.h
  clang-tools-extra/clangd/index/BackgroundRebuild.h
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/IndexAction.h
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/SymbolLocation.h
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/clangd/index/remote/Client.h
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/unittests/Annotations.h
  clang-tools-extra/clangd/unittests/LSPClient.h
  clang-tools-extra/clangd/unittests/Matchers.h
  clang-tools-extra/clangd/unittests/SyncAPI.h
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/TestIndex.h
  clang-tools-extra/clangd/unittests/TestScheme.h
  clang-tools-extra/clangd/unittests/TestTU.h
  clang-tools-extra/clangd/unittests/TestWorkspace.h
  clang-tools-extra/clangd/unittests/support/TestTracer.h
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TWEAKTESTING_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 
 #include "TestTU.h"
 #include "index/Index.h"
Index: clang-tools-extra/clangd/unittests/support/TestTracer.h
===
--- clang-tools-extra/clangd/unittests/support/TestTracer.h
+++ clang-tools-extra/clangd/unittests/support/TestTracer.h
@@ -10,8 +10,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SUPPORT_TESTTRACER_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_SUPPORT_TESTTRACER_H
 
 #include "support/Trace.h"
 #include "llvm/ADT/StringMap.h"
Index: clang-tools-extra/clangd/unittests/TestWorkspace.h
===
--- clang-tools-extra/clangd/unittests/TestWorkspace.h
+++ clang-tools-extra/clangd/unittests/TestWorkspace.h
@@ -13,8 +13,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
 
 #include "TestFS.h"
 #include "TestTU.h"
@@ -56,4 +56,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTWORKSPACE_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTWORKSPACE_H
Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -14,8 +14,8 @@
 //
 //===-===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
-#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTTU_H
 
 #include "../TidyProvider.h"
 #include "Compiler.h"
@@ -104,4 +104,4 @@
 } // namespace clangd
 } // namespace clang
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
+#endif // LLVM_CLANG_TOO

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114688#3166870 , @junaire wrote:

> I didn't know if I understand Aaron's words right, but I think I can update 
> the patch first...

I think you understood my suggestion pretty well, thanks!




Comment at: clang/lib/Sema/SemaChecking.cpp:2107
+
+auto CheckArgType = [this](CallExpr *TheCall) -> bool {
+  QualType ArgTy = TheCall->getArg(0)->getType();

I don't think using a lambda here (or below) adds a whole lot of value given 
that it's only called once. I think it'd cleaner to hoist the lambda logic into 
the case block directly. WDYT?


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

https://reviews.llvm.org/D114688

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.

The consensus internally seems to be to land this and see what happens on the 
amdgpu buildbot. Go for it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[clang] c99407e - [OpenMP] Make the new device runtime the default

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2021-12-02T11:11:45-05:00
New Revision: c99407e31c3977bf397531bf115a63b0fda68e4d

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

LOG: [OpenMP] Make the new device runtime the default

This patch changes the `-fopenmp-target-new-runtime` option which controls if
the new or old device runtime is used to be true by default.  Disabling this to
use the old runtime now requires using `-fno-openmp-target-new-runtime`.

Reviewed By: JonChesterfield, tianshilei1992, gregrodgers, ronlieb

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/test/Driver/amdgpu-openmp-toolchain.c
clang/test/Driver/openmp-offload-gpu.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 4e6dd20503446..6a1f557968a92 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@ def fno_openmp_assume_teams_oversubscription : 
Flag<["-"], "fno-openmp-assume-te
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",

diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 863e2c597d53a..f282f04b79311 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, false))
+ options::OPT_fno_openmp_target_new_runtime, true))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a7e41b24f2735..8d92c293d83a4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/false))
+   /*Default=*/true))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.

diff  --git a/clang/lib/Driver/ToolChains/Cuda.cpp 
b/clang/lib/Driver/ToolChains/Cuda.cpp
index 5397c7a9a0e66..ee573b89bed13 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -745,7 +745,7 @@ void CudaToolChain::addClangTargetOptions(
 
 std::string BitcodeSuffix;
 if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
-   options::OPT_fno_openmp_target_new_runtime, false))
+   options::OPT_fno_openmp_target_new_runtime, true))
   BitcodeSuffix = "new-nvptx-" + GpuArch.str();
 else
   BitcodeSuffix = "nvptx-" + GpuArch.str();

diff  --git a/clang/test/Driver/amdgpu-openmp-toolchain.c 
b/clang/test/Driver/amdgpu-openmp-toolchain.c
index 78fee12a5a98c..f6b22cd5973a8 100644
--- a/clang/test/Driver/amdgpu-openmp-toolchain.c
+++ b/clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -1,6 +1,6 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa 
-march=gfx906 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -fno-openmp-target-new-runtime 
-Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 
--libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
 
 // verify the tools invocations
@@ -14,7 +14,7 @@
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-o" 
"{{.*}}a-{{.*}}.o" "-x" "ir" "{{.*}}a-{{.*}}.bc"
 // CHECK: ld{{.*}}"-o" "a.out"{{.*}}"{{.*}}amdgpu-openmp-toolchain-{{.*}}.o" 
"{{.*}}a-{{.*}}.o" "-lomp"

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber 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 rGc99407e31c39: [OpenMP] Make the new device runtime the 
default (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c

Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -162,8 +162,8 @@
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
 // RUN:   --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget \
 // RUN:   -Xopenmp-target -march=sm_35 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-BCLIB-DIR %s
+// RUN:   -fopenmp-relocatable-target -fno-openmp-target-new-runtime -save-temps \
+// RUN:   -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-BCLIB-DIR %s
 
 /// Check with the new runtime enabled
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
@@ -185,8 +185,8 @@
 /// Create a bogus bitcode library and find it with LIBRARY_PATH
 // RUN:   env LIBRARY_PATH=%S/Inputs/libomptarget/subdir %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
 // RUN:   -Xopenmp-target -march=sm_35 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-ENV-BCLIB %s
+// RUN:   -fopenmp-relocatable-target -fno-openmp-target-new-runtime -save-temps \
+// RUN:   -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-ENV-BCLIB %s
 
 // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-test.bc
 // CHK-BCLIB-DIR: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget{{/|}}libomptarget-nvptx-sm_35.bc
@@ -204,7 +204,7 @@
 // RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-BCLIB-WARN %s
 
-// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_35.bc' found in the default clang lib directory or in LIBRARY_PATH; use '--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
+// CHK-BCLIB-WARN: no library 'libomptarget-new-nvptx-sm_35.bc' found in the default clang lib directory or in LIBRARY_PATH; use '--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
 
 /// ###
 
Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -1,6 +1,6 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -fno-openmp-target-new-runtime -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
 
 // verify the tools invocations
@@ -14,7 +14,7 @@
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-o" "{{.*}}a-{{.*}}.o" "-x" "ir" "{{.*}}a-{{.*}}.bc"
 // CHECK: ld{{.*}}"-o" "a.out"{{.*}}"{{.*}}amdgpu-openmp-toolchain-{{.*}}.o" "{{.*}}a-{{.*}}.o" "-lomp" "-lomptarget"
 
-// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 %s 2>&1 \
+// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -fno-openmp-target-new-runtime -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PHASES %s
 // phases
 // CHECK-PHASES: 0: input, "{{.*}}amdgpu-openmp-toolchain.c", c, (host-openmp)
@@ -36,13 +36,13 @@
 // CHECK-PHASES: 16: linker, {4, 15}, image, (host-openmp)
 
 // handling of --libomptarget-amdgcn-bc-path
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib/libomptarget-amdgcn-gfx803.bc %s 2>&1 | FileChe

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Fails on CI pretty much like it fails locally
https://lab.llvm.org/buildbot/#/builders/193/builds/2569


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

Looking at the documentation, this looks like a bug in the clang-tidy check:
https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-qualified-auto.html

The rule `llvm-qualified-auto` should not add a `const` qualifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[clang] 53adfa8 - [clang] Do not duplicate "EnableSplitLTOUnit" module flag

[David Greene via cfe-commits]

Author: David Greene
Date: 2021-12-02T08:24:56-08:00
New Revision: 53adfa8750eaf4558c41a50f699616545eb0151b

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

LOG: [clang] Do not duplicate "EnableSplitLTOUnit" module flag

If clang's output is set to bitcode and LTO is enabled, clang would
unconditionally add the flag to the module.  Unfortunately, if the input were a
bitcode or IR file and had the flag set, this would result in two copies of the
flag, which is illegal IR.  Guard the setting of the flag by checking whether it
already exists.  This follows existing practice for the related "ThinLTO" module
flag.

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

Added: 
clang/test/CodeGen/enable-split-lto-unit.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 648c7b3df8ed4..510f3911939cf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1034,8 +1034,9 @@ void 
EmitAssemblyHelper::EmitAssemblyWithLegacyPassManager(
 if (!ThinLinkOS)
   return;
   }
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   CodeGenOpts.EnableSplitLTOUnit);
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ CodeGenOpts.EnableSplitLTOUnit);
   PerModulePasses.add(createWriteThinLTOBitcodePass(
   *OS, ThinLinkOS ? &ThinLinkOS->os() : nullptr));
 } else {
@@ -1049,8 +1050,9 @@ void 
EmitAssemblyHelper::EmitAssemblyWithLegacyPassManager(
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- uint32_t(1));
+if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+   uint32_t(1));
   }
 
   PerModulePasses.add(createBitcodeWriterPass(
@@ -1451,8 +1453,9 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
 if (!ThinLinkOS)
   return;
   }
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   CodeGenOpts.EnableSplitLTOUnit);
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ CodeGenOpts.EnableSplitLTOUnit);
   MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &ThinLinkOS->os()
: nullptr));
 } else {
@@ -1465,8 +1468,9 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- uint32_t(1));
+if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+   uint32_t(1));
   }
   MPM.addPass(
   BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));

diff  --git a/clang/test/CodeGen/enable-split-lto-unit.ll 
b/clang/test/CodeGen/enable-split-lto-unit.ll
new file mode 100644
index 0..2ca09a1ae3a8a
--- /dev/null
+++ b/clang/test/CodeGen/enable-split-lto-unit.ll
@@ -0,0 +1,25 @@
+; Test that we do not duplicate the EnableSplitLTOUnit module flag.
+;
+; Disable the verifier so the compiler doesn't abort and thus lead to empty
+; output and false pass.
+;
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=full 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=FULL-NPM
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=thin 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=THIN-NPM
+; RUN: %clang_cc1 -flegacy-pass-manager -emit-llvm-bc -flto=full 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=FULL-OPM
+; RUN: %clang_cc1 -flegacy-pass-manager -emit-llvm-bc -flto=thin 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=THIN-OPM
+
+define dso_local void @main() local_unnamed_addr {
+entry:
+  ret void
+}
+
+; FULL-NPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !3}
+; FULL-OPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !3}
+; THIN-NPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !4}
+; THIN-OPM-NOT:

[PATCH] D112177: [clang] Do not duplicate "EnableSplitLTOUnit" module flag

[David Greene via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG53adfa8750ea: [clang] Do not duplicate 
"EnableSplitLTOUnit" module flag (authored by greened).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112177

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/enable-split-lto-unit.ll


Index: clang/test/CodeGen/enable-split-lto-unit.ll
===
--- /dev/null
+++ clang/test/CodeGen/enable-split-lto-unit.ll
@@ -0,0 +1,25 @@
+; Test that we do not duplicate the EnableSplitLTOUnit module flag.
+;
+; Disable the verifier so the compiler doesn't abort and thus lead to empty
+; output and false pass.
+;
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=full 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=FULL-NPM
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=thin 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=THIN-NPM
+; RUN: %clang_cc1 -flegacy-pass-manager -emit-llvm-bc -flto=full 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=FULL-OPM
+; RUN: %clang_cc1 -flegacy-pass-manager -emit-llvm-bc -flto=thin 
-disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=THIN-OPM
+
+define dso_local void @main() local_unnamed_addr {
+entry:
+  ret void
+}
+
+; FULL-NPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !3}
+; FULL-OPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !3}
+; THIN-NPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !4}
+; THIN-OPM-NOT: !llvm.module.flags = !{!0, !1, !2, !3, !4}
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 1, !"wchar_size", i32 2}
+!1 = !{i32 7, !"frame-pointer", i32 2}
+!2 = !{i32 1, !"ThinLTO", i32 0}
+!3 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1034,8 +1034,9 @@
 if (!ThinLinkOS)
   return;
   }
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   CodeGenOpts.EnableSplitLTOUnit);
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ CodeGenOpts.EnableSplitLTOUnit);
   PerModulePasses.add(createWriteThinLTOBitcodePass(
   *OS, ThinLinkOS ? &ThinLinkOS->os() : nullptr));
 } else {
@@ -1049,8 +1050,9 @@
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- uint32_t(1));
+if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+   uint32_t(1));
   }
 
   PerModulePasses.add(createBitcodeWriterPass(
@@ -1451,8 +1453,9 @@
 if (!ThinLinkOS)
   return;
   }
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   CodeGenOpts.EnableSplitLTOUnit);
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ CodeGenOpts.EnableSplitLTOUnit);
   MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &ThinLinkOS->os()
: nullptr));
 } else {
@@ -1465,8 +1468,9 @@
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
- uint32_t(1));
+if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+   uint32_t(1));
   }
   MPM.addPass(
   BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));


Index: clang/test/CodeGen/enable-split-lto-unit.ll
===
--- /dev/null
+++ clang/test/CodeGen/enable-split-lto-unit.ll
@@ -0,0 +1,25 @@
+; Test that we do not duplicate the EnableSplitLTOUnit module flag.
+;
+; Disable the verifier so the compiler doesn't abort and thus lead to empty
+; output and false pass.
+;
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=full -disable-llvm-verifier -o - %s | llvm-dis | FileCheck %s --check-prefix=FULL-NPM
+; RUN: %clang_cc1 -fno-legacy-pass-manager -emit-llvm-bc -flto=thin -disable-llvm-verifier -o - %s | llvm-dis | FileChe

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

thx for trying it out, please revert so we can look into it more on the AMDGPU 
target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D114890#3167061 , @ronlieb wrote:

> thx for trying it out, please revert so we can look into it more on the 
> AMDGPU target

I'm going to make a new revision to make AMDGPU use the old runtime by default, 
SG?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

yes, thanks, sounds good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:544-546
+  // Set to true for attributes that support parameter pack expansion in its
+  // arguments.
+  bit ParmExpansionArgsSupport = 0;

steffenlarsen wrote:
> aaron.ballman wrote:
> > From a design perspective, I think this is a property of the parameters of 
> > an attribute rather than of an attribute itself. So I'd rather not add a 
> > bit to the `Attr` class, but instead modify the `Argument` class. There are 
> > a few approaches to that which I can think of, and I'm not certain which is 
> > the best approach.
> > 
> > 1) Introduce `ExpressionPackArgument` and attributes can opt into using it 
> > if they want to support packs.
> > 2) Add this bit to `VariadicExprArgument` and attributes can opt into using 
> > it if they want to support packs.
> > 3) Change `VariadicExprArgument` to also mean "allows packs" and all 
> > attributes using this automatically support packs.
> > 
> > I think that my conservative preference is for #2, but my intuition is that 
> > #3 is where we probably ultimately want to get to.
> > 
> > There are definitely some open questions with this approach that we'll have 
> > to figure out:
> > 
> > * Can you mix packs with variadics in the same attribute?
> > * Can you have two pack arguments in the same attribute?
> > * Does the tablegen design also seem likely to work when we want to 
> > introduce type packs instead of value packs?
> > 
> > I think the safest bet is to be conservative and not allow mixing packs 
> > with variadics, and not allowing multiple packs. We should be able to 
> > diagnose that situation from ClangAttrEmitter.cpp to help attribute authors 
> > out. However, it'd be worth adding a FIXME comment to that diagnostic logic 
> > asking whether we want to relax the behavior at some point. But if you feel 
> > strongly that we should support these situations initially, I wouldn't be 
> > opposed.
> Having `ParmExpansionArgsSupport` in `Attr` would allow attributes to 
> populate with parameter packs across argument boundaries. It seems more 
> permissive, but frankly I have no attachment to it, so I am okay with tying 
> it to `Argument` instead.
> 
> Of the mentioned options, I agree #2 and #3 are the preferred paths. For 
> simplicity I prefer #2, though I will have to figure out how best to add a 
> check on individual arguments, which @erichkeane's suggestion to limit the 
> attributes to only allowing parameter packs in variadic arguments as the last 
> argument would help with. On the other hand, the population logic for most of 
> the relevant attributes are explicitly defined in 
> `clang/lib/Sema/SemaDeclAttr.cpp` (which will have to be changed with #3) so 
> being more permissive - like the current version of the patch is - gives more 
> freedom to the attributes.
> 
> > Does the tablegen design also seem likely to work when we want to introduce 
> > type packs instead of value packs?
> 
> I am not sure about how tablegen would take it, but the parser doesn't 
> currently parse more than a single type argument, so I don't see a need for 
> allowing type packs at the moment.
> 
> > I think the safest bet is to be conservative and not allow mixing packs 
> > with variadics, and not allowing multiple packs. We should be able to 
> > diagnose that situation from ClangAttrEmitter.cpp to help attribute authors 
> > out. However, it'd be worth adding a FIXME comment to that diagnostic logic 
> > asking whether we want to relax the behavior at some point. But if you feel 
> > strongly that we should support these situations initially, I wouldn't be 
> > opposed.
> 
> If we limit packs to `VariadicExprArgument` - like `annotate` does in the 
> current state of this patch - having multiple packs in the same argument is 
> simple to handle. Both are represented as `Expr` and the variadic argument 
> will simply contain two expressions until the templates are instantiated and 
> the packs are expanded. This is also a feature I would like to keep, though 
> it should be possible to work around the limitation if we conclude it can't 
> stay.
> 
> Having ParmExpansionArgsSupport in Attr would allow attributes to populate 
> with parameter packs across argument boundaries. 

That's what I want to avoid. :-D I would prefer that the arguments have to opt 
into that behavior because the alternative could get ambiguous. I'd rather we 
do some extra work to explicitly support that situation once we have a specific 
attribute in mind for it.

> ... which @erichkeane's suggestion to limit the attributes to only allowing 
> parameter packs in variadic arguments as the last argument would help with. 

I agree with that suggestion, btw.

> I am not sure about how tablegen would take it, but the parser doesn't 
> currently parse more than a single type argument, so I don't see a need for 
> allowing type packs at the moment.

Fair point, and yet anothe

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@xazax.hun  Updated the summary for better undersanding differencies between 
the Standards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114718

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:544-546
+  // Set to true for attributes that support parameter pack expansion in its
+  // arguments.
+  bit ParmExpansionArgsSupport = 0;

aaron.ballman wrote:
> steffenlarsen wrote:
> > aaron.ballman wrote:
> > > From a design perspective, I think this is a property of the parameters 
> > > of an attribute rather than of an attribute itself. So I'd rather not add 
> > > a bit to the `Attr` class, but instead modify the `Argument` class. There 
> > > are a few approaches to that which I can think of, and I'm not certain 
> > > which is the best approach.
> > > 
> > > 1) Introduce `ExpressionPackArgument` and attributes can opt into using 
> > > it if they want to support packs.
> > > 2) Add this bit to `VariadicExprArgument` and attributes can opt into 
> > > using it if they want to support packs.
> > > 3) Change `VariadicExprArgument` to also mean "allows packs" and all 
> > > attributes using this automatically support packs.
> > > 
> > > I think that my conservative preference is for #2, but my intuition is 
> > > that #3 is where we probably ultimately want to get to.
> > > 
> > > There are definitely some open questions with this approach that we'll 
> > > have to figure out:
> > > 
> > > * Can you mix packs with variadics in the same attribute?
> > > * Can you have two pack arguments in the same attribute?
> > > * Does the tablegen design also seem likely to work when we want to 
> > > introduce type packs instead of value packs?
> > > 
> > > I think the safest bet is to be conservative and not allow mixing packs 
> > > with variadics, and not allowing multiple packs. We should be able to 
> > > diagnose that situation from ClangAttrEmitter.cpp to help attribute 
> > > authors out. However, it'd be worth adding a FIXME comment to that 
> > > diagnostic logic asking whether we want to relax the behavior at some 
> > > point. But if you feel strongly that we should support these situations 
> > > initially, I wouldn't be opposed.
> > Having `ParmExpansionArgsSupport` in `Attr` would allow attributes to 
> > populate with parameter packs across argument boundaries. It seems more 
> > permissive, but frankly I have no attachment to it, so I am okay with tying 
> > it to `Argument` instead.
> > 
> > Of the mentioned options, I agree #2 and #3 are the preferred paths. For 
> > simplicity I prefer #2, though I will have to figure out how best to add a 
> > check on individual arguments, which @erichkeane's suggestion to limit the 
> > attributes to only allowing parameter packs in variadic arguments as the 
> > last argument would help with. On the other hand, the population logic for 
> > most of the relevant attributes are explicitly defined in 
> > `clang/lib/Sema/SemaDeclAttr.cpp` (which will have to be changed with #3) 
> > so being more permissive - like the current version of the patch is - gives 
> > more freedom to the attributes.
> > 
> > > Does the tablegen design also seem likely to work when we want to 
> > > introduce type packs instead of value packs?
> > 
> > I am not sure about how tablegen would take it, but the parser doesn't 
> > currently parse more than a single type argument, so I don't see a need for 
> > allowing type packs at the moment.
> > 
> > > I think the safest bet is to be conservative and not allow mixing packs 
> > > with variadics, and not allowing multiple packs. We should be able to 
> > > diagnose that situation from ClangAttrEmitter.cpp to help attribute 
> > > authors out. However, it'd be worth adding a FIXME comment to that 
> > > diagnostic logic asking whether we want to relax the behavior at some 
> > > point. But if you feel strongly that we should support these situations 
> > > initially, I wouldn't be opposed.
> > 
> > If we limit packs to `VariadicExprArgument` - like `annotate` does in the 
> > current state of this patch - having multiple packs in the same argument is 
> > simple to handle. Both are represented as `Expr` and the variadic argument 
> > will simply contain two expressions until the templates are instantiated 
> > and the packs are expanded. This is also a feature I would like to keep, 
> > though it should be possible to work around the limitation if we conclude 
> > it can't stay.
> > 
> > Having ParmExpansionArgsSupport in Attr would allow attributes to populate 
> > with parameter packs across argument boundaries. 
> 
> That's what I want to avoid. :-D I would prefer that the arguments have to 
> opt into that behavior because the alternative could get ambiguous. I'd 
> rather we do some extra work to explicitly support that situation once we 
> have a specific attribute in mind for it.
> 
> > ... which @erichkeane's suggestion to limit the attributes to only allowing 
> > parameter packs in variadic arguments as the last argument would help with. 
> 
> I agree with that suggestion, btw.
> 
> > I am not sure about how tab

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Here is a smaller reproducer:

  void bar(short k) {
++k; // k1 = k0 + 1
assert(k == 1);  // k1 == 1 --> k0 == 0
(long)k << 16;   // k0 + 1 <<  16
  }

And the explanation is the following. With this patch, when the analyzer 
evaluates the `(long)k << 16` expression then it can properly deduce the value 
of `k` being `1`. However, it was not possible in the baseline. Since we do not 
model neither promotion nor explicit casts we get the false positive report. 
This issue highlights the importance of the patch stack to implement the 
modeling of casts (starting with https://reviews.llvm.org/D99797).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113753

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

[PATCH] D113896: [clangd] cleanup of header guard names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> This looks to be a deliberate decision: 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h#L32
>
> Looking at the code under `clang/include`, it looks like most headers don't 
> have the `#endif // COMMENT`, so this might be correct. However, the style 
> guide is rather vague and doesn't mention this at all: 
> https://llvm.org/docs/CodingStandards.html#header-guard

ah right, I was looking at 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp#L289,
 which happens to be the base class. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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

[clang] 2a2b3a3 - [clang-cl] Define _MSVC_LANG for -std=c++2b

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-12-02T12:09:20-05:00
New Revision: 2a2b3a3e3df7b89e18be9ffab1e08d7ca578cf57

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

LOG: [clang-cl] Define _MSVC_LANG for -std=c++2b

This matches the value that msvc v19.29 VS16.11 uses for
_MSVC_LANG with /std:c++latest.

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

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.cpp
clang/test/Preprocessor/predefined-win-macros.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.cpp 
b/clang/lib/Basic/Targets/OSTargets.cpp
index abf46fcb61a5d..53748bf067cd2 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -181,7 +181,9 @@ static void addVisualCDefines(const LangOptions &Opts, 
MacroBuilder &Builder) {
   Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
 
 if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
-  if (Opts.CPlusPlus20)
+  if (Opts.CPlusPlus2b)
+Builder.defineMacro("_MSVC_LANG", "202004L");
+  else if (Opts.CPlusPlus20)
 Builder.defineMacro("_MSVC_LANG", "202002L");
   else if (Opts.CPlusPlus17)
 Builder.defineMacro("_MSVC_LANG", "201703L");

diff  --git a/clang/test/Preprocessor/predefined-win-macros.c 
b/clang/test/Preprocessor/predefined-win-macros.c
index 2ded133d271bc..df43ae59cef42 100644
--- a/clang/test/Preprocessor/predefined-win-macros.c
+++ b/clang/test/Preprocessor/predefined-win-macros.c
@@ -45,9 +45,14 @@
 // CHECK-MS-NOT: GXX
 
 // RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
-// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2A
-// CHECK-MS-CPP2A: #define _MSC_VER 1900
-// CHECK-MS-CPP2A: #define _MSVC_LANG 202002L
+// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP20
+// CHECK-MS-CPP20: #define _MSC_VER 1900
+// CHECK-MS-CPP20: #define _MSVC_LANG 202002L
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
+// RUN: -fms-compatibility-version=19.00 -std=c++2b -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2B
+// CHECK-MS-CPP2B: #define _MSC_VER 1900
+// CHECK-MS-CPP2B: #define _MSVC_LANG 202004L
 
 // RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN



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

[PATCH] D114952: [clang-cl] Define _MSVC_LANG for -std=c++2b

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a2b3a3e3df7: [clang-cl] Define _MSVC_LANG for -std=c++2b 
(authored by thakis).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114952

Files:
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/test/Preprocessor/predefined-win-macros.c


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -45,9 +45,14 @@
 // CHECK-MS-NOT: GXX
 
 // RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
-// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2A
-// CHECK-MS-CPP2A: #define _MSC_VER 1900
-// CHECK-MS-CPP2A: #define _MSVC_LANG 202002L
+// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP20
+// CHECK-MS-CPP20: #define _MSC_VER 1900
+// CHECK-MS-CPP20: #define _MSVC_LANG 202002L
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions 
-fms-compatibility \
+// RUN: -fms-compatibility-version=19.00 -std=c++2b -o - | FileCheck 
-match-full-lines %s --check-prefix=CHECK-MS-CPP2B
+// CHECK-MS-CPP2B: #define _MSC_VER 1900
+// CHECK-MS-CPP2B: #define _MSVC_LANG 202004L
 
 // RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -181,7 +181,9 @@
   Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
 
 if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
-  if (Opts.CPlusPlus20)
+  if (Opts.CPlusPlus2b)
+Builder.defineMacro("_MSVC_LANG", "202004L");
+  else if (Opts.CPlusPlus20)
 Builder.defineMacro("_MSVC_LANG", "202002L");
   else if (Opts.CPlusPlus17)
 Builder.defineMacro("_MSVC_LANG", "201703L");


Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -45,9 +45,14 @@
 // CHECK-MS-NOT: GXX
 
 // RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions -fms-compatibility \
-// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP2A
-// CHECK-MS-CPP2A: #define _MSC_VER 1900
-// CHECK-MS-CPP2A: #define _MSVC_LANG 202002L
+// RUN: -fms-compatibility-version=19.00 -std=c++20 -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP20
+// CHECK-MS-CPP20: #define _MSC_VER 1900
+// CHECK-MS-CPP20: #define _MSVC_LANG 202002L
+
+// RUN: %clang_cc1 %s -x c++ -E -dM -triple i686-pc-win32 -fms-extensions -fms-compatibility \
+// RUN: -fms-compatibility-version=19.00 -std=c++2b -o - | FileCheck -match-full-lines %s --check-prefix=CHECK-MS-CPP2B
+// CHECK-MS-CPP2B: #define _MSC_VER 1900
+// CHECK-MS-CPP2B: #define _MSVC_LANG 202004L
 
 // RUN: %clang_cc1 -triple i386-windows %s -E -dM -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-X86-WIN
Index: clang/lib/Basic/Targets/OSTargets.cpp
===
--- clang/lib/Basic/Targets/OSTargets.cpp
+++ clang/lib/Basic/Targets/OSTargets.cpp
@@ -181,7 +181,9 @@
   Builder.defineMacro("_HAS_CHAR16_T_LANGUAGE_SUPPORT", Twine(1));
 
 if (Opts.isCompatibleWithMSVC(LangOptions::MSVC2015)) {
-  if (Opts.CPlusPlus20)
+  if (Opts.CPlusPlus2b)
+Builder.defineMacro("_MSVC_LANG", "202004L");
+  else if (Opts.CPlusPlus20)
 Builder.defineMacro("_MSVC_LANG", "202002L");
   else if (Opts.CPlusPlus17)
 Builder.defineMacro("_MSVC_LANG", "201703L");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 391352.
martong marked 2 inline comments as done.
martong added a comment.

- Rename simplifySValImpl to simplifySValOnce
- Remove a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -21,6 +21,35 @@
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
+
+  // With one `simplifySValOnce` call, a compound symbols might collapse to
+  // simpler symbol tree that is still possible to further simplify. Thus, we
+  // do the simplification on a new symbol tree until we reach the simplest
+  // form, i.e. the fixpoint.
+  // Consider the following symbol `(b * b) * b * b` which has this tree:
+  //   *
+  //  / \
+  // *   b
+  ///  \
+  //   /b
+  // (b * b)
+  // Now, if the `b * b == 1` new constraint is added then during the first
+  // iteration we have the following transformations:
+  //   *  *
+  //  / \/ \
+  // *   b -->  b   b
+  ///  \
+  //   /b
+  //  1
+  // We need another iteration to reach the final result `1`.
+  SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
+
+  // Recursively descends into symbolic expressions and replaces symbols
+  // with their known values (in the sense of the getKnownValue() method).
+  // We traverse the symbol tree and query the constraint values for the
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValOnce(ProgramStateRef State, SVal V);
+
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
 ProgramStateManager &stateMgr)
@@ -40,8 +69,6 @@
   ///  (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
 
-  /// Recursively descends into symbolic expressions and replaces symbols
-  /// with their known values (in the sense of the getKnownValue() method).
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
 
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
@@ -1105,7 +1132,20 @@
   return nullptr;
 }
 
+SVal SimpleSValBuilder::simplifyUntilFixpoint(ProgramStateRef State, SVal Val) {
+  SVal SimplifiedVal = simplifySValOnce(State, Val);
+  while (SimplifiedVal != Val) {
+Val = SimplifiedVal;
+SimplifiedVal = simplifySValOnce(State, Val);
+  }
+  return SimplifiedVal;
+}
+
 SVal SimpleSValBuilder::simplifySVal(ProgramStateRef State, SVal V) {
+  return simplifyUntilFixpoint(State, V);
+}
+
+SVal SimpleSValBuilder::simplifySValOnce(ProgramStateRef State, SVal V) {
   // For now, this function tries to constant-fold symbols inside a
   // nonloc::SymbolVal, and does nothing else. More simplifications should
   // be possible, such as constant-folding an index in an ElementRegion.
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2191,42 +2191,6 @@
  Constraint->getMaxValue(), true);
 }
 
-// Simplify the given symbol with the help of the SValBuilder. In
-// SValBuilder::symplifySval, we traverse the symbol tree and query the
-// constraint values for the sub-trees and if a value is a constant we do the
-// constant folding. Compound symbols might collapse to simpler symbol tree
-// that is still possible to further simplify. Thus, we do the simplification on
-// a new symbol tree until we reach the simplest form, i.e. the fixpoint.
-//
-// Consider the following symbol `(b * b) * b * b` which has this tree:
-//   *
-//  / \
-// *   b
-///  \
-//   /b
-// (b * b)
-// Now, if the `b * b == 1` new constraint is added then during the first
-// iteration we have the following transformations:
-//   *  *
-//  / \/ \
-// *   b -->  b   b
-///  \
-//   /b
-//  1
-// We need another iteration to reach the final result `1`.
-LLVM_NODISCARD
-static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
-  const SymbolRef Sym) {
-  SVal Val = SVB.makeSymbolVal(Sym);
-  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
-  // Do the simplification until we can.
-  while (SimplifiedVal != Val) {
-Val = SimplifiedVal;
-SimplifiedVal = SVB.simplifySVal(State, Val);
-  }
-  return SimplifiedVal;
-}
-
 // Iterat

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D113898#3167050 , @kuhnel wrote:

> Looking at the documentation, this looks like a bug in the clang-tidy check:
> https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-qualified-auto.html
>
> The rule `llvm-qualified-auto` should not add a `const` qualifier.

I don't see how that is a bug: the docs specially cover the `const auto *` 
conversion in the first example by drawing the line between `observe()` 
(non-mutating function) and `change()` (possibly mutating function).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[PATCH] D114965: [OpenMP] Remove the new runtime default for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, tianshilei1992, 
gregrodgers, ronlieb.
Herald added subscribers: dang, kerbowa, guansong, t-tye, tpr, dstuttard, 
yaxunl, nhaehnle, jvesely, kzhuravl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a project: clang.

The new runtime is currently broken for AMD offloading. This patch makes
the default the old runtime only for the AMD target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114965

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/true))
+   /*Default=*/!getToolChain().getTriple().isAMDGCN()))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, true))
+ options::OPT_fno_openmp_target_new_runtime, false))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/true))
+   /*Default=*/!getToolChain().getTriple().isAMDGCN()))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, true))
+ options::OPT_fno_openmp_target_new_runtime, false))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114966: [clang][deps] Split filesystem caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
Herald added subscribers: dexonsmith, hiraditya.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114966

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -75,6 +75,12 @@
 : Name(Name.str()), UID(UID), MTime(MTime), User(User), Group(Group),
   Size(Size), Type(Type), Perms(Perms) {}
 
+Status Status::copyWithNewSize(const Status &In, uint64_t NewSize) {
+  return Status(In.getName(), In.getUniqueID(), In.getLastModificationTime(),
+In.getUser(), In.getGroup(), NewSize, In.getType(),
+In.getPermissions());
+}
+
 Status Status::copyWithNewName(const Status &In, const Twine &NewName) {
   return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
 In.getUser(), In.getGroup(), In.getSize(), In.getType(),
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -64,6 +64,8 @@
  uint64_t Size, llvm::sys::fs::file_type Type,
  llvm::sys::fs::perms Perms);
 
+  /// Get a copy of a Status with a different size.
+  static Status copyWithNewSize(const Status &In, uint64_t NewSize);
   /// Get a copy of a Status with a different name.
   static Status copyWithNewName(const Status &In, const Twine &NewName);
   static Status copyWithNewName(const llvm::sys::fs::file_status &In,
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -15,9 +15,9 @@
 using namespace tooling;
 using namespace dependencies;
 
-CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
-StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize) {
-  // Load the file and its content from the file system.
+static llvm::ErrorOr
+readFile(StringRef Filename, llvm::vfs::FileSystem &FS,
+ llvm::SmallString<1> &SharedOriginalContents) {
   llvm::ErrorOr> MaybeFile =
   FS.openFileForRead(Filename);
   if (!MaybeFile)
@@ -32,31 +32,27 @@
   if (!MaybeBuffer)
 return MaybeBuffer.getError();
 
+  const auto &Buffer = *MaybeBuffer;
+  SharedOriginalContents.reserve(Buffer->getBufferSize() + 1);
+  SharedOriginalContents.append(Buffer->getBufferStart(),
+Buffer->getBufferEnd());
+  // Implicitly null terminate the contents for Clang's lexer.
+  SharedOriginalContents.push_back('\0');
+  SharedOriginalContents.pop_back();
+  return *Stat;
+}
+
+static bool
+minimizeFile(const SmallString<1> &OriginalContents,
+ SmallString<1> &MinimizedContents,
+ PreprocessorSkippedRangeMapping &PPSkippedRangeMapping) {
   llvm::SmallString<1024> MinimizedFileContents;
   // Minimize the file down to directives that might affect the dependencies.
-  const auto &Buffer = *MaybeBuffer;
   SmallVector Tokens;
-  if (!Minimize || minimizeSourceToDependencyDirectives(
-   Buffer->getBuffer(), MinimizedFileContents, Tokens)) {
-// Use the original file unless requested otherwise, or
-// if the minimization failed.
-// FIXME: Propage the diagnostic if desired by the client.
-CachedFileSystemEntry Result;
-Result.MaybeStat = std::move(*Stat);
-Result.Contents.reserve(Buffer->getBufferSize() + 1);
-Result.Contents.append(Buffer->getBufferStart(), Buffer->getBufferEnd());
-// Implicitly null terminate the contents for Clang's lexer.
-Result.Contents.push_back('\0');
-Result.Contents.pop_back();
-return Result;
-  }
+  if (minimizeSourceToDependencyDirectives(OriginalContents.str(),
+   MinimizedFileContents, Tokens))
+return true;
 
-  CachedFileSystemEntry Result;
-  size_t Size = MinimizedFileContents.size();
-  Result.MaybeStat = llvm::vfs::Status(Stat->getName(), Stat->getUniqueID(),
-   Stat->getLastModificationTime(),
-   Stat->getUser(), Stat->getGroup(), Size,
-   Stat->getType(), Stat->getPermissions());
   // The contents produced by the minimizer must be null terminated.
   assert(MinimizedFileContents.data()[Mi

[PATCH] D114965: [OpenMP] Remove the new runtime default for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

I feel like I should make these options only enabled if we're in an OpenMP 
device. I just realized that this will probably enable it for anything that 
goes through here, even non-OpenMP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114965

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

[PATCH] D114965: [OpenMP] Remove the new runtime default for AMDGPU

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D114965#3167241 , @jhuber6 wrote:

> I feel like I should make these options only enabled if we're in an OpenMP 
> device. I just realized that this will probably enable it for anything that 
> goes through here, even non-OpenMP.

Well it's only for OpenMP with `-fopenmp`, but I guess it's fine to have that 
flag in the cc1 options even if offloading isn't used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114965

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

[clang] 96ff74a - [OpenMP] Remove the new runtime default for AMDGPU

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2021-12-02T12:35:58-05:00
New Revision: 96ff74a0d5982cb70ac25883f3b0d5bb99947aa9

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

LOG: [OpenMP] Remove the new runtime default for AMDGPU

The new runtime is currently broken for AMD offloading. This patch makes
the default the old runtime only for the AMD target.

Reviewed By: ronlieb

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 6a1f557968a92..4e6dd20503446 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@ def fno_openmp_assume_teams_oversubscription : 
Flag<["-"], "fno-openmp-assume-te
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",

diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index f282f04b79311..863e2c597d53a 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, true))
+ options::OPT_fno_openmp_target_new_runtime, false))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 8d92c293d83a4..c5aaa067c4f55 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/true))
+   /*Default=*/!getToolChain().getTriple().isAMDGCN()))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.



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

[PATCH] D114965: [OpenMP] Remove the new runtime default for AMDGPU

[Joseph Huber 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 rG96ff74a0d598: [OpenMP] Remove the new runtime default for 
AMDGPU (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114965

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/true))
+   /*Default=*/!getToolChain().getTriple().isAMDGCN()))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, true))
+ options::OPT_fno_openmp_target_new_runtime, false))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/true))
+   /*Default=*/!getToolChain().getTriple().isAMDGCN()))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, true))
+ options::OPT_fno_openmp_target_new_runtime, false))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], "fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114968: [clang][deps] Avoid reading file for stat calls

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D114966 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114968

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -290,7 +290,7 @@
 
 llvm::ErrorOr
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
-StringRef Filename) {
+StringRef Filename, RequestedInformation RI) {
   SharedStat *SharedStat = nullptr;
   llvm::Optional> GuardLock;
 
@@ -304,6 +304,12 @@
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
 
   if (!shouldMinimize(Filename)) {
+if (RI == RI_Status) {
+  // If the caller wants a status of a file that doesn't need minimization,
+  // reading it is not necessary.
+  return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+}
+
 const OriginalContents *LocalOriginalContents =
 getOrCreateLocalOriginalContents(Filename, UID, SharedStat, GuardLock);
 return FileSystemEntryResult(*LocalStat, LocalOriginalContents, nullptr);
@@ -321,7 +327,7 @@
 DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Status);
   if (!Result)
 return Result.getError();
   return Result->Status;
@@ -381,7 +387,7 @@
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Contents);
   if (!Result)
 return Result.getError();
   if (!Result->Status)
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -223,10 +223,16 @@
   StringRef Filename, llvm::sys::fs::UniqueID UID, SharedStat *&SharedStat,
   llvm::Optional> &Lock);
 
+  /// The information requested from \c getOrCreateFileSystemEntry.
+  enum RequestedInformation {
+RI_Status,
+RI_Contents,
+  };
+
   /// Get the full filesystem entry for \p Path from local cache, populating it
   /// if necessary.
   llvm::ErrorOr
-  getOrCreateFileSystemEntry(StringRef Path);
+  getOrCreateFileSystemEntry(StringRef Path, RequestedInformation RI);
 
   /// The global cache shared between worker threads.
   DependencyScanningFilesystemSharedCache &SharedCache;


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -290,7 +290,7 @@
 
 llvm::ErrorOr
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
-StringRef Filename) {
+StringRef Filename, RequestedInformation RI) {
   SharedStat *SharedStat = nullptr;
   llvm::Optional> GuardLock;
 
@@ -304,6 +304,12 @@
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
 
   if (!shouldMinimize(Filename)) {
+if (RI == RI_Status) {
+  // If the caller wants a status of a file that doesn't need minimization,
+  // reading it is not necessary.
+  return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+}
+
 const OriginalContents *LocalOriginalContents =
 getOrCreateLocalOriginalContents(Filename, UID, SharedStat, GuardLock);
 return FileSystemEntryResult(*LocalStat, LocalOriginalContents, nullptr);
@@ -321,7 +327,7 @@
 DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Status);
   if (!Result)
 return Result.getError();
   return Result->Status;
@@ -381,7 +387,7 @@
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Contents);
   if (!Result)
 return Result.getError();
   if (!Result->Status)
Index: clang/include/clang/Tooling/Depen

[PATCH] D114971: [clang][deps] Handle symlinks in minimizing FS

[Jan Svoboda via Phabricator via cfe-commits]

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

Depends on D114968 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114971

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/modules-symlink.c

Index: clang/test/ClangScanDeps/modules-symlink.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-symlink.c
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- cdb_pch.json
+[
+  {
+"directory": "DIR",
+"command": "clang -x c-header DIR/pch.h -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -o DIR/pch.h.gch",
+"file": "DIR/pch.h"
+  }
+]
+
+//--- cdb_tu.json
+[
+  {
+"directory": "DIR",
+"command": "clang -c DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+]
+
+//--- module.modulemap
+module mod { header "symlink.h" }
+
+//--- pch.h
+#include "symlink.h"
+
+//--- original.h
+// Comment that will be stripped by the minimizer.
+#define MACRO 1
+
+//--- tu.c
+#include "original.h"
+static int foo = MACRO; // Macro usage that will trigger
+// input file consistency checks.
+
+// RUN: ln -s %t/original.h %t/symlink.h
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_pch.json > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build > %t/result_pch.json
+//
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \
+// RUN:   --module-name=mod > %t/mod.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \
+// RUN:   --tu-index=0 > %t/pch.rsp
+//
+// RUN: %clang @%t/mod.cc1.rsp
+// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch -I %t @%t/pch.rsp
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_tu.json > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build > %t/result_tu.json
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -190,19 +190,20 @@
 }
 
 void DependencyScanningWorkerFilesystem::disableMinimization(
-StringRef RawFilename) {
-  llvm::SmallString<256> Filename;
-  llvm::sys::path::native(RawFilename, Filename);
-  NotToBeMinimized.insert(Filename);
+StringRef Filename) {
+  // Only requesting the status' \c UniqueID.
+  // This prevents \c getOrCreateFileSystemEntry to unnecessarily read/minimize
+  // the file and to call \c shouldMinimize that reads \c NotToBeMinimized we're
+  // not done setting up yet.
+  if (auto FileSystemEntry = getOrCreateFileSystemEntry(Filename, RI_UniqueID))
+NotToBeMinimized.insert(FileSystemEntry->Status->getUniqueID());
 }
 
-bool DependencyScanningWorkerFilesystem::shouldMinimize(StringRef RawFilename) {
-  if (!shouldMinimizeBasedOnExtension(RawFilename))
+bool DependencyScanningWorkerFilesystem::shouldMinimize(
+StringRef Filename, llvm::sys::fs::UniqueID UID) {
+  if (!shouldMinimizeBasedOnExtension(Filename))
 return false;
-
-  llvm::SmallString<256> Filename;
-  llvm::sys::path::native(RawFilename, Filename);
-  return !NotToBeMinimized.contains(Filename);
+  return !NotToBeMinimized.contains(UID);
 }
 
 void DependencyScanningWorkerFilesystem::ensureLocked(
@@ -302,8 +303,15 @@
 return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
 
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
+  if (RI == RI_UniqueID) {
+// We're returning the original stat of a file that might need minimization,
+// meaning the reported size won't be consistent with what we return for
+// RI_StatOrContents. That's fine though, since the caller promises to only
+// look the UniqueID of the returned status.
+return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+  }
 
-  if (!shouldMinimize(Filename)) {
+  if (!shouldMinimize(Filename, UID)) {
 if (RI == RI_Status) {
   // If the caller wants a status of a file that doesn't need minimization,
   // reading it is not necessary.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ cl

[PATCH] D114966: [clang][deps] Split filesystem caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 391369.
jansvoboda11 added a comment.

Add unit test, IWYU.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114966

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -75,6 +75,12 @@
 : Name(Name.str()), UID(UID), MTime(MTime), User(User), Group(Group),
   Size(Size), Type(Type), Perms(Perms) {}
 
+Status Status::copyWithNewSize(const Status &In, uint64_t NewSize) {
+  return Status(In.getName(), In.getUniqueID(), In.getLastModificationTime(),
+In.getUser(), In.getGroup(), NewSize, In.getType(),
+In.getPermissions());
+}
+
 Status Status::copyWithNewName(const Status &In, const Twine &NewName) {
   return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
 In.getUser(), In.getGroup(), In.getSize(), In.getType(),
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -64,6 +64,8 @@
  uint64_t Size, llvm::sys::fs::file_type Type,
  llvm::sys::fs::perms Perms);
 
+  /// Get a copy of a Status with a different size.
+  static Status copyWithNewSize(const Status &In, uint64_t NewSize);
   /// Get a copy of a Status with a different name.
   static Status copyWithNewName(const Status &In, const Twine &NewName);
   static Status copyWithNewName(const llvm::sys::fs::file_status &In,
Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -205,9 +205,11 @@
 }
 
 namespace dependencies {
-TEST(DependencyScanningFilesystem, IgnoredFilesHaveSeparateCache) {
+TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately1) {
   auto VFS = llvm::makeIntrusiveRefCnt();
-  VFS->addFile("/mod.h", 0, llvm::MemoryBuffer::getMemBuffer("// hi there!\n"));
+  VFS->addFile("/mod.h", 0,
+   llvm::MemoryBuffer::getMemBuffer("#include \n"
+"// hi there!\n"));
 
   DependencyScanningFilesystemSharedCache SharedCache;
   auto Mappings = std::make_unique();
@@ -223,14 +225,35 @@
   auto StatusFull3 = DepFS.status("/mod.h");
 
   EXPECT_TRUE(StatusMinimized0);
-  EXPECT_EQ(StatusMinimized0->getSize(), 0u);
+  EXPECT_EQ(StatusMinimized0->getSize(), 17u);
   EXPECT_TRUE(StatusFull1);
-  EXPECT_EQ(StatusFull1->getSize(), 13u);
+  EXPECT_EQ(StatusFull1->getSize(), 30u);
 
   EXPECT_TRUE(StatusMinimized2);
-  EXPECT_EQ(StatusMinimized2->getSize(), 0u);
+  EXPECT_EQ(StatusMinimized2->getSize(), 17u);
   EXPECT_TRUE(StatusFull3);
-  EXPECT_EQ(StatusFull3->getSize(), 13u);
+  EXPECT_EQ(StatusFull3->getSize(), 30u);
+}
+
+TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
+  auto VFS = llvm::makeIntrusiveRefCnt();
+  VFS->addFile("/mod.h", 0,
+   llvm::MemoryBuffer::getMemBuffer("#include \n"
+"// hi there!\n"));
+
+  DependencyScanningFilesystemSharedCache SharedCache;
+  auto Mappings = std::make_unique();
+  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS, Mappings.get());
+
+  DepFS.disableMinimization("/mod.h");
+  auto StatusFull0 = DepFS.status("/mod.h");
+  DepFS.enableMinimizationOfAllFiles();
+  auto StatusMinimized1 = DepFS.status("/mod.h");
+
+  EXPECT_TRUE(StatusFull0);
+  EXPECT_TRUE(StatusMinimized1);
+  EXPECT_EQ(StatusFull0->getSize(), 30u);
+  EXPECT_EQ(StatusMinimized1->getSize(), 17u);
 }
 
 } // end namespace dependencies
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -15,9 +15,9 @@
 using namespace tooling;
 using namespace dependencies;
 
-CachedFileSystemEntry CachedFileSystemEntry::createFileEntry(
-StringRef Filename, llvm::vfs::FileSystem &FS, bool Minimize) {
-  // Load the file and its content from the file system.
+static llvm::ErrorOr
+readFile(StringRef Filename, llvm::vfs::FileSystem &FS,
+ llvm::SmallString<1> &SharedOriginalContents) {
   llvm::ErrorOr> MaybeFile =
   FS.openFileForRead(Filename);
   if

[PATCH] D114968: [clang][deps] Avoid reading file for stat calls

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 391370.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114968

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -290,7 +290,7 @@
 
 llvm::ErrorOr
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
-StringRef Filename) {
+StringRef Filename, RequestedInformation RI) {
   SharedStat *SharedStat = nullptr;
   llvm::Optional> GuardLock;
 
@@ -304,6 +304,12 @@
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
 
   if (!shouldMinimize(Filename)) {
+if (RI == RI_Status) {
+  // If the caller wants a status of a file that doesn't need minimization,
+  // reading it is not necessary.
+  return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+}
+
 const OriginalContents *LocalOriginalContents =
 getOrCreateLocalOriginalContents(Filename, UID, SharedStat, GuardLock);
 return FileSystemEntryResult(*LocalStat, LocalOriginalContents, nullptr);
@@ -321,7 +327,7 @@
 DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Status);
   if (!Result)
 return Result.getError();
   return Result->Status;
@@ -381,7 +387,7 @@
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Contents);
   if (!Result)
 return Result.getError();
   if (!Result->Status)
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -225,10 +225,16 @@
   StringRef Filename, llvm::sys::fs::UniqueID UID, SharedStat *&SharedStat,
   llvm::Optional> &Lock);
 
+  /// The information requested from \c getOrCreateFileSystemEntry.
+  enum RequestedInformation {
+RI_Status,
+RI_Contents,
+  };
+
   /// Get the full filesystem entry for \p Path from local cache, populating it
   /// if necessary.
   llvm::ErrorOr
-  getOrCreateFileSystemEntry(StringRef Path);
+  getOrCreateFileSystemEntry(StringRef Path, RequestedInformation RI);
 
   /// The global cache shared between worker threads.
   DependencyScanningFilesystemSharedCache &SharedCache;


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -290,7 +290,7 @@
 
 llvm::ErrorOr
 DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry(
-StringRef Filename) {
+StringRef Filename, RequestedInformation RI) {
   SharedStat *SharedStat = nullptr;
   llvm::Optional> GuardLock;
 
@@ -304,6 +304,12 @@
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
 
   if (!shouldMinimize(Filename)) {
+if (RI == RI_Status) {
+  // If the caller wants a status of a file that doesn't need minimization,
+  // reading it is not necessary.
+  return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+}
+
 const OriginalContents *LocalOriginalContents =
 getOrCreateLocalOriginalContents(Filename, UID, SharedStat, GuardLock);
 return FileSystemEntryResult(*LocalStat, LocalOriginalContents, nullptr);
@@ -321,7 +327,7 @@
 DependencyScanningWorkerFilesystem::status(const Twine &Path) {
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Status);
   if (!Result)
 return Result.getError();
   return Result->Status;
@@ -381,7 +387,7 @@
   SmallString<256> OwnedFilename;
   StringRef Filename = Path.toStringRef(OwnedFilename);
 
-  auto Result = getOrCreateFileSystemEntry(Filename);
+  auto Result = getOrCreateFileSystemEntry(Filename, RI_Contents);
   if (!Result)
 return Result.getError();
   if (!Result->Status)
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
==

[PATCH] D114971: [clang][deps] Handle symlinks in minimizing FS

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 391371.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114971

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/ClangScanDeps/modules-symlink.c

Index: clang/test/ClangScanDeps/modules-symlink.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-symlink.c
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- cdb_pch.json
+[
+  {
+"directory": "DIR",
+"command": "clang -x c-header DIR/pch.h -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -o DIR/pch.h.gch",
+"file": "DIR/pch.h"
+  }
+]
+
+//--- cdb_tu.json
+[
+  {
+"directory": "DIR",
+"command": "clang -c DIR/tu.c -fmodules -gmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/pch.h -o DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+]
+
+//--- module.modulemap
+module mod { header "symlink.h" }
+
+//--- pch.h
+#include "symlink.h"
+
+//--- original.h
+// Comment that will be stripped by the minimizer.
+#define MACRO 1
+
+//--- tu.c
+#include "original.h"
+static int foo = MACRO; // Macro usage that will trigger
+// input file consistency checks.
+
+// RUN: ln -s %t/original.h %t/symlink.h
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_pch.json > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build > %t/result_pch.json
+//
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \
+// RUN:   --module-name=mod > %t/mod.cc1.rsp
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t/result_pch.json \
+// RUN:   --tu-index=0 > %t/pch.rsp
+//
+// RUN: %clang @%t/mod.cc1.rsp
+// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch -I %t @%t/pch.rsp
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb_tu.json > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build > %t/result_tu.json
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -190,19 +190,20 @@
 }
 
 void DependencyScanningWorkerFilesystem::disableMinimization(
-StringRef RawFilename) {
-  llvm::SmallString<256> Filename;
-  llvm::sys::path::native(RawFilename, Filename);
-  NotToBeMinimized.insert(Filename);
+StringRef Filename) {
+  // Only requesting the status' \c UniqueID.
+  // This prevents \c getOrCreateFileSystemEntry to unnecessarily read/minimize
+  // the file and to call \c shouldMinimize that reads \c NotToBeMinimized we're
+  // not done setting up yet.
+  if (auto FileSystemEntry = getOrCreateFileSystemEntry(Filename, RI_UniqueID))
+NotToBeMinimized.insert(FileSystemEntry->Status->getUniqueID());
 }
 
-bool DependencyScanningWorkerFilesystem::shouldMinimize(StringRef RawFilename) {
-  if (!shouldMinimizeBasedOnExtension(RawFilename))
+bool DependencyScanningWorkerFilesystem::shouldMinimize(
+StringRef Filename, llvm::sys::fs::UniqueID UID) {
+  if (!shouldMinimizeBasedOnExtension(Filename))
 return false;
-
-  llvm::SmallString<256> Filename;
-  llvm::sys::path::native(RawFilename, Filename);
-  return !NotToBeMinimized.contains(Filename);
+  return !NotToBeMinimized.contains(UID);
 }
 
 void DependencyScanningWorkerFilesystem::ensureLocked(
@@ -302,8 +303,15 @@
 return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
 
   llvm::sys::fs::UniqueID UID = (*LocalStat)->getUniqueID();
+  if (RI == RI_UniqueID) {
+// We're returning the original stat of a file that might need minimization,
+// meaning the reported size won't be consistent with what we return for
+// RI_StatOrContents. That's fine though, since the caller promises to only
+// look the UniqueID of the returned status.
+return FileSystemEntryResult(*LocalStat, nullptr, nullptr);
+  }
 
-  if (!shouldMinimize(Filename)) {
+  if (!shouldMinimize(Filename, UID)) {
 if (RI == RI_Status) {
   // If the caller wants a status of a file that doesn't need minimization,
   // reading it is not necessary.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/Dependency

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113743

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