[clang-tools-extra] e507711 - Attempt to fix Windows builds after D110386

2021-09-27 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2021-09-28T08:13:01+02:00
New Revision: e50771181b7e0d96b30ee33049dc05172125b927

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

LOG: Attempt to fix Windows builds after D110386

http://45.33.8.238/win/46013/summary.html

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp 
b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
index aeca6d8fa0196..8a6f5e8b4619e 100644
--- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -163,10 +163,9 @@ TEST_F(HeadersTest, CollectRewrittenAndResolved) {
   EXPECT_THAT(Includes.MainFileIncludes,
   UnorderedElementsAre(
   AllOf(Written("\"sub/bar.h\""), Resolved(BarHeader;
-  EXPECT_THAT(collectIncludes().includeDepth(getID(MainFile, Includes)),
-  UnorderedElementsAre(
-  Distance(getID(MainFile, Includes), 0u),
-  Distance(getID(testPath("sub/bar.h"), Includes), 1u)));
+  EXPECT_THAT(Includes.includeDepth(getID(MainFile, Includes)),
+  UnorderedElementsAre(Distance(getID(MainFile, Includes), 0u),
+   Distance(getID(BarHeader, Includes), 1u)));
 }
 
 TEST_F(HeadersTest, OnlyCollectInclusionsInMain) {
@@ -180,20 +179,17 @@ TEST_F(HeadersTest, OnlyCollectInclusionsInMain) {
 #include "bar.h"
 )cpp";
   auto Includes = collectIncludes();
-  EXPECT_THAT(
-  collectIncludes().MainFileIncludes,
-  UnorderedElementsAre(AllOf(Written("\"bar.h\""), Resolved(BarHeader;
-  EXPECT_THAT(Includes.includeDepth(getID(MainFile, Includes)),
+  EXPECT_THAT(Includes.MainFileIncludes,
   UnorderedElementsAre(
-  Distance(getID(MainFile, Includes), 0u),
-  Distance(getID(testPath("sub/bar.h"), Includes), 1u),
-  Distance(getID(testPath("sub/baz.h"), Includes), 2u)));
+  AllOf(Written("\"bar.h\""), Resolved(BarHeader;
+  EXPECT_THAT(Includes.includeDepth(getID(MainFile, Includes)),
+  UnorderedElementsAre(Distance(getID(MainFile, Includes), 0u),
+   Distance(getID(BarHeader, Includes), 1u),
+   Distance(getID(BazHeader, Includes), 2u)));
   // includeDepth() also works for non-main files.
-  EXPECT_THAT(
-  collectIncludes().includeDepth(getID(testPath("sub/bar.h"), Includes)),
-  UnorderedElementsAre(
-  Distance(getID(testPath("sub/bar.h"), Includes), 0u),
-  Distance(getID(testPath("sub/baz.h"), Includes), 1u)));
+  EXPECT_THAT(Includes.includeDepth(getID(BarHeader, Includes)),
+  UnorderedElementsAre(Distance(getID(BarHeader, Includes), 0u),
+   Distance(getID(BazHeader, Includes), 1u)));
 }
 
 TEST_F(HeadersTest, PreambleIncludesPresentOnce) {

diff  --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp 
b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index ab45c68c9e569..9195865294f19 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -519,7 +519,7 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) {
   auto AuxFE = FM.getFile(testPath("sub/aux.h"));
   ASSERT_TRUE(AuxFE);
   auto AuxID = Includes.getID(*AuxFE);
-  EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(AuxID));
+  EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
 }
 
 TEST(ParsedASTTest, PatchesDeletedIncludes) {



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


[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 375476.
cor3ntin added a comment.

Rename commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-unary-static-assert.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CXX/dcl.dcl/dcl.link/p2.cpp
  clang/test/CXX/dcl.dcl/p4-0x.cpp
  clang/test/FixIt/fixit-static-assert.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/asm.cpp
  clang/test/Parser/attr-availability-xcore.c
  clang/test/Parser/attr-availability.c
  clang/test/Sema/asm.c
  clang/test/SemaCXX/static-assert.cpp

Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -28,12 +28,12 @@
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
 S s2;
 
-static_assert(false, L"\x"); // expected-error {{static_assert failed L"\x"}}
-static_assert(false, u"\U000317FF"); // expected-error {{static_assert failed u"\U000317FF"}}
+static_assert(false, L"\x"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, u"\U000317FF"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 // FIXME: render this as u8"\u03A9"
-static_assert(false, u8"Ω"); // expected-error {{static_assert failed u8"\316\251"}}
-static_assert(false, L"\u1234"); // expected-error {{static_assert failed L"\x1234"}}
-static_assert(false, L"\x1ff" "0\x123" "fx\xf" "goop"); // expected-error {{static_assert failed L"\x1FF""0\x123""fx\xFgoop"}}
+static_assert(false, u8"Ω"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\u1234"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\x1ff" "0\x123" "fx\xf" "goop"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 
 template struct AlwaysFails {
   // Only give one error here.
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -37,14 +37,17 @@
   asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
   asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
   asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
-  asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} 
+  asm("nop"
+  : "=a"(a)
+  : "b"(b)
+  : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
 }
 
 // rdar://6094010
 void test3() {
   int x;
-  asm(L"foo" : "=r"(x)); // expected-error {{wide string}}
-  asm("foo" : L"=r"(x)); // expected-error {{wide string}}
+  asm(L"foo" : "=r"(x)); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+  asm("foo" : L"=r"(x)); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 }
 
 // 
Index: clang/test/Parser/attr-availability.c
===
--- clang/test/Parser/attr-availability.c
+++ clang/test/Parser/attr-availability.c
@@ -18,21 +18,25 @@
 
 void f6() __attribute__((availability(macosx,unavailable,introduced=10.5))); // expected-warning{{'unavailable' availability overrides all other availability information}}
 
-void f7() __attribute__((availability(macosx,message=L"wide"))); // expected-error {{expected string literal for optional message in 'availability' attribute}}
+void f7() __attribute__((availability(macosx,message=L"wide"))); // expected-error {{an unevaluated string literal cannot have an encoding pre

[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 375475.
cor3ntin added a comment.

Accept \r as an escape sequence n unevaluated string literal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

Files:
  clang-tools-extra/test/clang-tidy/checkers/modernize-unary-static-assert.cpp
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CXX/dcl.dcl/dcl.link/p2.cpp
  clang/test/CXX/dcl.dcl/p4-0x.cpp
  clang/test/FixIt/fixit-static-assert.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/asm.cpp
  clang/test/Parser/attr-availability-xcore.c
  clang/test/Parser/attr-availability.c
  clang/test/Sema/asm.c
  clang/test/SemaCXX/static-assert.cpp

Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -28,12 +28,12 @@
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
 S s2;
 
-static_assert(false, L"\x"); // expected-error {{static_assert failed L"\x"}}
-static_assert(false, u"\U000317FF"); // expected-error {{static_assert failed u"\U000317FF"}}
+static_assert(false, L"\x"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, u"\U000317FF"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 // FIXME: render this as u8"\u03A9"
-static_assert(false, u8"Ω"); // expected-error {{static_assert failed u8"\316\251"}}
-static_assert(false, L"\u1234"); // expected-error {{static_assert failed L"\x1234"}}
-static_assert(false, L"\x1ff" "0\x123" "fx\xf" "goop"); // expected-error {{static_assert failed L"\x1FF""0\x123""fx\xFgoop"}}
+static_assert(false, u8"Ω"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\u1234"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+static_assert(false, L"\x1ff" "0\x123" "fx\xf" "goop"); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 
 template struct AlwaysFails {
   // Only give one error here.
Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -37,14 +37,17 @@
   asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
   asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
   asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
-  asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} 
+  asm("nop"
+  : "=a"(a)
+  : "b"(b)
+  : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}}
 }
 
 // rdar://6094010
 void test3() {
   int x;
-  asm(L"foo" : "=r"(x)); // expected-error {{wide string}}
-  asm("foo" : L"=r"(x)); // expected-error {{wide string}}
+  asm(L"foo" : "=r"(x)); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
+  asm("foo" : L"=r"(x)); // expected-error {{an unevaluated string literal cannot have an encoding prefix}}
 }
 
 // 
Index: clang/test/Parser/attr-availability.c
===
--- clang/test/Parser/attr-availability.c
+++ clang/test/Parser/attr-availability.c
@@ -18,21 +18,25 @@
 
 void f6() __attribute__((availability(macosx,unavailable,introduced=10.5))); // expected-warning{{'unavailable' availability overrides all other availability information}}
 
-void f7() __attribute__((availability(macosx,message=L"wide"))); // expected-error {{expected string literal for optional message in 'availability' attribute}}
+void f7() __attribute__((availability(macosx,message=L"wide"))); // expected-error {{an unevalu

[clang-tools-extra] 1bcd6b5 - [clangd] Refactor IncludeStructure: use File (unsigned) for most computations

2021-09-27 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2021-09-28T07:44:28+02:00
New Revision: 1bcd6b51a98263d440ff7549070060f7e7b0326a

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

LOG: [clangd] Refactor IncludeStructure: use File (unsigned) for most 
computations

Preparation for D108194.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index 69338814ebdd2..1033e5e92dcde 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1379,14 +1379,16 @@ class CodeCompleteFlow {
   FileDistanceOptions ProxOpts{}; // Use defaults.
   const auto &SM = Recorder->CCSema->getSourceManager();
   llvm::StringMap ProxSources;
-  for (auto &Entry : Includes.includeDepth(
-   SM.getFileEntryForID(SM.getMainFileID())->getName())) {
-auto &Source = ProxSources[Entry.getKey()];
-Source.Cost = Entry.getValue() * ProxOpts.IncludeCost;
+  auto MainFileID =
+  Includes.getOrCreateID(SM.getFileEntryForID(SM.getMainFileID()));
+  for (auto &HeaderIDAndDepth : Includes.includeDepth(MainFileID)) {
+auto &Source =
+ProxSources[Includes.getRealPath(HeaderIDAndDepth.getFirst())];
+Source.Cost = HeaderIDAndDepth.getSecond() * ProxOpts.IncludeCost;
 // Symbols near our transitive includes are good, but only consider
 // things in the same directory or below it. Otherwise there can be
 // many false positives.
-if (Entry.getValue() > 0)
+if (HeaderIDAndDepth.getSecond() > 0)
   Source.MaxUpTraversals = 1;
   }
   FileProximity.emplace(ProxSources, ProxOpts);

diff  --git a/clang-tools-extra/clangd/Headers.cpp 
b/clang-tools-extra/clangd/Headers.cpp
index 111b05849b81b..33304b818d6f4 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -67,8 +67,9 @@ class RecordHeaders : public PPCallbacks {
 // Treat as if included from the main file.
 IncludingFileEntry = SM.getFileEntryForID(MainFID);
   }
-  Out->recordInclude(IncludingFileEntry->getName(), File->getName(),
- File->tryGetRealPathName());
+  auto IncludingID = Out->getOrCreateID(IncludingFileEntry),
+   IncludedID = Out->getOrCreateID(File);
+  Out->IncludeChildren[IncludingID].push_back(IncludedID);
 }
   }
 
@@ -154,38 +155,41 @@ collectIncludeStructureCallback(const SourceManager &SM,
   return std::make_unique(SM, Out);
 }
 
-void IncludeStructure::recordInclude(llvm::StringRef IncludingName,
- llvm::StringRef IncludedName,
- llvm::StringRef IncludedRealName) {
-  auto Child = fileIndex(IncludedName);
-  if (!IncludedRealName.empty() && RealPathNames[Child].empty())
-RealPathNames[Child] = std::string(IncludedRealName);
-  auto Parent = fileIndex(IncludingName);
-  IncludeChildren[Parent].push_back(Child);
+llvm::Optional
+IncludeStructure::getID(const FileEntry *Entry) const {
+  auto It = NameToIndex.find(Entry->getName());
+  if (It == NameToIndex.end())
+return llvm::None;
+  return It->second;
 }
 
-unsigned IncludeStructure::fileIndex(llvm::StringRef Name) {
-  auto R = NameToIndex.try_emplace(Name, RealPathNames.size());
+IncludeStructure::HeaderID
+IncludeStructure::getOrCreateID(const FileEntry *Entry) {
+  auto R = NameToIndex.try_emplace(
+  Entry->getName(),
+  static_cast(RealPathNames.size()));
   if (R.second)
 RealPathNames.emplace_back();
-  return R.first->getValue();
+  IncludeStructure::HeaderID Result = R.first->getValue();
+  std::string &RealPathName = RealPathNames[static_cast(Result)];
+  if (RealPathName.empty())
+RealPathName = Entry->tryGetRealPathName().str();
+  return Result;
 }
 
-llvm::StringMap
-IncludeStructure::includeDepth(llvm::StringRef Root) const {
+llvm::DenseMap
+IncludeStructure::includeDepth(HeaderID Root) const {
   // Include depth 0 is the main file only.
-  llvm::StringMap Result;
+  llvm::DenseMap Result;
+  assert(static_cast(Root) < RealPathNames.size());
   Result[Root] = 0;
-  std::vector CurrentLevel;
-  llvm::DenseSet Seen;
-  auto It = NameToIndex.find(Root);
-  if (It != NameToIndex.end()) {
-CurrentLevel.push_back(It->second);
-Seen.insert(It->second);
-  }
+  std::vector CurrentLevel;
+  CurrentLevel.push_back(Root);

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

2021-09-27 Thread Mahesha S via Phabricator via cfe-commits
hsmhsm added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

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


[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

2021-09-27 Thread Mahesha S via Phabricator via cfe-commits
hsmhsm updated this revision to Diff 375472.
hsmhsm added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/builtin-amdgcn-atomic-inc-dec.cpp
  clang/test/CodeGenCXX/vla.cpp
  clang/test/CodeGenSYCL/address-space-deduction.cpp
  clang/test/OpenMP/amdgcn_target_init_temp_alloca.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/parallel_if_codegen.cpp
  clang/test/OpenMP/parallel_if_codegen_PR51349.cpp
  clang/test/OpenMP/parallel_master_taskloop_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_codegen.cpp
  clang/test/OpenMP/target_codegen_global_capture.cpp
  clang/test/OpenMP/target_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_parallel_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/task_codegen.c
  clang/test/OpenMP/teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/vla_crash.c

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall requested changes to this revision.
rjmccall added a comment.
This revision now requires changes to proceed.

MSVC gets to chose the ABI rules for their platform.  It is not Clang policy to 
pick ABI rules and then break them later.

If you'd like to contribute an implementation of `[[msvc::no_unique_address]]` 
that matches MSVC's ABI,  that would be welcome.  I don't know, maybe that's as 
simple as making it use the existing implementation; @zygoloid might know 
better.  But "add an ABI feature ahead of the platform owner and then fix the 
ABI problems later" is not how we do things.


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

https://reviews.llvm.org/D110485

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread Ivan Garramona via Phabricator via cfe-commits
ivan171 added a comment.

In D110485#3026533 , @rjmccall wrote:

> We should not add a standard feature with ABI impact if we aren't certain 
> we're going to match MSVC's ABI on it.  I'm sorry, but this needs to wait 
> until there's an MSVC release that officially supports this.

This issue 
 on STL's 
repo suggests that they're waiting Clang get support for `no_unique_address` on 
Windows to start using it. Stephan said here 
 MSVC has 
implemented this, but with a different name.


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

https://reviews.llvm.org/D110485

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375468.
expnkx added a comment.

should be diff3


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

https://reviews.llvm.org/D110485

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Preprocessor/has_attribute.cpp
  clang/test/SemaCXX/cxx2a-no-unique-address.cpp


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
+// RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field 
non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to 
non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/test/Preprocessor/has_attribute.cpp
===
--- clang/test/Preprocessor/has_attribute.cpp
+++ clang/test/Preprocessor/has_attribute.cpp
@@ -65,7 +65,7 @@
 // CHECK: likely: 201803L
 // CHECK: maybe_unused: 201603L
 // ITANIUM: no_unique_address: 201803L
-// WINDOWS: no_unique_address: 0
+// WINDOWS: no_unique_address: 201803L
 // CHECK: nodiscard: 201907L
 // CHECK: noreturn: 200809L
 // CHECK: unlikely: 201803L
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr 
{
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
+// RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/test/Preprocessor/has_attribute.cpp
===
--- clang/test/Preprocessor/has_attribute.cpp
+++ clang/test/Preprocessor/has_attribute.cpp
@@ -65,7 +65,7 @@
 // CHECK: likely: 201803L
 // CHECK: maybe_unused: 201603L
 // ITANIUM: no_unique_address: 201803L
-// WINDOWS: no_unique_address: 0
+// WINDOWS: no_unique_address: 201803L
 // CHECK: nodiscard: 201907L
 // CHECK: noreturn: 200809L
 // CHECK: unlikely: 201803L
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr {
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment.

In D110485#3026533 , @rjmccall wrote:

> We should not add a standard feature with ABI impact if we aren't certain 
> we're going to match MSVC's ABI on it.  I'm sorry, but this needs to wait 
> until there's an MSVC release that officially supports this.

You are not actually matching msvc abi's layout because msvc supports 
[[msvc::no_unique_address]]. That is an MSVC release that officially supports.


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

https://reviews.llvm.org/D110485

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment.

In D110485#3026533 , @rjmccall wrote:

> We should not add a standard feature with ABI impact if we aren't certain 
> we're going to match MSVC's ABI on it.  I'm sorry, but this needs to wait 
> until there's an MSVC release that officially supports this.

but how are you going to deal with [[msvc::no_unique_address]] that works?


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

https://reviews.llvm.org/D110485

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375466.
expnkx added a comment.

itanium + microsoft test both


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

https://reviews.llvm.org/D110485

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/SemaCXX/cxx2a-no-unique-address.cpp


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field 
non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to 
non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr 
{
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr {
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D109950: [Clang] Enable IC/IF mode for __ibm128

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:44
 #include "clang/Basic/TargetCXXABI.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/XRayLists.h"

It's a shame to have to do this in such a pervasively-included header as 
`ASTContext.h`.  Can we make `TargetInfo::RealType` an `enum class` at 
namespace scope, so that we can just forward-declare it in this file?

This would also be a good opportunity to rename the enum to something like 
`ModeTypeKind`.

If we do that, it should be a separate patch, so that this can just be the 
`__ibm128` functionality change.



Comment at: clang/lib/Basic/TargetInfo.cpp:300
+if (ExplicitType == Ibm128 || ExplicitType == LongDouble)
+  return ExplicitType;
 break;

`__ibm128` is target-specific, right?  So this should return `NoFloat` if 
someone requests `IF` or `IC` and the target doesn't support it, just like we 
do when the target doesn't support `KF`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109950

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


[PATCH] D109948: [Clang] Enable _Complex __ibm128 type

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:1095
   CanQualType FloatComplexTy, DoubleComplexTy, LongDoubleComplexTy;
-  CanQualType Float128ComplexTy;
+  CanQualType Float128ComplexTy, Ibm128ComplexTy;
   CanQualType VoidPtrTy, NullPtrTy;

Huh.  I don't know why we precompute these.  I mean, it's not a huge start-up 
overhead for the compiler, but it's not nothing, and the use in 
`getFloatingTypeOfSizeWithinDomain` is not at all worth optimizing.

Would you mind writing up a patch that rips all of these out and makes the 
use-sites just build the appropriate complex type as needed?  And then we can 
land a patch for this that just does the one-liner to allow `_Complex __ibm128`.



Comment at: clang/lib/Sema/Sema.cpp:1898
+  (&Sem == &llvm::APFloat::PPCDoubleDouble() &&
+   !Context.getTargetInfo().hasIbm128Type()))
 LongDoubleMismatched = true;

Why is this change in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109948

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


[PATCH] D110281: Change __builtin_sycl_unique_stable_name to just use an Itanium mangling

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Oh, well, that's a huge simplification!  Thank you for pushing on this with the 
language group.  This LGTM.


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

https://reviews.llvm.org/D110281

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


[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I think it would be better to provide a generic ABI specification that is 
designed to "lower" `_BitInt` types into more basic concepts that ABIs 
typically already have rules for.  It would be appropriate to note at the top 
that this is only a generic specification and that the rules for any particular 
platform should be codified in a platform-specific ABI.  But we should make 
that easy so that platform owners who don't have strong opinions about the ABI 
can just say "as described in version 1.4 of the generic ABI at 
https://clang.llvm.org/...";

Now, that is all independent of what we actually decide the ABI should be.  But 
I do think that if we're going to make this available on all targets, we should 
strongly consider doing a "legalizing" lowering in the frontend: at ABI-exposed 
points (call arguments/parameters and memory accesses), `_BitInt` types should 
be translated into types that a naive backend can be trusted to handle in a way 
that matches the documented ABI.  Individual targets can then opt in to using 
the natural lowerings if they promise to match the ABI, but that becomes a 
decision that they make to enable better optimization and code generation, not 
something that's necessary for correctness.




Comment at: clang/docs/ReleaseNotes.rst:131
+  ``_BitInt(N)`` is supported as an extension in older C modes and in all C++
+  modes.
+

Should we document that this is still considered an experimental feature, in 
the sense of not yet officially having a stable ABI?



Comment at: clang/docs/ReleaseNotes.rst:166
+  previously used the ``_ExtInt`` types will now be mangled to instead use the
+  ``_BitInt(N)`` spelling in both the Microsoft and Itanium ABIs.
+

Probably more accurate to now say something like:

```
The ``_ExtInt(N)`` extension has been standardized in C2X as ``_BitInt(N)``.  
The mangling of this type in C++ has accordingly changed: under the Microsoft 
ABI it is now mangled using the ``_BitInt`` spelling, and under the Itanium ABI 
it is now mangled using a dedicated production.
```


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

https://reviews.llvm.org/D108643

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


[PATCH] D110068: [Clang][AST] Resolve FIXME: Remove ObjCObjectPointer from isSpecifierType

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

`ObjCObjectPointerType` used to have a much weirder representation for the 
builtin object types like `id` and `Class`, and it's possible that this was an 
artifact of that.  Otherwise I'm not sure, sorry.


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

https://reviews.llvm.org/D110068

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

We should not add a standard feature with ABI impact if we aren't certain we're 
going to match MSVC's ABI on it.  I'm sorry, this needs to wait until MSVC 
releases a compiler that supports this.


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

https://reviews.llvm.org/D110485

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375461.
expnkx added a comment.

I just find that Preprocessor test has a windows (msvc not gnu) specific test 
that tests no_unique_address.

add that to patch and see whether it is mergable.


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

https://reviews.llvm.org/D110485

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Preprocessor/has_attribute.cpp
  clang/test/SemaCXX/cxx2a-no-unique-address.cpp


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field 
non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to 
non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/test/Preprocessor/has_attribute.cpp
===
--- clang/test/Preprocessor/has_attribute.cpp
+++ clang/test/Preprocessor/has_attribute.cpp
@@ -64,8 +64,7 @@
 // CHECK: fallthrough: 201603L
 // CHECK: likely: 201803L
 // CHECK: maybe_unused: 201603L
-// ITANIUM: no_unique_address: 201803L
-// WINDOWS: no_unique_address: 0
+// CHECK: no_unique_address: 201803L
 // CHECK: nodiscard: 201907L
 // CHECK: noreturn: 200809L
 // CHECK: unlikely: 201803L
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr 
{
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/test/Preprocessor/has_attribute.cpp
===
--- clang/test/Preprocessor/has_attribute.cpp
+++ clang/test/Preprocessor/has_attribute.cpp
@@ -64,8 +64,7 @@
 // CHECK: fallthrough: 201603L
 // CHECK: likely: 201803L
 // CHECK: maybe_unused: 201603L
-// ITANIUM: no_unique_address: 201803L
-// WINDOWS: no_unique_address: 0
+// CHECK: no_unique_address: 201803L
 // CHECK: nodiscard: 201907L
 // CHECK: noreturn: 200809L
 // CHECK: unlikely: 201803L
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr {
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [clang] 76d845c - [clang-format] Fix unittest failures with -Werror

2021-09-27 Thread David Blaikie via cfe-commits
FWIW, a common way to address this issue is to use the 'unsigned' suffix,
as in:
EXPECT_EQ(x, 5u);

On Thu, Sep 23, 2021 at 4:24 PM Nemanja Ivanovic via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Nemanja Ivanovic
> Date: 2021-09-23T18:24:39-05:00
> New Revision: 76d845cb169f048cb6f2176c3e7a6534dc5af097
>
> URL:
> https://github.com/llvm/llvm-project/commit/76d845cb169f048cb6f2176c3e7a6534dc5af097
> DIFF:
> https://github.com/llvm/llvm-project/commit/76d845cb169f048cb6f2176c3e7a6534dc5af097.diff
>
> LOG: [clang-format] Fix unittest failures with -Werror
>
> Commit a44ab1702539 added a unit test that fails to build with
> -Werror which causes build bot breaks on bots that include that
> option in their build. This patch just adds the necessary casts to
> silence the warnings.
>
> Added:
>
>
> Modified:
> clang/unittests/Format/QualifierFixerTest.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/unittests/Format/QualifierFixerTest.cpp
> b/clang/unittests/Format/QualifierFixerTest.cpp
> index 1bb1792113049..0592cef1eaae5 100755
> --- a/clang/unittests/Format/QualifierFixerTest.cpp
> +++ b/clang/unittests/Format/QualifierFixerTest.cpp
> @@ -554,7 +554,7 @@ TEST_F(QualifierFixerTest,
> ConstVolatileQualifiersOrder) {
>Style.QualifierOrder = {"inline", "static", "const", "volatile",
> "type"};
>
>// The Default
> -  EXPECT_EQ(Style.QualifierOrder.size(), 5);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)5);
>
>verifyFormat("const volatile int a;", "const volatile int a;", Style);
>verifyFormat("const volatile int a;", "volatile const int a;", Style);
> @@ -603,7 +603,7 @@ TEST_F(QualifierFixerTest, InlineStatics) {
>FormatStyle Style = getLLVMStyle();
>Style.QualifierAlignment = FormatStyle::QAS_Left;
>Style.QualifierOrder = {"inline", "static", "const", "volatile",
> "type"};
> -  EXPECT_EQ(Style.QualifierOrder.size(), 5);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)5);
>
>verifyFormat("inline static const volatile int a;",
> "const inline static volatile int a;", Style);
> @@ -621,7 +621,7 @@ TEST_F(QualifierFixerTest, AmpEqual) {
>FormatStyle Style = getLLVMStyle();
>Style.QualifierAlignment = FormatStyle::QAS_Custom;
>Style.QualifierOrder = {"static", "type", "const"};
> -  EXPECT_EQ(Style.QualifierOrder.size(), 3);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)3);
>
>verifyFormat("foo(std::string const & = std::string()) const",
> "foo(const std::string & = std::string()) const", Style);
> @@ -634,7 +634,7 @@ TEST_F(QualifierFixerTest, MoveConstBeyondTypeSmall) {
>FormatStyle Style = getLLVMStyle();
>Style.QualifierAlignment = FormatStyle::QAS_Custom;
>Style.QualifierOrder = {"type", "const"};
> -  EXPECT_EQ(Style.QualifierOrder.size(), 2);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)2);
>
>verifyFormat("int const a;", "const int a;", Style);
>verifyFormat("int const *a;", "const int*a;", Style);
> @@ -648,7 +648,7 @@ TEST_F(QualifierFixerTest, MoveConstBeforeTypeSmall) {
>FormatStyle Style = getLLVMStyle();
>Style.QualifierAlignment = FormatStyle::QAS_Custom;
>Style.QualifierOrder = {"const", "type"};
> -  EXPECT_EQ(Style.QualifierOrder.size(), 2);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)2);
>
>verifyFormat("const int a;", "int const a;", Style);
>verifyFormat("const int *a;", "int const *a;", Style);
> @@ -670,7 +670,7 @@ TEST_F(QualifierFixerTest, MoveConstBeyondType) {
>FormatStyle Style = getLLVMStyle();
>Style.QualifierAlignment = FormatStyle::QAS_Custom;
>Style.QualifierOrder = {"static", "inline", "type", "const",
> "volatile"};
> -  EXPECT_EQ(Style.QualifierOrder.size(), 5);
> +  EXPECT_EQ(Style.QualifierOrder.size(), (size_t)5);
>
>verifyFormat("static inline int const volatile a;",
> "const inline static volatile int a;", Style);
> @@ -698,8 +698,8 @@ TEST_F(QualifierFixerTest, PrepareLeftRightOrdering) {
>QualifierAlignmentFixer::PrepareLeftRightOrdering(Style.QualifierOrder,
> Left,
>  Right,
> ConfiguredTokens);
>
> -  EXPECT_EQ(Left.size(), 2);
> -  EXPECT_EQ(Right.size(), 2);
> +  EXPECT_EQ(Left.size(), (size_t)2);
> +  EXPECT_EQ(Right.size(), (size_t)2);
>
>std::vector LeftResult = {"inline", "static"};
>std::vector RightResult = {"const", "volatile"};
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D110358: [WIP][OpenMP][DeviceRTL] Add the initial support for SIMD execution (NOT FOR REVIEW)

2021-09-27 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 updated this revision to Diff 375457.
tianshilei1992 added a comment.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

When it is `target simd`:

- outlined function can be emitted.
- function call to `__kmpc_simd_51` can be emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110358

Files:
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/DeviceRTL/include/Interface.h
  openmp/libomptarget/DeviceRTL/include/Mapping.h
  openmp/libomptarget/DeviceRTL/include/State.h
  openmp/libomptarget/DeviceRTL/include/Synchronization.h
  openmp/libomptarget/DeviceRTL/include/Types.h
  openmp/libomptarget/DeviceRTL/src/Kernel.cpp
  openmp/libomptarget/DeviceRTL/src/Mapping.cpp
  openmp/libomptarget/DeviceRTL/src/Parallelism.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp
  openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
  openmp/libomptarget/DeviceRTL/src/Workshare.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

Index: openmp/libomptarget/plugins/cuda/src/rtl.cpp
===
--- openmp/libomptarget/plugins/cuda/src/rtl.cpp
+++ openmp/libomptarget/plugins/cuda/src/rtl.cpp
@@ -1084,20 +1084,31 @@
 KernelTy *KernelInfo = reinterpret_cast(TgtEntryPtr);
 
 const bool IsSPMDGenericMode =
-KernelInfo->ExecutionMode == llvm::omp::OMP_TGT_EXEC_MODE_GENERIC_SPMD;
+KernelInfo->ExecutionMode & llvm::omp::OMP_TGT_EXEC_MODE_GENERIC_SPMD;
 const bool IsSPMDMode =
-KernelInfo->ExecutionMode == llvm::omp::OMP_TGT_EXEC_MODE_SPMD;
+!IsSPMDGenericMode &&
+(KernelInfo->ExecutionMode & llvm::omp::OMP_TGT_EXEC_MODE_SPMD);
 const bool IsGenericMode =
-KernelInfo->ExecutionMode == llvm::omp::OMP_TGT_EXEC_MODE_GENERIC;
+!IsSPMDGenericMode &&
+(KernelInfo->ExecutionMode & llvm::omp::OMP_TGT_EXEC_MODE_GENERIC);
+const bool IsSIMDMode =
+KernelInfo->ExecutionMode & llvm::omp::OMP_TGT_EXEC_MODE_SIMD;
 
 int CudaThreadsPerBlock;
 if (ThreadLimit > 0) {
-  DP("Setting CUDA threads per block to requested %d\n", ThreadLimit);
-  CudaThreadsPerBlock = ThreadLimit;
-  // Add master warp if necessary
-  if (IsGenericMode) {
-DP("Adding master warp: +%d threads\n", DeviceData[DeviceId].WarpSize);
-CudaThreadsPerBlock += DeviceData[DeviceId].WarpSize;
+  if (IsSIMDMode) {
+DP("Setting CUDA threads per block to requested %d\n",
+   ThreadLimit * DeviceData[DeviceId].WarpSize);
+CudaThreadsPerBlock = ThreadLimit * DeviceData[DeviceId].WarpSize;
+  } else {
+DP("Setting CUDA threads per block to requested %d\n", ThreadLimit);
+CudaThreadsPerBlock = ThreadLimit;
+// Add master warp if necessary
+if (IsGenericMode) {
+  DP("Adding master warp: +%d threads\n",
+ DeviceData[DeviceId].WarpSize);
+  CudaThreadsPerBlock += DeviceData[DeviceId].WarpSize;
+}
   }
 } else {
   DP("Setting CUDA threads per block to default %d\n",
Index: openmp/libomptarget/DeviceRTL/src/Workshare.cpp
===
--- openmp/libomptarget/DeviceRTL/src/Workshare.cpp
+++ openmp/libomptarget/DeviceRTL/src/Workshare.cpp
@@ -210,7 +210,7 @@
   static void dispatch_init(IdentTy *loc, int32_t threadId,
 kmp_sched_t schedule, T lb, T ub, ST st, ST chunk,
 DynamicScheduleTracker *DST) {
-int tid = mapping::getThreadIdInBlock();
+int tid = mapping::getLogicThreadId();
 T tnum = omp_get_num_threads();
 T tripCount = ub - lb + 1; // +1 because ub is inclusive
 ASSERT0(LT_FUSSY, threadId < tnum,
Index: openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
===
--- openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
+++ openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
@@ -214,8 +214,8 @@
 
 } // namespace impl
 
-void synchronize::init(bool IsSPMD) {
-  if (!IsSPMD)
+void synchronize::init(int8_t Mode) {
+  if (!mapping::utils::isSPMDMode(Mode) || mapping::utils::isSIMDMode(Mode))
 impl::namedBarrierInit();
 }
 
Index: openmp/libomptarget/DeviceRTL/src/State.cpp
===
--- openmp/libomptarget/DeviceRTL/src/State.cpp
+++ openmp/libomptarget/DeviceRTL/src/State.cpp
@@ -203,7 +203,7 @@
 
 struct TeamStateTy {
   /// TODO: provide a proper init function.
-  void init(bool IsSPMD);
+  void init(int Mode);
 
   bool op

[PATCH] D110607: [Inliner] Attempt to inline calls to alwaysinline functions first

2021-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I'll update the tests if this lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110607

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


[PATCH] D110607: [Inliner] Attempt to inline calls to alwaysinline functions first

2021-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added a reviewer: mtrofin.
Herald added subscribers: ormris, hiraditya, eraman.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

For PR46945 we added an extra alwaysinline inliner pass that runs before
the normal inliner pass. This was to fix the issue where we have two
mutually recursive functions, one marked alwaysinline, but due to inline
ordering, the other function may have been inlined into the alwaysinline
function first, causing the alwaysinline function to not be inlined into
the other function.

Adding a whole new pass is quite a bit of work. Rather, we can just
choose to handle alwaysinline calls first before inlining normal calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110607

Files:
  clang/test/Frontend/optimization-remark-line-directive.c
  clang/test/Frontend/optimization-remark-new-pm.c
  clang/test/Frontend/optimization-remark-with-hotness-new-pm.c
  clang/test/Frontend/optimization-remark.c
  llvm/include/llvm/Analysis/InlineAdvisor.h
  llvm/include/llvm/Analysis/InlineOrder.h
  llvm/include/llvm/Transforms/IPO/Inliner.h
  llvm/lib/Analysis/InlineAdvisor.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-print-pipeline.ll
  llvm/test/Transforms/Inline/inline_nossp.ll
  llvm/test/Transforms/Inline/inline_stats.ll
  llvm/test/Transforms/Inline/no-inline-line-tables.ll
  llvm/test/Transforms/Inline/optimization-remarks-with-hotness.ll
  llvm/test/Transforms/Inline/optimization-remarks.ll

Index: llvm/test/Transforms/Inline/optimization-remarks.ll
===
--- llvm/test/Transforms/Inline/optimization-remarks.ll
+++ llvm/test/Transforms/Inline/optimization-remarks.ll
@@ -19,13 +19,6 @@
 ; RUN:   -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \
 ; RUN:   FileCheck -check-prefixes=CHECK,HOTNESS,ALWAYS %s
 
-; RUN: opt < %s -passes=inliner-wrapper-no-mandatory-first -pass-remarks=inline -pass-remarks-missed=inline \
-; RUN:   -pass-remarks-analysis=inline -S 2>&1 | \
-; RUN:   FileCheck -check-prefixes=CHECK,NO_HOTNESS,ALWAYS %s
-; RUN: opt < %s -passes=inliner-wrapper-no-mandatory-first -pass-remarks=inline -pass-remarks-missed=inline \
-; RUN:   -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \
-; RUN:   FileCheck -check-prefixes=CHECK,HOTNESS,ALWAYS %s
-
 ; HOTNESS-DAG: fox will not be inlined into bar because its definition is unavailable
 ; NO_HOTNESS-NOT: fox will not be inlined into bar because its definition is unavailable
 ; ALWAYS-DAG: 'foo' inlined into 'bar' with (cost=always): always inline attribute
Index: llvm/test/Transforms/Inline/optimization-remarks-with-hotness.ll
===
--- llvm/test/Transforms/Inline/optimization-remarks-with-hotness.ll
+++ llvm/test/Transforms/Inline/optimization-remarks-with-hotness.ll
@@ -4,9 +4,6 @@
 ; RUN: opt < %s -passes=inline -pass-remarks=inline -pass-remarks-missed=inline \
 ; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 \
 ; RUN: | FileCheck %s
-; RUN: opt < %s -passes=inliner-wrapper-no-mandatory-first -pass-remarks=inline -pass-remarks-missed=inline \
-; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 \
-; RUN: | FileCheck %s
 
 ; CHECK: 'foo' inlined into 'bar' with (cost=always): always inline attribute (hotness: 30)
 ; CHECK: 'foz' not inlined into 'bar' because it should never be inlined (cost=never): noinline function attribute (hotness: 30)
Index: llvm/test/Transforms/Inline/no-inline-line-tables.ll
===
--- llvm/test/Transforms/Inline/no-inline-line-tables.ll
+++ llvm/test/Transforms/Inline/no-inline-line-tables.ll
@@ -22,17 +22,17 @@
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 
 ; Function Attrs: alwaysinline nounwind
-define i32 @g(i32 %x) #0 !dbg !15 {
+define i32 @g(i32 %y) #0 !dbg !15 {
 entry:
-  %x.addr = alloca i32, align 4
-  store i32 %x, i32* %x.addr, align 4
-  call void @llvm.dbg.declare(metadata i32* %x.addr, metadata !16, metadata !DIExpression()), !dbg !17
+  %y.addr = alloca i32, align 4
+  store i32 %y, i32* %y.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %y.addr, metadata !16, metadata !DIExpression()), !dbg !17
   br label %L, !dbg !17
 
 L:; preds = %entry
   call void @llvm.dbg.label(metadata !18), !dbg !19
-  store i32 42, i32* %x.addr, align 4, !dbg !20
-  %0 = load i32, i32* %x.addr, align 4, !dbg !21
+  store i32 42, i32* %y.addr, align 4, !dbg !20
+  %0 = load i32, i32* %y.addr, align 4, !dbg !21
   ret i32 %0, !dbg !21
 }
 
@@ -54,7 +54,7 @@
 ; st

[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx added a comment.

ninja: error: unknown target 'check-libc', did you mean 'check-lit'?

what's wrong with this?


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

https://reviews.llvm.org/D110485

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


[PATCH] D110599: [test] Don't run optimization-remark.c against the legacy PM

2021-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 375445.
aeubanks added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110599

Files:
  clang/test/Frontend/optimization-remark.c


Index: clang/test/Frontend/optimization-remark.c
===
--- clang/test/Frontend/optimization-remark.c
+++ clang/test/Frontend/optimization-remark.c
@@ -1,36 +1,30 @@
 // This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
-// and -Rpass-analysis) with the inliner. The test is designed to
-// always trigger the inliner, so it should be independent of the
-// optimization level (under the legacy PM). The inliner is not added to the 
new
-// PM pipeline unless optimizations are present.
-
-// The inliner for the new PM does not seem to be enabled at O0, but we still
-// get the same remarks with at least O1. The remarks are also slightly
-// different and located in another test file.
-// RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline 
-Rpass-missed=inline -O0 -fno-experimental-new-pass-manager -emit-llvm-only 
-verify
-// RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline 
-Rpass-missed=inline -O0 -fno-experimental-new-pass-manager -emit-llvm-only 
-debug-info-kind=line-tables-only -verify
-// RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>/dev/null | FileCheck %s
+// and -Rpass-analysis) with the inliner. We may not consider every call to be
+// inlined (new PM + -O0) so these tests run at -O1.
+
+// FIXME: emit always_inline remarks for -O1 even with -mllvm 
-mandatory-inlining-first=true
+
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rpass-analysis=inline -Rpass-missed=inline -emit-llvm-only -verify
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rpass-analysis=inline -Rpass-missed=inline -emit-llvm-only 
-debug-info-kind=line-tables-only -verify
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>/dev/null | FileCheck %s
 //
 // Check that we can override -Rpass= with -Rno-pass.
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -Rno-pass -emit-llvm -o - 2>&1 | FileCheck 
%s --check-prefix=CHECK-NO-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager 
-Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
-//
-// The inliner for the new PM does not seem to be enabled at O0, but we still
-// get the same remarks with at least O1.
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 
-emit-llvm -mllvm -mandatory-inlining-first=false -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 
-Rno-everything -Reverything -emit-llvm -mllvm -mandatory-inlining-first=false 
-o -  2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -Reverything -emit-llvm -o -  2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
 //
 // Check that -w doesn't disable remarks.
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager -w 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 -w 
-emit-llvm -mllvm -mandatory-inlining-first=false -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
 //
 // -Reverything implies -Rpass=.*.
-// RUN: %clang_cc1 %s -Reverything -emit-llvm -o - 2>&1 | FileChe

[PATCH] D107647: [PowerPC] MMA - Add __builtin_vsx_build_pair and __builtin_mma_build_acc builtins

2021-09-27 Thread Ahsan Saghir 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 rG593b074a096c: [PowerPC] MMA - Add __builtin_vsx_build_pair 
and __builtin_mma_build_acc… (authored by saghir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107647

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-build-pair-mma.c
  clang/test/Sema/ppc-pair-mma-types.c
  clang/test/SemaCXX/ppc-pair-mma-types.cpp

Index: clang/test/SemaCXX/ppc-pair-mma-types.cpp
===
--- clang/test/SemaCXX/ppc-pair-mma-types.cpp
+++ clang/test/SemaCXX/ppc-pair-mma-types.cpp
@@ -370,6 +370,7 @@
 return *vpp; // expected-error {{invalid use of PPC MMA type}}
   };
   auto f3 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_assemble_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
+  auto f4 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_build_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
 }
 
 // cast
Index: clang/test/Sema/ppc-pair-mma-types.c
===
--- clang/test/Sema/ppc-pair-mma-types.c
+++ clang/test/Sema/ppc-pair-mma-types.c
@@ -249,6 +249,7 @@
   __vector_pair vp1 = *vpp;
   __vector_pair vp2;
   __builtin_vsx_assemble_pair(&vp2, vc, vc);
+  __builtin_vsx_build_pair(&vp2, vc, vc);
   __vector_pair vp3;
   __vector_quad vq;
   __builtin_mma_xvf64ger(&vq, vp3, vc);
Index: clang/test/CodeGen/builtins-ppc-build-pair-mma.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-build-pair-mma.c
@@ -0,0 +1,51 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O3 -triple powerpc64le-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-LE
+// RUN: %clang_cc1 -O3 -triple powerpc64-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-BE
+
+// CHECK-LE-LABEL: @test1(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC4:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-LE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-LE-NEXT:ret void
+//
+// CHECK-BE-LABEL: @test1(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC4:%.*]])
+// CHECK-BE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-BE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-BE-NEXT:ret void
+//
+void test1(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1, vector unsigned char vc2,
+vector unsigned char vc3, vector unsigned char vc4, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_quad res;
+  __builtin_mma_build_acc(&res, vc1, vc2, vc3, vc4);
+  *((__vector_quad *)resp) = res;
+}
+
+// CHECK-LE-LABEL: @test2(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-LE-NEXT:store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-LE-NEXT:ret void
+//
+// CHECK-BE-LABEL: @test2(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:[[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]])
+// CHECK-BE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-BE-NEXT:store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-BE-NEXT:ret void
+//
+void test2(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1,
+vector unsigned char vc2, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_pair res;
+  __builtin_vsx_build_pair(&res, vc1, vc2);
+  *((__vector_pair *)resp) = res;
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -15910,6 +15910,17 @@
   }
   return Call;
 }
+if (BuiltinID == PP

[clang] 593b074 - [PowerPC] MMA - Add __builtin_vsx_build_pair and __builtin_mma_build_acc builtins

2021-09-27 Thread Ahsan Saghir via cfe-commits

Author: Ahsan Saghir
Date: 2021-09-27T19:51:28-05:00
New Revision: 593b074a096c542c6753916caa754f321a111f21

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

LOG: [PowerPC] MMA - Add __builtin_vsx_build_pair and __builtin_mma_build_acc 
builtins

This patch adds the following built-ins:

__builtin_vsx_build_pair
__builtin_mma_build_acc

Reviewed By: #powerpc, nemanjai, lei

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

Added: 
clang/test/CodeGen/builtins-ppc-build-pair-mma.c

Modified: 
clang/include/clang/Basic/BuiltinsPPC.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/Sema/ppc-pair-mma-types.c
clang/test/SemaCXX/ppc-pair-mma-types.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsPPC.def 
b/clang/include/clang/Basic/BuiltinsPPC.def
index f3f8614f21ca0..ba865def5b48a 100644
--- a/clang/include/clang/Basic/BuiltinsPPC.def
+++ b/clang/include/clang/Basic/BuiltinsPPC.def
@@ -817,6 +817,8 @@ CUSTOM_BUILTIN(mma_lxvp, vsx_lxvp, "W256SLiW256C*", false)
 CUSTOM_BUILTIN(mma_stxvp, vsx_stxvp, "vW256SLiW256C*", false)
 CUSTOM_BUILTIN(mma_assemble_pair, vsx_assemble_pair, "vW256*VV", false)
 CUSTOM_BUILTIN(mma_disassemble_pair, vsx_disassemble_pair, "vv*W256*", false)
+CUSTOM_BUILTIN(vsx_build_pair, vsx_assemble_pair,  "vW256*VV", false)
+CUSTOM_BUILTIN(mma_build_acc, mma_assemble_acc, "vW512*", false)
 
 // UNALIASED_CUSTOM_BUILTIN macro is used for built-ins that have
 // the same name as that of the intrinsic they generate, i.e. the

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 23ab2aee77914..cd08f5ffe6411 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -15910,6 +15910,17 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
   }
   return Call;
 }
+if (BuiltinID == PPC::BI__builtin_vsx_build_pair ||
+BuiltinID == PPC::BI__builtin_mma_build_acc) {
+  // Reverse the order of the operands for LE, so the
+  // same builtin call can be used on both LE and BE
+  // without the need for the programmer to swap operands.
+  // The operands are reversed starting from the second argument,
+  // the first operand is the pointer to the pair/accumulator
+  // that is being built.
+  if (getTarget().isLittleEndian())
+std::reverse(Ops.begin() + 1, Ops.end());
+}
 bool Accumulate;
 switch (BuiltinID) {
   #define CUSTOM_BUILTIN(Name, Intr, Types, Acc) \

diff  --git a/clang/test/CodeGen/builtins-ppc-build-pair-mma.c 
b/clang/test/CodeGen/builtins-ppc-build-pair-mma.c
new file mode 100644
index 0..5f00e6a506f59
--- /dev/null
+++ b/clang/test/CodeGen/builtins-ppc-build-pair-mma.c
@@ -0,0 +1,51 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O3 -triple powerpc64le-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-LE
+// RUN: %clang_cc1 -O3 -triple powerpc64-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-BE
+
+// CHECK-LE-LABEL: @test1(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> 
@llvm.ppc.mma.assemble.acc(<16 x i8> [[VC4:%.*]], <16 x i8> [[VC3:%.*]], <16 x 
i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-LE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 
64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-LE-NEXT:ret void
+//
+// CHECK-BE-LABEL: @test1(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> 
@llvm.ppc.mma.assemble.acc(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]], <16 x 
i8> [[VC3:%.*]], <16 x i8> [[VC4:%.*]])
+// CHECK-BE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-BE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 
64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-BE-NEXT:ret void
+//
+void test1(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1, 
vector unsigned char vc2,
+vector unsigned char vc3, vector unsigned char vc4, unsigned char 
*resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_quad res;
+  __builtin_mma_build_acc(&res, vc1, vc2, vc3, vc4);
+  *((__vector_quad *)resp) = res;
+}
+
+// CHECK-LE-LABEL: @test2(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <256 x i1> 
@llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-LE-NEXT:store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 
32, !

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-09-27 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:470
+  ExpectedHint{": S1", "x"},
+  ExpectedHint{": S2::Inner", "y"});
 }

mizvekov wrote:
> nridge wrote:
> > mizvekov wrote:
> > > mizvekov wrote:
> > > > nridge wrote:
> > > > > This test expectation change is suspicious. The type is being printed 
> > > > > with `PrintingPolicy::SuppressScope=true`, shouldn't that still be 
> > > > > respected?
> > > > Thanks for pointing that out, I will take a look, but I suspect this to 
> > > > be some TypePrinter issue.
> > > Could you explain to me why the former behavior is more desirable here?
> > > 
> > > I initially understood that this hint should provide the type written in 
> > > a form that would actually work if it replaced the 'auto'.
> > > It is strange, but if it is just meant as a hint, it's still fine I guess.
> > > 
> > > Or maybe this was broken in the first place and just was just missing a 
> > > FIXME?
> > The type is being printed with a `PrintingPolicy` which has the 
> > [non-default SuppressScope flag 
> > set](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang-tools-extra/clangd/InlayHints.cpp#34).
> > 
> > The 
> > [documentation](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang/include/clang/AST/PrettyPrinter.h#129)
> >  of this flag says "Suppresses printing of scope specifiers", which I 
> > understand to mean both namespace and class scope specifiers.
> > 
> > If you're asking why we are using this flag for inlay hints, it's to keep 
> > the hints short so they don't take up too much horizontal space. Since the 
> > hints are displayed inline, hints that are too long can result in the line 
> > of code visually extending past the width of the editor window, and we try 
> > to minimize the impact of this.
> I see.
> 
> The problem is that the ElaboratedType sugar does not respect this when 
> printing the nested name specifier it contains.
> The quick fix of trying to make it respect it shows that there are a bunch of 
> tests that expect it not to.
> It seems that specific policy is used in places that need it to recover the 
> type as written.
> 
> So if we want this behavior, we probably need to extend the printing policy 
> with a new flag here.
Thanks for the explanation!

I think this test expectation change is fine then, and would just suggest 
adding a "// FIXME: SuppressScope is not respected in this case" comment 
perhaps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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


[PATCH] D107647: [PowerPC] MMA - Add __builtin_vsx_build_pair and __builtin_mma_build_acc builtins

2021-09-27 Thread Ahsan Saghir via Phabricator via cfe-commits
saghir updated this revision to Diff 375443.
saghir added a comment.

Addressed review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107647

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-build-pair-mma.c
  clang/test/Sema/ppc-pair-mma-types.c
  clang/test/SemaCXX/ppc-pair-mma-types.cpp

Index: clang/test/SemaCXX/ppc-pair-mma-types.cpp
===
--- clang/test/SemaCXX/ppc-pair-mma-types.cpp
+++ clang/test/SemaCXX/ppc-pair-mma-types.cpp
@@ -370,6 +370,7 @@
 return *vpp; // expected-error {{invalid use of PPC MMA type}}
   };
   auto f3 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_assemble_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
+  auto f4 = [](vector unsigned char vc) { __vector_pair vp; __builtin_vsx_build_pair(&vp, vc, vc); return vp; }; // expected-error {{invalid use of PPC MMA type}}
 }
 
 // cast
Index: clang/test/Sema/ppc-pair-mma-types.c
===
--- clang/test/Sema/ppc-pair-mma-types.c
+++ clang/test/Sema/ppc-pair-mma-types.c
@@ -249,6 +249,7 @@
   __vector_pair vp1 = *vpp;
   __vector_pair vp2;
   __builtin_vsx_assemble_pair(&vp2, vc, vc);
+  __builtin_vsx_build_pair(&vp2, vc, vc);
   __vector_pair vp3;
   __vector_quad vq;
   __builtin_mma_xvf64ger(&vq, vp3, vc);
Index: clang/test/CodeGen/builtins-ppc-build-pair-mma.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-build-pair-mma.c
@@ -0,0 +1,51 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O3 -triple powerpc64le-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-LE
+// RUN: %clang_cc1 -O3 -triple powerpc64-unknown-unknown -target-cpu pwr10 \
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-BE
+
+// CHECK-LE-LABEL: @test1(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC4:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-LE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-LE-NEXT:ret void
+//
+// CHECK-BE-LABEL: @test1(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:[[TMP0:%.*]] = tail call <512 x i1> @llvm.ppc.mma.assemble.acc(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]], <16 x i8> [[VC3:%.*]], <16 x i8> [[VC4:%.*]])
+// CHECK-BE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <512 x i1>*
+// CHECK-BE-NEXT:store <512 x i1> [[TMP0]], <512 x i1>* [[TMP1]], align 64, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-BE-NEXT:ret void
+//
+void test1(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1, vector unsigned char vc2,
+vector unsigned char vc3, vector unsigned char vc4, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_quad res;
+  __builtin_mma_build_acc(&res, vc1, vc2, vc3, vc4);
+  *((__vector_quad *)resp) = res;
+}
+
+// CHECK-LE-LABEL: @test2(
+// CHECK-LE-NEXT:  entry:
+// CHECK-LE-NEXT:[[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC2:%.*]], <16 x i8> [[VC1:%.*]])
+// CHECK-LE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-LE-NEXT:store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-LE-NEXT:ret void
+//
+// CHECK-BE-LABEL: @test2(
+// CHECK-BE-NEXT:  entry:
+// CHECK-BE-NEXT:[[TMP0:%.*]] = tail call <256 x i1> @llvm.ppc.vsx.assemble.pair(<16 x i8> [[VC1:%.*]], <16 x i8> [[VC2:%.*]])
+// CHECK-BE-NEXT:[[TMP1:%.*]] = bitcast i8* [[RESP:%.*]] to <256 x i1>*
+// CHECK-BE-NEXT:store <256 x i1> [[TMP0]], <256 x i1>* [[TMP1]], align 32, !tbaa [[TBAA6:![0-9]+]]
+// CHECK-BE-NEXT:ret void
+//
+void test2(unsigned char *vqp, unsigned char *vpp, vector unsigned char vc1,
+vector unsigned char vc2, unsigned char *resp) {
+  __vector_quad vq = *((__vector_quad *)vqp);
+  __vector_pair vp = *((__vector_pair *)vpp);
+  __vector_pair res;
+  __builtin_vsx_build_pair(&res, vc1, vc2);
+  *((__vector_pair *)resp) = res;
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -15910,6 +15910,17 @@
   }
   return Call;
 }
+if (BuiltinID == PPC::BI__builtin_vsx_build_pair ||
+BuiltinID == PPC::BI__builtin_mma_build_acc) {
+  // Reverse the order of the operands for LE, so the
+  // same

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added a comment.

> "as binder can NOT discard a subset of a csect."?

Thanks, this does looks better!

> The linker (binder) doesn't have to discard a subset of csect, but it should 
> ensure there are not two non-local symbols with the same name.
> The latter is what I request.



> Without section deduplication we just end up with PDP-11 a.out extended with 
> .weak directive, it isn't too bad. Mach-O uses this model as well.
> (PE-COFF/ELF properly deduplicates in the absence of section based GC.)
> Having two non-local symbols could cause serious symbol resolution problems.

That I can definitely forward to AIX binder team, but we can't expect they 
implement it right away for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-09-27 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:470
+  ExpectedHint{": S1", "x"},
+  ExpectedHint{": S2::Inner", "y"});
 }

nridge wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > nridge wrote:
> > > > This test expectation change is suspicious. The type is being printed 
> > > > with `PrintingPolicy::SuppressScope=true`, shouldn't that still be 
> > > > respected?
> > > Thanks for pointing that out, I will take a look, but I suspect this to 
> > > be some TypePrinter issue.
> > Could you explain to me why the former behavior is more desirable here?
> > 
> > I initially understood that this hint should provide the type written in a 
> > form that would actually work if it replaced the 'auto'.
> > It is strange, but if it is just meant as a hint, it's still fine I guess.
> > 
> > Or maybe this was broken in the first place and just was just missing a 
> > FIXME?
> The type is being printed with a `PrintingPolicy` which has the [non-default 
> SuppressScope flag 
> set](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang-tools-extra/clangd/InlayHints.cpp#34).
> 
> The 
> [documentation](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang/include/clang/AST/PrettyPrinter.h#129)
>  of this flag says "Suppresses printing of scope specifiers", which I 
> understand to mean both namespace and class scope specifiers.
> 
> If you're asking why we are using this flag for inlay hints, it's to keep the 
> hints short so they don't take up too much horizontal space. Since the hints 
> are displayed inline, hints that are too long can result in the line of code 
> visually extending past the width of the editor window, and we try to 
> minimize the impact of this.
I see.

The problem is that the ElaboratedType sugar does not respect this when 
printing the nested name specifier it contains.
The quick fix of trying to make it respect it shows that there are a bunch of 
tests that expect it not to.
It seems that specific policy is used in places that need it to recover the 
type as written.

So if we want this behavior, we probably need to extend the printing policy 
with a new flag here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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


[PATCH] D110600: [clang-tidy] Fix add_new_check.py to generate correct list.rst autofix column from relative path

2021-09-27 Thread Matt Beardsley via Phabricator via cfe-commits
mattbeardsley created this revision.
mattbeardsley added a reviewer: kbobyrev.
Herald added a subscriber: xazax.hun.
mattbeardsley requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Previously, the code in add_new_check.py that looks for fixit keywords in check 
source files when generating list.rst assumed that the script would only be 
called from its own path. That means it doesn't find any source files for the 
checks it's attempting to scan for, and it defaults to writing out nothing in 
the "Offers fixes" column for all checks. Other parts of add_new_check.py work 
from other paths, just not this part.

After this fix, add_new_check.py's "offers fixes" column generation for 
list.rst will be consistent regardless of what path it's called from by using 
the caller path that's deduced elsewhere already from sys.argv[0].


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110600

Files:
  clang-tools-extra/clang-tidy/add_new_check.py


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -323,13 +323,13 @@
   def has_auto_fix(check_name):
 dirname, _, check_name = check_name.partition("-")
 
-checkerCode = get_actual_filename(dirname,
-  get_camel_name(check_name) + '.cpp')
+checker_code = get_actual_filename(os.path.join(clang_tidy_path, dirname),
+   get_camel_name(check_name) + '.cpp')
 
-if not os.path.isfile(checkerCode):
+if not os.path.isfile(checker_code):
   return ""
 
-with io.open(checkerCode, encoding='utf8') as f:
+with io.open(checker_code, encoding='utf8') as f:
   code = f.read()
   if 'FixItHint' in code or "ReplacementText" in code or "fixit" in code:
 # Some simple heuristics to figure out if a checker has an autofix or 
not.


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -323,13 +323,13 @@
   def has_auto_fix(check_name):
 dirname, _, check_name = check_name.partition("-")
 
-checkerCode = get_actual_filename(dirname,
-  get_camel_name(check_name) + '.cpp')
+checker_code = get_actual_filename(os.path.join(clang_tidy_path, dirname),
+   get_camel_name(check_name) + '.cpp')
 
-if not os.path.isfile(checkerCode):
+if not os.path.isfile(checker_code):
   return ""
 
-with io.open(checkerCode, encoding='utf8') as f:
+with io.open(checker_code, encoding='utf8') as f:
   code = f.read()
   if 'FixItHint' in code or "ReplacementText" in code or "fixit" in code:
 # Some simple heuristics to figure out if a checker has an autofix or not.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-09-27 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:470
+  ExpectedHint{": S1", "x"},
+  ExpectedHint{": S2::Inner", "y"});
 }

mizvekov wrote:
> mizvekov wrote:
> > nridge wrote:
> > > This test expectation change is suspicious. The type is being printed 
> > > with `PrintingPolicy::SuppressScope=true`, shouldn't that still be 
> > > respected?
> > Thanks for pointing that out, I will take a look, but I suspect this to be 
> > some TypePrinter issue.
> Could you explain to me why the former behavior is more desirable here?
> 
> I initially understood that this hint should provide the type written in a 
> form that would actually work if it replaced the 'auto'.
> It is strange, but if it is just meant as a hint, it's still fine I guess.
> 
> Or maybe this was broken in the first place and just was just missing a FIXME?
The type is being printed with a `PrintingPolicy` which has the [non-default 
SuppressScope flag 
set](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang-tools-extra/clangd/InlayHints.cpp#34).

The 
[documentation](https://searchfox.org/llvm/rev/e158b5634aa67ea3039a62c3d8bda79b77b3b21c/clang/include/clang/AST/PrettyPrinter.h#129)
 of this flag says "Suppresses printing of scope specifiers", which I 
understand to mean both namespace and class scope specifiers.

If you're asking why we are using this flag for inlay hints, it's to keep the 
hints short so they don't take up too much horizontal space. Since the hints 
are displayed inline, hints that are too long can result in the line of code 
visually extending past the width of the editor window, and we try to minimize 
the impact of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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


[PATCH] D110599: [test] Don't run optimization-remark.c against the legacy PM

2021-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We may not emit some inlining remarks under -O0 (which seems
reasonable), so use -O1 in these tests.

This happens to make it pass when the legacy PM is turned on by default.
Previously some RUN lines were failing due to differences in the two
pass managers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110599

Files:
  clang/test/Frontend/optimization-remark.c


Index: clang/test/Frontend/optimization-remark.c
===
--- clang/test/Frontend/optimization-remark.c
+++ clang/test/Frontend/optimization-remark.c
@@ -1,36 +1,31 @@
 // This file tests the -Rpass family of flags (-Rpass, -Rpass-missed
-// and -Rpass-analysis) with the inliner. The test is designed to
-// always trigger the inliner, so it should be independent of the
-// optimization level (under the legacy PM). The inliner is not added to the 
new
-// PM pipeline unless optimizations are present.
+// and -Rpass-analysis) with the inliner. We may not consider every call to be
+// inlined (new PM + -O0) so these tests run at -O1.
 
 // The inliner for the new PM does not seem to be enabled at O0, but we still
 // get the same remarks with at least O1. The remarks are also slightly
 // different and located in another test file.
-// RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline 
-Rpass-missed=inline -O0 -fno-experimental-new-pass-manager -emit-llvm-only 
-verify
-// RUN: %clang_cc1 %s -Rpass=inline -Rpass-analysis=inline 
-Rpass-missed=inline -O0 -fno-experimental-new-pass-manager -emit-llvm-only 
-debug-info-kind=line-tables-only -verify
-// RUN: %clang_cc1 %s -Rpass=inline -emit-llvm -o - 2>/dev/null | FileCheck %s
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rpass-analysis=inline -Rpass-missed=inline -emit-llvm-only -verify
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rpass-analysis=inline -Rpass-missed=inline -emit-llvm-only 
-debug-info-kind=line-tables-only -verify
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>/dev/null | FileCheck %s
 //
 // Check that we can override -Rpass= with -Rno-pass.
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -Rno-pass -emit-llvm -o - 2>&1 | FileCheck 
%s --check-prefix=CHECK-NO-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager 
-Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
-//
-// The inliner for the new PM does not seem to be enabled at O0, but we still
-// get the same remarks with at least O1.
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 
-emit-llvm -mllvm -mandatory-inlining-first=false -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 
-Rno-everything -Reverything -emit-llvm -mllvm -mandatory-inlining-first=false 
-o -  2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-NO-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-Rno-everything -Reverything -emit-llvm -o -  2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
 //
 // Check that -w doesn't disable remarks.
-// RUN: %clang_cc1 %s -Rpass=inline -fno-experimental-new-pass-manager -w 
-emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
-// RUN: %clang_cc1 %s -Rpass=inline -fexperimental-new-pass-manager -O1 -w 
-emit-llvm -mllvm -mandatory-inlining-first=false -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+// RUN: %clang_cc1 %s -O1 -mllvm -mandatory-inlining-first=false -Rpass=inline 
-w -emit-llvm 

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-27 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment.

Yeah, I can confirm that many cases are improved with this patch (others are 
more sensitive and depend on the __bos behavior I mentioned). With the 17 
kernel FORTIFY self-tests, all 17 fail without this patch. With this patch, 9 
start passing. Nice!


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

https://reviews.llvm.org/D109967

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


[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-27 Thread Fred Grim via Phabricator via cfe-commits
feg208 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17805
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"

HazardyKnusperkeks wrote:
> feg208 wrote:
> > HazardyKnusperkeks wrote:
> > > feg208 wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > Here is no alignment, maybe add another case with another line?
> > > > > 
> > > > > What happens if one line has the trailing comma and the next doesn't?
> > > > yesh this is pretty ugly. In the second case it crashes. This isn't 
> > > > going to be fixed for a bit
> > > What is that bit? Now we can still revert the original change for LLVM 13 
> > > (at least I think so, since there are new RC Tags on GitHub), or can fix 
> > > the crash.
> > I think we should fix the crash since it’s only an issue with the dangling 
> > comma but if the consensus is we should revert then I can do that as well
> Totally agree, could you please add the test cases? The ones that crash you 
> could comment out and fix afterwards.
> 
> I don't know how much time we have before the next version is released.
Let me see if I can fix the weirdness with the second test case but I'll time 
box it to two days so this doesn't drag out. I'd feel better about it not 
crashing just because we added an additional row.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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


[PATCH] D110485: Support [[no_unique_address]] for all targets.

2021-09-27 Thread cqwrteur via Phabricator via cfe-commits
expnkx updated this revision to Diff 375431.
expnkx added a comment.

do not know why it cannot be built. upload and then verify again


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

https://reviews.llvm.org/D110485

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/SemaCXX/cxx2a-no-unique-address.cpp


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field 
non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to 
non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr 
{
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];


Index: clang/test/SemaCXX/cxx2a-no-unique-address.cpp
===
--- clang/test/SemaCXX/cxx2a-no-unique-address.cpp
+++ clang/test/SemaCXX/cxx2a-no-unique-address.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++2a %s -verify -triple x86_64-linux-gnu
-// RUN: %clang_cc1 -std=c++2a %s -verify=unsupported -triple x86_64-windows
 
 [[no_unique_address]] int a; // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
 [[no_unique_address]] void f(); // expected-error {{only applies to non-bit-field non-static data members}} unsupported-warning {{unknown}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1686,7 +1686,7 @@
   let Documentation = [ArmMveStrictPolymorphismDocs];
 }
 
-def NoUniqueAddress : InheritableAttr, TargetSpecificAttr {
+def NoUniqueAddress : InheritableAttr {
   let Spellings = [CXX11<"", "no_unique_address", 201803>];
   let Subjects = SubjectList<[NonBitField], ErrorDiag>;
   let Documentation = [NoUniqueAddressDocs];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D110127: [Clang] Support typedef with btf_tag attributes

2021-09-27 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

@aaron.ballman ping. Hi, Aaron, did you get time looking at this patch again? I 
made some adjustments in test and explained overloadable attribute use case in 
comments. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110127

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


[PATCH] D110596: [CUDA] Move CUDA SDK include path further down the include search path.

2021-09-27 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 375425.
tra edited the summary of this revision.
tra added a comment.

Fixed the failing test affected by the search path changes.
Disabled addition of CUDA include path if -nogpuinc is in effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110596

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/cuda-detect.cu


Index: clang/test/Driver/cuda-detect.cu
===
--- clang/test/Driver/cuda-detect.cu
+++ clang/test/Driver/cuda-detect.cu
@@ -179,10 +179,10 @@
 // LIBDEVICE50-SAME: libdevice.compute_50.10.bc
 // PTX42-SAME: "-target-feature" "+ptx42"
 // PTX60-SAME: "-target-feature" "+ptx60"
-// CUDAINC-SAME: "-internal-isystem" 
"{{.*}}/Inputs/CUDA{{[_0-9]+}}/usr/local/cuda/include"
-// NOCUDAINC-NOT: "-internal-isystem" "{{.*}}/cuda/include"
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
+// CUDAINC-SAME: "-internal-isystem" 
"{{.*}}/Inputs/CUDA{{[_0-9]+}}/usr/local/cuda/include"
+// NOCUDAINC-NOT: "-internal-isystem" "{{.*}}/cuda/include"
 // -internal-externc-isystem flags must come *after* the cuda include flags,
 // because we must search the cuda include directory first.
 // CUDAINC-SAME: "-internal-externc-isystem"
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -299,8 +299,6 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
   CC1Args.push_back("-include");
   CC1Args.push_back("__clang_cuda_runtime_wrapper.h");
 }
@@ -867,6 +865,11 @@
 void CudaToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+
+  if (!DriverArgs.hasArg(options::OPT_nogpuinc) && CudaInstallation.isValid())
+CC1Args.append(
+{"-internal-isystem",
+ DriverArgs.MakeArgString(CudaInstallation.getIncludePath())});
 }
 
 void CudaToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &Args,


Index: clang/test/Driver/cuda-detect.cu
===
--- clang/test/Driver/cuda-detect.cu
+++ clang/test/Driver/cuda-detect.cu
@@ -179,10 +179,10 @@
 // LIBDEVICE50-SAME: libdevice.compute_50.10.bc
 // PTX42-SAME: "-target-feature" "+ptx42"
 // PTX60-SAME: "-target-feature" "+ptx60"
-// CUDAINC-SAME: "-internal-isystem" "{{.*}}/Inputs/CUDA{{[_0-9]+}}/usr/local/cuda/include"
-// NOCUDAINC-NOT: "-internal-isystem" "{{.*}}/cuda/include"
 // CUDAINC-SAME: "-include" "__clang_cuda_runtime_wrapper.h"
 // NOCUDAINC-NOT: "-include" "__clang_cuda_runtime_wrapper.h"
+// CUDAINC-SAME: "-internal-isystem" "{{.*}}/Inputs/CUDA{{[_0-9]+}}/usr/local/cuda/include"
+// NOCUDAINC-NOT: "-internal-isystem" "{{.*}}/cuda/include"
 // -internal-externc-isystem flags must come *after* the cuda include flags,
 // because we must search the cuda include directory first.
 // CUDAINC-SAME: "-internal-externc-isystem"
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -299,8 +299,6 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
   CC1Args.push_back("-include");
   CC1Args.push_back("__clang_cuda_runtime_wrapper.h");
 }
@@ -867,6 +865,11 @@
 void CudaToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+
+  if (!DriverArgs.hasArg(options::OPT_nogpuinc) && CudaInstallation.isValid())
+CC1Args.append(
+{"-internal-isystem",
+ DriverArgs.MakeArgString(CudaInstallation.getIncludePath())});
 }
 
 void CudaToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D110596: [CUDA] Move CUDA SDK include path further down the include search path.

2021-09-27 Thread Artem Belevich via Phabricator via cfe-commits
tra created this revision.
tra added reviewers: tambre, jlebar.
Herald added subscribers: bixia, yaxunl.
tra requested review of this revision.
Herald added a project: clang.

This allows clang to work on Linux distributions like Debian where
/include may be a symlink to /usr/include. We only need
`cuda_wrappers` to be present before the standard C++ library headers.
The CUDA SDK headers themselves do not need to be found that early.

This addresses https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=995122 
mentioned in post-commit comments on D108247 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110596

Files:
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -299,8 +299,6 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
   CC1Args.push_back("-include");
   CC1Args.push_back("__clang_cuda_runtime_wrapper.h");
 }
@@ -867,6 +865,8 @@
 void CudaToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+  CC1Args.append({"-internal-isystem",
+  
DriverArgs.MakeArgString(CudaInstallation.getIncludePath())});
 }
 
 void CudaToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &Args,


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -299,8 +299,6 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
   CC1Args.push_back("-include");
   CC1Args.push_back("__clang_cuda_runtime_wrapper.h");
 }
@@ -867,6 +865,8 @@
 void CudaToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {
   HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+  CC1Args.append({"-internal-isystem",
+  DriverArgs.MakeArgString(CudaInstallation.getIncludePath())});
 }
 
 void CudaToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-27 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 375422.
ychen added a comment.

- Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Analysis/dead-stores.c
  clang/test/CXX/basic/basic.link/p8.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/drs/dr20xx.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
  clang/test/CodeCompletion/pragma-macro-token-caching.c
  clang/test/Frontend/fixed_point_crash.c
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/Parser/cxx-ambig-decl-expr.cpp
  clang/test/Parser/cxx0x-ambig.cpp
  clang/test/Parser/cxx1z-init-statement.cpp
  clang/test/Parser/objc-messaging-1.m
  clang/test/Parser/objc-try-catch-1.m
  clang/test/Parser/objcxx11-attributes.mm
  clang/test/Sema/const-eval.c
  clang/test/Sema/exprs.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/sizeless-1.c
  clang/test/Sema/switch-1.c
  clang/test/Sema/vla-2.c
  clang/test/Sema/warn-type-safety.c
  clang/test/Sema/warn-unused-value.c
  clang/test/SemaCXX/attr-annotate.cpp
  clang/test/SemaCXX/builtin-constant-p.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/expression-traits.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/overloaded-operator.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/vector.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp
  clang/test/SemaCXX/warn-unused-value.cpp
  clang/test/SemaTemplate/derived.cpp
  clang/test/SemaTemplate/lambda-capture-pack.cpp

Index: clang/test/SemaTemplate/lambda-capture-pack.cpp
===
--- clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -18,7 +18,7 @@
 namespace PR41576 {
   template  constexpr int f(Xs ...xs) {
 return [&](auto ...ys) { // expected-note {{instantiation}}
-  return ((xs + ys), ...); // expected-warning {{unused}}
+  return ((xs + ys), ...); // expected-warning {{left operand of comma operator has no effect}}
 }(1, 2);
   }
   static_assert(f(3, 4) == 6); // expected-note {{instantiation}}
Index: clang/test/SemaTemplate/derived.cpp
===
--- clang/test/SemaTemplate/derived.cpp
+++ clang/test/SemaTemplate/derived.cpp
@@ -49,6 +49,6 @@
 
   class A {
 TFP m_p;
-void Enable() { 0, A(); } // expected-warning {{unused}}
+void Enable() { 0, A(); } // expected-warning {{left operand of comma operator has no effect}}
   };
 }
Index: clang/test/SemaCXX/warn-unused-value.cpp
===
--- clang/test/SemaCXX/warn-unused-value.cpp
+++ clang/test/SemaCXX/warn-unused-value.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++17 %s
 
 // PR4806
 namespace test0 {
@@ -138,3 +139,32 @@
   (void)arr3;
   (void)arr4;
 }
+
+#if __cplusplus >= 201103L // C++11 or later
+namespace test5 {
+int v[(5, 6)]; // expected-warning {{left operand of comma operator has no effect}}
+void foo() {
+  new double[false ? (1, 2) : 3]
+// FIXME: We shouldn't diagnose the unreachable constant expression
+// here.
+[false ? (1, 2) : 3]; // expected-warning {{left operand of comma operator has no effect}}
+}
+} // namespace test5
+
+// comma operator diagnoses should be suppressed in SFINAE context.
+template  int c(int) { return 0; }
+template  int c(double) { return 1; }
+int foo() { return c(0); }
+
+#endif
+
+#if __cplusplus >= 201703L // C++17 or later
+namespace test6 {
+auto b() {
+  if constexpr (false)
+return (1,0);
+  else
+return (1.0,0.0); // expected-warning {{left operand of comma operator has no effect}}
+}
+} // namespace test6
+#endif
Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -242,8 +242,8 @@
 
 template 
 class Foo {
-  typedef bool_seq<(xs::value, true)...> all_true;
-  typedef bool_seq<(xs::value, false)...> all_false;
+  typedef bool_seq<((void)xs::value, true)...> all_true;
+  typedef bool_seq<((void)xs::value, false)...> all_false;
   typedef bool_seq seq;
 };
 
Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ c

[PATCH] D110029: [OpenMP][Offloading] Use bitset to indicate execution mode instead of value

2021-09-27 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

This seems to be related to some things failing on amdgpu at present:

> Target AMDGPU RTL --> Error wrong exec_mode value specified in HSA code 
> object file: 3

The semantics for this mode seem to have been introduced in D106460 
, which updated the amdgpu plugin to match. I 
think applying roughly this patch to the amdgpu plugin will bring it back to 
working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110029

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


[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D110422#3025911 , @jsji wrote:

>> "as binder can NOT discard only part of a csect." this part caused confusion 
>> to me...
>
> OK, yeah, csect is special to XCOFF, so this is not the same concept of 
> sections in ELF.

"as binder can NOT discard a subset of a csect."?

>> I haven't verified but it is possible that PGO picks the first pair (zero 
>> value) and have an incorrect counter for `foo`.
>
> As I mentioned before, we verified (with `llvm-profdata show --all-functions 
> --counts`) on all SPEC benchmarks that all the counters are good with private 
> linkage.
>
>> ---
>>
>> Does your example imply that the weak symbol `__profc_foo` has 2 definitions 
>> with conflicting values?
>
> No, they are identical weak functions.  Those offsets are actually from real 
> example using `std::stringbuf`, 
> the foo is actually 
> `_ZNSt3__115basic_stringbufIcNS_11char_traitsIcEENS_9allocatorIcEEEC2Ej`.
>
>> Any idea whether the linker bug will be fixed?
>
> Unfortunately, I just double confirmed with AIX linker (binder) owner, 
> there is NO plan in near future of lifting the limitation that binder can NOT 
> discard only part of a csect.

The linker (binder) doesn't have to discard a subset of csect, but it should 
ensure there are not two non-local symbols with the same name.
The latter is what I request.

Without section deduplication we just end up with PDP-11 a.out extended with 
.weak directive, it isn't too bad.
Having two non-local symbols could cause serious symbol resolution problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-27 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen updated this revision to Diff 375414.
ychen added a comment.

- Disable comma operator diagnose in SFINAE context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Analysis/dead-stores.c
  clang/test/CXX/basic/basic.link/p8.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/drs/dr20xx.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
  clang/test/CodeCompletion/pragma-macro-token-caching.c
  clang/test/Frontend/fixed_point_crash.c
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/Parser/cxx-ambig-decl-expr.cpp
  clang/test/Parser/cxx0x-ambig.cpp
  clang/test/Parser/cxx1z-init-statement.cpp
  clang/test/Parser/objc-messaging-1.m
  clang/test/Parser/objc-try-catch-1.m
  clang/test/Parser/objcxx11-attributes.mm
  clang/test/Sema/const-eval.c
  clang/test/Sema/exprs.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/sizeless-1.c
  clang/test/Sema/switch-1.c
  clang/test/Sema/vla-2.c
  clang/test/Sema/warn-type-safety.c
  clang/test/Sema/warn-unused-value.c
  clang/test/SemaCXX/attr-annotate.cpp
  clang/test/SemaCXX/builtin-constant-p.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/expression-traits.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/overloaded-operator.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/vector.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp
  clang/test/SemaCXX/warn-unused-value.cpp
  clang/test/SemaTemplate/derived.cpp
  clang/test/SemaTemplate/lambda-capture-pack.cpp

Index: clang/test/SemaTemplate/lambda-capture-pack.cpp
===
--- clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -18,7 +18,7 @@
 namespace PR41576 {
   template  constexpr int f(Xs ...xs) {
 return [&](auto ...ys) { // expected-note {{instantiation}}
-  return ((xs + ys), ...); // expected-warning {{unused}}
+  return ((xs + ys), ...); // expected-warning {{left operand of comma operator has no effect}}
 }(1, 2);
   }
   static_assert(f(3, 4) == 6); // expected-note {{instantiation}}
Index: clang/test/SemaTemplate/derived.cpp
===
--- clang/test/SemaTemplate/derived.cpp
+++ clang/test/SemaTemplate/derived.cpp
@@ -49,6 +49,6 @@
 
   class A {
 TFP m_p;
-void Enable() { 0, A(); } // expected-warning {{unused}}
+void Enable() { 0, A(); } // expected-warning {{left operand of comma operator has no effect}}
   };
 }
Index: clang/test/SemaCXX/warn-unused-value.cpp
===
--- clang/test/SemaCXX/warn-unused-value.cpp
+++ clang/test/SemaCXX/warn-unused-value.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunused-value -std=c++17 %s
 
 // PR4806
 namespace test0 {
@@ -138,3 +139,26 @@
   (void)arr3;
   (void)arr4;
 }
+
+#if __cplusplus >= 201103L // C++11 or later
+namespace test5 {
+int v[(5, 6)]; // expected-warning {{left operand of comma operator has no effect}}
+void foo() {
+  new double[false ? (1, 2) : 3]
+// FIXME: We shouldn't diagnose the unreachable constant expression
+// here.
+[false ? (1, 2) : 3]; // expected-warning {{left operand of comma operator has no effect}}
+}
+} // namespace test5
+#endif
+
+#if __cplusplus >= 201703L // C++17 or later
+namespace test6 {
+auto b() {
+  if constexpr (false)
+return (1,0);
+  else
+return (1.0,0.0); // expected-warning {{left operand of comma operator has no effect}}
+}
+} // namespace test6
+#endif
Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -242,8 +242,8 @@
 
 template 
 class Foo {
-  typedef bool_seq<(xs::value, true)...> all_true;
-  typedef bool_seq<(xs::value, false)...> all_false;
+  typedef bool_seq<((void)xs::value, true)...> all_true;
+  typedef bool_seq<((void)xs::value, false)...> all_false;
   typedef bool_seq seq;
 };
 
Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -381,8 +381,8 @@
 typedef int inte2 __attribute__((__ext_vector_type__(2)));
 
 void test_vector_literal(inte4

[PATCH] D109967: Simplify handling of builtin with inline redefinition

2021-09-27 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment.

I'm setting up to test this patch (thank you!) using my current kernel FORTIFY 
improvements. Right now I have a bunch of compile-time behavior selftests 
written:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/overflow&id=3c5221f3f4fd865a780f544b72c68f4209bd2e76

It should be possible to do an A/B test against those from the kernel's view of 
its FORTIFY functions. However, due to other bugs with __builtin_object_size(), 
the kernel still can't use name-matched inlines:
https://github.com/ClangBuiltLinux/linux/issues/1401
i.e. D109967  will fix half of what is 
needed, but the plan in the kernel right now is to work around the problem 
entirely by using macros instead.

I'll report back on the results of my testing, though, since it should give a 
good sense of how much coverage the kernel can gain back with this bug fixed.


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

https://reviews.llvm.org/D109967

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


[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

2021-09-27 Thread Lei Huang via Phabricator via cfe-commits
lei accepted this revision as: lei.
lei added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Headers/altivec.h:17337
 static __inline__ vector long long __ATTRS_o_ai
 vec_vbpermq(vector unsigned char __a, vector unsigned char __b) {
   return __builtin_altivec_vbpermq(__a, __b);

bmahjour wrote:
> This should be guarded under P8. It would also be good to add a 
> `vec_vbpermd(vector unsigned long long ...)` counter part under 
> `__POWER9_VECTOR__` for consistency.
I think this is actually guarded under ` __POWER8_VECTOR__`.  See line 17237.
As far as I can see, there is no LLVM intrinsic corresponding to vbpermd.



Comment at: clang/lib/Headers/altivec.h:17352
+}
+#endif
+#ifdef __POWER9_VECTOR__

nit: It would be more clear if we had a comment here what this endif if for.  I 
assume it's `/* __POWER8_VECTOR__ */`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107899

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


[PATCH] D106876: Remove non-affecting module maps from PCM files.

2021-09-27 Thread Ilya Kuteev via Phabricator via cfe-commits
ilyakuteev added a comment.

@rsmith can you please help with committing patch to trunk? Do not sure if I 
have correct rights to do it. Were are several failing tests, seems like it is 
caused by bugs in pre-merge checks (For example this one on win: 
https://github.com/google/llvm-premerge-checks/issues/353). Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106876

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


[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added a comment.

> "as binder can NOT discard only part of a csect." this part caused confusion 
> to me...

OK, yeah, csect is special to XCOFF, so this is not the same concept of 
sections in ELF.

> I haven't verified but it is possible that PGO picks the first pair (zero 
> value) and have an incorrect counter for `foo`.

As I mentioned before, we verified (with `llvm-profdata show --all-functions 
--counts`) that all the counters are good.

> ---
>
> Does your example imply that the weak symbol `__profc_foo` has 2 definitions 
> with conflicting values?

No, they are identical weak functions.  Those offsets are actually from real 
example using `std::stringbuf`, 
the foo is actually 
`_ZNSt3__115basic_stringbufIcNS_11char_traitsIcEENS_9allocatorIcEEEC2Ej`.

> Any idea whether the linker bug will be fixed?

Unfortunately, I just double confirmed with AIX linker (binder) owner, 
there is NO plan of lifting the limitation that binder can NOT discard only 
part of a csect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> However, on AIX, the current binder can NOT discard the weak symbols if we 
> put all of them into the same csect, as binder can NOT discard only part of a 
> csect.

"as binder can NOT discard only part of a csect." this part caused confusion to 
me...

> CountersOffset is now 0x1da3 ( CounterPtr - CountersDelta / 
> sizeof(unit64_6))
>
> CountersOffset > MaxNumCounters !

It is true that there are two pairs of weak profc/profd, but the first profc 
has a zero value, so I assume that it is fine?

The second pair of profc/profd has correct value.

I haven't verified but it is possible that PGO picks the first pair (zero 
value) and have an incorrect counter for `foo`.

---

Does your example imply that the weak symbol `__profc_foo` has 2 definitions 
with conflicting values? 
I presume it can cause more fallout.
Any idea whether the linker bug will be fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D110586: Update `DynTypedNode` to support the conversion of TypeLocs.

2021-09-27 Thread James King via Phabricator via cfe-commits
jcking1034 created this revision.
jcking1034 added reviewers: ymandel, tdl-g, sbenza.
jcking1034 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This provides better support for TypeLocs to allow TypeLoc-related
Matchers to feature stricter typing and to avoid relying on the dynamic
casting of TypeLocs in matchers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110586

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/lib/AST/ASTTypeTraits.cpp

Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/TypeLoc.h"
 
 using namespace clang;
 
@@ -28,6 +29,8 @@
 {NKI_None, "TemplateName"},
 {NKI_None, "NestedNameSpecifierLoc"},
 {NKI_None, "QualType"},
+#define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"},
+#include "clang/AST/TypeLocNodes.def"
 {NKI_None, "TypeLoc"},
 {NKI_None, "CXXBaseSpecifier"},
 {NKI_None, "CXXCtorInitializer"},
@@ -127,6 +130,17 @@
   llvm_unreachable("invalid type kind");
  }
 
+ ASTNodeKind ASTNodeKind::getFromNode(const TypeLoc &T) {
+   switch (T.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  case TypeLoc::CLASS: \
+return ASTNodeKind(NKI_##CLASS##TypeLoc);
+#include "clang/AST/TypeLocNodes.def"
+   }
+   llvm_unreachable("invalid typeloc kind");
+ }
+
 ASTNodeKind ASTNodeKind::getFromNode(const OMPClause &C) {
   switch (C.getClauseKind()) {
 #define GEN_CLANG_CLAUSE_CLASS
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -53,8 +53,7 @@
   ASTNodeKind() : KindId(NKI_None) {}
 
   /// Construct an identifier for T.
-  template 
-  static ASTNodeKind getFromNodeKind() {
+  template  static ASTNodeKind getFromNodeKind() {
 return ASTNodeKind(KindToKindId::Id);
   }
 
@@ -63,6 +62,7 @@
   static ASTNodeKind getFromNode(const Decl &D);
   static ASTNodeKind getFromNode(const Stmt &S);
   static ASTNodeKind getFromNode(const Type &T);
+  static ASTNodeKind getFromNode(const TypeLoc &T);
   static ASTNodeKind getFromNode(const OMPClause &C);
   static ASTNodeKind getFromNode(const Attr &A);
   /// \}
@@ -133,6 +133,8 @@
 NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
+#define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc,
+#include "clang/AST/TypeLocNodes.def"
 NKI_TypeLoc,
 NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc,
 NKI_CXXBaseSpecifier,
@@ -172,8 +174,7 @@
   template  struct KindToKindId {
 static const NodeKindId Id = NKI_None;
   };
-  template 
-  struct KindToKindId : KindToKindId {};
+  template  struct KindToKindId : KindToKindId {};
 
   /// Per kind info.
   struct KindInfo {
@@ -198,6 +199,8 @@
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
+#define TYPELOC(CLASS, PARENT) KIND_TO_KIND_ID(CLASS##TypeLoc)
+#include "clang/AST/TypeLocNodes.def"
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
@@ -238,8 +241,7 @@
 class DynTypedNode {
 public:
   /// Creates a \c DynTypedNode from \c Node.
-  template 
-  static DynTypedNode create(const T &Node) {
+  template  static DynTypedNode create(const T &Node) {
 return BaseConverter::create(Node);
   }
 
@@ -262,8 +264,7 @@
   /// Retrieve the stored node as type \c T.
   ///
   /// Similar to \c get(), but asserts that the type is what we are expecting.
-  template 
-  const T &getUnchecked() const {
+  template  const T &getUnchecked() const {
 return BaseConverter::getUnchecked(NodeKind, &Storage);
   }
 
@@ -304,7 +305,7 @@
   return getUnchecked().getAsOpaquePtr() <
  Other.getUnchecked().getAsOpaquePtr();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind)) {
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind)) {
   auto TLA = getUnchecked();
   auto TLB = Other.getUnchecked();
   return std::make_pair(TLA.getType().getAsOpaquePtr(),
@@ -336,7 +337,7 @@
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
@@ -365,7 +366,7 @@
 }
 static unsigned getHashValue(const DynTypedNode &Val) {
   // FIXME: Add hashing support for the remaining types.
-  if (AST

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 Thread Fangrui Song 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 rG75f0194d3d25: [Driver] Remove confusing *-linux-android 
detection with non-android --target= (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110379

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/android-gcc-toolchain.c


Index: clang/test/Driver/android-gcc-toolchain.c
===
--- clang/test/Driver/android-gcc-toolchain.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
-// triple.
-//
-// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
-// RUN: | FileCheck %s
-//
-// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2084,21 +2084,19 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux", "aarch64-linux-android"};
+  "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
-   "arm-linux-androideabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
- "armeb-linux-androideabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -2112,17 +2110,15 @@
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux","x86_64-linux-android"};
+  "x86_64-amazon-linux"};
   static const char *const X32Triples[] = {"x86_64-linux-gnux32",
"x86_64-pc-linux-gnux32"};
   static const char *const X32LibDirs[] = {"/libx32", "/lib"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
-  "i586-linux-gnu", "i686-linux-gnu",
-  "i686-pc-linux-gnu",  "i386-redhat-linux6E",
-  "i686-redhat-linux",  "i386-redhat-linux",
-  "i586-suse-linux","i686-montavista-linux",
-  "i686-linux-android", "i686-gnu",
+  "i586-linux-gnu",  "i686-linux-gnu","i686-pc-linux-gnu",
+  "i386-redhat-linux6E", "i686-redhat-linux", "i386-redhat-linux",
+  "i586-suse-linux", "i686-montavista-linux", "i686-gnu",
   };
 
   static const char *const M68kLibDirs[] = {"/lib"};
@@ -2135,8 +2131,7 @@
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
-  "mipsel-linux-android"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = {
@@ -2147,8 +2142,7 @@
   static const char *const MIPS64ELTriples[] = {
   "mips64el-linux-gnu",  "mips-mti-linux-gnu",
   "mips-img-linux-gnu",  "mips64el-linux-gnuabi64",
-  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64",
-  "mips64el-linux-android"};
+  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64"};
 
   static const char *const MIPSN32LibDirs[] = {"/lib32"};
   static const char *const MIPSN32Triples[] = {"mips64-linux-gnuabin32",


Index: clang/test/Driver/android-gcc-toolchain.c
===
--- clang/test/Driver/android-gcc-toolchain.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// Test that gcc-toolcha

[clang] 75f0194 - [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2021-09-27T13:28:40-07:00
New Revision: 75f0194d3d252388635325a78490f5c0f49c4ca6

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

LOG: [Driver] Remove confusing *-linux-android detection with non-android 
--target=

These values allow, for example, `--target=aarch64` and
`--target=aarch64-linux-gnu` to detect `aarch64-linux-android`. This is
confusing. Users should specify `--target=aarch64-linux-android` to get Android 
GCC
installation.

Reverts D53463.

Reviewed By: nickdesaulniers, danalbert

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp

Removed: 
clang/test/Driver/android-gcc-toolchain.c



diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 9aca45312bb0..fe5bda5c6605 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2084,21 +2084,19 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux", "aarch64-linux-android"};
+  "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
-   "arm-linux-androideabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
- "armeb-linux-androideabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -2112,17 +2110,15 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux","x86_64-linux-android"};
+  "x86_64-amazon-linux"};
   static const char *const X32Triples[] = {"x86_64-linux-gnux32",
"x86_64-pc-linux-gnux32"};
   static const char *const X32LibDirs[] = {"/libx32", "/lib"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
-  "i586-linux-gnu", "i686-linux-gnu",
-  "i686-pc-linux-gnu",  "i386-redhat-linux6E",
-  "i686-redhat-linux",  "i386-redhat-linux",
-  "i586-suse-linux","i686-montavista-linux",
-  "i686-linux-android", "i686-gnu",
+  "i586-linux-gnu",  "i686-linux-gnu","i686-pc-linux-gnu",
+  "i386-redhat-linux6E", "i686-redhat-linux", "i386-redhat-linux",
+  "i586-suse-linux", "i686-montavista-linux", "i686-gnu",
   };
 
   static const char *const M68kLibDirs[] = {"/lib"};
@@ -2135,8 +2131,7 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
-  "mipsel-linux-android"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = {
@@ -2147,8 +2142,7 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
   static const char *const MIPS64ELTriples[] = {
   "mips64el-linux-gnu",  "mips-mti-linux-gnu",
   "mips-img-linux-gnu",  "mips64el-linux-gnuabi64",
-  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64",
-  "mips64el-linux-android"};
+  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64"};
 
   static const char *const MIPSN32LibDirs[] = {"/lib32"};
   static const char *const MIPSN32Triples[] = {"mips64-linux-gnuabin32",

diff  --git a/clang/test/Driver/android-g

[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added a comment.

OK, I may not describe the example clearly.

Let me use example code with offsets and llvm internal calculations as the 
example, so that you might be clearer.

Let us say we have 2 objects , which both have weak function foo (and some 
non-weak functions).

  [2054]  m   0x11ec0 .data 1  unamex
__llvm_prf_cnts
  [2056]  m   0x11ef8 .data 1weak
__profc__foo
  [1389]  m   0x110001540 .data 1  unamex
__llvm_prf_cnts
  [1391]  m   0x1100015a8 .data 1weak
__profc__foo<== chosen by binder
  
  [2290]  m   0x110001ca8 .data 1  unamex
__llvm_prf_data
  [2292]  m   0x110001ce0 .data 1weak
__profd__foo
  [1633]  m   0x110003678 .data 1  unamex
__llvm_prf_data
  [1635]  m   0x1100036b0 .data 1weak
__profd__foo<===  chosen by binder

In binding, binder chose 0x1100015a8 for `__profc__foo`, and 0x1100036b0 for 
`__profd__foo`. 
(Not the 1st in the csect, but 1st seen by binder)

At the beginning `CountersDelta` is 0xf218. ( 0x11ec0 [2054]  - 
0x110001ca8 [2290] ) .

The first record is for non-weak function, so we are OK.

Then we move forward to read foo counters, 
`CountersDelta` is then updated to 0xf1e0 ( 0xf218 - 
sizeof(Data))
`CounterPtr` is now 0x1100015a8 - 0x1100036b0 = 0xdef8

CountersOffset is now 0x1da3  ( CounterPtr - CountersDelta / 
sizeof(unit64_6))

CountersOffset > MaxNumCounters !

If we have more weak symbols, the symbols chosen by binder may be  interleaving 
in the csects,
we won't be able to calculate the CountersOffset correctly for all of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 375383.
MaskRay edited the summary of this revision.
MaskRay added a reverted change: D53463: [Driver] allow Android triples to 
alias for non Android targets.
MaskRay added a comment.

revert D53463 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110379

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/android-gcc-toolchain.c


Index: clang/test/Driver/android-gcc-toolchain.c
===
--- clang/test/Driver/android-gcc-toolchain.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
-// triple.
-//
-// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
-// RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
-// RUN: | FileCheck %s
-//
-// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2084,21 +2084,19 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux", "aarch64-linux-android"};
+  "aarch64-suse-linux"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
-   "arm-linux-androideabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
- "armeb-linux-androideabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -2112,17 +2110,15 @@
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux","x86_64-linux-android"};
+  "x86_64-amazon-linux"};
   static const char *const X32Triples[] = {"x86_64-linux-gnux32",
"x86_64-pc-linux-gnux32"};
   static const char *const X32LibDirs[] = {"/libx32", "/lib"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
-  "i586-linux-gnu", "i686-linux-gnu",
-  "i686-pc-linux-gnu",  "i386-redhat-linux6E",
-  "i686-redhat-linux",  "i386-redhat-linux",
-  "i586-suse-linux","i686-montavista-linux",
-  "i686-linux-android", "i686-gnu",
+  "i586-linux-gnu",  "i686-linux-gnu","i686-pc-linux-gnu",
+  "i386-redhat-linux6E", "i686-redhat-linux", "i386-redhat-linux",
+  "i586-suse-linux", "i686-montavista-linux", "i686-gnu",
   };
 
   static const char *const M68kLibDirs[] = {"/lib"};
@@ -2135,8 +2131,7 @@
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
-  "mipsel-linux-android"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = {
@@ -2147,8 +2142,7 @@
   static const char *const MIPS64ELTriples[] = {
   "mips64el-linux-gnu",  "mips-mti-linux-gnu",
   "mips-img-linux-gnu",  "mips64el-linux-gnuabi64",
-  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64",
-  "mips64el-linux-android"};
+  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64"};
 
   static const char *const MIPSN32LibDirs[] = {"/lib32"};
   static const char *const MIPSN32Triples[] = {"mips64-linux-gnuabin32",


Index: clang/test/Driver/android-gcc-toolchain.c
===
--- clang/test/Driver/android-gcc-toolchain.c
+++ /dev/null
@@ -1,8 +0,0 @@
-// Test that gcc-

Re: [clang] 2bd8493 - Improve type printing of const arrays to normalize array-of-const and const-array

2021-09-27 Thread David Blaikie via cfe-commits
Ping on this

On Wed, Sep 15, 2021 at 1:52 PM David Blaikie  wrote:

>
>
> On Tue, Sep 14, 2021 at 10:04 AM Richard Smith 
> wrote:
>
>> On Mon, 13 Sept 2021 at 19:24, David Blaikie via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>> Author: David Blaikie
>>> Date: 2021-09-13T19:17:05-07:00
>>> New Revision: 2bd84938470bf2e337801faafb8a67710f46429d
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/2bd84938470bf2e337801faafb8a67710f46429d
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/2bd84938470bf2e337801faafb8a67710f46429d.diff
>>>
>>> LOG: Improve type printing of const arrays to normalize array-of-const
>>> and const-array
>>>
>>> Since these map to the same effective type - render them the same/in the
>>> more legible way (const x[n]).
>>>
>>
>> Nice!
>>
>>
>>> Added:
>>>
>>>
>>> Modified:
>>> clang/lib/AST/TypePrinter.cpp
>>> clang/test/ARCMT/cxx-checking.mm
>>> clang/test/AST/ast-dump-APValue-arithmetic.cpp
>>> clang/test/AST/ast-dump-APValue-array.cpp
>>> clang/test/CXX/basic/basic.types/p10.cpp
>>> clang/test/Sema/assign.c
>>> clang/test/Sema/typedef-retain.c
>>> clang/test/SemaCXX/reinterpret-cast.cpp
>>> clang/test/SemaCXX/static-assert-cxx17.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> 
>>> diff  --git a/clang/lib/AST/TypePrinter.cpp
>>> b/clang/lib/AST/TypePrinter.cpp
>>> index aef1e4f3f4953..251db97c7db08 100644
>>> --- a/clang/lib/AST/TypePrinter.cpp
>>> +++ b/clang/lib/AST/TypePrinter.cpp
>>> @@ -200,11 +200,12 @@ bool TypePrinter::canPrefixQualifiers(const Type
>>> *T,
>>>// type expands to a simple string.
>>>bool CanPrefixQualifiers = false;
>>>NeedARCStrongQualifier = false;
>>> -  Type::TypeClass TC = T->getTypeClass();
>>> +  const Type *UnderlyingType = T;
>>>if (const auto *AT = dyn_cast(T))
>>> -TC = AT->desugar()->getTypeClass();
>>> +UnderlyingType = AT->desugar().getTypePtr();
>>>if (const auto *Subst = dyn_cast(T))
>>> -TC = Subst->getReplacementType()->getTypeClass();
>>> +UnderlyingType = Subst->getReplacementType().getTypePtr();
>>> +  Type::TypeClass TC = UnderlyingType->getTypeClass();
>>>
>>>switch (TC) {
>>>  case Type::Auto:
>>> @@ -243,6 +244,9 @@ bool TypePrinter::canPrefixQualifiers(const Type *T,
>>>
>>>  case Type::ConstantArray:
>>>  case Type::IncompleteArray:
>>> +  return canPrefixQualifiers(
>>> +
>>> cast(UnderlyingType)->getElementType().getTypePtr(),
>>> +  NeedARCStrongQualifier);
>>>  case Type::VariableArray:
>>>  case Type::DependentSizedArray:
>>>
>>
>> Can we give these two cases the same treatment?
>>
>
> Handled the DependentSizedArray in
> https://github.com/llvm/llvm-project/commit/40acc0adad59ac39e9a7a02fcd93161298500c00
>
> But per the comment in that commit I wasn't able to reproduce the problem
> with a variable array - though we could include it in the handling on
> principle/for consistency, even without a test/etc. Perhaps there's a way
> to test/provoke the behavior you might know that I couldn't figure out?
>
> Details from the commit:
>
> The VariableArray case I couldn't figure out how to test/provoke - you
>
> can't write/form a variable array in any context other than a local
>
> variable that I know of, and in that case `const int x[n]` is the
>
> normalized form already (array-of-const) and you can't use typedefs
>
> (since you can't typedef int[n] with variable 'n') to force the
>
> const-array AST that would produce the undesirable type printing "int
>
> const [n]".
>
> Oh, also - another quirk of array type printing that I'd be up for
> addressing if you've got an opinion on it:
>
> "int [n]" is printed with a space between the "int" and array.
> "int *const[n]" is printed without a space before the array (though both
> have a space after "int")
>
> ClangFormat formats these as "int[n]" and "int *const[n]" - with *
> left/right depending on ClangFormat's heuristics/configuration.
>
> Reckon we should remove the space in "int[n]"?
>
>
>
>>
>>
>>>NeedARCStrongQualifier = true;
>>
>>
>>> diff  --git a/clang/test/ARCMT/cxx-checking.mm b/clang/test/ARCMT/
>>> cxx-checking.mm
>>> index 952f3cdcbb79c..d6441def09b40 100644
>>> --- a/clang/test/ARCMT/cxx-checking.mm
>>> +++ b/clang/test/ARCMT/cxx-checking.mm
>>> @@ -80,7 +80,7 @@
>>>
>>>  struct FlexibleArrayMember0 {
>>>int length;
>>> -  id array[]; // expected-error{{flexible array member 'array' of type
>>> 'id __strong[]' with non-trivial destruction}}
>>> +  id array[]; // expected-error{{flexible array member 'array' of type
>>> '__strong id []' with non-trivial destruction}}
>>>  };
>>>
>>>  struct FlexibleArrayMember1 {
>>>
>>> diff  --git a/clang/test/AST/ast-dump-APValue-arithmetic.cpp
>>> b/clang/test/AST/ast-dump-APValue-arithmetic.cpp
>>> index 835d8c8108346..e51c1cee04cfe 100644
>>> --- a/clang/test/AST/ast-dump-APValu

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

2021-09-27 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

Sorry, I missed the email for this code review, let me check my filters aren't 
too agressive.

> The Android kernel build does not use the Android triple.

That's no longer the case, so we might as well just wholesale revert D53463 
 IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110379

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 375374.
cor3ntin marked 3 inline comments as done.
cor3ntin added a comment.

Add parens in CFG.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
  clang/test/CodeGenCXX/cxx2b-consteval-if.cpp

Index: clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+
+void should_be_used_1();
+void should_be_used_2();
+void should_be_used_3();
+constexpr void should_not_be_used() {}
+
+constexpr void f() {
+  if consteval {
+should_not_be_used();
+  } else {
+should_be_used_1();
+  }
+
+  if !consteval {
+should_be_used_2();
+  }
+
+  if !consteval {
+should_be_used_3();
+  } else {
+should_not_be_used();
+  }
+}
+
+void g() {
+  f();
+}
+
+// CHECK: should_be_used_1
+// CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -std=c++2b -verify %s
+
+void test_consteval() {
+  if consteval (void) 0; // expected-error {{expected { after consteval}}
+  if consteval {
+(void)0;
+  } else (void)0; // expected-error {{expected { after else}}
+
+  static_assert([] {
+if consteval {
+  return 0;
+}
+return 1;
+  }() == 0);
+
+  static_assert([] {
+if consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 0);
+
+  static_assert([] {
+if !consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 1);
+
+  static_assert([] {
+if not consteval {
+  return 0;
+}
+return 1;
+  }() == 1);
+}
+
+void test_consteval_jumps() {
+  if consteval { // expected-note 4{{jump enters controlled statement of if consteval}}
+goto a;
+goto b; // expected-error {{cannot jump from this goto statement to its label}}
+  a:;
+  } else {
+goto b;
+goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  b:;
+  }
+  goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  goto b; // expected-error {{cannot jump from this goto statement to its label}}
+}
+
+void test_consteval_switch() {
+  int x = 42;
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+case 1:;   // expected-error {{cannot jump from switch statement to this case label}}
+default:;  // expected-error {{cannot jump from switch statement to this case label}}
+} else {
+}
+  }
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+} else {
+case 2:;  // expected-error {{cannot jump from switch statement to this case label}}
+default:; // expected-error {{cannot jump from switch statement to this case label}}
+}
+  }
+}
+
+consteval int f(int i) { return i; }
+constexpr int g(int i) {
+  if consteval {
+return f(i);
+  } else {
+return 42;
+  }
+}
+static_assert(g(10) == 10);
+
+constexpr int h(int i) { // expected-note {{declared here}}
+  if !consteval {
+return f(i); // expected-error {{call to consteval function 'f' is not a constant expression}}\
+ // expected-note  {{cannot be used in a constant expression}}
+  }
+  return 0;
+}
+
+consteval void warn_in_consteval() {
+  if consteval { // expected-warning {{if consteval is always true in an immediate context}}
+if consteval {} // expected-warning {{if consteval is always true in an immediate context}}
+  }
+}
+
+const

[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+  if (IsConsteval && NotLocation.isValid()) {
+if (ElseStmt.isUnset())
+  ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);

aaron.ballman wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > So this is interesting.  I'm not sure how I feel about having the AST 
> > > > not represent the textual representation like this.  I'm interested 
> > > > what others have to say.
> > > > 
> > > > My understanding is that this makes:
> > > > 
> > > > `if consteval { thenstmt; } else { elsestmt;`
> > > > be represented as:
> > > > `IfStmt isConsteval, with getThen()== thenstmt`
> > > > 
> > > > however
> > > > `if not consteval { thenstmt; } else { elsestmt;}`
> > > > be represented as:
> > > > `IfStmt isConsteval, with getThen()== elsestmt`
> > > > 
> > > > IMO, this feels too clever.  
> > > > 
> > > > I think I'd prefer that the IfStmt know whether it is a 'not consteval' 
> > > > and select the right one that way.
> > > I haven't had the chance to go over this review yet, but this comment 
> > > caught my eye in my inbox so I figured I'd respond quickly.
> > > 
> > > The current approach is definitely clever, but I don't think it's the 
> > > right way to tackle this. Generally, the AST needs to retain enough 
> > > fidelity to be pretty printed back out to the original source, which 
> > > wouldn't work here. But also, this makes it harder to write AST matchers 
> > > over the construct because it's not really matching what the user wrote 
> > > in source (we sometimes get around this by having a "semantic" and 
> > > "syntactic" AST representation, but that seems like overkill here).
> > This is exactly the wording though. 
> > 
> > > An if statement of the form if ! consteval compound-statement is not 
> > > itself a consteval if statement, but is equivalent to the consteval if 
> > > statement `if consteval { } else compound-statement`
> >An if statement of the form `if ! consteval compound-statement1 else 
> > statement2` is not itself a consteval if statement, but is equivalent to 
> > the consteval if statement
> > 
> > Doing something else would require storing the not position, and I don't 
> > think we gain any functionality?
> Sure, the standard also talks about how a `for` loop is equivalent to a 
> `while` statement, but we still don't model our AST that way. We gain 
> functionality with the pretty printing facilities actually working and being 
> able to AST match more naturally (like wanting to match a `not` predicate of 
> an `if constexpr` statement).
I don't think you'd necessarily have to store the 'not' position, just the 
'not-ness'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 375370.
cor3ntin added a comment.

Formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
  clang/test/CodeGenCXX/cxx2b-consteval-if.cpp

Index: clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+
+void should_be_used_1();
+void should_be_used_2();
+void should_be_used_3();
+constexpr void should_not_be_used() {}
+
+constexpr void f() {
+  if consteval {
+should_not_be_used();
+  } else {
+should_be_used_1();
+  }
+
+  if !consteval {
+should_be_used_2();
+  }
+
+  if !consteval {
+should_be_used_3();
+  } else {
+should_not_be_used();
+  }
+}
+
+void g() {
+  f();
+}
+
+// CHECK: should_be_used_1
+// CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -std=c++2b -verify %s
+
+void test_consteval() {
+  if consteval (void) 0; // expected-error {{expected { after consteval}}
+  if consteval {
+(void)0;
+  } else (void)0; // expected-error {{expected { after else}}
+
+  static_assert([] {
+if consteval {
+  return 0;
+}
+return 1;
+  }() == 0);
+
+  static_assert([] {
+if consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 0);
+
+  static_assert([] {
+if !consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 1);
+
+  static_assert([] {
+if not consteval {
+  return 0;
+}
+return 1;
+  }() == 1);
+}
+
+void test_consteval_jumps() {
+  if consteval { // expected-note 4{{jump enters controlled statement of if consteval}}
+goto a;
+goto b; // expected-error {{cannot jump from this goto statement to its label}}
+  a:;
+  } else {
+goto b;
+goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  b:;
+  }
+  goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  goto b; // expected-error {{cannot jump from this goto statement to its label}}
+}
+
+void test_consteval_switch() {
+  int x = 42;
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+case 1:;   // expected-error {{cannot jump from switch statement to this case label}}
+default:;  // expected-error {{cannot jump from switch statement to this case label}}
+} else {
+}
+  }
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+} else {
+case 2:;  // expected-error {{cannot jump from switch statement to this case label}}
+default:; // expected-error {{cannot jump from switch statement to this case label}}
+}
+  }
+}
+
+consteval int f(int i) { return i; }
+constexpr int g(int i) {
+  if consteval {
+return f(i);
+  } else {
+return 42;
+  }
+}
+static_assert(g(10) == 10);
+
+constexpr int h(int i) { // expected-note {{declared here}}
+  if !consteval {
+return f(i); // expected-error {{call to consteval function 'f' is not a constant expression}}\
+ // expected-note  {{cannot be used in a constant expression}}
+  }
+  return 0;
+}
+
+consteval void warn_in_consteval() {
+  if consteval { // expected-warning {{if consteval is always true in an immediate context}}
+if consteval {} // expected-warning {{if consteval is always true in an immediate context}}
+  }
+}
+
+constexpr void warn_in_consteval2() {
+  if consteval {
+  

[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+  if (IsConsteval && NotLocation.isValid()) {
+if (ElseStmt.isUnset())
+  ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);

cor3ntin wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > So this is interesting.  I'm not sure how I feel about having the AST not 
> > > represent the textual representation like this.  I'm interested what 
> > > others have to say.
> > > 
> > > My understanding is that this makes:
> > > 
> > > `if consteval { thenstmt; } else { elsestmt;`
> > > be represented as:
> > > `IfStmt isConsteval, with getThen()== thenstmt`
> > > 
> > > however
> > > `if not consteval { thenstmt; } else { elsestmt;}`
> > > be represented as:
> > > `IfStmt isConsteval, with getThen()== elsestmt`
> > > 
> > > IMO, this feels too clever.  
> > > 
> > > I think I'd prefer that the IfStmt know whether it is a 'not consteval' 
> > > and select the right one that way.
> > I haven't had the chance to go over this review yet, but this comment 
> > caught my eye in my inbox so I figured I'd respond quickly.
> > 
> > The current approach is definitely clever, but I don't think it's the right 
> > way to tackle this. Generally, the AST needs to retain enough fidelity to 
> > be pretty printed back out to the original source, which wouldn't work 
> > here. But also, this makes it harder to write AST matchers over the 
> > construct because it's not really matching what the user wrote in source 
> > (we sometimes get around this by having a "semantic" and "syntactic" AST 
> > representation, but that seems like overkill here).
> This is exactly the wording though. 
> 
> > An if statement of the form if ! consteval compound-statement is not itself 
> > a consteval if statement, but is equivalent to the consteval if statement 
> > `if consteval { } else compound-statement`
>An if statement of the form `if ! consteval compound-statement1 else 
> statement2` is not itself a consteval if statement, but is equivalent to the 
> consteval if statement
> 
> Doing something else would require storing the not position, and I don't 
> think we gain any functionality?
Sure, the standard also talks about how a `for` loop is equivalent to a `while` 
statement, but we still don't model our AST that way. We gain functionality 
with the pretty printing facilities actually working and being able to AST 
match more naturally (like wanting to match a `not` predicate of an `if 
constexpr` statement).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

2021-09-27 Thread Di Mo via Phabricator via cfe-commits
modimo added a comment.

Thanks for the thorough review @tejohnson! I'll do additional validation on FB 
code that uses exceptions and if that all looks good I'll send up a change to 
turn this default on with the findings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36850

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+  if (IsConsteval && NotLocation.isValid()) {
+if (ElseStmt.isUnset())
+  ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);

aaron.ballman wrote:
> erichkeane wrote:
> > So this is interesting.  I'm not sure how I feel about having the AST not 
> > represent the textual representation like this.  I'm interested what others 
> > have to say.
> > 
> > My understanding is that this makes:
> > 
> > `if consteval { thenstmt; } else { elsestmt;`
> > be represented as:
> > `IfStmt isConsteval, with getThen()== thenstmt`
> > 
> > however
> > `if not consteval { thenstmt; } else { elsestmt;}`
> > be represented as:
> > `IfStmt isConsteval, with getThen()== elsestmt`
> > 
> > IMO, this feels too clever.  
> > 
> > I think I'd prefer that the IfStmt know whether it is a 'not consteval' and 
> > select the right one that way.
> I haven't had the chance to go over this review yet, but this comment caught 
> my eye in my inbox so I figured I'd respond quickly.
> 
> The current approach is definitely clever, but I don't think it's the right 
> way to tackle this. Generally, the AST needs to retain enough fidelity to be 
> pretty printed back out to the original source, which wouldn't work here. But 
> also, this makes it harder to write AST matchers over the construct because 
> it's not really matching what the user wrote in source (we sometimes get 
> around this by having a "semantic" and "syntactic" AST representation, but 
> that seems like overkill here).
This is exactly the wording though. 

> An if statement of the form if ! consteval compound-statement is not itself a 
> consteval if statement, but is equivalent to the consteval if statement `if 
> consteval { } else compound-statement`
   An if statement of the form `if ! consteval compound-statement1 else 
statement2` is not itself a consteval if statement, but is equivalent to the 
consteval if statement

Doing something else would require storing the not position, and I don't think 
we gain any functionality?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D36850: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

2021-09-27 Thread Di Mo 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 rG20faf789199d: [ThinLTO] Add noRecurse and noUnwind thinlink 
function attribute propagation (authored by modimo).

Changed prior to commit:
  https://reviews.llvm.org/D36850?vs=374934&id=375367#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36850

Files:
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  clang/test/CodeGen/thinlto-distributed-cfi.ll
  clang/test/CodeGen/thinlto-funcattr-prop.ll
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/IR/GlobalValue.h
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Transforms/IPO/FunctionAttrs.h
  llvm/include/llvm/Transforms/IPO/FunctionImport.h
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ModuleSummaryIndex.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/FunctionImport.cpp
  llvm/test/Assembler/thinlto-summary.ll
  llvm/test/Bitcode/thinlto-function-summary-refgraph.ll
  llvm/test/Bitcode/thinlto-type-vcalls.ll
  llvm/test/ThinLTO/X86/deadstrip.ll
  llvm/test/ThinLTO/X86/dot-dumper.ll
  llvm/test/ThinLTO/X86/dot-dumper2.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-undefined.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-weak.ll
  llvm/test/ThinLTO/X86/funcattrs-prop.ll
  llvm/test/ThinLTO/X86/funcimport_alwaysinline.ll
  llvm/test/ThinLTO/X86/function_entry_count.ll
  llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll

Index: llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
===
--- llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
+++ llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
@@ -3,15 +3,17 @@
 ; verification error.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
 
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
 ; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
 ; Copy from first module is prevailing and converted to weak_odr, copy
 ; from second module is preempted and converted to available_externally and
 ; removed from comdat.
-; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr comdat($c1) {
-; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr {
+; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] comdat($c1) {
+; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] {
+
+; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
 
 ; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
 ; NM1: W f
Index: llvm/test/ThinLTO/X86/function_entry_count.ll
===
--- llvm/test/ThinLTO/X86/function_entry_count.ll
+++ llvm/test/ThinLTO/X86/function_entry_count.ll
@@ -2,7 +2,7 @@
 ; RUN: opt -thinlto-bc %p/Inputs/function_entry_count.ll -write-relbf-to-summary -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
 
 ; First perform the thin link on the normal bitcode file.
-; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -thinlto-synthesize-entry-counts \
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts \
 ; RUN: -r=%t1.bc,g, \
 ; RUN: -r=%t1.bc,f,px \
 ; RUN: -r=%t1.bc,h,px \
@@ -10,15 +10,16 @@
 ; RUN: -r=%t2.bc,g,px
 ; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
 
-; RUN: llvm-lto -thinlto-action=run -thinlto-synthesize-entry-counts -exported-symbol=f \
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts -exported-symbol=f \
 ; RUN: -exported-symbol=g -exported-symbol=h -thinlto-save-temps=%t3. %t1.bc %t2.bc
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s
 
-; CHECK: define void @h() !prof ![[PROF2:[0-9]+]]
-; CHECK: define void @f(i32{{.*}}) !prof ![[PROF1:[0-9]+]]
+; CHECK: define void @h() [[ATTR:#[0-9]+]] !prof ![[PROF2:[0-9]+]]
+; CHECK: define void @f(i32{{.*}}) [[ATTR:#[0-9]+]] !prof ![[PROF1:[0-9]+]]
 ; CHECK: define available_externally void @

[clang] 20faf78 - [ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

2021-09-27 Thread via cfe-commits

Author: modimo
Date: 2021-09-27T12:28:07-07:00
New Revision: 20faf789199d31a40c7f1a359361980c65067aab

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

LOG: [ThinLTO] Add noRecurse and noUnwind thinlink function attribute 
propagation

Thinlink provides an opportunity to propagate function attributes across 
modules, enabling additional propagation opportunities.

This change propagates (currently default off, turn on with 
`disable-thinlto-funcattrs=1`) noRecurse and noUnwind based off of function 
summaries of the prevailing functions in bottom-up call-graph order. Testing on 
clang self-build:
1. There's a 35-40% increase in noUnwind functions due to the additional 
propagation opportunities.
2. Throughput is measured at 10-15% increase in thinlink time which itself is 
1.5% of E2E link time.

Implementation-wise this adds the following summary function attributes:
1. noUnwind: function is noUnwind
2. mayThrow: function contains a non-call instruction that 
`Instruction::mayThrow` returns true on (e.g. windows SEH instructions)
3. hasUnknownCall: function contains calls that don't make it into the summary 
call-graph thus should not be propagated from (e.g. indirect for now, could add 
no-opt functions as well)

Testing:
Clang self-build passes and 2nd stage build passes check-all
ninja check-all with newly added tests passing

Reviewed By: tejohnson

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

Added: 
clang/test/CodeGen/thinlto-funcattr-prop.ll
llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll
llvm/test/ThinLTO/X86/funcattrs-prop-maythrow.ll
llvm/test/ThinLTO/X86/funcattrs-prop-undefined.ll
llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
llvm/test/ThinLTO/X86/funcattrs-prop-weak.ll
llvm/test/ThinLTO/X86/funcattrs-prop.ll

Modified: 
clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
clang/test/CodeGen/thinlto-distributed-cfi.ll
llvm/include/llvm/AsmParser/LLToken.h
llvm/include/llvm/IR/GlobalValue.h
llvm/include/llvm/IR/ModuleSummaryIndex.h
llvm/include/llvm/LTO/LTO.h
llvm/include/llvm/Transforms/IPO/FunctionAttrs.h
llvm/include/llvm/Transforms/IPO/FunctionImport.h
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/AsmParser/LLLexer.cpp
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/ModuleSummaryIndex.cpp
llvm/lib/LTO/LTO.cpp
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/Assembler/thinlto-summary.ll
llvm/test/Bitcode/thinlto-function-summary-refgraph.ll
llvm/test/Bitcode/thinlto-type-vcalls.ll
llvm/test/ThinLTO/X86/deadstrip.ll
llvm/test/ThinLTO/X86/dot-dumper.ll
llvm/test/ThinLTO/X86/dot-dumper2.ll
llvm/test/ThinLTO/X86/funcimport_alwaysinline.ll
llvm/test/ThinLTO/X86/function_entry_count.ll
llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll 
b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
index e1b47522ac322..151f3fba304ed 100644
--- a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -7,7 +7,7 @@
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
-; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
+; RUN: llvm-lto2 run -thinlto-distributed-indexes -disable-thinlto-funcattrs=0 
%t.o \
 ; RUN:   -whole-program-visibility \
 ; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t2.index \
@@ -36,7 +36,7 @@
 ; Round trip it through llvm-as
 ; RUN: llvm-dis %t.o.thinlto.bc -o - | llvm-as -o - | llvm-dis -o - | 
FileCheck %s --check-prefix=CHECK-DIS
 ; CHECK-DIS: ^0 = module: (path: 
"{{.*}}thinlto-distributed-cfi-devirt.ll.tmp.o", hash: ({{.*}}, {{.*}}, {{.*}}, 
{{.*}}, {{.*}}))
-; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: 
(module: ^0, flags: (linkage: external, visibility: default, 
notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, 
typeIdInfo: (typeTests: (^2), typeCheckedLoadVCalls: (vFuncId: (^2, offset: 8), 
vFuncId: (^2, offset: 0))
+; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: 
(module: ^0, flags: (linkage: external, visibility: default, 
notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, 
funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, 
noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1), 
type

[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5942
   "jump bypasses initialization of VLA type alias">;
 def note_protected_by_constexpr_if : Note<
   "jump enters controlled statement of constexpr if">;

cor3ntin wrote:
> erichkeane wrote:
> > Oh boy this section of 'notes' seem like they deserve some level of 
> > merging with a %select.  I'd not want anything other than the `jump enters 
> > controlled statemennt of `  variants for this patch though.
> I don't think we can change that without some more refactor.
> see usages of `note_protected_by_constexpr_if`.
> The Diag ID is passed to a `GotoScope` emitting later. No way to pass 
> parameter. Changing that would be unrelated to this PR
good enough for me for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5942
   "jump bypasses initialization of VLA type alias">;
 def note_protected_by_constexpr_if : Note<
   "jump enters controlled statement of constexpr if">;

erichkeane wrote:
> Oh boy this section of 'notes' seem like they deserve some level of 
> merging with a %select.  I'd not want anything other than the `jump enters 
> controlled statemennt of `  variants for this patch though.
I don't think we can change that without some more refactor.
see usages of `note_protected_by_constexpr_if`.
The Diag ID is passed to a `GotoScope` emitting later. No way to pass 
parameter. Changing that would be unrelated to this PR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+  if (IsConsteval && NotLocation.isValid()) {
+if (ElseStmt.isUnset())
+  ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);

erichkeane wrote:
> So this is interesting.  I'm not sure how I feel about having the AST not 
> represent the textual representation like this.  I'm interested what others 
> have to say.
> 
> My understanding is that this makes:
> 
> `if consteval { thenstmt; } else { elsestmt;`
> be represented as:
> `IfStmt isConsteval, with getThen()== thenstmt`
> 
> however
> `if not consteval { thenstmt; } else { elsestmt;}`
> be represented as:
> `IfStmt isConsteval, with getThen()== elsestmt`
> 
> IMO, this feels too clever.  
> 
> I think I'd prefer that the IfStmt know whether it is a 'not consteval' and 
> select the right one that way.
I haven't had the chance to go over this review yet, but this comment caught my 
eye in my inbox so I figured I'd respond quickly.

The current approach is definitely clever, but I don't think it's the right way 
to tackle this. Generally, the AST needs to retain enough fidelity to be pretty 
printed back out to the original source, which wouldn't work here. But also, 
this makes it harder to write AST matchers over the construct because it's not 
really matching what the user wrote in source (we sometimes get around this by 
having a "semantic" and "syntactic" AST representation, but that seems like 
overkill here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[PATCH] D109178: [PowerPC] Disable vector types when not supported by subtarget features

2021-09-27 Thread Lei Huang via Phabricator via cfe-commits
lei updated this revision to Diff 375360.
lei added a comment.

Address review comments and add handling for vector long types without vsx.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109178

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Headers/altivec.h
  clang/lib/Sema/DeclSpec.cpp
  clang/test/CodeGen/builtins-ppc-int128.c
  clang/test/CodeGen/builtins-ppc-vsx.c
  clang/test/Parser/altivec-bool-128.c
  clang/test/Parser/altivec.c
  clang/test/Parser/cxx-altivec-bool-128.cpp
  clang/test/Parser/cxx-altivec.cpp
  clang/test/Sema/altivec-generic-overload.c
  clang/test/Sema/builtins-ppc.c

Index: clang/test/Sema/builtins-ppc.c
===
--- clang/test/Sema/builtins-ppc.c
+++ clang/test/Sema/builtins-ppc.c
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -target-feature +altivec -target-feature +crypto\
 // RUN: -triple powerpc64le-unknown-unknown -DTEST_CRYPTO -fsyntax-only  \
 // RUN: -verify %s
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +crypto \
+// RUN: -triple powerpc64le-unknown-unknown -DTEST_CRYPTO -fsyntax-only \
+// RUN: -target-feature +vsx -verify %s
 
 #ifdef TEST_HTM
 void test_htm() {
@@ -37,6 +40,7 @@
   return __builtin_crypto_vshasigmaw(a, 1, 15);
 }
 
+#ifdef __VSX__
 vector unsigned long long test_vshasigmad_or(void)
 {
   vector unsigned long long a = D_INIT
@@ -46,6 +50,7 @@
   vector unsigned long long e = __builtin_crypto_vshasigmad(a, 1, -15); // expected-error-re {{argument value {{.*}} is outside the valid range}}
   return __builtin_crypto_vshasigmad(a, 0, 15);
 }
+#endif
 
 #endif
 
Index: clang/test/Sema/altivec-generic-overload.c
===
--- clang/test/Sema/altivec-generic-overload.c
+++ clang/test/Sema/altivec-generic-overload.c
@@ -1,4 +1,8 @@
-// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec \
+// RUN:  -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -triple=powerpc64le-unknown-linux -target-feature +altivec \
+// RUN:  -target-feature +vsx -verify -verify-ignore-unexpected=note -pedantic -fsyntax-only \
+// RUN:  -target-cpu pwr8
 
 typedef signed char __v16sc __attribute__((__vector_size__(16)));
 typedef unsigned char __v16uc __attribute__((__vector_size__(16)));
@@ -20,9 +24,6 @@
 __v4si *__attribute__((__overloadable__)) convert1(vector signed int);
 __v4ui *__attribute__((__overloadable__)) convert1(vector unsigned int);
 __v2sll *__attribute__((__overloadable__)) convert1(vector signed long long);
-__v2ull *__attribute__((__overloadable__)) convert1(vector unsigned long long);
-__v1slll *__attribute__((__overloadable__)) convert1(vector signed __int128);
-__v1ulll *__attribute__((__overloadable__)) convert1(vector unsigned __int128);
 __v4f *__attribute__((__overloadable__)) convert1(vector float);
 __v2d *__attribute__((__overloadable__)) convert1(vector double);
 void __attribute__((__overloadable__)) convert1(vector bool int);
@@ -36,11 +37,17 @@
 vector unsigned int *__attribute__((__overloadable__)) convert2(__v4ui);
 vector signed long long *__attribute__((__overloadable__)) convert2(__v2sll);
 vector unsigned long long *__attribute__((__overloadable__)) convert2(__v2ull);
-vector signed __int128 *__attribute__((__overloadable__)) convert2(__v1slll);
-vector unsigned __int128 *__attribute__((__overloadable__)) convert2(__v1ulll);
 vector float *__attribute__((__overloadable__)) convert2(__v4f);
 vector double *__attribute__((__overloadable__)) convert2(__v2d);
 
+#ifdef __POWER8_VECTOR__
+__v1slll *__attribute__((__overloadable__)) convert1(vector signed __int128);
+__v1ulll *__attribute__((__overloadable__)) convert1(vector unsigned __int128);
+__v2ull *__attribute__((__overloadable__)) convert1(vector unsigned long long);
+vector signed __int128 *__attribute__((__overloadable__)) convert2(__v1slll);
+vector unsigned __int128 *__attribute__((__overloadable__)) convert2(__v1ulll);
+#endif
+
 void test() {
   __v16sc gv1;
   __v16uc gv2;
@@ -49,11 +56,14 @@
   __v4si gv5;
   __v4ui gv6;
   __v2sll gv7;
+  __v4f gv11;
+  __v2d gv12;
+
+#ifdef __POWER8_VECTOR__
   __v2ull gv8;
   __v1slll gv9;
   __v1ulll gv10;
-  __v4f gv11;
-  __v2d gv12;
+#endif
 
   vector signed char av1;
   vector unsigned char av2;
@@ -63,8 +73,10 @@
   vector unsigned int av6;
   vector signed long long av7;
   vector unsigned long long av8;
+#ifdef __POWER8_VECTOR__
   vector signed __int128 av9;
   vector unsigned __int128 av10;
+#endif
   vector float av11;
   vector double av12;
   vector bool int av13;
@@ -77,9 +89,11 @@
   __v4si *gv5_p = convert1(gv5);
   __v4ui *gv6_p = c

[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/Stmt.cpp:915
 
-IfStmt::IfStmt(const ASTContext &Ctx, SourceLocation IL, bool IsConstexpr,
-   Stmt *Init, VarDecl *Var, Expr *Cond, SourceLocation LPL,
-   SourceLocation RPL, Stmt *Then, SourceLocation EL, Stmt *Else)
+IfStmt::IfStmt(const ASTContext &Ctx, SourceLocation IL, ConstexprSpecKind 
ConstexprKind, Stmt *Init, VarDecl *Var, Expr *Cond,
+   SourceLocation LPL, SourceLocation RPL, Stmt *Then,

Does this need clang-format again?



Comment at: clang/lib/AST/Stmt.cpp:927
 
+  assert((!IsConsteval && !IsConstexpr) || IsConsteval != IsConstexpr);
+

erichkeane wrote:
> Oh dear... I'm not quite sure I want this as a scoped enum.
I see I mistyped here, i'm 'NOW' quite sure.  Thanks for fixing it anyway.



Comment at: clang/lib/AST/StmtPrinter.cpp:239
 void StmtPrinter::PrintRawIfStmt(IfStmt *If) {
+  if (If->isConsteval()) {
+OS << "if consteval";

I think I'm not a fan of the inversion mostly for this, the fact that we end up 
printing the reverse of what happens in 'if constexpr' is giving me some 
heartburn.



Comment at: clang/lib/Analysis/CFG.cpp:3050
   BinaryOperator *Cond =
-  I->getConditionVariable()
+  I->isConsteval() || I->getConditionVariable()
   ? nullptr

I think there is a ternary-confuse-warning in GCC that fires sometimes if you 
skip outer-parens, you might find it necessary/a good idea to put parens around 
the condition.



Comment at: clang/lib/Analysis/CFG.cpp:3064
 // See if this is a known constant.
-const TryResult &KnownVal = tryEvaluateBool(I->getCond());
+TryResult KnownVal;
+if (!I->isConsteval())

Note for future-reviewer: This ends up being an improvement here, TryResult 
contains only a single integer in a VERRY bizarre way, it seems to be intended 
to essentially be a 'true/false/unknown' bit of hackery (despite having 3 
states, its an integer?).  If someone runs across this comment in the future 
and wants an 'easy win', I suspect swapping it with a `llvm::Optional` or 
something would be at least an interface-win.



Comment at: clang/lib/Parse/ParseStmt.cpp:1541
+  if (IsConsteval && NotLocation.isValid()) {
+if (ElseStmt.isUnset())
+  ElseStmt = Actions.ActOnNullStmt(ThenStmtLoc);

So this is interesting.  I'm not sure how I feel about having the AST not 
represent the textual representation like this.  I'm interested what others 
have to say.

My understanding is that this makes:

`if consteval { thenstmt; } else { elsestmt;`
be represented as:
`IfStmt isConsteval, with getThen()== thenstmt`

however
`if not consteval { thenstmt; } else { elsestmt;}`
be represented as:
`IfStmt isConsteval, with getThen()== elsestmt`

IMO, this feels too clever.  

I think I'd prefer that the IfStmt know whether it is a 'not consteval' and 
select the right one that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

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


[clang] 18cf5b2 - Fixing docs build

2021-09-27 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2021-09-27T14:16:28-05:00
New Revision: 18cf5b220d3f5e5ee1998a163764e15e56763815

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

LOG: Fixing docs build

I always forget that new line...

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 231c20dab1ca..56e351183899 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3951,6 +3951,7 @@ Clang supports the pragma ``#pragma clang final``, which 
can be used to
 mark macros as final, meaning they cannot be undef'd or re-defined. For 
example:
 
 .. code-block:: c
+
#define FINAL_MACRO 1
#pragma clang final(FINAL_MACRO)
 



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


[PATCH] D108567: Implement #pragma clang final extension

2021-09-27 Thread Chris Bieneman 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 rG1e48ef20358f: Implement #pragma clang final extension 
(authored by beanz).

Changed prior to commit:
  https://reviews.llvm.org/D108567?vs=374377&id=375357#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/Lexer/Inputs/final-macro.h
  clang/test/Lexer/Inputs/unsafe-macro.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Lexer/final-macro.c
  clang/test/Lexer/pedantic-macro-interplay.c

Index: clang/test/Lexer/pedantic-macro-interplay.c
===
--- clang/test/Lexer/pedantic-macro-interplay.c
+++ clang/test/Lexer/pedantic-macro-interplay.c
@@ -11,4 +11,17 @@
 // not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as deprecated: Don't use this!}}
 #pragma clang restrict_expansion(UNSAFE_MACRO_2, "Don't use this!")
 
-// expected-no-diagnostics
+
+#define Foo 1
+#pragma clang final(Foo)
+// expected-note@+2{{macro marked 'deprecated' here}}
+// expected-note@+1{{macro marked 'deprecated' here}}
+#pragma clang deprecated(Foo)
+// expected-note@+1{{macro marked 'restrict_expansion' here}}
+#pragma clang restrict_expansion(Foo)
+
+// Test that unsafe_header and deprecated markings stick around after the undef
+#include "Inputs/final-macro.h"
+
+// expected-warning@+1{{macro 'Foo' has been marked as deprecated}}
+const int X = Foo;
Index: clang/test/Lexer/final-macro.c
===
--- /dev/null
+++ clang/test/Lexer/final-macro.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -Wfinal-macro %s -fsyntax-only -verify
+
+// Test warning production
+#define Foo 1
+// expected-note@+1 4{{macro marked 'final' here}}
+#pragma clang final(Foo)
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2
+
+// expected-warning@+1{{redefining builtin macro}}
+#define __TIME__ 1
+
+// expected-warning@+1{{undefining builtin macro}}
+#undef __TIMESTAMP__
+
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be undefined}}
+#undef Foo
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be redefined}}
+#define Foo 3
+
+// Test parse errors
+// expected-error@+1{{expected (}}
+#pragma clang final
+
+// expected-error@+1{{expected )}}
+#pragma clang final(Foo
+
+// expected-error@+1{{no macro named 'Baz'}}
+#pragma clang final(Baz)
+
+// expected-error@+1{{expected identifier}}
+#pragma clang final(4)
+
+// expected-error@+1{{expected (}}
+#pragma clang final Baz
+
+// no diagnostics triggered by these pragmas.
+#pragma clang deprecated(Foo)
+#pragma clang restrict_expansion(Foo)
Index: clang/test/Lexer/deprecate-macro.c
===
--- clang/test/Lexer/deprecate-macro.c
+++ clang/test/Lexer/deprecate-macro.c
@@ -6,10 +6,11 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang deprecated(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang deprecated(foo)
 
 #define bar 1
+// expected-note@+1{{macro marked 'deprecated' here}} 
 #pragma clang deprecated(bar, "bar is deprecated use 1")
 
 // expected-warning@+1{{macro 'bar' has been marked as deprecated: bar is deprecated use 1}}
@@ -17,6 +18,14 @@
 #endif
 
 #define foo 1
+// expected-note@+8{{macro marked 'deprecated' here}} 
+// expected-note@+7{{macro marked 'deprecated' here}} 
+// expected-note@+6{{macro marked 'deprecated' here}} 
+// expected-note@+5{{macro marked 'deprecated' here}} 
+// expected-note@+4{{macro marked 'deprecated' here}} 
+// expected-note@+3{{macro marked 'deprecated' here}} 
+// expected-note@+2{{macro marked 'deprecated' here}} 
+// expected-note@+1{{macro marked 'deprecated' here}} 
 #pragma clang deprecated(foo)
 
 // expected-error@+1{{expected )}}
@@ -39,7 +48,7 @@
 #endif
 
 int main(int argc, char** argv) {
-  // expected-error@+1{{no macro named main}}
+// expected-error@+1{{no macro named 'main'}}
 #pragma clang deprecated(main)
 
   // expected-warning@+1{{macro 'foo' has been marked as deprecated}}
Index: clang/test/Lexer/Inputs/unsafe-macro.h
===
--- clang/test/Lexer/Inputs/u

[clang] 1e48ef2 - Implement #pragma clang final extension

2021-09-27 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2021-09-27T14:11:16-05:00
New Revision: 1e48ef20358fb097f43d9b133e33bd23723d79c2

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

LOG: Implement #pragma clang final extension

This patch adds a new preprocessor extension ``#pragma clang final``
which enables warning on undefinition and re-definition of macros.

The intent of this warning is to extend beyond ``-Wmacro-redefined`` to
warn against any and all alterations to macros that are marked `final`.

This warning is part of the ``-Wpedantic-macros`` diagnostics group.

Reviewed By: aaron.ballman

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

Added: 
clang/test/Lexer/Inputs/final-macro.h
clang/test/Lexer/final-macro.c

Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Basic/IdentifierTable.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/PPDirectives.cpp
clang/lib/Lex/Pragma.cpp
clang/lib/Lex/Preprocessor.cpp
clang/test/Lexer/Inputs/unsafe-macro.h
clang/test/Lexer/deprecate-macro.c
clang/test/Lexer/pedantic-macro-interplay.c

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 28791a48a6c0d..231c20dab1caa 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3944,6 +3944,24 @@ will expansion of the macro within the main source file. 
For example:
 
 This warning is controlled by ``-Wpedantic-macros``.
 
+Final Macros
+
+
+Clang supports the pragma ``#pragma clang final``, which can be used to
+mark macros as final, meaning they cannot be undef'd or re-defined. For 
example:
+
+.. code-block:: c
+   #define FINAL_MACRO 1
+   #pragma clang final(FINAL_MACRO)
+
+   #define FINAL_MACRO // warning: FINAL_MACRO is marked final and should not 
be redefined
+   #undef FINAL_MACRO  // warning: FINAL_MACRO is marked final and should not 
be undefined
+
+This is useful for enforcing system-provided macros that should not be altered
+in user headers or code. This is controlled by ``-Wpedantic-macros``. Final 
+macros will always warn on redefinition, including situations with identical
+bodies and in system headers.
+
 Extended Integer Types
 ==
 

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index f48598eec3262..23e9144ea0084 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -648,6 +648,7 @@ def KeywordAsMacro : DiagGroup<"keyword-macro">;
 def ReservedIdAsMacro : DiagGroup<"reserved-macro-identifier">;
 def ReservedIdAsMacroAlias : DiagGroup<"reserved-id-macro", 
[ReservedIdAsMacro]>;
 def RestrictExpansionMacro : DiagGroup<"restrict-expansion">;
+def FinalMacro : DiagGroup<"final-macro">;
 
 // Just silence warnings about -Wstrict-aliasing for now.
 def : DiagGroup<"strict-aliasing=0">;
@@ -1322,4 +1323,5 @@ def PedanticMacros : DiagGroup<"pedantic-macros",
 [DeprecatedPragma,
  MacroRedefined,
  BuiltinMacroRedefined,
- RestrictExpansionMacro]>;
+ RestrictExpansionMacro,
+ FinalMacro]>;

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index faa84774167b9..763b7b0086369 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -551,7 +551,13 @@ def warn_pragma_restrict_expansion_macro_use :
 
 // - Note for macro annotations.
 def note_pp_macro_annotation :
-  Note<"macro marked '%select{deprecated|restrict_expansion}0' here">;
+  Note<"macro marked '%select{deprecated|restrict_expansion|final}0' here">;
+
+// - #pragma clang final(...)
+def warn_pragma_final_macro :
+  ExtWarn<"macro %0 has been marked as final and should not be "
+  "%select{undefined|redefined}1">,
+  InGroup;
 
 // - #pragma execution_character_set(...)
 def warn_pragma_exec_charset_expected :

diff  --git a/clang/include/clang/Basic/IdentifierTable.h 
b/clang/include/clang/Basic/IdentifierTable.h
index 9c4833b278294..d620138125569 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -127,7 +127,10 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
   // True if this macro is unsafe in headers.
   unsigned IsRestrictExpansion : 1;
 
-  // 23 bits left in a 64-bit word.
+  // True if this macro is final.
+  unsigned IsFinal : 1;
+
+  // 22 bits left in a 64-bit word.
 
   // Managed by the language front-end.
  

[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Supplement:
Even if you omit the vendor part in `-DLLVM_DEFAULT_TARGET_TRIPLE` (if 
unspecified, use `-DLLVM_HOST_TRIPLE`), `x86_64-linux-gnu`, you will see
`Target: x86_64-unknown-linux-gnu` in `clang -v` output (and similarly in 
`-dumpmachine`/`--print-target-triple`).

The reason is that clang normalizes the target triple quite early in toolchain 
selection.
(You may need debugging to figure out where.)

It'd probably make sense to honor `-DLLVM_DEFAULT_TARGET_TRIPLE` and don't add 
the omitted `vendor` part (`unknown` or `pc`), but it requires some work ;-)
Then Debian/Ubuntu and their derivatives will see `x86_64-linux-gnu` everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89013

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


[PATCH] D110482: [clang] Implement if consteval (P1938)

2021-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 375352.
cor3ntin added a comment.
Herald added a subscriber: dexonsmith.

use ConstexprSpecKind to describe whether the IfStmt
is constexpr or consteval.
Other minor fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110482

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
  clang/test/CodeGenCXX/cxx2b-consteval-if.cpp

Index: clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2b-consteval-if.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - | FileCheck %s --implicit-check-not=should_not_be_used
+
+void should_be_used_1();
+void should_be_used_2();
+void should_be_used_3();
+constexpr void should_not_be_used() {}
+
+constexpr void f() {
+  if consteval {
+should_not_be_used();
+  } else {
+should_be_used_1();
+  }
+
+  if !consteval {
+should_be_used_2();
+  }
+
+  if !consteval {
+should_be_used_3();
+  } else {
+should_not_be_used();
+  }
+}
+
+void g() {
+  f();
+}
+
+// CHECK: should_be_used_1
+// CHECK: should_be_used_2
+// CHECK: should_be_used_3
Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p4.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -std=c++2b -verify %s
+
+void test_consteval() {
+  if consteval (void) 0; // expected-error {{expected { after consteval}}
+  if consteval {
+(void)0;
+  } else (void)0; // expected-error {{expected { after else}}
+
+  static_assert([] {
+if consteval {
+  return 0;
+}
+return 1;
+  }() == 0);
+
+  static_assert([] {
+if consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 0);
+
+  static_assert([] {
+if !consteval {
+  return 0;
+} else {
+  return 1;
+}
+  }() == 1);
+
+  static_assert([] {
+if not consteval {
+  return 0;
+}
+return 1;
+  }() == 1);
+}
+
+void test_consteval_jumps() {
+  if consteval { // expected-note 4{{jump enters controlled statement of if consteval}}
+goto a;
+goto b; // expected-error {{cannot jump from this goto statement to its label}}
+  a:;
+  } else {
+goto b;
+goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  b:;
+  }
+  goto a; // expected-error {{cannot jump from this goto statement to its label}}
+  goto b; // expected-error {{cannot jump from this goto statement to its label}}
+}
+
+void test_consteval_switch() {
+  int x = 42;
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+case 1:;   // expected-error {{cannot jump from switch statement to this case label}}
+default:;  // expected-error {{cannot jump from switch statement to this case label}}
+} else {
+}
+  }
+  switch (x) {
+if consteval { // expected-note 2{{jump enters controlled statement of if consteval}}
+} else {
+case 2:;  // expected-error {{cannot jump from switch statement to this case label}}
+default:; // expected-error {{cannot jump from switch statement to this case label}}
+}
+  }
+}
+
+consteval int f(int i) { return i; }
+constexpr int g(int i) {
+  if consteval {
+return f(i);
+  } else {
+return 42;
+  }
+}
+static_assert(g(10) == 10);
+
+constexpr int h(int i) { // expected-note {{declared here}}
+  if !consteval {
+return f(i); // expected-error {{call to consteval function 'f' is not a constant expression}}\
+ // expected-note  {{cannot be used in a constant expression}}
+  }
+  return 0;
+}
+
+consteval void warn_in_consteval() {
+  if consteval { // expected-warning {{if consteval is always true in an immediate context}}
+if consteval {} // expected-warning 

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D109951#3015841 , @guitard0g wrote:

> @MyDeveloperDay @HazardyKnusperkeks I don't have commit access, could one of 
> you commit this for me? Thanks so much for your review!
>
> Name: Josh Learn
> Email: joshua_le...@apple.com
>
> (The test failures seem to be unrelated, but I'm fine waiting until they 
> start passing again if necessary)

Will do, but most likely not before the next weekend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109951

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


[PATCH] D109727: [Driver] Remove unneeded *-suse-* triples

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

@marxin Can you check this from Suse perspective? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109727

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


[PATCH] D108247: [CUDA] Improve CUDA version detection and diagnostics.

2021-09-27 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D108247#3025415 , @tambre wrote:

>> Another workaround would be to place a fake /usr/lib/cuda/include/cuda.h 
>> with something like this:
>
> My CMake CI bot [[ https://open.cdash.org/test/498767666 | encountered 
> `cmath` template errors ]] after I manually symlinked `/usr/lib/cuda/include` 
> to `/usr/include`, though that solved the version detection.
> Adding a shim like the one you show fixes everything and compiling works.
>
>> Clang does rely on very particular ordering of includes, so having CUDA 
>> headers in a location clang does not expect will lead to issues sooner or 
>> later.
>
> Having the headers in the system-wide `/usr/include` shouldn't be a problem 
> though, right?
> I'm not quite sure why simply having `/usr/lib/cuda/include` → `/usr/include` 
> symlink breaks as seen on my bot.

If you look at the logs of the broken build you will see that 
`/usr/lib/cuda/include/` appears fairly early in the search path, before the 
standard library C++ headers.
Symlinking it to /usr/include places a lot of other includes and that breaks 
the include order for the C++ library which has its own assumptions and 
requirements.

> Prior to this an empty `/usr/lib/cuda/include` and Clang finding them through 
> the regular include search path worked.

It's possible that the CUDA headers themselves do not need to be placed in the 
include path that early. We do need cuda_wrappers to be there, but at the 
moment I do not recall why we ended up placing cuda/include there as well. The 
fact that it worked for you with the empty directory suggests that it may not 
be necessary.

If we move cuda includes down to where /usr/include is in the search list, it 
should allow symlinking it to /usr/include.
I'll check whether that's feasible.

>> If the headers are not available where --cuda-path points clang to, I'm not 
>> sure what clang is supposed to do. Second-guessing explicit user input is 
>> not a good idea in general.
>
> I agree that second-guessing is bad, but expecting it to be available in the 
> include search path otherwise seems reasonable.

The issue with the include search path is that it's something that's not 
readily available at the clang driver level at the point in time  where we need 
to detect cuda version.
The driver does construct the include arguments for cc1 invocations, but all 
that happens quite a bit later after CUDA detection is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108247

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


[PATCH] D110481: fixes bug #51926 where dangling comma caused overrun

2021-09-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a subscriber: tstellar.
HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:17805
+   "Section{\n"
+   "0, bar(),\n"
+   "}\n"

feg208 wrote:
> HazardyKnusperkeks wrote:
> > feg208 wrote:
> > > HazardyKnusperkeks wrote:
> > > > Here is no alignment, maybe add another case with another line?
> > > > 
> > > > What happens if one line has the trailing comma and the next doesn't?
> > > yesh this is pretty ugly. In the second case it crashes. This isn't going 
> > > to be fixed for a bit
> > What is that bit? Now we can still revert the original change for LLVM 13 
> > (at least I think so, since there are new RC Tags on GitHub), or can fix 
> > the crash.
> I think we should fix the crash since it’s only an issue with the dangling 
> comma but if the consensus is we should revert then I can do that as well
Totally agree, could you please add the test cases? The ones that crash you 
could comment out and fix afterwards.

I don't know how much time we have before the next version is released.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110481

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


[PATCH] D110386: [clangd] Refactor IncludeStructure: use File (unsigned) for most computations

2021-09-27 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Reverted in 36dc5c048ac745ef62769dd824a9a13659afc2ef 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110386

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


[clang-tools-extra] 36dc5c0 - Revert "[clangd] Refactor IncludeStructure: use File (unsigned) for most computations"

2021-09-27 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2021-09-27T14:38:18-04:00
New Revision: 36dc5c048ac745ef62769dd824a9a13659afc2ef

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

LOG: Revert "[clangd] Refactor IncludeStructure: use File (unsigned) for most 
computations"

This reverts commit 0b1eff1bc5d004b1964bb9b1667e3efc034f3f62.
Breaks check-clangd on Windows, see comments on
https://reviews.llvm.org/D110386

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index 1033e5e92dcde..69338814ebdd2 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1379,16 +1379,14 @@ class CodeCompleteFlow {
   FileDistanceOptions ProxOpts{}; // Use defaults.
   const auto &SM = Recorder->CCSema->getSourceManager();
   llvm::StringMap ProxSources;
-  auto MainFileID =
-  Includes.getOrCreateID(SM.getFileEntryForID(SM.getMainFileID()));
-  for (auto &HeaderIDAndDepth : Includes.includeDepth(MainFileID)) {
-auto &Source =
-ProxSources[Includes.getRealPath(HeaderIDAndDepth.getFirst())];
-Source.Cost = HeaderIDAndDepth.getSecond() * ProxOpts.IncludeCost;
+  for (auto &Entry : Includes.includeDepth(
+   SM.getFileEntryForID(SM.getMainFileID())->getName())) {
+auto &Source = ProxSources[Entry.getKey()];
+Source.Cost = Entry.getValue() * ProxOpts.IncludeCost;
 // Symbols near our transitive includes are good, but only consider
 // things in the same directory or below it. Otherwise there can be
 // many false positives.
-if (HeaderIDAndDepth.getSecond() > 0)
+if (Entry.getValue() > 0)
   Source.MaxUpTraversals = 1;
   }
   FileProximity.emplace(ProxSources, ProxOpts);

diff  --git a/clang-tools-extra/clangd/Headers.cpp 
b/clang-tools-extra/clangd/Headers.cpp
index 33304b818d6f4..111b05849b81b 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -67,9 +67,8 @@ class RecordHeaders : public PPCallbacks {
 // Treat as if included from the main file.
 IncludingFileEntry = SM.getFileEntryForID(MainFID);
   }
-  auto IncludingID = Out->getOrCreateID(IncludingFileEntry),
-   IncludedID = Out->getOrCreateID(File);
-  Out->IncludeChildren[IncludingID].push_back(IncludedID);
+  Out->recordInclude(IncludingFileEntry->getName(), File->getName(),
+ File->tryGetRealPathName());
 }
   }
 
@@ -155,41 +154,38 @@ collectIncludeStructureCallback(const SourceManager &SM,
   return std::make_unique(SM, Out);
 }
 
-llvm::Optional
-IncludeStructure::getID(const FileEntry *Entry) const {
-  auto It = NameToIndex.find(Entry->getName());
-  if (It == NameToIndex.end())
-return llvm::None;
-  return It->second;
+void IncludeStructure::recordInclude(llvm::StringRef IncludingName,
+ llvm::StringRef IncludedName,
+ llvm::StringRef IncludedRealName) {
+  auto Child = fileIndex(IncludedName);
+  if (!IncludedRealName.empty() && RealPathNames[Child].empty())
+RealPathNames[Child] = std::string(IncludedRealName);
+  auto Parent = fileIndex(IncludingName);
+  IncludeChildren[Parent].push_back(Child);
 }
 
-IncludeStructure::HeaderID
-IncludeStructure::getOrCreateID(const FileEntry *Entry) {
-  auto R = NameToIndex.try_emplace(
-  Entry->getName(),
-  static_cast(RealPathNames.size()));
+unsigned IncludeStructure::fileIndex(llvm::StringRef Name) {
+  auto R = NameToIndex.try_emplace(Name, RealPathNames.size());
   if (R.second)
 RealPathNames.emplace_back();
-  IncludeStructure::HeaderID Result = R.first->getValue();
-  std::string &RealPathName = RealPathNames[static_cast(Result)];
-  if (RealPathName.empty())
-RealPathName = Entry->tryGetRealPathName().str();
-  return Result;
+  return R.first->getValue();
 }
 
-llvm::DenseMap
-IncludeStructure::includeDepth(HeaderID Root) const {
+llvm::StringMap
+IncludeStructure::includeDepth(llvm::StringRef Root) const {
   // Include depth 0 is the main file only.
-  llvm::DenseMap Result;
-  assert(static_cast(Root) < RealPathNames.size());
+  llvm::StringMap Result;
   Result[Root] = 0;
-  std::vector CurrentLevel;
-  CurrentLevel.push_back(Root);
-  llvm::DenseSet Seen;
-  Seen.insert(Root);
+  std::vector CurrentLevel;
+  llvm::DenseSet Seen;
+  auto It = NameToIndex.find(Root);
+  if (It != NameToIn

[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Thanks for the clarification, makes sense! I got the notification about the 
build failure, thanks for fixing. I actually added it in the first place but 
then noticed that checkers typically didn't include any STL header so I thought 
maybe there was a restriction on their usage or something. The code compiled 
just fine when running the "clang-check-tools" target. Good to know for the 
next time :)

PS: I've also reported the build errors in pre-merge, maybe they would have 
caught it earlier.


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

https://reviews.llvm.org/D110493

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


[PATCH] D108247: [CUDA] Improve CUDA version detection and diagnostics.

2021-09-27 Thread Raul Tambre via Phabricator via cfe-commits
tambre added a comment.

In D108247#3025238 , @tra wrote:

> So, what's the current state of affairs regarding CUDA SDK layout in debian?

`/usr/bin`: most executables 
, 
`nvcc` and `nvprof` shims
`/usr/lib/nvidia-cuda-toolkit/bin`: `cicc`, `crt`, `nvcc`, `nvprof`, `g++` and 
`gcc` wrappers for supporting different compilers
`/usr/lib/nvidia-cuda-toolkit/libdevice`: `libdevice.10.bc`
`/usr/lib/cuda/{bin,include,lib64}`: empty directories
`/usr/lib/cuda/nvvm/libdevice`: symlinked to 
`/usr/lib/nvidia-cuda-toolkit/libdevice`
`/usr/include`: headers

> CUDA SDK no longer ships version.txt, so cuda.h is the only reliable 
> mechanism for detecting CUDA version and clang does need to know it.
> In case the version has not been found we also can not choose an arbitrary 
> old version as the default.

Agreed, sensible.

> So, one workaround would be to install CUDA-11.4. AFAICT, we do find the 
> headers and ptxas, but misdetect CUDA version.

I don't think that's going to be acceptable for most distributions, which might 
ship a Clang supporting a newer CUDA than the one they ship.

> Another workaround would be to place a fake /usr/lib/cuda/include/cuda.h with 
> something like this:

My CMake CI bot [[ https://open.cdash.org/test/498767666 | encountered `cmath` 
template errors ]] after I manually symlinked `/usr/lib/cuda/include` to 
`/usr/include`, though that solved the version detection.
Adding a shim like the one you show fixes everything and compiling works.

> Clang does rely on very particular ordering of includes, so having CUDA 
> headers in a location clang does not expect will lead to issues sooner or 
> later.

Having the headers in the system-wide `/usr/include` shouldn't be a problem 
though, right?
I'm not quite sure why simply having `/usr/lib/cuda/include` → `/usr/include` 
symlink breaks as seen on my bot.
Prior to this an empty `/usr/lib/cuda/include` and Clang finding them through 
the regular include search path worked.

> If the headers are not available where --cuda-path points clang to, I'm not 
> sure what clang is supposed to do. Second-guessing explicit user input is not 
> a good idea in general.

I agree that second-guessing is bad, but expecting it to be available in the 
include search path otherwise seems reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108247

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


[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D110493#3025394 , @carlosgalvezp 
wrote:

> Super, thanks! It went really smooth :)
>
> Newbie question: I noticed that `[clang-tidy]` is no longer part of the 
> commit message, I thought the intention was to keep it to more easily glance 
> through the commit history?

We usually put `[project]` markings in the subject so it's easier for code 
reviewers to know what PRs are in an area of interest to them, and I suppose I 
could keep them as part of the commit message. But the commit message "title" 
is supposed to be concise (I hear 50 characters is a common measure for it), so 
I usually remove the [project] marker reflexively to keep the title of the 
commit brief.

Btw, this patch failed some bots because of a missing #include which I've added 
in ef0f728abe6ec43f2f75082c9b47ec7fade2ead2 
.


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

https://reviews.llvm.org/D110493

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


[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-27 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

In D103938#3025265 , @aaron.ballman 
wrote:

> In D103938#3024938 , @ychen wrote:
>
>> In D103938#3018918 , @ychen wrote:
>>
>>> In D103938#3018588 , 
>>> @aaron.ballman wrote:
>>>
 In D103938#3013503 , @thakis 
 wrote:

> This flags this code from absl:
>
>   template  typename std::enable_if::value, 
> int>::type =
> (GenT{}, 0)>
>   constexpr FlagDefaultArg DefaultArg(int) {
> return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kOneWord};
>   }
>
> (https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/flags/internal/flag.h;l=293?q=third_party%2Fabseil-cpp%2Fabsl%2Fflags%2Finternal%2Fflag.h)
>
>   ../../third_party/abseil-cpp/absl/flags/internal/flag.h:293:16: 
> warning: left operand of comma operator has no effect [-Wunused-value]
> (GenT{}, 0)>
>  ^   ~~
>   ../../third_party/abseil-cpp/absl/flags/internal/flag.h:293:16: 
> warning: left operand of comma operator has no effect [-Wunused-value]
> (GenT{}, 0)>
>  ^   ~~
>
> I guess it has a SFINAE effect there?

 Sorry for having missed this comment before when reviewing the code 
 @thakis, thanks for pinging on it! That does look like a false positive 
 assuming it's being used for SFINAE. @ychen, can you take a look (and 
 revert if it looks like it'll take a while to resolve)?
>>>
>>> @thakis sorry for missing that. Reverted. I'll take a look.
>>
>> Diagnoses are suppressed in SFINAE context by default 
>> (https://github.com/llvm/llvm-project/blob/3dbf27e762008757c0de7034c778d941bfeb0388/clang/include/clang/Basic/Diagnostic.td#L83,
>>  related code is `Sema::EmitCurrentDiagnostic`). The test case triggered the 
>> warning because the substitution is successful for that overload candidate. 
>> When substitution failed, the warning is suppressed as expected. The test 
>> case I used is
>>
>>   #include
>>   
>>   struct A {
>> int value;
>>   };
>>   
>>   template > typename std::enable_if::value, 
>> int>::type =
>>  (GenT{},0)>
>>   constexpr int DefaultArg(int) {
>> return GenT{}.value;
>>   }
>>   
>>   template 
>>   constexpr int DefaultArg(double) {
>> return 1;
>>   }
>>   
>>   int foo() {
>>   return DefaultArg(0);
>>   }
>>   int bar() {
>>   return DefaultArg(0);
>>   }
>>
>> So I think the behavior is expected and it seems absl already patch that 
>> code with `(void)`. Is it ok with you to reland the patch @aaron.ballman  
>> @thakis ? Thanks
>
> That will only help users who have newer versions of abseil. Also, I'm a bit 
> worried this may be a code pattern used by more than just abseil.
>
> The diagnostic text says that the comma operator has no effect, but this is a 
> case where it does have a compile-time effect so the diagnostic sounds 
> misleading. This suggests to me that we should silence the diagnostic in this 
> case (because we want on-by-default diagnostics to be as close to free from 
> false positives as possible). However, does silencing this cause significant 
> issues with false negatives in other code examples?

This makes great sense to me. IIUC, only constant-evaluation context could be 
in SFINAE context, so a check for this specific case should only affect the 
constant-evaluation context this patch is newly addressing. I'll update the 
patch and run some tests. Thanks for the quick reply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

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


[clang-tools-extra] ef0f728 - Add a missing include to appease the build bots

2021-09-27 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2021-09-27T14:19:39-04:00
New Revision: ef0f728abe6ec43f2f75082c9b47ec7fade2ead2

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

LOG: Add a missing include to appease the build bots

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
index dc2882418d35..badbf2a0d96f 100644
--- a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -13,6 +13,7 @@
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
+#include 
 
 using namespace clang::ast_matchers;
 



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


[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Super, thanks! It went really smooth :)

Newbye question: I noticed that `[clang-tidy]` is no longer part of the commit 
message, I thought the intention was to keep it to more easily glance through 
the commit history?


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

https://reviews.llvm.org/D110493

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


[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

2021-09-27 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D89013#3022332 , @sylvestre.ledru 
wrote:

> @phosek
> It was probably my fault (mix of different in-tree / out of tree builds)
> Quick question, why the path to `__config_site` contains this triple
> `x86_64-pc-linux-gnu`
> (in `/usr/lib/llvm-14/include/x86_64-pc-linux-gnu/c++/v1/__config_site`)
> when it is usually:
> `x86_64-linux-gnu`
> (does not contain `-pc`)
> thanks?

See the comments earlier in this review (such as 
https://reviews.llvm.org/D89013#2662244) where we discussed whether to use 
Debian multiarch triples or normalized LLVM triples. We've decided to use 
normalized LLVM triples to reduce the amount of custom logic needed and 
increase consistency across platforms.

The reason why `__config_site` uses `x86_64-pc-linux-gnu` is because that's the 
default target that was used by the build (you can also override it via 
`-DLLVM_DEFAULT_TARGET_TRIPLE=`). We would normalize `x86_64-linux-gnu` to 
`x86_64-unknown-linux-gnu`. The `-pc` vendor component was returned by the 
`config.guess` script, I believe that's going to be addressed by removal of 
that script in D109837 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89013

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


[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

In D110493#3025343 , @carlosgalvezp 
wrote:

> Awesome, thanks a lot for the review! :) Sure, you can use the following to 
> commit on my behalf, since I don't think I have such permissions:
>
> Carlos Galvez
> carlosgalv...@gmail.com

Thanks! I've commit on your behalf in b2a2c38349a18b89b04d342632d5ea02f86dfdd6 
.


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

https://reviews.llvm.org/D110493

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


[clang-tools-extra] b2a2c38 - Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Aaron Ballman via cfe-commits

Author: Carlos Galvez
Date: 2021-09-27T14:03:53-04:00
New Revision: b2a2c38349a18b89b04d342632d5ea02f86dfdd6

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

LOG: Fix bug in readability-uppercase-literal-suffix

Fixes https://bugs.llvm.org/show_bug.cgi?id=51790. The check triggers
incorrectly with non-type template parameters.

A bisect determined that the bug was introduced here:
https://github.com/llvm/llvm-project/commit/ea2225a10be986d226e041d20d36dff17e78daed

Unfortunately that patch can no longer be reverted on top of the main
branch, so add a fix instead. Add a unit test to avoid regression in
the future.

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/readability-uppercase-literal-suffix-integer.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
index 6abb5c3b877d6..dc2882418d358 100644
--- a/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -134,6 +134,11 @@ shouldReplaceLiteralSuffix(const Expr &Literal,
   CharSourceRange::getTokenRange(*Range), SM, LO, &Invalid);
   assert(!Invalid && "Failed to retrieve the source text.");
 
+  // Make sure the first character is actually a digit, instead of
+  // something else, like a non-type template parameter.
+  if (!std::isdigit(static_cast(LiteralSourceText.front(
+return llvm::None;
+
   size_t Skip = 0;
 
   // Do we need to ignore something before actually looking for the suffix?

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-uppercase-literal-suffix-integer.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-uppercase-literal-suffix-integer.cpp
index a6f38a8e7f261..847501f098f4a 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-uppercase-literal-suffix-integer.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-uppercase-literal-suffix-integer.cpp
@@ -270,3 +270,29 @@ void c() { l &a(); }
 void d();
 void d() { c(); }
 } // namespace
+
+// Check that non-type template parameters do not cause any diags.
+// https://bugs.llvm.org/show_bug.cgi?id=51790
+template 
+struct Vector {
+  static constexpr int kCapacity = capacity;
+};
+
+template 
+constexpr int Vector::kCapacity;
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:22: warning: integer literal has suffix 
'ity', which is not uppercase
+
+template 
+struct Foo {
+  static constexpr int kFoo = foo1u;
+};
+
+template 
+constexpr int Foo::kFoo;
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:19: warning: integer literal has suffix 
'u', which is not uppercase
+
+// The template needs to be instantiated for diagnostics to show up
+void test_non_type_template_parameter() {
+  int x = Vector<10>::kCapacity;
+  int f = Foo<10>::kFoo;
+}



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


[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Awesome, thanks a lot for the review! :) Sure, you can use the following to 
commit on my behalf, since I don't think I have such permissions:

Carlos Galvez
carlosgalv...@gmail.com


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

https://reviews.llvm.org/D110493

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


[PATCH] D110525: [docs] fix docs for bugprone-virtual-near-miss & performance-type-promotion-in-math-fn

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'm not certain how, but somehow this review is set to be uneditable, so I 
cannot mark it as accepted (or close it later once it lands).

> bakinovsky-m created this object with edit policy "Administrators".

I think you may need to edit the review's policy so it goes back to whatever 
the default is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110525

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


[PATCH] D110525: [docs] fix docs for bugprone-virtual-near-miss & performance-type-promotion-in-math-fn

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thanks for the fixes, LGTM! Do you need me to commit on your behalf? If so, 
please let me know what name and email address you would like me to use for 
patch attribution in git.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110525

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


[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Sorry, I still don't understand this response:

> We have 3 sets weak symbols here: weak_func, profc_weak_foo, profd_weak_func. 
> We can't ensure that binder always choose 3 of them from same object.

So this part of the description isn't clear to me:

> However, on AIX, the current binder can NOT discard the weak symbols if we 
> put all of them into the same csect, as binder can NOT discard only part of a 
> csect.
>
> This creates a unique challenge for using those symbols to calculate some 
> relative offset.

Say a.o has a weak `foo, __profc_foo, __profd_foo`. The `__profd_foo` 
references the `__profc_foo`

b.o has another set of weak `foo, __profc_foo, __profd_foo`. The `__profd_foo` 
references the `__profc_foo`.

What's the linker (called binder?) behavior? "We can't ensure that binder 
always choose 3 of them from same object."?
But is that a problem? For linkonce_odr functions, they should have identical 
semantics in a.o and b.o.
(There can be ODR issues if you use advanced PGO techniques like value 
profiling, but I assume that it does not work and will be difficult to make 
work and more importantly, it is also of lower value anyway.)

So the linker picking the a.o `foo` and the b.o `__profc_foo/__profd_foo` isn't 
a poblem.

I understand that PrivateLinkage can avoid the issues but you need to justify 
the complexity in `InstrProfiling.cpp` with the value.
Currently I don't think using weak symbol is a big problem, especially if the 
AIX linker will be fixed anyway in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D110493: [clang-tidy] Fix bug in readability-uppercase-literal-suffix

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you! Would you like me to commit on your behalf? If so, please let 
me know what name and email address you'd like me to use for patch attribution 
in git.


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

https://reviews.llvm.org/D110493

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


[PATCH] D110422: [AIX] Enable PGO without LTO

2021-09-27 Thread Jinsong Ji via Phabricator via cfe-commits
jsji added a comment.

In D110422#3025133 , @MaskRay wrote:

> The description isn't clear why the previous `weak` symbol usage does not 
> work. Have you verified that it does not work?

Yes, we started with the `weak` symbols for `profc`/`profd` first, and tested 
it. We met problems, and we spent time investigate why they are not working.

I have update the description to include most of the details. Let me know if it 
is still unclear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110422

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


[PATCH] D103938: Diagnose -Wunused-value based on CFG reachability

2021-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D103938#3024938 , @ychen wrote:

> In D103938#3018918 , @ychen wrote:
>
>> In D103938#3018588 , 
>> @aaron.ballman wrote:
>>
>>> In D103938#3013503 , @thakis 
>>> wrote:
>>>
 This flags this code from absl:

   template >>> typename std::enable_if::value, 
 int>::type =
 (GenT{}, 0)>
   constexpr FlagDefaultArg DefaultArg(int) {
 return {FlagDefaultSrc(GenT{}.value), FlagDefaultKind::kOneWord};
   }

 (https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/flags/internal/flag.h;l=293?q=third_party%2Fabseil-cpp%2Fabsl%2Fflags%2Finternal%2Fflag.h)

   ../../third_party/abseil-cpp/absl/flags/internal/flag.h:293:16: warning: 
 left operand of comma operator has no effect [-Wunused-value]
 (GenT{}, 0)>
  ^   ~~
   ../../third_party/abseil-cpp/absl/flags/internal/flag.h:293:16: warning: 
 left operand of comma operator has no effect [-Wunused-value]
 (GenT{}, 0)>
  ^   ~~

 I guess it has a SFINAE effect there?
>>>
>>> Sorry for having missed this comment before when reviewing the code 
>>> @thakis, thanks for pinging on it! That does look like a false positive 
>>> assuming it's being used for SFINAE. @ychen, can you take a look (and 
>>> revert if it looks like it'll take a while to resolve)?
>>
>> @thakis sorry for missing that. Reverted. I'll take a look.
>
> Diagnoses are suppressed in SFINAE context by default 
> (https://github.com/llvm/llvm-project/blob/3dbf27e762008757c0de7034c778d941bfeb0388/clang/include/clang/Basic/Diagnostic.td#L83,
>  related code is `Sema::EmitCurrentDiagnostic`). The test case triggered the 
> warning because the substitution is successful for that overload candidate. 
> When substitution failed, the warning is suppressed as expected. The test 
> case I used is
>
>   #include
>   
>   struct A {
> int value;
>   };
>   
>   template  typename std::enable_if::value, 
> int>::type =
>  (GenT{},0)>
>   constexpr int DefaultArg(int) {
> return GenT{}.value;
>   }
>   
>   template 
>   constexpr int DefaultArg(double) {
> return 1;
>   }
>   
>   int foo() {
>   return DefaultArg(0);
>   }
>   int bar() {
>   return DefaultArg(0);
>   }
>
> So I think the behavior is expected and it seems absl already patch that code 
> with `(void)`. Is it ok with you to reland the patch @aaron.ballman  @thakis 
> ? Thanks

That will only help users who have newer versions of abseil. Also, I'm a bit 
worried this may be a code pattern used by more than just abseil.

The diagnostic text says that the comma operator has no effect, but this is a 
case where it does have a compile-time effect so the diagnostic sounds 
misleading. This suggests to me that we should silence the diagnostic in this 
case (because we want on-by-default diagnostics to be as close to free from 
false positives as possible). However, does silencing this cause significant 
issues with false negatives in other code examples?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103938

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


[PATCH] D108247: [CUDA] Improve CUDA version detection and diagnostics.

2021-09-27 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

So, what's the current state of affairs regarding CUDA SDK layout in debian?
Clang does rely on very particular ordering of includes, so having CUDA headers 
in a location clang does not expect will lead to issues sooner or later.
If the headers are not available where --cuda-path points clang to, I'm not 
sure what clang is supposed to do. Second-guessing explicit user input is not a 
good idea in general.

CUDA SDK no longer ships version.txt, so cuda.h is the only reliable mechanism 
for detecting CUDA version and clang does need to know it.

In case the version has not been found we also can not choose an arbitrary old 
version as the default.

So, one workaround would be to install CUDA-11.4. AFAICT, we do find the 
headers and ptxas, but misdetect CUDA version.
Another workaround would be to place a fake /usr/lib/cuda/include/cuda.h with 
something like this:

  #pragma push_macro("CUDA_VERSION")
  #define CUDA_VERSION 11030
  #pragma pop_macro("CUDA_VERSION")
  #include_next 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108247

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


[PATCH] D110304: [HIP] Fix linking of asanrt.bc

2021-09-27 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rGc4afb5f81b62: [HIP] Fix linking of asanrt.bc (authored by 
yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D110304?vs=374868&id=375329#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110304

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
  clang/test/CodeGenCUDA/amdgpu-asan.cu
  clang/test/Driver/hip-sanitize-options.hip

Index: clang/test/Driver/hip-sanitize-options.hip
===
--- clang/test/Driver/hip-sanitize-options.hip
+++ clang/test/Driver/hip-sanitize-options.hip
@@ -34,7 +34,7 @@
 // CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}
 // CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
 
-// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
+// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]"
 // NORDC: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
 // NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
 
Index: clang/test/CodeGenCUDA/amdgpu-asan.cu
===
--- clang/test/CodeGenCUDA/amdgpu-asan.cu
+++ clang/test/CodeGenCUDA/amdgpu-asan.cu
@@ -1,6 +1,20 @@
+// Create a sample address sanitizer bitcode library.
+
+// RUN: %clang_cc1 -x ir -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm-bc \
+// RUN:   -disable-llvm-passes -o %t.asanrtl.bc %S/Inputs/amdgpu-asanrtl.ll
+
+// Check sanitizer runtime library functions survive
+// optimizations without being removed or parameters altered.
+
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -mlink-bitcode-file %t.asanrtl.bc -x hip \
+// RUN:   | FileCheck -check-prefix=ASAN %s
+
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
 // RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
-// RUN:   -x hip | FileCheck -check-prefix=ASAN %s
+// RUN:   -O3 -mlink-bitcode-file %t.asanrtl.bc -x hip \
+// RUN:   | FileCheck -check-prefix=ASAN %s
 
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
 // RUN:   -fcuda-is-device -target-cpu gfx906 -x hip \
@@ -8,8 +22,10 @@
 
 // REQUIRES: amdgpu-registered-target
 
-// ASAN-DAG: declare void @__amdgpu_device_library_preserve_asan_functions()
+// ASAN-DAG: define weak void @__amdgpu_device_library_preserve_asan_functions()
 // ASAN-DAG: @__amdgpu_device_library_preserve_asan_functions_ptr = weak addrspace(1) constant void ()* @__amdgpu_device_library_preserve_asan_functions
 // ASAN-DAG: @llvm.compiler.used = {{.*}}@__amdgpu_device_library_preserve_asan_functions_ptr
+// ASAN-DAG: define weak void @__asan_report_load1(i64 %{{.*}})
 
-// CHECK-NOT: @__amdgpu_device_library_preserve_asan_functions_ptr
+// CHECK-NOT: @__amdgpu_device_library_preserve_asan_functions
+// CHECK-NOT: @__asan_report_load1
Index: clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
===
--- /dev/null
+++ clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll
@@ -0,0 +1,13 @@
+; Sample code for amdgpu address sanitizer runtime.
+
+; Note the runtime functions need to have weak linkage and default
+; visibility, otherwise they may be internalized and removed by GlobalOptPass.
+
+define weak void @__amdgpu_device_library_preserve_asan_functions() {
+  tail call void @__asan_report_load1(i64 0)
+  ret void
+}
+
+define weak void @__asan_report_load1(i64 %0) {
+  ret void
+}
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -83,7 +83,7 @@
llvm::opt::ArgStringList &CC1Args) const override;
   void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args) const override;
-  llvm::SmallVector
+  llvm::SmallVector
   getHIPDeviceLibs(const llvm::opt::ArgList &Args) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -88,8 +88,8 @@
 
  

[clang] c4afb5f - [HIP] Fix linking of asanrt.bc

2021-09-27 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2021-09-27T13:25:46-04:00
New Revision: c4afb5f81b62b903e4af8a92235e1b901e184040

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

LOG: [HIP] Fix linking of asanrt.bc

HIP currently uses -mlink-builtin-bitcode to link all bitcode libraries, which
changes the linkage of functions to be internal once they are linked in. This
works for common bitcode libraries since these functions are not intended
to be exposed for external callers.

However, the functions in the sanitizer bitcode library is intended to be
called by instructions generated by the sanitizer pass. If their linkage is
changed to internal, their parameters may be altered by optimizations before
the sanitizer pass, which renders them unusable by the sanitizer pass.

To fix this issue, HIP toolchain links the sanitizer bitcode library with
-mlink-bitcode-file, which does not change the linkage.

A struct BitCodeLibraryInfo is introduced in ToolChain as a generic
approach to pass the bitcode library information between ToolChain and Tool.

Reviewed by: Artem Belevich

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

Added: 
clang/test/CodeGenCUDA/Inputs/amdgpu-asanrtl.ll

Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/HIP.cpp
clang/lib/Driver/ToolChains/HIP.h
clang/test/CodeGenCUDA/amdgpu-asan.cu
clang/test/Driver/hip-sanitize-options.hip

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index 882ae40086cea..fa5095e98018b 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -113,6 +113,13 @@ class ToolChain {
 RM_Disabled,
   };
 
+  struct BitCodeLibraryInfo {
+std::string Path;
+bool ShouldInternalize;
+BitCodeLibraryInfo(StringRef Path, bool ShouldInternalize = true)
+: Path(Path), ShouldInternalize(ShouldInternalize) {}
+  };
+
   enum FileType { FT_Object, FT_Static, FT_Shared };
 
 private:
@@ -681,7 +688,7 @@ class ToolChain {
   const llvm::opt::ArgList &Args) 
const;
 
   /// Get paths of HIP device libraries.
-  virtual llvm::SmallVector
+  virtual llvm::SmallVector
   getHIPDeviceLibs(const llvm::opt::ArgList &Args) const;
 
   /// Return sanitizers which are available in this toolchain.

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 7272cc29cc7c0..302bfb348b29f 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1026,7 +1026,7 @@ void ToolChain::AddCudaIncludeArgs(const ArgList 
&DriverArgs,
 void ToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
   ArgStringList &CC1Args) const {}
 
-llvm::SmallVector
+llvm::SmallVector
 ToolChain::getHIPDeviceLibs(const ArgList &DriverArgs) const {
   return {};
 }

diff  --git a/clang/lib/Driver/ToolChains/HIP.cpp 
b/clang/lib/Driver/ToolChains/HIP.cpp
index c4e840de86e15..665d5bab7218d 100644
--- a/clang/lib/Driver/ToolChains/HIP.cpp
+++ b/clang/lib/Driver/ToolChains/HIP.cpp
@@ -88,8 +88,8 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, 
const JobAction &JA,
 
   if (Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
false))
-llvm::for_each(TC.getHIPDeviceLibs(Args), [&](StringRef BCFile) {
-  LldArgs.push_back(Args.MakeArgString(BCFile));
+llvm::for_each(TC.getHIPDeviceLibs(Args), [&](auto BCFile) {
+  LldArgs.push_back(Args.MakeArgString(BCFile.Path));
 });
 
   const char *Lld = Args.MakeArgString(getToolChain().GetProgramPath("lld"));
@@ -276,9 +276,10 @@ void HIPToolChain::addClangTargetOptions(
 CC1Args.push_back("-fapply-global-visibility-to-externs");
   }
 
-  llvm::for_each(getHIPDeviceLibs(DriverArgs), [&](StringRef BCFile) {
-CC1Args.push_back("-mlink-builtin-bitcode");
-CC1Args.push_back(DriverArgs.MakeArgString(BCFile));
+  llvm::for_each(getHIPDeviceLibs(DriverArgs), [&](auto BCFile) {
+CC1Args.push_back(BCFile.ShouldInternalize ? "-mlink-builtin-bitcode"
+   : "-mlink-bitcode-file");
+CC1Args.push_back(DriverArgs.MakeArgString(BCFile.Path));
   });
 }
 
@@ -359,9 +360,9 @@ VersionTuple HIPToolChain::computeMSVCVersion(const Driver 
*D,
   return HostTC.computeMSVCVersion(D, Args);
 }
 
-llvm::SmallVector
+llvm::SmallVector
 HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const {
-  llvm::SmallVector BCLibs;
+  llvm::SmallVector BCLibs;
   if (DriverArgs.hasArg(options::OPT_nogpulib))
 return {};
   ArgStringList LibraryPaths;
@@ -382,7 +383,7 @@ HIPToolChain::getHIPDeviceLibs(const llvm::opt::ArgList 
&Dr

[PATCH] D105759: [WIP] Implement P2361 Unevaluated string literals

2021-09-27 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Lex/LiteralSupport.cpp:95-96
+  case '?':
+  case 'n':
+  case 't':
+return true;

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > cor3ntin wrote:
> > > > > aaron.ballman wrote:
> > > > > > Do you intend to miss a bunch of escapes like `\'` and `\r` (etc)?
> > > > > \' is there. I am less sure about '\r' and '\a'. for example. This is 
> > > > > something I realized after writing P2361.
> > > > > what does '\a` in static assert mean? even '\r' is not so obvious
> > > > Looking at the list again, I think only `\a` is really of interest 
> > > > here. I know some folks like @jfb have mentioned that `\a` could be 
> > > > used to generate an alert sound on a terminal, which is a somewhat 
> > > > useful feature for a failed static assertion if you squint at it hard 
> > > > enough.
> > > > 
> > > > But the rest of the missing ones do seem more questionable to support.
> > > @jfb and @cor3ntin -- any opinions on whether `\a` should be supported? 
> > > My opinion is that it should be supported because it has some utility for 
> > > anyone running the compiler from a command line, but it's a pretty weak 
> > > opinion.
> > I might consider rejecting ANY character escape in the less-than-32 part of 
> > the table.  
> > 
> > For consistency at least, I don't see value in allowing \a if we're 
> > rejecting layout things like \t.
> But that's just it, we're accepting `\t` and `\n` with this code.
Ah!  I missed that this is an allow-list instead of a deny-list.  That makes me 
way more comfortable with this code.

IMO, I'd suggest we we allow '\r' (since wouldn't we have problems on Windows 
at that point, being unable to accept a printable newline for windows?), but 
disallow `\a` for now unless someone comes up with a really good reason to 
allow it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105759

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


  1   2   >