date:20201124

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:86
+
+  auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel3,

the order of elements in `IncomingLevel2` is not sorted, so on some buildbots 
you are receiving "caller3" as the first element of the result, and "caller2" 
in others.

sorry for missing it the first time, it might make sense to sort containers by 
name before returning from `incomingCalls`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D92000: [clangd] Collect main file refs by default

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

i remember discussing this in the past, and having "negative" feelings about 
it. I couldn't find the discussion on the issues page tho, could you toss me 
the link if you can find it? (it might've been an offline discussion as well, 
sorry if that's the case for not dumping a summary).

But if there were no discussions before, it would be nice to see some numbers, 
for increase in memory and disk usage for LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92000

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

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

2020-11-24 Thread Nathan Ridge via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp


Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -76,12 +76,12 @@
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-
FromRanges(Source.range("Caller1C");
+  EXPECT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
   EXPECT_THAT(IncomingLevel3,
@@ -121,8 +121,8 @@
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
   EXPECT_THAT(IncomingLevel2,
-  UnorderedElementsAre(AllOf(From(WithName("caller2")),
- 
FromRanges(Source.range("Caller1");
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -149,10 +149,10 @@
   EXPECT_THAT(Items, ElementsAre(WithName("Waldo::find")));
   auto Incoming = incomingCalls(Items[0], Index.get());
   EXPECT_THAT(Incoming,
-  UnorderedElementsAre(AllOf(From(WithName("caller1")),
- FromRanges(Source.range("Caller1"))),
-   AllOf(From(WithName("caller2")),
- 
FromRanges(Source.range("Caller2");
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -219,9 +219,9 @@
   FromRanges(Caller1C.range();
 
 auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-EXPECT_THAT(IncomingLevel2,
-UnorderedElementsAre(
-AllOf(From(WithName("caller2")),
+EXPECT_THAT(
+IncomingLevel2,
+ElementsAre(AllOf(From(WithName("caller2")),
   FromRanges(Caller2C.range("A"), 
Caller2C.range("B"))),
 AllOf(From(WithName("caller3")),
   FromRanges(Caller3C.range("Caller1");
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1716,6 +1716,11 @@
   Results.push_back(
   CallHierarchyIncomingCall{std::move(*CHI), std::move(It->second)});
   });
+  // Sort results by name of container.
+  llvm::sort(Results, [](const CallHierarchyIncomingCall &A,
+ const CallHierarchyIncomingCall &B) {
+return A.from.name < B.from.name;
+  });
   return Results;
 }
 


Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -76,12 +76,12 @@
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-FromRanges(Sou

[PATCH] D92000: [clangd] Collect main file refs by default

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

NVM, found the summary 
https://github.com/clangd/clangd/issues/162#issuecomment-653981038


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92000

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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

2020-11-24 Thread Nathan Ridge via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:86
+
+  auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel3,

kadircet wrote:
> the order of elements in `IncomingLevel2` is not sorted, so on some buildbots 
> you are receiving "caller3" as the first element of the result, and "caller2" 
> in others.
> 
> sorry for missing it the first time, it might make sense to sort containers 
> by name before returning from `incomingCalls`
Thanks for catching that! Patch at https://reviews.llvm.org/D92000


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

2020-11-24 Thread Nathan Ridge via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:86
+
+  auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel3,

nridge wrote:
> kadircet wrote:
> > the order of elements in `IncomingLevel2` is not sorted, so on some 
> > buildbots you are receiving "caller3" as the first element of the result, 
> > and "caller2" in others.
> > 
> > sorry for missing it the first time, it might make sense to sort containers 
> > by name before returning from `incomingCalls`
> Thanks for catching that! Patch at https://reviews.llvm.org/D92000
Sorry, wrong link, correct link is: https://reviews.llvm.org/D92009


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D92000: [clangd] Collect main file refs by default

2020-11-24 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

Note, the call hierarchy feature is pretty significantly impaired without this. 
See the example of `Selection::createEach()` discussed in this comment 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92000

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

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

Thanks!




Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:79
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-
FromRanges(Source.range("Caller1C");
+  EXPECT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),

let's also change these from EXPECT_ to ASSERT_. As we are dereferencing 
elements in the following calls. Same in other places too.



Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:92
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
   EXPECT_THAT(IncomingLevel4, ElementsAre());
 }

s/ElementsAre()/IsEmpty()/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92009

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

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

2020-11-24 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 307262.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -31,6 +31,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
@@ -69,27 +70,27 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-FromRanges(Source.range("Caller1C");
+  ASSERT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel3,
+  ASSERT_THAT(IncomingLevel3,
   ElementsAre(AllOf(From(WithName("caller3")),
 FromRanges(Source.range("Caller2");
 
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel4, ElementsAre());
+  EXPECT_THAT(IncomingLevel4, IsEmpty());
 }
 
 TEST(CallHierarchy, MainFileOnlyRef) {
@@ -113,16 +114,16 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
   EXPECT_THAT(IncomingLevel2,
-  UnorderedElementsAre(AllOf(From(WithName("caller2")),
- FromRanges(Source.range("Caller1");
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -146,13 +147,13 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("Waldo::find")));
+  ASSERT_THAT(Items, ElementsAre(WithName("Waldo::find")));
   auto Incoming = incomingCalls(Items[0], Index.get());
   EXPECT_THAT(Incoming,
-  UnorderedElementsAre(AllOf(From(WithName("caller1")),
- FromRanges(Source.range("Caller1"))),
-   AllOf(From(WithName("caller2")),
- FromRanges(Source.range("Caller2");
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -212,27 +213,27 @@
   auto CheckCallHierarchy = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
 std::vector Items =
 prepareCallHierarchy(AST, Pos, TUPath);
-EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+ASSERT_THAT(Items, ElementsAre(WithName("callee")));
 auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-EXPECT_THAT(IncomingLevel1,
+ASSERT_THAT(IncomingLevel1,
 ElementsAre(AllOf(From(WithName("caller1")),

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

2020-11-24 Thread Nathan Ridge 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 rG5b6f47595bab: [clangd] Sort results of incomingCalls request 
by container name (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -31,6 +31,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
@@ -69,27 +70,27 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-FromRanges(Source.range("Caller1C");
+  ASSERT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel3,
+  ASSERT_THAT(IncomingLevel3,
   ElementsAre(AllOf(From(WithName("caller3")),
 FromRanges(Source.range("Caller2");
 
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel4, ElementsAre());
+  EXPECT_THAT(IncomingLevel4, IsEmpty());
 }
 
 TEST(CallHierarchy, MainFileOnlyRef) {
@@ -113,16 +114,16 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
   EXPECT_THAT(IncomingLevel2,
-  UnorderedElementsAre(AllOf(From(WithName("caller2")),
- FromRanges(Source.range("Caller1");
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -146,13 +147,13 @@
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("Waldo::find")));
+  ASSERT_THAT(Items, ElementsAre(WithName("Waldo::find")));
   auto Incoming = incomingCalls(Items[0], Index.get());
   EXPECT_THAT(Incoming,
-  UnorderedElementsAre(AllOf(From(WithName("caller1")),
- FromRanges(Source.range("Caller1"))),
-   AllOf(From(WithName("caller2")),
- FromRanges(Source.range("Caller2");
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -212,27 +213,27 @@
   auto CheckCallHierarchy = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
 std::vector Items =
 prepareCallHierarchy(AST, Pos, TUPath);
-EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+ASSERT_THAT(Items, ElementsAre(WithName("callee")));
 auto IncomingLevel1 = incomingCalls(Ite

[clang-tools-extra] 5b6f475 - [clangd] Sort results of incomingCalls request by container name

2020-11-24 Thread Nathan Ridge via cfe-commits


Author: Nathan Ridge
Date: 2020-11-24T03:29:02-05:00
New Revision: 5b6f47595bab686c6c3351fc1bd1f564add80dbf

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

LOG: [clangd] Sort results of incomingCalls request by container name

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index e319636f9076..31e963cb853f 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1716,6 +1716,11 @@ incomingCalls(const CallHierarchyItem &Item, const 
SymbolIndex *Index) {
   Results.push_back(
   CallHierarchyIncomingCall{std::move(*CHI), std::move(It->second)});
   });
+  // Sort results by name of container.
+  llvm::sort(Results, [](const CallHierarchyIncomingCall &A,
+ const CallHierarchyIncomingCall &B) {
+return A.from.name < B.from.name;
+  });
   return Results;
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp 
b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
index ce192466b442..8b5069d8dae8 100644
--- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -31,6 +31,7 @@ namespace {
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
@@ -69,27 +70,27 @@ TEST(CallHierarchy, IncomingOneFile) {
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-  AllOf(From(WithName("caller2")),
-FromRanges(Source.range("Caller1A"),
-   Source.range("Caller1B"))),
-  AllOf(From(WithName("caller3")),
-
FromRanges(Source.range("Caller1C");
+  ASSERT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel3,
+  ASSERT_THAT(IncomingLevel3,
   ElementsAre(AllOf(From(WithName("caller3")),
 FromRanges(Source.range("Caller2");
 
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel4, ElementsAre());
+  EXPECT_THAT(IncomingLevel4, IsEmpty());
 }
 
 TEST(CallHierarchy, MainFileOnlyRef) {
@@ -113,16 +114,16 @@ TEST(CallHierarchy, MainFileOnlyRef) {
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
   ElementsAre(AllOf(From(WithName("caller1")),
 FromRanges(Source.range("Callee");
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
   EXPECT_THAT(IncomingLevel2,
-  UnorderedElementsAre(AllOf(From(WithName("caller2")),
- 
FromRanges(Source.range("Caller1");
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -146,13 +147,13 @@ TEST(CallHierarchy, IncomingQualified) {
 
   std::vector Items =
   prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("Waldo::find")));
+  ASSERT_THAT(Items, ElementsAre(WithName("Waldo::find")));
   auto Incoming = incomingCalls(Items[0]

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Sergey Dmitriev via Phabricator via cfe-commits

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

Use a different container that preserves existing elements on modification
for storing temporary file names. Current container can make StringRefs
returned earlier invalid on reallocation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92010

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -394,7 +395,7 @@
 if (std::error_code EC =
 sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
   return createFileError(File, EC);
-Files.push_back(File);
+Files.push_front(File);
 
 if (Contents) {
   std::error_code EC;
@@ -403,11 +404,11 @@
 return createFileError(File, EC);
   OS.write(Contents->data(), Contents->size());
 }
-return Files.back();
+return Files.front();
   }
 
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };
 
 } // end anonymous namespace


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -394,7 +395,7 @@
 if (std::error_code EC =
 sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
   return createFileError(File, EC);
-Files.push_back(File);
+Files.push_front(File);
 
 if (Contents) {
   std::error_code EC;
@@ -403,11 +404,11 @@
 return createFileError(File, EC);
   OS.write(Contents->data(), Contents->size());
 }
-return Files.back();
+return Files.front();
   }
 
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };
 
 } // end anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92012: [clangd][query-driver] Extract target

2020-11-24 Thread Aleksandr Platonov via Phabricator via cfe-commits

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

In some cases system includes extractions is not enough, we also need target 
specific defines.
The problems appears when clang default target is not the same as toolchain's 
one (GCC cross-compiler, MinGW on Windows).
After this patch `query-driver` also extracts target and adds 
`-target=` compile option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92012

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test

Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -12,6 +12,7 @@
 # RUN: echo '[ -z "${args##*"-isysroot=/isysroot"*}" ] || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo " $* " | grep " --sysroot /my/sysroot/path " || exit' >> %t.dir/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/my_driver.sh
+# RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir/ >&2' >> %t.dir/my_driver.sh
 # RUN: echo 'echo %t.dir/my/dir2/ >&2' >> %t.dir/my_driver.sh
@@ -45,7 +46,7 @@
   "uri": "file://INPUT_DIR/the-file.cpp",
   "languageId":"cpp",
   "version":1,
-  "text":"#include \n#include "
+  "text":"#include \n#include \n#if !defined(__ARM_ARCH) || !defined(__gnu_linux__)\n#error \"Invalid target\"\n#endif"
 }
   }
 }
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -56,18 +56,34 @@
 namespace clangd {
 namespace {
 
-std::vector parseDriverOutput(llvm::StringRef Output) {
+std::pair, std::string>
+parseDriverOutput(llvm::StringRef Output) {
   std::vector SystemIncludes;
+  std::string Target;
   const char SIS[] = "#include <...> search starts here:";
   const char SIE[] = "End of search list.";
+  const char TS[] = "Target: ";
   llvm::SmallVector Lines;
   Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 
   auto StartIt = llvm::find_if(
-  Lines, [SIS](llvm::StringRef Line) { return Line.trim() == SIS; });
+  Lines, [TS](llvm::StringRef Line) { return Line.trim().startswith(TS); });
+
+  if (StartIt != Lines.end()) {
+llvm::StringRef TargetLine = StartIt->trim();
+TargetLine.consume_front(TS);
+Target = TargetLine.str();
+++StartIt;
+  } else {
+// Start from the beginning if target was not found.
+StartIt = Lines.begin();
+  }
+  StartIt =
+  llvm::find_if(llvm::make_range(StartIt, Lines.end()),
+[SIS](llvm::StringRef Line) { return Line.trim() == SIS; });
   if (StartIt == Lines.end()) {
 elog("System include extraction: start marker not found: {0}", Output);
-return {};
+return {SystemIncludes, Target};
   }
   ++StartIt;
   const auto EndIt =
@@ -75,21 +91,21 @@
 [SIE](llvm::StringRef Line) { return Line.trim() == SIE; });
   if (EndIt == Lines.end()) {
 elog("System include extraction: end marker missing: {0}", Output);
-return {};
+return {SystemIncludes, Target};
   }
 
   for (llvm::StringRef Line : llvm::make_range(StartIt, EndIt)) {
 SystemIncludes.push_back(Line.trim().str());
 vlog("System include extraction: adding {0}", Line);
   }
-  return SystemIncludes;
+  return {SystemIncludes, Target};
 }
 
-std::vector
-extractSystemIncludes(PathRef Driver, llvm::StringRef Lang,
-  llvm::ArrayRef CommandLine,
-  const llvm::Regex &QueryDriverRegex) {
-  trace::Span Tracer("Extract system includes");
+std::pair, std::string>
+extractSystemIncludesAndTarget(PathRef Driver, llvm::StringRef Lang,
+   llvm::ArrayRef CommandLine,
+   const llvm::Regex &QueryDriverRegex) {
+  trace::Span Tracer("Extract system includes and target");
   SPAN_ATTACH(Tracer, "driver", Driver);
   SPAN_ATTACH(Tracer, "lang", Lang);
 
@@ -168,11 +184,15 @@
 return {};
   }
 
-  auto Includes = parseDriverOutput(BufOrError->get()->getBuffer());
-  log("System include extractor: successfully executed {0}, got includes: "
+  auto IncludesAndTarget = parseDriverOutput(BufOrError->get()->getBuffer());
+  log("System includes extractor: successfully executed {0}, got includes: "
   "\"{1}\"",
-  Driver, llvm::join(Includes, ", "));
-  return Includes;
+  Driver, llvm::join(IncludesAndTarget.first, ", "));
+  if (!

[PATCH] D92012: [clangd][query-driver] Extract target

2020-11-24 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added reviewers: sammccall, kadircet.
ArcsinX added a comment.

I'm not sure if this solution is elegant enough. I will be happy to hear 
advices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92012

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

[PATCH] D91859: [clangd] Fix shared-lib builds

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 307281.
kadircet added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

- Link protobuf and grpc++ publicly to generated targets


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  llvm/cmake/modules/FindGRPC.cmake

Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -112,7 +112,7 @@
 
   add_clang_library(${LibraryName} ${GeneratedProtoSource}
 PARTIAL_SOURCES_INTENDED
-LINK_LIBS grpc++ protobuf)
+LINK_LIBS PUBLIC grpc++ protobuf)
 
   # Ensure dependency headers are generated before dependent protos are built.
   # DEPENDS arg is a list of "Foo.proto". While they're logically relative to
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -146,7 +146,8 @@
 if (CLANGD_ENABLE_REMOTE)
   target_link_libraries(ClangdTests
 PRIVATE
-clangdRemoteMarshalling)
+clangdRemoteMarshalling
+RemoteIndexProto)
 endif()
 
 if (CLANGD_BUILD_XPC)
Index: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -17,6 +17,4 @@
   RemoteIndexProto
   RemoteIndexServiceProto
   clangdRemoteMarshalling
-
-  grpc++
   )
Index: clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
@@ -4,7 +4,6 @@
   LINK_LIBS
   RemoteIndexProto
 
-  protobuf
   clangDaemon
   clangdSupport
 
Index: clang-tools-extra/clangd/index/remote/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -3,6 +3,12 @@
   generate_protos(RemoteIndexServiceProto "Service.proto"
 DEPENDS "Index.proto"
 GRPC)
+  # FIXME: Move this into generate_protos. Currently we only mention proto
+  # filename as a dependency, but linking requires target name.
+  target_link_libraries(RemoteIndexServiceProto
+PRIVATE
+RemoteIndexProto
+)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 
@@ -18,8 +24,8 @@
 RemoteIndexProto
 RemoteIndexServiceProto
 clangdRemoteMarshalling
-protobuf
-grpc++
+clangBasic
+clangDaemon
 clangdSupport
 
 DEPENDS
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -168,17 +168,18 @@
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+
 if(CLANG_INCLUDE_TESTS)
-add_subdirectory(test)
-add_subdirectory(unittests)
+  add_subdirectory(test)
+  add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
 
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91428: Add support for multiple program address spaces

2020-11-24 Thread Paulo Matos via Phabricator via cfe-commits

pmatos added a comment.

Thanks, @arichardson and @jrtc27 for your comments. 
I am definitely surprised to find that if you explicitly mark the call with the 
address space, this patch is not required. At first look, this RFC is not 
required any more but I need sometime to investigate further. If no changes are 
necessary, this is indeed good news.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91428

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

[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

2020-11-24 Thread Peter Smith via Phabricator via cfe-commits

psmith added a comment.

Radio silence so far; I think no news is good news applies in this case. I'm 
happy to say no objections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91760

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

[PATCH] D91952: [clangd] Add support for within-file rename of complicated fields

2020-11-24 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

I assume this fixes https://github.com/clangd/clangd/issues/582?

can you check the static members in template classes etc? I think the AST is 
different.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:131
+// Clang AST does not store relevant information about the field that is
+// instantiated.
+const auto *TemplateSpec =

yeah, this is a bit unfortunate, I don't have a better solution (extending the 
AST is one option, but it may be not easy). I think we can live with the 
heuristic.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:137
+const CXXRecordDecl *FieldParent =
+TemplateSpec->getTemplateInstantiationPattern();
+// Field is not instantiation.

nit: I think we can simplify the code a bit more:

```
const auto* Template = Field->getParent()->getTemplateInstantiationPattern();
if (!Template)
  return...;

```



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:142
+for (const FieldDecl *Candidate : FieldParent->fields()) {
+  if (Field->getFieldIndex() == Candidate->getFieldIndex()) {
+assert(Field->getLocation() == Candidate->getLocation() &&

I think we could also check the equality of their names.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:143
+  if (Field->getFieldIndex() == Candidate->getFieldIndex()) {
+assert(Field->getLocation() == Candidate->getLocation() &&
+   "Field should be generated from Candidate so it has the same "

this assertion seems too strong to me -- this is a heuristics, it may have 
false positives. I think we can remove it. 



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:149
+}
+llvm_unreachable("FieldParent should have field with same index as 
Field.");
+  }

use `elog`?



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:545
+  R"cpp(
+class Foo {
+public:

I think this is a normal rename-field case, it should already work prior to the 
patch.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:575
+  T Variable[42];
+  U Another;
+

`Another` seems to be not needed. I'd suggest removing it to make the testcase 
as tense as possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91952

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

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

2020-11-24 Thread LuoYuanke via Phabricator via cfe-commits

LuoYuanke updated this revision to Diff 307298.
LuoYuanke retitled this revision from "[X86] Add x86_amx type for intel AMX. " 
to "[X86] Add x86_amx type for intel AMX.".
LuoYuanke added a comment.

Address Craig's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91927

Files:
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/CodeGen/ValueTypes.td
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/Type.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -248,7 +248,8 @@
   IIT_V128 = 47,
   IIT_BF16 = 48,
   IIT_STRUCT9 = 49,
-  IIT_V256 = 50
+  IIT_V256 = 50,
+  IIT_AMX  = 51,
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -276,6 +277,7 @@
   case MVT::token: return Sig.push_back(IIT_TOKEN);
   case MVT::Metadata: return Sig.push_back(IIT_METADATA);
   case MVT::x86mmx: return Sig.push_back(IIT_MMX);
+  case MVT::x86amx: return Sig.push_back(IIT_AMX);
   // MVT::OtherVT is used to mean the empty struct type here.
   case MVT::Other: return Sig.push_back(IIT_EMPTYSTRUCT);
   // MVT::isVoid is used to represent varargs here.
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -76,6 +76,7 @@
   case MVT::f128: return "MVT::f128";
   case MVT::ppcf128:  return "MVT::ppcf128";
   case MVT::x86mmx:   return "MVT::x86mmx";
+  case MVT::x86amx:   return "MVT::x86amx";
   case MVT::Glue: return "MVT::Glue";
   case MVT::isVoid:   return "MVT::isVoid";
   case MVT::v1i1: return "MVT::v1i1";
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-type.ll
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -8,18 +8,70 @@
 @buf = dso_local global [1024 x i8] zeroinitializer, align 16
 @buf2 = dso_local global [1024 x i8] zeroinitializer, align 16
 
+; test bitcast x86_amx to <256 x i32>
+define dso_local void @test_user_empty(x86_amx %in) #2 {
+; CHECK-LABEL: @test_user_empty(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:ret void
+;
+entry:
+  %t = bitcast x86_amx %in to <256 x i32>
+  ret void
+}
+
+; test bitcast <256 x i32> to x86_amx
+define dso_local void @test_user_empty2(<256 x i32> %in) #2 {
+; CHECK-LABEL: @test_user_empty2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:ret void
+;
+entry:
+  %t = bitcast <256 x i32> %in to x86_amx
+  ret void
+}
+
+define dso_local void @test_src_add(<256 x i32> %x, <256 x i32> %y, i16 %r, i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = alloca <256 x i32>, align 1024
+; CHECK-NEXT:[[ADD:%.*]] = add <256 x i32> [[Y:%.*]], [[X:%.*]]
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast <256 x i32>* [[TMP0]] to i8*
+; CHECK-NEXT:store <256 x i32> [[ADD]], <256 x i32>* [[TMP0]], align 1024
+; CHECK-NEXT:[[TMP2:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[R:%.*]], i16 [[C:%.*]], i8* [[TMP1]], i64 64)
+; CHECK-NEXT:call void @llvm.x86.tilestored64.internal(i16 [[R]], i16 [[C]], i8* [[BUF:%.*]], i64 [[S:%.*]], x86_amx [[TMP2]]) [[ATTR3:#.*]]
+; CHECK-NEXT:ret void
+;
+entry:
+  %add = add <256 x i32> %y, %x
+  %t = bitcast <256 x i32> %add to x86_amx
+  call void @llvm.x86.tilestored64.internal(i16 %r, i16 %c, i8* %buf, i64 %s, x86_amx %t) #3
+  ret void
+}
+
+define dso_local void @test_src_add2(<256 x i32> %x, i16 %r, i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = alloca <256 x i32>, align 1024
+; CHECK-NEXT:[[T1:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[R:%.*]], i16 [[C:%.*]]

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

2020-11-24 Thread LuoYuanke via Phabricator via cfe-commits

LuoYuanke updated this revision to Diff 307300.
LuoYuanke added a comment.

Address Craig's comments.
Change dyn_cast to cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91927

Files:
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/CodeGen/ValueTypes.td
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/Type.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -248,7 +248,8 @@
   IIT_V128 = 47,
   IIT_BF16 = 48,
   IIT_STRUCT9 = 49,
-  IIT_V256 = 50
+  IIT_V256 = 50,
+  IIT_AMX  = 51,
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -276,6 +277,7 @@
   case MVT::token: return Sig.push_back(IIT_TOKEN);
   case MVT::Metadata: return Sig.push_back(IIT_METADATA);
   case MVT::x86mmx: return Sig.push_back(IIT_MMX);
+  case MVT::x86amx: return Sig.push_back(IIT_AMX);
   // MVT::OtherVT is used to mean the empty struct type here.
   case MVT::Other: return Sig.push_back(IIT_EMPTYSTRUCT);
   // MVT::isVoid is used to represent varargs here.
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -76,6 +76,7 @@
   case MVT::f128: return "MVT::f128";
   case MVT::ppcf128:  return "MVT::ppcf128";
   case MVT::x86mmx:   return "MVT::x86mmx";
+  case MVT::x86amx:   return "MVT::x86amx";
   case MVT::Glue: return "MVT::Glue";
   case MVT::isVoid:   return "MVT::isVoid";
   case MVT::v1i1: return "MVT::v1i1";
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-type.ll
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -8,18 +8,70 @@
 @buf = dso_local global [1024 x i8] zeroinitializer, align 16
 @buf2 = dso_local global [1024 x i8] zeroinitializer, align 16
 
+; test bitcast x86_amx to <256 x i32>
+define dso_local void @test_user_empty(x86_amx %in) #2 {
+; CHECK-LABEL: @test_user_empty(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:ret void
+;
+entry:
+  %t = bitcast x86_amx %in to <256 x i32>
+  ret void
+}
+
+; test bitcast <256 x i32> to x86_amx
+define dso_local void @test_user_empty2(<256 x i32> %in) #2 {
+; CHECK-LABEL: @test_user_empty2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:ret void
+;
+entry:
+  %t = bitcast <256 x i32> %in to x86_amx
+  ret void
+}
+
+define dso_local void @test_src_add(<256 x i32> %x, <256 x i32> %y, i16 %r, i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = alloca <256 x i32>, align 1024
+; CHECK-NEXT:[[ADD:%.*]] = add <256 x i32> [[Y:%.*]], [[X:%.*]]
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast <256 x i32>* [[TMP0]] to i8*
+; CHECK-NEXT:store <256 x i32> [[ADD]], <256 x i32>* [[TMP0]], align 1024
+; CHECK-NEXT:[[TMP2:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[R:%.*]], i16 [[C:%.*]], i8* [[TMP1]], i64 64)
+; CHECK-NEXT:call void @llvm.x86.tilestored64.internal(i16 [[R]], i16 [[C]], i8* [[BUF:%.*]], i64 [[S:%.*]], x86_amx [[TMP2]]) [[ATTR3:#.*]]
+; CHECK-NEXT:ret void
+;
+entry:
+  %add = add <256 x i32> %y, %x
+  %t = bitcast <256 x i32> %add to x86_amx
+  call void @llvm.x86.tilestored64.internal(i16 %r, i16 %c, i8* %buf, i64 %s, x86_amx %t) #3
+  ret void
+}
+
+define dso_local void @test_src_add2(<256 x i32> %x, i16 %r, i16 %c, i8* %buf, i64 %s) #2 {
+; CHECK-LABEL: @test_src_add2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TMP0:%.*]] = alloca <256 x i32>, align 1024
+; CHECK-NEXT:[[T1:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[R:%.*]], i16 [[C:%.*]], i8* [[BUF:%.*]], i64 [[S:%.*]]) [[ATTR3]]
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast <256 x i32>* [

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

2020-11-24 Thread Francesco Petrogalli via Phabricator via cfe-commits

fpetrogalli updated this revision to Diff 307306.
fpetrogalli added a comment.

Rebase on top of D92020 . NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

Files:
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  
llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll

Index: llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
@@ -0,0 +1,42 @@
+; RUN: opt -mtriple aarch64-gnu-linux -mattr=+sve -instcombine -S < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; If this check fails please read
+; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on
+; how to resolve it.
+
+; WARN-NOT: warning
+
+; CHECK-LABEL: @debug_local_scalable(
+define  @debug_local_scalable( %tostore) {
+  %vx = alloca , align 16
+  call void @llvm.dbg.declare(metadata * %vx, metadata !5, metadata !DIExpression()), !dbg !15
+  store  %tostore, * %vx, align 16
+  %ret = call  @f(* %vx)
+  ret  %ret
+}
+
+declare  @f(*)
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !DILocalVariable(name: "vx", scope: !6, file: !7, line: 26, type: !8)
+!6 = distinct !DISubprogram(name: "debug_local_scalable", scope: null, file: !1, line: 25, scopeLine: 25, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DIFile(filename: "test.c", directory: "/tmp/")
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "svfloat64_t", file: !9, line: 56, baseType: !10)
+!9 = !DIFile(filename: "arm_sve.h", directory: "/tmp/")
+!10 = !DIDerivedType(tag: DW_TAG_typedef, name: "__SVFloat64_t", file: !1, baseType: !11)
+!11 = !DICompositeType(tag: DW_TAG_array_type, baseType: !12, flags: DIFlagVector, elements: !13)
+!12 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)
+!13 = !{!14}
+!14 = !DISubrange(lowerBound: 0, upperBound: !DIExpression(DW_OP_constu, 1, DW_OP_bregx, 46, 0, DW_OP_mul, DW_OP_constu, 1, DW_OP_minus))
+!15 = !DILocation(line: 26, column: 15, scope: !6)
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1368,16 +1368,22 @@
 /// least n bits.
 static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   const DataLayout &DL = DII->getModule()->getDataLayout();
-  uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
-  if (auto FragmentSize = DII->getFragmentSizeInBits())
-return ValueSize >= *FragmentSize;
+  TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+  if (Optional FragmentSize = DII->getFragmentSizeInBits()) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // We can't always calculate the size of the DI variable (e.g. if it is a
   // VLA). Try to use the size of the alloca that the dbg intrinsic describes
   // intead.
   if (DII->isAddressOfVariable())
 if (auto *AI = dyn_cast_or_null(DII->getVariableLocation()))
-  if (auto FragmentSize = AI->getAllocationSizeInBits(DL))
-return ValueSize >= *FragmentSize;
+  if (Optional FragmentSize = AI->getAllocationSizeInBits(DL)) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // Could not determine size of variable. Conservatively return false.
   return false;
 }
Index: llvm/lib/Transforms/Utils/Debugify.cpp
===
--- llvm/lib/Transforms/Utils/Debugify.cpp
+++ llvm/lib/Transforms/Utils/Debugify.cpp
@@ -44,8 +44,9 @@
 
 raw_ostream &dbg() { return Quiet ? nulls() : errs(); }
 
-uint64_t getAllocSizeInBits(Module &M, Type *Ty) {
-  return Ty->isSized() ? M.getDataLayout().getTypeAllocSizeInBits(Ty) : 0;
+TypeSize getAllocSizeInBits(Module &M, Type *T

[PATCH] D83698: Port Target option flags to new option parsing system

2020-11-24 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 commandeered this revision.
jansvoboda11 added a reviewer: dang.
jansvoboda11 added a comment.

Taking over this patch as Daniel is no longer involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83698

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

[PATCH] D83698: [clang][cli] Port Target option flags to new option parsing system

2020-11-24 Thread Jan Svoboda via Phabricator via cfe-commits

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

Undo moving of options (NFC) & rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83698

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3721,9 +3721,6 @@
   Opts.LinkerVersion =
   std::string(Args.getLastArgValue(OPT_target_linker_version));
   Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
-  Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
-  Opts.NVPTXUseShortPointers = Args.hasFlag(
-  options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
   Opts.AllowAMDGPUUnsafeFPAtomics =
   Args.hasFlag(options::OPT_munsafe_fp_atomics,
options::OPT_mno_unsafe_fp_atomics, false);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -692,7 +692,7 @@
   "Generate relocatable device code, also known as separate compilation mode", 
"", "">;
 def : Flag<["-"], "fcuda-rdc">, Alias;
 def : Flag<["-"], "fno-cuda-rdc">, Alias;
-defm cuda_short_ptr : OptInFFlag<"cuda-short-ptr",
+defm cuda_short_ptr : BooleanMarshalledFFlag<"cuda-short-ptr", 
"TargetOpts->NVPTXUseShortPointers", "false",
   "Use 32-bit pointers for accessing const/local/shared address spaces">;
 def rocm_path_EQ : Joined<["--"], "rocm-path=">, Group,
   HelpText<"ROCm installation path, used for finding and automatically linking 
required bitcode libraries.">;
@@ -1049,7 +1049,7 @@
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;
 defm jump_tables : OptOutFFlag<"jump-tables", "Use", "Do not use", " jump 
tables for lowering switches">;
-defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", 
"Disable", " support for int128_t type">;
+defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", 
"Disable", " support for int128_t type", [], "TargetOpts->ForceEnableInt128">;
 defm keep_static_consts : OptInFFlag<"keep-static-consts", "Keep", "Don't 
keep", " static const variables if unused", [NoXarchOption]>;
 defm fixed_point : OptInFFlag<"fixed-point", "Enable", "Disable", " fixed 
point types">;
 defm cxx_static_destructors : OptOutFFlag<"c++-static-destructors", "",


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3721,9 +3721,6 @@
   Opts.LinkerVersion =
   std::string(Args.getLastArgValue(OPT_target_linker_version));
   Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
-  Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
-  Opts.NVPTXUseShortPointers = Args.hasFlag(
-  options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
   Opts.AllowAMDGPUUnsafeFPAtomics =
   Args.hasFlag(options::OPT_munsafe_fp_atomics,
options::OPT_mno_unsafe_fp_atomics, false);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -692,7 +692,7 @@
   "Generate relocatable device code, also known as separate compilation mode", "", "">;
 def : Flag<["-"], "fcuda-rdc">, Alias;
 def : Flag<["-"], "fno-cuda-rdc">, Alias;
-defm cuda_short_ptr : OptInFFlag<"cuda-short-ptr",
+defm cuda_short_ptr : BooleanMarshalledFFlag<"cuda-short-ptr", "TargetOpts->NVPTXUseShortPointers", "false",
   "Use 32-bit pointers for accessing const/local/shared address spaces">;
 def rocm_path_EQ : Joined<["--"], "rocm-path=">, Group,
   HelpText<"ROCm installation path, used for finding and automatically linking required bitcode libraries.">;
@@ -1049,7 +1049,7 @@
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;
 defm jump_tables : OptOutFFlag<"jump-tables", "Use", "Do not use", " jump tables for lowering switches">;
-defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", "Disable", " support for int128_t type">;
+defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", "Disable", " support for int128_t type", [], "TargetOpts->ForceEnableInt128">;
 defm keep_static_consts : OptInFFlag<"keep-static-consts", "Keep", "Don't keep", " static const variables if unused", [NoXarchOption]>;
 defm fixed_point : OptInFFlag<"fixed-point", "Enable", "Disable", " fixed poin

[PATCH] D91859: [clangd] Fix shared-lib builds

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf726101b6240: [clangd] Fix shared-lib builds (authored by 
kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  llvm/cmake/modules/FindGRPC.cmake

Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -112,7 +112,7 @@
 
   add_clang_library(${LibraryName} ${GeneratedProtoSource}
 PARTIAL_SOURCES_INTENDED
-LINK_LIBS grpc++ protobuf)
+LINK_LIBS PUBLIC grpc++ protobuf)
 
   # Ensure dependency headers are generated before dependent protos are built.
   # DEPENDS arg is a list of "Foo.proto". While they're logically relative to
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -146,7 +146,8 @@
 if (CLANGD_ENABLE_REMOTE)
   target_link_libraries(ClangdTests
 PRIVATE
-clangdRemoteMarshalling)
+clangdRemoteMarshalling
+RemoteIndexProto)
 endif()
 
 if (CLANGD_BUILD_XPC)
Index: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -17,6 +17,4 @@
   RemoteIndexProto
   RemoteIndexServiceProto
   clangdRemoteMarshalling
-
-  grpc++
   )
Index: clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
@@ -4,7 +4,6 @@
   LINK_LIBS
   RemoteIndexProto
 
-  protobuf
   clangDaemon
   clangdSupport
 
Index: clang-tools-extra/clangd/index/remote/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -3,6 +3,12 @@
   generate_protos(RemoteIndexServiceProto "Service.proto"
 DEPENDS "Index.proto"
 GRPC)
+  # FIXME: Move this into generate_protos. Currently we only mention proto
+  # filename as a dependency, but linking requires target name.
+  target_link_libraries(RemoteIndexServiceProto
+PRIVATE
+RemoteIndexProto
+)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 
@@ -18,8 +24,8 @@
 RemoteIndexProto
 RemoteIndexServiceProto
 clangdRemoteMarshalling
-protobuf
-grpc++
+clangBasic
+clangDaemon
 clangdSupport
 
 DEPENDS
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -168,17 +168,18 @@
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+
 if(CLANG_INCLUDE_TESTS)
-add_subdirectory(test)
-add_subdirectory(unittests)
+  add_subdirectory(test)
+  add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support for Clangd" OFF)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual installation.")
 
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] f726101 - [clangd] Fix shared-lib builds

2020-11-24 Thread Kadir Cetinkaya via cfe-commits


Author: Kadir Cetinkaya
Date: 2020-11-24T13:05:20+01:00
New Revision: f726101b6240a6740b3c0926af759da5e7336f8a

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

LOG: [clangd] Fix shared-lib builds

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

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/remote/CMakeLists.txt
clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
clang-tools-extra/clangd/unittests/CMakeLists.txt
llvm/cmake/modules/FindGRPC.cmake

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index b8300ddaf548..2ce5d31e623e 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -168,17 +168,18 @@ if ( CLANGD_BUILD_XPC )
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  include(FindGRPC)
+endif()
+
 if(CLANG_INCLUDE_TESTS)
-add_subdirectory(test)
-add_subdirectory(unittests)
+  add_subdirectory(test)
+  add_subdirectory(unittests)
 endif()
 
 # FIXME(kirillbobyrev): Document this in the LLVM docs once remote index is 
stable.
 option(CLANGD_ENABLE_REMOTE "Use gRPC library to enable remote index support 
for Clangd" OFF)
 set(GRPC_INSTALL_PATH "" CACHE PATH "Path to gRPC library manual 
installation.")
 
-if (CLANGD_ENABLE_REMOTE)
-  include(FindGRPC)
-endif()
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)

diff  --git a/clang-tools-extra/clangd/index/remote/CMakeLists.txt 
b/clang-tools-extra/clangd/index/remote/CMakeLists.txt
index 8625fa8f351e..eaa000b745e5 100644
--- a/clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -3,6 +3,12 @@ if (CLANGD_ENABLE_REMOTE)
   generate_protos(RemoteIndexServiceProto "Service.proto"
 DEPENDS "Index.proto"
 GRPC)
+  # FIXME: Move this into generate_protos. Currently we only mention proto
+  # filename as a dependency, but linking requires target name.
+  target_link_libraries(RemoteIndexServiceProto
+PRIVATE
+RemoteIndexProto
+)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 
@@ -18,8 +24,8 @@ if (CLANGD_ENABLE_REMOTE)
 RemoteIndexProto
 RemoteIndexServiceProto
 clangdRemoteMarshalling
-protobuf
-grpc++
+clangBasic
+clangDaemon
 clangdSupport
 
 DEPENDS

diff  --git a/clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt 
b/clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
index 7b78ba3bb690..a5f6ebd179ec 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/marshalling/CMakeLists.txt
@@ -4,7 +4,6 @@ add_clang_library(clangdRemoteMarshalling
   LINK_LIBS
   RemoteIndexProto
 
-  protobuf
   clangDaemon
   clangdSupport
 

diff  --git a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt 
b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
index 595c406eff0f..e6959db6bbd8 100644
--- a/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -17,6 +17,4 @@ target_link_libraries(clangd-index-server
   RemoteIndexProto
   RemoteIndexServiceProto
   clangdRemoteMarshalling
-
-  grpc++
   )

diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt 
b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index e7baf880e504..72ce97ed31b6 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -146,7 +146,8 @@ target_link_libraries(ClangdTests
 if (CLANGD_ENABLE_REMOTE)
   target_link_libraries(ClangdTests
 PRIVATE
-clangdRemoteMarshalling)
+clangdRemoteMarshalling
+RemoteIndexProto)
 endif()
 
 if (CLANGD_BUILD_XPC)

diff  --git a/llvm/cmake/modules/FindGRPC.cmake 
b/llvm/cmake/modules/FindGRPC.cmake
index f2c9bee38c93..7031c5f0016a 100644
--- a/llvm/cmake/modules/FindGRPC.cmake
+++ b/llvm/cmake/modules/FindGRPC.cmake
@@ -112,7 +112,7 @@ function(generate_protos LibraryName ProtoFile)
 
   add_clang_library(${LibraryName} ${GeneratedProtoSource}
 PARTIAL_SOURCES_INTENDED
-LINK_LIBS grpc++ protobuf)
+LINK_LIBS PUBLIC grpc++ protobuf)
 
   # Ensure dependency headers are generated before dependent protos are built.
   # DEPENDS arg is a list of "Foo.proto". While they're logically relative to



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

[PATCH] D91713: [libomptarget] Implement get_device_num for amdgcn, nvptx

2020-11-24 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield updated this revision to Diff 307215.
JonChesterfield added a comment.
Herald added subscribers: llvm-commits, libc-commits, libcxx-commits, 
lldb-commits, Sanitizers, cfe-commits, mravishankar, teijeong, frasercrmck, 
dexonsmith, awarzynski, rdzhabarov, ecnelises, tatianashp, wenlei, lxfind, 
ThomasRaoux, cishida, mehdi_amini, dang, nikic, AlexeySotkin, msifontes, 
jurahul, laytonio, cmtice, Kayjukh, vkmr, grosul1, martong, Joonsoo, 
stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, 
csigg, nicolasvasilache, antiagainst, shauheen, rriddle, luismarques, apazos, 
sameer.abuasal, usaxena95, pzheng, pengfei, s.egerton, lenary, dmgreen, Jim, 
asbirlea, thopre, mstorsjo, lebedev.ri, kadircet, jocewei, rupprecht, PkmX, 
jfb, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, steven_wu, 
atanasyan, mgrang, edward-jones, george.burgess.iv, zzheng, MaskRay, jrtc27, 
gbedwell, niosHD, cryptoad, sabuasal, simoncook, johnrusso, rbar, asb, 
javed.absar, fedor.sergeev, kbarton, aheejin, hiraditya, eraman, 
jgravelle-google, krytarowski, arichardson, sbc100, mgorny, nemanjai, sdardis, 
emaste, dylanmckay, jyknight, dschuff, arsenm, qcolombet, MatzeB, jholewinski.
Herald added a reviewer: whitequark.
Herald added a reviewer: bollu.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: espindola.
Herald added a reviewer: andreadb.
Herald added a reviewer: alexshap.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: shafik.
Herald added a reviewer: mravishankar.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: jpienaar.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: silvas.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added projects: clang, Sanitizers, LLDB, libc++, libc-project, 
libc++abi, MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
This revision now requires review to proceed.

- Use header from include, add test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91713

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/index/Relation.cpp
  clang-tools-extra/clangd/index/Relation.h
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/test/index-serialization/Inputs/sample.cpp
  clang-tools-extra/clangd/test/index-serialization/Inputs/sample.h
  clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx
  clang-tools-extra/clangd/test/index-serialization/version-is-correct.test
  clang-tools-extra/clangd/test/lit.cfg.py
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
  clang-tools-extra/clangd/test/remote-index/public-log.test
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/gmock/foo.h
  clang-tools-extra/test/clang-tidy/checkers/abseil-redundant-strcat-calls.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang/cmake/caches/CrossWinToARMLinux.cmake
  clang/docs/DiagnosticsReference.rst
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

2020-11-24 Thread Pengfei Wang via Phabricator via cfe-commits

pengfei added inline comments.



Comment at: llvm/lib/IR/DataLayout.cpp:819
+  case Type::X86_AMXTyID:
+return Align(64);
   default:

Should be 512 bits?



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:79
+// -->
+// %addr = alloca <256 x i32>, align 1024
+// store <256 x i32> %src, <256 x i32>* %addr, align 1024

Why the alignment not be 64?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91927

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

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

2020-11-24 Thread Pengfei Wang via Phabricator via cfe-commits

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:164
+}
+// %src = call x86_amx @llvm.x86.tileloadd64.internal(%row, %col, 
%addr,
+// %stride);

`%src` is not used here.



Comment at: llvm/utils/TableGen/IntrinsicEmitter.cpp:252
+  IIT_V256 = 50,
+  IIT_AMX  = 51,
 };

Remove `,`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91927

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

[PATCH] D91952: [clangd] Add support for within-file rename of complicated fields

2020-11-24 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 307317.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.

Resolve most actionable comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91952

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -540,6 +540,76 @@
 }
   )cpp",
 
+  // Fields in classes & partial and full specialiations.
+  R"cpp(
+template
+struct Foo {
+  T [[Vari^able]] = 42;
+};
+
+void foo() {
+  Foo f;
+  f.[[Varia^ble]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T Variable;
+  void bar() { ++Variable; }
+};
+
+template<>
+struct Foo {
+  unsigned [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+  R"cpp(
+struct Foo {
+  static int [[Var^iable]];
+};
+
+int Foo::[[Var^iable]] = 42;
+
+void foo() {
+  int LocalInt = Foo::[[Var^iable]];
+}
+  )cpp",
+
   // Template parameters.
   R"cpp(
 template 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -119,11 +119,28 @@
 // declaration.
 while (Method->isVirtual() && Method->size_overridden_methods())
   Method = *Method->overridden_methods().begin();
-return dyn_cast(Method->getCanonicalDecl());
+return Method->getCanonicalDecl();
   }
   if (const auto *Function = dyn_cast(D))
 if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
   return canonicalRenameDecl(Template);
+  if (const auto *Field = dyn_cast(D)) {
+// This is a hacky way to do something like
+// CXXMethodDecl::getInstantiatedFromMemberFunction for the field because
+// Clang AST does not store relevant information about the field that is
+// instantiated.
+const auto *FieldParent = dyn_cast(Field->getParent())
+  ->getTemplateInstantiationPattern();
+if (!FieldParent)
+  return Field->getCanonicalDecl();
+// Field is not instantiation.
+if (!FieldParent || Field->getParent() == FieldParent)
+  return Field->getCanonicalDecl();
+for (const FieldDecl *Candidate : FieldParent->fields())
+  if (Field->getFieldIndex() == Candidate->getFieldIndex())
+return Candidate->getCanonicalDecl();
+llvm_unreachable("FieldParent should have field with same index as Field.");
+  }
   return dyn_cast(D->getCanonicalDecl());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91952: [clangd] Add support for within-file rename of complicated fields

2020-11-24 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev planned changes to this revision.
kbobyrev added a comment.

  template
  struct Foo {
static T [[Var^iable]];
  };
  
  template <>
  int Foo::[[Variable^]] = 42;
  
  template <>
  bool Foo::[[Variable^]] = true;
  
  void foo() {
int LocalInt = Foo::[[Variable^]];
bool LocalBool = Foo::[[Variable^]];
  }

This doesn't work yet, need to investigate.




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:131
+// Clang AST does not store relevant information about the field that is
+// instantiated.
+const auto *TemplateSpec =

hokein wrote:
> yeah, this is a bit unfortunate, I don't have a better solution (extending 
> the AST is one option, but it may be not easy). I think we can live with the 
> heuristic.
Yes, I thought about that but realized it might be too much effort, maybe I 
should put a FIXME here.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:142
+for (const FieldDecl *Candidate : FieldParent->fields()) {
+  if (Field->getFieldIndex() == Candidate->getFieldIndex()) {
+assert(Field->getLocation() == Candidate->getLocation() &&

hokein wrote:
> I think we could also check the equality of their names.
Yes, that is what @sammccall proposed too, but names not simple `unsigned` 
numbers, checking for that is just more expensive.



Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:575
+  T Variable[42];
+  U Another;
+

hokein wrote:
> `Another` seems to be not needed. I'd suggest removing it to make the 
> testcase as tense as possible.
In this case there is only one field and I'd be happy to check that the correct 
field gets renamed in case there are two of them (this is when the index 
comparison is checked). Otherwise this would pass without the index checking 
(there is one field, we don't check if it's actually the one we need and it 
gets renamed regardless).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91952

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

[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-24 Thread Hafiz Abid Qadeer via Phabricator via cfe-commits

abidh added a comment.

Hi @lenary,
Do you any more comments on the patch?


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

https://reviews.llvm.org/D91442

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

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:411
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };

What about `llvm::iplist`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

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

[clang] 3823665 - [docs] Try to make this bullet list in ThinLTO.rst actually be a bullet list

2020-11-24 Thread Hans Wennborg via cfe-commits


Author: Hans Wennborg
Date: 2020-11-24T14:08:42+01:00
New Revision: 38236656ab4a4bea5e582f709929003abfa1ddcd

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

LOG: [docs] Try to make this bullet list in ThinLTO.rst actually be a bullet 
list

Added: 


Modified: 
clang/docs/ThinLTO.rst

Removed: 




diff  --git a/clang/docs/ThinLTO.rst b/clang/docs/ThinLTO.rst
index 0da822f498b9..fa6d28e13ba7 100644
--- a/clang/docs/ThinLTO.rst
+++ b/clang/docs/ThinLTO.rst
@@ -124,9 +124,13 @@ be reduced to ``N`` via:
   ``/opt:lldltojobs=N``
 
 Other possible values for ``N`` are:
-- 0: Use one thread per physical core (default)
-- 1: Use a single thread only (disable multi-threading)
-- all: Use one thread per logical core (uses all hyper-threads)
+
+- 0:
+  Use one thread per physical core (default)
+- 1:
+  Use a single thread only (disable multi-threading)
+- all:
+  Use one thread per logical core (uses all hyper-threads)
 
 Incremental
 ---



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

[clang] cb08558 - [HIP] Fix regressions due to fp contract change

2020-11-24 Thread Yaxun Liu via cfe-commits


Author: Yaxun (Sam) Liu
Date: 2020-11-24T08:10:06-05:00
New Revision: cb08558caa3bad69213b08e6361586491232c745

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

LOG: [HIP] Fix regressions due to fp contract change

Recently HIP toolchain made a change to use clang instead of opt/llc to do 
compilation
(https://reviews.llvm.org/D81861). The intention is to make HIP toolchain 
canonical like
other toolchains.

However, this change introduced an unintentional change regarding backend fp 
fuse
option, which caused regressions in some HIP applications.

Basically before the change, HIP toolchain used clang to generate bitcode, then 
use
opt/llc to optimize bitcode and generate ISA. As such, the amdgpu backend takes
the default fp fuse mode which is 'Standard'. This mode respect contract flag of
fmul/fadd instructions and do not fuse fmul/fadd instructions without contract 
flag.

However, after the change, HIP toolchain now use clang to generate IR, do 
optimization,
and generate ISA as one process. Now amdgpu backend fp fuse option is determined
by -ffp-contract option, which is 'fast' by default. And this 
-ffp-contract=fast language option
is translated to 'Fast' fp fuse option in backend. Suddenly backend starts to 
fuse fmul/fadd
instructions without contract flag.

This causes wrong result for some device library functions, e.g. tan(-1e20), 
which should
return 0.8446, now returns -0.933. What is worse is that since backend with 
'Fast' fp fuse
option does not respect contract flag, there is no way to use #pragma clang fp 
contract
directive to enforce fp contract requirements.

This patch fixes the regression by introducing a new value 'fast-honor-pragmas' 
for -ffp-contract
and use it for HIP by default. 'fast-honor-pragmas' is equivalent to 'fast' in 
frontend but
let the backend to use 'Standard' fp fuse option. 'fast-honor-pragmas' is 
useful since 'Fast'
fp fuse option in backend does not honor contract flag, it is of little use to 
HIP
applications since all code with #pragma STDC FP_CONTRACT or any IR from a
source compiled with -ffp-contract=on is broken.

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaAttr.cpp
clang/test/CodeGenCUDA/fp-contract.cu
clang/test/Driver/autocomplete.c

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index e17e4e1c46a7..c50be9e429ae 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3209,8 +3209,9 @@ statements in C).
 
 The pragma can also be used with ``off`` which turns FP contraction off for a
 section of the code. This can be useful when fast contraction is otherwise
-enabled for the translation unit with the ``-ffp-contract=fast`` flag.
-
+enabled for the translation unit with the ``-ffp-contract=fast-honor-pragmas`` 
flag.
+Note that ``-ffp-contract=fast`` will override pragmas to fuse multiply and
+addition across statements regardless of any controlling pragmas.
 
 ``#pragma clang fp exceptions`` specifies floating point exception behavior. It
 may take one the the values: ``ignore``, ``maytrap`` or ``strict``. Meaning of

diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 2c59ba929ef4..27ec7a85599d 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1336,15 +1336,16 @@ are listed below.
The C standard permits intermediate floating-point results within an
expression to be computed with more precision than their type would
normally allow. This permits operation fusing, and Clang takes advantage
-   of this by default. This behavior can be controlled with the
-   ``FP_CONTRACT`` pragma. Please refer to the pragma documentation for a
-   description of how the pragma interacts with this option.
+   of this by default. This behavior can be controlled with the ``FP_CONTRACT``
+   and ``clang fp contract`` pragmas. Please refer to the pragma documentation
+   for a description of how the pragmas interact with this option.
 
Valid values are:
 
-   * ``fast`` (everywhere)
-   * ``on`` (according to FP_CONTRACT pragma, default)
+   * ``fast`` (fuse across statements disregarding pragmas, default for CUDA)
+   * ``on`` (fuse in the same statement unless dictated by pragmas, default 
for languages other than CUDA/HIP)
* ``off`` (never fuse)
+   * ``fast-honor-pragmas`` (fuse across statements unless dictated by 
pragmas, default for HIP)
 
 .. _opt_fhonor-infinities:
 

diff  --git a/clang/in

[PATCH] D90174: [HIP] Fix regressions due to fp contract change

2020-11-24 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb08558caa3b: [HIP] Fix regressions due to fp contract 
change (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90174

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGenCUDA/fp-contract.cu
  clang/test/Driver/autocomplete.c

Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -68,6 +68,7 @@
 // FNOSANICOVERALL-NEXT: trace-pc-guard
 // RUN: %clang --autocomplete=-ffp-contract= | FileCheck %s -check-prefix=FFPALL
 // FFPALL: fast
+// FFPALL-NEXT: fast-honor-pragmas 
 // FFPALL-NEXT: off
 // FFPALL-NEXT: on
 // RUN: %clang --autocomplete=-flto= | FileCheck %s -check-prefix=FLTOALL
Index: clang/test/CodeGenCUDA/fp-contract.cu
===
--- clang/test/CodeGenCUDA/fp-contract.cu
+++ clang/test/CodeGenCUDA/fp-contract.cu
@@ -1,32 +1,298 @@
-// REQUIRES: x86-registered-target
-// REQUIRES: nvptx-registered-target
+// REQUIRES: x86-registered-target, nvptx-registered-target, amdgpu-registered-target
 
-// By default we should fuse multiply/add into fma instruction.
+// By default CUDA uses -ffp-contract=fast, HIP uses -ffp-contract=fast-honor-pragmas.
+// we should fuse multiply/add into fma instruction.
+// In IR, fmul/fadd instructions with contract flag are emitted.
+// In backend
+//nvptx -  assumes fast fp fuse option, which fuses
+// mult/add insts disregarding contract flag and
+// llvm.fmuladd intrinsics.
+//amdgcn - assumes standard fp fuse option, which only
+// fuses mult/add insts with contract flag and
+// llvm.fmuladd intrinsics.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
+// RUN:   -disable-llvm-passes -o - %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -target-cpu gfx906 -disable-llvm-passes -o - -x hip %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-ON %s
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
-// RUN:   -disable-llvm-passes -o - %s | FileCheck -check-prefix ENABLED %s
+// RUN:   -O3 -o - %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-OPT-FAST %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x hip %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FASTSTD %s
+
+// Check separate compile/backend steps corresponding to -save-temps.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm \
+// RUN:   -O3 -disable-llvm-passes -target-cpu gfx906 -o %t.ll -x hip %s
+// RUN: cat %t.ll  | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST-IR %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x ir %t.ll \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FASTSTD %s
 
 // Explicit -ffp-contract=fast
+// In IR, fmul/fadd instructions with contract flag are emitted.
+// In backend
+//nvptx/amdgcn - assumes fast fp fuse option, which fuses
+//   mult/add insts disregarding contract flag and
+//   llvm.fmuladd intrinsics.
+
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
 // RUN:   -ffp-contract=fast -disable-llvm-passes -o - %s \
-// RUN:   | FileCheck -check-prefix ENABLED %s
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -target-cpu gfx906 -disable-llvm-passes -o - -x hip %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-ON %s
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -S \
+// RUN:   -O3 -o - %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,NV-OPT-FAST %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -S \
+// RUN:   -O3 -target-cpu gfx906 -o - -x hip %s \
+// RUN:   -ffp-contract=fast \
+// RUN:   | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST %s
+
+// Check separate compile/backend steps corresponding to -save-temps.
+// When input is IR, -ffp-contract has no effect. Backend uses default
+// default FP fuse option.
+
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn-amd-amdhsa -emit-llvm \
+// RUN:   -ffp-contract=fast \
+// RUN:   -O3 -disable-llvm-passes -target-cpu gfx906 -o %t.ll -x hip %s
+// RUN: cat %t.ll  | FileCheck -check-prefixes=COMMON,AMD-OPT-FAST-IR %s
+// RUN

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a comment.

Btw how about making some checks simpler. We could always define feature macros 
`__opencl_c_atomic_scope_device`, `__opencl_c_generic_address_space` for  
OpenCL 2.0 or C++ for OpenCL . Then everywhere we would only need to check 
feature macros instead of language versions and feature macros together.

  #if !(defined()) && (defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ 
== CL_VERSION_2_0))
  #define __opencl_c_atomic_scope_device  1
  #define __opencl_c_generic_address_space   1
  ...
  #endif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a subscriber: azabaznov.
Anastasia added a comment.

I assume your change depends on some other changes that add the feature macro 
definitions. So I think we should link the other changes in at some point. 
CCing to @azabaznov here who is working on the features too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a comment.

In D92004#2412953 , @airlied wrote:

> this file has never been clang-format clean, happy to make changes if 
> reviewer decides they are needed.

This has not been considered a conventional clang source so to say, so the 
coding style has not been followed. But it is quite annoying now with 
formatting hooks being installed. Ideally, it would be good to format this 
properly but this makes retrieving the history harder and it is extremely 
valuable. So I suggest not to reformat the old code but for the new code I 
leave it up to you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D91088: [CUDA][HIP] Fix capturing reference to host variable

2020-11-24 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl marked 2 inline comments as done.
yaxunl added a comment.

ping




Comment at: clang/lib/Sema/SemaExpr.cpp:357
 
+  if (LangOpts.CUDAIsDevice) {
+auto *FD = dyn_cast_or_null(CurContext);

tra wrote:
> This could use a comment about why we only check `D->H` references.
> `H->D` references will still work (sort of) because on the host side we have 
> matching `shadow` variables.
> 
> 
the comment has been added in the separated patch for diagnostics.


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

https://reviews.llvm.org/D91088

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

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Anton Zabaznov via Phabricator via cfe-commits

azabaznov added a comment.

Hi! Great to see the interest in OpenCL C 3.0!

I'm being working already at a proper feature macros definition to unify both 
clang and header OpenCL C 3.0 related changes, here is the change: 
https://reviews.llvm.org/D89869. So as this patch will be merged then the check 
for supported feature can be done uniformly in all OpenCL versions:

Use:

  #ifdef __opencl_c_pipes

instead of

  #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0 && __OPENCL_C_VERSION__ < CL_VERSION_3_0) || 
(__OPENCL_C_VERSION__ >= CL_VERSION_3_0 && defined(__opencl_c_pipes))

The main concepts are described in RFC thread here: 
https://lists.llvm.org/pipermail/cfe-dev/2020-September/066883.html. Will be 
glad to discuss any technical details and concerns. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D91884: clang+lld: Improve clang+ld.darwinnew.lld interaction, pass -demangle

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe16c0a9a6897: clang+lld: Improve clang+ld.darwinnew.lld 
interaction, pass -demangle (authored by thakis).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D91884?vs=307211&id=307322#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91884

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-ld-demangle-lld.c
  lld/Common/Strings.cpp
  lld/MachO/Config.h
  lld/MachO/Driver.cpp
  lld/MachO/Options.td
  lld/MachO/Symbols.cpp
  lld/MachO/Writer.cpp
  lld/test/ELF/undef.s
  lld/test/MachO/demangle.s
  lld/test/MachO/silent-ignore.test
  lld/tools/lld/CMakeLists.txt
  llvm/utils/gn/secondary/lld/tools/lld/BUILD.gn

Index: llvm/utils/gn/secondary/lld/tools/lld/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/tools/lld/BUILD.gn
+++ llvm/utils/gn/secondary/lld/tools/lld/BUILD.gn
@@ -4,6 +4,7 @@
   "lld-link",
   "ld.lld",
   "ld64.lld",
+  "ld64.darwinnew.lld",
   "wasm-ld",
 ]
 foreach(target, symlinks) {
Index: lld/tools/lld/CMakeLists.txt
===
--- lld/tools/lld/CMakeLists.txt
+++ lld/tools/lld/CMakeLists.txt
@@ -24,7 +24,8 @@
   RUNTIME DESTINATION bin)
 
 if(NOT LLD_SYMLINKS_TO_CREATE)
-  set(LLD_SYMLINKS_TO_CREATE lld-link ld.lld ld64.lld wasm-ld)
+  set(LLD_SYMLINKS_TO_CREATE
+  lld-link ld.lld ld64.lld ld64.darwinnew.lld wasm-ld)
 endif()
 
 foreach(link ${LLD_SYMLINKS_TO_CREATE})
Index: lld/test/MachO/silent-ignore.test
===
--- lld/test/MachO/silent-ignore.test
+++ lld/test/MachO/silent-ignore.test
@@ -1,5 +1,4 @@
 RUN: %lld -v \
-RUN:   -demangle \
 RUN:   -dynamic \
 RUN:   -no_deduplicate \
 RUN:   -lto_library /lib/foo \
Index: lld/test/MachO/demangle.s
===
--- /dev/null
+++ lld/test/MachO/demangle.s
@@ -0,0 +1,15 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+
+# RUN: not %lld %t.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not %lld -demangle %t.o -o /dev/null 2>&1 | \
+# RUN: FileCheck --check-prefix=DEMANGLE %s
+
+# CHECK: undefined symbol __Z1fv
+# DEMANGLE: undefined symbol f()
+
+.globl _main
+_main:
+  callq __Z1fv
+  ret
Index: lld/test/ELF/undef.s
===
--- lld/test/ELF/undef.s
+++ lld/test/ELF/undef.s
@@ -27,9 +27,7 @@
 # CHECK-NEXT: >>>   {{.*}}:(.text+0x15)
 # CHECK-NEXT: >>> the vtable symbol may be undefined because the class is missing its key function (see https://lld.llvm.org/missingkeyfunction)
 
-# Check that this symbol isn't demangled
-
-# CHECK:  error: undefined symbol: __Z3fooi
+# CHECK:  error: undefined symbol: foo(int)
 # CHECK-NEXT: >>> referenced by undef.s
 # CHECK-NEXT: >>>   {{.*}}:(.text+0x1A)
 
Index: lld/MachO/Writer.cpp
===
--- lld/MachO/Writer.cpp
+++ lld/MachO/Writer.cpp
@@ -379,7 +379,7 @@
 for (Reloc &r : isec->relocs) {
   if (auto *s = r.referent.dyn_cast()) {
 if (isa(s))
-  error("undefined symbol " + s->getName() + ", referenced from " +
+  error("undefined symbol " + toString(*s) + ", referenced from " +
 sys::path::filename(isec->file->getName()));
 else
   target->prepareSymbolRelocation(s, isec, r);
Index: lld/MachO/Symbols.cpp
===
--- lld/MachO/Symbols.cpp
+++ lld/MachO/Symbols.cpp
@@ -33,8 +33,8 @@
 
 // Returns a symbol for an error message.
 std::string lld::toString(const Symbol &sym) {
-  if (Optional s = demangleItanium(sym.getName()))
-return *s;
+  if (config->demangle)
+return demangleItanium(sym.getName());
   return std::string(sym.getName());
 }
 
Index: lld/MachO/Options.td
===
--- lld/MachO/Options.td
+++ lld/MachO/Options.td
@@ -1107,9 +1107,7 @@
  Flags<[HelpHidden]>,
  Group;
 def demangle : Flag<["-"], "demangle">,
- HelpText<"This option is undocumented in ld64">,
- Flags<[HelpHidden]>,
- Group;
+ HelpText<"Demangle symbol names in diagnostics">;
 def dyld_env : Flag<["-"], "dyld_env">,
  HelpText<"This option is undocumented in ld64">,
  Flags<[HelpHidden]>,
Index: lld/MachO/Driver.cpp
===
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -584,6 +584,7 @@
   config->runtimePaths = args::getStrings(args, OPT_

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a subscriber: svenvh.
Anastasia added a comment.

> It will need some decent review and maybe some testing by other CL users to 
> make sure I've haven't messed up when used with a fully featured CL 3.0 stack.

Clang functionality is expected to be tested in llvm-project and with OpenCL 
headers we are violating the process to be honest. Aside from that it slows 
down the development in my opinion because making larger changes is extremely 
hard without the testing. Relying on reviews is not great for various reasons. 
We constantly fix bugs related to the headers.

The biggest issue with testing that header originally was that the parsing of 
it is so slow - with only one test we have exceeded the acceptable limits 
immediately. However, to mitigate this problem there is no reason we couldn't 
add header testing conditioned on a CMake option passed during the build 
configuration. So by default only minimal functionality could be tested as it 
is done now but if some option is being passed to cmake (say 
`ENABLE_FULL_OPENCL_BIFS_TESTS`) we could do comprehensive testing of the 
header. If there is interest in such an approach we could start from just 
adding the tests related to OpenCL 3.0 changes and then slowly build more tests 
in the future.

Another area where such tests would be needed is for Tablegen header enabled by 
`-fdeclare-opencl-builtins` that is developed to replace the regular header 
eventually because it is much quicker to parse. @svenvh I imagine the testing 
functionality between the two headers will be identical?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D92024: [clang] Partially implement P0692R1 from C++20 (access checking on specializations)

2020-11-24 Thread Alex Orlov via Phabricator via cfe-commits

aorlov created this revision.
aorlov added reviewers: broadwaylamb, rsmith, saar.raz, doug.gregor.
aorlov added projects: LLVM, clang.
Herald added a subscriber: cfe-commits.
aorlov requested review of this revision.

Disable usual access checking rules to template argument names in a declaration 
of partial specializations, explicit instantiation or explicit specialization 
(C++20 13.7.5/10, 13.9.1/6).

Fixes: https://llvm.org/PR37424


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92024

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/temp/temp.spec/func.spec.cpp
  clang/test/CXX/temp/temp.spec/part.spec.cpp

Index: clang/test/CXX/temp/temp.spec/part.spec.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.spec/part.spec.cpp
@@ -0,0 +1,321 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// C++20 [temp.class.spec] 17.6.5/10:
+//   The usual access checking rules do not apply to non-dependent names used
+//   to specify template arguments of the simple-template-id of the partial
+//   specialization.
+
+// class for tests
+class TestClass {
+public:
+  class PublicClass {};
+  template  class TemplatePublicClass {};
+
+  using AliasPublicClass = unsigned char;
+
+  void publicFunc();
+  void publicFuncOverloaded();
+  void publicFuncOverloaded(int);
+
+  static void publicStaticFunc();
+  static void publicStaticFuncOverloaded();
+  static void publicStaticFuncOverloaded(int);
+
+  static constexpr int publicStaticInt = 42;
+
+protected:
+  class ProtectedClass {};
+  template  class TemplateProtectedClass {};
+
+  using AliasProtectedClass = const char;
+
+  void protectedFunc();
+  void protectedFuncOverloaded();
+  void protectedFuncOverloaded(int);
+
+  static void protectedStaticFunc();
+  static void protectedStaticFuncOverloaded();
+  static void protectedStaticFuncOverloaded(int);
+
+  static constexpr int protectedStaticInt = 43;
+
+private:
+  class PrivateClass {};
+  template  class TemplatePrivateClass {};
+
+  using AliasPrivateClass = char *;
+
+  void privateFunc();
+  void privateFuncOverloaded();
+  void privateFuncOverloaded(int);
+
+  static void privateStaticFunc();
+  static void privateStaticFuncOverloaded();
+  static void privateStaticFuncOverloaded(int);
+
+  static constexpr int privateStaticInt = 44;
+};
+
+void globalFunction() {}
+
+//--//
+
+// template declarations for full
+template  class CT1 {};
+template  class CT2 {};
+template  class CT3 {};
+template  class CT4 {};
+template  class CT5 {};
+template  class CT6 {
+  template  class NCT1 {};
+  template  class NCT2; // forward declaration
+};
+
+// full specializations
+
+// public
+template <> class CT1;
+template  class CT1>;
+template <> class CT1>;
+template <> class CT1;
+template <> class CT2;
+template <> class CT3;
+template <> class CT4<&TestClass::publicFunc>;
+template <> class CT4<&TestClass::publicFuncOverloaded>;
+template <> class CT5<&TestClass::publicStaticFunc>;
+template <> class CT5<&TestClass::publicStaticFuncOverloaded>;
+template <> class CT5<&globalFunction>;
+template <> template <> class CT6::NCT1;
+
+template <> class CT1 {};
+template  class CT1> {};
+template <> class CT1> {};
+template <> class CT1 {};
+template <> class CT2 {};
+template <> class CT3 {};
+template <> class CT4<&TestClass::publicFunc> {};
+template <> class CT4<&TestClass::publicFuncOverloaded> {};
+template <> class CT5<&TestClass::publicStaticFunc> {};
+template <> class CT5<&TestClass::publicStaticFuncOverloaded> {};
+template <> class CT5<&globalFunction> {};
+template <> template <> class CT6::NCT1 {};
+template <> template  class CT6::NCT2 {}; // declaration
+
+// protected
+template <> class CT1;
+template  class CT1>;
+template <> class CT1>;
+template <> class CT1;
+template <> class CT2;
+template <> class CT3;
+template <> class CT4<&TestClass::protectedFunc>;
+template <> class CT4<&TestClass::protectedFuncOverloaded>;
+template <> class CT5<&TestClass::protectedStaticFunc>;
+template <> class CT5<&TestClass::protectedStaticFuncOverloaded>;
+template <> template <> class CT6::NCT1;
+
+template <> class CT1 {};
+template  class CT1> {};
+template <> class CT1> {};
+template <> class CT1 {};
+template <> class CT2 {};
+template <> class CT3 {};
+template <> class CT4<&TestClass::protectedFunc> {};
+template <> class CT4<&TestClass::protectedFuncOverloaded> {};
+template <> class CT5<&TestClass::protectedStaticFunc> {};
+template <> class CT5<&TestClass::protectedStaticFuncOverloaded> {};
+template <> template <> class CT6::NCT1 {};
+template <> template  class CT6::NCT2 {}; // declaration
+
+// private
+template <> class CT1;
+template  class CT1>;
+template <> class CT1>;
+template <> class CT1;
+template <> class CT2;
+template <

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

2020-11-24 Thread Rainer Orth via Phabricator via cfe-commits

ro added inline comments.



Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)

MaskRay wrote:
> ro wrote:
> > MaskRay wrote:
> > > ro wrote:
> > > > MaskRay wrote:
> > > > > ro wrote:
> > > > > > MaskRay wrote:
> > > > > > > GNU ld reports a warning instead of an error when an unknown `-z` 
> > > > > > > is seen. The warning remains a warning even with 
> > > > > > > `--fatal-warnings`.
> > > > > > > GNU ld reports a warning instead of an error when an unknown `-z` 
> > > > > > > is seen. The warning remains a warning even with 
> > > > > > > `--fatal-warnings`.
> > > > > > 
> > > > > > Thanks for reminding me about that misfeature of GNU `ld`.  I guess 
> > > > > > `check_linker_flags` needs to be updated to handle that.
> > > > > > In the case at hand, it won't matter either way: the flag is only 
> > > > > > passed to `ld`, which on Solaris is guaranteed to be the native 
> > > > > > linker.  Once (if at all) I get around to completing D85309, I can 
> > > > > > deal with that.  For now, other targets won't see linker warnings 
> > > > > > about this flag, other than when the flag is used at build time.
> > > > > OK. Then I guess you can condition this when the OS is Solaris?
> > > > > OK. Then I guess you can condition this when the OS is Solaris?
> > > > 
> > > > I fear not: `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` is tested inside an `if` 
> > > > in `Solaris.cpp`: this code is also compiled on non-Solaris hosts.  Why 
> > > > are you worried about the definition being always present?
> > > It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong 
> > > result for GNU ld, even if it is not used for non-Solaris. We should make 
> > > the value correct in other configurations.
> > > It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong 
> > > result for GNU ld, even if it is not used for non-Solaris. We should make 
> > > the value correct in other configurations.
> > 
> > Tell the binutils maintainers that ;-)  While I'm still unconcerned about 
> > this particular case (it's only used on a Solaris host where `clang` 
> > hardcodes the use of `/usr/bin/ld`), I continue to be annoyed by GNU `ld`'s 
> > nonsensical (or even outright dangerous) behaviour of accepting every `-z` 
> > option.
> > 
> > It took me some wrestling with `cmake` , but now `check_linker_flag` 
> > correctly rejects `-z ` flags where GNU `ld` produces the warning.
> > 
> > Some caveats about the implementation:
> > - `check_cxx_compiler_flag` doesn't support the `FAIL_REGEX` arg, so I had 
> > to switch to `check_cxx_source_compiles` instead.
> > - While it would be more appropriate to add the linker flag under test to 
> > `CMAKE_REQUIRED_LINK_OPTIONS`, that is only present since `cmake` 3.14 
> > while LLVM still only requires 3.13.
> > warning: -z.* ignored
> 
> Doesn't this stop working if binutils starts to use `error: -z.* ignored`? 
> Isn't there a way to call `check_linker_flag` only when the target is 
> Solaris? Does `LLVM_LINKER_IS_SOLARISLD` work?
> > warning: -z.* ignored
> 
> Doesn't this stop working if binutils starts to use `error: -z.* ignored`? 

No: `FAIL_REGEX` only adds to error detection, every other condition just 
remains as is.

> Isn't there a way to call `check_linker_flag` only when the target is 
> Solaris? Does `LLVM_LINKER_IS_SOLARISLD` work?

That would be wrong: this is about working around a GNU `ld` bug.  Imagine 
adding a new `-z` option to `lld` which either GNU `ld` didn't adopt at all or 
only in the latest release.  You'd want to reject the use of that option on 
earlier GNU `ld` just the same, no Solaris in sight.

As I said: `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` must always be defined because 
`Solaris.cpp` is always compiled no matter what the target.

I really don't understand what you're trying to guard against by putting up 
roadblock after roadblock for this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

2020-11-24 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D91898#2413004 , @NoQ wrote:

>> if the TCB-based functions are a small subset of the code then this 
>> attribute is better
>
> Yes, that's exactly the case. We want most functions to be untrusted by 
> default but also have no restrictions imposed or warnings emitted for them 
> when they do their usual function things.

Excellent, thank you both for considering it!




Comment at: clang/include/clang/Basic/Attr.td:3585
+def EnforceTCB : InheritableAttr {
+  let Spellings = [GCC<"enforce_tcb">];
+  let Subjects = SubjectList<[Function]>;

I don't think GCC supports these attributes, so this spelling should probably 
be `Clang` rather than `GCC`.



Comment at: clang/include/clang/Basic/AttrDocs.td:5497
+  called from a function with the enforce_tcb attribute. A function may be
+  a part of multiple TCBs. Invocations of function pointers and C++ methods
+  are not checked. Builtins are considered to a part of every TCB.

Are there plans to support this on function pointers or other kinds of callable 
things?

Also, it seems rather odd that C++ methods are not checked. I could somewhat 
understand not checking virtual functions, but why not member functions where 
the target is known? Why not static member functions?



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1246
+
+def TCBEnforcement : DiagGroup<"tcb-enforcement">;

Are you planning on adding more diagnostics under this group? If not, I'd 
suggest defining it inline (see below).



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11052
+def warn_tcb_enforcement_violation : Warning<
+  "TCB [%0] has been violated by calling a function [%1] that is not in the 
TCB.">,
+  InGroup;





Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11053
+  "TCB [%0] has been violated by calling a function [%1] that is not in the 
TCB.">,
+  InGroup;
 } // end of sema component.

...assuming the group won't be reused by too many diagnostics.



Comment at: clang/include/clang/Sema/Sema.h:12454
 
+  void CheckTCBEnforcement(CallExpr *TheCall,  FunctionDecl *Callee);
+

Can you make these `const` pointers?



Comment at: clang/lib/Sema/SemaChecking.cpp:16070
+void Sema::CheckTCBEnforcement(CallExpr *TheCall,  FunctionDecl *Callee) {
+  FunctionDecl *Caller = getCurFunctionDecl();
+





Comment at: clang/lib/Sema/SemaChecking.cpp:16072
+
+  // Calls to builtins are not enforced
+  if (!Caller || !Caller->hasAttr() || Callee->getBuiltinID() 
!= 0) {





Comment at: clang/lib/Sema/SemaChecking.cpp:16073-16075
+  if (!Caller || !Caller->hasAttr() || Callee->getBuiltinID() 
!= 0) {
+return;
+  }





Comment at: clang/lib/Sema/SemaChecking.cpp:16079-16084
+  for (const auto *Attr : Callee->specific_attrs()) {
+CalleeTCBs.insert(Attr->getTCBName());
+  }
+  for (const auto *Attr : Callee->specific_attrs()) {
+CalleeTCBs.insert(Attr->getTCBName());
+  }

Pretty sure you can remove the manual loops here with something like this.



Comment at: clang/lib/Sema/SemaChecking.cpp:16086
+
+  // Go through the TCBs the caller is a part of and emit warnings if Caller 
is in a TCB that the Callee is not
+  for (const auto *Attr : Caller->specific_attrs()) {

Can you be sure to run the patch through clang-format, this looks like it's 
over the 80 col limit.



Comment at: clang/lib/Sema/SemaChecking.cpp:16088
+  for (const auto *Attr : Caller->specific_attrs()) {
+llvm::StringRef CallerTCB = Attr->getTCBName();
+





Comment at: clang/lib/Sema/SemaChecking.cpp:16090-16092
+if (CalleeTCBs.count(CallerTCB) == 0) {
+  Diag(TheCall->getExprLoc(), diag::warn_tcb_enforcement_violation) << 
CallerTCB << Callee->getName();
+}





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7339
 
+template
+static void handleEnforceTCBAttr(Sema &S, Decl *D, const ParsedAttr &AL) {





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7344
+return;
+  D->addAttr(Attr::Create(S.Context, Argument, AL));
+}





Comment at: clang/test/Sema/attr-enforce-tcb.c:1
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+





Comment at: clang/test/Sema/attr-enforce-tcb.c:15
+void foo9 (void);
+
+void

Some more interesting tests:
```
// Ensure that attribute merging works as expected across redeclarations.
void foo10() PLACE_IN_TCB("baz");
void foo10() PLACE_IN_TCB("bar");
void foo10() PLACE_IN_TCB("quux");

// Ensure that attribute instantiation works

[PATCH] D91952: [clangd] Add support for within-file rename of complicated fields

2020-11-24 Thread Kirill Bobyrev via Phabricator via cfe-commits

kbobyrev updated this revision to Diff 307327.
kbobyrev added a comment.

Added support for static templated fields. Ready for the review again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91952

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -540,6 +540,94 @@
 }
   )cpp",
 
+  // Fields in classes & partial and full specialiations.
+  R"cpp(
+template
+struct Foo {
+  T [[Vari^able]] = 42;
+};
+
+void foo() {
+  Foo f;
+  f.[[Varia^ble]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  T Variable[42];
+  U Another;
+
+  void bar() {}
+};
+
+template
+struct Foo {
+  T Variable;
+  void bar() { ++Variable; }
+};
+
+template<>
+struct Foo {
+  unsigned [[Var^iable]];
+  void bar() { ++[[Var^iable]]; }
+};
+
+void foo() {
+  Foo f;
+  f.[[Var^iable]] = 9000;
+}
+  )cpp",
+  // Static fields.
+  R"cpp(
+struct Foo {
+  static int [[Var^iable]];
+};
+
+int Foo::[[Var^iable]] = 42;
+
+void foo() {
+  int LocalInt = Foo::[[Var^iable]];
+}
+  )cpp",
+  R"cpp(
+template
+struct Foo {
+  static T [[Var^iable]];
+};
+
+template <>
+int Foo::[[Var^iable]] = 42;
+
+template <>
+bool Foo::[[Var^iable]] = true;
+
+void foo() {
+  int LocalInt = Foo::[[Var^iable]];
+  bool LocalBool = Foo::[[Var^iable]];
+}
+  )cpp",
+
   // Template parameters.
   R"cpp(
 template 
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -124,6 +124,31 @@
   if (const auto *Function = dyn_cast(D))
 if (const FunctionTemplateDecl *Template = Function->getPrimaryTemplate())
   return canonicalRenameDecl(Template);
+  if (const auto *Field = dyn_cast(D)) {
+// This is a hacky way to do something like
+// CXXMethodDecl::getInstantiatedFromMemberFunction for the field because
+// Clang AST does not store relevant information about the field that is
+// instantiated.
+// FIXME: Maybe a proper way of fixing this would be enhancing Clang AST
+// although it might be a bigger effort.
+const auto *FieldParent = dyn_cast(Field->getParent())
+  ->getTemplateInstantiationPattern();
+if (!FieldParent)
+  return Field->getCanonicalDecl();
+// Field is not instantiation.
+if (!FieldParent || Field->getParent() == FieldParent)
+  return Field->getCanonicalDecl();
+for (const FieldDecl *Candidate : FieldParent->fields())
+  if (Field->getFieldIndex() == Candidate->getFieldIndex())
+return Candidate->getCanonicalDecl();
+elog("FieldParent should have field with same index as Field.");
+  }
+  if (const auto *VD = dyn_cast(D)) {
+const VarDecl *OriginalVD = VD->getInstantiatedFromStaticDataMember();
+if (OriginalVD)
+  VD = OriginalVD;
+return VD->getCanonicalDecl();
+  }
   return dyn_cast(D->getCanonicalDecl());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

2020-11-24 Thread Francesco Petrogalli via Phabricator via cfe-commits

fpetrogalli updated this revision to Diff 307329.
fpetrogalli added a comment.

Add comment to `DbgVariableIntrinsic::getFragmentSizeInBits()`. NFC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

Files:
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  
llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll

Index: llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/debug-declare-no-warnings-on-scalable-vectors.ll
@@ -0,0 +1,42 @@
+; RUN: opt -mtriple aarch64-gnu-linux -mattr=+sve -instcombine -S < %s 2>%t | FileCheck %s
+; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
+
+; If this check fails please read
+; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on
+; how to resolve it.
+
+; WARN-NOT: warning
+
+; CHECK-LABEL: @debug_local_scalable(
+define  @debug_local_scalable( %tostore) {
+  %vx = alloca , align 16
+  call void @llvm.dbg.declare(metadata * %vx, metadata !5, metadata !DIExpression()), !dbg !15
+  store  %tostore, * %vx, align 16
+  %ret = call  @f(* %vx)
+  ret  %ret
+}
+
+declare  @f(*)
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !DILocalVariable(name: "vx", scope: !6, file: !7, line: 26, type: !8)
+!6 = distinct !DISubprogram(name: "debug_local_scalable", scope: null, file: !1, line: 25, scopeLine: 25, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DIFile(filename: "test.c", directory: "/tmp/")
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "svfloat64_t", file: !9, line: 56, baseType: !10)
+!9 = !DIFile(filename: "arm_sve.h", directory: "/tmp/")
+!10 = !DIDerivedType(tag: DW_TAG_typedef, name: "__SVFloat64_t", file: !1, baseType: !11)
+!11 = !DICompositeType(tag: DW_TAG_array_type, baseType: !12, flags: DIFlagVector, elements: !13)
+!12 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)
+!13 = !{!14}
+!14 = !DISubrange(lowerBound: 0, upperBound: !DIExpression(DW_OP_constu, 1, DW_OP_bregx, 46, 0, DW_OP_mul, DW_OP_constu, 1, DW_OP_minus))
+!15 = !DILocation(line: 26, column: 15, scope: !6)
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1368,16 +1368,22 @@
 /// least n bits.
 static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   const DataLayout &DL = DII->getModule()->getDataLayout();
-  uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
-  if (auto FragmentSize = DII->getFragmentSizeInBits())
-return ValueSize >= *FragmentSize;
+  TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy);
+  if (Optional FragmentSize = DII->getFragmentSizeInBits()) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // We can't always calculate the size of the DI variable (e.g. if it is a
   // VLA). Try to use the size of the alloca that the dbg intrinsic describes
   // intead.
   if (DII->isAddressOfVariable())
 if (auto *AI = dyn_cast_or_null(DII->getVariableLocation()))
-  if (auto FragmentSize = AI->getAllocationSizeInBits(DL))
-return ValueSize >= *FragmentSize;
+  if (Optional FragmentSize = AI->getAllocationSizeInBits(DL)) {
+assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
+   "Both sizes should agree on the scalable flag.");
+return TypeSize::isKnownGE(ValueSize, *FragmentSize);
+  }
   // Could not determine size of variable. Conservatively return false.
   return false;
 }
Index: llvm/lib/Transforms/Utils/Debugify.cpp
===
--- llvm/lib/Transforms/Utils/Debugify.cpp
+++ llvm/lib/Transforms/Utils/Debugify.cpp
@@ -44,8 +44,9 @@
 
 raw_ostream &dbg() { return Quiet ? nulls() : errs(); }
 
-uint64_t getAllocSizeInBits(Module &M, Type *Ty) {
-  return Ty->isSized() ? M.getDataLayout().getTypeAllocSizeInBits(Ty) : 0;
+TypeSize getAllocSizeInBits(Module &M, Typ

[PATCH] D91975: [clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

2020-11-24 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 aside from some small nits, thank you for the fix!




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp:346
 
+// We understand typedefs
+void typedef_context() {





Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp:351
+  myint64_t i64;
+
+  i = i64;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

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

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits

spatel added a comment.

In D88712#2412874 , 
@venkataramanan.kumar.llvm wrote:

> In D88712#2412366 , @MaskRay wrote:
>
>> In D88712#2411841 , 
>> @venkataramanan.kumar.llvm wrote:
>>
>>> 
>>
>> For your example:
>>
>>   extern double log (double) asm ("" "log_finite") __attribute__ ((nothrow));
>>   
>>   double mylog (double d) { return log(d); }
>>
>> The intention is to emit `log_finite`, no matter the `-ffast-math`. Both GCC 
>> and Clang (after this patch) emit `jmp log_finite`.
>>
>> The previous behavior (according to your comment, with an older glibc) was 
>> undesired. So I think the right suggestion is to upgrade glibc.
>
> Then there optimizations like vectorization, inst combine which works on the 
> LLVM intrinsics.  But they will be not happening now  with  log_finite calls.

I'm not sure about the expected semantics/lowering for the finite calls, but 
can you add something under LibCallSimplifier::optimizeFloatingPointLibCall() 
that would turn it into the LLVM log intrinsic with appropriate FMF? Codegen 
would need to be responsible for converting it back to finite call(s) if those 
are available?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

[PATCH] D91806: [SVE] Remove warning from debug info on scalable vector.

2020-11-24 Thread Francesco Petrogalli via Phabricator via cfe-commits

fpetrogalli added a comment.

Hi @sdesmalen

I have extracted D92020  to implement only the 
change of interface for `AllocaInst::getAllocationSizeInBits`.

I wanted to extract also the interface change in 
`DbgVariableIntrinsic::getFragmentSizeInBits()` but such change would require 
also to change `valueCoversEntireFragment`: essentially, this is already what I 
am doing in this patch.,

> This patch needs to be retitled to what this is actually doing: changing the 
> getTypeAllocationSizeInBits and getFragmentSizeInBits to return a TypeSize 
> instead of unsigned.

Given that the only change resulting from this patch is removing the warning, I 
think it makes sense to keep the summary of the patch as it is.

I have added a comment in `DbgVariableIntrinsic::getFragmentSizeInBits()` 
explaining that to get full support for scalable types in 
`DbgVariableIntrinsic::getFragmentSizeInBits()` we should update the 
`DIVariable::getSizeInBits` to carry the scalable flag for scalable variables. 
I am happy to continue in that direction (it seems an extensive change with 
quite a few implications), I just wanted to double check whether 1. you agree 
on the need to change `DIVariable::getSizeInBits` to privide `TypeSize` info, 
and 2.  if you think if you are happy for the warning fix (this patch)  to go 
in before the `DIVariable::getSizeInBits` interface change, or 3. do you see a 
third way? :)

Cheers,

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

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

[PATCH] D92028: Fix driver test from e16c0a9a689719

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

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

The test failed silently if lld wasn't built alongside clang.
But the test uses -###, so the "invalid linker name in -fuse-ld=lld"
diag didn't make clang fail, and something else happened to match
"-demangle", so the test passed.

To fix, pass -B to a directory with two empty +x files (which works
on non-Windows), and look for `"-demangle"` instead of just `-demangle`.
Also force linker_version to 0 and pass a darwin triple.


https://reviews.llvm.org/D92028

Files:
  clang/test/Driver/Inputs/lld/ld64.lld
  clang/test/Driver/Inputs/lld/ld64.lld.darwinnew
  clang/test/Driver/darwin-ld-demangle-lld.c


Index: clang/test/Driver/darwin-ld-demangle-lld.c
===
--- clang/test/Driver/darwin-ld-demangle-lld.c
+++ clang/test/Driver/darwin-ld-demangle-lld.c
@@ -1,6 +1,11 @@
 // With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+// REQUIRES: shell
 
-// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
 // FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
-// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
-// CHECK: -demangle
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld.darwinnew -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
+// CHECK: "-demangle"


Index: clang/test/Driver/darwin-ld-demangle-lld.c
===
--- clang/test/Driver/darwin-ld-demangle-lld.c
+++ clang/test/Driver/darwin-ld-demangle-lld.c
@@ -1,6 +1,11 @@
 // With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+// REQUIRES: shell
 
-// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
 // FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
-// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
-// CHECK: -demangle
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld.darwinnew -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
+// CHECK: "-demangle"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90928: [OpenCL] Check for extension string extension lookup

2020-11-24 Thread Erik Tomusk via Phabricator via cfe-commits

erik2020 updated this revision to Diff 307338.
erik2020 added a comment.

Added doxygen comments to updated functions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90928

Files:
  clang/include/clang/Basic/OpenCLOptions.h


Index: clang/include/clang/Basic/OpenCLOptions.h
===
--- clang/include/clang/Basic/OpenCLOptions.h
+++ clang/include/clang/Basic/OpenCLOptions.h
@@ -32,38 +32,71 @@
   };
   llvm::StringMap OptMap;
 public:
+  /// Check if \c Ext is a recognized OpenCL extension.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \returns \c true if \c Ext is known, \c false otherwise.
   bool isKnown(llvm::StringRef Ext) const {
 return OptMap.find(Ext) != OptMap.end();
   }
 
+  /// Check if \c Ext is an enabled OpenCL extension.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \returns \c true if \c Ext is known and enabled, \c false otherwise.
   bool isEnabled(llvm::StringRef Ext) const {
-return OptMap.find(Ext)->second.Enabled;
+auto E = OptMap.find(Ext);
+return E != OptMap.end() && E->second.Enabled;
   }
 
-  // Is supported as either an extension or an (optional) core feature for
-  // OpenCL version \p CLVer.
+  /// Check if \c Ext is supported as either an extension or an (optional) core
+  /// feature for the given OpenCL version.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \param LO - \c LangOptions specifying the OpenCL version.
+  /// \returns \c true if \c Ext is known and supported, \c false otherwise.
   bool isSupported(llvm::StringRef Ext, const LangOptions &LO) const {
+auto E = OptMap.find(Ext);
+if (E == OptMap.end()) {
+  return false;
+}
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
-auto I = OptMap.find(Ext)->getValue();
+auto I = E->getValue();
 return I.Supported && I.Avail <= CLVer;
   }
 
-  // Is supported (optional) OpenCL core features for OpenCL version \p CLVer.
-  // For supported extension, return false.
+  /// Check if \c Ext is supported as an (optional) OpenCL core features for
+  /// the given OpenCL version.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \param LO - \c LangOptions specifying the OpenCL version.
+  /// \returns \c true if \c Ext is known and supported, \c false otherwise.
   bool isSupportedCore(llvm::StringRef Ext, const LangOptions &LO) const {
+auto E = OptMap.find(Ext);
+if (E == OptMap.end()) {
+  return false;
+}
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
-auto I = OptMap.find(Ext)->getValue();
+auto I = E->getValue();
 return I.Supported && I.Avail <= CLVer && I.Core != ~0U && CLVer >= I.Core;
   }
 
-  // Is supported OpenCL extension for OpenCL version \p CLVer.
-  // For supported (optional) core feature, return false.
+  /// Check if \c Ext is a supported OpenCL extension for the given OpenCL
+  /// version.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \param LO - \c LangOptions specifying the OpenCL version.
+  /// \returns \c true if \c Ext is known and supported, \c false otherwise.
   bool isSupportedExtension(llvm::StringRef Ext, const LangOptions &LO) const {
+auto E = OptMap.find(Ext);
+if (E == OptMap.end()) {
+  return false;
+}
 // In C++ mode all extensions should work at least as in v2.0.
 auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
-auto I = OptMap.find(Ext)->getValue();
+auto I = E->getValue();
 return I.Supported && I.Avail <= CLVer && (I.Core == ~0U || CLVer < 
I.Core);
   }
 


Index: clang/include/clang/Basic/OpenCLOptions.h
===
--- clang/include/clang/Basic/OpenCLOptions.h
+++ clang/include/clang/Basic/OpenCLOptions.h
@@ -32,38 +32,71 @@
   };
   llvm::StringMap OptMap;
 public:
+  /// Check if \c Ext is a recognized OpenCL extension.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \returns \c true if \c Ext is known, \c false otherwise.
   bool isKnown(llvm::StringRef Ext) const {
 return OptMap.find(Ext) != OptMap.end();
   }
 
+  /// Check if \c Ext is an enabled OpenCL extension.
+  ///
+  /// \param Ext - Extension to look up.
+  /// \returns \c true if \c Ext is known and enabled, \c false otherwise.
   bool isEnabled(llvm::StringRef Ext) const {
-return OptMap.find(Ext)->second.Enabled;
+auto E = OptMap.find(Ext);
+return E != OptMap.end() && E->second.Enabled;
   }
 
-  // Is supported as either an extension or an (optional) core feature for
-  // OpenCL version \p CLVer.
+  /// Check if \c Ext is supported as either an extension or an (optional) core
+  /// feature for the given OpenCL version.
+  ///
+  /// \param Ext - Extension to look

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Sergey Dmitriev via Phabricator via cfe-commits

sdmitriev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:411
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };

ABataev wrote:
> What about `llvm::iplist`?
It can be changed to iplist as well, but why do you think it is a better option 
here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

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

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:411
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };

sdmitriev wrote:
> ABataev wrote:
> > What about `llvm::iplist`?
> It can be changed to iplist as well, but why do you think it is a better 
> option here?
Just better to use LLVM API if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

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

[PATCH] D90775: [clangd] ExternalIndex turns off BackgroundIndex only if it isn't set

2020-11-24 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:308
+  if (C.Index.Background == Config::BackgroundPolicy::Auto)
+C.Index.Background = Config::BackgroundPolicy::Skip;
 });

kadircet wrote:
> sammccall wrote:
> > doesn't this mean that if a user has explicit `Background: Build` at a 
> > higher scope, then a more narrowly scoped external index won't disable it?
> > 
> > This seems at least as confusing as the problems this is trying to solve.
> > doesn't this mean that if a user has explicit Background: Build at a higher 
> > scope, then a more narrowly scoped external index won't disable it?
> 
> Yes. The motivating case I had was something like this:
> - Let's say user turns background-index on inside 
> `llvm-project/clang-tools-extra/clangd/.clangd`.
> - Defines an external index via user-config for `llvm-project`.
> 
> Since user-config is the inner-most provider, it will turn off the 
> backgrond-indexing implicitly for the whole project, and user won't be able 
> to turn it on for a subdirectory without specifying that in user-config 
> directory too.
In that scenario, I'm not sure whether the user wants background indexing on or 
not. (e.g. more likely *someone else* turned it on in the project config, who 
wasn't aware of the user config).

Maybe there's a principle here that project config for an inner directory is 
actually more specific and therefore should take precedence over user config 
for an outer directory? That seems like it might be at least as useful as the 
current behavior, but we shouldn't be implementing it setting-by-setting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90775

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

[PATCH] D92028: Fix driver test from e16c0a9a689719

2020-11-24 Thread Hans Wennborg via Phabricator via cfe-commits

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



Comment at: clang/test/Driver/darwin-ld-demangle-lld.c:11
+// RUN:   | FileCheck %s
+// CHECK: "-demangle"

nit: most tests have a blank line between the RUN block and the checks/other 
stuff


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

https://reviews.llvm.org/D92028

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

[PATCH] D91966: [clangd] AddUsing: Used spelled text instead of type name.

2020-11-24 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz updated this revision to Diff 307347.
adamcz marked an inline comment as done.
adamcz added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91966

Files:
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2520,6 +2520,9 @@
   EXPECT_UNAVAILABLE(Header + "void fun() { ::ban::fo^o(); }");
   EXPECT_AVAILABLE(Header + "void fun() { banana::fo^o(); }");
 
+  // Do not offer code action on typo-corrections.
+  EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
+
   // Check that we do not trigger in header files.
   FileName = "test.h";
   ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
@@ -2793,6 +2796,35 @@
 
 void fun() {
   ff();
+})cpp"},
+// using alias; insert using for the spelled name.
+{R"cpp(
+#include "test.hpp"
+
+void fun() {
+  one::u^u u;
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using one::uu;
+
+void fun() {
+  uu u;
+})cpp"},
+// using namespace.
+{R"cpp(
+#include "test.hpp"
+using namespace one;
+namespace {
+two::c^c C;
+})cpp",
+ R"cpp(
+#include "test.hpp"
+using namespace one;
+namespace {using two::cc;
+
+cc C;
 })cpp"}};
   llvm::StringMap EditedFiles;
   for (const auto &Case : Cases) {
@@ -2809,6 +2841,7 @@
   static void mm() {}
 };
 }
+using uu = two::cc;
 })cpp";
   EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
 }
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -43,9 +43,10 @@
   }
 
 private:
-  // The qualifier to remove. Set by prepare().
+  // All of the following are set by prepare().
+  // The qualifier to remove.
   NestedNameSpecifierLoc QualifierToRemove;
-  // The name following QualifierToRemove. Set by prepare().
+  // The name following QualifierToRemove.
   llvm::StringRef Name;
 };
 REGISTER_TWEAK(AddUsing)
@@ -206,8 +207,17 @@
   return false;
 }
 
+std::string getNNSLAsString(NestedNameSpecifierLoc &NNSL,
+const PrintingPolicy &Policy) {
+  std::string Out;
+  llvm::raw_string_ostream OutStream(Out);
+  NNSL.getNestedNameSpecifier()->print(OutStream, Policy);
+  return OutStream.str();
+}
+
 bool AddUsing::prepare(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
+  const auto &TB = Inputs.AST->getTokens();
 
   // Do not suggest "using" in header files. That way madness lies.
   if (isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
@@ -247,11 +257,20 @@
 }
   } else if (auto *T = Node->ASTNode.get()) {
 if (auto E = T->getAs()) {
-  if (auto *BaseTypeIdentifier =
-  E.getType().getUnqualifiedType().getBaseTypeIdentifier()) {
-Name = BaseTypeIdentifier->getName();
-QualifierToRemove = E.getQualifierLoc();
-  }
+  QualifierToRemove = E.getQualifierLoc();
+
+  auto SpelledTokens =
+  TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
+  if (!SpelledTokens)
+return false;
+  auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(),
+   SpelledTokens->back());
+  Name = SpelledRange.text(SM);
+
+  std::string QualifierToRemoveStr = getNNSLAsString(
+  QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
+  if (!Name.consume_front(QualifierToRemoveStr))
+return false; // What's spelled doesn't match the qualifier.
 }
   }
 
@@ -283,20 +302,13 @@
 
 Expected AddUsing::apply(const Selection &Inputs) {
   auto &SM = Inputs.AST->getSourceManager();
-  auto &TB = Inputs.AST->getTokens();
 
-  // Determine the length of the qualifier under the cursor, then remove it.
-  auto SpelledTokens = TB.spelledForExpanded(
-  TB.expandedTokens(QualifierToRemove.getSourceRange()));
-  if (!SpelledTokens) {
-return error("Could not determine length of the qualifier");
-  }
-  unsigned Length =
-  syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back())
-  .length();
+  std::string QualifierToRemoveStr = getNNSLAsString(
+  QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
   tooling::Replacements R;
   if (auto Err = R.add(tooling::Replacement(
-  SM, SpelledTokens->front().location(), Length, ""))) {
+  SM, SM.getSpellingLoc(QualifierToRemove.getBeginLoc()),
+  QualifierToRemoveStr.length(), ""))) {
 return std::mo

[PATCH] D91966: [clangd] AddUsing: Used spelled text instead of type name.

2020-11-24 Thread Adam Czachorowski via Phabricator via cfe-commits

adamcz added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:268
+   SpelledTokens->back());
+  llvm::StringRef NameRef = SpelledRange.text(SM);
+

kadircet wrote:
> what about saving the full range here, and using it's `start` and `length - 
> name.size()` as removal range in `apply`?
> 
> similarly we can also use the `range.text` as text to insert. that way we can 
> inline `getNNSLAsString` as it won't be needed elsewhere anymore.
We only have a range in this branch of the code. The other one (for 
DeclRefExpr) is not using TokenBuffers, so no range right now.

We may change it in the future. Or would you prefer this changed now?



Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:274
+return false; // What's spelled doesn't match the qualifier.
+  Name = NameRef.str();
 }

kadircet wrote:
> it is not that important, but I suppose `NameRef` is going to be alive during 
> apply, so I suppose we can keep `Name` as a `llvm::StringRef`.
Right, it used to be NameRef, then in some random version of the code 
pre-review it had to be a string and I never reverted that. Good point, thanks.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2814
+  uu u;
 })cpp"}};
   llvm::StringMap EditedFiles;

kadircet wrote:
> Tests don't seem to be covering something like:
> ```
> namespace foo { namespace bar { struct X {}; } }
> using namespace foo;
> bar::X^ x;
> ```
So this has the unfortunate side-effect of triggering a variation of 
https://github.com/clangd/clangd/issues/585

That's something I'm fixing in a later change (although this case is still not 
handled). For now, I'm putting an extra namespace scope to make this test 
correct, while still testing your example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91966

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

[PATCH] D91310: [AMDGPU] Add -mcode-object-version=n

2020-11-24 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

ping


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

https://reviews.llvm.org/D91310

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

[PATCH] D91975: [clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

2020-11-24 Thread Eric Seidel via Phabricator via cfe-commits

gridaphobe updated this revision to Diff 307348.
gridaphobe added a comment.

Add a couple passing tests involving typedefs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,17 @@
   DERP(i, .5l);
 }
 
+// We understand typedefs.
+void typedef_context() {
+  typedef long long myint64_t;
+  int i;
+  myint64_t i64;
+
+  i64 = i64; // Okay, no conversion.
+  i64 = i;   // Okay, no narrowing.
+
+  i = i64;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 
'myint64_t' (aka 'long long') to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+}
+
 } // namespace floats
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  implicitCastExpr(hasImplicitDestinationType(builtinType()),
-   hasSourceExpression(hasType(builtinType())),
+  implicitCastExpr(hasImplicitDestinationType(
+   hasUnqualifiedDesugaredType(builtinType())),
+   hasSourceExpression(hasType(
+   hasUnqualifiedDesugaredType(builtinType(,
unless(hasSourceExpression(IsCeilFloorCallExpr)),
unless(hasParent(castExpr())),
unless(isInTemplateInstantiation()))
@@ -58,16 +60,18 @@
 
   // Binary operators:
   //   i += 0.5;
-  Finder->addMatcher(binaryOperator(isAssignmentOperator(),
-hasLHS(expr(hasType(builtinType(,
-hasRHS(expr(hasType(builtinType(,
-unless(hasRHS(IsCeilFloorCallExpr)),
-unless(isInTemplateInstantiation()),
-// The `=` case generates an implicit cast
-// which is covered by the previous 
matcher.
-unless(hasOperatorName("=")))
- .bind("binary_op"),
- this);
+  Finder->addMatcher(
+  binaryOperator(
+  isAssignmentOperator(),
+  hasLHS(expr(hasType(hasUnqualifiedDesugaredType(builtinType(),
+  hasRHS(expr(hasType(hasUnqualifiedDesugaredType(builtinType(),
+  unless(hasRHS(IsCeilFloorCallExpr)),
+  unless(isInTemplateInstantiation()),
+  // The `=` case generates an implicit cast
+  // which is covered by the previous matcher.
+  unless(hasOperatorName("=")))
+  .bind("binary_op"),
+  this);
 }
 
 static const BuiltinType *getBuiltinType(const Expr &E) {


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,17 @@
   DERP(i, .5l);
 }
 
+// We understand typedefs.
+void typedef_context() {
+  typedef long long myint64_t;
+  int i;
+  myint64_t i64;
+
+  i64 = i64; // Okay, no conversion.
+  i64 = i;   // Okay, no narrowing.
+
+  i = i64;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'myint64_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
 } // namespace floats
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  implicitCastExpr(hasImplicitDestinationType(builtinType()),
-   hasSourceExpression(hasType(builtinType())),
+  implicitCastEx

[PATCH] D91029: [clangd] Implement clang-tidy options from config

2020-11-24 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 307349.
njames93 added a comment.

Small tweaks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91029

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/TidyProvider.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h

Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -17,6 +17,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
+#include "../TidyProvider.h"
 #include "Compiler.h"
 #include "ParsedAST.h"
 #include "TestFS.h"
@@ -58,8 +59,7 @@
   // Extra arguments for the compiler invocation.
   std::vector ExtraArgs;
 
-  llvm::Optional ClangTidyChecks;
-  llvm::Optional ClangTidyWarningsAsErrors;
+  mutable TidyProvider ClangTidyProvider = {};
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -59,8 +59,8 @@
 FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = &FS;
   Inputs.Opts = ParseOptions();
-  Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
-  Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
+  if (ClangTidyProvider)
+Inputs.ClangTidyProvider = ClangTidyProvider;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
 Inputs.Opts.SuggestMissingIncludes = true;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -250,7 +251,8 @@
   TestTU TU;
   // this check runs the preprocessor, we need to make sure it does not break
   // our recording logic.
-  TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("modernize-use-trailing-return-type");
   TU.Code = "inline int foo() {}";
 
   auto AST = TU.build();
@@ -406,7 +408,7 @@
   "replay-preamble-module", "");
   TestTU TU;
   // This check records inclusion directives replayed by clangd.
-  TU.ClangTidyChecks = "replay-preamble-check";
+  TU.ClangTidyProvider = fixedTidyProvider("replay-preamble-check");
   llvm::Annotations Test(R"cpp(
 $hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
 $hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -14,6 +14,7 @@
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
+#include "TidyProvider.h"
 #include "index/MemIndex.h"
 #include "support/Path.h"
 #include "clang/Basic/Diagnostic.h"
@@ -129,7 +130,7 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ClangTidyChecks = "-*,google-explicit-constructor";
+  TU.ClangTidyProvider = fixedTidyProvider("-*,google-explicit-constructor");
   EXPECT_THAT(
   TU.build().getDiagnostics(),
   ElementsAre(
@@ -201,8 +202,9 @@
   auto TU = TestTU::withCode(Test.code());
   // Enable alias clang-tidy checks, these check emits the same diagnostics
   // (except the check name).
-  TU.ClangTidyChecks = "-*, readability-uppercase-literal-suffix, "
-   "hicpp-uppercase-literal-suffix";
+  TU.ClangTidyProvider =
+  fixedTidyProvider("-*, readability-uppercase-literal-suffix, "
+"hicpp-uppercase-literal-suffix");
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
   TU.build().getDiagnostics(),
@@ -245,9 +247,9 @@
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   TU.HeaderFilename = "assert.h"; // Suppress "not found" e

[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

2020-11-24 Thread Sven van Haastregt via Phabricator via cfe-commits

svenvh added a comment.

In D92004#2413603 , @Anastasia wrote:

> @svenvh I imagine the testing functionality between the two headers will be 
> identical?

Mostly, assuming you intend to test in the conventional way of feeding a .cl 
file to Clang.  Although currently `-fdeclare-opencl-builtins` only provides a 
subset of the builtins in `opencl-c.h`.

In the past I did have a look at the possibility of automated equivalence 
checking of declarations provided by both `-fdeclare-opencl-builtins` and 
`opencl-c.h`.  It shouldn't be impossible, but it's not trivial (one reason 
being that gentype-expansion is currently done in Sema).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92004

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

[PATCH] D92033: [OpenCL] Move kernel arg type tests into one file

2020-11-24 Thread Sven van Haastregt via Phabricator via cfe-commits

svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added a subscriber: yaxunl.
Herald added a project: clang.
svenvh requested review of this revision.

Keep all kernel parameter type diagnostic tests in
`invalid-kernel-parameters.cl`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92033

Files:
  clang/test/SemaOpenCL/invalid-kernel-parameters.cl
  clang/test/SemaOpenCL/invalid-kernel.cl


Index: clang/test/SemaOpenCL/invalid-kernel.cl
===
--- clang/test/SemaOpenCL/invalid-kernel.cl
+++ clang/test/SemaOpenCL/invalid-kernel.cl
@@ -1,12 +1,6 @@
 // RUN: %clang_cc1 -verify %s
 // RUN: %clang_cc1 -cl-std=CL2.0 -verify %s
 
-kernel void no_ptrptr(global int * global *i) { } // expected-error{{kernel 
parameter cannot be declared as a pointer to a pointer}}
-
-__kernel void no_privateptr(__private int *i) { } // expected-error {{pointer 
arguments to kernel functions must reside in '__global', '__constant' or 
'__local' address space}}
-
-__kernel void no_privatearray(__private int i[]) { } // expected-error 
{{pointer arguments to kernel functions must reside in '__global', '__constant' 
or '__local' address space}}
-
 kernel int bar()  { // expected-error {{kernel must have void return type}}
   return 6;
 }
@@ -30,6 +24,3 @@
 int* constant x(int* x) { // expected-error {{return value cannot be qualified 
with address space}}
   return x + 1;
 }
-
-__kernel void testKernel(int *ptr) { // expected-error {{pointer arguments to 
kernel functions must reside in '__global', '__constant' or '__local' address 
space}}
-}
Index: clang/test/SemaOpenCL/invalid-kernel-parameters.cl
===
--- clang/test/SemaOpenCL/invalid-kernel-parameters.cl
+++ clang/test/SemaOpenCL/invalid-kernel-parameters.cl
@@ -1,9 +1,21 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple spir-unknown-unknown
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple spir-unknown-unknown 
-cl-std=CL2.0
 
 kernel void half_arg(half x) { } // expected-error{{declaring function 
parameter of type '__private half' is not allowed; did you forget * ?}}
 
 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
+// expected-error@+1{{kernel parameter cannot be declared as a pointer to a 
pointer}}
+kernel void no_ptrptr(global int * global *i) { }
+
+// expected-error@+1{{pointer arguments to kernel functions must reside in 
'__global', '__constant' or '__local' address space}}
+__kernel void no_privateptr(__private int *i) { }
+
+// expected-error@+1{{pointer arguments to kernel functions must reside in 
'__global', '__constant' or '__local' address space}}
+__kernel void no_privatearray(__private int i[]) { }
+
+// expected-error@+1{{pointer arguments to kernel functions must reside in 
'__global', '__constant' or '__local' address space}}
+__kernel void no_addrsp_ptr(int *ptr) { }
 
 // Disallowed: parameters with type
 // bool, half, size_t, ptrdiff_t, intptr_t, and uintptr_t
@@ -65,14 +77,14 @@
 
 typedef struct Foo // expected-note{{within field of type 'Foo' declared here}}
 {
-  int* ptrField; // expected-note{{field of illegal pointer type '__private 
int *' declared here}}
+  int* ptrField; // expected-note-re{{field of illegal pointer type 
'__{{private|generic}} int *' declared here}}
 } Foo;
 
 kernel void pointer_in_struct_arg(Foo arg) { } // expected-error{{struct 
kernel parameters may not contain pointers}}
 
 typedef union FooUnion // expected-note{{within field of type 'FooUnion' 
declared here}}
 {
-  int* ptrField; // expected-note{{field of illegal pointer type '__private 
int *' declared here}}
+  int* ptrField; // expected-note-re{{field of illegal pointer type 
'__{{private|generic}} int *' declared here}}
 } FooUnion;
 
 kernel void pointer_in_union_arg(FooUnion arg) { }// expected-error{{union 
kernel parameters may not contain pointers}}
@@ -82,7 +94,7 @@
   int x;
   struct InnerNestedPointer
   {
-int* ptrField; // expected-note 3 {{field of illegal pointer type 
'__private int *' declared here}}
+int* ptrField; // expected-note-re 3 {{field of illegal pointer type 
'__{{private|generic}} int *' declared here}}
   } inner; // expected-note 3 {{within field of type 'struct 
InnerNestedPointer' declared here}}
 } NestedPointer;
 
@@ -96,7 +108,7 @@
   struct InnerNestedPointerComplex
   {
 int innerFoo;
-int* innerPtrField; // expected-note{{field of illegal pointer type 
'__private int *' declared here}}
+int* innerPtrField; // expected-note-re{{field of illegal pointer type 
'__{{private|generic}} int *' declared here}}
   } inner; // expected-note{{within field of type 'struct 
InnerNestedPointerComplex' declared here}}
 
   float y;
@@ -167,8 +179,7 @@
 
 struct ArrayOfPtr // expected-note{{within field of type 'ArrayOfPtr' declared 
here}}
 {
-  float *arr[3]; // expected-note{{field of illegal type '__private float 
*[3]' declared here}}
-

[PATCH] D91531: [RFC][OpenCL] Provide mechanisms for defining extension macros

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added a comment.

In D91531#2412532 , @azabaznov wrote:

>> So if I understand it well you are suggesting to perform a check for every 
>> parsed macro definition that would identify whether the macro has a name 
>> matching what is provided in -cl-ext=? Then if the name matches it would 
>> check whether the macro should be defined or not?
>
> No, sorry for confusing. There is no need to check every parsed macro. For 
> example an internal pragma can be used for these purposes (`#pragma OPENCL 
> EXTENSION all : undef` in example below). So imagine if we can differentiate 
> header-only extensions and features and all of them are defined in header. 
> After the list of definitions the special pragma is used and is a processed 
> in a way that it inserts undef for each macro definition which relates to 
> header-only extension/feature and was turned off with the option 
> (`-cl-ext=-cl_khr_depth_images`):
>
>   #define cl_khr_depth_images 1
>   #define cl_khr_fp64 1
>   #define cl_khr_mipmap_image  1
>   ...
>   #pragma OPENCL EXTENSION all : undef
>
> However, this might be hard to maintain and I'm not sure yet that this is 
> even legally to do in current `Preprocessor` design, but this is at least 
> more scalable than adding `#if defined(__undef_` for each extension in the 
> end of the header.  Nevertheless, that's what I meant about preserving the 
> current interface.

Regarding the scalability, I think we can hide the fact that `__undef_ ` are being added using some mechanisms I have suggested earlier. As for 
interface, I agree that this will be different. But I don't know whether it is 
a new problem. For C and C++ it is similar - the functionality is being 
extended via libraries where possible without modifying the compiler. Adding 
everything in the compiler is just not going to work. It is just that we ended 
up doing this more than we should have. Do you see a big value in hooking the 
extensions to `-cl-ext`? then we could think of some slight modification in its 
current behavior but I am worried that this might introduce a place for nasty 
to debug issues.

Regarding the idea of a new pragma I think this is slightly violating normal 
flow. Such pragma only provides partial functionality i.e. it is useless 
without `-cl-ext`. The flag is a frontend only so there is no way to set it for 
the regular users. Also this is something that we should hide from the users 
really. But there are no good mechanisms to do so. We could, of course, think 
of some ways to refine the pragma and for that, it is probably better to submit 
a separate RFC with your initial ideas. But I feel this might bring more 
problems than it would solve. Also in the past, we have reinvented the wheel 
several times in OpenCL and the community is just not big enough to maintain 
everything so if we can reuse functionality from C or C++ we should just do it.

> Also, I didn't quite get how the proposing hook will allow users to declare 
> own extensions outside the codebase. Are you expecting them to use existing 
> `-cl-ext` or `-Dcl_khr_depth_images`? In the later case they won't able to 
> use `#pragma OPENCL EXTESNION cl_khr_depth_images : enable` (while the 
> specification does not describe for which extensions pragma is exactly needed 
> or not, but still)

Vendor extensions that genuinely need the fronted modifications can be added to 
`OpenCLExtensions.def`, as I am not suggesting to deprecate them. However if 
extensions can be implemented as libraries then the macro can be added using 
either:

- Internal header
- Command line flag `-D`
- Target hooks `TargetInfo::getOpenCLExtensionDefines`

I would recommend using either 1 or 2. It is easier to dial with OpenCL C code 
than dialing with C++ code that adds OpenCL C code.

Regarding the pragma - that should be added only if it is needed and I think 
most of the current extensions don't need it. In the future, we should probably 
change the definition of extensions in `OpenCLExtensions.def` to allow 
specifying whether pragma is to be added or not. So I think we should just stop 
adding the pragma everywhere for no reason.

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

https://reviews.llvm.org/D91531

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

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Venkataramanan Kumar via Phabricator via cfe-commits

venkataramanan.kumar.llvm added a comment.

In D88712#2413688 , @spatel wrote:

> In D88712#2412874 , 
> @venkataramanan.kumar.llvm wrote:
>
>> In D88712#2412366 , @MaskRay wrote:
>>
>>> In D88712#2411841 , 
>>> @venkataramanan.kumar.llvm wrote:
>>>
 
>>>
>>> For your example:
>>>
>>>   extern double log (double) asm ("" "log_finite") __attribute__ 
>>> ((nothrow));
>>>   
>>>   double mylog (double d) { return log(d); }
>>>
>>> The intention is to emit `log_finite`, no matter the `-ffast-math`. Both 
>>> GCC and Clang (after this patch) emit `jmp log_finite`.
>>>
>>> The previous behavior (according to your comment, with an older glibc) was 
>>> undesired. So I think the right suggestion is to upgrade glibc.
>>
>> Then there optimizations like vectorization, inst combine which works on the 
>> LLVM intrinsics.  But they will be not happening now  with  log_finite calls.
>
> I'm not sure about the expected semantics/lowering for the finite calls, but 
> can you add something under LibCallSimplifier::optimizeFloatingPointLibCall() 
> that would turn it into the LLVM log intrinsic with appropriate FMF? Codegen 
> would need to be responsible for converting it back to finite call(s) if 
> those are available?

Hi Sanjay I thought codegen no longer lowers them back to finite calls 
https://reviews.llvm.org/rGcd0926d087a85c5ee1222ca80980b4440214a822


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Sergey Dmitriev via Phabricator via cfe-commits

sdmitriev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:411
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };

ABataev wrote:
> sdmitriev wrote:
> > ABataev wrote:
> > > What about `llvm::iplist`?
> > It can be changed to iplist as well, but why do you think it is a better 
> > option here?
> Just better to use LLVM API if possible.
Well, I do not think it will bring any benefits here compared to the 
forward_list. iplist is an intrusive list, so it requires an element to provide 
access to prev/next elements in the list, and because of that element type 
should be derived from ilist_node. So, switching to iplist would require adding 
one more class for the list element type. I agree that it is not difficult to 
do, but I just do not see why it needs to be done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

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

[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

2020-11-24 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:186
 LANGOPT(PPCIEEELongDouble, 1, 0, "use IEEE 754 quadruple-precision 
for long double")
+LANGOPT(EnableAIXExtendedAltivecABI, 1, 0, "__EXTABI__  predefined 
macro")
 COMPATIBLE_VALUE_LANGOPT(PICLevel, 2, 0, "__PIC__ level")

minor nit: remove extra spaces in front of `1`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1445
+  Args.getLastArg(OPT_mabi_EQ_vec_default, OPT_mabi_EQ_vec_extabi)) {
+if (!T.isOSAIX() || !T.isOSBinFormatXCOFF())
+  Diags.Report(diag::err_drv_unsupported_opt_for_target)

I am wondering why do we add `!T.isOSBinFormatXCOFF()` back?



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1449
+
+if (!Args.hasArg(OPT_maltivec))
+  Diags.Report(diag::err_aix_altivec);

Just a record here, as we discussed offline, we don't need to emit this error 
in frontend.



Comment at: clang/test/CodeGen/altivec.c:7
+ 
+// RUN: %clang -S -emit-llvm -maltivec -mabi=vec-extabi -target 
powerpc-unknown-aix %s -o - | FileCheck %s
+// RUN: not %clang -S -emit-llvm -mabi=vec-default -target powerpc-unknown-aix 
%s 2>&1  | FileCheck  %s --check-prefix=AIX-ATVER

When user specify `-maltivec / -target-feature +altivec`  without using any abi 
option,  the compiler will assume default altivec abi. In this situation, since 
default abi hasn’t been implemented, we should emit an error. So can we also 
add testcases for :

```
// RUN: not %clang -S -emit-llvm -maltivec -target powerpc-unknown-aix %s 2>&1 
| FileCheck %s --check-prefix=AIX-ERROR
and
// RUN: not %clang_cc1 -target-feature +altivec -triple powerpc-unknown-aix 
-emit-llvm %s 2>&1 | FileCheck %s --check-prefix=AIX-ERROR
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Alexey Bataev via Phabricator via cfe-commits

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

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

[clang] 5ce85e6 - Fix driver test from e16c0a9a689719

2020-11-24 Thread Nico Weber via cfe-commits


Author: Nico Weber
Date: 2020-11-24T10:51:08-05:00
New Revision: 5ce85e66358a69e786093756c77fae2e140947c1

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

LOG: Fix driver test from e16c0a9a689719

The test failed silently if lld wasn't built alongside clang.
But the test uses -###, so the "invalid linker name in -fuse-ld=lld"
diag didn't make clang fail, and something else happened to match
"-demangle", so the test passed.

To fix, pass -B to a directory with two empty +x files (which works
on non-Windows), and look for `"-demangle"` instead of just `-demangle`.
Also force linker_version to 0 and pass a darwin triple.

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

Added: 
clang/test/Driver/Inputs/lld/ld64.lld
clang/test/Driver/Inputs/lld/ld64.lld.darwinnew

Modified: 
clang/test/Driver/darwin-ld-demangle-lld.c

Removed: 




diff  --git a/clang/test/Driver/Inputs/lld/ld64.lld 
b/clang/test/Driver/Inputs/lld/ld64.lld
new file mode 100755
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/Inputs/lld/ld64.lld.darwinnew 
b/clang/test/Driver/Inputs/lld/ld64.lld.darwinnew
new file mode 100755
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/darwin-ld-demangle-lld.c 
b/clang/test/Driver/darwin-ld-demangle-lld.c
index dfa48353e5a7..84facc8d1539 100644
--- a/clang/test/Driver/darwin-ld-demangle-lld.c
+++ b/clang/test/Driver/darwin-ld-demangle-lld.c
@@ -1,6 +1,12 @@
 // With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+// REQUIRES: shell
 
-// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
 // FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
-// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
-// CHECK: -demangle
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld.darwinnew -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: "-demangle"



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

[PATCH] D92028: Fix driver test from e16c0a9a689719

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ce85e66358a: Fix driver test from e16c0a9a689719 (authored 
by thakis).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D92028?vs=307335&id=307361#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92028

Files:
  clang/test/Driver/Inputs/lld/ld64.lld
  clang/test/Driver/Inputs/lld/ld64.lld.darwinnew
  clang/test/Driver/darwin-ld-demangle-lld.c


Index: clang/test/Driver/darwin-ld-demangle-lld.c
===
--- clang/test/Driver/darwin-ld-demangle-lld.c
+++ clang/test/Driver/darwin-ld-demangle-lld.c
@@ -1,6 +1,12 @@
 // With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+// REQUIRES: shell
 
-// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
 // FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
-// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
-// CHECK: -demangle
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld.darwinnew -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: "-demangle"


Index: clang/test/Driver/darwin-ld-demangle-lld.c
===
--- clang/test/Driver/darwin-ld-demangle-lld.c
+++ clang/test/Driver/darwin-ld-demangle-lld.c
@@ -1,6 +1,12 @@
 // With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+// REQUIRES: shell
 
-// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
 // FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
-// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
-// CHECK: -demangle
+// RUN: %clang --target=x86_64-apple-darwin -### \
+// RUN:   -fuse-ld=lld.darwinnew -B%S/Inputs/lld -mlinker-version=0 %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK: "-demangle"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92033: [OpenCL] Move kernel arg type tests into one file

2020-11-24 Thread Anastasia Stulova via Phabricator via cfe-commits

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

LGTM! Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92033

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-11-24 Thread Douglas Chen via Phabricator via cfe-commits

dougpuob marked 5 inline comments as done.
dougpuob added a comment.

Hi @aaron.ballman and @Eugene.Zelenko, thank you for your suggestions. I will 
improve them and upload my diff later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[clang-tools-extra] 9e83d0b - [clangd] Mention when CXXThis is implicit in exposed AST.

2020-11-24 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2020-11-24T16:57:56+01:00
New Revision: 9e83d0bcdfe86fd13f2817c9f40c5ca0b08f1443

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

LOG: [clangd] Mention when CXXThis is implicit in exposed AST.

Seeing an implicit this in the AST is pretty confusing I think.
While here, also mention when `this` is const.

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

Added: 


Modified: 
clang-tools-extra/clangd/DumpAST.cpp
clang-tools-extra/clangd/unittests/DumpASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/DumpAST.cpp 
b/clang-tools-extra/clangd/DumpAST.cpp
index 9ea17e876de7..12698b42ef3e 100644
--- a/clang-tools-extra/clangd/DumpAST.cpp
+++ b/clang-tools-extra/clangd/DumpAST.cpp
@@ -234,6 +234,14 @@ class DumpVisitor : public 
RecursiveASTVisitor {
   return UnaryOperator::getOpcodeStr(UO->getOpcode()).str();
 if (const auto *CCO = dyn_cast(S))
   return CCO->getConstructor()->getNameAsString();
+if (const auto *CTE = dyn_cast(S)) {
+  bool Const = CTE->getType()->getPointeeType().isLocalConstQualified();
+  if (CTE->isImplicit())
+return Const ? "const, implicit" : "implicit";
+  if (Const)
+return "const";
+  return "";
+}
 if (isa(S) || isa(S) ||
 isa(S) || isa(S) ||
 isa(S) || isa(S))

diff  --git a/clang-tools-extra/clangd/unittests/DumpASTTests.cpp 
b/clang-tools-extra/clangd/unittests/DumpASTTests.cpp
index 4b524820cb4c..23c673284f66 100644
--- a/clang-tools-extra/clangd/unittests/DumpASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DumpASTTests.cpp
@@ -76,29 +76,32 @@ declaration: Namespace - root
   type: Record - S
   )"},
   {R"cpp(
-template  int root() {
-  (void)root();
+namespace root {
+template  int tmpl() {
+  (void)tmpl();
   return T::value;
+}
 }
   )cpp",
R"(
-declaration: FunctionTemplate - root
-  declaration: TemplateTypeParm - T
-  declaration: Function - root
-type: FunctionProto
-  type: Builtin - int
-statement: Compound
-  expression: CStyleCast - ToVoid
-type: Builtin - void
-expression: Call
-  expression: ImplicitCast - FunctionToPointerDecay
-expression: DeclRef - root
-  template argument: Type
-type: Builtin - unsigned int
-  statement: Return
-expression: DependentScopeDeclRef - value
-  specifier: TypeSpec
-type: TemplateTypeParm - T
+declaration: Namespace - root
+  declaration: FunctionTemplate - tmpl
+declaration: TemplateTypeParm - T
+declaration: Function - tmpl
+  type: FunctionProto
+type: Builtin - int
+  statement: Compound
+expression: CStyleCast - ToVoid
+  type: Builtin - void
+  expression: Call
+expression: ImplicitCast - FunctionToPointerDecay
+  expression: DeclRef - tmpl
+template argument: Type
+  type: Builtin - unsigned int
+statement: Return
+  expression: DependentScopeDeclRef - value
+specifier: TypeSpec
+  type: TemplateTypeParm - T
   )"},
   {R"cpp(
 struct Foo { char operator+(int); };
@@ -116,10 +119,28 @@ declaration: Var - root
   type: Record - Foo
   expression: IntegerLiteral - 42
   )"},
+  {R"cpp(
+struct Bar {
+  int x;
+  int root() const {
+return x;
+  }
+};
+  )cpp",
+   R"(
+declaration: CXXMethod - root
+  type: FunctionProto
+type: Builtin - int
+  statement: Compound
+statement: Return
+  expression: ImplicitCast - LValueToRValue
+expression: Member - x
+  expression: CXXThis - const, implicit
+  )"},
   };
   for (const auto &Case : Cases) {
 ParsedAST AST = TestTU::withCode(Case.first).build();
-auto Node = dumpAST(DynTypedNode::create(findDecl(AST, "root")),
+auto Node = dumpAST(DynTypedNode::create(findUnqualifiedDecl(AST, "root")),
 AST.getTokens(), AST.getASTContext());
 EXPECT_EQ(llvm::StringRef(Case.second).trim(),
   llvm::StringRef(llvm::to_string(Node)).trim());



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

[PATCH] D91868: [clangd] Mention when CXXThis is implicit in exposed AST.

2020-11-24 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG9e83d0bcdfe8: [clangd] Mention when CXXThis is implicit in 
exposed AST. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D91868?vs=306681&id=307363#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91868

Files:
  clang-tools-extra/clangd/DumpAST.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp

Index: clang-tools-extra/clangd/unittests/DumpASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/DumpASTTests.cpp
+++ clang-tools-extra/clangd/unittests/DumpASTTests.cpp
@@ -76,29 +76,32 @@
   type: Record - S
   )"},
   {R"cpp(
-template  int root() {
-  (void)root();
+namespace root {
+template  int tmpl() {
+  (void)tmpl();
   return T::value;
+}
 }
   )cpp",
R"(
-declaration: FunctionTemplate - root
-  declaration: TemplateTypeParm - T
-  declaration: Function - root
-type: FunctionProto
-  type: Builtin - int
-statement: Compound
-  expression: CStyleCast - ToVoid
-type: Builtin - void
-expression: Call
-  expression: ImplicitCast - FunctionToPointerDecay
-expression: DeclRef - root
-  template argument: Type
-type: Builtin - unsigned int
-  statement: Return
-expression: DependentScopeDeclRef - value
-  specifier: TypeSpec
-type: TemplateTypeParm - T
+declaration: Namespace - root
+  declaration: FunctionTemplate - tmpl
+declaration: TemplateTypeParm - T
+declaration: Function - tmpl
+  type: FunctionProto
+type: Builtin - int
+  statement: Compound
+expression: CStyleCast - ToVoid
+  type: Builtin - void
+  expression: Call
+expression: ImplicitCast - FunctionToPointerDecay
+  expression: DeclRef - tmpl
+template argument: Type
+  type: Builtin - unsigned int
+statement: Return
+  expression: DependentScopeDeclRef - value
+specifier: TypeSpec
+  type: TemplateTypeParm - T
   )"},
   {R"cpp(
 struct Foo { char operator+(int); };
@@ -116,10 +119,28 @@
   type: Record - Foo
   expression: IntegerLiteral - 42
   )"},
+  {R"cpp(
+struct Bar {
+  int x;
+  int root() const {
+return x;
+  }
+};
+  )cpp",
+   R"(
+declaration: CXXMethod - root
+  type: FunctionProto
+type: Builtin - int
+  statement: Compound
+statement: Return
+  expression: ImplicitCast - LValueToRValue
+expression: Member - x
+  expression: CXXThis - const, implicit
+  )"},
   };
   for (const auto &Case : Cases) {
 ParsedAST AST = TestTU::withCode(Case.first).build();
-auto Node = dumpAST(DynTypedNode::create(findDecl(AST, "root")),
+auto Node = dumpAST(DynTypedNode::create(findUnqualifiedDecl(AST, "root")),
 AST.getTokens(), AST.getASTContext());
 EXPECT_EQ(llvm::StringRef(Case.second).trim(),
   llvm::StringRef(llvm::to_string(Node)).trim());
Index: clang-tools-extra/clangd/DumpAST.cpp
===
--- clang-tools-extra/clangd/DumpAST.cpp
+++ clang-tools-extra/clangd/DumpAST.cpp
@@ -234,6 +234,14 @@
   return UnaryOperator::getOpcodeStr(UO->getOpcode()).str();
 if (const auto *CCO = dyn_cast(S))
   return CCO->getConstructor()->getNameAsString();
+if (const auto *CTE = dyn_cast(S)) {
+  bool Const = CTE->getType()->getPointeeType().isLocalConstQualified();
+  if (CTE->isImplicit())
+return Const ? "const, implicit" : "implicit";
+  if (Const)
+return "const";
+  return "";
+}
 if (isa(S) || isa(S) ||
 isa(S) || isa(S) ||
 isa(S) || isa(S))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92037: clang: Pass -platform-version to new MachO LLD

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

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

New MachO LLD doesn't implement the old -macos_version_min (etc)
flags, but it understands the modern platform_version flag.
So make the clang driver pass that when using new MachO LLD.

Also, while here, don't pass -lto_library to LLD, since it
links in LTO libraries statically (which it can because it's
versioned alongside clang).


https://reviews.llvm.org/D92037

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-ld-platform-version-ios.c
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-ld-platform-version-tvos.c
  clang/test/Driver/darwin-ld-platform-version-watchos.c

Index: clang/test/Driver/darwin-ld-platform-version-watchos.c
===
--- clang/test/Driver/darwin-ld-platform-version-watchos.c
+++ clang/test/Driver/darwin-ld-platform-version-watchos.c
@@ -1,12 +1,24 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=400 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 -fuse-ld=lld.darwinnew \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o -B%S/Inputs/lld 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-NEW %s
-// RUN: %clang -target x86_64-apple-watchos6-simulator -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
+// RUN: %clang -target x86_64-apple-watchos6-simulator \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // LINKER-OLD: "-watchos_version_min" "5.2.0"
Index: clang/test/Driver/darwin-ld-platform-version-tvos.c
===
--- clang/test/Driver/darwin-ld-platform-version-tvos.c
+++ clang/test/Driver/darwin-ld-platform-version-tvos.c
@@ -1,12 +1,24 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 -fuse-ld=lld.darwinnew \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o -B%S/Inputs/lld 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-NEW %s
-// RUN: %clang -target x86_64-apple-tvos13-simulator -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
+// RUN: %clang -target x86_64-apple-tvos13-simulator \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // LINKER-OLD: "-tvos_version_min" "12.3.0"
Index: clang/test/Driver/darwin-ld-platform-version-macos.c
===
--- clang/test/Driver/darwin-ld-platform-version-macos.c
+++ clang/test/Driver/darwin-ld-platform-version-macos.c
@@ -1,20 +1,42 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %S/Inputs/MacOSX10.14.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target x8

[PATCH] D92037: clang: Pass -platform-version to new MachO LLD

2020-11-24 Thread Hans Wennborg via Phabricator via cfe-commits

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



Comment at: clang/include/clang/Driver/ToolChain.h:334
   /// LLD's supported flags, error output, etc.
-  std::string GetLinkerPath(bool *LinkerIsLLD = nullptr) const;
+  /// If LinkerIsLLDDarwinNew is non-nullptr, it's set if it's the in-progress
+  /// new version in lld/MachO.

ultra nit: the "in-progress" part doesn't really add much to the comment and 
might be obsolete soon, so i would probably have left that out. Also, strictly 
speaking, it's not immediately clear what the second "it" refers to here.


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

https://reviews.llvm.org/D92037

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

[PATCH] D92010: [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Sergey Dmitriev 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 rG1b0ca81a6c35: [clang-offload-bundler] use std::forward_list 
for storing temp file names [NFC] (authored by sdmitriev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92010

Files:
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -394,7 +395,7 @@
 if (std::error_code EC =
 sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
   return createFileError(File, EC);
-Files.push_back(File);
+Files.push_front(File);
 
 if (Contents) {
   std::error_code EC;
@@ -403,11 +404,11 @@
 return createFileError(File, EC);
   OS.write(Contents->data(), Contents->size());
 }
-return Files.back();
+return Files.front();
   }
 
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };
 
 } // end anonymous namespace


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -394,7 +395,7 @@
 if (std::error_code EC =
 sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
   return createFileError(File, EC);
-Files.push_back(File);
+Files.push_front(File);
 
 if (Contents) {
   std::error_code EC;
@@ -403,11 +404,11 @@
 return createFileError(File, EC);
   OS.write(Contents->data(), Contents->size());
 }
-return Files.back();
+return Files.front();
   }
 
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };
 
 } // end anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1b0ca81 - [clang-offload-bundler] use std::forward_list for storing temp file names [NFC]

2020-11-24 Thread Sergey Dmitriev via cfe-commits


Author: Sergey Dmitriev
Date: 2020-11-24T08:07:31-08:00
New Revision: 1b0ca81a6c358d2d52d4f84b7f42e620ead1ed26

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

LOG: [clang-offload-bundler] use std::forward_list for storing temp file names 
[NFC]

Use a different container that preserves existing elements on modification
for storing temporary file names. Current container can make StringRefs
returned earlier invalid on reallocation.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Removed: 




diff  --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp 
b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
index 44c46a89a859..a1b2fecb4a80 100644
--- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -394,7 +395,7 @@ class TempFileHandlerRAII {
 if (std::error_code EC =
 sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
   return createFileError(File, EC);
-Files.push_back(File);
+Files.push_front(File);
 
 if (Contents) {
   std::error_code EC;
@@ -403,11 +404,11 @@ class TempFileHandlerRAII {
 return createFileError(File, EC);
   OS.write(Contents->data(), Contents->size());
 }
-return Files.back();
+return Files.front();
   }
 
 private:
-  SmallVector, 4u> Files;
+  std::forward_list> Files;
 };
 
 } // end anonymous namespace



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

[clang] 9a8386d - clang: Pass -platform-version to new MachO LLD

2020-11-24 Thread Nico Weber via cfe-commits


Author: Nico Weber
Date: 2020-11-24T11:16:03-05:00
New Revision: 9a8386dba889b038c23bfc89dd0ff3cf55bbf86a

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

LOG: clang: Pass -platform-version to new MachO LLD

New MachO LLD doesn't implement the old -macos_version_min (etc)
flags, but it understands the modern platform_version flag.
So make the clang driver pass that when using new MachO LLD.

Also, while here, don't pass -lto_library to LLD, since it
links in LTO libraries statically (which it can because it's
versioned alongside clang).

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

Added: 


Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/test/Driver/darwin-ld-platform-version-ios.c
clang/test/Driver/darwin-ld-platform-version-macos.c
clang/test/Driver/darwin-ld-platform-version-tvos.c
clang/test/Driver/darwin-ld-platform-version-watchos.c

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index 58df4d04aea6..7aa8ba7b1da9 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -331,7 +331,10 @@ class ToolChain {
   /// is LLD. If it's set, it can be assumed that the linker is LLD built
   /// at the same revision as clang, and clang can make assumptions about
   /// LLD's supported flags, error output, etc.
-  std::string GetLinkerPath(bool *LinkerIsLLD = nullptr) const;
+  /// If LinkerIsLLDDarwinNew is non-nullptr, it's set if the linker is
+  /// the new version in lld/MachO.
+  std::string GetLinkerPath(bool *LinkerIsLLD = nullptr,
+bool *LinkerIsLLDDarwinNew = nullptr) const;
 
   /// Returns the linker path for emitting a static library.
   std::string GetStaticLibToolPath() const;

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index ae1838aeb9db..0330afdcec48 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -548,9 +548,12 @@ std::string ToolChain::GetProgramPath(const char *Name) 
const {
   return D.GetProgramPath(Name, *this);
 }
 
-std::string ToolChain::GetLinkerPath(bool *LinkerIsLLD) const {
+std::string ToolChain::GetLinkerPath(bool *LinkerIsLLD,
+ bool *LinkerIsLLDDarwinNew) const {
   if (LinkerIsLLD)
 *LinkerIsLLD = false;
+  if (LinkerIsLLDDarwinNew)
+*LinkerIsLLDDarwinNew = false;
 
   // Get -fuse-ld= first to prevent -Wunused-command-line-argument. -fuse-ld= 
is
   // considered as the linker flavor, e.g. "bfd", "gold", or "lld".
@@ -603,9 +606,11 @@ std::string ToolChain::GetLinkerPath(bool *LinkerIsLLD) 
const {
 
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
 if (llvm::sys::fs::can_execute(LinkerPath)) {
+  // FIXME: Remove lld.darwinnew here once it's the only MachO lld.
   if (LinkerIsLLD)
-// FIXME: Remove lld.darwinnew here once it's the only MachO lld.
 *LinkerIsLLD = UseLinker == "lld" || UseLinker == "lld.darwinnew";
+  if (LinkerIsLLDDarwinNew)
+*LinkerIsLLDDarwinNew = UseLinker == "lld.darwinnew";
   return LinkerPath;
 }
   }

diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index ddfab0e6ab7c..db3d57a48098 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -204,7 +204,8 @@ static bool shouldLinkerNotDedup(bool IsLinkerOnlyAction, 
const ArgList &Args) {
 void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
  ArgStringList &CmdArgs,
  const InputInfoList &Inputs,
- unsigned Version[5], bool LinkerIsLLD) const {
+ unsigned Version[5], bool LinkerIsLLD,
+ bool LinkerIsLLDDarwinNew) const {
   const Driver &D = getToolChain().getDriver();
   const toolchains::MachO &MachOTC = getMachOToolChain();
 
@@ -252,7 +253,9 @@ void darwin::Linker::AddLinkArgs(Compilation &C, const 
ArgList &Args,
   // Since this is passed unconditionally, ld64 will never look for 
libLTO.dylib
   // next to it. That's ok since ld64 using a libLTO.dylib not matching the
   // clang version won't work anyways.
-  if (Version[0] >= 133) {
+  // lld is built at the same revision as clang and statically links in
+  // LLVM libraries, so it doesn't need libLTO.dylib.
+  if (Version[0] >= 133 && !LinkerIsLLD) {
 // Search for libLTO in /../lib/libLTO.dylib
 StringRef P = llvm::sys::path::parent_path(D.Dir);
 SmallString<128> LibLTOPath(P);
@@ -3

[PATCH] D92037: clang: Pass -platform-version to new MachO LLD

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a8386dba889: clang: Pass -platform-version to new MachO LLD 
(authored by thakis).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D92037?vs=307365&id=307369#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92037

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/darwin-ld-platform-version-ios.c
  clang/test/Driver/darwin-ld-platform-version-macos.c
  clang/test/Driver/darwin-ld-platform-version-tvos.c
  clang/test/Driver/darwin-ld-platform-version-watchos.c

Index: clang/test/Driver/darwin-ld-platform-version-watchos.c
===
--- clang/test/Driver/darwin-ld-platform-version-watchos.c
+++ clang/test/Driver/darwin-ld-platform-version-watchos.c
@@ -1,12 +1,24 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=400 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64_32-apple-watchos5.2 -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 -fuse-ld=lld.darwinnew \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o -B%S/Inputs/lld 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-NEW %s
-// RUN: %clang -target x86_64-apple-watchos6-simulator -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64_32-apple-watchos5.2 \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
+// RUN: %clang -target x86_64-apple-watchos6-simulator \
+// RUN:   -isysroot %S/Inputs/WatchOS6.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // LINKER-OLD: "-watchos_version_min" "5.2.0"
Index: clang/test/Driver/darwin-ld-platform-version-tvos.c
===
--- clang/test/Driver/darwin-ld-platform-version-tvos.c
+++ clang/test/Driver/darwin-ld-platform-version-tvos.c
@@ -1,12 +1,24 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=400 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-OLD %s
-// RUN: %clang -target arm64-apple-tvos12.3 -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 -fuse-ld=lld.darwinnew \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=0 \
+// RUN:   -### %t.o -B%S/Inputs/lld 2>&1 \
 // RUN:   | FileCheck --check-prefix=LINKER-NEW %s
-// RUN: %clang -target x86_64-apple-tvos13-simulator -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 -### %t.o 2>&1 \
+// RUN: %clang -target arm64-apple-tvos12.3 \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
+// RUN:   | FileCheck --check-prefix=LINKER-NEW %s
+// RUN: %clang -target x86_64-apple-tvos13-simulator \
+// RUN:   -isysroot %S/Inputs/iPhoneOS13.0.sdk -mlinker-version=520 \
+// RUN:   -### %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=SIMUL %s
 
 // LINKER-OLD: "-tvos_version_min" "12.3.0"
Index: clang/test/Driver/darwin-ld-platform-version-macos.c
===
--- clang/test/Driver/darwin-ld-platform-version-macos.c
+++ clang/test/Driver/darwin-ld-platform-version-macos.c
@@ -1,20 +1,42 @@
 // RUN: touch %t.o
 
-// RUN: %clang -target x86_64-apple-macos10.13 -isysroot %S/Inputs/MacOSX10.14.sdk -mlinker-version=0 -### %t.o 2>&1 \
+// RUN: %clang -target x86_64-apple-macos10.13 \
+// RUN:   -isysroo

[PATCH] D92037: clang: Pass -platform-version to new MachO LLD

2020-11-24 Thread Nico Weber via Phabricator via cfe-commits

thakis marked an inline comment as done.
thakis added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92037

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

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added a comment.

Adding another reviewer - @wmi can you take a look? This is a straightforward 
compile time fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91676

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

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

2020-11-24 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko created this revision.
vsavchenko added reviewers: aaron.ballman, dexonsmith, rsmith, Bigcheese, 
rjmccall, NoQ, doug.gregor, ravikandhadai, dcoughlin.
Herald added subscribers: cfe-commits, Charusso, mgorny.
Herald added a project: clang.
vsavchenko requested review of this revision.

This commit introduces a new attribute `called_once`.
It can be applied to function-like parameters to signify that
this parameter should be called exactly once.  This concept
is particularly widespread in asynchronous programs.

Additionally, this commit introduce a new group of dataflow
analysis-based warnings to check this property.  It identifies
and reports the following situations:

- parameter is called twice
- parameter is never called
- parameter is not called on one of the paths

Current implementation can also automatically infer `called_once`
attribute for completion handler paramaters that should follow the
same principle by convention.  This behavior is OFF by default and
can be turned on by using `-Wcompletion-handler`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92039

Files:
  clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/CalledOnceCheck.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaObjC/attr-called-once.m
  clang/test/SemaObjC/warn-called-once.m

Index: clang/test/SemaObjC/warn-called-once.m
===
--- /dev/null
+++ clang/test/SemaObjC/warn-called-once.m
@@ -0,0 +1,887 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -fobjc-exceptions -Wcompletion-handler %s
+
+#define NULL (void *)0
+#define CALLED_ONCE __attribute__((called_once))
+#define NORETURN __attribute__((noreturn))
+
+@protocol NSObject
+@end
+@interface NSObject 
+- (id)copy;
+- (id)class;
+- autorelease;
+@end
+
+typedef unsigned int NSUInteger;
+typedef struct {
+} NSFastEnumerationState;
+
+@interface NSArray <__covariant NSFastEnumeration>
+- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
+@end
+@interface NSMutableArray : NSArray 
+- addObject:anObject;
+@end
+@class NSString, Protocol;
+extern void NSLog(NSString *format, ...);
+
+void escape(void (^callback)(void));
+void escape_void(void *);
+void indirect_call(void (^callback)(void) CALLED_ONCE);
+void indirect_conv(void (^completionHandler)(void));
+void filler(void);
+void exit(int) NORETURN;
+
+void double_call_one_block(void (^callback)(void) CALLED_ONCE) {
+  callback(); // expected-note{{previous call is here}}
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void double_call_one_block_parens(void (^callback)(void) CALLED_ONCE) {
+  (callback)(); // expected-note{{previous call is here}}
+  (callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void double_call_one_block_ptr(void (*callback)(void) CALLED_ONCE) {
+  callback(); // expected-note{{previous call is here}}
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void double_call_one_block_ptr_deref(void (*callback)(void) CALLED_ONCE) {
+  (*callback)(); // expected-note{{previous call is here}}
+  (*callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void multiple_call_one_block(void (^callback)(void) CALLED_ONCE) {
+  // We don't really need to repeat the same warning for the same parameter.
+  callback(); // no-warning
+  callback(); // no-warning
+  callback(); // no-warning
+  callback(); // expected-note{{previous call is here}}
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void double_call_branching_1(int cond, void (^callback)(void) CALLED_ONCE) {
+  if (cond) {
+callback(); // expected-note{{previous call is here}}
+  } else {
+cond += 42;
+  }
+  callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+}
+
+void double_call_branching_2(int cond, void (^callback)(void) CALLED_ONCE) {
+  callback(); // expected-note{{previous call is here}}
+
+  if (cond) {
+callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}}
+  } else {
+cond += 42;
+  }
+}
+
+void double_call_branching_3(int cond, void (^callback)(void) CALLED_ONCE) {
+  if (cond) {
+callback();
+  } else {
+callback();
+  }
+  // no-warning
+}
+
+void double_call_branching_4(int cond1, int cond2, void (^callback)(void) CALLED_ONCE) {
+  if (cond1) {
+cond2 = !cond2;
+  } else {
+callback(); // expected-note{{previous call is here}}
+  }
+
+  if (cond2) {
+ca

[PATCH] D91966: [clangd] AddUsing: Used spelled text instead of type name.

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:268
+   SpelledTokens->back());
+  llvm::StringRef NameRef = SpelledRange.text(SM);
+

adamcz wrote:
> kadircet wrote:
> > what about saving the full range here, and using it's `start` and `length - 
> > name.size()` as removal range in `apply`?
> > 
> > similarly we can also use the `range.text` as text to insert. that way we 
> > can inline `getNNSLAsString` as it won't be needed elsewhere anymore.
> We only have a range in this branch of the code. The other one (for 
> DeclRefExpr) is not using TokenBuffers, so no range right now.
> 
> We may change it in the future. Or would you prefer this changed now?
> We only have a range in this branch of the code. The other one (for 
> DeclRefExpr) is not using TokenBuffers, so no range right now.

Ah right, I've forgot that one. My main concern was the extra `getSpellingLoc` 
in `apply`.

> We may change it in the future. Or would you prefer this changed now?

No hurries.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:2814
+  uu u;
 })cpp"}};
   llvm::StringMap EditedFiles;

adamcz wrote:
> kadircet wrote:
> > Tests don't seem to be covering something like:
> > ```
> > namespace foo { namespace bar { struct X {}; } }
> > using namespace foo;
> > bar::X^ x;
> > ```
> So this has the unfortunate side-effect of triggering a variation of 
> https://github.com/clangd/clangd/issues/585
> 
> That's something I'm fixing in a later change (although this case is still 
> not handled). For now, I'm putting an extra namespace scope to make this test 
> correct, while still testing your example.
sorry for being confusing, I was actually suggesting putting:
```
namespace foo { namespace bar { struct X {}; } }
using namespace foo;
```
into the header (to prevent that bug from triggering), and only having the 
usage in the CC file. but this works too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91966

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

[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

2020-11-24 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:10333
+  /// Check if there is an active global `omp begin assumes` directive.
+  bool isInOpenMPAssumeScope() { return !OMPAssumeScoped.empty(); }
+

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > `const` member functions.
> > > It may have side-effects with template instantiations. What if we have 
> > > something like this:
> > > ```
> > > #pragma omp begin assumes ...
> > > template 
> > > struct S {
> > > ...
> > > }
> > > #pragma omp end assumes
> > > 
> > > void bar() {
> > > #pragma omp assumes ...
> > >   {
> > > S s;
> > > s.foo();
> > >   }
> > > }
> > > ```
> > > ?
> > > 
> > > `struct S` will be instantiated in the second assume region context 
> > > though I doubt this is the user intention.
> > I'm not sure what problem you expect here. Could you elaborate what you 
> > think should happen or should not?
> May it lead to the wrong compiler assumptions and result in the incorrect 
> codegen?
I'm still not following your example. What wrong assumptions and/or incorrect 
code generation do you expect? Could you provide an example with actual 
assumptions and then describe what the problem is please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980

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

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

2020-11-24 Thread Michael Spencer via Phabricator via cfe-commits

Bigcheese accepted this revision.
Bigcheese added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)&options::CC1Option) {
\
+const auto &Extracted = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) 
\

jansvoboda11 wrote:
> dexonsmith wrote:
> > dexonsmith wrote:
> > > jansvoboda11 wrote:
> > > > Bigcheese wrote:
> > > > > Will this ever have an issue with lifetime? I can see various values 
> > > > > for `EXTRACTOR` causing issues here. https://abseil.io/tips/107
> > > > > 
> > > > > 
> > > > > It would be good to at least document somewhere the restrictions on 
> > > > > `EXTRACTOR`.
> > > > Mentioned the reference lifetime extension in a comment near extractor 
> > > > definitions.
> > > It might be safer to refactor as:
> > > ```
> > > // Capture the extracted value as a lambda argument to avoid potential
> > > // lifetime extension issues.
> > > [&](const auto &Extracted) {
> > >   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> > > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> > > }(EXTRACTOR(this->KEYPATH));
> > > ```
> > > 
> > Might be even better to avoid the generic lambda:
> > ```
> > // Capture the extracted value as a lambda argument to avoid potential
> > // lifetime extension issues.
> > using ExtractedType =
> > std::remove_const_t > decltype(EXTRACTOR(this->KEYPATH))>>
> > [&](const ExtractedType &Extracted) {
> >   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> > }(EXTRACTOR(this->KEYPATH));
> > ```
> > (since generic vs. non-generic could affect compile-time of 
> > CompilerInvocation.cpp given how many instances there will be).
> Thanks for the suggestions @dexonsmith. I'm having trouble writing a test 
> case where the lambda workaround produces a different result than `const auto 
> &` variable.
> @Bigcheese, could you show a concrete example of an extractor that causes 
> issues so I can test it out?
I think I was confused about when this can happen. The `const auto &` will 
never cause a conversion that would prevent lifetime extension. The only place 
this would break is if the copy constructor of the type is rather broken, which 
isn't a reasonable case to support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

2020-11-24 Thread Michael Spencer via Phabricator via cfe-commits

Bigcheese added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)&options::CC1Option) {
\
+const auto &Extracted = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) 
\

Bigcheese wrote:
> jansvoboda11 wrote:
> > dexonsmith wrote:
> > > dexonsmith wrote:
> > > > jansvoboda11 wrote:
> > > > > Bigcheese wrote:
> > > > > > Will this ever have an issue with lifetime? I can see various 
> > > > > > values for `EXTRACTOR` causing issues here. 
> > > > > > https://abseil.io/tips/107
> > > > > > 
> > > > > > 
> > > > > > It would be good to at least document somewhere the restrictions on 
> > > > > > `EXTRACTOR`.
> > > > > Mentioned the reference lifetime extension in a comment near 
> > > > > extractor definitions.
> > > > It might be safer to refactor as:
> > > > ```
> > > > // Capture the extracted value as a lambda argument to avoid potential
> > > > // lifetime extension issues.
> > > > [&](const auto &Extracted) {
> > > >   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> > > > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> > > > }(EXTRACTOR(this->KEYPATH));
> > > > ```
> > > > 
> > > Might be even better to avoid the generic lambda:
> > > ```
> > > // Capture the extracted value as a lambda argument to avoid potential
> > > // lifetime extension issues.
> > > using ExtractedType =
> > > std::remove_const_t > > decltype(EXTRACTOR(this->KEYPATH))>>
> > > [&](const ExtractedType &Extracted) {
> > >   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> > > DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> > > }(EXTRACTOR(this->KEYPATH));
> > > ```
> > > (since generic vs. non-generic could affect compile-time of 
> > > CompilerInvocation.cpp given how many instances there will be).
> > Thanks for the suggestions @dexonsmith. I'm having trouble writing a test 
> > case where the lambda workaround produces a different result than `const 
> > auto &` variable.
> > @Bigcheese, could you show a concrete example of an extractor that causes 
> > issues so I can test it out?
> I think I was confused about when this can happen. The `const auto &` will 
> never cause a conversion that would prevent lifetime extension. The only 
> place this would break is if the copy constructor of the type is rather 
> broken, which isn't a reasonable case to support.
Now I remember what the issue was, it's if the `EXTRACTOR` itself creates an 
owning temporary and converts back to a reference type: 
https://godbolt.org/z/xsvh4f

This is kind of weird to do, the `EXTRACTOR` would need to take an owning type 
with an implicit conversion from the type of `KEYPATH`, and then return a 
reference type. I'm not sure how realistic that situation is, but it seems fine 
to defend against with Duncan's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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

[PATCH] D92041: Add hover info for `this` expr

2020-11-24 Thread xndcn via Phabricator via cfe-commits

xndcn created this revision.
xndcn added reviewers: sammccall, kadircet.
xndcn added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
xndcn requested review of this revision.

How about add hover information for `this` expr?
It seems useful to show related information about the class for `this` expr 
sometimes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92041

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2019,6 +2019,23 @@
 HI.NamespaceScope = "";
 HI.Definition = "@interface MYObject\n@end";
   }},
+  {
+  R"cpp(// this expr
+   namespace ns {
+ class Foo {
+   Foo* bar() {
+ return [[t^his]];
+   }
+ };
+   };
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "this";
+HI.Type = "ns::Foo *";
+HI.Kind = index::SymbolKind::Unknown;
+HI.NamespaceScope = "ns::";
+HI.Definition = "class Foo {}";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -606,6 +606,17 @@
   return HI;
 }
 
+/// Generate a \p Hover object given the \p this pointer.
+HoverInfo getHoverContents(const CXXThisExpr *CTE, const SymbolIndex *Index) {
+  const NamedDecl *D = CTE->getType()->getPointeeType()->getAsCXXRecordDecl();
+  HoverInfo HI = getHoverContents(D, Index);
+  HI.Name = "this";
+  // TODO: determine the symbol kind.
+  HI.Kind = index::SymbolKind::Unknown;
+  HI.Type = printType(CTE->getType(), D->getASTContext().getPrintingPolicy());
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -860,6 +871,8 @@
 if (!HI->Value)
   HI->Value = printExprValue(N, AST.getASTContext());
 maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy());
+  } else if (const CXXThisExpr *CTE = N->ASTNode.get()) {
+HI = getHoverContents(CTE, Index);
   } else if (const Expr *E = N->ASTNode.get()) {
 HI = getHoverContents(E, AST);
   }


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2019,6 +2019,23 @@
 HI.NamespaceScope = "";
 HI.Definition = "@interface MYObject\n@end";
   }},
+  {
+  R"cpp(// this expr
+   namespace ns {
+ class Foo {
+   Foo* bar() {
+ return [[t^his]];
+   }
+ };
+   };
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "this";
+HI.Type = "ns::Foo *";
+HI.Kind = index::SymbolKind::Unknown;
+HI.NamespaceScope = "ns::";
+HI.Definition = "class Foo {}";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -606,6 +606,17 @@
   return HI;
 }
 
+/// Generate a \p Hover object given the \p this pointer.
+HoverInfo getHoverContents(const CXXThisExpr *CTE, const SymbolIndex *Index) {
+  const NamedDecl *D = CTE->getType()->getPointeeType()->getAsCXXRecordDecl();
+  HoverInfo HI = getHoverContents(D, Index);
+  HI.Name = "this";
+  // TODO: determine the symbol kind.
+  HI.Kind = index::SymbolKind::Unknown;
+  HI.Type = printType(CTE->getType(), D->getASTContext().getPrintingPolicy());
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -860,6 +871,8 @@
 if (!HI->Value)
   HI->Value = printExprValue(N, AST.getASTContext());
 maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy());
+  } else if (const CXXThisExpr *CTE = N->ASTNode.get()) {
+HI = getHoverContents(CTE, Index);
   } else if (const Expr *E = N->ASTNode.get()) {
 HI = getHoverContents(E, AST);
   }
___
cfe-commits mailing list
cfe-commi

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits

spatel added a comment.

In D88712#2413877 , 
@venkataramanan.kumar.llvm wrote:

> In D88712#2413688 , @spatel wrote:
>
>> In D88712#2412874 , 
>> @venkataramanan.kumar.llvm wrote:
>>
>>> In D88712#2412366 , @MaskRay wrote:
>>>
 In D88712#2411841 , 
 @venkataramanan.kumar.llvm wrote:

> 

 For your example:

   extern double log (double) asm ("" "log_finite") __attribute__ 
 ((nothrow));
   
   double mylog (double d) { return log(d); }

 The intention is to emit `log_finite`, no matter the `-ffast-math`. Both 
 GCC and Clang (after this patch) emit `jmp log_finite`.

 The previous behavior (according to your comment, with an older glibc) was 
 undesired. So I think the right suggestion is to upgrade glibc.
>>>
>>> Then there optimizations like vectorization, inst combine which works on 
>>> the LLVM intrinsics.  But they will be not happening now  with  log_finite 
>>> calls.
>>
>> I'm not sure about the expected semantics/lowering for the finite calls, but 
>> can you add something under 
>> LibCallSimplifier::optimizeFloatingPointLibCall() that would turn it into 
>> the LLVM log intrinsic with appropriate FMF? Codegen would need to be 
>> responsible for converting it back to finite call(s) if those are available?
>
> Hi Sanjay I thought codegen no longer lowers them back to finite calls 
> https://reviews.llvm.org/rGcd0926d087a85c5ee1222ca80980b4440214a822

There was a comment in D74712  that we might 
be moving too fast. If you would like to revert/adjust that patch, raise a bug 
or post a patch to discuss? If the goal is to be able to vectorize the finite 
calls, then we will need some way to represent those. Alternatively, we could 
change something in the cost models to enable more unrolling, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

[PATCH] D83698: [clang][cli] Port Target option flags to new option parsing system

2020-11-24 Thread Michael Spencer via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83698

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

[clang] 44174b3 - [NFC][tests] Replace non-portable grep with FileCheck

2020-11-24 Thread Hubert Tong via cfe-commits


Author: Hubert Tong
Date: 2020-11-24T12:15:07-05:00
New Revision: 44174b3d518ed70482ff5df2879523a4e26f92cc

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

LOG: [NFC][tests] Replace non-portable grep with FileCheck

After commit 2482648a795afbe12774168bbbf70dc14c031267, a GNU grep option
is just passed unconditionally to `grep` in general. This patch fixes
the test for platforms where `grep` is not GNU grep.

Added: 


Modified: 
clang/test/CodeGen/thinlto_embed_bitcode.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto_embed_bitcode.ll 
b/clang/test/CodeGen/thinlto_embed_bitcode.ll
index 971d4005435d..590cadbfd418 100644
--- a/clang/test/CodeGen/thinlto_embed_bitcode.ll
+++ b/clang/test/CodeGen/thinlto_embed_bitcode.ll
@@ -18,7 +18,7 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t.o -x ir %t1.bc -c 
-fthinlto-index=%t.o.thinlto.bc -mllvm -lto-embed-bitcode=post-merge-pre-opt
 ; RUN: llvm-readelf -S %t.o | FileCheck %s 
--check-prefixes=CHECK-ELF,CHECK-ELF-CMD
 ; RUN: llvm-objcopy --dump-section=.llvmcmd=%t-embedded.cmd %t.o /dev/null
-; RUN: grep --text x86_64-unknown-linux-gnu %t-embedded.cmd | count 1
+; RUN: FileCheck %s --check-prefixes=CHECK-EMBEDDED-CMD <%t-embedded.cmd
 ; RUN: llvm-objcopy --dump-section=.llvmbc=%t-embedded.bc %t.o /dev/null
 ; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s 
--check-prefixes=CHECK,CHECK-NOOPT
 ; We should only need the index and the post-thinlto merged module to generate 
@@ -43,3 +43,6 @@
 ; CHECK-NOOPT-NEXT: call void @bar()
 ; CHECK-NOOPT: define available_externally void @bar()
 ; CHECK-NOOPT-NEXT: ret void
+
+; CHECK-EMBEDDED-CMD: x86_64-unknown-linux-gnu{{.*$}}
+; CHECK-EMBEDDED-CMD-NOT: x86_64-unknown-linux-gnu



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

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Wei Mi via Phabricator via cfe-commits

wmi added inline comments.



Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302
+  // has no effect on the min visibility computed below by the recursive 
caller.
+  if (!Visited.insert(RD).second)
+return llvm::GlobalObject::VCallVisibilityTranslationUnit;
+

If a CXXRecordDecl is visited twice, the visibility returned in the second 
visit could be larger than necessary. Will it change the final result? If it 
will, can we cache the visibility result got in the first visit instead of 
returning the max value? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91676

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

[PATCH] D91812: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

2020-11-24 Thread Teresa Johnson 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 rG6e4c1cf29388: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip 
in ThinLTO backends (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91812

Files:
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp


Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1030,6 +1030,10 @@
 
 void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
  Constant *TheFn, bool &IsExported) {
+  // Don't devirtualize function if we're told to skip it
+  // in -wholeprogramdevirt-skip.
+  if (FunctionsToSkip.match(TheFn->stripPointerCasts()->getName()))
+return;
   auto Apply = [&](CallSiteInfo &CSInfo) {
 for (auto &&VCallSite : CSInfo.CallSites) {
   if (RemarksEnabled)
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -40,8 +40,16 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: 
allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: 
branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: 
"_ZN1A1nEi") ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
-; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 
-Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s 
--check-prefixes=CHECK-IR --check-prefixes=REMARKS
+
+; Check that the devirtualization is suppressed via -wholeprogramdevirt-skip
+; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu -mllvm 
-wholeprogramdevirt-skip=_ZN1A1nEi \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 
-Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s 
--check-prefixes=SKIP-IR --check-prefixes=SKIP-REMARKS
+
+; REMARKS: single-impl: devirtualized a call to _ZN1A1nEi
+; SKIP-REMARKS-NOT: single-impl
 
 ; Check that backend does not fail generating native code.
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
@@ -78,6 +86,7 @@
 
   ; Check that the call was devirtualized.
   ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; SKIP-IR-NOT: %call = tail call i32 @_ZN1A1nEi
   %call = tail call i32 %4(%struct.A* nonnull %obj, i32 %a)
   %vtable16 = load i8*, i8** %0
   %5 = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable16, i32 0, 
metadata !"_ZTS1A")


Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1030,6 +1030,10 @@
 
 void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
  Constant *TheFn, bool &IsExported) {
+  // Don't devirtualize function if we're told to skip it
+  // in -wholeprogramdevirt-skip.
+  if (FunctionsToSkip.match(TheFn->stripPointerCasts()->getName()))
+return;
   auto Apply = [&](CallSiteInfo &CSInfo) {
 for (auto &&VCallSite : CSInfo.CallSites) {
   if (RemarksEnabled)
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -40,8 +40,16 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi") ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
-; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 -Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s --check-prefixes=CHECK-IR --check-prefixes=REMARKS
+
+; Check that the devirtualization is suppressed via -wholeprogramdevirt-skip
+; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu -mllvm -wholeprogramdevirt-skip=_ZN1A1nEi \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 -Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s --check-prefixes=SKIP-IR --check

[PATCH] D88172: [clangd] Extract common file-caching logic from ConfigProvider.

2020-11-24 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

@kbobyrev ping... I think we do actually want to land this, for use with 
`.clang-tidy` files after D91029 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88172

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

[clang] 6e4c1cf - [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

2020-11-24 Thread Teresa Johnson via cfe-commits


Author: Teresa Johnson
Date: 2020-11-24T09:35:07-08:00
New Revision: 6e4c1cf2938842ceefc2712f0007843369dd16ca

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

LOG: [ThinLTO/WPD] Enable -wholeprogramdevirt-skip in ThinLTO backends

Previously this option could be used to skip devirtualizations of the
given functions in regular LTO and in the ThinLTO indexing step. This
change allows them to be skipped in the backend as well, which is useful
when debugging WPD in a distributed ThinLTO backend.

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

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll 
b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
index 5c753ba6f93c..0a330a53e948 100644
--- a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -40,8 +40,16 @@
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: 
allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: 
branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: 
"_ZN1A1nEi") ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
-; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 \
-; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 
-Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s 
--check-prefixes=CHECK-IR --check-prefixes=REMARKS
+
+; Check that the devirtualization is suppressed via -wholeprogramdevirt-skip
+; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu -mllvm 
-wholeprogramdevirt-skip=_ZN1A1nEi \
+; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 
-Rpass=wholeprogramdevirt \
+; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s 
--check-prefixes=SKIP-IR --check-prefixes=SKIP-REMARKS
+
+; REMARKS: single-impl: devirtualized a call to _ZN1A1nEi
+; SKIP-REMARKS-NOT: single-impl
 
 ; Check that backend does not fail generating native code.
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
@@ -78,6 +86,7 @@ cont:
 
   ; Check that the call was devirtualized.
   ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; SKIP-IR-NOT: %call = tail call i32 @_ZN1A1nEi
   %call = tail call i32 %4(%struct.A* nonnull %obj, i32 %a)
   %vtable16 = load i8*, i8** %0
   %5 = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable16, i32 0, 
metadata !"_ZTS1A")

diff  --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp 
b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index e97f1acbb396..5350d85e11f3 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1030,6 +1030,10 @@ bool DevirtIndex::tryFindVirtualCallTargets(
 
 void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
  Constant *TheFn, bool &IsExported) {
+  // Don't devirtualize function if we're told to skip it
+  // in -wholeprogramdevirt-skip.
+  if (FunctionsToSkip.match(TheFn->stripPointerCasts()->getName()))
+return;
   auto Apply = [&](CallSiteInfo &CSInfo) {
 for (auto &&VCallSite : CSInfo.CallSites) {
   if (RemarksEnabled)



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

[PATCH] D92041: Add hover info for `this` expr

2020-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

Thanks, this sounds like a sensible idea. I got a few suggestions for the 
implementation though.




Comment at: clang-tools-extra/clangd/Hover.cpp:610
+/// Generate a \p Hover object given the \p this pointer.
+HoverInfo getHoverContents(const CXXThisExpr *CTE, const SymbolIndex *Index) {
+  const NamedDecl *D = CTE->getType()->getPointeeType()->getAsCXXRecordDecl();

what about using the existing `getHoverContents(QualType ..)` overload instead ?



Comment at: clang-tools-extra/clangd/Hover.cpp:644
 // FIXME: Support hover for literals (esp user-defined)
 llvm::Optional getHoverContents(const Expr *E, ParsedAST &AST) {
   // There's not much value in hovering over "42" and getting a hover card

can you handle `CXXThisExpr` inside this function, instead of an extra branch 
in the `getHover`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92041

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

[PATCH] D91676: Avoid redundant work when computing vtable vcall visibility

2020-11-24 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added inline comments.



Comment at: clang/lib/CodeGen/CGVTables.cpp:1301-1302
+  // has no effect on the min visibility computed below by the recursive 
caller.
+  if (!Visited.insert(RD).second)
+return llvm::GlobalObject::VCallVisibilityTranslationUnit;
+

wmi wrote:
> If a CXXRecordDecl is visited twice, the visibility returned in the second 
> visit could be larger than necessary. Will it change the final result? If it 
> will, can we cache the visibility result got in the first visit instead of 
> returning the max value? 
The recursive callsites compute the std::min of the current TypeVis and the one 
returned by the recursive call. So returning the max guarantees that there is 
no effect on the current TypeVis. Let me know if the comment can be clarified 
(that's what I meant by "so that it has no effect on the min visibility 
computed below ...". Note that the initial non-recursive invocation always has 
an empty Visited set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91676

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

[PATCH] D91029: [clangd] Implement clang-tidy options from config

2020-11-24 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

OK this looks really great, thanks so much for persisting with this.

Comments are mostly simple nits/simplifications, with the exception of `Order` 
which is a... slightly trickier simplification, but seems worth doing.

Tomorrow I'll adapt our internal clang-tidy config bits to work with this 
interface...




Comment at: clang-tools-extra/clangd/ParsedAST.cpp:241
+/// Empty clang tidy provider, using this as a provider will disable 
clang-tidy.
+static void emptyTidyProvider(tidy::ClangTidyOptions &, llvm::StringRef,
+  unsigned &, PathRef) {}

or just use fixedTidyProvider("-*")



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:74
+auto EmptyDefaults = tidy::ClangTidyOptions::getDefaults();
+EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
+EmptyDefaults.User = llvm::sys::Process::GetEnv("USER");

I'm not sure if this applies anymore - it's going to be the bottom of the 
stack, and nullopt is the default



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:75
+EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
+EmptyDefaults.User = llvm::sys::Process::GetEnv("USER");
+#ifdef _WIN32

nit: hoist the getenv out of the lambda?



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:75
+EmptyDefaults.Checks.reset(); // So we can tell if checks were ever set.
+EmptyDefaults.User = llvm::sys::Process::GetEnv("USER");
+#ifdef _WIN32

sammccall wrote:
> nit: hoist the getenv out of the lambda?
seems like we can just assign to Opts.User directly?



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:99
+  "misc-definitions-in-headers");
+  Opts.Checks.emplace(DefaultChecks);
+}

nit: = seems clearer than emplace



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:110
+  tidy::ClangTidyOptions &Opts, llvm::StringRef, unsigned &, PathRef) {
+mergeCheckList(Opts.Checks, Checks);
+mergeCheckList(Opts.WarningsAsErrors, WarningsAsErrors);

I think should just be assignment rather than merge (same with 
WarningsAsErrors).
The name suggests that it always returns the same set of checks.

(Failing that, maybe call it `addTidyChecks`)



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:167
+
+if (FS->makeAbsolute(AbsolutePath))
+  return;

(as noted elsewhere, the path is always absolute in clangd and we should make 
this an interface requiremnet)



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:170
+
+llvm::sys::path::remove_dots(AbsolutePath, true);
+llvm::StringRef Directory = llvm::sys::path::parent_path(AbsolutePath);

I don't think we need to try to stat the directory here.

FileOptionsProvider does, but looking at the history, this seems related to 
some combination of:
 - arg validation (the parameter there is a directory name, rather than a 
filename)
 - trying to return an appropriate error_code (original return type was 
ErrorOr)
 - iterating over multiple possible config files
 - maybe caching



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:181
+}
+
+for (llvm::StringRef CurrentDirectory = Directory;

can you add a FIXME to add caching here?

I hadn't realized FileOptionsProvider *was* actually doing caching.

(I don't think this is a big problem in the short term, and I don't think we 
should solve it inline in this patch, rather I should land D88172 and use that, 
which doesn't have the annoying "cache forever" behavior)



Comment at: clang-tools-extra/clangd/TidyProvider.cpp:194
+for (auto &Option : llvm::reverse(OptionStack))
+  Opts.mergeWith(Option, ++Order);
+  };

This order business is a real tangle, and looks like legacy to me (qualified vs 
unqualified names).

I don't really think we want to use/expose it beyond the requirement that we 
don't change the meaning of existing sets of `.clang-tidy` config files.
And I don't think we want to bother teaching clangd's config system about it.

So I'd suggest a hack/simplification:
 - we remove the `order` parameter from TidyProvider
 - here in provideClangTidyFiles, we just use a local Order which counts up 
from 1. This preserves the relative precedence of .clang-tidy files
 - in provideClangdConfig, we always set the order to max_uint (always wins)
 - if we ever add checkoptions to any other providers, we set the order to 0 
(always loses)
 - combine() doesn't touch order

This *does* duplicate the fact that .clangd config is stacked on top of 
.clang-tidy in ClangdMain, but in a way that only matters with disputes between 
qualified/unqualified names.



Comment at: clang

[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

2020-11-24 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D91760#2413294 , @psmith wrote:

> Radio silence so far; I think no news is good news applies in this case. I'm 
> happy to say no objections.

Thanks. I'll commit then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91760

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

[clang] f96fef8 - [Driver] Default Generic_GCC aarch64 to -fasynchronous-unwind-tables

2020-11-24 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2020-11-24T09:51:32-08:00
New Revision: f96fef89b5eca29f2dc41e97829c20bf742cdcd9

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

LOG: [Driver] Default Generic_GCC aarch64 to -fasynchronous-unwind-tables

In GCC, `aarch64-*-linux` and `aarch64-*-freebsd` made the switch in 2018
(https://gcc.gnu.org/pipermail/gcc-patches/2018-March/495549.html).
In Clang, FreeBSD/Fuchsia/NetBSD/MinGW aarch64 default to 
-fasynchronous-unwind-tables.

This patch defaults Generic_GCC aarch64 (which affects Linux) to use 
-fasynchronous-unwind-tables.

Reviewed By: nickdesaulniers

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/aarch64-features.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 7d75e90c6092..08158ba4bae8 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2672,7 +2672,13 @@ void Generic_GCC::printVerboseInfo(raw_ostream &OS) 
const {
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList &Args) const {
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {

diff  --git a/clang/test/Driver/aarch64-features.c 
b/clang/test/Driver/aarch64-features.c
index 7c3f8754049a..8d8cc3c68afe 100644
--- a/clang/test/Driver/aarch64-features.c
+++ b/clang/test/Driver/aarch64-features.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 // RUN: %clang -target arm64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 
+// CHECK: "-munwind-tables"
+
 // The AArch64 PCS states that chars should be unsigned.
 // CHECK: fno-signed-char
 



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

1 2 >

1 - 100 of 188 matches

Mail list logo