[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 331469.
RedDocMD added a comment.

Added some more positive tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp


Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -321,3 +321,29 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get(); 
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithWhileLoop(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  while (!RP) {// expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithForLoop(std::unique_ptr P) {
+  for (A *RP = P.get(); !RP;) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Obtained null inner pointer from 'P'}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -464,7 +464,7 @@
 if (() != smartptr::getNullDereferenceBugType() ||
 !BR.isInteresting(ThisRegion))
   return;
-if (!State->assume(InnerPointerVal.castAs(), true))
+if (!BR.isInteresting(InnerPointerVal) || 
!BR.isInteresting(InnerPointerVal.getAsSymbol()))
   return;
 if (ThisRegion->canPrintPretty()) {
   OS << "Obtained null inner pointer from";
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
@@ -87,6 +87,8 @@
   auto R = std::make_unique(NullDereferenceBugType,
 OS.str(), ErrNode);
   R->markInteresting(DerefRegion);
+  const Expr *BugExpr = Call.getOriginExpr();
+  bugreporter::trackExpressionValue(ErrNode, BugExpr, *R);
   C.emitReport(std::move(R));
 }
 


Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -321,3 +321,29 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get(); 
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithWhileLoop(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  while (!RP) {// expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithForLoop(std::unique_ptr P) {
+  for (A *RP = P.get(); !RP;) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Obtained null inner pointer from 'P'}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

> Does the following test work?

@NoQ, it seems to be working.

> So you're saying that simply by always tracking the (final) raw pointer value 
> and checking whether the raw value is interesting upon .get() you dodge the 
> communication problem entirely

I would not say it has been dodged, but rather that problem had already been 
solved by `trackExpressionValue`. At line 1949 of BugReporterVisitors.cpp 
(inside the `trackExpressionValue` function) is:

  if (LVState->getAnalysisManager().getAnalyzerOptions().ShouldTrackConditions)
  report.addVisitor(std::make_unique(
InputNode));

Approximately, `TrackControlDependencyCondBRVisitor` is a visitor that looks 
into condition statements and via mutual recursion with `trackExpressionValue` 
marks SVal's as interesting if they are used in a condition and that condition 
constrains the `Expr` on which the visitor was originally called on. This gave 
me the idea that calling `trackExpressionValue` is all that we really need to 
do, since it already contains a visitor to discover the interestingness we 
need. Looking into this function made me feel that `trackExpressionValue` is 
actually a very powerful function which solves a lot of these communication 
problems.

> Do i understand correctly that this doesn't happen anymore when you stopped 
> creating a new node?

Yes, and I found out my blunder after staring at the exploded graph dump. 
Creating a new node was un-necessary since `trackExpressionValue` needs a node 
corresponding to the expression where we find the bug, and that was already 
being created above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 331461.
RedDocMD added a comment.

Added a negative test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp


Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -321,3 +321,11 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get(); 
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -464,7 +464,7 @@
 if (() != smartptr::getNullDereferenceBugType() ||
 !BR.isInteresting(ThisRegion))
   return;
-if (!State->assume(InnerPointerVal.castAs(), true))
+if (!BR.isInteresting(InnerPointerVal) || 
!BR.isInteresting(InnerPointerVal.getAsSymbol()))
   return;
 if (ThisRegion->canPrintPretty()) {
   OS << "Obtained null inner pointer from";
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
@@ -87,6 +87,8 @@
   auto R = std::make_unique(NullDereferenceBugType,
 OS.str(), ErrNode);
   R->markInteresting(DerefRegion);
+  const Expr *BugExpr = Call.getOriginExpr();
+  bugreporter::trackExpressionValue(ErrNode, BugExpr, *R);
   C.emitReport(std::move(R));
 }
 


Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -321,3 +321,11 @@
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get(); 
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -464,7 +464,7 @@
 if (() != smartptr::getNullDereferenceBugType() ||
 !BR.isInteresting(ThisRegion))
   return;
-if (!State->assume(InnerPointerVal.castAs(), true))
+if (!BR.isInteresting(InnerPointerVal) || !BR.isInteresting(InnerPointerVal.getAsSymbol()))
   return;
 if (ThisRegion->canPrintPretty()) {
   OS << "Obtained null inner pointer from";
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
@@ -87,6 +87,8 @@
   auto R = std::make_unique(NullDereferenceBugType,
 OS.str(), ErrNode);
   R->markInteresting(DerefRegion);
+  const Expr *BugExpr = Call.getOriginExpr();
+  bugreporter::trackExpressionValue(ErrNode, BugExpr, *R);
   C.emitReport(std::move(R));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

By tracking the call-expression you're basically tracking the raw pointer value 
because that's what operators `*` and `->` return. Of course operator `*` 
returns an lvalue reference rather than a pointer but we don't make a 
difference when it comes to `SVal` representation.

F15896979: 525m0x.jpg 

So you're saying that simply by always tracking the (final) raw pointer value 
and checking whether the raw value is interesting upon `.get()` you dodge the 
communication problem entirely. I think this is quite a statement! I'd like a 
stronger evidence for that than passing a couple of tests. Does the following 
test work?:

  void test(std::unique_ptr P) {
A *a = P.get(); // unlike your positive test this doesn't deserve a note
// because we weren't looking at 'a' when we concluded
// that the pointer is null
if (!P) {
  P->foo();
}
  }



> Essentially, when a `unique_ptr` is moved and subsequently used, it triggers 
> two warnings - one from `SmartPointerModelling` and another from `MoveChecker`

Do i understand correctly that this doesn't happen anymore when you stopped 
creating a new node?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Xun, great to see more improvements in this area.




Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221
 CGF.EmitBlock(RealSuspendBlock);
+  } else if (ForcestackStart) {
+Builder.CreateCall(

ChuanqiXu wrote:
> lxfind wrote:
> > ChuanqiXu wrote:
> > > ChuanqiXu wrote:
> > > > can we rewrite it into:
> > > > ```
> > > > else if (SuspendRet != nullptr && SuspendRet->getType()->isClassType()) 
> > > > {
> > > >  // generate:
> > > >  // llvm.coro.forcestack(SuspendRet)
> > > > }
> > > > ```
> > > Sorry I find we can't did it directly. As you said, we need to traverse 
> > > down SuspendRet. And I still think we should did it only at CodeGen part 
> > > since it looks not so hard. I guess we could make it in above 10~15 lines 
> > > of codes.
> > Traversing down AST isn't the hard part. The hard part is to search the 
> > emitted IR, and look for the temporary alloca used to store the returned 
> > handle.
> Yes, I get your point. If we want to traverse the emitted IR, we could only 
> search for the use-chain backward, which is also very odd. Let's see if there 
> is other ways to modify the ASTNodes to make it more naturally.
I'm curious whether did you consider annotating instructions with some new 
custom metadata instead of using intrinsics? If so, what would be the tradeoff? 
For example, if you could conditionally attach metadata some "begin" metadata 
here:

`auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});`

and "end" metadata here:

`auto *SuspendResult = Builder.CreateCall(CoroSuspend, {SaveCall, 
Builder.getInt1(IsFinalSuspend)});`



Comment at: clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp:53
 
-// check that the lifetime of the coroutine handle used to obtain the address 
is contained within single basic block, and hence does not live across 
suspension points.
-// CHECK-LABEL: final.suspend:
-// CHECK: %[[PTR1:.+]] = bitcast 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* 
%[[ADDR_TMP:.+]] to i8*
-// CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[PTR1]])
-// CHECK: call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 {{[^,]*}} %[[ADDR_TMP]])
-// CHECK-NEXT:%[[PTR2:.+]] = bitcast 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[ADDR_TMP]] 
to i8*
-// CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[PTR2]])
+// CHECK-LABEL: |-FunctionDecl {{.*}} foo 'detached_task ()'
+// first ExprWithCleanups is the initial await

Nice tests. The codegen should live in a different file from the AST dump one, 
you can put the later in `test/clang/SemaCXX` or `tes/clang/AST`.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1308
+// Like the sideeffect intrinsic defined above, this intrinsic is treated by 
the
+// optimizer as having opaque side effects so that it won't be get rid of or 
moved
 // out of the block it probes.

This change seems unrelated to this patch.



Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:2083
+  ForceStackList ForceStacks;
+  for (auto  : instructions(F))
+if (auto *II = dyn_cast())

`collectForceStacks` is only called once from a function that already traverses 
all instructions, can you take advantage of that to collect 
`llvm::Intrinsic::coro_forcestack_begin/end`?



Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:2085
+if (auto *II = dyn_cast())
+  if (II->getIntrinsicID() == llvm::Intrinsic::coro_forcestack_begin) {
+assert(II->getNumUses() == 1 &&

Do such intrinsics never get removed? What happens when this hits a backend?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98638

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

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

[Artem Dergachev 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 rGc75b2261a0aa: [analyzer] Introduce common bug category 
Unused code. (authored by dergachev.a).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98741

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -2169,7 +2169,7 @@
 

descriptionValue stored to foo during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -5654,7 +5654,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -382,7 +382,7 @@
 

descriptionValue stored to x during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -450,7 +450,7 @@
 

descriptionValue stored to obj1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -518,7 +518,7 @@
 

descriptionValue stored to obj4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -586,7 +586,7 @@
 

descriptionValue stored to obj5 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -654,7 +654,7 @@
 

descriptionValue stored to obj6 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1064,7 +1064,7 @@
 

descriptionValue stored to cf1 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1132,7 +1132,7 @@
 

descriptionValue stored to cf2 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1200,7 +1200,7 @@
 

descriptionValue stored to cf3 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

@@ -1268,7 +1268,7 @@
 

descriptionValue stored to cf4 during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2368,7 +2368,7 @@
 

descriptionValue stored to x is never read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

@@ -11409,7 +11409,7 @@
 

descriptionValue stored to foo during its initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores

Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
===
--- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -22,6 +22,7 @@
 const char *const CXXObjectLifecycle = "C++ object lifecycle";
 const char *const CXXMoveSemantics = "C++ move semantics";
 const char *const SecurityError = "Security error";
+const char *const UnusedCode = "Unused code";
 } // namespace 

[clang] c75b226 - [analyzer] Introduce common bug category "Unused code".

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2021-03-17T20:58:27-07:00
New Revision: c75b2261a0aada6bf7ddd91f91139c6f06a8e367

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

LOG: [analyzer] Introduce common bug category "Unused code".

This category is generic enough to hold a variety of checkers.
Currently it contains the Dead Stores checker and an alpha unreachable
code checker.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
index 062a604a7551..392bc484bf62 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
@@ -21,6 +21,7 @@ extern const char *const UnixAPI;
 extern const char *const CXXObjectLifecycle;
 extern const char *const CXXMoveSemantics;
 extern const char *const SecurityError;
+extern const char *const UnusedCode;
 } // namespace categories
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
index 6bc186aa2755..8c86e83608b1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -260,8 +260,8 @@ class DeadStoreObs : public LiveVariables::Observer {
 break;
 }
 
-BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(),
-   L, R, Fixits);
+BR.EmitBasicReport(AC->getDecl(), Checker, BugType, categories::UnusedCode,
+   os.str(), L, R, Fixits);
   }
 
   void CheckVarDecl(const VarDecl *VD, const Expr *Ex, const Expr *Val,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
index 74eec81ffb3e..d231be64c2e1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -169,7 +169,7 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph 
,
 if (SM.isInSystemHeader(SL) || SM.isInExternCSystemHeader(SL))
   continue;
 
-B.EmitBasicReport(D, this, "Unreachable code", "Dead code",
+B.EmitBasicReport(D, this, "Unreachable code", categories::UnusedCode,
   "This statement is never executed", DL, SR);
   }
 }

diff  --git a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp 
b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
index b61e3075f3ba..d12c35ef156a 100644
--- a/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -22,6 +22,7 @@ const char *const UnixAPI = "Unix API";
 const char *const CXXObjectLifecycle = "C++ object lifecycle";
 const char *const CXXMoveSemantics = "C++ move semantics";
 const char *const SecurityError = "Security error";
+const char *const UnusedCode = "Unused code";
 } // namespace categories
 } // namespace ento
 } // namespace clang

diff  --git a/clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist 
b/clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
index 74e11075fe3d..1d82f3cd8424 100644
--- a/clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -2368,7 +2368,7 @@
 

descriptionValue stored to x is never 
read
-   categoryDead store
+   categoryUnused code
typeDead increment
check_namedeadcode.DeadStores

@@ -11409,7 +11409,7 @@
 

descriptionValue stored to foo during its 
initialization is never read
-   categoryDead store
+   categoryUnused code
typeDead initialization
check_namedeadcode.DeadStores


diff  --git a/clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist 
b/clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
index d3a1a5c6c47f..b8bc7361 100644
--- a/clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -382,7 +382,7 @@
 

descriptionValue stored to x during its 

[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

[Zakk Chen via Phabricator via cfe-commits]

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



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:700
+for (auto Idx : CTypeOrder) {
+  if (Seen.count(Idx))
+PrintFatalError(

craig.topper wrote:
> You can use
> 
> ```
> if (!Seen.insert(Idx).second)
>   PrintFatalError
> ```
> 
> This avoids walking the set twice.
> 
Fixed directly in upstream patch. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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

[PATCH] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Zakk Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG95c0125f2bc6: [Clang][RISCV] Add rvv vsetvl and vsetvlmax 
intrinsic functions. (authored by khchen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96843

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvlmax.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -145,6 +145,8 @@
   bool HasMaskedOffOperand;
   bool HasVL;
   bool HasGeneric;
+  bool HasAutoDef; // There is automiatic definition in header
+  std::string ManualCodegen;
   RVVTypePtr OutputType; // Builtin output type
   RVVTypes InputTypes;   // Builtin input types
   // The types we use to obtain the specific LLVM intrinsic. They are index of
@@ -159,8 +161,8 @@
   RVVIntrinsic(StringRef Name, StringRef Suffix, StringRef MangledName,
StringRef IRName, bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL, bool HasGeneric,
-   const RVVTypes ,
-   const std::vector );
+   bool HasAutoDef, StringRef ManualCodegen, const RVVTypes ,
+   const std::vector );
   ~RVVIntrinsic() = default;
 
   StringRef getName() const { return Name; }
@@ -169,6 +171,8 @@
   bool hasMaskedOffOperand() const { return HasMaskedOffOperand; }
   bool hasVL() const { return HasVL; }
   bool hasGeneric() const { return HasGeneric; }
+  bool hasManualCodegen() const { return !ManualCodegen.empty(); }
+  bool hasAutoDef() const { return HasAutoDef; }
   size_t getNumOperand() const { return InputTypes.size(); }
   StringRef getIRName() const { return IRName; }
   uint8_t getRISCVExtensions() const { return RISCVExtensions; }
@@ -190,6 +194,7 @@
 class RVVEmitter {
 private:
   RecordKeeper 
+  std::string HeaderCode;
   // Concat BasicType, LMUL and Proto as key
   StringMap LegalTypes;
   StringSet<> IllegalTypes;
@@ -637,11 +642,13 @@
StringRef NewMangledName, StringRef IRName,
bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL,
-   bool HasGeneric, const RVVTypes ,
+   bool HasGeneric, bool HasAutoDef,
+   StringRef ManualCodegen, const RVVTypes ,
const std::vector )
 : IRName(IRName), HasSideEffects(HasSideEffects),
   HasMaskedOffOperand(HasMaskedOffOperand), HasVL(HasVL),
-  HasGeneric(HasGeneric) {
+  HasGeneric(HasGeneric), HasAutoDef(HasAutoDef),
+  ManualCodegen(ManualCodegen.str()) {
 
   // Init Name and MangledName
   Name = NewName.str();
@@ -702,7 +709,13 @@
 }
 
 void RVVIntrinsic::emitCodeGenSwitchBody(raw_ostream ) const {
+
   OS << "  ID = Intrinsic::riscv_" + getIRName() + ";\n";
+  if (hasManualCodegen()) {
+OS << ManualCodegen;
+OS << "break;\n";
+return;
+  }
   OS << "  IntrinsicTypes = {";
   ListSeparator LS;
   for (const auto  : IntrinsicTypes) {
@@ -792,6 +805,11 @@
   std::vector> Defs;
   createRVVIntrinsics(Defs);
 
+  // Print header code
+  if (!HeaderCode.empty()) {
+OS << HeaderCode;
+  }
+
   auto printType = [&](auto T) {
 OS << "typedef " << T->getClangBuiltinStr() << " " << T->getTypeStr()
<< ";\n";
@@ -910,7 +928,6 @@
 
 void RVVEmitter::createRVVIntrinsics(
 std::vector> ) {
-
   std::vector RV = Records.getAllDerivedDefinitions("RVVBuiltin");
   for (auto *R : RV) {
 StringRef Name = R->getValueAsString("Name");
@@ -924,11 +941,18 @@
 bool HasGeneric = R->getValueAsBit("HasGeneric");
 bool HasSideEffects = R->getValueAsBit("HasSideEffects");
 std::vector Log2LMULList = R->getValueAsListOfInts("Log2LMUL");
+StringRef ManualCodegen = R->getValueAsString("ManualCodegen");
+StringRef ManualCodegenMask = R->getValueAsString("ManualCodegenMask");
 std::vector IntrinsicTypes =
 R->getValueAsListOfInts("IntrinsicTypes");
 StringRef IRName = R->getValueAsString("IRName");
 StringRef IRNameMask = R->getValueAsString("IRNameMask");
 
+StringRef HeaderCodeStr = R->getValueAsString("HeaderCode");
+bool HasAutoDef = HeaderCodeStr.empty();
+if (!HeaderCodeStr.empty()) {
+  HeaderCode += HeaderCodeStr.str();
+}
 // Parse prototype and create a list of primitive type with transformers
 // (operand) in ProtoSeq. ProtoSeq[0] is output operand.
 SmallVector ProtoSeq;
@@ -955,7 +979,7 @@
   ProtoMaskSeq.push_back("z");
 }
 
-// Create intrinsics for each type and LMUL.
+// Create Intrinsics for each type and LMUL.
 for (char I : TypeRange) {
   

[clang] 95c0125 - [Clang][RISCV] Add rvv vsetvl and vsetvlmax intrinsic functions.

[Zakk Chen via cfe-commits]

Author: Zakk Chen
Date: 2021-03-17T20:26:06-07:00
New Revision: 95c0125f2bc610d9c51d4fbdd1144fcab40f3b51

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

LOG: [Clang][RISCV] Add rvv vsetvl and vsetvlmax intrinsic functions.

Reviewed By: craig.topper

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

Added: 
clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvlmax.c

Modified: 
clang/include/clang/Basic/riscv_vector.td
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index c69b5be1798c..0efe10c94f2e 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -159,7 +159,11 @@ class RVVBuiltin Log2LMUL = [0, 1, 2, 3, -1, -2, -3];
 
-  // Emit the automatic clang codegen. It describes what types we have to use
+  // Manual code in clang codegen riscv_vector_builtin_cg.inc
+  code ManualCodegen = [{}];
+  code ManualCodegenMask = [{}];
+
+  // When emit the automatic clang codegen, it describes what types we have to 
use
   // to obtain the specific LLVM intrinsic. -1 means the return type, 
otherwise,
   // k >= 0 meaning the k-th operand (counting from zero) of the codegen'd
   // parameter of the unmasked version. k can't be the mask operand's position.
@@ -171,6 +175,11 @@ class RVVBuiltin;
+
+  let HeaderCode =
+[{
+#define vsetvlmax_e8mf8() __builtin_rvv_vsetvlimax(0, 5)
+#define vsetvlmax_e8mf4() __builtin_rvv_vsetvlimax(0, 6)
+#define vsetvlmax_e8mf2() __builtin_rvv_vsetvlimax(0, 7)
+#define vsetvlmax_e8m1() __builtin_rvv_vsetvlimax(0, 0)
+#define vsetvlmax_e8m2() __builtin_rvv_vsetvlimax(0, 1)
+#define vsetvlmax_e8m4() __builtin_rvv_vsetvlimax(0, 2)
+#define vsetvlmax_e8m8() __builtin_rvv_vsetvlimax(0, 3)
+
+#define vsetvlmax_e16mf4() __builtin_rvv_vsetvlimax(1, 6)
+#define vsetvlmax_e16mf2() __builtin_rvv_vsetvlimax(1, 7)
+#define vsetvlmax_e16m1() __builtin_rvv_vsetvlimax(1, 0)
+#define vsetvlmax_e16m2() __builtin_rvv_vsetvlimax(1, 1)
+#define vsetvlmax_e16m4() __builtin_rvv_vsetvlimax(1, 2)
+#define vsetvlmax_e16m8() __builtin_rvv_vsetvlimax(1, 3)
+
+#define vsetvlmax_e32mf2() __builtin_rvv_vsetvlimax(2, 7)
+#define vsetvlmax_e32m1() __builtin_rvv_vsetvlimax(2, 0)
+#define vsetvlmax_e32m2() __builtin_rvv_vsetvlimax(2, 1)
+#define vsetvlmax_e32m4() __builtin_rvv_vsetvlimax(2, 2)
+#define vsetvlmax_e32m8() __builtin_rvv_vsetvlimax(2, 3)
+
+#define vsetvlmax_e64m1() __builtin_rvv_vsetvlimax(3, 0)
+#define vsetvlmax_e64m2() __builtin_rvv_vsetvlimax(3, 1)
+#define vsetvlmax_e64m4() __builtin_rvv_vsetvlimax(3, 2)
+#define vsetvlmax_e64m8() __builtin_rvv_vsetvlimax(3, 3)
+
+}] in
+  def vsetvlimax : RVVBuiltin<"", "zKzKz", "i">;
+}
+
 // 12. Vector Integer Arithmetic Instructions
 // 12.1. Vector Single-Width Integer Add and Subtract
 defm vadd : RVVBinBuiltinSet<"vadd", "csil",

diff  --git a/clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c 
b/clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
new file mode 100644
index ..e837653304a7
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
@@ -0,0 +1,451 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+
+// RUN: %clang_cc1 -triple riscv32 -target-feature +experimental-v -emit-llvm 
-o - %s \
+// RUN:   | FileCheck --check-prefix=CHECK-RV32 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -emit-llvm 
-o - %s \
+// RUN:   | FileCheck --check-prefix=CHECK-RV64 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-v -Werror 
-Wall -o - \
+// RUN:   %s > /dev/null 2>&1 | FileCheck --check-prefix=ASM --allow-empty 
%s
+
+// ASM-NOT: warning
+#include 
+
+// CHECK-RV32-LABEL: @test_vsetvl_e8m1(
+// CHECK-RV32-NEXT:  entry:
+// CHECK-RV32-NEXT:[[AVL_ADDR:%.*]] = alloca i32, align 4
+// CHECK-RV32-NEXT:store i32 [[AVL:%.*]], i32* [[AVL_ADDR]], align 4
+// CHECK-RV32-NEXT:[[TMP0:%.*]] = load i32, i32* [[AVL_ADDR]], align 4
+// CHECK-RV32-NEXT:[[TMP1:%.*]] = call i32 @llvm.riscv.vsetvli.i32(i32 
[[TMP0]], i32 0, i32 0)
+// CHECK-RV32-NEXT:ret i32 [[TMP1]]
+//
+// CHECK-RV64-LABEL: @test_vsetvl_e8m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[AVL_ADDR:%.*]] = alloca i64, align 8
+// CHECK-RV64-NEXT:store i64 [[AVL:%.*]], i64* [[AVL_ADDR]], align 8
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = load i64, i64* [[AVL_ADDR]], align 8
+// CHECK-RV64-NEXT:[[TMP1:%.*]] = call i64 @llvm.riscv.vsetvli.i64(i64 
[[TMP0]], i64 0, i64 0)
+// CHECK-RV64-NEXT:ret i64 [[TMP1]]
+//
+size_t test_vsetvl_e8m1(size_t avl) {
+  return vsetvl_e8m1(avl);
+}
+
+// CHECK-RV32-LABEL: @test_vsetvl_e8m2(
+// 

[PATCH] D97916: [Driver][RISCV] Support parsing multi-lib config from GCC.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 331448.
kito-cheng added a comment.

ChangeLog:

- Add more verbose message during reading GCC multilib configuration


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97916

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/bin/riscv32-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/lib/gcc/riscv32-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv32_elf_sdk/riscv32-unknown-elf/lib/rv64imac/lp64/crt0.o
  clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/bin/riscv64-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/bin/riscv64-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad2/bin/riscv64-unknown-elf-gcc
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad2/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad2/riscv64-unknown-elf/lib/crt0.o
  clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad3/bin/.keep
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad3/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv64_elf_sdk_bad3/riscv64-unknown-elf/lib/crt0.o
  clang/test/Driver/riscv-toolchain-gcc-multilib.c

Index: clang/test/Driver/riscv-toolchain-gcc-multilib.c
===
--- /dev/null
+++ clang/test/Driver/riscv-toolchain-gcc-multilib.c
@@ -0,0 +1,64 @@
+// Test case for scanning input of GCC output as multilib config
+// Skip this test on Windows, we can't create a dummy GCC to output
+// multilib config, ExecuteAndWait only execute *.exe file.
+// UNSUPPORTED: system-windows
+
+// RUN: %clang %s \
+// RUN:   -target riscv32-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv32_elf_sdk \
+// RUN:   --print-multi-lib \
+// RUN:   | FileCheck -check-prefix=C-RV32-GCC-MULTI-LIB %s
+// C-RV32-GCC-MULTI-LIB: rv32iac/ilp32;@march=rv32iac@mabi=ilp32
+// C-RV32-GCC-MULTI-LIB-NEXT: rv32imafc/ilp32f;@march=rv32iac@mabi=ilp32f
+// C-RV32-GCC-MULTI-LIB-NEXT: rv64imafdc/lp64d;@march=rv64imafdc@mabi=lp64d
+// 

[PATCH] D98688: [-Wcalled-once-parameter] Harden analysis in terms of block use

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thanks!!




Comment at: clang/test/SemaObjC/warn-called-once.m:861
   // We consider captures by blocks as escapes
-  [self indirect_call:(^{
+  [self indirect_call:(^{ // expected-note{{previous call is here}}
   callback();

What would it take to move this note to the `callback();` line? It would be 
great to do so because blocks are often huge and `^` is often hard to notice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98688

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

To be clear, what I'm trying to say is that — absent consensus that it's a 
major architecture shift is appropriate — we need to consider what 
functionality is reasonably achievable without it.  I'm sure that covers most 
of what you're trying to do; it just may not include everything.

One of the fortunate things about working in a REPL is that many ABI 
considerations go completely out the window.  The most important of those is 
the abstract need for symbol-matching; that is, there's practically no reason 
why a C REPL needs to use the simple C symbol mangling, because nobody expects 
code written outside the REPL to link up with user-entered declarations.  So we 
can be quite aggressive about how we emit declarations "behind the scenes"; C 
mode really just means C's user-level semantics, calling convention, and type 
layout, but not any of C's ordinary interoperation/compatibility requirements.


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

https://reviews.llvm.org/D96033

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

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Tiago Macarios via Phabricator via cfe-commits]

tiagoma added a comment.
Herald added a subscriber: shchenz.

Can we get this in? I work in Microsoft Office and we have been using this 
checker and it works great! There are a couple of issues with it and I would 
like to contribute fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

In D98757#2631042 , @lebedev.ri wrote:

> The ongoing special-casing of `X86_AMXTy` through the llvm due to the 
> inability of the existing backend passes to handle certain llvm ir constructs.

We have bring up it to llvm-dev.
BTW,** All the Type should see as target independent.** (Even it support by 
less targets or 1 target)

Current we see  “ if (Ty.isVectorTy()) {…}” is make sense in Mid-End. 
Why we can’t see “if (Ty.isX86_AMXTy()){…}” is make sense ?

**Just because more targets support the VectorTy, less target (only x86) 
support the AMXTy ?
The logic is not make sense.**


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98757

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

[clang] d672d52 - Revert "[CodeGenModule] Set dso_local for Mach-O GlobalValue"

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2021-03-17T17:27:41-07:00
New Revision: d672d5219a72d2e13dcc257116876d41955e36b2

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

LOG: Revert "[CodeGenModule] Set dso_local for Mach-O GlobalValue"

This reverts commit 809a1e0ffd7af40ee27270ff8ba2ffc927330e71.

Mach-O doesn't support dso_local and this change broke XNU because of the use 
of dso_local.

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/attr-weak-import.c
clang/test/CodeGenCXX/bitfield-layout.cpp
clang/test/CodeGenCXX/const-init.cpp
clang/test/CodeGenCXX/linkage.cpp
clang/test/CodeGenCXX/temporaries.cpp
clang/test/CodeGenCXX/type_visibility.cpp
clang/test/CodeGenCXX/visibility.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f3a73f8783dc..3a197e85ef7b 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -978,20 +978,14 @@ static bool shouldAssumeDSOLocal(const CodeGenModule ,
   if (TT.isOSBinFormatCOFF() || (TT.isOSWindows() && TT.isOSBinFormatMachO()))
 return true;
 
-  const auto  = CGM.getCodeGenOpts();
-  llvm::Reloc::Model RM = CGOpts.RelocationModel;
-  const auto  = CGM.getLangOpts();
-
-  if (TT.isOSBinFormatMachO()) {
-if (RM == llvm::Reloc::Static)
-  return true;
-return GV->isStrongDefinitionForLinker();
-  }
-
   // Only handle COFF and ELF for now.
   if (!TT.isOSBinFormatELF())
 return false;
 
+  // If this is not an executable, don't assume anything is local.
+  const auto  = CGM.getCodeGenOpts();
+  llvm::Reloc::Model RM = CGOpts.RelocationModel;
+  const auto  = CGM.getLangOpts();
   if (RM != llvm::Reloc::Static && !LOpts.PIE) {
 // On ELF, if -fno-semantic-interposition is specified and the target
 // supports local aliases, there will be neither CC1

diff  --git a/clang/test/CodeGen/attr-weak-import.c 
b/clang/test/CodeGen/attr-weak-import.c
index 85989f03a277..f02d09e81509 100644
--- a/clang/test/CodeGen/attr-weak-import.c
+++ b/clang/test/CodeGen/attr-weak-import.c
@@ -18,9 +18,9 @@ extern int E __attribute__((weak_import));
 int E;
 extern int E __attribute__((weak_import));
 
-// CHECK: @A = dso_local global i32
+// CHECK: @A = global i32
 // CHECK-NOT: @B =
-// CHECK: @C = dso_local global i32
-// CHECK: @D = dso_local global i32
-// CHECK: @E = dso_local global i32
+// CHECK: @C = global i32
+// CHECK: @D = global i32
+// CHECK: @E = global i32
 

diff  --git a/clang/test/CodeGenCXX/bitfield-layout.cpp 
b/clang/test/CodeGenCXX/bitfield-layout.cpp
index 79dbf9c691c4..d570b8f33e34 100644
--- a/clang/test/CodeGenCXX/bitfield-layout.cpp
+++ b/clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -93,7 +93,7 @@ int test_trunc_int() {
   } const U = {15};  // 0b
   return U.i;
 }
-// CHECK: define dso_local i32 @test_trunc_int()
+// CHECK: define{{.*}} i32 @test_trunc_int()
 // CHECK: ret i32 -1
 
 int test_trunc_three_bits() {
@@ -102,7 +102,7 @@ int test_trunc_three_bits() {
   } const U = {15};  // 0b
   return U.i;
 }
-// CHECK: define dso_local i32 @test_trunc_three_bits()
+// CHECK: define{{.*}} i32 @test_trunc_three_bits()
 // CHECK: ret i32 -1
 
 int test_trunc_1() {
@@ -111,7 +111,7 @@ int test_trunc_1() {
   } const U = {15};  // 0b
   return U.i;
 }
-// CHECK: define dso_local i32 @test_trunc_1()
+// CHECK: define{{.*}} i32 @test_trunc_1()
 // CHECK: ret i32 -1
 
 int test_trunc_zero() {
@@ -120,7 +120,7 @@ int test_trunc_zero() {
   } const U = {80};  // 0b0101
   return U.i;
 }
-// CHECK: define dso_local i32 @test_trunc_zero()
+// CHECK: define{{.*}} i32 @test_trunc_zero()
 // CHECK: ret i32 0
 
 int test_constexpr() {
@@ -129,7 +129,7 @@ int test_constexpr() {
   } const U = {1 + 2 + 4 + 8}; // 0b
   return U.i;
 }
-// CHECK: define dso_local i32 @test_constexpr()
+// CHECK: define{{.*}} i32 @test_constexpr()
 // CHECK: ret i32 -1
 
 int test_notrunc() {
@@ -138,7 +138,7 @@ int test_notrunc() {
   } const U = {1 + 2 + 4 + 8}; // 0b
   return U.i;
 }
-// CHECK: define dso_local i32 @test_notrunc()
+// CHECK: define{{.*}} i32 @test_notrunc()
 // CHECK: ret i32 15
 
 long long test_trunc_long_long() {
@@ -147,6 +147,6 @@ long long test_trunc_long_long() {
   } const U = {0b010001001101};
   return U.i;
 }
-// CHECK: define dso_local i64 @test_trunc_long_long()
+// CHECK: define{{.*}} i64 @test_trunc_long_long()
 // CHECK: ret i64 3917
 }

diff  --git a/clang/test/CodeGenCXX/const-init.cpp 
b/clang/test/CodeGenCXX/const-init.cpp
index 5b305bc5e4d6..f5c9dae7ba4b 100644
--- a/clang/test/CodeGenCXX/const-init.cpp
+++ b/clang/test/CodeGenCXX/const-init.cpp
@@ -2,17 +2,17 @@
 // 

[PATCH] D98458: Revert "[CodeGenModule] Set dso_local for Mach-O GlobalValue"

[Alex Lorenz 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 rGd672d5219a72: Revert [CodeGenModule] Set dso_local for 
Mach-O GlobalValue (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D98458?vs=330077=331421#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98458

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-weak-import.c
  clang/test/CodeGenCXX/bitfield-layout.cpp
  clang/test/CodeGenCXX/const-init.cpp
  clang/test/CodeGenCXX/linkage.cpp
  clang/test/CodeGenCXX/temporaries.cpp
  clang/test/CodeGenCXX/type_visibility.cpp
  clang/test/CodeGenCXX/visibility.cpp

Index: clang/test/CodeGenCXX/visibility.cpp
===
--- clang/test/CodeGenCXX/visibility.cpp
+++ clang/test/CodeGenCXX/visibility.cpp
@@ -18,7 +18,7 @@
   };
   H DEFAULT a;
   X<> b;
-  // CHECK: _ZN6test301bE = dso_local global
+  // CHECK: _ZN6test301bE = global
   // CHECK-HIDDEN: _ZN6test301bE = hidden global
 }
 
@@ -33,7 +33,7 @@
   class DEFAULT A { };
 
   X::definition a;
-  // CHECK: @_ZN6test251aE = dso_local global
+  // CHECK: @_ZN6test251aE = global
   // CHECK-HIDDEN: @_ZN6test251aE = hidden global
 }
 
@@ -41,7 +41,7 @@
   class DEFAULT foo {
   };
   foo myvec;
-  // CHECK: @_ZN6test285myvecE = dso_local global
+  // CHECK: @_ZN6test285myvecE = global
   // CHECK-HIDDEN: @_ZN6test285myvecE = hidden global
 }
 
@@ -53,8 +53,8 @@
   DEFAULT extern RECT data_rect;
   RECT data_rect = { -1};
 #pragma GCC visibility pop
-  // CHECK: @_ZN6test299data_rectE = dso_local global
-  // CHECK-HIDDEN: @_ZN6test299data_rectE = dso_local global
+  // CHECK: @_ZN6test299data_rectE = global
+  // CHECK-HIDDEN: @_ZN6test299data_rectE = global
 }
 
 namespace test40 {
@@ -103,17 +103,17 @@
 
 // CHECK: @_ZN5Test425VariableInHiddenNamespaceE = hidden global i32 10
 // CHECK: @_ZN5Test71aE = hidden global
-// CHECK: @_ZN5Test71bE = dso_local global
-// CHECK: @test9_var = dso_local global
-// CHECK-HIDDEN: @test9_var = dso_local global
+// CHECK: @_ZN5Test71bE = global
+// CHECK: @test9_var = global
+// CHECK-HIDDEN: @test9_var = global
 // CHECK: @_ZN6Test121A6hiddenE = external hidden global
 // CHECK: @_ZN6Test121A7visibleE = external global
 // CHECK-HIDDEN: @_ZN6Test121A6hiddenE = external hidden global
 // CHECK-HIDDEN: @_ZN6Test121A7visibleE = external global
 // CHECK: @_ZN6Test131B1aE = hidden global
-// CHECK: @_ZN6Test131C1aE = dso_local global
+// CHECK: @_ZN6Test131C1aE = global
 // CHECK-HIDDEN: @_ZN6Test131B1aE = hidden global
-// CHECK-HIDDEN: @_ZN6Test131C1aE = dso_local global
+// CHECK-HIDDEN: @_ZN6Test131C1aE = global
 // CHECK: @_ZN6Test143varE = external global
 // CHECK-HIDDEN: @_ZN6Test143varE = external global
 // CHECK: @_ZN6Test154TempINS_1AEE5Inner6bufferE = external global [0 x i8]
@@ -134,8 +134,8 @@
 
   void C::D::g() {
   }
-  // CHECK: _ZTVN6test271CIiE1DE = dso_local unnamed_addr constant
-  // CHECK-HIDDEN: _ZTVN6test271CIiE1DE = dso_local unnamed_addr constant
+  // CHECK: _ZTVN6test271CIiE1DE = unnamed_addr constant
+  // CHECK-HIDDEN: _ZTVN6test271CIiE1DE = unnamed_addr constant
 }
 
 // CHECK: @_ZTVN5Test63fooE = linkonce_odr hidden unnamed_addr constant
@@ -195,7 +195,7 @@
   };
   
   // A has default visibility.
-  // CHECK-LABEL: define dso_local void @_ZN5Test41A1fEv
+  // CHECK-LABEL: define void @_ZN5Test41A1fEv
   void A::f() { } 
 }
 
@@ -209,7 +209,7 @@
   
   namespace NS {
 // g is in NS, but this NS decl is not hidden.
-// CHECK-LABEL: define dso_local void @_ZN5Test52NS1gEv
+// CHECK-LABEL: define void @_ZN5Test52NS1gEv
 void g() { }
   }
 }
@@ -268,8 +268,8 @@
 void DEFAULT test9_fun(struct A *a) { }
 struct A DEFAULT test9_var; // above
   }
-  // CHECK-LABEL: define dso_local void @test9_fun(
-  // CHECK-HIDDEN-LABEL: define dso_local void @test9_fun(
+  // CHECK-LABEL: define void @test9_fun(
+  // CHECK-HIDDEN-LABEL: define void @test9_fun(
 
   void test() {
 A a = test9_var;
@@ -285,8 +285,8 @@
 void foo(A*);
   };
 
-  // CHECK-LABEL: define dso_local void @_ZN6Test101B3fooEPNS_1AE(
-  // CHECK-HIDDEN-LABEL: define dso_local void @_ZN6Test101B3fooEPNS_1AE(
+  // CHECK-LABEL: define void @_ZN6Test101B3fooEPNS_1AE(
+  // CHECK-HIDDEN-LABEL: define void @_ZN6Test101B3fooEPNS_1AE(
   void B::foo(A*) {}
 }
 
@@ -507,7 +507,7 @@
 static void test3();
   };
 
-  // CHECK-LABEL: define dso_local void @_ZN6Test201AILj1EE5test2Ev()
+  // CHECK-LABEL: define void @_ZN6Test201AILj1EE5test2Ev()
   void A<1>::test2() {}
 
   // CHECK: declare void @_ZN6Test201AILj1EE5test3Ev()
@@ -684,8 +684,8 @@
   template<>
   void C::f() { }
 
-  // CHECK-LABEL: define dso_local void @_ZN6test261CIiE1fEv
-  // CHECK-HIDDEN-LABEL: define dso_local void @_ZN6test261CIiE1fEv
+  // CHECK-LABEL: define 

[clang] 3315bd0 - PR49619: Remove delayed call to noteFailed.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-03-17T17:25:18-07:00
New Revision: 3315bd0beb4cf23f838bd522a1f0e3fcc0a9fae2

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

LOG: PR49619: Remove delayed call to noteFailed.

This would assert if we hit the evaluation step limit between starting
to delay the call and finishing. In any case, delaying the call was
largely pointless as it doesn't really matter when we mark the
evaluation as having had side effects.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/Sema/integer-overflow.c

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 4213beb915af..624b1bfde4e6 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12472,25 +12472,6 @@ void 
DataRecursiveIntBinOpEvaluator::process(EvalResult ) {
 }
 
 namespace {
-/// Used when we determine that we should fail, but can keep evaluating prior 
to
-/// noting that we had a failure.
-class DelayedNoteFailureRAII {
-  EvalInfo 
-  bool NoteFailure;
-
-public:
-  DelayedNoteFailureRAII(EvalInfo , bool NoteFailure = true)
-  : Info(Info), NoteFailure(NoteFailure) {}
-  ~DelayedNoteFailureRAII() {
-if (NoteFailure) {
-  bool ContinueAfterFailure = Info.noteFailure();
-  (void)ContinueAfterFailure;
-  assert(ContinueAfterFailure &&
- "Shouldn't have kept evaluating on failure.");
-}
-  }
-};
-
 enum class CmpResult {
   Unequal,
   Less,
@@ -12858,12 +12839,14 @@ bool RecordExprEvaluator::VisitBinCmp(const 
BinaryOperator *E) {
 }
 
 bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
-  // We don't call noteFailure immediately because the assignment happens after
-  // we evaluate LHS and RHS.
-  if (!Info.keepEvaluatingAfterFailure() && E->isAssignmentOp())
-return Error(E);
+  // We don't support assignment in C. C++ assignments don't get here because
+  // assignment is an lvalue in C++.
+  if (E->isAssignmentOp()) {
+Error(E);
+if (!Info.noteFailure())
+  return false;
+  }
 
-  DelayedNoteFailureRAII MaybeNoteFailureLater(Info, E->isAssignmentOp());
   if (DataRecursiveIntBinOpEvaluator::shouldEnqueue(E))
 return DataRecursiveIntBinOpEvaluator(*this, Result).Traverse(E);
 

diff  --git a/clang/test/Sema/integer-overflow.c 
b/clang/test/Sema/integer-overflow.c
index 39395d9bc1fd..79e9294067de 100644
--- a/clang/test/Sema/integer-overflow.c
+++ b/clang/test/Sema/integer-overflow.c
@@ -203,3 +203,12 @@ struct s2 {
 }
   }
 };
+
+void PR49619() {
+  int n;
+  n = ({
+while (1)
+  ;
+0;
+  });
+}



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

[PATCH] D98814: [CUDA][HIP] Mark device var used by host only

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CGCUDANV.cpp:1102
+  Info.D->isUsed() && !Info.D->hasAttr()) {
+CGM.addCompilerUsedGlobal(Info.Var);
+  }

Do we want to limit it further to only externally-visible variables?
I think we already externalize the variables we want to be visible across 
host/device boundary.
If the variable is not visible, there's no point keeping it around as the 
runtime will not be able to find it in the GPU binary.


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

https://reviews.llvm.org/D98814

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.



In D98783#2632244 , @yaxunl wrote:

> In D98783#2632143 , @tra wrote:
>
>> Shouldn't the unused ASCs be eliminated by DCE? We seem to be cleaning up 
>> the consequences of a problem that happened somewhere else.
>
> DCE does not eliminate these unused ASCs because normally they should not 
> exist. That's why we do not want them to be kept in the IR emitted by clang.

TBH, these 'invisible' ASCs do bother me. I wonder what else is present in the 
IR that we can't easily examine with existing tools?
In principle I'm OK with eliminating such dangling ASCs here as a short-term 
workaround, but I think it's potentially a more general issue which needs a 
more general solution.

One way to consistently deal with something like that is to codegen something 
using those ASCs, but which would be considered unused and later DCE'd along 
with unused ASCs. 
We could use some sort of counterpart for `@llvm.used` only for potentially 
unused IR we create.
Tying them to such `@llvm_maybe_unused` would avoid the problem.


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

https://reviews.llvm.org/D98783

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

[PATCH] D98775: [AST] Add introspection support for Decls

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 331413.
steveire added a comment.
Herald added a subscriber: mgorny.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98775

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- clang/unittests/Introspection/IntrospectionTest.cpp
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -42,7 +42,7 @@
 
 #define STRING_LOCATION_PAIR(INSTANCE, LOC) Pair(#LOC, INSTANCE->LOC)
 
-TEST(Introspection, SourceLocations) {
+TEST(Introspection, SourceLocations_Stmt) {
   auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
   std::make_shared());
   auto  = AST->getASTContext();
@@ -79,3 +79,69 @@
   EXPECT_THAT(ExpectedRanges, UnorderedElementsAre(STRING_LOCATION_PAIR(
   FooCall, getSourceRange(;
 }
+
+TEST(Introspection, SourceLocations_Decl) {
+  auto AST =
+  buildASTFromCode(R"cpp(
+namespace ns1 {
+namespace ns2 {
+template  struct Foo {};
+template  struct Bar {
+  struct Nested {
+template 
+Foo method(int i, bool b) const noexcept(true);
+  };
+};
+} // namespace ns2
+} // namespace ns1
+
+template 
+template 
+ns1::ns2::Foo ns1::ns2::Bar::Nested::method(int i, bool b) const
+noexcept(true) {}
+)cpp",
+   "foo.cpp", std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  cxxMethodDecl(hasName("method"), isDefinition()).bind("method"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *MethodDecl = BoundNodes[0].getNodeAs("method");
+
+  auto Result = NodeIntrospection::GetLocations(MethodDecl);
+
+  if (Result.LocationAccessors.empty() && Result.RangeAccessors.empty()) {
+return;
+  }
+
+  auto ExpectedLocations =
+  FormatExpected(Result.LocationAccessors);
+
+  EXPECT_THAT(ExpectedLocations,
+  UnorderedElementsAre(
+  STRING_LOCATION_PAIR(MethodDecl, getBeginLoc()),
+  STRING_LOCATION_PAIR(MethodDecl, getBodyRBrace()),
+  STRING_LOCATION_PAIR(MethodDecl, getEllipsisLoc()),
+  STRING_LOCATION_PAIR(MethodDecl, getInnerLocStart()),
+  STRING_LOCATION_PAIR(MethodDecl, getLocation()),
+  STRING_LOCATION_PAIR(MethodDecl, getOuterLocStart()),
+  STRING_LOCATION_PAIR(MethodDecl, getPointOfInstantiation()),
+  STRING_LOCATION_PAIR(MethodDecl, getTypeSpecEndLoc()),
+  STRING_LOCATION_PAIR(MethodDecl, getTypeSpecStartLoc()),
+  STRING_LOCATION_PAIR(MethodDecl, getEndLoc(;
+
+  auto ExpectedRanges = FormatExpected(Result.RangeAccessors);
+
+  EXPECT_THAT(
+  ExpectedRanges,
+  UnorderedElementsAre(
+  STRING_LOCATION_PAIR(MethodDecl, getExceptionSpecSourceRange()),
+  STRING_LOCATION_PAIR(MethodDecl, getParametersSourceRange()),
+  STRING_LOCATION_PAIR(MethodDecl, getReturnTypeSourceRange()),
+  STRING_LOCATION_PAIR(MethodDecl, getSourceRange(;
+}
Index: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
===
--- clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -177,6 +177,9 @@
 NodeLocationAccessors NodeIntrospection::GetLocations(clang::Stmt const *) {
   return {};
 }
+NodeLocationAccessors NodeIntrospection::GetLocations(clang::Decl const *) {
+  return {};
+}
 NodeLocationAccessors
 NodeIntrospection::GetLocations(clang::DynTypedNode const &) {
   return {};
Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -26,7 +26,8 @@
   isDefinition(),
   isSameOrDerivedFrom(
   // TODO: Extend this with other clades
-  namedDecl(hasName("clang::Stmt")).bind("nodeClade")),
+  namedDecl(hasAnyName("clang::Stmt", "clang::Decl"))
+  .bind("nodeClade")),
   optionally(isDerivedFrom(cxxRecordDecl().bind("derivedFrom"
   .bind("className"),
   this);
Index: clang/lib/Tooling/CMakeLists.txt
===
--- clang/lib/Tooling/CMakeLists.txt
+++ clang/lib/Tooling/CMakeLists.txt
@@ -40,6 +40,9 @@
 NodeLocationAccessors 

[PATCH] D98827: [AST] Ensure that an empty json file is generated if compile errors

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98827

Files:
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp


Index: clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
===
--- clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
+++ clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
@@ -48,7 +48,13 @@
 public:
   ASTSrcLocGenerationAction() : Processor(JsonOutputPath) {}
 
-  ~ASTSrcLocGenerationAction() { Processor.generate(); }
+  void ExecuteAction() override {
+clang::ASTFrontendAction::ExecuteAction();
+if (getCompilerInstance().getDiagnostics().getNumErrors() > 0)
+  Processor.generateEmpty();
+else
+  Processor.generate();
+  }
 
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance ,
Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
@@ -30,6 +30,7 @@
  StringRef File);
 
   void generate();
+  void generateEmpty();
 
 private:
   void run(const ast_matchers::MatchFinder::MatchResult ) override;
Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -79,17 +79,16 @@
   return JsonObj;
 }
 
-void WriteJSON(std::string JsonPath,
-   llvm::StringMap const ,
-   llvm::StringMap> const ,
-   llvm::StringMap const ) {
+void WriteJSON(std::string JsonPath, llvm::json::Object &,
+   llvm::json::Object &,
+   llvm::json::Object &) {
   llvm::json::Object JsonObj;
 
   using llvm::json::toJSON;
 
-  JsonObj["classInheritance"] = ::toJSON(ClassInheritance);
-  JsonObj["classesInClade"] = ::toJSON(ClassesInClade);
-  JsonObj["classEntries"] = ::toJSON(ClassEntries);
+  JsonObj["classInheritance"] = std::move(ClassInheritance);
+  JsonObj["classesInClade"] = std::move(ClassesInClade);
+  JsonObj["classEntries"] = std::move(ClassEntries);
 
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
@@ -101,9 +100,12 @@
 }
 
 void ASTSrcLocProcessor::generate() {
-  WriteJSON(JsonPath, ClassInheritance, ClassesInClade, ClassEntries);
+  WriteJSON(JsonPath, ::toJSON(ClassInheritance), ::toJSON(ClassesInClade),
+::toJSON(ClassEntries));
 }
 
+void ASTSrcLocProcessor::generateEmpty() { WriteJSON(JsonPath, {}, {}, {}); }
+
 std::vector
 CaptureMethods(std::string TypeString, const clang::CXXRecordDecl *ASTClass,
const MatchFinder::MatchResult ) {


Index: clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
===
--- clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
+++ clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
@@ -48,7 +48,13 @@
 public:
   ASTSrcLocGenerationAction() : Processor(JsonOutputPath) {}
 
-  ~ASTSrcLocGenerationAction() { Processor.generate(); }
+  void ExecuteAction() override {
+clang::ASTFrontendAction::ExecuteAction();
+if (getCompilerInstance().getDiagnostics().getNumErrors() > 0)
+  Processor.generateEmpty();
+else
+  Processor.generate();
+  }
 
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance ,
Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
@@ -30,6 +30,7 @@
  StringRef File);
 
   void generate();
+  void generateEmpty();
 
 private:
   void run(const ast_matchers::MatchFinder::MatchResult ) override;
Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -79,17 +79,16 @@
   return JsonObj;
 }
 
-void WriteJSON(std::string JsonPath,
-   llvm::StringMap const ,
-   llvm::StringMap> const ,
-   llvm::StringMap const ) {
+void WriteJSON(std::string JsonPath, llvm::json::Object &,
+   llvm::json::Object &,
+   llvm::json::Object &) {
   llvm::json::Object JsonObj;
 
   using llvm::json::toJSON;
 
-  JsonObj["classInheritance"] = ::toJSON(ClassInheritance);
-  JsonObj["classesInClade"] = ::toJSON(ClassesInClade);
-  JsonObj["classEntries"] = 

[PATCH] D98824: [Tooling] Handle compilation databases with clang-cl commands generated by CMake 3.19+

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:181
+  // ...including when the inputs are passed after --
+  if (Opt.matches(OPT__DASH_DASH)) {
+continue;

You can leave out the braces on a one-line statement: 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98824

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Richard Smith - zygoloid 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 rGa875721d8a2d: PR49585: Emit the jump destination for a for 
loop continue from within the… (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Scope.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCXX/for-cond-var.cpp
  clang/test/Parser/cxx2a-init-statement.cpp
  clang/test/SemaCXX/scope-check.cpp

Index: clang/test/SemaCXX/scope-check.cpp
===
--- clang/test/SemaCXX/scope-check.cpp
+++ clang/test/SemaCXX/scope-check.cpp
@@ -629,3 +629,19 @@
 }
 
 } // namespace seh
+
+void continue_scope_check() {
+  // These are OK.
+  for (; ({break; true;});) {}
+  for (; ({continue; true;});) {}
+  for (; int n = ({break; 0;});) {}
+  for (; int n = 0; ({break;})) {}
+  for (; int n = 0; ({continue;})) {}
+
+  // This would jump past the initialization of 'n' to the increment (where 'n'
+  // is in scope).
+  for (; int n = ({continue; 0;});) {} // expected-error {{cannot jump from this continue statement to the loop increment; jump bypasses initialization of loop condition variable}}
+
+  // An intervening loop makes it OK again.
+  for (; int n = ({while (true) continue; 0;});) {}
+}
Index: clang/test/Parser/cxx2a-init-statement.cpp
===
--- clang/test/Parser/cxx2a-init-statement.cpp
+++ clang/test/Parser/cxx2a-init-statement.cpp
@@ -31,4 +31,12 @@
 
   for (int n = 0; static int m = 0; ++n) {} // expected-error {{type name does not allow storage class}}
   for (int n = 0; static int m : arr1) {} // expected-error {{loop variable 'm' may not be declared 'static'}}
+
+  // The init-statement and range are not break / continue scopes. (But the body is.)
+  for (int n = ({ break; 0; }); int m : arr1) {} // expected-error {{not in loop}}
+  for (int n = ({ continue; 0; }); int m : arr1) {} // expected-error {{not in loop}}
+  for (int arr[3]; int n : *({ break;  })) {} // expected-error {{not in loop}}
+  for (int arr[3]; int n : *({ continue;  })) {} // expected-error {{not in loop}}
+  for (int n = 0; int m : arr1) { break; }
+  for (int n = 0; int m : arr1) { continue; }
 }
Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -triple x86_64-linux | FileCheck %s
+
+struct A {
+  A();
+  A(const A &);
+  ~A();
+  operator bool();
+  void *data;
+};
+
+A make();
+bool cond();
+void f(int);
+
+// PR49585: Ensure that 'continue' performs the proper cleanups in the presence
+// of a for loop condition variable.
+//
+// CHECK: define {{.*}} void @_Z7PR49585v(
+void PR49585() {
+  for (
+  // CHECK: call void @_Z1fi(i32 1)
+  // CHECK: br label %[[for_cond:.*]]
+  f(1);
+
+  // CHECK: [[for_cond]]:
+  // CHECK: call {{.*}} @_Z4makev(
+  // CHECK: call {{.*}} @_ZN1AcvbEv(
+  // CHECK: br i1 {{.*}}, label %[[for_body:.*]], label %[[for_cond_cleanup:.*]]
+  A a = make();
+
+  // CHECK: [[for_cond_cleanup]]:
+  // CHECK: store
+  // CHECK: br label %[[cleanup:.*]]
+
+  f(2)) {
+// CHECK: [[for_body]]:
+// CHECK: call {{.*}} @_Z4condv(
+// CHECK: br i1 {{.*}}, label %[[if_then:.*]], label %[[if_end:.*]]
+if (cond()) {
+  // CHECK: [[if_then]]:
+  // CHECK: call {{.*}} @_Z1fi(i32 3)
+  // CHECK: br label %[[for_inc:.*]]
+  f(3);
+  continue;
+}
+
+// CHECK: [[if_end]]:
+// CHECK: call {{.*}} @_Z1fi(i32 4)
+// CHECK: br label %[[for_inc]]
+f(4);
+  }
+
+  // CHECK: [[for_inc]]:
+  // CHECK: call void @_Z1fi(i32 2)
+  // CHECK: store
+  // CHECK: br label %[[cleanup]]
+
+  // CHECK: [[cleanup]]:
+  // CHECK: call void @_ZN1AD1Ev(
+  // CHECK: load
+  // CHECK: switch {{.*}} label
+  // CHECK-NEXT: label %[[cleanup_cont:.*]]
+  // CHECK-NEXT: label %[[for_end:.*]]
+
+  // CHECK: [[cleanup_cont]]:
+  // CHECK: br label %[[for_cond]]
+
+  // CHECK [[for_end]]:
+  // CHECK: ret void
+}
+
+// CHECK: define {{.*}} void @_Z13PR49585_breakv(
+void PR49585_break() {
+  for (
+  // CHECK: call void @_Z1fi(i32 1)
+  // CHECK: br label %[[for_cond:.*]]
+  f(1);
+
+  // CHECK: [[for_cond]]:
+  // CHECK: call {{.*}} @_Z4makev(
+  // CHECK: call {{.*}} @_ZN1AcvbEv(
+  // CHECK: br i1 {{.*}}, label %[[for_body:.*]], label %[[for_cond_cleanup:.*]]
+  A a = make();
+
+  // CHECK: [[for_cond_cleanup]]:
+  // CHECK: store
+  // CHECK: br 

[clang] a875721 - PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-03-17T16:24:04-07:00
New Revision: a875721d8a2dacb894106a2cefa18828bf08f25d

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

LOG: PR49585: Emit the jump destination for a for loop 'continue' from within 
the scope of the condition variable.

The condition variable is in scope in the loop increment, so we need to
emit the jump destination from wthin the scope of the condition
variable.

For GCC compatibility (and compatibility with real-world 'FOR_EACH'
macros), 'continue' is permitted in a statement expression within the
condition of a for loop, though, so there are two cases here:

* If the for loop has no condition variable, we can emit the jump
  destination before emitting the condition.

* If the for loop has a condition variable, we must defer emitting the
  jump destination until after emitting the variable. We diagnose a
  'continue' appearing in the initializer of the condition variable,
  because it would jump past the initializer into the scope of that
  variable.

Reviewed By: rjmccall

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

Added: 
clang/test/CodeGenCXX/for-cond-var.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Scope.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/Parser/cxx2a-init-statement.cpp
clang/test/SemaCXX/scope-check.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7b07a570b104..492ff63fe5ad 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5776,6 +5776,9 @@ def note_goto_ms_asm_label : Note<
 def warn_unused_label : Warning<"unused label %0">,
   InGroup, DefaultIgnore;
 
+def err_continue_from_cond_var_init : Error<
+  "cannot jump from this continue statement to the loop increment; "
+  "jump bypasses initialization of loop condition variable">;
 def err_goto_into_protected_scope : Error<
   "cannot jump from this goto statement to its label">;
 def ext_goto_into_protected_scope : ExtWarn<

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index a9f063d410a0..290b451771a6 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1991,7 +1991,8 @@ class Parser : public CodeCompletionHandler {
   Sema::ConditionResult ParseCXXCondition(StmtResult *InitStmt,
   SourceLocation Loc,
   Sema::ConditionKind CK,
-  ForRangeInfo *FRI = nullptr);
+  ForRangeInfo *FRI = nullptr,
+  bool EnterForConditionScope = false);
 
   
//======//
   // C++ Coroutines

diff  --git a/clang/include/clang/Sema/Scope.h 
b/clang/include/clang/Sema/Scope.h
index b7260f15fe1b..b499ba1e7c2a 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -129,11 +129,17 @@ class Scope {
 /// This is a compound statement scope.
 CompoundStmtScope = 0x40,
 
-/// We are between inheritance colon and the real class/struct definition 
scope.
+/// We are between inheritance colon and the real class/struct definition
+/// scope.
 ClassInheritanceScope = 0x80,
 
 /// This is the scope of a C++ catch statement.
 CatchScope = 0x100,
+
+/// This is a scope in which a condition variable is currently being
+/// parsed. If such a scope is a ContinueScope, it's invalid to jump to the
+/// continue block from here.
+ConditionVarScope = 0x200,
   };
 
 private:
@@ -247,6 +253,17 @@ class Scope {
 return const_cast(this)->getContinueParent();
   }
 
+  // Set whether we're in the scope of a condition variable, where 'continue'
+  // is disallowed despite being a continue scope.
+  void setIsConditionVarScope(bool InConditionVarScope) {
+Flags = (Flags & ~ConditionVarScope) |
+(InConditionVarScope ? ConditionVarScope : 0);
+  }
+
+  bool isConditionVarScope() const {
+return Flags & ConditionVarScope;
+  }
+
   /// getBreakParent - Return the closest scope that a break statement
   /// would be affected by.
   Scope *getBreakParent() {

diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 85306f6882ef..6461e2011216 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -948,8 +948,8 @@ void 

[PATCH] D98815: [OPENMP51]Initial support for the use clause

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc615927c8e38: [OPENMP51]Initial support for the use clause. 
(authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98815

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/interop_ast_print.cpp
  clang/test/OpenMP/interop_messages.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -271,6 +271,9 @@
 def OMPC_Init : Clause<"init"> {
   let clangClass = "OMPInitClause";
 }
+def OMPC_Use : Clause<"use"> {
+  let clangClass = "OMPUseClause";
+}
 def OMPC_Destroy : Clause<"destroy"> {
   let clangClass = "OMPDestroyClause";
 }
@@ -1649,6 +1652,7 @@
 VersionedClause,
 VersionedClause,
 VersionedClause,
+VersionedClause,
   ];
 }
 def OMP_Unknown : Directive<"unknown"> {
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -718,6 +718,7 @@
 CHECK_SIMPLE_CLAUSE(UseDeviceAddr, OMPC_use_device_addr)
 CHECK_SIMPLE_CLAUSE(Write, OMPC_write)
 CHECK_SIMPLE_CLAUSE(Init, OMPC_init)
+CHECK_SIMPLE_CLAUSE(Use, OMPC_use)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Allocator, OMPC_allocator)
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2282,6 +2282,10 @@
   VisitOMPClauseList(C);
 }
 
+void OMPClauseEnqueue::VisitOMPUseClause(const OMPUseClause *C) {
+  Visitor->AddStmt(C->getInteropVar());
+}
+
 void OMPClauseEnqueue::VisitOMPDestroyClause(const OMPDestroyClause *) {}
 
 void OMPClauseEnqueue::VisitOMPUnifiedAddressClause(
Index: clang/test/OpenMP/interop_messages.cpp
===
--- clang/test/OpenMP/interop_messages.cpp
+++ clang/test/OpenMP/interop_messages.cpp
@@ -14,6 +14,9 @@
   //expected-error@+1 {{use of undeclared identifier 'NoDeclVar'}}
   #pragma omp interop init(target:NoDeclVar) init(target:Another)
 
+  //expected-error@+1 {{use of undeclared identifier 'NoDeclVar'}}
+  #pragma omp interop use(NoDeclVar) use(Another)
+
   //expected-error@+2 {{expected interop type: 'target' and/or 'targetsync'}}
   //expected-error@+1 {{expected expression}}
   #pragma omp interop init(InteropVar) init(target:Another)
@@ -32,14 +35,23 @@
   #pragma omp interop init(prefer_type(1,"sycl",3),target:IntVar) \
   init(target:Another)
 
+  //expected-error@+1 {{interop variable must be of type 'omp_interop_t'}}
+  #pragma omp interop use(IntVar) use(Another)
+
   //expected-error@+1 {{interop variable must be of type 'omp_interop_t'}}
   #pragma omp interop init(prefer_type(1,"sycl",3),target:SVar) \
   init(target:Another)
 
+  //expected-error@+1 {{interop variable must be of type 'omp_interop_t'}}
+  #pragma omp interop use(SVar) use(Another)
+
   int a, b;
   //expected-error@+1 {{expected variable of type 'omp_interop_t'}}
   #pragma omp interop init(target:a+b) init(target:Another)
 
+  //expected-error@+1 {{expected variable of type 'omp_interop_t'}}
+  #pragma omp interop use(a+b) use(Another)
+
   const omp_interop_t C = (omp_interop_t)5;
   //expected-error@+1 {{expected non-const variable of type 'omp_interop_t'}}
   #pragma omp interop init(target:C) init(target:Another)
@@ -64,6 +76,12 @@
   //expected-error@+1 {{interop variable 'InteropVar' used in multiple action clauses}}
   #pragma omp interop init(target:InteropVar) init(target:InteropVar)
 
+  //expected-error@+1 {{interop variable 'InteropVar' used in multiple action clauses}}
+  #pragma omp interop use(InteropVar) use(InteropVar)
+
+  //expected-error@+1 {{interop variable 'InteropVar' used in multiple action clauses}}
+  #pragma omp interop init(target:InteropVar) use(InteropVar)
+
   //expected-error@+1 {{directive '#pragma omp interop' cannot contain more than one 'device' clause}}
   #pragma omp interop init(target:InteropVar) device(0) device(1)
 
@@ -79,5 +97,7 @@
   int InteropVar;
   //expected-error@+1 {{'omp_interop_t' 

[clang] c615927 - [OPENMP51]Initial support for the use clause.

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2021-03-17T15:46:14-07:00
New Revision: c615927c8e38d8608d7b5fa294a25dc44df0eb68

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

LOG: [OPENMP51]Initial support for the use clause.

Added basic parsing/sema/serialization support for the 'use' clause.

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

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/OpenMP/interop_ast_print.cpp
clang/test/OpenMP/interop_messages.cpp
clang/tools/libclang/CIndex.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 4e4e719ff184..b342ffb93256 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7489,6 +7489,77 @@ class OMPInitClause final
   }
 };
 
+/// This represents the 'use' clause in '#pragma omp ...' directives.
+///
+/// \code
+/// #pragma omp interop use(obj)
+/// \endcode
+class OMPUseClause final : public OMPClause {
+  friend class OMPClauseReader;
+
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+  /// Location of interop variable.
+  SourceLocation VarLoc;
+
+  /// The interop variable.
+  Stmt *InteropVar = nullptr;
+
+  /// Set the interop variable.
+  void setInteropVar(Expr *E) { InteropVar = E; }
+
+  /// Sets the location of '('.
+  void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+
+  /// Sets the location of the interop variable.
+  void setVarLoc(SourceLocation Loc) { VarLoc = Loc; }
+
+public:
+  /// Build 'use' clause with and interop variable expression \a InteropVar.
+  ///
+  /// \param InteropVar The interop variable.
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param VarLoc Location of the interop variable.
+  /// \param EndLoc Ending location of the clause.
+  OMPUseClause(Expr *InteropVar, SourceLocation StartLoc,
+   SourceLocation LParenLoc, SourceLocation VarLoc,
+   SourceLocation EndLoc)
+  : OMPClause(llvm::omp::OMPC_use, StartLoc, EndLoc), LParenLoc(LParenLoc),
+VarLoc(VarLoc), InteropVar(InteropVar) {}
+
+  /// Build an empty clause.
+  OMPUseClause()
+  : OMPClause(llvm::omp::OMPC_use, SourceLocation(), SourceLocation()) {}
+
+  /// Returns the location of '('.
+  SourceLocation getLParenLoc() const { return LParenLoc; }
+
+  /// Returns the location of the interop variable.
+  SourceLocation getVarLoc() const { return VarLoc; }
+
+  /// Returns the interop variable.
+  Expr *getInteropVar() const { return cast(InteropVar); }
+
+  child_range children() { return child_range(,  + 1); }
+
+  const_child_range children() const {
+return const_child_range(,  + 1);
+  }
+
+  child_range used_children() {
+return child_range(child_iterator(), child_iterator());
+  }
+  const_child_range used_children() const {
+return const_child_range(const_child_iterator(), const_child_iterator());
+  }
+
+  static bool classof(const OMPClause *T) {
+return T->getClauseKind() == llvm::omp::OMPC_use;
+  }
+};
+
 /// This represents 'destroy' clause in the '#pragma omp depobj'
 /// directive.
 ///

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 3983e47d4f77..4a7c234e374b 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -3203,6 +3203,12 @@ bool 
RecursiveASTVisitor::VisitOMPInitClause(OMPInitClause *C) {
   return true;
 }
 
+template 
+bool RecursiveASTVisitor::VisitOMPUseClause(OMPUseClause *C) {
+  TRY_TO(TraverseStmt(C->getInteropVar()));
+  return true;
+}
+
 template 
 bool RecursiveASTVisitor::VisitOMPDestroyClause(OMPDestroyClause *) {
   return true;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 21ea3b492d48..978c5de57646 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10992,6 +10992,11 @@ class Sema final {
SourceLocation VarLoc,
SourceLocation EndLoc);
 
+  /// Called on well-formed 'use' clause.
+  OMPClause *ActOnOpenMPUseClause(Expr *InteropVar, SourceLocation StartLoc,
+  SourceLocation LParenLoc,
+  

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[John McCall via Phabricator via cfe-commits]

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

Well, that's certainly some bug.  Patch LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98824: [Tooling] Handle compilation databases with clang-cl commands generated by CMake 3.19+

[Janusz Nykiel via Phabricator via cfe-commits]

jnykiel created this revision.
jnykiel added a reviewer: aganea.
Herald added subscribers: usaxena95, kadircet, abidh.
jnykiel requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

As of CMake commit https://gitlab.kitware.com/cmake/cmake/-/commit/d993ebd4,
which first appeared in CMake 3.19.x series, in the compile commands for 
clang-cl,
CMake puts `--` before the input file. When operating on such a database, the
`InterpolatingCompilationDatabase` - specifically, the `TransferableCommand`
constructor - does not recognize that pattern and so, does not strip the input,
or the double dash when 'transferring' the compile command. This results in a
incorrect compile command - with the double dash and old input file left in, and
the language options and new input file appended after them, where they're all
treated as inputs, including the language version option.

Test files for some tests have names similar enough to be matched to commands
from the database, e.g.:

`.../path-mappings.test.tmp/server/bar.cpp`

can be matched to:

`.../Driver/ToolChains/BareMetal.cpp`

etc. When that happens, the tool being tested tries to use the matched, and
incorrectly 'transferred' compile command, and fails, reporting errors similar
to:

`error: no such file or directory: '/std:c++14'; did you mean '/std:c++14'? 
[clang-diagnostic-error]`

This happens in at least 4 tests:

  Clang Tools :: clang-tidy/checkers/performance-trivially-destructible.cpp
  Clangd :: check-fail.test
  Clangd :: check.test
  Clangd :: path-mappings.test


The fix assumes that everything after the '--' is an input file and does not
include it in the 'transferred' command.

[1] 
https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#summary-of-whats-new-in-this-release-of-visual-studio-2019-version-1690


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98824

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -812,6 +812,12 @@
   EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
 }
 
+TEST_F(InterpolateTest, StripDoubleDash) {
+  add("dir/foo.cpp", "-Wall -- dir/foo.cpp");
+  // everything after -- is stripped
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
 TEST_F(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,11 @@
Opt.matches(OPT__SLASH_Fo
 continue;
 
+  // ...including when the inputs are passed after --
+  if (Opt.matches(OPT__DASH_DASH)) {
+continue;
+  }
+
   // Strip -x, but record the overridden language.
   if (const auto GivenType = tryParseTypeArg(*Arg)) {
 Type = *GivenType;


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -812,6 +812,12 @@
   EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
 }
 
+TEST_F(InterpolateTest, StripDoubleDash) {
+  add("dir/foo.cpp", "-Wall -- dir/foo.cpp");
+  // everything after -- is stripped
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
 TEST_F(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,11 @@
Opt.matches(OPT__SLASH_Fo
 continue;
 
+  // ...including when the inputs are passed after --
+  if (Opt.matches(OPT__DASH_DASH)) {
+continue;
+  }
+
   // Strip -x, but record the overridden language.
   if (const auto GivenType = tryParseTypeArg(*Arg)) {
 Type = *GivenType;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74361: [Clang] Undef attribute for global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

This appears to have missed a case for openmp. Credit to @jdoerfert for the 
repro: https://godbolt.org/z/xWTYbv

  struct DST {
long X;
long Y;
  };
  
  #pragma omp declare target
   DST G2 [[clang::loader_uninitialized, clang::address_space(5)]];
  #pragma omp end declare target

  error: no matching constructor for initialization of 
'__attribute__((address_space(5))) DST'

There shouldn't be a constructor call there. Noting the limitation here so I 
can find it easily in the near future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

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

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG48ab9674b21b: [ASTMatchers][NFC] Use move semantics when 
passing matchers around. (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98792

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -143,7 +143,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -151,7 +151,7 @@
 return ::clang::ast_matchers::internal::makeMatcher(   \
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param));\
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType const );  \
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
   const Type ,\
@@ -192,8 +192,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -202,7 +202,7 @@
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param1, \
Param2));   \
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType1 const , \
  ParamType2 const );\
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -281,7 +281,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -333,8 +333,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ 

[clang] 48ab967 - [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-03-17T22:03:08Z
New Revision: 48ab9674b21be2c6206ccc04602d4a3e4c812953

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

LOG: [ASTMatchers][NFC] Use move semantics when passing matchers around.

Changing matchers to use non-const members and adding r-value overloads of 
matcher conversions enables move optimisations.
I don't have performance figures but I can say this knocked 120k from the 
clang-tidy binary(86k was from the .text section) on a Release with assertions 
build(x86_64-unknown-linux-gnu).

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchersInternal.h
clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h 
b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 53b37b338a55..e8f427bafa25 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -541,12 +541,18 @@ class Matcher {
   /// Convert \c this into a \c Matcher by applying dyn_cast<> to the
   /// argument.
   /// \c To must be a base class of \c T.
-  template 
-  Matcher dynCastTo() const {
+  template  Matcher dynCastTo() const LLVM_LVALUE_FUNCTION {
 static_assert(std::is_base_of::value, "Invalid dynCast call.");
 return Matcher(Implementation);
   }
 
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  template  Matcher dynCastTo() && {
+static_assert(std::is_base_of::value, "Invalid dynCast call.");
+return Matcher(std::move(Implementation));
+  }
+#endif
+
   /// Forwards the call to the underlying MatcherInterface pointer.
   bool matches(const T ,
ASTMatchFinder *Finder,
@@ -563,7 +569,13 @@ class Matcher {
   ///
   /// The returned matcher keeps the same restrictions as \c this and remembers
   /// that it is meant to support nodes of type \c T.
-  operator DynTypedMatcher() const { return Implementation; }
+  operator DynTypedMatcher() const LLVM_LVALUE_FUNCTION {
+return Implementation;
+  }
+
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  operator DynTypedMatcher() && { return std::move(Implementation); }
+#endif
 
   /// Allows the conversion of a \c Matcher to a \c
   /// Matcher.
@@ -870,7 +882,7 @@ class HasOverloadedOperatorNameMatcher : public 
SingleNodeMatcherInterface {
Names, getOperatorSpelling(Node.getOverloadedOperator()));
   }
 
-  const std::vector Names;
+  std::vector Names;
 };
 
 /// Matches named declarations with a specific name.
@@ -904,8 +916,8 @@ class HasNameMatcher : public 
SingleNodeMatcherInterface {
   /// It is slower but simple and works on all cases.
   bool matchesNodeFullSlow(const NamedDecl ) const;
 
-  const bool UseUnqualifiedMatch;
-  const std::vector Names;
+  bool UseUnqualifiedMatch;
+  std::vector Names;
 };
 
 /// Trampoline function to use VariadicFunction<> to construct a
@@ -926,7 +938,7 @@ class HasDeclarationMatcher : public MatcherInterface {
   static_assert(std::is_same>::value,
 "instantiated with wrong types");
 
-  const DynTypedMatcher InnerMatcher;
+  DynTypedMatcher InnerMatcher;
 
 public:
   explicit HasDeclarationMatcher(const Matcher )
@@ -1315,20 +1327,36 @@ template  class VariadicOperatorMatcher 
{
   VariadicOperatorMatcher(DynTypedMatcher::VariadicOperator Op, Ps &&... 
Params)
   : Op(Op), Params(std::forward(Params)...) {}
 
-  template  operator Matcher() const {
+  template  operator Matcher() const LLVM_LVALUE_FUNCTION {
 return DynTypedMatcher::constructVariadic(
Op, ASTNodeKind::getFromNodeKind(),
getMatchers(std::index_sequence_for()))
 .template unconditionalConvertTo();
   }
 
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  template  operator Matcher() && {
+return DynTypedMatcher::constructVariadic(
+   Op, ASTNodeKind::getFromNodeKind(),
+   getMatchers(std::index_sequence_for()))
+.template unconditionalConvertTo();
+  }
+#endif
 private:
   // Helper method to unpack the tuple into a vector.
   template 
-  std::vector getMatchers(std::index_sequence) const {
+  std::vector
+  getMatchers(std::index_sequence) const LLVM_LVALUE_FUNCTION {
 return {Matcher(std::get(Params))...};
   }
 
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  template 
+  std::vector getMatchers(std::index_sequence) && {
+return {Matcher(std::get(std::move(Params)))...};
+  }
+#endif
+
   const DynTypedMatcher::VariadicOperator Op;
   std::tuple Params;
 };
@@ -1417,12 +1445,18 @@ class ArgumentAdaptingMatcherFuncAdaptor {
 
   using ReturnTypes = ToTypes;
 
-  template  operator Matcher() const {
+  template  operator Matcher() const 

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Note that this fixes a miscompile where we'd destroy a for loop condition 
variable twice when encountering a `continue` (once before the increment and 
again after).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: rjmccall.
rsmith requested review of this revision.
Herald added a project: clang.

The condition variable is in scope in the loop increment, so we need to
emit the jump destination from wthin the scope of the condition
variable.

For GCC compatibility (and compatibility with real-world 'FOR_EACH'
macros), 'continue' is permitted in a statement expression within the
condition of a for loop, though, so there are two cases here:

- If the for loop has no condition variable, we can emit the jump destination 
before emitting the condition.

- If the for loop has a condition variable, we must defer emitting the jump 
destination until after emitting the variable. We diagnose a 'continue' 
appearing in the initializer of the condition variable, because it would jump 
past the initializer into the scope of that variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98816

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Scope.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCXX/for-cond-var.cpp
  clang/test/Parser/cxx2a-init-statement.cpp
  clang/test/SemaCXX/scope-check.cpp

Index: clang/test/SemaCXX/scope-check.cpp
===
--- clang/test/SemaCXX/scope-check.cpp
+++ clang/test/SemaCXX/scope-check.cpp
@@ -629,3 +629,19 @@
 }
 
 } // namespace seh
+
+void continue_scope_check() {
+  // These are OK.
+  for (; ({break; true;});) {}
+  for (; ({continue; true;});) {}
+  for (; int n = ({break; 0;});) {}
+  for (; int n = 0; ({break;})) {}
+  for (; int n = 0; ({continue;})) {}
+
+  // This would jump past the initialization of 'n' to the increment (where 'n'
+  // is in scope).
+  for (; int n = ({continue; 0;});) {} // expected-error {{cannot jump from this continue statement to the loop increment; jump bypasses initialization of loop condition variable}}
+
+  // An intervening loop makes it OK again.
+  for (; int n = ({while (true) continue; 0;});) {}
+}
Index: clang/test/Parser/cxx2a-init-statement.cpp
===
--- clang/test/Parser/cxx2a-init-statement.cpp
+++ clang/test/Parser/cxx2a-init-statement.cpp
@@ -31,4 +31,12 @@
 
   for (int n = 0; static int m = 0; ++n) {} // expected-error {{type name does not allow storage class}}
   for (int n = 0; static int m : arr1) {} // expected-error {{loop variable 'm' may not be declared 'static'}}
+
+  // The init-statement and range are not break / continue scopes. (But the body is.)
+  for (int n = ({ break; 0; }); int m : arr1) {} // expected-error {{not in loop}}
+  for (int n = ({ continue; 0; }); int m : arr1) {} // expected-error {{not in loop}}
+  for (int arr[3]; int n : *({ break;  })) {} // expected-error {{not in loop}}
+  for (int arr[3]; int n : *({ continue;  })) {} // expected-error {{not in loop}}
+  for (int n = 0; int m : arr1) { break; }
+  for (int n = 0; int m : arr1) { continue; }
 }
Index: clang/test/CodeGenCXX/for-cond-var.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/for-cond-var.cpp
@@ -0,0 +1,125 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -triple x86_64-linux | FileCheck %s
+
+struct A {
+  A();
+  A(const A &);
+  ~A();
+  operator bool();
+  void *data;
+};
+
+A make();
+bool cond();
+void f(int);
+
+// PR49585: Ensure that 'continue' performs the proper cleanups in the presence
+// of a for loop condition variable.
+//
+// CHECK: define {{.*}} void @_Z7PR49585v(
+void PR49585() {
+  for (
+  // CHECK: call void @_Z1fi(i32 1)
+  // CHECK: br label %[[for_cond:.*]]
+  f(1);
+
+  // CHECK: [[for_cond]]:
+  // CHECK: call {{.*}} @_Z4makev(
+  // CHECK: call {{.*}} @_ZN1AcvbEv(
+  // CHECK: br i1 {{.*}}, label %[[for_body:.*]], label %[[for_cond_cleanup:.*]]
+  A a = make();
+
+  // CHECK: [[for_cond_cleanup]]:
+  // CHECK: store
+  // CHECK: br label %[[cleanup:.*]]
+
+  f(2)) {
+// CHECK: [[for_body]]:
+// CHECK: call {{.*}} @_Z4condv(
+// CHECK: br i1 {{.*}}, label %[[if_then:.*]], label %[[if_end:.*]]
+if (cond()) {
+  // CHECK: [[if_then]]:
+  // CHECK: call {{.*}} @_Z1fi(i32 3)
+  // CHECK: br label %[[for_inc:.*]]
+  f(3);
+  continue;
+}
+
+// CHECK: [[if_end]]:
+// CHECK: call {{.*}} @_Z1fi(i32 4)
+// CHECK: br label %[[for_inc]]
+f(4);
+  }
+
+  // CHECK: [[for_inc]]:
+  // CHECK: call void @_Z1fi(i32 2)
+  // CHECK: store
+  // CHECK: br label %[[cleanup]]
+
+  // CHECK: [[cleanup]]:
+  // CHECK: call void @_ZN1AD1Ev(
+  // CHECK: load
+  // CHECK: switch {{.*}} label
+  // CHECK-NEXT: label %[[cleanup_cont:.*]]
+  // CHECK-NEXT: label %[[for_end:.*]]
+
+  // CHECK: [[cleanup_cont]]:
+  // CHECK: br label 

[PATCH] D98814: [CUDA][HIP] Mark device var used by host only

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
yaxunl requested review of this revision.

Add device variables to llvm.compiler.used if they are
ODR-used by either host or device functions.

This is necessary to prevent them from being
eliminated by whole-program optimization
where the compiler has no way to know a device
variable is used by some host code.


https://reviews.llvm.org/D98814

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/test/CodeGenCUDA/host-used-device-var.cu


Index: clang/test/CodeGenCUDA/host-used-device-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/host-used-device-var.cu
@@ -0,0 +1,33 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -x hip %s \
+// RUN:   -std=c++11 -O3 -mllvm -amdgpu-internalize-symbols -emit-llvm -o - \
+// RUN:   | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// Check device variables used by neither host nor device functioins are not 
kept.
+
+// CHECK-NOT: @v1
+__device__ int v1;
+
+// CHECK-NOT: @v2
+__constant__ int v2;
+
+// Check device variables used by host functions are kept.
+
+// CHECK: @u1
+__device__ int u1;
+
+// CHECK: @u2
+__constant__ int u2;
+
+// Check device variables with used attribute are always kept.
+
+// CHECK: @u3
+__device__ __attribute__((used)) int u3;
+
+int fun1() {
+  return u1 + u2;
+}
+
+// CHECK: @llvm.compiler.used = {{[^@]*}} @u1 {{[^@]*}} @u2 {{[^@]*}} @u3
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -1084,6 +1084,24 @@
 llvm::Function *CGNVCUDARuntime::finalizeModule() {
   if (CGM.getLangOpts().CUDAIsDevice) {
 transformManagedVars();
+
+// Mark ODR-used device variables as compiler used to prevent it from being
+// eliminated by optimization. This is necessary for device variables
+// ODR-used by host functions. Sema correctly marks them as ODR-used no
+// matter whether they are ODR-used by device or host functions.
+//
+// We do not need to do this if the variable has used attribute since it
+// has already been added.
+for (auto & : DeviceVars) {
+  auto Kind = Info.Flags.getKind();
+  if (!Info.Var->isDeclaration() &&
+  (Kind == DeviceVarFlags::Variable ||
+   Kind == DeviceVarFlags::Surface ||
+   Kind == DeviceVarFlags::Texture) &&
+  Info.D->isUsed() && !Info.D->hasAttr()) {
+CGM.addCompilerUsedGlobal(Info.Var);
+  }
+}
 return nullptr;
   }
   return makeModuleCtorFunction();


Index: clang/test/CodeGenCUDA/host-used-device-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/host-used-device-var.cu
@@ -0,0 +1,33 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -x hip %s \
+// RUN:   -std=c++11 -O3 -mllvm -amdgpu-internalize-symbols -emit-llvm -o - \
+// RUN:   | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// Check device variables used by neither host nor device functioins are not kept.
+
+// CHECK-NOT: @v1
+__device__ int v1;
+
+// CHECK-NOT: @v2
+__constant__ int v2;
+
+// Check device variables used by host functions are kept.
+
+// CHECK: @u1
+__device__ int u1;
+
+// CHECK: @u2
+__constant__ int u2;
+
+// Check device variables with used attribute are always kept.
+
+// CHECK: @u3
+__device__ __attribute__((used)) int u3;
+
+int fun1() {
+  return u1 + u2;
+}
+
+// CHECK: @llvm.compiler.used = {{[^@]*}} @u1 {{[^@]*}} @u2 {{[^@]*}} @u3
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -1084,6 +1084,24 @@
 llvm::Function *CGNVCUDARuntime::finalizeModule() {
   if (CGM.getLangOpts().CUDAIsDevice) {
 transformManagedVars();
+
+// Mark ODR-used device variables as compiler used to prevent it from being
+// eliminated by optimization. This is necessary for device variables
+// ODR-used by host functions. Sema correctly marks them as ODR-used no
+// matter whether they are ODR-used by device or host functions.
+//
+// We do not need to do this if the variable has used attribute since it
+// has already been added.
+for (auto & : DeviceVars) {
+  auto Kind = Info.Flags.getKind();
+  if (!Info.Var->isDeclaration() &&
+  (Kind == DeviceVarFlags::Variable ||
+   Kind == DeviceVarFlags::Surface ||
+   Kind == DeviceVarFlags::Texture) &&
+  Info.D->isUsed() && !Info.D->hasAttr()) {
+CGM.addCompilerUsedGlobal(Info.Var);
+  }
+}
 return nullptr;
   }
   return makeModuleCtorFunction();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D96033#2630978 , @v.g.vassilev 
wrote:

>>> I'm nervous in general about the looming idea of declaration unloading, but 
>>> the fact that it's been working in Cling for a long time gives me some 
>>> confidence that we can do that in a way that's not prohibitively expensive 
>>> and invasive.
>>
>> I don't really know the jargon here.
>
> "Code unloading" is mentioned here 
> https://reviews.llvm.org/D96033?id=323531#inline-911819

I see.  So you want to be able to sort of "roll back" Sema to a previous 
version of the semantic state, and yet you also need IRGen to *not* be rolled 
back because of lazy emission.  That seems... tough.  I don't think we're 
really architected to support that goal — frankly, I'm not sure C and C++ are 
architected to support that goal — and I'm concerned that it might require a 
ton of extra complexity and risk for us.  I say this not to be dismissive, but 
to clarify how I see our various responsibilities here.  Clang's primary 
mission is to be a static compiler, and it's been architected with that in 
mind, and it's acceptable for us to put that mission above other goals if we 
think they're in conflict.  So as I see it, your responsibility here is to 
persuade us of one of the following:

- that you can achieve your goals without introducing major problems for 
Clang's primary mission,
- that the changes you'll need to make will ultimately have substantial 
benefits for Clang's primary mission, or
- that we should change how we think about Clang's primary mission.

We probably need to talk about it.

>> The biggest problem that I foresee around having a full-featured C REPL is 
>> the impossibility of replacing existing types — you might be able to 
>> introduce a *new* struct with a particular name, break the redeclaration 
>> chain, and have it shadow the old one, and we could probably chase down all 
>> the engineering problems that that causes in the compiler, but it's never 
>> going to be a particularly satisfying model.
>
> Indeed. Cling did not have this feature until recently. We indeed "shadow" 
> the reachable declaration maintaining (using inline namespaces) the redecl 
> chain invariants (relevant code here 
> ).
>  I am not sure that approach can work outside C++.
>
>> If we don't have to worry about that, then I feel like the largest problem 
>> is probably the IRGen interaction — in particular, whether we're going to 
>> have to start serializing IRGen state the same way that Sema state has to be 
>> serialized across PCH boundaries.  But I'm sure the people who are working 
>> on this have more knowledge of what issues they're seeing than I can just 
>> abstractly anticipate.
>
> We find ourselves patching often that area in CodeGen when upgrading llvm 
> (eg. here 
> ).
>  The current cling model requires the state to be transferred to subsequent 
> calls of IRGen. We briefly touched that topic in 
> https://reviews.llvm.org/D3#812418 (and onward) and I thought we had a 
> plan how to move forward.

Ah, I sort of remember that conversation.


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

https://reviews.llvm.org/D96033

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

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added a comment.

Before this patch, clang would happily ignore a `-ffree-form` flag. With this 
patch, none of `-Wno-error=unknown-argument`, `-Wno-unknown-argument` , 
`-Qunused-arguments` help to avoid clang from exiting with

  error: unknown argument: '-ffree-form'

Why can't clang ignore these flags as any other unknown flags?

As a background: in the build system I'm dealing with, I cannot avoid that 
fortran flags are passed to the linking command. As C++ and fortran is 
involved, I prefer using clang++ as the linking command and explicitly link the 
fortran runtime library (at the moment gfortran, but in the future probably the 
flang runtime library)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95460

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 331363.
yaxunl added a comment.

bug fix. only remove unused casts.


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

https://reviews.llvm.org/D98783

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/unused-global-var.cu

Index: clang/test/CodeGenCUDA/unused-global-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/unused-global-var.cu
@@ -0,0 +1,50 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -x hip %s \
+// RUN:   -std=c++11 -O3 -mllvm -amdgpu-internalize-symbols -emit-llvm -o - \
+// RUN:   | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// AMDGPU internalize unused global variables for whole-program compilation
+// (-fno-gpu-rdc for each TU, or -fgpu-rdc for LTO), which are then
+// eliminated by global DCE. If there are invisible unused address space casts
+// for global variables, the internalization and elimination of unused global
+// variales will be hindered. This test makes sure no such address space
+// casts.
+
+// Check unused device/constant variables are eliminated.
+
+// CHECK-NOT: @v1
+__device__ int v1;
+
+// CHECK-NOT: @v2
+__constant__ int v2;
+
+// CHECK-NOT: @_ZL2v3
+constexpr int v3 = 1;
+
+// Check managed variables are always kept.
+
+// CHECK: @v4
+__managed__ int v4;
+
+// Check used device/constant variables are not eliminated.
+// CHECK: @u1
+__device__ int u1;
+
+// CHECK: @u2
+__constant__ int u2;
+
+// Check u3 is kept because its address is taken.
+// CHECK: @_ZL2u3
+constexpr int u3 = 2;
+
+// Check u4 is not kept because it is not ODR-use.
+// CHECK-NOT: @_ZL2u4
+constexpr int u4 = 3;
+
+__device__ int fun1(const int& x);
+
+__global__ void kern1(int *x) {
+  *x = u1 + u2 + fun1(u3) + u4;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -371,6 +371,14 @@
   llvm::SmallVector, 8>
 GlobalValReplacements;
 
+  /// Potentially unused address space casts of global variables to be cleaned
+  /// up. In CUDA/HIP, global variables are emitted as global variables in
+  /// device or constant address space which are then casted to default address
+  /// space. If the global variables are not used, the address space casts
+  /// become invisible LLVM constants, causing spurious use of the global
+  /// variables which prevents them from being erased.
+  llvm::DenseSet GlobalVarCasts;
+
   /// Variables for which we've emitted globals containing their constant
   /// values along with the corresponding globals, for opportunistic reuse.
   llvm::DenseMap InitializerConstants;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -768,6 +768,11 @@
   // that might affect the DLL storage class or the visibility, and
   // before anything that might act on these.
   setVisibilityFromDLLStorageClass(LangOpts, getModule());
+
+  // Remove unused address space casts of global variables.
+  for (auto *Cast : GlobalVarCasts)
+if (Cast->use_empty())
+  Cast->destroyConstant();
 }
 
 void CodeGenModule::EmitOpenCLMetadata() {
@@ -3938,9 +3943,14 @@
 : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default);
   assert(getContext().getTargetAddressSpace(ExpectedAS) ==
  Ty->getPointerAddressSpace());
-  if (AddrSpace != ExpectedAS)
-return getTargetCodeGenInfo().performAddrSpaceCast(*this, GV, AddrSpace,
-   ExpectedAS, Ty);
+  if (AddrSpace != ExpectedAS) {
+auto *Cast = getTargetCodeGenInfo().performAddrSpaceCast(
+*this, GV, AddrSpace, ExpectedAS, Ty);
+// Record address space casts of global variables for cleaning up if unused.
+if (Cast != GV)
+  GlobalVarCasts.insert(Cast);
+return Cast;
+  }
 
   return GV;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98812: [OPENMP]Map data field with l-value reference types.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, jyu2, mikerice.
Herald added subscribers: guansong, yaxunl.
ABataev requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.
Herald added projects: clang, OpenMP.

Added initial support dfor the mapping of the data members with l-value
reference types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98812

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_codegen_28.cpp
  clang/test/OpenMP/target_map_codegen_35.cpp
  openmp/libomptarget/test/mapping/data_member_ref.cpp

Index: openmp/libomptarget/test/mapping/data_member_ref.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/data_member_ref.cpp
@@ -0,0 +1,49 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+
+struct View {
+  int Data;
+};
+
+struct Foo {
+  Foo(View ) : VRef(V) {}
+  View 
+};
+
+int main() {
+  View V;
+  V.Data = 123456;
+  Foo Bar(V);
+
+  // CHECK: Host 123456.
+  printf("Host %d.\n", Bar.VRef.Data);
+#pragma omp target map(Bar.VRef)
+  {
+// CHECK: Device 123456.
+printf("Device %d.\n", Bar.VRef.Data);
+V.Data = 654321;
+// CHECK: Device 654321.
+printf("Device %d.\n", Bar.VRef.Data);
+  }
+  // CHECK: Host 654321 654321.
+  printf("Host %d %d.\n", Bar.VRef.Data, V.Data);
+  V.Data = 123456;
+  // CHECK: Host 123456.
+  printf("Host %d.\n", Bar.VRef.Data);
+#pragma omp target map(Bar) map(Bar.VRef)
+  {
+// CHECK: Device 123456.
+printf("Device %d.\n", Bar.VRef.Data);
+V.Data = 654321;
+// CHECK: Device 654321.
+printf("Device %d.\n", Bar.VRef.Data);
+  }
+  // CHECK: Host 654321 654321.
+  printf("Host %d %d.\n", Bar.VRef.Data, V.Data);
+  return 0;
+}
Index: clang/test/OpenMP/target_map_codegen_35.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_map_codegen_35.cpp
@@ -0,0 +1,182 @@
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+///==///
+// RUN: %clang_cc1 -DCK35 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK35 --check-prefix CK35-64
+// RUN: %clang_cc1 -DCK35 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK35 --check-prefix CK35-64
+// RUN: %clang_cc1 -DCK35 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK35 --check-prefix CK35-32
+// RUN: %clang_cc1 -DCK35 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK35 --check-prefix CK35-32
+
+// RUN: %clang_cc1 -DCK35 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK35 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK35 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// RUN: %clang_cc1 -DCK35 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY32 %s
+// SIMD-ONLY32-NOT: {{__kmpc|__tgt}}
+#ifdef CK35
+
+class S {

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:52
 
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {

worth some comments on this.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
   return llvm::any_of(NameList, [](const std::string ) {
-  return llvm::Regex(Name).match(Node.getName());
-});
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());

If we pass the name as a `llvm::StringRef`, we can use `if 
(Name.contains("::"))` which seems a bit clearer to me.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
   return llvm::any_of(NameList, [](const std::string ) {
-  return llvm::Regex(Name).match(Node.getName());
-});
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());

hokein wrote:
> If we pass the name as a `llvm::StringRef`, we can use `if 
> (Name.contains("::"))` which seems a bit clearer to me.
there is a tricky case where the `::` is a prefix of the Name, e.g.  a global 
symbol x, the Name is `::x`, and `getQualifiedNameAsString` of symbol x is `x` 
(without the `::` prefix), then the matcher will not find a match, but we 
expect this is a match right?




Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

ymandel wrote:
> flx wrote:
> > ymandel wrote:
> > > flx wrote:
> > > > ymandel wrote:
> > > > > nit: while this change is not much worse than the existing code, the 
> > > > > matcher really should perform this test for "::", and create the 
> > > > > Regex, for each string just once. Regex construction is relatively 
> > > > > expensive, and matchers of this sort (very generic) are often called 
> > > > > quite frequently.
> > > > > 
> > > > > for that matter, this matcher is now really close to `matchesName` 
> > > > > and would probably be best as a first-class matcher, rather than 
> > > > > limited to clang-tidy/utils. But, I'll admit that such is beyond the 
> > > > > scope of this patch.
> > > > It wouldn't be hard to change it something like this:
> > > > 
> > > > 
> > > > ```
> > > > struct MatcherPair {
> > > >
> > > > llvm::Regex regex;  
> > > >  
> > > > bool matchQualifiedName = false;
> > > >  
> > > >   };
> > > >  
> > > >   std::vector Matchers;
> > > >  
> > > >   std::transform(   
> > > >  
> > > >   NameList.begin(), NameList.end(), std::back_inserter(Matchers),   
> > > >  
> > > >   [](const std::string& Name) { 
> > > >  
> > > > return MatcherPair{ 
> > > >  
> > > > .regex = llvm::Regex(Name), 
> > > >  
> > > > .matchQualifiedName = Name.find("::") != std::string::npos, 
> > > >  
> > > > };  
> > > >  
> > > >   });   
> > > >  
> > > >   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) {   
> > > >  
> > > > if (Matcher.matchQualifiedName) {   
> > > >  
> > > >   return Matcher.regex.match(Node.getQualifiedNameAsString());  
> > > >  
> > > > }   
> > > >  
> > > > return Matcher.regex.match(Node.getName()); 
> > > > 

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via Phabricator via cfe-commits]

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

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98792

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -143,7 +143,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -151,7 +151,7 @@
 return ::clang::ast_matchers::internal::makeMatcher(   \
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param));\
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType const );  \
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
   const Type ,\
@@ -192,8 +192,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -202,7 +202,7 @@
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param1, \
Param2));   \
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType1 const , \
  ParamType2 const );\
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -281,7 +281,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -333,8 +333,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -469,7 +469,7 @@
  *Builder) const override; \

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1564
 public:
-  PolymorphicMatcher(const ParamTypes &... Params) : Params(Params...) {}
+  PolymorphicMatcher(const ParamTypes &...Params) : Params(Params...) {}
 

aaron.ballman wrote:
> This change seems unintentional?
I think it's a clang-format being a little eager, not sure though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98792

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

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[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 a small nit.




Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1564
 public:
-  PolymorphicMatcher(const ParamTypes &... Params) : Params(Params...) {}
+  PolymorphicMatcher(const ParamTypes &...Params) : Params(Params...) {}
 

This change seems unintentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98792

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

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Pinging @alexfh for opinions about the check (especially any concerns about 
true or false positive rates). I continue to think this is a good check that's 
worth moving forward on.




Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:31-32
+
+  char DissimilarBelow;
+  char SimilarAbove;
+

`signed char` since we're doing `> -1` below? Or better yet, `int8_t` because 
these aren't really characters?



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:51
+{true, 40, 50}, // Substring.
+{true, 50, 66}, // Levenshtein.
+{true, 75, 85}, // Jaro-Winkler.

We should probably document where all these numbers come from, but `66` 
definitely jumps out at me as being a bit strange. :-D



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:93
+/// Returns how many % X is of Y.
+static inline double percentage(double X, double Y) { return X / Y * 100; }
+





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:102-108
+  if (AbbreviationDictionary.find(Arg) != AbbreviationDictionary.end())
+if (Param.equals(AbbreviationDictionary.lookup(Arg)))
+  return true;
+
+  if (AbbreviationDictionary.find(Param) != AbbreviationDictionary.end())
+if (Arg.equals(AbbreviationDictionary.lookup(Param)))
+  return true;





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:271-272
+
+// Checks whether ArgType is an array type identical to ParamType`s array type.
+// Enforces array elements` qualifier compatibility as well.
+static bool isCompatibleWithArrayReference(const QualType ,





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:296
+
+// Checks if multilevel pointers` qualifiers compatibility continues on the
+// current pointer level.





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:301-302
+// and if PramType has a cv-qualifier that's not in ArgType, then every * in
+// ParamType to the right
+// of that cv-qualifier, except the last one, must also be const-qualified.
+static bool arePointersStillQualCompatible(QualType ArgType, QualType 
ParamType,

Can re-flow the comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:341
+
+  } while (ParamType->isAnyPointerType() && ArgType->isAnyPointerType());
+  // The final type does not match, or pointer levels differ.

Elsewhere we're using `isPointerType()` which is subtly different because it 
excludes ObjC object pointers. We should be consistent about the usage.



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:347
+// Checks whether ArgType converts implicitly to ParamType.
+static bool areTypesCompatible(QualType ArgType, QualType ParamType,
+   const ASTContext ) {

It seems like we're doing an awful lot of the same work as 
`ASTContext::typesAreCompatible()` and type compatibility rules are pretty 
complex, so I worry about this implementation being different than the 
`ASTContext` implementation. Have you explored whether we can reuse more of the 
logic from `ASTContext` here, or are they doing fundamentally different kinds 
of type compatibility checks?



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:462
+: ClangTidyCheck(Name, Context) {
+  const auto  = [this](Heuristic H) -> bool {
+auto Idx = static_cast(H);





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:467
+  };
+  const auto  = [this](Heuristic H, Bound B) -> char {
+auto Idx = static_cast(H);





Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.h:38
+  enum class Bound {
+/// The percentage below which the heuristic deems the strings
+/// dissimilar.

The comment is somewhat confusing because the enumerators have the values 0 and 
1, which are valid percentages but not likely what the comment means.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20689

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

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D97915#2632493 , @lxfind wrote:

> I am not sure how this would work, maybe I am missing something.
> But this patch tries to round up the frame pointer by looking at the 
> difference between the alignment of new and the alignment of the frame.
> The alignment of new only gives you the guaranteed alignment for new, but not 
> necessarily the maximum alignment, e.g. if the alignment of new is 16, the 
> returned pointer can still be a multiple 32. And that difference matters.
>
> Let's consider a frame that only has the two pointers and a promise with 
> alignment requirement of 64. The alignment of new is 16.
> Now you will calculate the difference to be 48, and create a padding of 48 
> before the frame:
> But if the returned pointer from new is actually a multiple of 32 (but not 
> 64), the frame will no longer be aligned to 64 (but (32 + 48) % 64 = 16).

48 is the maximal possible adjustment needed. For this particular case, 
`EmitBuiltinAlignTo` would make the real adjustment 32 since (32 + 32) % 64 == 
0.

> So from what I can tell, if we cannot pass alignment to new, we need to look 
> at the address returned by new dynamically to decide the padding.

Indeed, that's what `EmitBuiltinAlignTo` is for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Anton Korobeynikov via Phabricator via cfe-commits]

asl accepted this revision.
asl 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/D98493/new/

https://reviews.llvm.org/D98493

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

[PATCH] D98191: [flang][driver] Add support for `-fdebug-dump-symbols-sources`

[Kiran Chandramohan via Phabricator via cfe-commits]

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



Comment at: flang/include/flang/Frontend/FrontendOptions.h:60
+
+  /// Parse, run semantics and the dump symbol sources map
+  GetSymbolsSources

Nit: the -> then


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98191

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

[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

[Craig Topper via Phabricator via cfe-commits]

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

LGTM other than that one comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:582
+  StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ,
+ CLANG_DEFAULT_LINKER);
+  if (Linker.empty())

asl wrote:
> How is CLANG_DEFAULT_LINKER initialized by default? Is it just empty?
Yes, by default it's empty.
The definition is at 
https://github.com/llvm/llvm-project/blob/d9ef6bc42643ae4feab3f9eca97864d72034f2ce/clang/CMakeLists.txt#L241
 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:700
+for (auto Idx : CTypeOrder) {
+  if (Seen.count(Idx))
+PrintFatalError(

You can use

```
if (!Seen.insert(Idx).second)
  PrintFatalError
```

This avoids walking the set twice.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Anton Korobeynikov via Phabricator via cfe-commits]

asl added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:582
+  StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ,
+ CLANG_DEFAULT_LINKER);
+  if (Linker.empty())

How is CLANG_DEFAULT_LINKER initialized by default? Is it just empty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via Phabricator via cfe-commits]

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

Fix PP guards erroneously using #ifdef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98792

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -143,7 +143,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -151,7 +151,7 @@
 return ::clang::ast_matchers::internal::makeMatcher(   \
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param));\
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType const );  \
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
   const Type ,\
@@ -192,8 +192,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -202,7 +202,7 @@
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param1, \
Param2));   \
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType1 const , \
  ParamType2 const );\
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -281,7 +281,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -333,8 +333,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -469,7 +469,7 @@
  *Builder) const override;   

[PATCH] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D96843/new/

https://reviews.llvm.org/D96843

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

[PATCH] D98494: [NFC] Minor cleanup to use default setting of getLastArg()

[Anton Korobeynikov via Phabricator via cfe-commits]

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

Looks trivial enough for post-commit review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98494

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

[PATCH] D93347: [Test] Fix undef var in attr-speculative-load-hardening.c

[Thomas Preud'homme 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 rG2426b1fa66f9: [Test] Fix undef var in 
attr-speculative-load-hardening.c (authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93347

Files:
  clang/test/CodeGen/attr-speculative-load-hardening.c


Index: clang/test/CodeGen/attr-speculative-load-hardening.c
===
--- clang/test/CodeGen/attr-speculative-load-hardening.c
+++ clang/test/CodeGen/attr-speculative-load-hardening.c
@@ -12,4 +12,4 @@
 
 // NOSLH: @{{.*}}test1{{.*}}[[NOSLH:#[0-9]+]]
 
-// NOSLH-NOT: attributes [[SLH]] = { {{.*}}speculative_load_hardening{{.*}} }
+// NOSLH-NOT: attributes [[NOSLH]] = { {{.*}}speculative_load_hardening{{.*}} }


Index: clang/test/CodeGen/attr-speculative-load-hardening.c
===
--- clang/test/CodeGen/attr-speculative-load-hardening.c
+++ clang/test/CodeGen/attr-speculative-load-hardening.c
@@ -12,4 +12,4 @@
 
 // NOSLH: @{{.*}}test1{{.*}}[[NOSLH:#[0-9]+]]
 
-// NOSLH-NOT: attributes [[SLH]] = { {{.*}}speculative_load_hardening{{.*}} }
+// NOSLH-NOT: attributes [[NOSLH]] = { {{.*}}speculative_load_hardening{{.*}} }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2426b1f - [Test] Fix undef var in attr-speculative-load-hardening.c

[Thomas Preud'homme via cfe-commits]

Author: Thomas Preud'homme
Date: 2021-03-17T19:12:25Z
New Revision: 2426b1fa66f95d2b6b874e422edceccdf6431162

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

LOG: [Test] Fix undef var in attr-speculative-load-hardening.c

Fix use of undefined variable in CHECK-NOT directive in clang test
CodeGen/attr-speculative-load-hardening.c.

Reviewed By: kristof.beyls

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

Added: 


Modified: 
clang/test/CodeGen/attr-speculative-load-hardening.c

Removed: 




diff  --git a/clang/test/CodeGen/attr-speculative-load-hardening.c 
b/clang/test/CodeGen/attr-speculative-load-hardening.c
index 97bccd03585c..784640f93932 100644
--- a/clang/test/CodeGen/attr-speculative-load-hardening.c
+++ b/clang/test/CodeGen/attr-speculative-load-hardening.c
@@ -12,4 +12,4 @@ int test1() {
 
 // NOSLH: @{{.*}}test1{{.*}}[[NOSLH:#[0-9]+]]
 
-// NOSLH-NOT: attributes [[SLH]] = { {{.*}}speculative_load_hardening{{.*}} }
+// NOSLH-NOT: attributes [[NOSLH]] = { {{.*}}speculative_load_hardening{{.*}} }



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

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

I am not sure how this would work, maybe I am missing something.
But this patch tries to round up the frame pointer by looking at the difference 
between the alignment of new and the alignment of the frame.
The alignment of new only gives you the guaranteed alignment for new, but not 
necessarily the maximum alignment, e.g. if the alignment of new is 16, the 
returned pointer can still be a multiple 32. And that difference matters.

Let's consider a frame that only has the two pointers and a promise with 
alignment requirement of 64. The alignment of new is 16.
Now you will calculate the difference to be 48, and create a padding of 48 
before the frame:
But if the returned pointer from new is actually a multiple of 32 (but not 64), 
the frame will no longer be aligned to 64 (but (32 + 48) % 64 = 16).
So from what I can tell, if we cannot pass alignment to new, we need to look at 
the address returned by new dynamically to decide the padding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97915

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

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

C functions may be declared and defined in different prototypes like below. 
This patch unifies the checks for mangling names in symbol linkage name 
emission and debug linkage name emission so that the two names are consistent.

static int go(int);

static int go(a) int a;
{

  return a;

}

Test Plan:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98799

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/unique-internal-linkage-names-dwarf.c


Index: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
===
--- clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
+++ clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
@@ -8,21 +8,48 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -debug-info-kind=limited 
-dwarf-version=5 -funique-internal-linkage-names -emit-llvm -o -  %s | 
FileCheck %s --check-prefix=UNIQUE
 
 static int glob;
+// foo should be given a uniquefied name under -funique-internal-linkage-names.
 static int foo(void) {
   return glob;
 }
 
+// bar should not be given a uniquefied name under 
-funique-internal-linkage-names, 
+// since it doesn't come with valid prototype.
+static int bar(a) int a;
+{
+  return glob + a;
+}
+
+// go should be given a uniquefied name under -funique-internal-linkage-names, 
even 
+// if its definition doesn't come with a valid prototype, but the declaration 
here
+// has a prototype.
+static int go(int);
+
 void baz() {
   foo();
+  bar(1);
+  go(2);
 }
 
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+
 // PLAIN: @glob = internal global i32
 // PLAIN: define internal i32 @foo()
+// PLAIN: define internal i32 @bar(i32 %a)
 // PLAIN: distinct !DIGlobalVariable(name: "glob"{{.*}})
 // PLAIN: distinct !DISubprogram(name: "foo"{{.*}})
+// PLAIN: distinct !DISubprogram(name: "bar"{{.*}})
+// PLAIN: distinct !DISubprogram(name: "go"{{.*}})
 // PLAIN-NOT: linkageName:
 //
 // UNIQUE: @glob = internal global i32
 // UNIQUE: define internal i32 @_ZL3foov.[[MODHASH:__uniq.[0-9]+]]()
+// UNIQUE: define internal i32 @bar(i32 %a)
+// UNIQUE: define internal i32 @_ZL2goi.[[MODHASH]](i32 %a)
 // UNIQUE: distinct !DIGlobalVariable(name: "glob"{{.*}})
 // UNIQUE: distinct !DISubprogram(name: "foo", linkageName: 
"_ZL3foov.[[MODHASH]]"{{.*}})
+// UNIQUE: distinct !DISubprogram(name: "go", linkageName: 
"_ZL2goi.[[MODHASH]]"{{.*}})
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3522,7 +3522,8 @@
llvm::DIScope *,
llvm::DINodeArray ,
llvm::DINode::DIFlags ) {
-  const auto *FD = cast(GD.getDecl());
+  GlobalDecl CanonicalGD = GD.getCanonicalDecl();
+  const auto *FD = cast(CanonicalGD.getDecl());
   Name = getFunctionName(FD);
   // Use mangled name as linkage name for C/C++ functions.
   if (FD->hasPrototype()) {
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -640,7 +640,7 @@
 
   // For C functions without prototypes, return false as their
   // names should not be mangled.
-  if (!FD->getType()->getAs())
+  if (!FD->hasPrototype())
 return false;
 
   if (isInternalLinkageDecl(ND))


Index: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
===
--- clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
+++ clang/test/CodeGen/unique-internal-linkage-names-dwarf.c
@@ -8,21 +8,48 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -debug-info-kind=limited -dwarf-version=5 -funique-internal-linkage-names -emit-llvm -o -  %s | FileCheck %s --check-prefix=UNIQUE
 
 static int glob;
+// foo should be given a uniquefied name under -funique-internal-linkage-names.
 static int foo(void) {
   return glob;
 }
 
+// bar should not be given a uniquefied name under -funique-internal-linkage-names, 
+// since it doesn't come with valid prototype.
+static int bar(a) int a;
+{
+  return glob + a;
+}
+
+// go should be given a uniquefied name under -funique-internal-linkage-names, even 
+// if its definition doesn't come with a valid prototype, but the declaration here
+// has a prototype.
+static int go(int);
+
 void baz() {
   foo();
+  bar(1);
+  go(2);
 }
 
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+
 // PLAIN: @glob = internal global i32
 // PLAIN: define internal i32 @foo()
+// PLAIN: define internal i32 @bar(i32 %a)
 // PLAIN: distinct !DIGlobalVariable(name: "glob"{{.*}})
 // 

[PATCH] D97371: [clang][parser] Remove questionable ProhibitAttributes() call in objc parsing

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D97371#2596643 , @tbaeder wrote:

> They are being applied to the `@try` at least (`-ast-print` prints them) and 
> we do some error checking for missing call expressions in 
> `handleNoMergeAttr()` in `SemaStmtAttr.cpp`. I don't know much about 
> Objective C so I am not sure how to check that the attribute really has any 
> effect in the end.

It's just like `try`/`catch`/`finally` in languages like Java, where you can 
put arbitrary statements inside the nested blocks.  The attribute should affect 
all child statements.  Just find an existing `nomerge` test case that checks 
that IRGen is affected, put that code inside a `@try {  } 
@catch(...) {}`, and verify that the IRGen for that code is still affected.  
Maybe also verify that it works if you put it inside the `@catch` or `@finally` 
block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97371

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

[PATCH] D98798: Produce waring for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser added a comment.

I used formatting similar to the existing code, which is not what clang-format 
is expecting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98798

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

[PATCH] D98798: Produce waring for performing pointer arithmetic on a null pointer.

[Jamie Schmeiser via Phabricator via cfe-commits]

jamieschmeiser created this revision.
jamieschmeiser added a reviewer: andrew.w.kaylor.
jamieschmeiser requested review of this revision.
Herald added a project: clang.

Test and produce warning for subtracting a pointer from null or subtracting 
null from a pointer.  Reuse existing warning that this is undefined behaviour.  
Also add unit test for both warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98798

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-addition.c


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a 
null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on 
a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer 
arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this


Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -29,4 +29,6 @@
   // Cases that don't match the GNU inttoptr idiom get a different warning.
   f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
   int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10720,7 +10720,15 @@
LHS.get(), RHS.get()))
 return QualType();
 
-  // FIXME: Add warnings for nullptr - ptr.
+  // Subtracting from a null pointer should produce a warning.
+  if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
+  // Subtracting a null pointer should produce a warning.
+  if (RHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNotNull))
+diagnoseArithmeticOnNullPointer(*this, Loc, RHS.get(), false);
 
   // The pointee type may have zero size.  As an extension, a structure or
   // union may have zero size or an array may have zero length.  In this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98520: [OpenCL] Remove spurious atomic_fetch tablegen builtins

[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/D98520/new/

https://reviews.llvm.org/D98520

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

[PATCH] D93347: [Test] Fix undef var in attr-speculative-load-hardening.c

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls accepted this revision.
kristof.beyls 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/D93347/new/

https://reviews.llvm.org/D93347

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

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Thanks, I have changed it to '.clcpp' and also replied in RFC: 
https://lists.llvm.org/pipermail/cfe-dev/2021-March/067936.html

Let's wait a few days for some more feedback.


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

https://reviews.llvm.org/D96771

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

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 331317.
Anastasia added a comment.

Back to .clcpp!


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

https://reviews.llvm.org/D96771

Files:
  clang/include/clang/Basic/LangStandard.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/FrontendOptions.cpp
  clang/test/CodeGenOpenCLCXX/address-space-deduction.cl
  clang/test/CodeGenOpenCLCXX/address-space-deduction.clcpp
  clang/test/CodeGenOpenCLCXX/address-space-deduction2.cl
  clang/test/CodeGenOpenCLCXX/address-space-deduction2.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-conversion.cl
  clang/test/CodeGenOpenCLCXX/addrspace-conversion.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
  clang/test/CodeGenOpenCLCXX/addrspace-derived-base.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-new-delete.cl
  clang/test/CodeGenOpenCLCXX/addrspace-new-delete.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-of-this.cl
  clang/test/CodeGenOpenCLCXX/addrspace-of-this.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-operators.cl
  clang/test/CodeGenOpenCLCXX/addrspace-operators.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-references.cl
  clang/test/CodeGenOpenCLCXX/addrspace-references.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl
  clang/test/CodeGenOpenCLCXX/addrspace-with-class.clcpp
  clang/test/CodeGenOpenCLCXX/addrspace_cast.cl
  clang/test/CodeGenOpenCLCXX/addrspace_cast.clcpp
  clang/test/CodeGenOpenCLCXX/atexit.cl
  clang/test/CodeGenOpenCLCXX/atexit.clcpp
  clang/test/CodeGenOpenCLCXX/constexpr.cl
  clang/test/CodeGenOpenCLCXX/constexpr.clcpp
  clang/test/CodeGenOpenCLCXX/global_init.cl
  clang/test/CodeGenOpenCLCXX/global_init.clcpp
  clang/test/CodeGenOpenCLCXX/local_addrspace_init.cl
  clang/test/CodeGenOpenCLCXX/local_addrspace_init.clcpp
  clang/test/CodeGenOpenCLCXX/method-overload-address-space.cl
  clang/test/CodeGenOpenCLCXX/method-overload-address-space.clcpp
  clang/test/CodeGenOpenCLCXX/template-address-spaces.cl
  clang/test/CodeGenOpenCLCXX/template-address-spaces.clcpp
  clang/test/Driver/cxx_for_opencl.clcpp
  clang/test/Driver/lit.local.cfg
  clang/test/SemaOpenCLCXX/address-space-castoperators.cl
  clang/test/SemaOpenCLCXX/address-space-castoperators.clcpp
  clang/test/SemaOpenCLCXX/address-space-cond.cl
  clang/test/SemaOpenCLCXX/address-space-cond.clcpp
  clang/test/SemaOpenCLCXX/address-space-deduction.cl
  clang/test/SemaOpenCLCXX/address-space-deduction.clcpp
  clang/test/SemaOpenCLCXX/address-space-lambda.cl
  clang/test/SemaOpenCLCXX/address-space-lambda.clcpp
  clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
  clang/test/SemaOpenCLCXX/address-space-of-this-class-scope.clcpp
  clang/test/SemaOpenCLCXX/address-space-of-this.cl
  clang/test/SemaOpenCLCXX/address-space-of-this.clcpp
  clang/test/SemaOpenCLCXX/address-space-references.cl
  clang/test/SemaOpenCLCXX/address-space-references.clcpp
  clang/test/SemaOpenCLCXX/address-space-templates.cl
  clang/test/SemaOpenCLCXX/address-space-templates.clcpp
  clang/test/SemaOpenCLCXX/address_space_overloading.cl
  clang/test/SemaOpenCLCXX/address_space_overloading.clcpp
  clang/test/SemaOpenCLCXX/addrspace-auto.cl
  clang/test/SemaOpenCLCXX/addrspace-auto.clcpp
  clang/test/SemaOpenCLCXX/addrspace_cast.cl
  clang/test/SemaOpenCLCXX/addrspace_cast.clcpp
  clang/test/SemaOpenCLCXX/addrspace_cast_ast_dump.cl
  clang/test/SemaOpenCLCXX/addrspace_cast_ast_dump.clcpp
  clang/test/SemaOpenCLCXX/invalid-kernel.cl
  clang/test/SemaOpenCLCXX/invalid-kernel.clcpp
  clang/test/SemaOpenCLCXX/members.cl
  clang/test/SemaOpenCLCXX/members.clcpp
  clang/test/SemaOpenCLCXX/method-overload-address-space.cl
  clang/test/SemaOpenCLCXX/method-overload-address-space.clcpp
  clang/test/SemaOpenCLCXX/newdelete.cl
  clang/test/SemaOpenCLCXX/newdelete.clcpp
  clang/test/SemaOpenCLCXX/references.cl
  clang/test/SemaOpenCLCXX/references.clcpp
  clang/test/SemaOpenCLCXX/restricted.cl
  clang/test/SemaOpenCLCXX/restricted.clcpp
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -26,7 +26,7 @@
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.c', '.cpp', '.i', '.cppm', '.m', '.mm', '.cu',
-   '.ll', '.cl', '.s', '.S', '.modulemap', '.test', '.rs', '.ifs']
+   '.ll', '.cl', '.clcpp', '.s', '.S', '.modulemap', '.test', '.rs', '.ifs']
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
Index: clang/test/SemaOpenCLCXX/restricted.clcpp
===
--- clang/test/SemaOpenCLCXX/restricted.clcpp
+++ clang/test/SemaOpenCLCXX/restricted.clcpp
@@ -1,4 +1,4 @@
-// RUN: 

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D98783#2632143 , @tra wrote:

> Shouldn't the unused ASCs be eliminated by DCE? We seem to be cleaning up the 
> consequences of a problem that happened somewhere else.

DCE does not eliminate these unused ASCs because normally they should not 
exist. That's why we do not want them to be kept in the IR emitted by clang.


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

https://reviews.llvm.org/D98783

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D98783#2632153 , @tra wrote:

> I do not see any hanging ASCs in the generated IR with clang@HEAD: 
> https://godbolt.org/z/TE4Yhr
>
> What do I miss?

The unused casts are invisible since they are not used by any visible IR's. To 
see them, you need to dump the users of these global variables explicitly.


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

https://reviews.llvm.org/D98783

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

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

[Stephen Kelly via Phabricator via cfe-commits]

steveire added reviewers: steveire, njames93.
steveire added a comment.

Could you rebase this? Parts of it were common with another patch which was 
merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69218

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit added a comment.

I've found yet another bug. When `AllowShortEnumsOnASingleLine` is `true` and a 
multiline enum is forced (by line length, trailing comma, etc.), multiple names 
for the enum are placed on separate lines.

Example:

  enum { A, B, C, } ShortEnum1, ShortEnum2;

Is refactored to

  enum {
  A,
  B,
  C,
  } ShortEnum1,
  ShortEnum2;

Instead of the expected

  enum {
  A,
  B,
  C,
  } ShortEnum1, ShortEnum2;

`ColumnLimit` is not causing this.
When `AllowShortEnumsOnASingleLine` is `false`, the expected behaviour occurs. 
This affects a unit test I added, so I'll be fixing this as well in this diff.




Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680
+if (remainingFile[whileIndex] != '\n' &&
+(remainingFile[whileIndex] == ' ' &&
+ remainingFile[whileIndex - 1] == ' ')) {
+  remainingLineCharCount++;

curdeius wrote:
> atirit wrote:
> > curdeius wrote:
> > > I don't really understand these conditions on spaces. Could you explain 
> > > your intent, please?
> > > You really need to add specific tests for that, playing with the value of 
> > > ColumnLimit, adding spacing etc.
> > Repeated spaces, e.g. `enum {   A,  B, C } SomeEnum;` are removed during 
> > formatting. Since they wouldn't be part of the formatted line, they 
> > shouldn't be counted towards the column limit. Only one space need be 
> > considered. Removed spaces, e.g. `enum{A,B,C}SomeEnum;` are handled by the 
> > fact that `clang-format` runs multiple passes. On the first pass, spaces 
> > would be added. On the second pass, assuming the line is then too long, the 
> > above code would catch it and break up the enum.
> > 
> > I'll add unit tests to check if spaces are being handled correctly.
> Since you use `== ' '` twice, `remainingLineCharCount` will count only 
> consecutive spaces, right?
> But you want to count other characters, no?
> So, IIUC, the condition you want is `rF[wI] != '\n' && !(rF[wI] == ' ' && 
> rF[wI - 1] == ' ')` (mind the negation). It will count characters other than 
> a newline and it will only count a series of consecutive spaces as one char. 
> WDYT?
Ah yes, that's my bad. Must have made a typo. Fixed in the next commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I do not see any hanging ASCs in the generated IR with clang@HEAD: 
https://godbolt.org/z/TE4Yhr

What do I miss?


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

https://reviews.llvm.org/D98783

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

[PATCH] D98774: [AST] De-duplicate empty node introspection

[Stephen Kelly via Phabricator via cfe-commits]

steveire marked 4 inline comments as done.
steveire added a comment.

In D98774#2631898 , @thakis wrote:

> Why were there two copies in the first place? What was the copy in the cmake 
> file good for, why wasn't the one written by generate_cxx_src_locs.py 
> sufficient?

AFAIK python is not a hard-requirement of the llvm/clang build.

> But pulling it into its own file seems nicer than having it inline in python 
> or cmake code.

Yes.

> Another observation: it's a bit strange that the empty impl codepath doesn't 
> share any code with the non-empty code path. The py script could do something 
> like
>
>   if use_empty_implementation:
> jsonData = { classesInClade': [] 'classNames': [] }
>
> and then just fall through to the normal generation code and everything 
> should in theory work with much fewer special cases, right?

I don't think this cuts down on special cases. Also, it looks like it would 
generate code with `using`s which are never used

  using LocationAndString = SourceLocationMap::value_type;
  using RangeAndString = SourceRangeMap::value_type;

which would lead us to having to handle "unused using" warnings.

Copying the file seems a good compromise between the options.

> (What's a "clade"?)

The group of types which have a common ancestor. Everything inheriting from 
`Stmt` is in a different clade to everything inheriting from `Decl`.




Comment at: clang/lib/Tooling/CMakeLists.txt:38
+  ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
+  COPYONLY
 )

thakis wrote:
> Is configure_file(COYPONLY) different from file(COPY)? In what way?
> 
> Are you expecting to use cmake vars in the .in file eventually?
`configure_file` only copies the file if it is different. `file(COPY)` does 
too, but it doesn't seem to include a way to specify the destination file name.



Comment at: clang/lib/Tooling/CMakeLists.txt:96
+--empty-implementation
+  "${CMAKE_CURRENT_SOURCE_DIR}/EmptyNodeIntrospection.inc.in"
   COMMAND

thakis wrote:
> What's the advantage of making a copy above? Why not call the checked-in file 
> `EmptyNodeIntrospection.inc` and pass the path to it directly here?
I'm not sure what you mean. The `configure_file` is inside the `if` and this 
line is inside the `else`. Does that clear it up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98774

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Shouldn't the unused ASCs be eliminated by DCE? We seem to be cleaning up the 
consequences of a problem that happened somewhere else.


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

https://reviews.llvm.org/D98783

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

[PATCH] D98707: [clang][ASTImporter] Fix import of VarDecl regarding thread local storage spec

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

steakhal updated this revision to Diff 331302.
steakhal added a comment.

Revert to the previous version and move to line 738 just before 
"ImportRecordTypeInFunc".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98707

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -735,6 +735,12 @@
  has(declStmt(hasSingleDecl(varDecl(hasName("d");
 }
 
+TEST_P(ImportDecl, ImportedVarDeclPreservesThreadLocalStorage) {
+  MatchVerifier Verifier;
+  testImport("thread_local int declToImport;", Lang_CXX11, "", Lang_CXX11,
+ Verifier, varDecl(hasThreadStorageDuration()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportRecordTypeInFunc) {
   Decl *FromTU = getTuDecl("int declToImport() { "
"  struct data_t {int a;int b;};"
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4018,6 +4018,7 @@
   D->getStorageClass()))
 return ToVar;
 
+  ToVar->setTSCSpec(D->getTSCSpec());
   ToVar->setQualifierInfo(ToQualifierLoc);
   ToVar->setAccess(D->getAccess());
   ToVar->setLexicalDeclContext(LexicalDC);


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -735,6 +735,12 @@
  has(declStmt(hasSingleDecl(varDecl(hasName("d");
 }
 
+TEST_P(ImportDecl, ImportedVarDeclPreservesThreadLocalStorage) {
+  MatchVerifier Verifier;
+  testImport("thread_local int declToImport;", Lang_CXX11, "", Lang_CXX11,
+ Verifier, varDecl(hasThreadStorageDuration()));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportRecordTypeInFunc) {
   Decl *FromTU = getTuDecl("int declToImport() { "
"  struct data_t {int a;int b;};"
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4018,6 +4018,7 @@
   D->getStorageClass()))
 return ToVar;
 
+  ToVar->setTSCSpec(D->getTSCSpec());
   ToVar->setQualifierInfo(ToQualifierLoc);
   ToVar->setAccess(D->getAccess());
   ToVar->setLexicalDeclContext(LexicalDC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98774: [AST] De-duplicate empty node introspection

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 331300.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98774

Files:
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/EmptyNodeIntrospection.inc.in

Index: clang/lib/Tooling/EmptyNodeIntrospection.inc.in
===
--- /dev/null
+++ clang/lib/Tooling/EmptyNodeIntrospection.inc.in
@@ -0,0 +1,20 @@
+//===- EmptyNodeIntrospection.inc.in --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+namespace clang {
+namespace tooling {
+
+NodeLocationAccessors NodeIntrospection::GetLocations(clang::Stmt const *) {
+  return {};
+}
+NodeLocationAccessors
+NodeIntrospection::GetLocations(clang::DynTypedNode const &) {
+  return {};
+}
+} // namespace tooling
+} // namespace clang
Index: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
===
--- clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -4,7 +4,8 @@
 import os
 import sys
 import json
-
+import filecmp
+import shutil
 import argparse
 
 class Generator(object):
@@ -148,13 +149,16 @@
   help='Read API description from FILE', metavar='FILE')
 parser.add_argument('--output-file', help='Generate output in FILEPATH',
   metavar='FILEPATH')
-parser.add_argument('--empty-implementation',
+parser.add_argument('--use-empty-implementation',
   help='Generate empty implementation',
   action="store", type=int)
+parser.add_argument('--empty-implementation',
+  help='Copy empty implementation from FILEPATH',
+  action="store", metavar='FILEPATH')
 
 options = parser.parse_args()
 
-use_empty_implementation = options.empty_implementation
+use_empty_implementation = options.use_empty_implementation
 
 if (not use_empty_implementation
 and not os.path.exists(options.json_input_path)):
@@ -168,22 +172,9 @@
 use_empty_implementation = True
 
 if use_empty_implementation:
-with open(os.path.join(os.getcwd(),
-  options.output_file), 'w') as f:
-f.write("""
-namespace clang {
-namespace tooling {
-
-NodeLocationAccessors NodeIntrospection::GetLocations(clang::Stmt const *) {
-  return {};
-}
-NodeLocationAccessors
-NodeIntrospection::GetLocations(clang::DynTypedNode const &) {
-  return {};
-}
-} // namespace tooling
-} // namespace clang
-""")
+if not os.path.exists(options.output_file) or \
+not filecmp.cmp(options.empty_implementation, options.output_file):
+shutil.copyfile(options.empty_implementation, options.output_file)
 sys.exit(0)
 
 g = Generator()
Index: clang/lib/Tooling/CMakeLists.txt
===
--- clang/lib/Tooling/CMakeLists.txt
+++ clang/lib/Tooling/CMakeLists.txt
@@ -32,21 +32,10 @@
 OR NOT LLVM_NATIVE_ARCH IN_LIST LLVM_TARGETS_TO_BUILD
 OR NOT X86 IN_LIST LLVM_TARGETS_TO_BUILD
 )
-  file(GENERATE OUTPUT ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
-CONTENT "
-namespace clang {
-namespace tooling {
-
-NodeLocationAccessors NodeIntrospection::GetLocations(clang::Stmt const *) {
-  return {};
-}
-NodeLocationAccessors
-NodeIntrospection::GetLocations(clang::DynTypedNode const &) {
-  return {};
-}
-} // namespace tooling
-} // namespace clang
-"
+configure_file(
+  EmptyNodeIntrospection.inc.in
+  ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
+  COPYONLY
 )
 set(CLANG_TOOLING_BUILD_AST_INTROSPECTION "OFF" CACHE BOOL "")
 else()
@@ -98,11 +87,14 @@
   OUTPUT ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/ASTNodeAPI.json
 ${CMAKE_CURRENT_SOURCE_DIR}/DumpTool/generate_cxx_src_locs.py
+${CMAKE_CURRENT_SOURCE_DIR}/EmptyNodeIntrospection.inc.in
   COMMAND
   ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/DumpTool/generate_cxx_src_locs.py
 --json-input-path ${CMAKE_CURRENT_BINARY_DIR}/ASTNodeAPI.json
 --output-file NodeIntrospection.inc
---empty-implementation ${skip_expensive_processing}
+--use-empty-implementation ${skip_expensive_processing}
+--empty-implementation
+  "${CMAKE_CURRENT_SOURCE_DIR}/EmptyNodeIntrospection.inc.in"
   COMMAND
   ${CMAKE_COMMAND} -E copy_if_different
 

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680
+if (remainingFile[whileIndex] != '\n' &&
+(remainingFile[whileIndex] == ' ' &&
+ remainingFile[whileIndex - 1] == ' ')) {
+  remainingLineCharCount++;

atirit wrote:
> curdeius wrote:
> > I don't really understand these conditions on spaces. Could you explain 
> > your intent, please?
> > You really need to add specific tests for that, playing with the value of 
> > ColumnLimit, adding spacing etc.
> Repeated spaces, e.g. `enum {   A,  B, C } SomeEnum;` are removed during 
> formatting. Since they wouldn't be part of the formatted line, they shouldn't 
> be counted towards the column limit. Only one space need be considered. 
> Removed spaces, e.g. `enum{A,B,C}SomeEnum;` are handled by the fact that 
> `clang-format` runs multiple passes. On the first pass, spaces would be 
> added. On the second pass, assuming the line is then too long, the above code 
> would catch it and break up the enum.
> 
> I'll add unit tests to check if spaces are being handled correctly.
Since you use `== ' '` twice, `remainingLineCharCount` will count only 
consecutive spaces, right?
But you want to count other characters, no?
So, IIUC, the condition you want is `rF[wI] != '\n' && !(rF[wI] == ' ' && rF[wI 
- 1] == ' ')` (mind the negation). It will count characters other than a 
newline and it will only count a series of consecutive spaces as one char. WDYT?



Comment at: clang/lib/Format/TokenAnnotator.cpp:3681
+ remainingFile[whileIndex - 1] == ' ')) {
+  remainingLineCharCount++;
+}

Nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: klimek, aaron.ballman, steveire.
Herald added a subscriber: pengfei.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changing matchers to use non-const members and adding r-value overloads of 
matcher conversions enables move optimisations.
I don't have performance figures but I can say this knocked 120k from the 
clang-tidy binary(86k was from the .text section) on a Release with assertions 
build(x86_64-unknown-linux-gnu).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98792

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h

Index: clang/include/clang/ASTMatchers/ASTMatchersMacros.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersMacros.h
+++ clang/include/clang/ASTMatchers/ASTMatchersMacros.h
@@ -143,7 +143,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -151,7 +151,7 @@
 return ::clang::ast_matchers::internal::makeMatcher(   \
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param));\
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType const );  \
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
   const Type ,\
@@ -192,8 +192,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2; \
   };   \
   }\
   inline ::clang::ast_matchers::internal::Matcher DefineMatcher( \
@@ -202,7 +202,7 @@
 new internal::matcher_##DefineMatcher##OverloadId##Matcher(Param1, \
Param2));   \
   }\
-  typedef ::clang::ast_matchers::internal::Matcher(  \
+  typedef ::clang::ast_matchers::internal::Matcher ( \
   ##_Type##OverloadId)(ParamType1 const , \
  ParamType2 const );\
   inline bool internal::matcher_##DefineMatcher##OverloadId##Matcher::matches( \
@@ -281,7 +281,7 @@
  *Builder) const override; \
\
   private: \
-ParamType const Param; \
+ParamType Param;   \
   };   \
   }\
   inline ::clang::ast_matchers::internal::PolymorphicMatcher<  \
@@ -333,8 +333,8 @@
  *Builder) const override; \
\
   private: \
-ParamType1 const Param1;   \
-ParamType2 const Param2;   \
+ParamType1 Param1; \
+ParamType2 Param2;   

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

[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 once the optional bits are fixed up.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:529
+/// ReturnStmts return them from the function.
+class Returned {
+  llvm::SmallVector 
ReturnedParams;

whisperity wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > Question: would it make sense to (someday, not now) consider output 
> > > parameters similar to return statements? e.g.,
> > > ```
> > > void int_swap(int , int ) {
> > >   int temp = foo;
> > >   foo = bar;
> > >   bar = temp;
> > > }
> > > ```
> > > As a question for today: should `co_return` should be handled similarly 
> > > as `return`?
> > 1. Maybe. Unfortunately, one needs to be careful with that. Originally I 
> > did implement a "5th" heuristic that dealt with ignoring parameters that 
> > had the same builtin operator used on them. However, while it silenced a 
> > few false positives, it started creating **massive** amounts of false 
> > negatives.
> > (In the concrete example, I think `foo = bar` will silence them because 
> > //"appears in the same expression"//.)
> > 
> > 2. I don't know. It would require a deeper understanding of Coroutines 
> > themselves, and how people use them.
> If you don't mind me posting images, I can show a direct example. Things 
> where the inability to track data flow really bites us in the backside.
> 
> Examples from Apache Xerces:
> 
> Here's a false positive that would be silenced by the logic of //"using the 
> same operateur on the two params"//.
> {F15891567}
> 
> And here is a false negative from the exact same logic.
> {F15891568}
> 
> Maybe it could be //some// solace that we're restricting to 
> non-const-lvalue-references (and pointers-to-non-const ??) and only 
> assignment (**only** assignment?!))...
Ah, point #1 is an excellent point. As for #2, I think we can handle it in a 
follow-up, but I believe `co_return` follows the same usage patterns as 
`return`, generally.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:563-566
+  llvm::Optional SameExpr;
+  llvm::Optional PassToFun;
+  llvm::Optional SameMember;
+  llvm::Optional Returns;

whisperity wrote:
> aaron.ballman wrote:
> > I don't think there's a need to make these optional when we're using the 
> > `Enabled` flag.
> The original idea was that each heuristic could be individually toggled by 
> the user... But I think we can agree that would be a bad idea and just 
> overengineering.
Yeah, I think it's probably better to drop the optionals here -- we can revisit 
the decision later if there's user feedback requesting finer granularity here.


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

https://reviews.llvm.org/D78652

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

[PATCH] D98558: [OPENMP51]Initial support for the interop directive

[Mike Rice 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 rG410f09af09b9: [OPENMP51]Initial support for the interop 
directive. (authored by mikerice).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98558

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/interop_ast_print.cpp
  clang/test/OpenMP/interop_messages.cpp
  clang/test/OpenMP/taskgroup_messages.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -268,6 +268,9 @@
 OMP_ORDER_concurrent
   ];
 }
+def OMPC_Init : Clause<"init"> {
+  let clangClass = "OMPInitClause";
+}
 def OMPC_Destroy : Clause<"destroy"> {
   let clangClass = "OMPDestroyClause";
 }
@@ -1640,6 +1643,14 @@
 VersionedClause
   ];
 }
+def OMP_interop : Directive<"interop"> {
+  let allowedClauses = [
+VersionedClause,
+VersionedClause,
+VersionedClause,
+VersionedClause,
+  ];
+}
 def OMP_Unknown : Directive<"unknown"> {
   let isDefault = true;
 }
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -717,6 +717,7 @@
 CHECK_SIMPLE_CLAUSE(Update, OMPC_update)
 CHECK_SIMPLE_CLAUSE(UseDeviceAddr, OMPC_use_device_addr)
 CHECK_SIMPLE_CLAUSE(Write, OMPC_write)
+CHECK_SIMPLE_CLAUSE(Init, OMPC_init)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Allocator, OMPC_allocator)
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -807,6 +807,9 @@
   case Stmt::OMPTargetTeamsDistributeSimdDirectiveClass:
 K = CXCursor_OMPTargetTeamsDistributeSimdDirective;
 break;
+  case Stmt::OMPInteropDirectiveClass:
+K = CXCursor_OMPInteropDirective;
+break;
   case Stmt::BuiltinBitCastExprClass:
 K = CXCursor_BuiltinBitCastExpr;
   }
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2278,6 +2278,10 @@
 
 void OMPClauseEnqueue::VisitOMPNogroupClause(const OMPNogroupClause *) {}
 
+void OMPClauseEnqueue::VisitOMPInitClause(const OMPInitClause *C) {
+  VisitOMPClauseList(C);
+}
+
 void OMPClauseEnqueue::VisitOMPDestroyClause(const OMPDestroyClause *) {}
 
 void OMPClauseEnqueue::VisitOMPUnifiedAddressClause(
@@ -5655,6 +5659,8 @@
 "OMPTargetTeamsDistributeParallelForSimdDirective");
   case CXCursor_OMPTargetTeamsDistributeSimdDirective:
 return cxstring::createRef("OMPTargetTeamsDistributeSimdDirective");
+  case CXCursor_OMPInteropDirective:
+return cxstring::createRef("OMPInteropDirective");
   case CXCursor_OverloadCandidate:
 return cxstring::createRef("OverloadCandidate");
   case CXCursor_TypeAliasTemplateDecl:
Index: clang/test/OpenMP/taskgroup_messages.cpp
===
--- clang/test/OpenMP/taskgroup_messages.cpp
+++ clang/test/OpenMP/taskgroup_messages.cpp
@@ -71,7 +71,7 @@
 foo();
   }
 
-#pragma omp taskgroup init // expected-warning {{extra tokens at the end of '#pragma omp taskgroup' are ignored}}
+#pragma omp taskgroup initi // expected-warning {{extra tokens at the end of '#pragma omp taskgroup' are ignored}}
   ;
   return 0;
 }
Index: clang/test/OpenMP/interop_messages.cpp
===
--- /dev/null
+++ 

[clang] 410f09a - [OPENMP51]Initial support for the interop directive.

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2021-03-17T09:42:07-07:00
New Revision: 410f09af09b9261f51663773bee01ec7b37e8fd4

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

LOG: [OPENMP51]Initial support for the interop directive.

Added basic parsing/sema/serialization support for interop directive.
Support for the 'init' clause.

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

Added: 
clang/test/OpenMP/interop_ast_print.cpp
clang/test/OpenMP/interop_messages.cpp

Modified: 
clang/include/clang-c/Index.h
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/StmtOpenMP.h
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/StmtNodes.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtOpenMP.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaExceptionSpec.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/test/OpenMP/taskgroup_messages.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CXCursor.cpp
flang/lib/Semantics/check-omp-structure.cpp
llvm/include/llvm/Frontend/OpenMP/OMP.td

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index b052501c6b24..bcc063051b8c 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2576,7 +2576,11 @@ enum CXCursorKind {
*/
   CXCursor_OMPCanonicalLoop = 289,
 
-  CXCursor_LastStmt = CXCursor_OMPCanonicalLoop,
+  /** OpenMP interop directive.
+   */
+  CXCursor_OMPInteropDirective = 290,
+
+  CXCursor_LastStmt = CXCursor_OMPInteropDirective,
 
   /**
* Cursor that represents the translation unit itself.

diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 958f2b9e0e6f..4e4e719ff184 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7366,6 +7366,129 @@ class OMPOrderClause final : public OMPClause {
   }
 };
 
+/// This represents the 'init' clause in '#pragma omp ...' directives.
+///
+/// \code
+/// #pragma omp interop init(target:obj)
+/// \endcode
+class OMPInitClause final
+: public OMPVarListClause,
+  private llvm::TrailingObjects {
+  friend class OMPClauseReader;
+  friend OMPVarListClause;
+  friend TrailingObjects;
+
+  /// Location of interop variable.
+  SourceLocation VarLoc;
+
+  bool IsTarget = false;
+  bool IsTargetSync = false;
+
+  void setInteropVar(Expr *E) { varlist_begin()[0] = E; }
+
+  void setIsTarget(bool V) { IsTarget = V; }
+
+  void setIsTargetSync(bool V) { IsTargetSync = V; }
+
+  /// Sets the location of the interop variable.
+  void setVarLoc(SourceLocation Loc) { VarLoc = Loc; }
+
+  /// Build 'init' clause.
+  ///
+  /// \param IsTarget Uses the 'target' interop-type.
+  /// \param IsTargetSync Uses the 'targetsync' interop-type.
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param VarLoc Location of the interop variable.
+  /// \param EndLoc Ending location of the clause.
+  /// \param N Number of expressions.
+  OMPInitClause(bool IsTarget, bool IsTargetSync, SourceLocation StartLoc,
+SourceLocation LParenLoc, SourceLocation VarLoc,
+SourceLocation EndLoc, unsigned N)
+  : OMPVarListClause(llvm::omp::OMPC_init, StartLoc,
+LParenLoc, EndLoc, N),
+VarLoc(VarLoc), IsTarget(IsTarget), IsTargetSync(IsTargetSync) {}
+
+  /// Build an empty clause.
+  OMPInitClause(unsigned N)
+  : OMPVarListClause(llvm::omp::OMPC_init, SourceLocation(),
+SourceLocation(), SourceLocation(), N) 
{
+  }
+
+public:
+  /// Creates a fully specified clause.
+  ///
+  /// \param C AST context.
+  /// \param InteropVar The interop variable.
+  /// \param PrefExprs The list of preference expressions.
+  /// \param IsTarget Uses the 'target' interop-type.
+  /// \param IsTargetSync Uses the 'targetsync' interop-type.
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param VarLoc Location of the interop variable.
+  /// \param EndLoc Ending location of the clause.

[PATCH] D98707: [clang][ASTImporter] Fix import of VarDecl regarding thread local storage spec

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

I did not notice that `ImportVariables` is different than `ImportDecl`, I like 
the first version of the test better (with `ImportDecl`) (but the current is 
not wrong), can you change it back only at a different position (line 738 can 
be good, before test "ImportRecordTypeInFunc")?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98707

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

flx wrote:
> ymandel wrote:
> > flx wrote:
> > > ymandel wrote:
> > > > nit: while this change is not much worse than the existing code, the 
> > > > matcher really should perform this test for "::", and create the Regex, 
> > > > for each string just once. Regex construction is relatively expensive, 
> > > > and matchers of this sort (very generic) are often called quite 
> > > > frequently.
> > > > 
> > > > for that matter, this matcher is now really close to `matchesName` and 
> > > > would probably be best as a first-class matcher, rather than limited to 
> > > > clang-tidy/utils. But, I'll admit that such is beyond the scope of this 
> > > > patch.
> > > It wouldn't be hard to change it something like this:
> > > 
> > > 
> > > ```
> > > struct MatcherPair {  
> > >  
> > > llvm::Regex regex;
> > >
> > > bool matchQualifiedName = false;  
> > >
> > >   };  
> > >
> > >   std::vector Matchers;  
> > >
> > >   std::transform( 
> > >
> > >   NameList.begin(), NameList.end(), std::back_inserter(Matchers), 
> > >
> > >   [](const std::string& Name) {   
> > >
> > > return MatcherPair{   
> > >
> > > .regex = llvm::Regex(Name),   
> > >
> > > .matchQualifiedName = Name.find("::") != std::string::npos,   
> > >
> > > };
> > >
> > >   }); 
> > >
> > >   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) { 
> > >
> > > if (Matcher.matchQualifiedName) { 
> > >
> > >   return Matcher.regex.match(Node.getQualifiedNameAsString());
> > >
> > > } 
> > >
> > > return Matcher.regex.match(Node.getName());   
> > >
> > >   });   
> > > ```
> > > 
> > > Is this what you had in mind?
> > Thanks, that's almost it. The key point is that the code before the 
> > `return` be executed once, during matcher construction, and not each time 
> > `match` is called. You could define your own class (instead of using the 
> > macro) or just your own factory function:
> > 
> > ```
> > struct MatcherPair {
> >
> > llvm::Regex regex;  
> >  
> > bool matchQualifiedName = false;
> >  
> > };
> > 
> > AST_MATCHER_P(NamedDecl, matchesAnyListedNameImpl, 
> > std::vector, Matchers) {
> >   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) {   
> >  
> > if (Matcher.matchQualifiedName) {   
> >  
> >   return Matcher.regex.match(Node.getQualifiedNameAsString());  
> >  
> > }   
> >  
> > return Matcher.regex.match(Node.getName()); 
> >   

[PATCH] D97371: [clang][parser] Remove questionable ProhibitAttributes() call in objc parsing

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

John, do you have any more comments on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97371

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

[PATCH] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Zakk Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/RISCV.cpp:169
+Builder.defineMacro("__rvv_e64", "3");
+Builder.defineMacro("__rvv_e128", "4");
+

craig.topper wrote:
> Are intending to support e128?
remove! thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96843

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

[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

[Zakk Chen via Phabricator via cfe-commits]

khchen added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:687
+
+unsigned Skew = 0;
+if (HasMaskedOffOperand)

craig.topper wrote:
> ```
> unsigned Skew = HasMaskedOffOperand ? 1 : 0;
> ```
> 
> unless this needs to get more complicated in a future patch?
> 
No. thanks.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:695
+// Verify the result of CTypeOrder has legal value.
+if (std::upper_bound(CTypeOrder.begin(), CTypeOrder.end(),
+ CTypeOrder.size() - 1) != CTypeOrder.end())

craig.topper wrote:
> std::upper_bound requires a list to be sorted. It tells you the upper 
> location of where the value belongs in the sorted sequence. std::max_element 
> can tell you the largest value in an unsorted range.
thanks for point out my error.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:699
+  "The index of PermuteOperand is bigger than the operand number");
+if (std::unique(CTypeOrder.begin(), CTypeOrder.end()) != CTypeOrder.end())
+  PrintFatalError(

craig.topper wrote:
> std::unique only compares adjacent values. As far it is concerned "[1, 0, 1]" 
> is unique because the adjacent values are different. To check for duplicates 
> I think you need to sort it first and then you want std::adjacent_find rather 
> than std::unique. std::unique modifies the range and shifts them down. 
> std::adjacent_find just tells you were the duplicate is. Another option is to 
> iterate and use a set to keep track of values you already saw.
thanks, I prefer the latter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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

[PATCH] D98774: [AST] De-duplicate empty node introspection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Why were there two copies in the first place? What was the copy in the cmake 
file good for, why wasn't the one written by generate_cxx_src_locs.py 
sufficient? But pulling it into its own file seems nicer than having it inline 
in python or cmake code.

Another observation: it's a bit strange that the empty impl codepath doesn't 
share any code with the non-empty code path. The py script could do something 
like

  if use_empty_implementation:
jsonData = { classesInClade': [] 'classNames': [] }

and then just fall through to the normal generation code and everything should 
in theory work with much fewer special cases, right?

(What's a "clade"?)

(I still think the whole approach feels off, as mentioned on the other review.)




Comment at: clang/lib/Tooling/CMakeLists.txt:38
+  ${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
+  COPYONLY
 )

Is configure_file(COYPONLY) different from file(COPY)? In what way?

Are you expecting to use cmake vars in the .in file eventually?



Comment at: clang/lib/Tooling/CMakeLists.txt:89
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/ASTNodeAPI.json
 ${CMAKE_CURRENT_SOURCE_DIR}/DumpTool/generate_cxx_src_locs.py
   COMMAND

You need to add a dep on the inc file here so that this step reruns if 
EmptyNodeIntrospection.inc changes.



Comment at: clang/lib/Tooling/CMakeLists.txt:96
+--empty-implementation
+  "${CMAKE_CURRENT_SOURCE_DIR}/EmptyNodeIntrospection.inc.in"
   COMMAND

What's the advantage of making a copy above? Why not call the checked-in file 
`EmptyNodeIntrospection.inc` and pass the path to it directly here?



Comment at: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:176
   options.output_file), 'w') as f:
-f.write("""
-namespace clang {
-namespace tooling {
-
-NodeLocationAccessors NodeIntrospection::GetLocations(clang::Stmt const *) {
-  return {};
-}
-NodeLocationAccessors
-NodeIntrospection::GetLocations(clang::DynTypedNode const &) {
-  return {};
-}
-} // namespace tooling
-} // namespace clang
-""")
+f.write(options.empty_implementation)
 sys.exit(0)

Shouldn't this be `f.write(open(options.empty_implementation).read())`?

If you're changing this, you could also make it so that it only writes the 
output if it'd be different from what it's there, with something like this:



Comment at: llvm/utils/gn/secondary/clang/lib/Tooling/BUILD.gn:8
+"--use-empty-implementation=1",
+"--empty-implementation=" + rebase_path(".") + 
"/EmptyNodeIntrospection.inc",
 "--output-file=" + rebase_path(outputs[0], root_build_dir),

Don't worry about the gn build, just don't update it at all.

(The input file would have to go into `inputs` for correct incremental builds, 
the `rebase_path` looks not quite right, there's nothing that copies the 
.inc.in file to .inc, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98774

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

ymandel wrote:
> flx wrote:
> > ymandel wrote:
> > > nit: while this change is not much worse than the existing code, the 
> > > matcher really should perform this test for "::", and create the Regex, 
> > > for each string just once. Regex construction is relatively expensive, 
> > > and matchers of this sort (very generic) are often called quite 
> > > frequently.
> > > 
> > > for that matter, this matcher is now really close to `matchesName` and 
> > > would probably be best as a first-class matcher, rather than limited to 
> > > clang-tidy/utils. But, I'll admit that such is beyond the scope of this 
> > > patch.
> > It wouldn't be hard to change it something like this:
> > 
> > 
> > ```
> > struct MatcherPair {
> >
> > llvm::Regex regex;  
> >  
> > bool matchQualifiedName = false;
> >  
> >   };
> >  
> >   std::vector Matchers;
> >  
> >   std::transform(   
> >  
> >   NameList.begin(), NameList.end(), std::back_inserter(Matchers),   
> >  
> >   [](const std::string& Name) { 
> >  
> > return MatcherPair{ 
> >  
> > .regex = llvm::Regex(Name), 
> >  
> > .matchQualifiedName = Name.find("::") != std::string::npos, 
> >  
> > };  
> >  
> >   });   
> >  
> >   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) {   
> >  
> > if (Matcher.matchQualifiedName) {   
> >  
> >   return Matcher.regex.match(Node.getQualifiedNameAsString());  
> >  
> > }   
> >  
> > return Matcher.regex.match(Node.getName()); 
> >  
> >   });   
> > ```
> > 
> > Is this what you had in mind?
> Thanks, that's almost it. The key point is that the code before the `return` 
> be executed once, during matcher construction, and not each time `match` is 
> called. You could define your own class (instead of using the macro) or just 
> your own factory function:
> 
> ```
> struct MatcherPair {  
>  
> llvm::Regex regex;
>
> bool matchQualifiedName = false;  
>
> };
> 
> AST_MATCHER_P(NamedDecl, matchesAnyListedNameImpl, std::vector, 
> Matchers) {
>   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) { 
>
> if (Matcher.matchQualifiedName) { 
>
>   return Matcher.regex.match(Node.getQualifiedNameAsString());
>
> } 
>
> return Matcher.regex.match(Node.getName());   
>
>   });
> }
> 
> Matcher matchesAnyListedName(std::vector NameList) {
>   std::vector Matchers;  
>  

[PATCH] D98692: [clang-tidy] Add cppcoreguidelines-comparison-operator

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D98692#2631504 , @nullptr.cpp wrote:

> The relevant rule is F.16: For "in" parameters, pass cheaply-copied types by 
> value and others by reference to const 
> .
> This should be done in another checker dedicated for parameter passing.

That's not exactly a relevant rule, That describes handling of `In` parameters. 
However, the point I'm making is we should enforce that parameters to 
comparison functions are `In` parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98692

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

[PATCH] D98783: [CUDA][HIP] Remove unused addr space casts

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.
yaxunl requested review of this revision.

In device compilation, clang emit global variables in device or constant
address space then cast them to default address space. If global variables
are not used, there are invisible address space casts as LLVM constants
in the LLVM module. These casts cause spurious use of global variables
and prevent them from being eliminated by global DCE.

Such casts will disappear if the module is saved and reloaded, but stays
if the module is not saved and reloaded. This causes difference in generated ISA
depending on whether -save-temps is used.

The patch removes the invisible unused casts of global variables.


https://reviews.llvm.org/D98783

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/unused-global-var.cu

Index: clang/test/CodeGenCUDA/unused-global-var.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/unused-global-var.cu
@@ -0,0 +1,50 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -x hip %s \
+// RUN:   -std=c++11 -O3 -mllvm -amdgpu-internalize-symbols -emit-llvm -o - \
+// RUN:   | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// AMDGPU internalize unused global variables for whole-program compilation
+// (-fno-gpu-rdc for each TU, or -fgpu-rdc for LTO), which are then
+// eliminated by global DCE. If there are invisible unused address space casts
+// for global variables, the internalization and elimination of unused global
+// variales will be hindered. This test makes sure no such address space
+// casts.
+
+// Check unused device/constant variables are eliminated.
+
+// CHECK-NOT: @v1
+__device__ int v1;
+
+// CHECK-NOT: @v2
+__constant__ int v2;
+
+// CHECK-NOT: @_ZL2v3
+constexpr int v3 = 1;
+
+// Check managed variables are always kept.
+
+// CHECK: @v4
+__managed__ int v4;
+
+// Check used device/constant variables are not eliminated.
+// CHECK: @u1
+__device__ int u1;
+
+// CHECK: @u2
+__constant__ int u2;
+
+// Check u3 is kept because its address is taken.
+// CHECK: @_ZL2u3
+constexpr int u3 = 2;
+
+// Check u4 is not kept because it is not ODR-use.
+// CHECK-NOT: @_ZL2u4
+constexpr int u4 = 3;
+
+__device__ int fun1(const int& x);
+
+__global__ void kern1(int *x) {
+  *x = u1 + u2 + fun1(u3) + u4;
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -371,6 +371,14 @@
   llvm::SmallVector, 8>
 GlobalValReplacements;
 
+  /// Potentially unused address space casts of global variables to be cleaned
+  /// up. In CUDA/HIP, global variables are emitted as global variables in
+  /// device or constant address space which are then casted to default address
+  /// space. If the global variables are not used, the address space casts
+  /// become invisible LLVM constants, causing spurious use of the global
+  /// variables which prevents them from being erased.
+  llvm::DenseSet GlobalVarCasts;
+
   /// Variables for which we've emitted globals containing their constant
   /// values along with the corresponding globals, for opportunistic reuse.
   llvm::DenseMap InitializerConstants;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -768,6 +768,11 @@
   // that might affect the DLL storage class or the visibility, and
   // before anything that might act on these.
   setVisibilityFromDLLStorageClass(LangOpts, getModule());
+
+  // Remove unused address space casts of global variables.
+  for (auto *Cast : GlobalVarCasts)
+if (Cast->use_empty())
+  Cast->destroyConstant();
 }
 
 void CodeGenModule::EmitOpenCLMetadata() {
@@ -3938,9 +3943,13 @@
 : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default);
   assert(getContext().getTargetAddressSpace(ExpectedAS) ==
  Ty->getPointerAddressSpace());
-  if (AddrSpace != ExpectedAS)
-return getTargetCodeGenInfo().performAddrSpaceCast(*this, GV, AddrSpace,
-   ExpectedAS, Ty);
+  if (AddrSpace != ExpectedAS) {
+auto *Cast = getTargetCodeGenInfo().performAddrSpaceCast(
+*this, GV, AddrSpace, ExpectedAS, Ty);
+// Record address space casts of global variables for cleaning up if unused.
+GlobalVarCasts.insert(Cast);
+return Cast;
+  }
 
   return GV;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680
+if (remainingFile[whileIndex] != '\n' &&
+(remainingFile[whileIndex] == ' ' &&
+ remainingFile[whileIndex - 1] == ' ')) {
+  remainingLineCharCount++;

curdeius wrote:
> I don't really understand these conditions on spaces. Could you explain your 
> intent, please?
> You really need to add specific tests for that, playing with the value of 
> ColumnLimit, adding spacing etc.
Repeated spaces, e.g. `enum {   A,  B, C } SomeEnum;` are removed during 
formatting. Since they wouldn't be part of the formatted line, they shouldn't 
be counted towards the column limit. Only one space need be considered. Removed 
spaces, e.g. `enum{A,B,C}SomeEnum;` are handled by the fact that `clang-format` 
runs multiple passes. On the first pass, spaces would be added. On the second 
pass, assuming the line is then too long, the above code would catch it and 
break up the enum.

I'll add unit tests to check if spaces are being handled correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D96843: [Clang][RISCV] Add vsetvl and vsetvlmax.

[Zakk Chen via Phabricator via cfe-commits]

khchen updated this revision to Diff 331267.
khchen marked 2 inline comments as done.
khchen added a comment.

1. address Craig's comments.
2. update test by using 2>&1 instead of 2>%t


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96843

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvl.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vsetvlmax.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -145,6 +145,8 @@
   bool HasMaskedOffOperand;
   bool HasVL;
   bool HasGeneric;
+  bool HasAutoDef; // There is automiatic definition in header
+  std::string ManualCodegen;
   RVVTypePtr OutputType; // Builtin output type
   RVVTypes InputTypes;   // Builtin input types
   // The types we use to obtain the specific LLVM intrinsic. They are index of
@@ -159,8 +161,8 @@
   RVVIntrinsic(StringRef Name, StringRef Suffix, StringRef MangledName,
StringRef IRName, bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL, bool HasGeneric,
-   const RVVTypes ,
-   const std::vector );
+   bool HasAutoDef, StringRef ManualCodegen, const RVVTypes ,
+   const std::vector );
   ~RVVIntrinsic() = default;
 
   StringRef getName() const { return Name; }
@@ -169,6 +171,8 @@
   bool hasMaskedOffOperand() const { return HasMaskedOffOperand; }
   bool hasVL() const { return HasVL; }
   bool hasGeneric() const { return HasGeneric; }
+  bool hasManualCodegen() const { return !ManualCodegen.empty(); }
+  bool hasAutoDef() const { return HasAutoDef; }
   size_t getNumOperand() const { return InputTypes.size(); }
   StringRef getIRName() const { return IRName; }
   uint8_t getRISCVExtensions() const { return RISCVExtensions; }
@@ -190,6 +194,7 @@
 class RVVEmitter {
 private:
   RecordKeeper 
+  std::string HeaderCode;
   // Concat BasicType, LMUL and Proto as key
   StringMap LegalTypes;
   StringSet<> IllegalTypes;
@@ -637,11 +642,13 @@
StringRef NewMangledName, StringRef IRName,
bool HasSideEffects, bool IsMask,
bool HasMaskedOffOperand, bool HasVL,
-   bool HasGeneric, const RVVTypes ,
+   bool HasGeneric, bool HasAutoDef,
+   StringRef ManualCodegen, const RVVTypes ,
const std::vector )
 : IRName(IRName), HasSideEffects(HasSideEffects),
   HasMaskedOffOperand(HasMaskedOffOperand), HasVL(HasVL),
-  HasGeneric(HasGeneric) {
+  HasGeneric(HasGeneric), HasAutoDef(HasAutoDef),
+  ManualCodegen(ManualCodegen.str()) {
 
   // Init Name and MangledName
   Name = NewName.str();
@@ -702,7 +709,13 @@
 }
 
 void RVVIntrinsic::emitCodeGenSwitchBody(raw_ostream ) const {
+
   OS << "  ID = Intrinsic::riscv_" + getIRName() + ";\n";
+  if (hasManualCodegen()) {
+OS << ManualCodegen;
+OS << "break;\n";
+return;
+  }
   OS << "  IntrinsicTypes = {";
   ListSeparator LS;
   for (const auto  : IntrinsicTypes) {
@@ -792,6 +805,11 @@
   std::vector> Defs;
   createRVVIntrinsics(Defs);
 
+  // Print header code
+  if (!HeaderCode.empty()) {
+OS << HeaderCode;
+  }
+
   auto printType = [&](auto T) {
 OS << "typedef " << T->getClangBuiltinStr() << " " << T->getTypeStr()
<< ";\n";
@@ -910,7 +928,6 @@
 
 void RVVEmitter::createRVVIntrinsics(
 std::vector> ) {
-
   std::vector RV = Records.getAllDerivedDefinitions("RVVBuiltin");
   for (auto *R : RV) {
 StringRef Name = R->getValueAsString("Name");
@@ -924,11 +941,18 @@
 bool HasGeneric = R->getValueAsBit("HasGeneric");
 bool HasSideEffects = R->getValueAsBit("HasSideEffects");
 std::vector Log2LMULList = R->getValueAsListOfInts("Log2LMUL");
+StringRef ManualCodegen = R->getValueAsString("ManualCodegen");
+StringRef ManualCodegenMask = R->getValueAsString("ManualCodegenMask");
 std::vector IntrinsicTypes =
 R->getValueAsListOfInts("IntrinsicTypes");
 StringRef IRName = R->getValueAsString("IRName");
 StringRef IRNameMask = R->getValueAsString("IRNameMask");
 
+StringRef HeaderCodeStr = R->getValueAsString("HeaderCode");
+bool HasAutoDef = HeaderCodeStr.empty();
+if (!HeaderCodeStr.empty()) {
+  HeaderCode += HeaderCodeStr.str();
+}
 // Parse prototype and create a list of primitive type with transformers
 // (operand) in ProtoSeq. ProtoSeq[0] is output operand.
 SmallVector ProtoSeq;
@@ -955,7 +979,7 @@
   ProtoMaskSeq.push_back("z");
 }
 
-// Create intrinsics for each type and LMUL.
+// Create Intrinsics for each type and LMUL.
 for (char I : TypeRange) {
   for 

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

flx wrote:
> ymandel wrote:
> > nit: while this change is not much worse than the existing code, the 
> > matcher really should perform this test for "::", and create the Regex, for 
> > each string just once. Regex construction is relatively expensive, and 
> > matchers of this sort (very generic) are often called quite frequently.
> > 
> > for that matter, this matcher is now really close to `matchesName` and 
> > would probably be best as a first-class matcher, rather than limited to 
> > clang-tidy/utils. But, I'll admit that such is beyond the scope of this 
> > patch.
> It wouldn't be hard to change it something like this:
> 
> 
> ```
> struct MatcherPair {  
>  
> llvm::Regex regex;
>
> bool matchQualifiedName = false;  
>
>   };  
>
>   std::vector Matchers;  
>
>   std::transform( 
>
>   NameList.begin(), NameList.end(), std::back_inserter(Matchers), 
>
>   [](const std::string& Name) {   
>
> return MatcherPair{   
>
> .regex = llvm::Regex(Name),   
>
> .matchQualifiedName = Name.find("::") != std::string::npos,   
>
> };
>
>   }); 
>
>   return llvm::any_of(Matchers, [](const MatcherPair& Matcher) { 
>
> if (Matcher.matchQualifiedName) { 
>
>   return Matcher.regex.match(Node.getQualifiedNameAsString());
>
> } 
>
> return Matcher.regex.match(Node.getName());   
>
>   });   
> ```
> 
> Is this what you had in mind?
Thanks, that's almost it. The key point is that the code before the `return` be 
executed once, during matcher construction, and not each time `match` is 
called. You could define your own class (instead of using the macro) or just 
your own factory function:

```
struct MatcherPair {
   
llvm::Regex regex;  
 
bool matchQualifiedName = false;
 
};

AST_MATCHER_P(NamedDecl, matchesAnyListedNameImpl, std::vector, 
Matchers) {
  return llvm::any_of(Matchers, [](const MatcherPair& Matcher) {   
 
if (Matcher.matchQualifiedName) {   
 
  return Matcher.regex.match(Node.getQualifiedNameAsString());  
 
}   
 
return Matcher.regex.match(Node.getName()); 
 
  });
}

Matcher matchesAnyListedName(std::vector NameList) {
  std::vector Matchers;
 
  std::transform(   
 
  

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

[Timm Bäder 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 rGd9ef6bc42643: [clang] Disable LTO and LLD on SystemZ for 
stage3 builds (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D89942?vs=331246=331262#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89942

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/3-stage-base.cmake


Index: clang/cmake/caches/3-stage-base.cmake
===
--- clang/cmake/caches/3-stage-base.cmake
+++ clang/cmake/caches/3-stage-base.cmake
@@ -1,14 +1,34 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
 set(LLVM_BUILD_EXTERNAL_COMPILER_RT ON CACHE BOOL "")
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 
-# Use LLD do have less requirements on system linker, unless we're on an apple
-# platform where the system compiler is to be prefered.
 if(APPLE)
+  # Use LLD to have fewer requirements on system linker, unless we're on an 
apple
+  # platform where the system compiler is to be preferred.
+  set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+elseif(CMAKE_HOST_UNIX)
+  # s390/SystemZ is unsupported by LLD, so don't try to enable LTO if it
+  # cannot work.
+  # We do our own uname business here since the appropriate variables from 
CMake
+  # and llvm are not yet available.
+  find_program(CMAKE_UNAME uname /bin /usr/bin /usr/local/bin )
+  if(CMAKE_UNAME)
+exec_program(${CMAKE_UNAME} ARGS -m OUTPUT_VARIABLE 
CMAKE_HOST_SYSTEM_PROCESSOR
+RETURN_VALUE val)
+  endif(CMAKE_UNAME)
+
+  if("${CMAKE_HOST_SYSTEM_PROCESSOR}" MATCHES "s390")
+set(BOOTSTRAP_LLVM_ENABLE_LTO OFF CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
-else()
+  else()
+set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+  endif()
+
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 endif()
 
 
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -640,6 +640,11 @@
   set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-bins/)
 
   if(BOOTSTRAP_LLVM_ENABLE_LLD)
+# adding lld to clang-bootstrap-deps without having it enabled in
+# LLVM_ENABLE_PROJECTS just generates a cryptic error message.
+if (NOT "lld" IN_LIST LLVM_ENABLE_PROJECTS)
+  message(FATAL_ERROR "LLD is enabled in the boostrap build, but lld is 
not in LLVM_ENABLE_PROJECTS")
+endif()
 add_dependencies(clang-bootstrap-deps lld)
   endif()
 


Index: clang/cmake/caches/3-stage-base.cmake
===
--- clang/cmake/caches/3-stage-base.cmake
+++ clang/cmake/caches/3-stage-base.cmake
@@ -1,14 +1,34 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
 set(LLVM_BUILD_EXTERNAL_COMPILER_RT ON CACHE BOOL "")
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 
-# Use LLD do have less requirements on system linker, unless we're on an apple
-# platform where the system compiler is to be prefered.
 if(APPLE)
+  # Use LLD to have fewer requirements on system linker, unless we're on an apple
+  # platform where the system compiler is to be preferred.
+  set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+elseif(CMAKE_HOST_UNIX)
+  # s390/SystemZ is unsupported by LLD, so don't try to enable LTO if it
+  # cannot work.
+  # We do our own uname business here since the appropriate variables from CMake
+  # and llvm are not yet available.
+  find_program(CMAKE_UNAME uname /bin /usr/bin /usr/local/bin )
+  if(CMAKE_UNAME)
+exec_program(${CMAKE_UNAME} ARGS -m OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_PROCESSOR
+RETURN_VALUE val)
+  endif(CMAKE_UNAME)
+
+  if("${CMAKE_HOST_SYSTEM_PROCESSOR}" MATCHES "s390")
+set(BOOTSTRAP_LLVM_ENABLE_LTO OFF CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
-else()
+  else()
+set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+  endif()
+
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 endif()
 
 
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -640,6 +640,11 @@
   set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-bins/)
 
   if(BOOTSTRAP_LLVM_ENABLE_LLD)
+# adding lld to clang-bootstrap-deps without having it enabled in
+# LLVM_ENABLE_PROJECTS just generates a cryptic error message.
+if (NOT "lld" 

[clang] d9ef6bc - [clang] Disable LTO and LLD on SystemZ for stage3 builds

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2021-03-17T15:15:41+01:00
New Revision: d9ef6bc42643ae4feab3f9eca97864d72034f2ce

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

LOG: [clang] Disable LTO and LLD on SystemZ for stage3 builds

LLD does not support SystemZ, so it doesn't make sense to use it for
boostrap/stage3 builds, and using LTO in these cases won't work.

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/cmake/caches/3-stage-base.cmake

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 7af05c331e94..aa38110b6d22 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -640,6 +640,11 @@ if (CLANG_ENABLE_BOOTSTRAP)
   set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-bins/)
 
   if(BOOTSTRAP_LLVM_ENABLE_LLD)
+# adding lld to clang-bootstrap-deps without having it enabled in
+# LLVM_ENABLE_PROJECTS just generates a cryptic error message.
+if (NOT "lld" IN_LIST LLVM_ENABLE_PROJECTS)
+  message(FATAL_ERROR "LLD is enabled in the boostrap build, but lld is 
not in LLVM_ENABLE_PROJECTS")
+endif()
 add_dependencies(clang-bootstrap-deps lld)
   endif()
 

diff  --git a/clang/cmake/caches/3-stage-base.cmake 
b/clang/cmake/caches/3-stage-base.cmake
index 88ab5d77f16f..31391aa4defc 100644
--- a/clang/cmake/caches/3-stage-base.cmake
+++ b/clang/cmake/caches/3-stage-base.cmake
@@ -1,14 +1,34 @@
 set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
 set(LLVM_BUILD_EXTERNAL_COMPILER_RT ON CACHE BOOL "")
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 
-# Use LLD do have less requirements on system linker, unless we're on an apple
-# platform where the system compiler is to be prefered.
 if(APPLE)
+  # Use LLD to have fewer requirements on system linker, unless we're on an 
apple
+  # platform where the system compiler is to be preferred.
+  set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+elseif(CMAKE_HOST_UNIX)
+  # s390/SystemZ is unsupported by LLD, so don't try to enable LTO if it
+  # cannot work.
+  # We do our own uname business here since the appropriate variables from 
CMake
+  # and llvm are not yet available.
+  find_program(CMAKE_UNAME uname /bin /usr/bin /usr/local/bin )
+  if(CMAKE_UNAME)
+exec_program(${CMAKE_UNAME} ARGS -m OUTPUT_VARIABLE 
CMAKE_HOST_SYSTEM_PROCESSOR
+RETURN_VALUE val)
+  endif(CMAKE_UNAME)
+
+  if("${CMAKE_HOST_SYSTEM_PROCESSOR}" MATCHES "s390")
+set(BOOTSTRAP_LLVM_ENABLE_LTO OFF CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD OFF CACHE BOOL "")
-else()
+  else()
+set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+  endif()
+
+else()
+  set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 endif()
 
 



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

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:529
+/// ReturnStmts return them from the function.
+class Returned {
+  llvm::SmallVector 
ReturnedParams;

whisperity wrote:
> aaron.ballman wrote:
> > Question: would it make sense to (someday, not now) consider output 
> > parameters similar to return statements? e.g.,
> > ```
> > void int_swap(int , int ) {
> >   int temp = foo;
> >   foo = bar;
> >   bar = temp;
> > }
> > ```
> > As a question for today: should `co_return` should be handled similarly as 
> > `return`?
> 1. Maybe. Unfortunately, one needs to be careful with that. Originally I did 
> implement a "5th" heuristic that dealt with ignoring parameters that had the 
> same builtin operator used on them. However, while it silenced a few false 
> positives, it started creating **massive** amounts of false negatives.
> (In the concrete example, I think `foo = bar` will silence them because 
> //"appears in the same expression"//.)
> 
> 2. I don't know. It would require a deeper understanding of Coroutines 
> themselves, and how people use them.
If you don't mind me posting images, I can show a direct example. Things where 
the inability to track data flow really bites us in the backside.

Examples from Apache Xerces:

Here's a false positive that would be silenced by the logic of //"using the 
same operateur on the two params"//.
{F15891567}

And here is a false negative from the exact same logic.
{F15891568}

Maybe it could be //some// solace that we're restricting to 
non-const-lvalue-references (and pointers-to-non-const ??) and only assignment 
(**only** assignment?!))...


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

https://reviews.llvm.org/D78652

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

ymandel wrote:
> nit: while this change is not much worse than the existing code, the matcher 
> really should perform this test for "::", and create the Regex, for each 
> string just once. Regex construction is relatively expensive, and matchers of 
> this sort (very generic) are often called quite frequently.
> 
> for that matter, this matcher is now really close to `matchesName` and would 
> probably be best as a first-class matcher, rather than limited to 
> clang-tidy/utils. But, I'll admit that such is beyond the scope of this patch.
It wouldn't be hard to change it something like this:


```
struct MatcherPair {
   
llvm::Regex regex;  
 
bool matchQualifiedName = false;
 
  };
 
  std::vector Matchers;
 
  std::transform(   
 
  NameList.begin(), NameList.end(), std::back_inserter(Matchers),   
 
  [](const std::string& Name) { 
 
return MatcherPair{ 
 
.regex = llvm::Regex(Name), 
 
.matchQualifiedName = Name.find("::") != std::string::npos, 
 
};  
 
  });   
 
  return llvm::any_of(Matchers, [](const MatcherPair& Matcher) {   
 
if (Matcher.matchQualifiedName) {   
 
  return Matcher.regex.match(Node.getQualifiedNameAsString());  
 
}   
 
return Matcher.regex.match(Node.getName()); 
 
  });   
```

Is this what you had in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

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

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity marked 2 inline comments as done.
whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:529
+/// ReturnStmts return them from the function.
+class Returned {
+  llvm::SmallVector 
ReturnedParams;

aaron.ballman wrote:
> Question: would it make sense to (someday, not now) consider output 
> parameters similar to return statements? e.g.,
> ```
> void int_swap(int , int ) {
>   int temp = foo;
>   foo = bar;
>   bar = temp;
> }
> ```
> As a question for today: should `co_return` should be handled similarly as 
> `return`?
1. Maybe. Unfortunately, one needs to be careful with that. Originally I did 
implement a "5th" heuristic that dealt with ignoring parameters that had the 
same builtin operator used on them. However, while it silenced a few false 
positives, it started creating **massive** amounts of false negatives.
(In the concrete example, I think `foo = bar` will silence them because 
//"appears in the same expression"//.)

2. I don't know. It would require a deeper understanding of Coroutines 
themselves, and how people use them.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:563-566
+  llvm::Optional SameExpr;
+  llvm::Optional PassToFun;
+  llvm::Optional SameMember;
+  llvm::Optional Returns;

aaron.ballman wrote:
> I don't think there's a need to make these optional when we're using the 
> `Enabled` flag.
The original idea was that each heuristic could be individually toggled by the 
user... But I think we can agree that would be a bad idea and just 
overengineering.


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

https://reviews.llvm.org/D78652

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

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:529
+/// ReturnStmts return them from the function.
+class Returned {
+  llvm::SmallVector 
ReturnedParams;

Question: would it make sense to (someday, not now) consider output parameters 
similar to return statements? e.g.,
```
void int_swap(int , int ) {
  int temp = foo;
  foo = bar;
  bar = temp;
}
```
As a question for today: should `co_return` should be handled similarly as 
`return`?



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:563-566
+  llvm::Optional SameExpr;
+  llvm::Optional PassToFun;
+  llvm::Optional SameMember;
+  llvm::Optional Returns;

I don't think there's a need to make these optional when we're using the 
`Enabled` flag.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:481
+
+  assert(TargetIdx.hasValue() && "Matched, but didn't find index?");
+  TargetParams[PassedParamOfThisFn].insert(

whisperity wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > I *think* you could run into this assert with K C function where there 
> > > is a `FunctionDecl` to get back to but the decl claims it has no params 
> > > because it has no prototype (while the call expression actually has the 
> > > arguments). However, there may be other code that protects us from this 
> > > case.
> > At face value, I would say the fact that a K function //has no params// 
> > declared means that the matcher above will not be able to do 
> > `forEachArgumentWithParam`, because the argument vector is N long, but the 
> > parameter vector is empty.
> > 
> > Nevertheless, it won't hurt to add a test case for this though.
> No assertion, no crash. But unfortunately, we generate a false positive. But 
> we don't crash. I added a test for this, just in case.
Thanks for adding the test case! I think a false positive is reasonable enough 
given how rarely I expect people using K C function declarations will worry 
about swapped arguments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:532-536
+  if (find(FD->parameters(), ReturnedParam) == FD->param_end())
+// Returning an Expr to a ParmVarDecl that **isn't** parameter of the
+// function should not be possible. Make sure a mistake of the matcher
+// does **not** interfere with the heuristic's result.
+continue;

whisperity wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > Is this a FIXME because there's a matcher issue this works around or 
> > > should it be an assertion?
> > I was unable to create a test for this case. This part of the code, while I 
> > was twisting my head left and right, is one of the strictest remnant of the 
> > old version of the check... There definitely was a project in the test set 
> > back in December 2019 which made the check crash or emit massive false 
> > positives, and this thing was added to guard against that.
> > 
> > At a hunch, I think //maybe// a lambda returning one of its captures (which 
> > are, AFAIK, `ParmVarDecl`s of the constructor!) could be what caused the 
> > crash/false positive. I will check this out in detail soon.
> I do not remember why I thought this makes crashes because I tried and no 
> crashing now (which is good), but lambdas defined inside a function's body is 
> definitely a good reproducer for when this is hit.
> 
> Rephrased the comment, and added a test about this.
Thank you for the clarification and test coverage!


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

https://reviews.llvm.org/D78652

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

(but please wait for "accept" from an owner).




Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:56
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());
+}

nit: while this change is not much worse than the existing code, the matcher 
really should perform this test for "::", and create the Regex, for each string 
just once. Regex construction is relatively expensive, and matchers of this 
sort (very generic) are often called quite frequently.

for that matter, this matcher is now really close to `matchesName` and would 
probably be best as a first-class matcher, rather than limited to 
clang-tidy/utils. But, I'll admit that such is beyond the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

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

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

[Chris Bieneman via Phabricator via cfe-commits]

beanz accepted this revision.
beanz added a comment.

LGTM.

The `*_LTO` and `*_LLD` variables are usable independently of each other even 
on non-Apple platforms, so it makes sense to need to set them both. LLVM’s LTO 
is supported in Gold, and I believe our build system supports MSVC’s LTCG 
(which is their version of LTO).


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

https://reviews.llvm.org/D89942

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

[PATCH] D96355: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with optionally considering differently qualified types mixable

[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!


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

https://reviews.llvm.org/D96355

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