[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Patch looks good to me.  I think we're running some internal tests; do you want 
to wait for those?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89212

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

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

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

rsmith updated this revision to Diff 297767.
rsmith added a comment.

- Don't warn if we see a weak definition for a declaration that's already


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89212

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-weak.c
  clang/test/Sema/init.c

Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -155,7 +155,7 @@
 int PR4386_a = ((void *) PR4386_bar) != 0;
 int PR4386_b = ((void *) PR4386_foo) != 0; // expected-error{{initializer element is not a compile-time constant}}
 int PR4386_c = ((void *) PR4386_zed) != 0;
-int PR4386_zed() __attribute((weak));
+int PR4386_zed() __attribute((weak)); // expected-warning{{'PR4386_zed' declared weak after its first use}} expected-note {{attribute}}
 
 //  (derived from SPEC vortex benchmark)
 typedef char strty[10];
Index: clang/test/Sema/attr-weak.c
===
--- clang/test/Sema/attr-weak.c
+++ clang/test/Sema/attr-weak.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-unknown-macosx10.3.0 -DMACOS
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-linux-gnu -DLINUX
 
 extern int f0() __attribute__((weak));
 extern int g0 __attribute__((weak));
@@ -25,3 +26,55 @@
 
 static void pr14946_f();
 void pr14946_f() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
+
+void some_function();
+
+void pr47663_a();
+void pr47663_b();
+static void pr47663_c();
+void pr47663_d();
+void pr47663_e(); // expected-warning {{declared weak after its first use}}
+void pr47663_f(); // expected-note {{possible target}}
+void pr47663_g();
+int pr47663_h;
+void pr47663_z() __attribute__((weak));
+void use() {
+  int arr_a[_a ? 1 : -1];
+  int arr_b[_b ? 1 : -1];
+  int arr_c[_c ? 1 : -1];
+  int arr_d[_d ? 1 : -1];
+  int arr_e[_e ? 1 : -1];
+  int arr_f[_f ? 1 : -1];
+  int arr_g[_g ? 1 : -1];
+  int arr_h[_h ? 1 : -1]; // expected-warning {{will always evaluate to 'true'}}
+  int arr_z[_z ? -1 : 1];
+}
+void pr47663_a() __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_b() __attribute__((weak_import)); // expected-warning {{declared weak after its first use}} expected-note {{'weak_import' attribute here}}
+#ifdef LINUX
+static void pr47663_c() __attribute__((weakref, alias("might_not_exist"))); // expected-warning {{declared weak after its first use}} expected-note {{'weakref' attribute here}}
+#endif
+#ifdef MACOS
+void pr47663_d() __attribute__((availability(macos, introduced=10.4))); // expected-warning {{declared weak after its first use}} expected-note {{'availability' attribute here}}
+#endif
+#pragma weak pr47663_e // expected-note {{pragma 'weak' here}}
+
+// FIXME: This should warn; see PR47796. But it actually creates a bogus
+// overload set. When this is fixed, ensure we produce the 'declared weak after
+// its first use' warning.
+#pragma weak pr47663_f = some_function // expected-note {{possible target}}
+void q() { pr47663_f; } // expected-error {{overloaded}}
+
+#pragma clang attribute push (__attribute__((weak)), apply_to = function) // expected-note {{'weak' attribute here}}
+void pr47663_g(); // expected-warning {{declared weak after its first use}}
+#pragma clang attribute pop
+extern int pr47663_h __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_z() __attribute__((weak_import));
+
+// 'weak' on a definition means something different from 'weak' on a
+// declaration. Don't warn in that case.
+void weak_def_after_use();
+extern int weak_def_after_use_v;
+void use_wdau() { weak_def_after_use(); weak_def_after_use_v = 0; }
+__attribute__((weak)) void weak_def_after_use() {}
+__attribute__((weak)) int weak_def_after_use_v;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8154,6 +8154,9 @@
   W.setUsed(true);
   if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
 IdentifierInfo *NDId = ND->getIdentifier();
+// FIXME (PR47796): Check for a previous declaration with the target name
+// here, and build a redeclaration of it. Check whether the previous
+// declaration is used and warn if so.
 NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation());
 NewD->addAttr(
 AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
Index: 

[PATCH] D76211: OpenMP Metadirective with user defined condition

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Are you still working on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76211

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Galina Kistanova via cfe-commits]

Thanks, Vitaly!

Let's have them there for at least 24 hours, shall we?

Could you move sanitizer-buildbot1, sanitizer-buildbot3,
sanitizer-buildbot7 as well, please?

AnnotatedCommand on the staging has been tested functionally and is good.
My only concern at this point is how it would handle a heavy load, so the
more bots we will have on the staging the better.

If somebody else could move their AnnotatedCommand bots to the staging
area, that would be much appreciated.

Thanks

Galina

On Mon, Oct 12, 2020 at 9:45 PM Vitaly Buka  wrote:

> Looks like staging AnnotatedCommand fixed step statuses, so we can see
> which one is green.
> Please let me know when to switch bots back from the staging.
> Thank you!
>
> On Mon, 12 Oct 2020 at 21:38, Vitaly Buka  wrote:
>
>> Switched all but PPC, I don't have access to them. But they run the same
>> script as sanitizer-x86_64-linux.
>> http://lab.llvm.org:8014/#/waterfall?tags=sanitizer
>>
>> On Mon, 12 Oct 2020 at 19:19, Galina Kistanova 
>> wrote:
>>
>>> We have a better version of AnnotatedCommand on the staging. It should
>>> be a functional equivalent of the old one.
>>> We need to stress test it well before moving to the production build bot.
>>>
>>> For that we need all sanitizer + other bots which use the
>>> AnnotatedCommand directly or indirectly moved temporarily to the staging.
>>>
>>> Please let me know when that could be arranged.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:
>>>
 On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
 lldb-comm...@lists.llvm.org> wrote:

> They are online now -
> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>
> AnnotatedCommand has severe design conflict with the new buildbot.
> We have changed it to be safe and still do something useful, but it
> will need more love and care.
>
> Please let me know if you have some spare time to work on porting
> AnnotatedCommand.
>

 That's unfortunate, it would've been good to know that earlier. I and
 another team member have spent a fair amount of time porting things to use
 more AnnotatedCommand steps, because it gives us the flexibility to test
 steps locally and make changes to the steps without restarting the buildbot
 master. IMO that is the Right Way to define steps: a script that you can
 run locally on a machine that satisfies the OS and dep requirements of the
 script.

 I am restarting the two bots that I am responsible for, and may need
 some help debugging further issues soon. I'll let you know.

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via cfe-commits]

Looks like staging AnnotatedCommand fixed step statuses, so we can see
which one is green.
Please let me know when to switch bots back from the staging.
Thank you!

On Mon, 12 Oct 2020 at 21:38, Vitaly Buka  wrote:

> Switched all but PPC, I don't have access to them. But they run the same
> script as sanitizer-x86_64-linux.
> http://lab.llvm.org:8014/#/waterfall?tags=sanitizer
>
> On Mon, 12 Oct 2020 at 19:19, Galina Kistanova 
> wrote:
>
>> We have a better version of AnnotatedCommand on the staging. It should be
>> a functional equivalent of the old one.
>> We need to stress test it well before moving to the production build bot.
>>
>> For that we need all sanitizer + other bots which use the
>> AnnotatedCommand directly or indirectly moved temporarily to the staging.
>>
>> Please let me know when that could be arranged.
>>
>> Thanks
>>
>> Galina
>>
>> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:
>>
>>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>>> lldb-comm...@lists.llvm.org> wrote:
>>>
 They are online now -
 http://lab.llvm.org:8011/#/waterfall?tags=sanitizer

 AnnotatedCommand has severe design conflict with the new buildbot.
 We have changed it to be safe and still do something useful, but it
 will need more love and care.

 Please let me know if you have some spare time to work on porting
 AnnotatedCommand.

>>>
>>> That's unfortunate, it would've been good to know that earlier. I and
>>> another team member have spent a fair amount of time porting things to use
>>> more AnnotatedCommand steps, because it gives us the flexibility to test
>>> steps locally and make changes to the steps without restarting the buildbot
>>> master. IMO that is the Right Way to define steps: a script that you can
>>> run locally on a machine that satisfies the OS and dep requirements of the
>>> script.
>>>
>>> I am restarting the two bots that I am responsible for, and may need
>>> some help debugging further issues soon. I'll let you know.
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via cfe-commits]

Switched all but PPC, I don't have access to them. But they run the same
script as sanitizer-x86_64-linux.
http://lab.llvm.org:8014/#/waterfall?tags=sanitizer

On Mon, 12 Oct 2020 at 19:19, Galina Kistanova  wrote:

> We have a better version of AnnotatedCommand on the staging. It should be
> a functional equivalent of the old one.
> We need to stress test it well before moving to the production build bot.
>
> For that we need all sanitizer + other bots which use the AnnotatedCommand
> directly or indirectly moved temporarily to the staging.
>
> Please let me know when that could be arranged.
>
> Thanks
>
> Galina
>
> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:
>
>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>> lldb-comm...@lists.llvm.org> wrote:
>>
>>> They are online now -
>>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>>
>>> AnnotatedCommand has severe design conflict with the new buildbot.
>>> We have changed it to be safe and still do something useful, but it will
>>> need more love and care.
>>>
>>> Please let me know if you have some spare time to work on porting
>>> AnnotatedCommand.
>>>
>>
>> That's unfortunate, it would've been good to know that earlier. I and
>> another team member have spent a fair amount of time porting things to use
>> more AnnotatedCommand steps, because it gives us the flexibility to test
>> steps locally and make changes to the steps without restarting the buildbot
>> master. IMO that is the Right Way to define steps: a script that you can
>> run locally on a machine that satisfies the OS and dep requirements of the
>> script.
>>
>> I am restarting the two bots that I am responsible for, and may need some
>> help debugging further issues soon. I'll let you know.
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 913f600 - Canonicalize declaration pointers when forming APValues.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-12T19:32:57-07:00
New Revision: 913f6005669cfb590c99865a90bc51ed0983d09d

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

LOG: Canonicalize declaration pointers when forming APValues.

References to different declarations of the same entity aren't different
values, so shouldn't have different representations.

Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb, most recently
reverted in 9a33f027ac7d73e14ae287e78ab554142d1cbc8f due to a bug caused
by ObjCInterfaceDecls not propagating availability attributes along
their redeclaration chains; that bug was fixed in
e2d4174e9c66251d1b408234b53f53d0903c0285.

Added: 


Modified: 
clang/include/clang/AST/APValue.h
clang/lib/AST/APValue.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
clang/test/CodeGenCXX/weak-external.cpp
clang/test/OpenMP/ordered_messages.cpp

Removed: 




diff  --git a/clang/include/clang/AST/APValue.h 
b/clang/include/clang/AST/APValue.h
index 9e9468645fe7..ac8ed0818af0 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -177,6 +177,7 @@ class APValue {
   return !(LHS == RHS);
 }
 friend llvm::hash_code hash_value(const LValueBase );
+friend struct llvm::DenseMapInfo;
 
   private:
 PtrTy Ptr;
@@ -204,8 +205,7 @@ class APValue {
 
   public:
 LValuePathEntry() : Value() {}
-LValuePathEntry(BaseOrMemberType BaseOrMember)
-: Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
+LValuePathEntry(BaseOrMemberType BaseOrMember);
 static LValuePathEntry ArrayIndex(uint64_t Index) {
   LValuePathEntry Result;
   Result.Value = Index;

diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 22145beafd8d..7efd0caf3f1d 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -38,7 +38,7 @@ static_assert(
 "Type is insufficiently aligned");
 
 APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V)
-: Ptr(P), Local{I, V} {}
+: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr), Local{I, V} {}
 APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
 : Ptr(P), Local{I, V} {}
 
@@ -90,13 +90,19 @@ bool operator==(const APValue::LValueBase ,
 const APValue::LValueBase ) {
   if (LHS.Ptr != RHS.Ptr)
 return false;
-  if (LHS.is())
+  if (LHS.is() || LHS.is())
 return true;
   return LHS.Local.CallIndex == RHS.Local.CallIndex &&
  LHS.Local.Version == RHS.Local.Version;
 }
 }
 
+APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) {
+  if (const Decl *D = BaseOrMember.getPointer())
+BaseOrMember.setPointer(D->getCanonicalDecl());
+  Value = reinterpret_cast(BaseOrMember.getOpaqueValue());
+}
+
 void APValue::LValuePathEntry::profile(llvm::FoldingSetNodeID ) const {
   ID.AddInteger(Value);
 }
@@ -125,14 +131,16 @@ APValue::LValueBase::operator bool () const {
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getEmptyKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getEmptyKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getEmptyKey();
+  return B;
 }
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getTombstoneKey() {
-  return clang::APValue::LValueBase(
-  DenseMapInfo::getTombstoneKey());
+  clang::APValue::LValueBase B;
+  B.Ptr = DenseMapInfo::getTombstoneKey();
+  return B;
 }
 
 namespace clang {
@@ -926,8 +934,10 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, 
bool IsDerivedMember,
   assert(isAbsent() && "Bad state change");
   MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData;
   Kind = MemberPointer;
-  MPD->MemberAndIsDerivedMember.setPointer(Member);
+  MPD->MemberAndIsDerivedMember.setPointer(
+  Member ? cast(Member->getCanonicalDecl()) : nullptr);
   MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember);
   MPD->resizePath(Path.size());
-  memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const 
CXXRecordDecl*));
+  for (unsigned I = 0; I != Path.size(); ++I)
+MPD->getPath()[I] = Path[I]->getCanonicalDecl();
 }

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 6bb8aa026ad8..a6c7f30528eb 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4686,11 +4686,9 @@ char *Buffer = new (getASTContext(), 1) char[Name.size() 
+ 1];
 void ValueDecl::anchor() {}
 
 bool ValueDecl::isWeak() const {
-  for (const auto *I : attrs())
-if (isa(I) || isa(I))
-  return true;
-
-  return isWeakImported();
+  auto *MostRecent = getMostRecentDecl();
+  return 

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

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

rsmith added a comment.

In D76620#2324858 , @erichkeane wrote:

> The feature that this supports is a part of the SYCL 2020 Provisional Spec, I 
> thought that was sufficient.  We'll look into an RFC to re-submit in the 
> future.

Does that cover only functionality that can be implemented in terms of this 
builtin, or the builtin itself? If so, what is the functionality that needs to 
be supported? That information will be a good starting point for the RFC.

In D76620#2326499 , @keryell wrote:

> Otherwise, just removing a feature used for almost 6 months will cause some 
> kind of forking pain...

Well, there have at least not been any LLVM releases with this feature yet. 
(We're on the cusp of releasing LLVM 11, which would be the first release with 
this functionality.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89286: [DebugInfo] Check for templated static data member when adding constant to record static fields

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1421-1425
+  else if (auto *TemplateDecl = Var->getInstantiatedFromStaticDataMember()) {
+// Inline static data members might not have an initialization.
+if (TemplateDecl->getInit())
+  Value = TemplateDecl->evaluateValue();
+  }

I'd suspect this might not be the most general solution - because it's 
retrieving the value from the original template static member. Two issues come 
to mind:
1) If the initialization is dependent or otherwise not knowable at 
compile-time, does this code at least not crash (I assume "evaluateValue" 
returns nullptr if the value can't be evaluated for whatever reason)
2) If the initialization is dependent, it might still be knowable, eg:

```
template
struct static_decl_templ {
  static constexpr int static_constexpr_decl_templ_var = sizeof(T);
};
```

I'm guessing this patch as-is doesn't provide a value in this case, but it 
should be achievable, I would guess/hope/imagine - but don't really know 
exactly how to do that off-hand. (@zygoloid would know - but maybe a bit of 
looking around will find it if/before he has a chance to weigh in here)





Comment at: clang/test/CodeGenCXX/debug-info-static-member.cpp:140-144
+#if __cplusplus >= 201103L
+  static constexpr int static_constexpr_decl_templ_var = 8;
+#else
+  static const int static_constexpr_decl_templ_var = 8;
+#endif

I guess the non-11 version of this variable is intended as a stub to make the 
test across different language levels simpler?

I think it might be better to make that more explicit - rather than having a 
variable with "constexpr" in the name that isn't actually constexpr. I'd say 
only add the variable under that macro condition - but change the test to use 
multiple --check-prefixes, and the check for this variable to use a CPP11 
prefix or something like that.

Hmm, I'm slightly confused now that I Think about it - it looks like the macro 
is about testing if the version is >= 11. But the change also adds a test for 
C++17? Is C++17 likely to be any different than C++11 for any of this? (or 
C++20 for that matter) if it's not any different, I'd probably leave the 17 
case out & expect the 11 case is sufficient coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89286

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

Re: [clang] 9dcd96f - Canonicalize declaration pointers when forming APValues.

[Richard Smith via cfe-commits]

Thanks, yes, it looks like we don't properly inherit inheritable attributes
in redeclarations of ObjCInterfaceDecls.

Fixed in e2d4174e9c66251d1b408234b53f53d0903c0285. Let's try unreverting
and see if things look better.

On Mon, 12 Oct 2020 at 11:08, Arthur Eubanks  wrote:

> Could there be an issue with Objective C property attributes?
> https://crbug.com/1134762
>
> On Sun, Oct 11, 2020 at 2:11 PM Richard Smith 
> wrote:
>
>> Please can you try building with https://reviews.llvm.org/D89212 applied
>> first, and see if it produces any warnings? If so, you're probably hitting
>> PR47663, and should fix that by moving the 'weak' attribute earlier.
>>
>> On Sun, 11 Oct 2020 at 11:18, Richard Smith 
>> wrote:
>>
>>> On Fri, 9 Oct 2020 at 19:12, Arthur Eubanks  wrote:
>>>
 I think this is the cause of https://crbug.com/1134762. Can we
 speculatively revert while coming up with a repro? Or would you like a
 repro first?

>>>
>>> If you're blocked on this, sure, please go ahead and revert until you
>>> have a repro. But the revert will block ongoing development work, so a
>>> reproducer would definitely be appreciated! Per PR47663, there are some
>>> pretty fundamental problems with adding the 'weak' attribute "too late", so
>>> if that's what's going on, the best approach might be to move the 'weak'
>>> attribute to an earlier declaration.
>>>
>>> On Sun, Sep 27, 2020 at 7:06 PM Richard Smith via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

>
> Author: Richard Smith
> Date: 2020-09-27T19:05:26-07:00
> New Revision: 9dcd96f728863d40d6f5922ed52732fdd728fb5f
>
> URL:
> https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f
> DIFF:
> https://github.com/llvm/llvm-project/commit/9dcd96f728863d40d6f5922ed52732fdd728fb5f.diff
>
> LOG: Canonicalize declaration pointers when forming APValues.
>
> References to different declarations of the same entity aren't
> different
> values, so shouldn't have different representations.
>
> Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb with fixed
> handling
> for weak declarations. We now look for attributes on the most recent
> declaration when determining whether a declaration is weak. (Second
> recommit with further fixes for mishandling of weak declarations. Our
> behavior here is fundamentally unsound -- see PR47663 -- but this
> approach attempts to not make things worse.)
>
> Added:
>
>
> Modified:
> clang/include/clang/AST/APValue.h
> clang/lib/AST/APValue.cpp
> clang/lib/AST/Decl.cpp
> clang/lib/AST/DeclBase.cpp
> clang/lib/AST/ExprConstant.cpp
> clang/lib/CodeGen/CGExprConstant.cpp
> clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
> clang/test/CodeGenCXX/weak-external.cpp
> clang/test/OpenMP/ordered_messages.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/AST/APValue.h
> b/clang/include/clang/AST/APValue.h
> index 5103cfa8604e..6307f8a92e5a 100644
> --- a/clang/include/clang/AST/APValue.h
> +++ b/clang/include/clang/AST/APValue.h
> @@ -174,6 +174,7 @@ class APValue {
>return !(LHS == RHS);
>  }
>  friend llvm::hash_code hash_value(const LValueBase );
> +friend struct llvm::DenseMapInfo;
>
>private:
>  PtrTy Ptr;
> @@ -201,8 +202,7 @@ class APValue {
>
>public:
>  LValuePathEntry() : Value() {}
> -LValuePathEntry(BaseOrMemberType BaseOrMember)
> -:
> Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
> +LValuePathEntry(BaseOrMemberType BaseOrMember);
>  static LValuePathEntry ArrayIndex(uint64_t Index) {
>LValuePathEntry Result;
>Result.Value = Index;
>
> diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
> index 08ae0ff3c67d..32d3ff7ce1d0 100644
> --- a/clang/lib/AST/APValue.cpp
> +++ b/clang/lib/AST/APValue.cpp
> @@ -38,7 +38,7 @@ static_assert(
>  "Type is insufficiently aligned");
>
>  APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I,
> unsigned V)
> -: Ptr(P), Local{I, V} {}
> +: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr),
> Local{I, V} {}
>  APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
>  : Ptr(P), Local{I, V} {}
>
> @@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase ,
>  const APValue::LValueBase ) {
>if (LHS.Ptr != RHS.Ptr)
>  return false;
> -  if (LHS.is())
> +  if (LHS.is() || LHS.is())
>  return true;
>return LHS.Local.CallIndex == RHS.Local.CallIndex &&
>   LHS.Local.Version == 

[clang] e2d4174 - Ensure that InheritedAttrs are properly inherited along a redeclaration

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-12T19:22:31-07:00
New Revision: e2d4174e9c66251d1b408234b53f53d0903c0285

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

LOG: Ensure that InheritedAttrs are properly inherited along a redeclaration
chain for ObjCInterfaceDecls.

Only one such declaration can actually have attributes (the definition,
if any), but generally we assume that we can look for InheritedAttrs on
the most recent declaration.

Added: 


Modified: 
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/CodeGenObjC/attr-availability.m

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 6ef6fd1d8c1c..733a50e7da5a 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -1066,6 +1066,11 @@ Decl *Sema::ActOnStartClassInterface(
 
   ProcessDeclAttributeList(TUScope, IDecl, AttrList);
   AddPragmaAttributes(TUScope, IDecl);
+
+  // Merge attributes from previous declarations.
+  if (PrevIDecl)
+mergeDeclAttributes(IDecl, PrevIDecl);
+
   PushOnScopeChains(IDecl, TUScope);
 
   // Start the definition of this class. If we're in a redefinition case, there
@@ -3125,6 +3130,9 @@ Sema::ActOnForwardClassDeclaration(SourceLocation 
AtClassLoc,
   IdentLocs[i]);
 IDecl->setAtEndRange(IdentLocs[i]);
 
+if (PrevIDecl)
+  mergeDeclAttributes(IDecl, PrevIDecl);
+
 PushOnScopeChains(IDecl, TUScope);
 CheckObjCDeclScope(IDecl);
 DeclsInGroup.push_back(IDecl);

diff  --git a/clang/test/CodeGenObjC/attr-availability.m 
b/clang/test/CodeGenObjC/attr-availability.m
index 375a5be4fadd..9ed7678eafb9 100644
--- a/clang/test/CodeGenObjC/attr-availability.m
+++ b/clang/test/CodeGenObjC/attr-availability.m
@@ -22,3 +22,14 @@ @implementation WeakClass2(MyCategory) @end
 
 @implementation WeakClass2(YourCategory) @end
 
+// CHECK-10_4: @"OBJC_CLASS_$_WeakClass3" = extern_weak global
+// CHECK-10_5: @"OBJC_CLASS_$_WeakClass3" = extern_weak global
+// CHECK-10_6: @"OBJC_CLASS_$_WeakClass3" = external global
+__attribute__((availability(macosx,introduced=10.6)))
+@interface WeakClass3 @end
+@class WeakClass3;
+
+@implementation WeakClass3(MyCategory) @end
+
+@implementation WeakClass3(YourCategory) @end
+



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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Galina Kistanova via cfe-commits]

We have a better version of AnnotatedCommand on the staging. It should be a
functional equivalent of the old one.
We need to stress test it well before moving to the production build bot.

For that we need all sanitizer + other bots which use the AnnotatedCommand
directly or indirectly moved temporarily to the staging.

Please let me know when that could be arranged.

Thanks

Galina

On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:

> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
> lldb-comm...@lists.llvm.org> wrote:
>
>> They are online now - http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>
>> AnnotatedCommand has severe design conflict with the new buildbot.
>> We have changed it to be safe and still do something useful, but it will
>> need more love and care.
>>
>> Please let me know if you have some spare time to work on porting
>> AnnotatedCommand.
>>
>
> That's unfortunate, it would've been good to know that earlier. I and
> another team member have spent a fair amount of time porting things to use
> more AnnotatedCommand steps, because it gives us the flexibility to test
> steps locally and make changes to the steps without restarting the buildbot
> master. IMO that is the Right Way to define steps: a script that you can
> run locally on a machine that satisfies the OS and dep requirements of the
> script.
>
> I am restarting the two bots that I am responsible for, and may need some
> help debugging further issues soon. I'll let you know.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

[Ronan Keryell via Phabricator via cfe-commits]

keryell added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3704-3705
+  Result = E->EvaluateAsRValue(Eval, Context, true);
+else
+  Result = E->EvaluateAsLValue(Eval, Context, true);
+

Tyker wrote:
> keryell wrote:
> > aaron.ballman wrote:
> > > Tyker wrote:
> > > > aaron.ballman wrote:
> > > > > Under what circumstances would we want the constant expressions to be 
> > > > > lvalues? I would have guessed you would want to call 
> > > > > `Expr::EvaluateAsConstantExpr()` instead of either of these.
> > > > Expr::EvaluateAsConstantExpr will evaluate expression in there value 
> > > > category.
> > > > this can be quite surprising in some situations like:
> > > > ```
> > > > const int g_i = 0;
> > > > [[clang::annotate("test", g_i)]] void t() {} // annotation carries a 
> > > > pointer/reference on g_i
> > > > [[clang::annotate("test", (int)g_i)]] void t1() {} // annotation 
> > > > carries the value 0
> > > > [[clang::annotate("test", g_i + 0)]] void t1() {} // annotation carries 
> > > > the value 0
> > > > 
> > > > ```
> > > > with EvaluateAsRValue in all of the cases above the annotation will 
> > > > carry the value 0.
> > > > 
> > > > optionally we could wrap expression with an LValue to RValue cast and 
> > > > call EvaluateAsConstantExpr this should also work.
> > > Thank you for the explanation. I think wrapping with an lvalue to rvalue 
> > > conversion may make more sense -- `EvaluateAsRValue()` tries really hard 
> > > to get an rvalue out of something even if the standard says that's not 
> > > okay. It'll automatically apply the lvalue to rvalue conversion if 
> > > appropriate, but it'll also do more than that (such as evaluating side 
> > > effects).
> > This needs some motivating comments and explanations in the code.
> > Also the variations on `g_i` annotation above are quite interesting and 
> > should appear in the unit tests. I am unsure they are now.
> > Perhaps more interesting with other values than `0` in the example. :-)
> > I guess we can still use `[[clang::annotate("test", _i)]] void t() {}` to 
> > have a pointer on `g_i` and `(int&)g_i` for reference? To add to the unit 
> > tests if not already checked.
> > 
> i changed to use EvaluateAsConstantExpr + LValueToRvalue cast.
> 
> > This needs some motivating comments and explanations in the code.
> > Also the variations on `g_i` annotation above are quite interesting and 
> > should appear in the unit tests. I am unsure they are now.
> they are tested not as expressively as above but they are tested.
> 
> > Perhaps more interesting with other values than `0` in the example. :-)
> > I guess we can still use `[[clang::annotate("test", _i)]] void t() {}` to 
> > have a pointer on `g_i`
> yes this is how it is working.
> > `(int&)g_i` for reference? To add to the unit tests if not already checked.
> (int&)g_i has the salve value category and same type as g_i it just has a 
> noop cast around it.
> pointer an reference are indistinguishable in an APValue the only difference 
> is the c++ type of what they represent.
> and they are of course lowered to the same thing. so we only need the pointer 
> case.
> 
I see. Thanks for the clarification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88645

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

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[JunMa via Phabricator via cfe-commits]

junparser added a comment.

In D89066#2325358 , @lxfind wrote:

> In D89066#2324291 , @junparser wrote:
>
>> In D89066#2324151 , @lxfind wrote:
>>
>>> In D89066#2324115 , @junparser 
>>> wrote:
>>>
 why we should not do this with normal await call?
>>>
>>> To be honest, I don't know yet. My understanding of how expression cleanup 
>>> and temp lifetime management is insufficient at the moment.
>>> But first of all, without adding any cleanup expression here, I saw ASAN 
>>> failures due to heap-use-after-free, because sometimes the frame have 
>>> already been destroyed after the await_suspend call, and yet we are still 
>>> writing into the frame due to unnecessarily cross-suspend lifetime. 
>>> However, if I apply the cleanup to all await_suepend calls, it also causes 
>>> ASAN failures as it's cleaning up data that's still alive.
>>> So this patch is more of a temporary walkaround to stop bleeding without 
>>> causing any trouble.
>>> I plan to get back to this latter after I am done with the spilling/alloca 
>>> issues.
>>
>> I'm not familiar with ASAN instrumentation. Do you have any testcases to 
>> explain this?
>
> Unfortunately I don't.  But this is not related to ASAN. Basically, this is 
> causing destructing of objects that should still be alive. I suspect that 
> it's because ExprWithCleanups always clean up temps that belongs to the full 
> expression, not just the sub-expression in it.

should we also need to add ExprWithCleanups when expand co_await to await_ready 
& await_suspend and await_resume expression which may fix this issue？


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

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

[PATCH] D89286: [DebugInfo] Check for templated static data member when adding constant to record static fields

[Amy Huang via Phabricator via cfe-commits]

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

Debug info for inline static data members was missing the constant value,
because the initializer for these static data members is not always emitted.

Now also try to get the initializer from the templated static data member if
the variable doesn't have an initializer.

(related to discussion on https://bugs.llvm.org/show_bug.cgi?id=47580)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89286

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-static-member.cpp


Index: clang/test/CodeGenCXX/debug-info-static-member.cpp
===
--- clang/test/CodeGenCXX/debug-info-static-member.cpp
+++ clang/test/CodeGenCXX/debug-info-static-member.cpp
@@ -1,6 +1,7 @@
 // RUN: %clangxx -target x86_64-unknown-unknown -g %s -emit-llvm -S -o - | 
FileCheck %s
 // RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++98 %s -emit-llvm -S 
-o - | FileCheck %s
 // RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++11 %s -emit-llvm -S 
-o - | FileCheck %s
+// RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++17 %s -emit-llvm -S 
-o - | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -gcodeview 
-debug-info-kind=limited %s -emit-llvm -o - | FileCheck --check-prefix MSVC %s
 // PR14471
 
@@ -45,6 +46,9 @@
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_decl_templ_var"
+// CHECK-SAME:   extraData: i32 7
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: 
"static_constexpr_decl_templ_var"
+// CHECK-SAME:   extraData: i32 8
 
 int C::a = 4;
 // CHECK: [[B]] = !DIGlobalVariableExpression(var: [[BV:.*]], expr: 
!DIExpression())
@@ -133,6 +137,11 @@
 template
 struct static_decl_templ {
   static const int static_decl_templ_var = 7;
+#if __cplusplus >= 201103L
+  static constexpr int static_constexpr_decl_templ_var = 8;
+#else
+  static const int static_constexpr_decl_templ_var = 8;
+#endif
 };
 
 template
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1415,14 +1415,20 @@
   unsigned LineNumber = getLineNumber(Var->getLocation());
   StringRef VName = Var->getName();
   llvm::Constant *C = nullptr;
-  if (Var->getInit()) {
-const APValue *Value = Var->evaluateValue();
-if (Value) {
-  if (Value->isInt())
-C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
-  if (Value->isFloat())
-C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
-}
+  APValue *Value = nullptr;
+  if (Var->getInit())
+Value = Var->evaluateValue();
+  else if (auto *TemplateDecl = Var->getInstantiatedFromStaticDataMember()) {
+// Inline static data members might not have an initialization.
+if (TemplateDecl->getInit())
+  Value = TemplateDecl->evaluateValue();
+  }
+
+  if (Value) {
+if (Value->isInt())
+  C = llvm::ConstantInt::get(CGM.getLLVMContext(), Value->getInt());
+if (Value->isFloat())
+  C = llvm::ConstantFP::get(CGM.getLLVMContext(), Value->getFloat());
   }
 
   llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);


Index: clang/test/CodeGenCXX/debug-info-static-member.cpp
===
--- clang/test/CodeGenCXX/debug-info-static-member.cpp
+++ clang/test/CodeGenCXX/debug-info-static-member.cpp
@@ -1,6 +1,7 @@
 // RUN: %clangxx -target x86_64-unknown-unknown -g %s -emit-llvm -S -o - | FileCheck %s
 // RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++98 %s -emit-llvm -S -o - | FileCheck %s
 // RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++11 %s -emit-llvm -S -o - | FileCheck %s
+// RUN: %clangxx -target x86_64-unknown-unknown -g -std=c++17 %s -emit-llvm -S -o - | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -gcodeview -debug-info-kind=limited %s -emit-llvm -o - | FileCheck --check-prefix MSVC %s
 // PR14471
 
@@ -45,6 +46,9 @@
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_decl_templ_var"
+// CHECK-SAME:   extraData: i32 7
+// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "static_constexpr_decl_templ_var"
+// CHECK-SAME:   extraData: i32 8
 
 int C::a = 4;
 // CHECK: [[B]] = !DIGlobalVariableExpression(var: [[BV:.*]], expr: !DIExpression())
@@ -133,6 +137,11 @@
 template
 struct static_decl_templ {
   static const int static_decl_templ_var = 7;
+#if __cplusplus >= 201103L
+  static constexpr int static_constexpr_decl_templ_var = 8;
+#else
+  static const int 

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a reviewer: Ralender.
keryell added a comment.

Enabling this feature only with SYCL seems like an easy and quick mitigation, 
as SYCL compilers downstream can easily update their runtime to the new builtin 
name.
Otherwise, just removing a feature used for almost 6 months will cause some 
kind of forking pain...
Anyway, improving the feature and implementation at the same time with an RFC 
for a wider usage looks like a great idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89102: [X86] Add HRESET instruction.

[Pengfei Wang 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 rG412cdcf2edf2: [X86] Add HRESET instruction. (authored by 
pengfei).

Changed prior to commit:
  https://reviews.llvm.org/D89102?vs=297402=297732#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89102

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/cpuid.h
  clang/lib/Headers/hresetintrin.h
  clang/lib/Headers/immintrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/x86-hreset-intrin.c
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/x86_target_features.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86InstrFormats.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/test/MC/Disassembler/X86/x86-32.txt
  llvm/test/MC/Disassembler/X86/x86-64.txt
  llvm/test/MC/X86/x86-32-coverage.s
  llvm/test/MC/X86/x86-64.s

Index: llvm/test/MC/X86/x86-64.s
===
--- llvm/test/MC/X86/x86-64.s
+++ llvm/test/MC/X86/x86-64.s
@@ -2014,3 +2014,7 @@
 // CHECK: tdcall
 // CHECK: encoding: [0x66,0x0f,0x01,0xcc]
 tdcall
+
+// CHECK: hreset
+// CHECK: encoding: [0xf3,0x0f,0x3a,0xf0,0xc0,0x01]
+hreset $1
Index: llvm/test/MC/X86/x86-32-coverage.s
===
--- llvm/test/MC/X86/x86-32-coverage.s
+++ llvm/test/MC/X86/x86-32-coverage.s
@@ -10891,4 +10891,8 @@
 
 // CHECK: tdcall
 // CHECK: encoding: [0x66,0x0f,0x01,0xcc]
-tdcall
\ No newline at end of file
+tdcall
+
+// CHECK: hreset
+// CHECK: encoding: [0xf3,0x0f,0x3a,0xf0,0xc0,0x01]
+hreset $1
Index: llvm/test/MC/Disassembler/X86/x86-64.txt
===
--- llvm/test/MC/Disassembler/X86/x86-64.txt
+++ llvm/test/MC/Disassembler/X86/x86-64.txt
@@ -712,3 +712,6 @@
 
 #CHECK: tdcall
 0x66 0x0f 0x01 0xcc
+
+# CHECK: hreset $1
+0xf3 0x0f 0x3a 0xf0 0xc0 0x01
Index: llvm/test/MC/Disassembler/X86/x86-32.txt
===
--- llvm/test/MC/Disassembler/X86/x86-32.txt
+++ llvm/test/MC/Disassembler/X86/x86-32.txt
@@ -1000,3 +1000,6 @@
 
 #CHECK: tdcall
 0x66 0x0f 0x01 0xcc
+
+# CHECK: hreset $1
+0xf3 0x0f 0x3a 0xf0 0xc0 0x01
Index: llvm/lib/Target/X86/X86Subtarget.h
===
--- llvm/lib/Target/X86/X86Subtarget.h
+++ llvm/lib/Target/X86/X86Subtarget.h
@@ -401,6 +401,9 @@
   /// Processor support key locker wide instructions
   bool HasWIDEKL = false;
 
+  /// Processor supports HRESET instruction
+  bool HasHRESET = false;
+
   /// Processor supports SERIALIZE instruction
   bool HasSERIALIZE = false;
 
@@ -736,6 +739,7 @@
   bool hasENQCMD() const { return HasENQCMD; }
   bool hasKL() const { return HasKL; }
   bool hasWIDEKL() const { return HasWIDEKL; }
+  bool hasHRESET() const { return HasHRESET; }
   bool hasSERIALIZE() const { return HasSERIALIZE; }
   bool hasTSXLDTRK() const { return HasTSXLDTRK; }
   bool useRetpolineIndirectCalls() const { return UseRetpolineIndirectCalls; }
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -972,6 +972,7 @@
 def HasENQCMD: Predicate<"Subtarget->hasENQCMD()">;
 def HasKL: Predicate<"Subtarget->hasKL()">;
 def HasWIDEKL: Predicate<"Subtarget->hasWIDEKL()">;
+def HasHRESET: Predicate<"Subtarget->hasHRESET()">;
 def HasSERIALIZE : Predicate<"Subtarget->hasSERIALIZE()">;
 def HasTSXLDTRK  : Predicate<"Subtarget->hasTSXLDTRK()">;
 def HasAMXTILE   : Predicate<"Subtarget->hasAMXTILE()">;
@@ -2913,6 +2914,13 @@
 def : InstAlias<"clzero\t{%eax|eax}", (CLZERO32r)>, Requires<[Not64BitMode]>;
 def : InstAlias<"clzero\t{%rax|rax}", (CLZERO64r)>, Requires<[In64BitMode]>;
 
+//===--===//
+// HRESET Instruction
+//
+let Uses = [EAX], SchedRW = [WriteSystem] in
+  def HRESET : Ii8<0xF0, MRM_C0, (outs), (ins i32u8imm:$imm), "hreset\t$imm", []>,
+   Requires<[HasHRESET]>, TAXS;
+
 //===--===//
 // SERIALIZE Instruction
 //
Index: llvm/lib/Target/X86/X86InstrFormats.td
===
--- llvm/lib/Target/X86/X86InstrFormats.td
+++ llvm/lib/Target/X86/X86InstrFormats.td
@@ -216,6 +216,7 @@
 class TAPS : TA { Prefix OpPrefix = PS; 

[clang] 412cdcf - [X86] Add HRESET instruction.

[via cfe-commits]

Author: Wang, Pengfei
Date: 2020-10-13T08:47:26+08:00
New Revision: 412cdcf2edf2344632e01d5f71da4bbd9838ab7d

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

LOG: [X86] Add HRESET instruction.

For more details about these instructions, please refer to the latest ISE 
document: 
https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference.

Reviewed By: craig.topper

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

Added: 
clang/lib/Headers/hresetintrin.h
clang/lib/Headers/x86gprintrin.h
clang/test/CodeGen/x86-hreset-intrin.c

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/cpuid.h
clang/lib/Headers/immintrin.h
clang/test/Driver/x86-target-features.c
clang/test/Preprocessor/x86_target_features.c
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/Support/X86TargetParser.def
llvm/lib/Support/Host.cpp
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Target/X86/X86.td
llvm/lib/Target/X86/X86InstrFormats.td
llvm/lib/Target/X86/X86InstrInfo.td
llvm/lib/Target/X86/X86Subtarget.h
llvm/test/MC/Disassembler/X86/x86-32.txt
llvm/test/MC/Disassembler/X86/x86-64.txt
llvm/test/MC/X86/x86-32-coverage.s
llvm/test/MC/X86/x86-64.s

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 10e0203ce241..ff3decbca70c 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -3261,6 +3261,8 @@ X86
 
 .. option:: -mgfni, -mno-gfni
 
+.. option:: -mhreset, -mno-hreset
+
 .. option:: -minvpcid, -mno-invpcid
 
 .. option:: -mkl, -mno-kl

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8e0343710d68..20acd2072068 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3260,6 +3260,8 @@ def minvpcid : Flag<["-"], "minvpcid">, 
Group;
 def mno_invpcid : Flag<["-"], "mno-invpcid">, Group;
 def mgfni : Flag<["-"], "mgfni">, Group;
 def mno_gfni : Flag<["-"], "mno-gfni">, Group;
+def mhreset : Flag<["-"], "mhreset">, Group;
+def mno_hreset : Flag<["-"], "mno-hreset">, Group;
 def mkl : Flag<["-"], "mkl">, Group;
 def mno_kl : Flag<["-"], "mno-kl">, Group;
 def mwidekl : Flag<["-"], "mwidekl">, Group;

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 98ac13b1ae9b..9b607a3b3941 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -298,6 +298,8 @@ bool 
X86TargetInfo::handleTargetFeatures(std::vector ,
   HasINVPCID = true;
 } else if (Feature == "+enqcmd") {
   HasENQCMD = true;
+} else if (Feature == "+hreset") {
+  HasHRESET = true;
 } else if (Feature == "+amx-bf16") {
   HasAMXBF16 = true;
 } else if (Feature == "+amx-int8") {
@@ -712,6 +714,8 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
,
 Builder.defineMacro("__INVPCID__");
   if (HasENQCMD)
 Builder.defineMacro("__ENQCMD__");
+  if (HasHRESET)
+Builder.defineMacro("__HRESET__");
   if (HasAMXTILE)
 Builder.defineMacro("__AMXTILE__");
   if (HasAMXINT8)
@@ -848,6 +852,7 @@ bool X86TargetInfo::isValidFeatureName(StringRef Name) 
const {
   .Case("fsgsbase", true)
   .Case("fxsr", true)
   .Case("gfni", true)
+  .Case("hreset", true)
   .Case("invpcid", true)
   .Case("kl", true)
   .Case("widekl", true)
@@ -936,6 +941,7 @@ bool X86TargetInfo::hasFeature(StringRef Feature) const {
   .Case("fsgsbase", HasFSGSBASE)
   .Case("fxsr", HasFXSR)
   .Case("gfni", HasGFNI)
+  .Case("hreset", HasHRESET)
   .Case("invpcid", HasINVPCID)
   .Case("kl", HasKL)
   .Case("widekl", HasWIDEKL)

diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 4fc495a09bbb..441ab961e293 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -129,6 +129,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
   bool HasENQCMD = false;
   bool HasKL = false;  // For key locker
   bool HasWIDEKL = false; // For wide key locker
+  bool HasHRESET = false;
   bool HasAMXTILE = false;
   bool HasAMXINT8 = false;
   bool HasAMXBF16 = false;

diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index 533ff4506ffe..7d0b2a0938ba 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -65,6 +65,7 @@ set(files
   fmaintrin.h
   fxsrintrin.h
   gfniintrin.h
+  hresetintrin.h
   htmintrin.h
   

[PATCH] D89284: [clangd] Propagate CollectMainFileRefs to BackgroundIndex

[Nathan Ridge via Phabricator via cfe-commits]

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

This appears to have been an omission in D83536 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89284

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


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -218,6 +218,7 @@
 BGOpts.ContextProvider = [this](PathRef P) {
   return createProcessingContext(P);
 };
+BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
 BackgroundIdx = std::make_unique(
 TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -218,6 +218,7 @@
 BGOpts.ContextProvider = [this](PathRef P) {
   return createProcessingContext(P);
 };
+BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
 BackgroundIdx = std::make_unique(
 TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

@plotfi Sorry this work has stalled, unfortunately I haven't had any bandwidth 
to drive it forward.

At this point I don't think there are any outstanding concerns with this patch. 
If anyone is willing to rebase and land it, I would be really grateful. It 
looks like part of the motivation behind this is removing some downstream diff 
with the apple/llvm-project fork (as in D89078 
). I think extra work might be needed to do 
that, since the HotColdSplitting pass is on-by-default in the Apple fork. 
Turning the pass on by default on llvm.org could result in a fair amount of 
fallout, as the pass might interact badly with the machine function splitter 
(https://lists.llvm.org/pipermail/llvm-dev/2020-August/144012.html). It also 
hasn't had as much testing on non-Darwin targets (afaik). One solution might be 
to enable -fsplit-cold-code under -O only for Darwin targets.


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

https://reviews.llvm.org/D57265

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

[PATCH] D89276: Support ObjC in IncludeInserter

[Joe Turner via Phabricator via cfe-commits]

compositeprimes updated this revision to Diff 297728.
compositeprimes edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -137,6 +137,18 @@
   }
 };
 
+class ObjCGeneratedHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCGeneratedHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector headersToInclude() const override {
+return {"clang_tidy/tests/generated_file.proto.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -156,6 +168,9 @@
   {"path/to/z/header.h", "\n"},
   {"path/to/header.h", "\n"},
   {"path/to/header2.h", "\n"},
+  // Generated headers
+  {"clang_tidy/tests/"
+   "generated_file.proto.h", "\n"},
   // Fake system headers.
   {"stdlib.h", "\n"},
   {"unistd.h", "\n"},
@@ -706,6 +721,37 @@
"insert_includes_test_header.mm"));
 }
 
+TEST(IncludeInserterTest, InsertGeneratedHeaderObjectiveC) {
+  const char *PreCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+#include 
+#include 
+
+#include "path/to/a/header.h"
+
+void foo() {
+  int a = 0;
+})";
+  const char *PostCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+#include 
+#include 
+
+#include "path/to/a/header.h"
+
+#import "clang_tidy/tests/generated_file.proto.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "devtools/cymbal/clang_tidy/tests/"
+   "insert_includes_test_header.mm"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -31,7 +31,8 @@
 IK_CSystemInclude = 1,   ///< e.g. ``#include ``
 IK_CXXSystemInclude = 2, ///< e.g. ``#include ``
 IK_NonSystemInclude = 3, ///< e.g. ``#include "bar.h"``
-IK_InvalidInclude = 4///< total number of valid ``IncludeKind``s
+IK_GeneratedInclude = 4, ///< e.g. ``#include "bar.proto.h"``
+IK_InvalidInclude = 5///< total number of valid ``IncludeKind``s
   };
 
   /// ``IncludeSorter`` constructor; takes the FileID and name of the file to be
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -91,6 +91,12 @@
   return IncludeSorter::IK_MainTUInclude;
 }
   }
+  if (Style == IncludeSorter::IS_Google_ObjC) {
+if (IncludeFile.endswith(".generated.h") ||
+IncludeFile.endswith(".proto.h") || IncludeFile.endswith(".pbobjc.h")) {
+  return IncludeSorter::IK_GeneratedInclude;
+}
+  }
   return IncludeSorter::IK_NonSystemInclude;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89078: Add `-f[no-]split-cold-code` toggling outlining & enable in -O

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

@compnerd thank you for working on upstreaming this patch! Would you be open to 
commandeering D57265  instead? This is my 
(unfortunately stalled) attempt to upstream the same patch, and it has some 
review concerns from Fedor Sergeev, Philip Pfaffe, and others addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89078

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

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/Basic/LangOptions.h:299
+  /// This overrides the default ABI used by the target.
+  llvm::Optional CXXABIOverride;
+

I'd call it just `CXXABI` which matches the other variables.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3469
+
+  // The value can be empty, which indicates the system default should be used.
+  StringRef CXXABI = Args.getLastArgValue(OPT_fcxx_abi_EQ);

Other flags like `-funwindlib` use either empty string or `"platform"`, perhaps 
we should support the same for `-fc++-abi`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2174
+  }
+
   unsigned ArgNo = 0;

rsmith wrote:
> CJ-Johnson wrote:
> > jdoerfert wrote:
> > > Even if null pointer is valid we should place dereferenceable.
> > > 
> > > We also could never place nonnull and let the middle-end make the 
> > > dereferenceable -> nonnull deduction, though I don't see why we can't 
> > > just add nonnull here.
> > I re-ran ninja check after making this fix and addressed the newly-affected 
> > tests. So the patch is fully up to date :)
> The LLVM LangRef says that in address space 0, `dereferenceable` implies 
> `nonnull`, so I don't think we can emit `dereferenceable` in 
> `NullPointerIsValid` mode, and we'd need to use `dereferenceable_or_null` 
> instead. (Perhaps the LangRef is wrong, though, and the 
> `null_pointer_is_valid` function attribute overrides that determination.)
> (Perhaps the LangRef is wrong, though, and the null_pointer_is_valid function 
> attribute overrides that determination.)

This is the case. IMHO, dereferenceable has to be correct here, regardless of 
the mode. You could access the object in the function entry, which we would use 
to deduce dereferenceable, and if the `NullPointerIsValid` is not translated to 
the function attribute also to `nonnull`. 


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

https://reviews.llvm.org/D17993

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

[clang] cffb0dd - [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2020-10-12T16:48:50-07:00
New Revision: cffb0dd54d41d8e249d2009467c4beb5b681ba26

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

LOG: [SemaTemplate] Stop passing insertion position around during VarTemplate 
instantiation

They can get stale at use time because of updates from other recursive
specializations. Instead, rely on the existence of previous declarations to add
the specialization.

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/Template.h
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaTemplate/instantiate-var-template.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ca1f87cfdb2b..b5f0c08300bf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9171,7 +9171,7 @@ class Sema final {
   const TemplateArgumentList ,
   const TemplateArgumentListInfo ,
   SmallVectorImpl ,
-  SourceLocation PointOfInstantiation, void *InsertPos,
+  SourceLocation PointOfInstantiation,
   LateInstantiatedAttrVec *LateAttrs = nullptr,
   LocalInstantiationScope *StartingScope = nullptr);
   VarTemplateSpecializationDecl *CompleteVarTemplateSpecializationDecl(

diff  --git a/clang/include/clang/Sema/Template.h 
b/clang/include/clang/Sema/Template.h
index 91d175fdd050..0dcaf565591b 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -600,7 +600,7 @@ enum class TemplateSubstitutionKind : char {
 TagDecl *NewDecl);
 
 Decl *VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *FromVar, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *FromVar,
 const TemplateArgumentListInfo ,
 ArrayRef Converted,
 VarTemplateSpecializationDecl *PrevDecl = nullptr);

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 8baf5b96fbf8..4ecae8faad66 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4584,7 +4584,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, 
SourceLocation TemplateLoc,
   // FIXME: LateAttrs et al.?
   VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
   Template, InstantiationPattern, *InstantiationArgs, TemplateArgs,
-  Converted, TemplateNameLoc, InsertPos /*, LateAttrs, StartingScope*/);
+  Converted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
   if (!Decl)
 return true;
 

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9420bd04b7a9..7200dc72825d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3628,11 +3628,11 @@ Decl 
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
 return nullptr;
 
   return VisitVarTemplateSpecializationDecl(
-  InstVarTemplate, D, InsertPos, VarTemplateArgsInfo, Converted, PrevDecl);
+  InstVarTemplate, D, VarTemplateArgsInfo, Converted, PrevDecl);
 }
 
 Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *D, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *D,
 const TemplateArgumentListInfo ,
 ArrayRef Converted,
 VarTemplateSpecializationDecl *PrevDecl) {
@@ -3655,8 +3655,11 @@ Decl 
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
   SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
   VarTemplate, DI->getType(), DI, D->getStorageClass(), Converted);
   Var->setTemplateArgsInfo(TemplateArgsInfo);
-  if (InsertPos)
+  if (!PrevDecl) {
+void *InsertPos = nullptr;
+VarTemplate->findSpecialization(Converted, InsertPos);
 VarTemplate->AddSpecialization(Var, InsertPos);
+  }
 
   if (SemaRef.getLangOpts().OpenCL)
 SemaRef.deduceOpenCLAddressSpace(Var);
@@ -4865,7 +4868,7 @@ VarTemplateSpecializationDecl 
*Sema::BuildVarTemplateInstantiation(
 const TemplateArgumentList ,
 const TemplateArgumentListInfo ,
 SmallVectorImpl ,
-SourceLocation PointOfInstantiation, void *InsertPos,
+SourceLocation PointOfInstantiation,
 LateInstantiatedAttrVec *LateAttrs,
 LocalInstantiationScope *StartingScope) {
   if (FromVar->isInvalidDecl())
@@ -4904,7 +4907,7 @@ VarTemplateSpecializationDecl 
*Sema::BuildVarTemplateInstantiation(
 
   return cast_or_null(
   Instantiator.VisitVarTemplateSpecializationDecl(
-  VarTemplate, FromVar, InsertPos, TemplateArgsInfo, Converted));
+  VarTemplate, 

[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcffb0dd54d41: [SemaTemplate] Stop passing insertion position 
around during VarTemplate… (authored by bruno).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87853

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp

Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -40,3 +40,10 @@
   template A models;
   template<> struct B models<>; // expected-error {{incomplete type 'struct B'}} expected-note {{forward declaration}}
 }
+
+namespace InvalidInsertPos {
+  template T v;
+  template decltype(v) v;
+  template<> int v;
+  int k = v;
+}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3628,11 +3628,11 @@
 return nullptr;
 
   return VisitVarTemplateSpecializationDecl(
-  InstVarTemplate, D, InsertPos, VarTemplateArgsInfo, Converted, PrevDecl);
+  InstVarTemplate, D, VarTemplateArgsInfo, Converted, PrevDecl);
 }
 
 Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *D, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *D,
 const TemplateArgumentListInfo ,
 ArrayRef Converted,
 VarTemplateSpecializationDecl *PrevDecl) {
@@ -3655,8 +3655,11 @@
   SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
   VarTemplate, DI->getType(), DI, D->getStorageClass(), Converted);
   Var->setTemplateArgsInfo(TemplateArgsInfo);
-  if (InsertPos)
+  if (!PrevDecl) {
+void *InsertPos = nullptr;
+VarTemplate->findSpecialization(Converted, InsertPos);
 VarTemplate->AddSpecialization(Var, InsertPos);
+  }
 
   if (SemaRef.getLangOpts().OpenCL)
 SemaRef.deduceOpenCLAddressSpace(Var);
@@ -4865,7 +4868,7 @@
 const TemplateArgumentList ,
 const TemplateArgumentListInfo ,
 SmallVectorImpl ,
-SourceLocation PointOfInstantiation, void *InsertPos,
+SourceLocation PointOfInstantiation,
 LateInstantiatedAttrVec *LateAttrs,
 LocalInstantiationScope *StartingScope) {
   if (FromVar->isInvalidDecl())
@@ -4904,7 +4907,7 @@
 
   return cast_or_null(
   Instantiator.VisitVarTemplateSpecializationDecl(
-  VarTemplate, FromVar, InsertPos, TemplateArgsInfo, Converted));
+  VarTemplate, FromVar, TemplateArgsInfo, Converted));
 }
 
 /// Instantiates a variable template specialization by completing it
@@ -5327,8 +5330,8 @@
 TemplateDeclInstantiator Instantiator(*this, Var->getDeclContext(),
   TemplateArgs);
 Var = cast_or_null(Instantiator.VisitVarTemplateSpecializationDecl(
-VarSpec->getSpecializedTemplate(), Def, nullptr,
-VarSpec->getTemplateArgsInfo(), VarSpec->getTemplateArgs().asArray()));
+VarSpec->getSpecializedTemplate(), Def, VarSpec->getTemplateArgsInfo(),
+VarSpec->getTemplateArgs().asArray(), VarSpec));
 if (Var) {
   llvm::PointerUnion PatternPtr =
@@ -5338,12 +5341,6 @@
 cast(Var)->setInstantiationOf(
 Partial, >getTemplateInstantiationArgs());
 
-  // Merge the definition with the declaration.
-  LookupResult R(*this, Var->getDeclName(), Var->getLocation(),
- LookupOrdinaryName, forRedeclarationInCurContext());
-  R.addDecl(OldVar);
-  MergeVarDecl(Var, R);
-
   // Attach the initializer.
   InstantiateVariableInitializer(Var, Def, TemplateArgs);
 }
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -4584,7 +4584,7 @@
   // FIXME: LateAttrs et al.?
   VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
   Template, InstantiationPattern, *InstantiationArgs, TemplateArgs,
-  Converted, TemplateNameLoc, InsertPos /*, LateAttrs, StartingScope*/);
+  Converted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
   if (!Decl)
 return true;
 
Index: clang/include/clang/Sema/Template.h
===
--- clang/include/clang/Sema/Template.h
+++ clang/include/clang/Sema/Template.h
@@ -600,7 +600,7 @@
 TagDecl *NewDecl);
 
 Decl *VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *FromVar, void *InsertPos,
+VarTemplateDecl *VarTemplate, 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

This is neat! Are there plans to add vscode client support to invoke this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89277

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

[PATCH] D89098: [clang] Fix returning the underlying VarDecl as top-level decl for VarTemplateDecl.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I posted about this 
 a while 
ago, but wasn't sure how to fix. Thanks for fixing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89098

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

[PATCH] D60954: [clangd] Test case demonstrating variable template issue

[Nathan Ridge via Phabricator via cfe-commits]

nridge abandoned this revision.
nridge added a comment.
Herald added a subscriber: usaxena95.

Fixed by D89098 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60954

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

[PATCH] D88463: [clangd] Try harder to get accurate ranges for documentSymbols in macros

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb764edc59ff7: [clangd] Try harder to get accurate ranges for 
documentSymbols in macros (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88463

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -639,19 +639,27 @@
 #define FF(name) \
   class name##_Test {};
 
-$expansion[[FF]](abc);
+$expansion1[[FF]](abc);
 
 #define FF2() \
-  class $spelling[[Test]] {};
+  class Test {};
 
-FF2();
+$expansion2[[FF2]]();
+
+#define FF3() \
+  void waldo()
+
+$fullDef[[FF3() {
+  int var = 42;
+}]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAre(
-  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion"))),
-  AllOf(WithName("Test"), SymNameRange(Main.range("spelling");
+  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion1"))),
+  AllOf(WithName("Test"), SymNameRange(Main.range("expansion2"))),
+  AllOf(WithName("waldo"), SymRange(Main.range("fullDef");
 }
 
 TEST(DocumentSymbols, FuncTemplates) {
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -171,7 +171,6 @@
 llvm::Optional declToSym(ASTContext , const NamedDecl ) 
{
   auto  = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = nameLocation(ND, SM);
   SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc()));
   SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc()));
   const auto SymbolRange =
@@ -179,10 +178,6 @@
   if (!SymbolRange)
 return llvm::None;
 
-  Position NameBegin = sourceLocToPosition(SM, NameLoc);
-  Position NameEnd = sourceLocToPosition(
-  SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
-
   index::SymbolInfo SymInfo = index::getSymbolInfo();
   // FIXME: this is not classifying constructors, destructors and operators
   //correctly (they're all "methods").
@@ -194,10 +189,35 @@
   SI.deprecated = ND.isDeprecated();
   SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()),
sourceLocToPosition(SM, SymbolRange->getEnd())};
-  SI.selectionRange = Range{NameBegin, NameEnd};
+
+  SourceLocation NameLoc = ND.getLocation();
+  SourceLocation FallbackNameLoc;
+  if (NameLoc.isMacroID()) {
+if (isSpelledInSource(NameLoc, SM)) {
+  // Prefer the spelling loc, but save the expansion loc as a fallback.
+  FallbackNameLoc = SM.getExpansionLoc(NameLoc);
+  NameLoc = SM.getSpellingLoc(NameLoc);
+} else {
+  NameLoc = SM.getExpansionLoc(NameLoc);
+}
+  }
+  auto ComputeSelectionRange = [&](SourceLocation L) -> Range {
+Position NameBegin = sourceLocToPosition(SM, L);
+Position NameEnd = sourceLocToPosition(
+SM, Lexer::getLocForEndOfToken(L, 0, SM, Ctx.getLangOpts()));
+return Range{NameBegin, NameEnd};
+  };
+
+  SI.selectionRange = ComputeSelectionRange(NameLoc);
+  if (!SI.range.contains(SI.selectionRange) && FallbackNameLoc.isValid()) {
+// 'selectionRange' must be contained in 'range'. In cases where clang
+// reports unrelated ranges, we first try falling back to the expansion
+// loc for the selection range.
+SI.selectionRange = ComputeSelectionRange(FallbackNameLoc);
+  }
   if (!SI.range.contains(SI.selectionRange)) {
-// 'selectionRange' must be contained in 'range', so in cases where clang
-// reports unrelated ranges we need to reconcile somehow.
+// If the containment relationship still doesn't hold, throw away
+// 'range' and use 'selectionRange' for both.
 SI.range = SI.selectionRange;
   }
   return SI;


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -639,19 +639,27 @@
 #define FF(name) \
   class name##_Test {};
 
-$expansion[[FF]](abc);
+$expansion1[[FF]](abc);
 
 #define FF2() \
-  class $spelling[[Test]] {};
+  class Test {};
 
-FF2();
+$expansion2[[FF2]]();
+
+#define FF3() \
+  void waldo()
+
+$fullDef[[FF3() {
+  int var = 42;
+}]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
   

[clang-tools-extra] b764edc - [clangd] Try harder to get accurate ranges for documentSymbols in macros

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-10-12T19:26:36-04:00
New Revision: b764edc59ff7768e052bc2b9e76e3bb69dd5147b

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

LOG: [clangd] Try harder to get accurate ranges for documentSymbols in macros

Fixes https://github.com/clangd/clangd/issues/500

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

Added: 


Modified: 
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindSymbols.cpp 
b/clang-tools-extra/clangd/FindSymbols.cpp
index 3169ffd98038..abd03ecb0464 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -171,7 +171,6 @@ namespace {
 llvm::Optional declToSym(ASTContext , const NamedDecl ) 
{
   auto  = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = nameLocation(ND, SM);
   SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc()));
   SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc()));
   const auto SymbolRange =
@@ -179,10 +178,6 @@ llvm::Optional declToSym(ASTContext , 
const NamedDecl ) {
   if (!SymbolRange)
 return llvm::None;
 
-  Position NameBegin = sourceLocToPosition(SM, NameLoc);
-  Position NameEnd = sourceLocToPosition(
-  SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
-
   index::SymbolInfo SymInfo = index::getSymbolInfo();
   // FIXME: this is not classifying constructors, destructors and operators
   //correctly (they're all "methods").
@@ -194,10 +189,35 @@ llvm::Optional declToSym(ASTContext , 
const NamedDecl ) {
   SI.deprecated = ND.isDeprecated();
   SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()),
sourceLocToPosition(SM, SymbolRange->getEnd())};
-  SI.selectionRange = Range{NameBegin, NameEnd};
+
+  SourceLocation NameLoc = ND.getLocation();
+  SourceLocation FallbackNameLoc;
+  if (NameLoc.isMacroID()) {
+if (isSpelledInSource(NameLoc, SM)) {
+  // Prefer the spelling loc, but save the expansion loc as a fallback.
+  FallbackNameLoc = SM.getExpansionLoc(NameLoc);
+  NameLoc = SM.getSpellingLoc(NameLoc);
+} else {
+  NameLoc = SM.getExpansionLoc(NameLoc);
+}
+  }
+  auto ComputeSelectionRange = [&](SourceLocation L) -> Range {
+Position NameBegin = sourceLocToPosition(SM, L);
+Position NameEnd = sourceLocToPosition(
+SM, Lexer::getLocForEndOfToken(L, 0, SM, Ctx.getLangOpts()));
+return Range{NameBegin, NameEnd};
+  };
+
+  SI.selectionRange = ComputeSelectionRange(NameLoc);
+  if (!SI.range.contains(SI.selectionRange) && FallbackNameLoc.isValid()) {
+// 'selectionRange' must be contained in 'range'. In cases where clang
+// reports unrelated ranges, we first try falling back to the expansion
+// loc for the selection range.
+SI.selectionRange = ComputeSelectionRange(FallbackNameLoc);
+  }
   if (!SI.range.contains(SI.selectionRange)) {
-// 'selectionRange' must be contained in 'range', so in cases where clang
-// reports unrelated ranges we need to reconcile somehow.
+// If the containment relationship still doesn't hold, throw away
+// 'range' and use 'selectionRange' for both.
 SI.range = SI.selectionRange;
   }
   return SI;

diff  --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp 
b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
index a7a0188a85fd..43658284937e 100644
--- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -639,19 +639,27 @@ TEST(DocumentSymbols, FromMacro) {
 #define FF(name) \
   class name##_Test {};
 
-$expansion[[FF]](abc);
+$expansion1[[FF]](abc);
 
 #define FF2() \
-  class $spelling[[Test]] {};
+  class Test {};
 
-FF2();
+$expansion2[[FF2]]();
+
+#define FF3() \
+  void waldo()
+
+$fullDef[[FF3() {
+  int var = 42;
+}]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAre(
-  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion"))),
-  AllOf(WithName("Test"), SymNameRange(Main.range("spelling");
+  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion1"))),
+  AllOf(WithName("Test"), SymNameRange(Main.range("expansion2"))),
+  AllOf(WithName("waldo"), SymRange(Main.range("fullDef");
 }
 
 TEST(DocumentSymbols, FuncTemplates) {



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

[PATCH] D88463: [clangd] Try harder to get accurate ranges for documentSymbols in macros

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 297712.
nridge marked 2 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88463

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -613,19 +613,27 @@
 #define FF(name) \
   class name##_Test {};
 
-$expansion[[FF]](abc);
+$expansion1[[FF]](abc);
 
 #define FF2() \
-  class $spelling[[Test]] {};
+  class Test {};
 
-FF2();
+$expansion2[[FF2]]();
+
+#define FF3() \
+  void waldo()
+
+$fullDef[[FF3() {
+  int var = 42;
+}]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAre(
-  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion"))),
-  AllOf(WithName("Test"), SymNameRange(Main.range("spelling");
+  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion1"))),
+  AllOf(WithName("Test"), SymNameRange(Main.range("expansion2"))),
+  AllOf(WithName("waldo"), SymRange(Main.range("fullDef");
 }
 
 TEST(DocumentSymbols, FuncTemplates) {
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -132,7 +132,6 @@
 llvm::Optional declToSym(ASTContext , const NamedDecl ) 
{
   auto  = Ctx.getSourceManager();
 
-  SourceLocation NameLoc = nameLocation(ND, SM);
   SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc()));
   SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc()));
   const auto SymbolRange =
@@ -140,10 +139,6 @@
   if (!SymbolRange)
 return llvm::None;
 
-  Position NameBegin = sourceLocToPosition(SM, NameLoc);
-  Position NameEnd = sourceLocToPosition(
-  SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
-
   index::SymbolInfo SymInfo = index::getSymbolInfo();
   // FIXME: this is not classifying constructors, destructors and operators
   //correctly (they're all "methods").
@@ -155,10 +150,35 @@
   SI.deprecated = ND.isDeprecated();
   SI.range = Range{sourceLocToPosition(SM, SymbolRange->getBegin()),
sourceLocToPosition(SM, SymbolRange->getEnd())};
-  SI.selectionRange = Range{NameBegin, NameEnd};
+
+  SourceLocation NameLoc = ND.getLocation();
+  SourceLocation FallbackNameLoc;
+  if (NameLoc.isMacroID()) {
+if (isSpelledInSource(NameLoc, SM)) {
+  // Prefer the spelling loc, but save the expansion loc as a fallback.
+  FallbackNameLoc = SM.getExpansionLoc(NameLoc);
+  NameLoc = SM.getSpellingLoc(NameLoc);
+} else {
+  NameLoc = SM.getExpansionLoc(NameLoc);
+}
+  }
+  auto ComputeSelectionRange = [&](SourceLocation L) -> Range {
+Position NameBegin = sourceLocToPosition(SM, L);
+Position NameEnd = sourceLocToPosition(
+SM, Lexer::getLocForEndOfToken(L, 0, SM, Ctx.getLangOpts()));
+return Range{NameBegin, NameEnd};
+  };
+
+  SI.selectionRange = ComputeSelectionRange(NameLoc);
+  if (!SI.range.contains(SI.selectionRange) && FallbackNameLoc.isValid()) {
+// 'selectionRange' must be contained in 'range'. In cases where clang
+// reports unrelated ranges, we first try falling back to the expansion
+// loc for the selection range.
+SI.selectionRange = ComputeSelectionRange(FallbackNameLoc);
+  }
   if (!SI.range.contains(SI.selectionRange)) {
-// 'selectionRange' must be contained in 'range', so in cases where clang
-// reports unrelated ranges we need to reconcile somehow.
+// If the containment relationship still doesn't hold, throw away
+// 'range' and use 'selectionRange' for both.
 SI.range = SI.selectionRange;
   }
   return SI;


Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -613,19 +613,27 @@
 #define FF(name) \
   class name##_Test {};
 
-$expansion[[FF]](abc);
+$expansion1[[FF]](abc);
 
 #define FF2() \
-  class $spelling[[Test]] {};
+  class Test {};
 
-FF2();
+$expansion2[[FF2]]();
+
+#define FF3() \
+  void waldo()
+
+$fullDef[[FF3() {
+  int var = 42;
+}]]
   )");
   TU.Code = Main.code().str();
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAre(
-  AllOf(WithName("abc_Test"), SymNameRange(Main.range("expansion"))),
-

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D88712#2326223 , @rsmith wrote:

> In D88712#2325823 , @MaskRay wrote:
>
>> In D88712#2324105 , @rsmith wrote:
>>
>>> What are the expected semantics in this case? Is this:
>>>
>>> - the function is still the builtin, but if it ends up being a libcall, 
>>> call a function with a different asm name, or
>>> - the function is not considered to be the builtin any more, or
>>> - something else?
>>>
>>> I think this patch is approximately the first thing, but it's also cutting 
>>> off emission for cases where we wouldn't emit a libcall. Should we make 
>>> that distinction?
>>
>> Yes, we do the first one. I mentioned the limitation in the description. 
>> This happens with some functions like `abs` which clang has customized emit 
>> without using a library call.
>
> What should happen for a case where the LLVM mid-level optimizers insert a 
> libcall (for example, inventing a call to `memcpy`), and `memcpy` was renamed 
> at the source level to another name? Do we still call the original name?

In that case, I believe GCC does not handle it as well. glibc's approach is to 
... let GNU as handle it (D88625  ports the 
behavior to MC).

> Generally I wonder if we should instead be renaming the function in LLVM's 
> `TargetLibraryInfo`, rather than getting the frontend to try to set up a 
> situation where LLVM is unlikely to generate a call to the "wrong" name for 
> the builtin.

Ideally, probably yes. Then clang does asm mangling for no-builtin functions 
and LLVM does asm manging for builtin functions... There needs to be a new IR 
level feature. There is complexity in the overloaded builtins...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

[clang] d80ecdf - [Coroutine] Rename coro-semmetric-transfer.cpp and possibly fix test failure

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2020-10-12T15:29:07-07:00
New Revision: d80ecdf27faf2c45a4264064ddfd5c4524dadce4

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

LOG: [Coroutine] Rename coro-semmetric-transfer.cpp and possibly fix test 
failure

Some tests start to fail after https://reviews.llvm.org/D89066.
It's because the size of pointers are different on different targets.
Limit the target in the command so there is no confusion.
Also noticed I had typo in the test name.
Adding disable-llvm-passes option to make the test more stable as well.

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

Added: 
clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp

Modified: 


Removed: 
clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp



diff  --git a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp 
b/clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
similarity index 58%
rename from clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
rename to clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
index 9833f14b273d..4f841a918bcf 100644
--- a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ b/clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@ detached_task foo() {
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address 
ended right away.
-// CHECK:   %{{.*}} = call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address 
is contained within single basic block.
+// 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]])



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

[PATCH] D89269: [Coroutine] Rename coro-semmetric-transfer.cpp and fix test failure

[Xun Li 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 rGd80ecdf27faf: [Coroutine] Rename coro-semmetric-transfer.cpp 
and possibly fix test failure (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89269

Files:
  clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
  clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp


Index: clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address 
ended right away.
-// CHECK:   %{{.*}} = call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address 
is contained within single basic block.
+// 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]])


Index: clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address ended right away.
-// CHECK:   %{{.*}} = call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address is contained within single basic block.
+// 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]])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Performs a detailed profiling of clangd lsp server and conveys the
result to the client via showMessage.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,33 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/dumpMemoryTree","params":{}}
+# CHECK:  "method": "window/showMessage",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "message": "
+# CHECK-SAME:   clangd_lsp_server: {{[0-9]+}} bytes\n
+# CHECK-SAME: clangd_server: {{[0-9]+}} bytes\n
+# CHECK-SAME:   tuscheduler: {{[0-9]+}} bytes\n
+# CHECK-SAME: /clangd-test/main.cpp: {{[0-9]+}} bytes\n
+# CHECK-SAME:   ast: {{[0-9]+}} bytes\n
+# CHECK-SAME:   preamble: {{[0-9]+}} bytes\n
+# CHECK-SAME:   dynamic_index: {{[0-9]+}} bytes\n
+# CHECK-SAME: preamble: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME:   index: {{[0-9]+}} bytes\n
+# CHECK-SAME: main_file: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME: /clangd-test/main.cpp: {{[0-9]+}} bytes\n
+# CHECK-SAME:   relations: {{[0-9]+}} bytes\n
+# CHECK-SAME:   references: {{[0-9]+}} bytes\n
+# CHECK-SAME:   symbols: {{[0-9]+}} bytes\n
+# CHECK-SAME:   index: {{[0-9]+}} bytes"
+# CHECK-NEXT:"type": 3
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -24,6 +24,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/JSON.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -141,6 +142,9 @@
   void onSemanticTokens(const SemanticTokensParams &, Callback);
   void onSemanticTokensDelta(const SemanticTokensDeltaParams &,
  Callback);
+  /// This is a clangd extension. It invokes a showMessage with current
+  /// profiling info.
+  void onDumpMemoryTree(const NoParams &, Callback);
 
   std::vector getFixes(StringRef File, const clangd::Diagnostic );
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -36,8 +36,11 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1383,6 +1386,38 @@
   });
 }
 
+void ClangdLSPServer::onDumpMemoryTree(const NoParams &,
+   Callback Reply) {
+  llvm::BumpPtrAllocator Alloc;
+  MemoryTree MT();
+  profile(MT);
+
+  std::function
+  DumpTree = [&](llvm::StringRef Key, const MemoryTree ,
+ llvm::raw_ostream , size_t Indent) {
+size_t Total = MT.self();
+std::string ChildText;
+llvm::raw_string_ostream ChildStream(ChildText);
+for (const auto  : MT.children()) {
+  ChildStream << '\n';
+  Total +=
+  DumpTree(Entry.first, Entry.getSecond(), ChildStream, Indent + 2);
+}
+OS << std::string(Indent, ' ') << Key << ": " << Total << " bytes"
+   << ChildStream.str();
+return Total;
+  };
+
+  ShowMessageParams Msg;
+  llvm::raw_string_ostream Str(Msg.message);
+  DumpTree("clangd_lsp_server", MT, Str, 0);
+  Str.flush();
+
+  notify("window/showMessage", std::move(Msg));
+  Reply(nullptr);
+}
+
 ClangdLSPServer::ClangdLSPServer(class Transport ,
  const ThreadsafeFS ,
  const ClangdLSPServer::Options )
@@ -1425,6 +1460,7 @@
   MsgHandler->bind("textDocument/documentLink", ::onDocumentLink);
   

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

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

rsmith added a comment.

In D88712#2325823 , @MaskRay wrote:

> In D88712#2324105 , @rsmith wrote:
>
>> What are the expected semantics in this case? Is this:
>>
>> - the function is still the builtin, but if it ends up being a libcall, call 
>> a function with a different asm name, or
>> - the function is not considered to be the builtin any more, or
>> - something else?
>>
>> I think this patch is approximately the first thing, but it's also cutting 
>> off emission for cases where we wouldn't emit a libcall. Should we make that 
>> distinction?
>
> Yes, we do the first one. I mentioned the limitation in the description. This 
> happens with some functions like `abs` which clang has customized emit 
> without using a library call.

What should happen for a case where the LLVM mid-level optimizers insert a 
libcall (for example, inventing a call to `memcpy`), and `memcpy` was renamed 
at the source level to another name? Do we still call the original name?

Generally I wonder if we should instead be renaming the function in LLVM's 
`TargetLibraryInfo`, rather than getting the frontend to try to set up a 
situation where LLVM is unlikely to generate a call to the "wrong" name for the 
builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

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

rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

A lot of the test changes here seem to be over-constraining the existing tests. 
Tests that don't care about `nonnull` / `dereferenceable` `this` pointers 
should generally not be checking for that. Instead of looking for `nonnull 
dereferenceable(...)`, I'd prefer to see `{{[^,]*}}` or similar to skip any 
parameter attributes.

I would also like to see some tests that directly cover the new functionality: 
specifically, tests that check that the argument to `dereferenceable` is 
correct, including cases such as classes with virtual bases, virtual function 
calls, calls through pointers to member functions, and a check that we don't 
emit the attributes under `-fno-delete-null-pointer-checks`.




Comment at: clang/lib/CodeGen/CGCall.cpp:2174
+  }
+
   unsigned ArgNo = 0;

CJ-Johnson wrote:
> jdoerfert wrote:
> > Even if null pointer is valid we should place dereferenceable.
> > 
> > We also could never place nonnull and let the middle-end make the 
> > dereferenceable -> nonnull deduction, though I don't see why we can't just 
> > add nonnull here.
> I re-ran ninja check after making this fix and addressed the newly-affected 
> tests. So the patch is fully up to date :)
The LLVM LangRef says that in address space 0, `dereferenceable` implies 
`nonnull`, so I don't think we can emit `dereferenceable` in 
`NullPointerIsValid` mode, and we'd need to use `dereferenceable_or_null` 
instead. (Perhaps the LangRef is wrong, though, and the `null_pointer_is_valid` 
function attribute overrides that determination.)



Comment at: clang/test/CodeGenCXX/array-default-argument.cpp:22-24
+  // CHECK: {{call|invoke}} {{.*}} @_ZN1BC1E1A([[TEMPORARY:.*]])
   // CHECK-EH:  unwind label %[[A_AND_PARTIAL_ARRAY_LPAD:.*]]
+  // CHECK: {{call|invoke}} {{.*}} @_ZN1AD1Ev([[TEMPORARY:.*]])

This has changed the meaning of the test. Previously we were checking the 
argument bound in the first call is also passed to the second and third calls; 
now we're not checking the call arguments at all, because we re-bind 
`TEMPORARY` in each `CHECK` line.



Comment at: 
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp:126
 //
-// CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* %[[RIGHT]])
+// CHECK: call x86_thiscallcc void %[[VFUN_VALUE]](i8* nonnull %[[RIGHT]])
 // CHECK: ret

Why does this call get `nonnull` but not `dereferenceable`? Seems like we 
should be able to use at least `dereferenceable(sizeof(Right))` here -- but I 
think we could actually be more aggressive and pass 
`dereferenceable(sizeof(ChildOverride) - offsetof(ChildOverride, ))`.


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

https://reviews.llvm.org/D17993

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

[PATCH] D78658: [clang][Frontend] Add missing error handling

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Perhaps this'd be more robust with ScopeExit?  ( 
https://llvm.org/doxygen/ScopeExit_8h_source.html )


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

https://reviews.llvm.org/D78658

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

[PATCH] D89276: Support ObjC in IncludeInserter

[Joe Turner via Phabricator via cfe-commits]

compositeprimes created this revision.
compositeprimes added a reviewer: alexfh.
compositeprimes added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
compositeprimes requested review of this revision.

Update IncludeSorter/IncludeInserter to support objective-c google style (part 
1):

1. Correctly consider .mm/.m extensions
2. Correctly categorize category headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89276

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp
@@ -28,8 +28,10 @@
 
 class IncludeInserterCheckBase : public ClangTidyCheck {
 public:
-  IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context)
-  : ClangTidyCheck(CheckName, Context) {}
+  IncludeInserterCheckBase(StringRef CheckName, ClangTidyContext *Context,
+   utils::IncludeSorter::IncludeStyle Style =
+   utils::IncludeSorter::IS_Google)
+  : ClangTidyCheck(CheckName, Context), Inserter(Style) {}
 
   void registerPPCallbacks(const SourceManager , Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override {
@@ -50,7 +52,7 @@
 
   virtual std::vector headersToInclude() const = 0;
 
-  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
+  utils::IncludeInserter Inserter;
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {
@@ -111,6 +113,30 @@
   }
 };
 
+class ObjCEarlyInAlphabetHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCEarlyInAlphabetHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector headersToInclude() const override {
+return {"a/header.h"};
+  }
+};
+
+class ObjCCategoryHeaderInserterCheck : public IncludeInserterCheckBase {
+public:
+  ObjCCategoryHeaderInserterCheck(StringRef CheckName,
+ ClangTidyContext *Context)
+  : IncludeInserterCheckBase(CheckName, Context,
+ utils::IncludeSorter::IS_Google_ObjC) {}
+
+  std::vector headersToInclude() const override {
+return {"clang_tidy/tests/insert_includes_test_header+foo.h"};
+  }
+};
+
 template 
 std::string runCheckOnCode(StringRef Code, StringRef Filename) {
   std::vector Errors;
@@ -120,6 +146,10 @@
   {"clang_tidy/tests/"
"insert_includes_test_header.h",
"\n"},
+  // ObjC category.
+  {"clang_tidy/tests/"
+   "insert_includes_test_header+foo.h",
+   "\n"},
   // Non system headers
   {"a/header.h", "\n"},
   {"path/to/a/header.h", "\n"},
@@ -635,6 +665,47 @@
"insert_includes_test_header.cc"));
 }
 
+TEST(IncludeInserterTest, InsertHeaderObjectiveC) {
+  const char *PreCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+void foo() {
+  int a = 0;
+})";
+  const char *PostCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+#import "a/header.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "repo/clang_tidy/tests/"
+   "insert_includes_test_header.mm"));
+}
+
+TEST(IncludeInserterTest, InsertCategoryHeaderObjectiveC) {
+  const char *PreCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+
+void foo() {
+  int a = 0;
+})";
+  const char *PostCode = R"(
+#import "clang_tidy/tests/insert_includes_test_header.h"
+#import "clang_tidy/tests/insert_includes_test_header+foo.h"
+
+void foo() {
+  int a = 0;
+})";
+
+  EXPECT_EQ(PostCode, runCheckOnCode(
+  PreCode, "devtools/cymbal/clang_tidy/tests/"
+   "insert_includes_test_header.mm"));
+}
+
 } // anonymous namespace
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.h
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.h
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.h
@@ -23,7 +23,7 @@

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: sbc100.
Herald added subscribers: cfe-commits, ecnelises, jgravelle-google, dschuff.
Herald added a project: clang.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.

If crt1-command.o exists in the sysroot, the libc has new-style command
support, so use it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89274

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


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89136: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I cleaned up the access and removed `getBufferPtr` separately in 
d07b290e4b7c55823895e88b683de4178ffc66db 
, then 
landed my change in 69feac12d0539a7cc19cbda906d46f67029486e1 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89136

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

[PATCH] D89136: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69feac12d053: Lex: Avoid MemoryBuffer* key in 
ExcludedPreprocessorDirectiveSkipMapping, NFC (authored by dexonsmith).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D89136?vs=297648=297694#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89136

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


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -252,7 +252,7 @@
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->Buffer.get()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -380,7 +380,10 @@
   std::pair HashFileOffset =
   SourceMgr.getDecomposedLoc(HashLoc);
   const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);
-  auto It = ExcludedConditionalDirectiveSkipMappings->find(Buf);
+  if (!Buf)
+return None;
+  auto It =
+  ExcludedConditionalDirectiveSkipMappings->find(Buf->getBufferStart());
   if (It == ExcludedConditionalDirectiveSkipMappings->end())
 return None;
 
Index: 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
===
--- 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
+++ 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
@@ -23,8 +23,7 @@
 /// The datastructure that holds the mapping between the active memory buffers
 /// and the individual skip mappings.
 using ExcludedPreprocessorDirectiveSkipMapping =
-llvm::DenseMap;
+llvm::DenseMap;
 
 } // end namespace clang
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -252,7 +252,7 @@
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->Buffer.get()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -380,7 +380,10 @@
   std::pair HashFileOffset =
   SourceMgr.getDecomposedLoc(HashLoc);
   const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);
-  auto It = ExcludedConditionalDirectiveSkipMappings->find(Buf);
+  if (!Buf)
+return None;
+  auto It =
+  ExcludedConditionalDirectiveSkipMappings->find(Buf->getBufferStart());
   if (It == ExcludedConditionalDirectiveSkipMappings->end())
 return None;
 
Index: clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
===
--- clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
+++ clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
@@ -23,8 +23,7 @@
 /// The datastructure that holds the mapping between the active memory buffers
 /// and the individual skip mappings.
 using ExcludedPreprocessorDirectiveSkipMapping =
-llvm::DenseMap;
+llvm::DenseMap;
 
 } // end namespace clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 69feac1 - Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Duncan P . N . Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-10-12T17:39:01-04:00
New Revision: 69feac12d0539a7cc19cbda906d46f67029486e1

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

LOG: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, 
NFC

This is a prep patch for changing SourceManager to return
`Optional` instead of `MemoryBuffer`. With that change the
address of the MemoryBuffer will be gone, so instead use the start of the
buffer as the key for this map.

No functionality change intended, as it's expected that the pointer identity
matches between the buffers and the buffer data.

Radar-Id: rdar://70139990
Differential Revision: https://reviews.llvm.org/D89136

Added: 


Modified: 

clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
clang/lib/Lex/PPDirectives.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 




diff  --git 
a/clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h 
b/clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
index 893b7ba7a9f5..1a0d5ed57b28 100644
--- 
a/clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
+++ 
b/clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
@@ -23,8 +23,7 @@ using PreprocessorSkippedRangeMapping = 
llvm::DenseMap;
 /// The datastructure that holds the mapping between the active memory buffers
 /// and the individual skip mappings.
 using ExcludedPreprocessorDirectiveSkipMapping =
-llvm::DenseMap;
+llvm::DenseMap;
 
 } // end namespace clang
 

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index e4b901a950ae..57349d4a439d 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -380,7 +380,10 @@ Optional 
Preprocessor::getSkippedRangeForExcludedConditionalBlock(
   std::pair HashFileOffset =
   SourceMgr.getDecomposedLoc(HashLoc);
   const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);
-  auto It = ExcludedConditionalDirectiveSkipMappings->find(Buf);
+  if (!Buf)
+return None;
+  auto It =
+  ExcludedConditionalDirectiveSkipMappings->find(Buf->getBufferStart());
   if (It == ExcludedConditionalDirectiveSkipMappings->end())
 return None;
 

diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 63eab82820cc..1c10b7d727a5 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -252,7 +252,7 @@ llvm::ErrorOr> 
MinimizedVFSFile::create(
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->Buffer.get()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));



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

[clang] ac73caf - Ensure TreeTransform considers ParmVarDecls as transformed Decls

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2020-10-12T14:38:04-07:00
New Revision: ac73cafac0e523879b42b305106cd6e67bfb412e

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

LOG: Ensure TreeTransform considers ParmVarDecls as transformed Decls

See PR47804:

TreeTransform uses TransformedLocalDecls as a map of declarations that
have been transformed already. When doing a "TransformDecl", which
happens in the cases of updating a DeclRefExpr's target, the default
implementation simply returns the already transformed declaration.

However, this was not including ParmVarDecls. SO, any use of
TreeTransform that didn't re-implement TransformDecl would NOT properly
update the target of a DeclRefExpr, resulting in odd behavior.

In the case of Typo-recovery, the result was that a lambda that used its
own parameter would cause an error, since it thought that the
ParmVarDecl referenced was a different lambda. Additionally, this caused
a problem in the AST (a declrefexpr into another scope) such that a
future instantiation would cause an assertion.

This patch ensures that the ParmVarDecl transforming process records
into TransformedLocalDecls so that the DeclRefExpr is ALSO updated.

Added: 
clang/test/SemaCXX/pr47804.cpp

Modified: 
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8439e72025b8..9d519616856b 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -5479,6 +5479,7 @@ ParmVarDecl 
*TreeTransform::TransformFunctionTypeParam(
  /* DefArg */ nullptr);
   newParm->setScopeInfo(OldParm->getFunctionScopeDepth(),
 OldParm->getFunctionScopeIndex() + indexAdjustment);
+  transformedLocalDecl(OldParm, {newParm});
   return newParm;
 }
 

diff  --git a/clang/test/SemaCXX/pr47804.cpp b/clang/test/SemaCXX/pr47804.cpp
new file mode 100644
index ..3ac1de553ffc
--- /dev/null
+++ b/clang/test/SemaCXX/pr47804.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
+
+template 
+bool all_of(InputIt first, Pred p);
+
+template  void load_test() {
+  // Ensure that this doesn't crash during CorrectDelayedTyposInExpr,
+  // or any other use of TreeTransform that doesn't implement TransformDecl
+  // separately.  Also, this should only error on 'output', not that 'x' is not
+  // captured.
+  // expected-error@+1 {{use of undeclared identifier 'output'}}
+  all_of(output, [](T x) { return x; });
+}
+
+int main() {
+  load_test();
+  return 0;
+}



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

[PATCH] D89270: [clang-tidy] Add an example for misc-unused-alias-decls

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: gribozavr.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
sylvestre.ledru requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89270

Files:
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst


Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
@@ -5,3 +5,10 @@
 
 
 Finds unused namespace alias declarations.
+
+.. code-block:: c++
+
+  namespace my_namespace {
+  class C {};
+  }
+  namespace unused_alias = ::my_namespace;


Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
@@ -5,3 +5,10 @@
 
 
 Finds unused namespace alias declarations.
+
+.. code-block:: c++
+
+  namespace my_namespace {
+  class C {};
+  }
+  namespace unused_alias = ::my_namespace;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d07b290 - DependencyScanning: pull factory function into MinimizedVFS, NFC

[Duncan P . N . Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-10-12T17:25:10-04:00
New Revision: d07b290e4b7c55823895e88b683de4178ffc66db

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

LOG: DependencyScanning: pull factory function into MinimizedVFS, NFC

Avoid need for getBufferPtr API, simplifying another patch. No
functionality change.

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 




diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index b1b87e7fa573..63eab82820cc 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -217,9 +217,11 @@ class MinimizedVFSFile final : public llvm::vfs::File {
llvm::vfs::Status Stat)
   : Buffer(std::move(Buffer)), Stat(std::move(Stat)) {}
 
-  llvm::ErrorOr status() override { return Stat; }
+  static llvm::ErrorOr>
+  create(const CachedFileSystemEntry *Entry,
+ ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings);
 
-  const llvm::MemoryBuffer *getBufferPtr() const { return Buffer.get(); }
+  llvm::ErrorOr status() override { return Stat; }
 
   llvm::ErrorOr>
   getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -234,9 +236,11 @@ class MinimizedVFSFile final : public llvm::vfs::File {
   llvm::vfs::Status Stat;
 };
 
-llvm::ErrorOr>
-createFile(const CachedFileSystemEntry *Entry,
-   ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) {
+} // end anonymous namespace
+
+llvm::ErrorOr> MinimizedVFSFile::create(
+const CachedFileSystemEntry *Entry,
+ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) {
   if (Entry->isDirectory())
 return llvm::ErrorOr>(
 std::make_error_code(std::errc::is_a_directory));
@@ -248,14 +252,12 @@ createFile(const CachedFileSystemEntry *Entry,
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->getBufferPtr()] =
+(*PPSkipMappings)[Result->Buffer.get()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));
 }
 
-} // end anonymous namespace
-
 llvm::ErrorOr>
 DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) {
   SmallString<256> OwnedFilename;
@@ -265,5 +267,5 @@ DependencyScanningWorkerFilesystem::openFileForRead(const 
Twine ) {
   getOrCreateFileSystemEntry(Filename);
   if (!Result)
 return Result.getError();
-  return createFile(Result.get(), PPSkipMappings);
+  return MinimizedVFSFile::create(Result.get(), PPSkipMappings);
 }



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

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

Test failures are being fixed in https://reviews.llvm.org/D89269.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

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

[PATCH] D89269: [Coroutine] Rename coro-semmetric-transfer.cpp and possibly fix test failure

[Xun Li via Phabricator via cfe-commits]

lxfind created this revision.
lxfind added reviewers: wenlei, junparser.
Herald added subscribers: cfe-commits, modimo, modocache.
Herald added a project: clang.
lxfind requested review of this revision.

Some tests start to fail after https://reviews.llvm.org/D89066.
It's because the size of pointers are different on different targets.
Limit the target in the command so there is no confusion.
Also noticed I had typo in the test name.
Adding disable-llvm-passes option to make the test more stable as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89269

Files:
  clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
  clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp


Index: clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address 
ended right away.
-// CHECK:   %{{.*}} = call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address 
is contained within single basic block.
+// 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]])


Index: clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address ended right away.
-// CHECK:   %{{.*}} = call i8* @{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address is contained within single basic block.
+// 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]])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Vedant Kumar via Phabricator via cfe-commits]

vsk removed a subscriber: loic-joly-sonarsource.
vsk added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1169
+createBranchRegion(S->getCond(), BodyCount,
+   subtractCounters(CondCount, BodyCount));
   }

alanphipps wrote:
> vsk wrote:
> > If `theWhileStmt->getCond()` is-a BinaryOperator, it would not be 
> > considered an instrumented condition and no branch region would be created 
> > here. OTOH, if the condition is instrumented (e.g. as it would be in `while 
> > (x);`), a branch region would be created.
> > 
> > Is this the expected outcome? It seems a little bit inconsistent compared 
> > to either a) allowing the RecursiveASTVisitor to identify expressions that 
> > require branch regions, or b) guaranteeing that each while statement comes 
> > with a branch region for its condition.
> > 
> > I have the same question about the `createBranchRegion` calls in 
> > VisitDoStmt, VisitForStmt, etc. (But I'm not concerned about the calls to 
> > `createBranchRegion` in VisitBinL{And,Or} and VisitSwitch*.)
> Right, that's the expected outcome, but I think I see what you're saying in 
> that it could be confusing to call "createBranchRegion()" that may not 
> actually create a branch region in some cases.  
> 
> I could move the check for isInstrumentedCondition() out of 
> createBranchRegion() to make it clear that a branch region is only created 
> for when that is TRUE, but I wanted to encapsulate as much as I could in the 
> function to avoid duplication.  Perhaps it would be better to rename 
> "createBranchRegion()" to something more like 
> "createBranchRegionForInstrumentedCond()"?
> 
> The same behavior exists for VisitBinL{And,Or} since those conditions could 
> also be nested BinaryOperators.
I think it's fine for createBranchRegion to not guarantee that a region is 
created, since the logic is better-encapsulated this way.

I misunderstood how the feature is supposed to work. I assumed that all 
interesting branch regions would "automatically" be created during the 
recursive AST visit, by virtue of the logic you've added to VisitBinL{And,Or}. 
But it looks like the goal is to also show branch regions around loop 
constructs, regardless of whether or not the loop condition contains a binop.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1277
+// Create Branch Region around condition.
+createBranchRegion(S->getCond(), BodyCount,
+   subtractCounters(LoopCount, BodyCount));

alanphipps wrote:
> vsk wrote:
> > Is the condition of a CXXForRangeStmt something that's visible to the user? 
> > It looks more like a clang implementation detail (but I could be mistaken).
> Technically the condition isn't visible, per se, but the range-based 
> construct implies a branchable condition that other vendors do track, so I 
> still think it's useful to show it.
Seems reasonable -- I was just wondering whether a CXXForRangeStmt ever has a 
non-null condition attached. (I didn't see an example of this in loops.cpp, but 
perhaps I just missed the correct test case.)



Comment at: llvm/include/llvm/ProfileData/InstrProfData.inc:662
 /* Indexed profile format version (start from 1). */
-#define INSTR_PROF_INDEX_VERSION 6
+#define INSTR_PROF_INDEX_VERSION 7
 /* Coverage mapping format version (start from 0). */

Note that there's a copy of this file in llvm-project/compiler-rt (this allows 
the compiler-rt subtree to be checked out and built without any llvm 
dependency). Please copy over this change to 
compiler-rt/include/profile/InstrProfData.inc.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:258
   break;
+case CounterMappingRegion::BranchRegion:
+  // For a Branch Region, read two successive counters.

Could you bump the coverage mapping format version, and document the extension 
to the format in docs/CoverageMappingFormat.rst?

This should prevent an older llvm-cov from reporting "malformed object" when 
encountering a branch region.


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

https://reviews.llvm.org/D84467

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

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In D88978#2325991 , @ABataev wrote:

> In D88978#2325982 , @scott.linder 
> wrote:
>
>> @ABataev Sorry if I'm pulling you in without enough context/work on my end, 
>> but I wanted to ask how the Clang codegen for OpenMP locals works at a high 
>> level?
>>
>> Is the idea that instead of an `alloc` the frontend can insert calls into 
>> the runtime in some cases, like `__kmpc_alloc` (e.g. for `firstprivate` as 
>> in https://reviews.llvm.org/D5140)?
>
> Yes, right.
>
>> If that is the case, I assume there is no equivalent to SROA/Mem2Reg here?
>
> I assume, no.
>
>> I am trying to understand conceptually where the debug info for the source 
>> level local should be tied to in the IR, and at least for locals which use 
>> `alloc` it has turned out to be much simpler to tie the variable directly to 
>> the `alloc` itself rather than bitcasts and things which obscure the 
>> relationship.

Ok, thank you! I think the simplest thing is for me to update the patch to tie 
the debug info to the call to the runtime allocator, then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88978

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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a subscriber: aaron.ballman.
Szelethus added inline comments.



Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307
   .Case(FULLNAME, HELPTEXT)
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > This thing still worries me but this definitely isn't a link-time 
> > > dependency.
> > D53277#1285757, rGb8cfcc71469d40a98f4cc79fcdc46cd67bea45f7
> Ok, what's the proper solution here? This is clearly a layering violation 
> now; this generic diagnostic consumer shouldn't know anything about the 
> Static Analyzer specifically. I guess we could separate it into an 
> independent polymorphic object ("DescriptionGetter" or something like that) 
> that the Static Analyzer would instantiate manually. Or ideally we could ship 
> this information with the bug report itself.
> 
> I'll add a FIXME and try to reproduce potential modules problems locally.
I am puzzled myself :/ Maybe we could ask @aaron.ballman, since he landed most 
of these changes back in the day?


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

https://reviews.llvm.org/D67422

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

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D88978#2325982 , @scott.linder 
wrote:

> @ABataev Sorry if I'm pulling you in without enough context/work on my end, 
> but I wanted to ask how the Clang codegen for OpenMP locals works at a high 
> level?
>
> Is the idea that instead of an `alloc` the frontend can insert calls into the 
> runtime in some cases, like `__kmpc_alloc` (e.g. for `firstprivate` as in 
> https://reviews.llvm.org/D5140)?

Yes, right.

> If that is the case, I assume there is no equivalent to SROA/Mem2Reg here?

I assume, no.

> I am trying to understand conceptually where the debug info for the source 
> level local should be tied to in the IR, and at least for locals which use 
> `alloc` it has turned out to be much simpler to tie the variable directly to 
> the `alloc` itself rather than bitcasts and things which obscure the 
> relationship.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88978

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

[PATCH] D87604: [X86] Convert integer _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

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

https://reviews.llvm.org/D87604

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

[PATCH] D88978: [WIP] Attach debug intrinsics to allocas, and use correct address space

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

@ABataev Sorry if I'm pulling you in without enough context/work on my end, but 
I wanted to ask how the Clang codegen for OpenMP locals works at a high level?

Is the idea that instead of an `alloc` the frontend can insert calls into the 
runtime in some cases, like `__kmpc_alloc` (e.g. for `firstprivate` as in 
https://reviews.llvm.org/D5140)?

If that is the case, I assume there is no equivalent to SROA/Mem2Reg here? I am 
trying to understand conceptually where the debug info for the source level 
local should be tied to in the IR, and at least for locals which use `alloc` it 
has turned out to be much simpler to tie the variable directly to the `alloc` 
itself rather than bitcasts and things which obscure the relationship.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88978

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

[PATCH] D89225: [MinGW][clang-shlib] Build only when LLVM_LINK_LLVM_DYLIB is enabled

[Martin Storsjö 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 rG3b1d018c0dba: [MinGW][clang-shlib] Build only when 
LLVM_LINK_LLVM_DYLIB is enabled (authored by mati865, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89225

Files:
  clang/tools/CMakeLists.txt


Index: clang/tools/CMakeLists.txt
===
--- clang/tools/CMakeLists.txt
+++ clang/tools/CMakeLists.txt
@@ -15,7 +15,9 @@
 
 add_clang_subdirectory(clang-rename)
 add_clang_subdirectory(clang-refactor)
-if(UNIX OR MINGW)
+# For MinGW we only enable shared library if LLVM_LINK_LLVM_DYLIB=ON.
+# Without that option resulting library is too close to 2^16 DLL exports limit.
+if(UNIX OR (MINGW AND LLVM_LINK_LLVM_DYLIB))
   add_clang_subdirectory(clang-shlib)
 endif()
 


Index: clang/tools/CMakeLists.txt
===
--- clang/tools/CMakeLists.txt
+++ clang/tools/CMakeLists.txt
@@ -15,7 +15,9 @@
 
 add_clang_subdirectory(clang-rename)
 add_clang_subdirectory(clang-refactor)
-if(UNIX OR MINGW)
+# For MinGW we only enable shared library if LLVM_LINK_LLVM_DYLIB=ON.
+# Without that option resulting library is too close to 2^16 DLL exports limit.
+if(UNIX OR (MINGW AND LLVM_LINK_LLVM_DYLIB))
   add_clang_subdirectory(clang-shlib)
 endif()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D87946#2325934 , @grokos wrote:

> In D87946#2325756 , @jhuber6 wrote:
>
>> Current build, fails `offloading/target_depend_nowait` for an unknown reason 
>> after calling cuStreamSynchronize in __tgt_target_teams_mapper_nowait.
>
> Is your tree up to date? We had a problem with this test, which was fixed by 
> D84470 .

I did a pull a few hours ago, so I have that patch in my tree. I compiled it 
both with master and this patch applied, it worked on master and failed using 
my patch. There are some slight differences between the code generated between 
the two in the device code generated, the host code only differs in the 
`ident_t` argument.

Master just has this under the device module

  %class.omptarget_nvptx_Queue = type { [32 x 
%class.omptarget_nvptx_ThreadPrivateContext], [32 x 
%class.omptarget_nvptx_ThreadPrivateContext*], i32, [32 x i32], i32, [8 x i8] }

While this patch has this in addition.

  %class.omptarget_nvptx_Queue = type { [32 x 
%class.omptarget_nvptx_ThreadPrivateContext.34], [32 x 
%class.omptarget_nvptx_ThreadPrivateContext.34*], i32, [32 x i32], i32, [8 x 
i8] }
  %class.omptarget_nvptx_ThreadPrivateContext.34 = type { 
%class.omptarget_nvptx_TeamDescr.32, [1024 x %class.omptarget_nvptx_TaskDescr], 
[1024 x %class.omptarget_nvptx_TaskDescr*], %union.anon, [1024 x i32], [1024 x 
i64], [1024 x i64], [1024 x i64], [1024 x i64], i64, [8 x i8] }
  %class.omptarget_nvptx_TeamDescr.32 = type { 
%class.omptarget_nvptx_TaskDescr, %class.omptarget_nvptx_WorkDescr, [32 x 
%struct.__kmpc_data_sharing_worker_slot_static] }

This probably has nothing to do with the problem but I'm wondering why this 
patch does anything that changes the device codegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87946

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

[clang] 3b1d018 - [MinGW][clang-shlib] Build only when LLVM_LINK_LLVM_DYLIB is enabled

[Martin Storsjö via cfe-commits]

Author: Mateusz Mikuła
Date: 2020-10-12T23:28:23+03:00
New Revision: 3b1d018c0dba45408164f5e69cb400976efa350f

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

LOG: [MinGW][clang-shlib] Build only when LLVM_LINK_LLVM_DYLIB is enabled

Otherwise it's easy to hit 2^16 DLL exports limit.

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

Added: 


Modified: 
clang/tools/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/CMakeLists.txt b/clang/tools/CMakeLists.txt
index 85a85812a8d4..84e3fb156f1a 100644
--- a/clang/tools/CMakeLists.txt
+++ b/clang/tools/CMakeLists.txt
@@ -15,7 +15,9 @@ add_clang_subdirectory(c-index-test)
 
 add_clang_subdirectory(clang-rename)
 add_clang_subdirectory(clang-refactor)
-if(UNIX OR MINGW)
+# For MinGW we only enable shared library if LLVM_LINK_LLVM_DYLIB=ON.
+# Without that option resulting library is too close to 2^16 DLL exports limit.
+if(UNIX OR (MINGW AND LLVM_LINK_LLVM_DYLIB))
   add_clang_subdirectory(clang-shlib)
 endif()
 



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

[libunwind] fc5e68f - [libunwind] [SEH] Don't interact with foreign exceptions

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2020-10-12T23:28:22+03:00
New Revision: fc5e68fab965bdc8fdf6db9ae2603f9dd02dec5b

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

LOG: [libunwind] [SEH] Don't interact with foreign exceptions

This unfortunately means that we don't execute C++ destructors when
unwinding past such frames for a different SEH unwind purpose (e.g.
as part of setjmp/longjmp), but that case isn't handled properly at
the moment (the original unwind intent is lost and we end up with an
unhandled exception). This patch makes sure the foreign unwind terminates
as intended.

After executing a handler, _Unwind_Resume doesn't have access to
the target frame parameter of the original foreign unwind. We also
currently blindly set ExceptionCode to STATUS_GCC_THROW - we could
set that correctly by storing the original code in _GCC_specific_handler,
but we don't have access to the original target frame value.

This also matches what libgcc's SEH unwinding code does in this case.

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

Added: 


Modified: 
libunwind/src/Unwind-seh.cpp

Removed: 




diff  --git a/libunwind/src/Unwind-seh.cpp b/libunwind/src/Unwind-seh.cpp
index 403ab2d77110..6e2b4e73e41e 100644
--- a/libunwind/src/Unwind-seh.cpp
+++ b/libunwind/src/Unwind-seh.cpp
@@ -46,18 +46,6 @@ using namespace libunwind;
 /// handling.
 #define STATUS_GCC_UNWIND MAKE_GCC_EXCEPTION(1) // 0x21474343
 
-/// Class of foreign exceptions based on unrecognized SEH exceptions.
-static const uint64_t kSEHExceptionClass = 0x434C4E4753454800; // CLNGSEH\0
-
-/// Exception cleanup routine used by \c _GCC_specific_handler to
-/// free foreign exceptions.
-static void seh_exc_cleanup(_Unwind_Reason_Code urc, _Unwind_Exception *exc) {
-  (void)urc;
-  if (exc->exception_class != kSEHExceptionClass)
-_LIBUNWIND_ABORT("SEH cleanup called on non-SEH exception");
-  free(exc);
-}
-
 static int __unw_init_seh(unw_cursor_t *cursor, CONTEXT *ctx);
 static DISPATCHER_CONTEXT *__unw_seh_get_disp_ctx(unw_cursor_t *cursor);
 static void __unw_seh_set_disp_ctx(unw_cursor_t *cursor,
@@ -108,10 +96,10 @@ _GCC_specific_handler(PEXCEPTION_RECORD ms_exc, PVOID 
frame, PCONTEXT ms_ctx,
 }
   } else {
 // Foreign exception.
-exc = (_Unwind_Exception *)malloc(sizeof(_Unwind_Exception));
-exc->exception_class = kSEHExceptionClass;
-exc->exception_cleanup = seh_exc_cleanup;
-memset(exc->private_, 0, sizeof(exc->private_));
+// We can't interact with them (we don't know the original target frame
+// that we should pass on to RtlUnwindEx in _Unwind_Resume), so just
+// pass without calling our destructors here.
+return ExceptionContinueSearch;
   }
   if (!ctx) {
 __unw_init_seh(, disp->ContextRecord);



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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307
   .Case(FULLNAME, HELPTEXT)
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER

Szelethus wrote:
> NoQ wrote:
> > This thing still worries me but this definitely isn't a link-time 
> > dependency.
> D53277#1285757, rGb8cfcc71469d40a98f4cc79fcdc46cd67bea45f7
Ok, what's the proper solution here? This is clearly a layering violation now; 
this generic diagnostic consumer shouldn't know anything about the Static 
Analyzer specifically. I guess we could separate it into an independent 
polymorphic object ("DescriptionGetter" or something like that) that the Static 
Analyzer would instantiate manually. Or ideally we could ship this information 
with the bug report itself.

I'll add a FIXME and try to reproduce potential modules problems locally.


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

https://reviews.llvm.org/D67422

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

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

[George Rokos via Phabricator via cfe-commits]

grokos added a comment.

In D87946#2325756 , @jhuber6 wrote:

> Current build, fails `offloading/target_depend_nowait` for an unknown reason 
> after calling cuStreamSynchronize in __tgt_target_teams_mapper_nowait.

Is your tree up to date? We had a problem with this test, which was fixed by 
D84470 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87946

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

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In D88154#2325713 , 
@venkataramanan.kumar.llvm wrote:

> As per review comments from Sanjay,  updated the test case to use metadata.  
> Also autogenerated the checks in the test cases using 
> llvm/utils/update_test_checks.py.

Thanks. But wouldn't it be better test coverage to vary the 
"llvm.loop.vectorize.width". Why is it always "4"?
I don't have any experience with this lib, so no real feedback on the code 
itself. Hopefully another reviewer can approve if there are no other concerns.


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

https://reviews.llvm.org/D88154

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added subscribers: aaron.ballman, lebedev.ri.
jdoerfert added a comment.

Overall this looks sane to me. Don't know who wants to accept this. @rjmccall 
@lebedev.ri @aaron.ballman @rsmith




Comment at: clang/lib/CodeGen/CGCall.cpp:2165
+assert(getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0 &&
+   "Expected `this` pointer without address space attribute.");
+

I'm unsure why this assertion has to hold and more importantly why we need it. 
@arsenm do you?


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

https://reviews.llvm.org/D17993

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

[PATCH] D89102: [X86] Add HRESET instruction.

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

https://reviews.llvm.org/D89102

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

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

There seems to be build failures in the buildbot, but I don't understand why 
it's happening.. (unable to repro locally and the patterns seem reasonable)
http://lab.llvm.org:8011/#/builders/12/builds/92/steps/7/logs/FAIL__Clang__coro-semmetric-transfer_cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

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

[PATCH] D89136: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Alex Lorenz via Phabricator via cfe-commits]

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



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:249
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->getBufferPtr()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();

`Buffer` is private, does this work here, or do you need a `friend`? Either 
way, LGTM


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

https://reviews.llvm.org/D89136

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

[PATCH] D66782: SourceManager: Prefer Optional over MemoryBuffer*

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

I'm going to change my approach here to be less error-prone. The real goal is 
changing from `MemoryBuffer*` to `MemoryBufferRef`. I was adding `Optional<>` 
as a cleanup but that's really a nice-to-have, which is better landed 
independently.

1. Add getBufferOrNone and getBufferDataOrNone, which return 
`Optional`.
2. Move users of getBuffer and getBufferData that use the `bool* Invalid` out 
parameter over to that API.
3. (this patch) Change getBuffer and getBufferData to return `MemoryBufferRef`, 
dropping the (now unused) Invalid parameter. Keep the semantics where they 
return a fake buffer if the real one couldn't be found.
4. (optional, later) Rename getBuffer to  getBufferOrFake (same for Data)
5. (optional, later) Migrate audited APIs over to a new/clean getBuffer API 
that asserts on invalid/None

Compare to current approach in the patch:

- good: callers will still get expected results when invalid (for those that 
don’t check); no audit necessary
- good: getBuffer and getBufferData will match
- good: more incremental
- bad: a bit more churn for getBuffer users, since pointer semantics 
(`MemoryBuffer*`) change to reference semantics (`MemoryBufferRef`)
- bad: delay cleanup where callers expecting fake data explicitly use an 
`OrFake` API


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

https://reviews.llvm.org/D66782

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D88712#2324105 , @rsmith wrote:

> What are the expected semantics in this case? Is this:
>
> - the function is still the builtin, but if it ends up being a libcall, call 
> a function with a different asm name, or
> - the function is not considered to be the builtin any more, or
> - something else?
>
> I think this patch is approximately the first thing, but it's also cutting 
> off emission for cases where we wouldn't emit a libcall. Should we make that 
> distinction?

Yes, we do the first one. I mentioned the limitation in the description. This 
happens with some functions like `abs` which clang has customized emit without 
using a library call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 297667.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Mention the limitation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88712

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/asm-label.c
  clang/test/CodeGen/redefine_extname.c


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, 
src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LINUX
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefix=DARWIN
+// RUN: %clang_cc1 -triple=i686-pc-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm %s -o - | FileCheck 
%s --check-prefixes=CHECK,DARWIN
 
 char *strerror(int) asm("alias");
 int x __asm("foo");
@@ -17,3 +17,33 @@
 // DARWIN: @"\01bar" = internal global i32 0
 // DARWIN: @"\01foo" = global i32 0
 // DARWIN: declare i8* @"\01alias"(i32)
+
+extern void *memcpy(void *__restrict, const void *__restrict, unsigned long);
+extern __typeof(memcpy) memcpy asm("__GI_memcpy");
+void test_memcpy(void *dst, void *src, unsigned long n) {
+  memcpy(dst, src, n);
+}
+// CHECK-LABEL: @test_memcpy(
+// LINUX: call i8* @__GI_memcpy(
+// LINUX:   declare i8* @__GI_memcpy(i8*, i8*, i32)
+// DARWIN:call i8* @"\01__GI_memcpy"(
+// DARWIN:  declare i8* @"\01__GI_memcpy"(i8*, i8*, i32)
+
+long lrint(double x) asm("__GI_lrint");
+long test_lrint(double x) {
+  return lrint(x);
+}
+// CHECK-LABEL: @test_lrint(
+// LINUX: call i32 @__GI_lrint(
+// LINUX:   declare i32 @__GI_lrint(double)
+// DARWIN:call i32 @"\01__GI_lrint"(
+// DARWIN:  declare i32 @"\01__GI_lrint"(double)
+
+/// NOTE: GCC can optimize out abs in -O1 or above. Clang does not
+/// communicate the mapping to the backend so the libcall cannot be eliminated.
+int abs(int x) asm("__GI_abs");
+long test_abs(int x) {
+  return abs(x);
+}
+// CHECK-LABEL: @test_abs(
+// LINUX: call i32 @__GI_abs(
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -1663,13 +1663,18 @@
Result.Val.getFloat()));
   }
 
+  // If the builtin has been declared explicitly with an assembler label,
+  // disable the specialized emitting below.
+  const unsigned BuiltinIDIfNoAsmLabel =
+  FD->hasAttr() ? 0 : BuiltinID;
+
   // There are LLVM math intrinsics/instructions corresponding to math library
   // functions except the LLVM op will never set errno while the math library
   // might. Also, math builtins have the same semantics as their math library
   // twins. Thus, we can transform math library and builtin calls to their
   // LLVM counterparts if the call is marked 'const' (known to never set 
errno).
   if (FD->hasAttr()) {
-switch (BuiltinID) {
+switch (BuiltinIDIfNoAsmLabel) {
 case Builtin::BIceil:
 case Builtin::BIceilf:
 case Builtin::BIceill:
@@ -1946,7 +1951,7 @@
 }
   }
 
-  switch (BuiltinID) {
+  switch (BuiltinIDIfNoAsmLabel) {
   default: break;
   case Builtin::BI__builtin___CFStringMakeConstantString:
   case Builtin::BI__builtin___NSStringMakeConstantString:


Index: clang/test/CodeGen/redefine_extname.c
===
--- clang/test/CodeGen/redefine_extname.c
+++ clang/test/CodeGen/redefine_extname.c
@@ -30,3 +30,9 @@
 int baz() { return foo_static(); }
 // CHECK-NOT: call i32 @bar_static()
 
+// Check that pragma redefine_extname applies to builtin functions.
+typedef unsigned long size_t;
+extern void *memcpy(void *, const void *, size_t);
+#pragma redefine_extname memcpy __GI_memcpy
+void *test_memcpy(void *dst, const void *src, size_t n) { return memcpy(dst, src, n); }
+// CHECK: call i8* @__GI_memcpy(
Index: clang/test/CodeGen/asm-label.c
===
--- clang/test/CodeGen/asm-label.c
+++ clang/test/CodeGen/asm-label.c
@@ -1,5 +1,5 @@
-// RUN: 

[clang] 9a33f02 - Revert "Canonicalize declaration pointers when forming APValues."

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2020-10-12T12:37:24-07:00
New Revision: 9a33f027ac7d73e14ae287e78ab554142d1cbc8f

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

LOG: Revert "Canonicalize declaration pointers when forming APValues."

This reverts commit 9dcd96f728863d40d6f5922ed52732fdd728fb5f.

See https://crbug.com/1134762.

Added: 


Modified: 
clang/include/clang/AST/APValue.h
clang/lib/AST/APValue.cpp
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
clang/test/CodeGenCXX/weak-external.cpp
clang/test/OpenMP/ordered_messages.cpp

Removed: 




diff  --git a/clang/include/clang/AST/APValue.h 
b/clang/include/clang/AST/APValue.h
index ac8ed0818af0..9e9468645fe7 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -177,7 +177,6 @@ class APValue {
   return !(LHS == RHS);
 }
 friend llvm::hash_code hash_value(const LValueBase );
-friend struct llvm::DenseMapInfo;
 
   private:
 PtrTy Ptr;
@@ -205,7 +204,8 @@ class APValue {
 
   public:
 LValuePathEntry() : Value() {}
-LValuePathEntry(BaseOrMemberType BaseOrMember);
+LValuePathEntry(BaseOrMemberType BaseOrMember)
+: Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
 static LValuePathEntry ArrayIndex(uint64_t Index) {
   LValuePathEntry Result;
   Result.Value = Index;

diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 7efd0caf3f1d..22145beafd8d 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -38,7 +38,7 @@ static_assert(
 "Type is insufficiently aligned");
 
 APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V)
-: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr), Local{I, V} {}
+: Ptr(P), Local{I, V} {}
 APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
 : Ptr(P), Local{I, V} {}
 
@@ -90,19 +90,13 @@ bool operator==(const APValue::LValueBase ,
 const APValue::LValueBase ) {
   if (LHS.Ptr != RHS.Ptr)
 return false;
-  if (LHS.is() || LHS.is())
+  if (LHS.is())
 return true;
   return LHS.Local.CallIndex == RHS.Local.CallIndex &&
  LHS.Local.Version == RHS.Local.Version;
 }
 }
 
-APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType BaseOrMember) {
-  if (const Decl *D = BaseOrMember.getPointer())
-BaseOrMember.setPointer(D->getCanonicalDecl());
-  Value = reinterpret_cast(BaseOrMember.getOpaqueValue());
-}
-
 void APValue::LValuePathEntry::profile(llvm::FoldingSetNodeID ) const {
   ID.AddInteger(Value);
 }
@@ -131,16 +125,14 @@ APValue::LValueBase::operator bool () const {
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getEmptyKey() {
-  clang::APValue::LValueBase B;
-  B.Ptr = DenseMapInfo::getEmptyKey();
-  return B;
+  return clang::APValue::LValueBase(
+  DenseMapInfo::getEmptyKey());
 }
 
 clang::APValue::LValueBase
 llvm::DenseMapInfo::getTombstoneKey() {
-  clang::APValue::LValueBase B;
-  B.Ptr = DenseMapInfo::getTombstoneKey();
-  return B;
+  return clang::APValue::LValueBase(
+  DenseMapInfo::getTombstoneKey());
 }
 
 namespace clang {
@@ -934,10 +926,8 @@ void APValue::MakeMemberPointer(const ValueDecl *Member, 
bool IsDerivedMember,
   assert(isAbsent() && "Bad state change");
   MemberPointerData *MPD = new ((void*)(char*)Data.buffer) MemberPointerData;
   Kind = MemberPointer;
-  MPD->MemberAndIsDerivedMember.setPointer(
-  Member ? cast(Member->getCanonicalDecl()) : nullptr);
+  MPD->MemberAndIsDerivedMember.setPointer(Member);
   MPD->MemberAndIsDerivedMember.setInt(IsDerivedMember);
   MPD->resizePath(Path.size());
-  for (unsigned I = 0; I != Path.size(); ++I)
-MPD->getPath()[I] = Path[I]->getCanonicalDecl();
+  memcpy(MPD->getPath(), Path.data(), Path.size()*sizeof(const 
CXXRecordDecl*));
 }

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index a6c7f30528eb..6bb8aa026ad8 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4686,9 +4686,11 @@ char *Buffer = new (getASTContext(), 1) char[Name.size() 
+ 1];
 void ValueDecl::anchor() {}
 
 bool ValueDecl::isWeak() const {
-  auto *MostRecent = getMostRecentDecl();
-  return MostRecent->hasAttr() ||
- MostRecent->hasAttr() || isWeakImported();
+  for (const auto *I : attrs())
+if (isa(I) || isa(I))
+  return true;
+
+  return isWeakImported();
 }
 
 void ImplicitParamDecl::anchor() {}

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index ab2b55c0762e..f4314d0bd961 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -720,7 +720,7 @@ bool Decl::isWeakImported() 

[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 297661.
jhuber6 added a subscriber: tianshilei1992.
jhuber6 added a comment.

Current build, fails `offloading/target_depend_nowait` for an unknown reason 
after calling cuStreamSynchronize in __tgt_target_teams_mapper_nowait.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87946

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/capturing_in_templates.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/declare_target_link_codegen.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_lambda_pointer_capturing.cpp
  clang/test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
  clang/test/OpenMP/target_codegen.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
  clang/test/OpenMP/target_defaultmap_codegen.cpp
  clang/test/OpenMP/target_depend_codegen.cpp
  clang/test/OpenMP/target_device_codegen.cpp
  clang/test/OpenMP/target_enter_data_codegen.cpp
  clang/test/OpenMP/target_enter_data_depend_codegen.cpp
  clang/test/OpenMP/target_exit_data_codegen.cpp
  clang/test/OpenMP/target_exit_data_depend_codegen.cpp
  clang/test/OpenMP/target_firstprivate_codegen.cpp
  clang/test/OpenMP/target_is_device_ptr_codegen.cpp
  clang/test/OpenMP/target_map_codegen_00.cpp
  clang/test/OpenMP/target_map_codegen_01.cpp
  clang/test/OpenMP/target_map_codegen_02.cpp
  clang/test/OpenMP/target_map_codegen_03.cpp
  clang/test/OpenMP/target_map_codegen_04.cpp
  clang/test/OpenMP/target_map_codegen_05.cpp
  clang/test/OpenMP/target_map_codegen_06.cpp
  clang/test/OpenMP/target_map_codegen_07.cpp
  clang/test/OpenMP/target_map_codegen_08.cpp
  clang/test/OpenMP/target_map_codegen_09.cpp
  clang/test/OpenMP/target_map_codegen_10.cpp
  clang/test/OpenMP/target_map_codegen_11.cpp
  clang/test/OpenMP/target_map_codegen_12.cpp
  clang/test/OpenMP/target_map_codegen_13.cpp
  clang/test/OpenMP/target_map_codegen_14.cpp
  clang/test/OpenMP/target_map_codegen_15.cpp
  clang/test/OpenMP/target_map_codegen_16.cpp
  clang/test/OpenMP/target_map_codegen_17.cpp
  clang/test/OpenMP/target_map_codegen_18.inc
  clang/test/OpenMP/target_map_codegen_19.cpp
  clang/test/OpenMP/target_map_codegen_20.cpp
  clang/test/OpenMP/target_map_codegen_21.cpp
  clang/test/OpenMP/target_map_codegen_22.cpp
  clang/test/OpenMP/target_map_codegen_23.cpp
  clang/test/OpenMP/target_map_codegen_24.cpp
  clang/test/OpenMP/target_map_codegen_25.cpp
  clang/test/OpenMP/target_map_codegen_26.cpp
  clang/test/OpenMP/target_map_codegen_27.cpp
  clang/test/OpenMP/target_map_codegen_28.cpp
  clang/test/OpenMP/target_map_codegen_29.cpp
  clang/test/OpenMP/target_map_codegen_30.cpp
  clang/test/OpenMP/target_map_codegen_31.cpp
  clang/test/OpenMP/target_map_codegen_32.cpp
  clang/test/OpenMP/target_map_codegen_33.cpp
  clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
  clang/test/OpenMP/target_parallel_codegen.cpp
  clang/test/OpenMP/target_parallel_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_codegen.cpp
  clang/test/OpenMP/target_parallel_for_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_uses_allocators_codegen.cpp
  clang/test/OpenMP/target_parallel_for_uses_allocators_codegen.cpp
  

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Xun Li 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 rGdce8f2bb25ea: [Coroutine][Sema] Only tighten the suspend 
call temp lifetime for final awaiter (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

Files:
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp


Index: clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
 
 #include "Inputs/coroutine.h"
 
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -375,7 +375,7 @@
 // returning await_suspend that results in a guaranteed tail call to the target
 // coroutine.
 static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
-   SourceLocation Loc) {
+   SourceLocation Loc, bool IsImplicit) {
   if (RetType->isReferenceType())
 return nullptr;
   Type const *T = RetType.getTypePtr();
@@ -398,10 +398,17 @@
diag::warn_coroutine_handle_address_invalid_return_type)
 << JustAddress->getType();
 
-  // The coroutine handle used to obtain the address is no longer needed
-  // at this point, clean it up to avoid unnecessarily long lifetime which
-  // could lead to unnecessary spilling.
-  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
+  // After the await_suspend call on the awaiter, the coroutine may have
+  // been destroyed. In that case, we can not store anything to the frame
+  // from this point on. Hence here we wrap it immediately with a cleanup. This
+  // could have applied to all await_suspend calls. However doing so causes
+  // alive objects being destructed for reasons that need further
+  // investigations. Here we walk-around it temporarily by only doing it after
+  // the suspend call on the final awaiter (indicated by IsImplicit) where it's
+  // most common to happen.
+  // TODO: Properly clean up the temps generated by await_suspend calls.
+  if (IsImplicit)
+JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }
@@ -409,7 +416,8 @@
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl 
*CoroPromise,
-  SourceLocation Loc, Expr *E) 
{
+  SourceLocation Loc, Expr *E,
+  bool IsImplicit) {
   OpaqueValueExpr *Operand = new (S.Context)
   OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
@@ -458,7 +466,8 @@
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
 
 // Experimental support for coroutine_handle returning await_suspend.
-if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+if (Expr *TailCallSuspend =
+maybeTailCall(S, RetType, AwaitSuspend, Loc, IsImplicit))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
@@ -870,8 +879,8 @@
   SourceLocation CallLoc = E->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, CallLoc, E, IsImplicit);
   if (RSS.IsInvalid)
 return ExprError();
 
@@ -925,8 +934,8 @@
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, Loc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, Loc, E, /*IsImplicit*/ false);
   if (RSS.IsInvalid)
 return ExprError();
 


Index: clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+// RUN: %clang -std=c++14 

[clang] dce8f2b - [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2020-10-12T12:00:20-07:00
New Revision: dce8f2bb25ea1d01533d8e602f2520492fa67259

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

LOG: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final 
awaiter

In https://reviews.llvm.org/D87470 I added the change to tighten the lifetime 
of the expression awaiter.await_suspend().address.
Howver it was incorrect. ExprWithCleanups will call the dtor and end the 
lifetime for all the temps created in the current full expr.
When this is called on a normal await call, we don't want to do that.
We only want to do this for the call on the final_awaiter, to avoid writing 
into the frame after the frame is destroyed.
This change fixes it, by checking IsImplicit.

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

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 565f907e05b2..5582c728aa2d 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -375,7 +375,7 @@ static ExprResult buildMemberCall(Sema , Expr *Base, 
SourceLocation Loc,
 // returning await_suspend that results in a guaranteed tail call to the target
 // coroutine.
 static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
-   SourceLocation Loc) {
+   SourceLocation Loc, bool IsImplicit) {
   if (RetType->isReferenceType())
 return nullptr;
   Type const *T = RetType.getTypePtr();
@@ -398,10 +398,17 @@ static Expr *maybeTailCall(Sema , QualType RetType, 
Expr *E,
diag::warn_coroutine_handle_address_invalid_return_type)
 << JustAddress->getType();
 
-  // The coroutine handle used to obtain the address is no longer needed
-  // at this point, clean it up to avoid unnecessarily long lifetime which
-  // could lead to unnecessary spilling.
-  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
+  // After the await_suspend call on the awaiter, the coroutine may have
+  // been destroyed. In that case, we can not store anything to the frame
+  // from this point on. Hence here we wrap it immediately with a cleanup. This
+  // could have applied to all await_suspend calls. However doing so causes
+  // alive objects being destructed for reasons that need further
+  // investigations. Here we walk-around it temporarily by only doing it after
+  // the suspend call on the final awaiter (indicated by IsImplicit) where it's
+  // most common to happen.
+  // TODO: Properly clean up the temps generated by await_suspend calls.
+  if (IsImplicit)
+JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }
@@ -409,7 +416,8 @@ static Expr *maybeTailCall(Sema , QualType RetType, Expr 
*E,
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl 
*CoroPromise,
-  SourceLocation Loc, Expr *E) 
{
+  SourceLocation Loc, Expr *E,
+  bool IsImplicit) {
   OpaqueValueExpr *Operand = new (S.Context)
   OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
@@ -458,7 +466,8 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema , 
VarDecl *CoroPromise,
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
 
 // Experimental support for coroutine_handle returning await_suspend.
-if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+if (Expr *TailCallSuspend =
+maybeTailCall(S, RetType, AwaitSuspend, Loc, IsImplicit))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
@@ -870,8 +879,8 @@ ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation 
Loc, Expr *E,
   SourceLocation CallLoc = E->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, CallLoc, E, IsImplicit);
   if (RSS.IsInvalid)
 return ExprError();
 
@@ -925,8 +934,8 @@ ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr 
*E) {
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-  

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Xun Li via Phabricator via cfe-commits]

lxfind updated this revision to Diff 297656.
lxfind added a comment.

Add more comments and TODO


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

Files:
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp


Index: clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
 
 #include "Inputs/coroutine.h"
 
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -375,7 +375,7 @@
 // returning await_suspend that results in a guaranteed tail call to the target
 // coroutine.
 static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
-   SourceLocation Loc) {
+   SourceLocation Loc, bool IsImplicit) {
   if (RetType->isReferenceType())
 return nullptr;
   Type const *T = RetType.getTypePtr();
@@ -398,10 +398,17 @@
diag::warn_coroutine_handle_address_invalid_return_type)
 << JustAddress->getType();
 
-  // The coroutine handle used to obtain the address is no longer needed
-  // at this point, clean it up to avoid unnecessarily long lifetime which
-  // could lead to unnecessary spilling.
-  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
+  // After the await_suspend call on the awaiter, the coroutine may have
+  // been destroyed. In that case, we can not store anything to the frame
+  // from this point on. Hence here we wrap it immediately with a cleanup. This
+  // could have applied to all await_suspend calls. However doing so causes
+  // alive objects being destructed for reasons that need further
+  // investigations. Here we walk-around it temporarily by only doing it after
+  // the suspend call on the final awaiter (indicated by IsImplicit) where it's
+  // most common to happen.
+  // TODO: Properly clean up the temps generated by await_suspend calls.
+  if (IsImplicit)
+JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }
@@ -409,7 +416,8 @@
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl 
*CoroPromise,
-  SourceLocation Loc, Expr *E) 
{
+  SourceLocation Loc, Expr *E,
+  bool IsImplicit) {
   OpaqueValueExpr *Operand = new (S.Context)
   OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
@@ -458,7 +466,8 @@
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
 
 // Experimental support for coroutine_handle returning await_suspend.
-if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+if (Expr *TailCallSuspend =
+maybeTailCall(S, RetType, AwaitSuspend, Loc, IsImplicit))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
@@ -870,8 +879,8 @@
   SourceLocation CallLoc = E->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, CallLoc, E, IsImplicit);
   if (RSS.IsInvalid)
 return ExprError();
 
@@ -925,8 +934,8 @@
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, Loc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, Loc, E, /*IsImplicit*/ false);
   if (RSS.IsInvalid)
 return ExprError();
 


Index: clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
===
--- clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
Index: clang/lib/Sema/SemaCoroutine.cpp

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2174
+  }
+
   unsigned ArgNo = 0;

jdoerfert wrote:
> Even if null pointer is valid we should place dereferenceable.
> 
> We also could never place nonnull and let the middle-end make the 
> dereferenceable -> nonnull deduction, though I don't see why we can't just 
> add nonnull here.
I re-ran ninja check after making this fix and addressed the newly-affected 
tests. So the patch is fully up to date :)


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

https://reviews.llvm.org/D17993

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

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Venkataramanan Kumar via Phabricator via cfe-commits]

venkataramanan.kumar.llvm updated this revision to Diff 297654.
venkataramanan.kumar.llvm added a comment.

As per review comments from Sanjay,  updated the test case to use metadata.  
Also autogenerated the checks in the test cases using 
llvm/utils/update_test_checks.py.


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

https://reviews.llvm.org/D88154

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/autocomplete.c
  llvm/include/llvm/Analysis/TargetLibraryInfo.h
  llvm/include/llvm/Analysis/VecFuncs.def
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll
  llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
  llvm/test/Transforms/Util/add-TLI-mappings.ll

Index: llvm/test/Transforms/Util/add-TLI-mappings.ll
===
--- llvm/test/Transforms/Util/add-TLI-mappings.ll
+++ llvm/test/Transforms/Util/add-TLI-mappings.ll
@@ -3,6 +3,8 @@
 ; RUN: opt -vector-library=MASSV  -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
 ; RUN: opt -vector-library=MASSV  -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,MASSV
 ; RUN: opt -vector-library=Accelerate -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE
+; RUN: opt -vector-library=LIBMVEC-X86 -inject-tli-mappings-S < %s | FileCheck %s  --check-prefixes=COMMON,LIBMVEC-X86
+; RUN: opt -vector-library=LIBMVEC-X86 -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,LIBMVEC-X86
 ; RUN: opt -vector-library=Accelerate -passes=inject-tli-mappings -S < %s | FileCheck %s  --check-prefixes=COMMON,ACCELERATE
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -21,6 +23,9 @@
 ; MASSV-SAME: i8* bitcast (<4 x float> (<4 x float>)* @__log10f4_massv to i8*)
 ; ACCELERATE-SAME:  [1 x i8*] [
 ; ACCELERATE-SAME:i8* bitcast (<4 x float> (<4 x float>)* @vlog10f to i8*)
+; LIBMVEC-X86-SAME: [2 x i8*] [
+; LIBMVEC-X86-SAME:   i8* bitcast (<2 x double> (<2 x double>)* @_ZGVbN2v_sin to i8*),
+; LIBMVEC-X86-SAME:   i8* bitcast (<4 x double> (<4 x double>)* @_ZGVdN4v_sin to i8*)
 ; COMMON-SAME:  ], section "llvm.metadata"
 
 define double @sin_f64(double %in) {
@@ -28,6 +33,7 @@
 ; SVML: call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; MASSV:call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; ACCELERATE:   call double @sin(double %{{.*}})
+; LIBMVEC-X86:  call double @sin(double %{{.*}}) #[[SIN:[0-9]+]]
 ; No mapping of "sin" to a vector function for Accelerate.
 ; ACCELERATE-NOT: _ZGV_LLVM_{{.*}}_sin({{.*}})
   %call = tail call double @sin(double %in)
@@ -39,10 +45,12 @@
 define float @call_llvm.log10.f32(float %in) {
 ; COMMON-LABEL: @call_llvm.log10.f32(
 ; SVML: call float @llvm.log10.f32(float %{{.*}})
+; LIBMVEC-X86:  call float @llvm.log10.f32(float %{{.*}})
 ; MASSV:call float @llvm.log10.f32(float %{{.*}}) #[[LOG10:[0-9]+]]
 ; ACCELERATE:   call float @llvm.log10.f32(float %{{.*}}) #[[LOG10:[0-9]+]]
 ; No mapping of "llvm.log10.f32" to a vector function for SVML.
 ; SVML-NOT: _ZGV_LLVM_{{.*}}_llvm.log10.f32({{.*}})
+; LIBMVEC-X86-NOT: _ZGV_LLVM_{{.*}}_llvm.log10.f32({{.*}})
   %call = tail call float @llvm.log10.f32(float %in)
   ret float %call
 }
@@ -62,3 +70,7 @@
 
 ; ACCELERATE:  attributes #[[LOG10]] = { "vector-function-abi-variant"=
 ; ACCELERATE-SAME:   "_ZGV_LLVM_N4v_llvm.log10.f32(vlog10f)" }
+
+; LIBMVEC-X86:  attributes #[[SIN]] = { "vector-function-abi-variant"=
+; LIBMVEC-X86-SAME:   "_ZGV_LLVM_N2v_sin(_ZGVbN2v_sin),
+; LIBMVEC-X86-SAME:   _ZGV_LLVM_N4v_sin(_ZGVdN4v_sin)" }
Index: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll
@@ -0,0 +1,1010 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -vector-library=LIBMVEC-X86  -inject-tli-mappings -loop-vectorize -S < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare double @sin(double) #0
+declare float @sinf(float) #0
+declare double @llvm.sin.f64(double) #0
+declare float @llvm.sin.f32(float) #0
+
+declare double @cos(double) #0
+declare float @cosf(float) #0
+declare double @llvm.cos.f64(double) #0
+declare float @llvm.cos.f32(float) #0
+
+define void @sin_f64(double* nocapture %varray) {
+; CHECK-LABEL: @sin_f64(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:br i1 false, label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
+; CHECK:   vector.ph:
+; CHECK-NEXT:br label [[VECTOR_BODY:%.*]]
+; CHECK:   

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 297655.
CJ-Johnson added a comment.

Apply dereferenceable even if null is a valid address


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

https://reviews.llvm.org/D17993

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CXX/except/except.spec/p14-ir.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGen/temporary-lifetime.cpp
  clang/test/CodeGenCUDA/device-var-init.cu
  clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
  
clang/test/CodeGenCXX/RelativeVTablesABI/child-inheritted-from-parent-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-1.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-2.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/member-function-pointer.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-and-child-in-comdats.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/pass-byval-attributes.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/apple-kext.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/arm64-constructor-return.cpp
  clang/test/CodeGenCXX/array-default-argument.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/attr-disable-tail-calls.cpp
  clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  clang/test/CodeGenCXX/attr.cpp
  clang/test/CodeGenCXX/auto-variable-template.cpp
  clang/test/CodeGenCXX/blocks-cxx11.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/builtin_LINE.cpp
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/conditional-gnu-ext.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/constructors.cpp
  clang/test/CodeGenCXX/copy-constructor-elim-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-constructors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-init-captures.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-copy-omission.cpp
  clang/test/CodeGenCXX/cxx1z-decomposition.cpp
  clang/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx1z-lambda-star-this.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  clang/test/CodeGenCXX/default-arg-temps.cpp
  clang/test/CodeGenCXX/default-arguments.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/derived-to-base-conv.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure.cpp
  clang/test/CodeGenCXX/dllexport-members.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/duplicate-mangled-name.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/empty-nontrivially-copyable.cpp
  clang/test/CodeGenCXX/exceptions-seh-filter-captures.cpp
  clang/test/CodeGenCXX/exceptions-seh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/CodeGenCXX/float128-declarations.cpp
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/CodeGenCXX/global-dtor-no-atexit.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenCXX/goto.cpp
  

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Reid Kleckner via cfe-commits]

On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
lldb-comm...@lists.llvm.org> wrote:

> They are online now - http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>
> AnnotatedCommand has severe design conflict with the new buildbot.
> We have changed it to be safe and still do something useful, but it will
> need more love and care.
>
> Please let me know if you have some spare time to work on porting
> AnnotatedCommand.
>

That's unfortunate, it would've been good to know that earlier. I and
another team member have spent a fair amount of time porting things to use
more AnnotatedCommand steps, because it gives us the flexibility to test
steps locally and make changes to the steps without restarting the buildbot
master. IMO that is the Right Way to define steps: a script that you can
run locally on a machine that satisfies the OS and dep requirements of the
script.

I am restarting the two bots that I am responsible for, and may need some
help debugging further issues soon. I'll let you know.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson added inline comments.



Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:1082
+bool isGFX9Plus(const MCSubtargetInfo ) {
+  return isGFX9(STI) || isGFX10(STI);
+}

CJ-Johnson wrote:
> arsenm wrote:
> > How are these changes related?
> This is a mistake. I did not add this and I'm not sure why it is being 
> included in the change.
I believe this has been fixed now. Can you confirm?


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

https://reviews.llvm.org/D17993

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2174
+  }
+
   unsigned ArgNo = 0;

Even if null pointer is valid we should place dereferenceable.

We also could never place nonnull and let the middle-end make the 
dereferenceable -> nonnull deduction, though I don't see why we can't just add 
nonnull here.


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

https://reviews.llvm.org/D17993

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

[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[Wenlei He via Phabricator via cfe-commits]

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

LGTM. I think we can get this in first to address the fall out from 
https://reviews.llvm.org/D87470, while investigating ASAN failure.




Comment at: clang/lib/Sema/SemaCoroutine.cpp:401
 
-  // The coroutine handle used to obtain the address is no longer needed
-  // at this point, clean it up to avoid unnecessarily long lifetime which
-  // could lead to unnecessary spilling.
-  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
+  // After the suspend call on the final awaiter, the coroutine may have
+  // been destroyed. In that case, we can not store anything to the frame

Can you add comment explaining why we don't cleanup for all await, probably a 
TODO? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89066

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

[PATCH] D89136: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

Agreed, looks cleaner without the indirection.


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

https://reviews.llvm.org/D89136

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I think this is the wrong patch now. @xbolva00 posted a command that works if 
`git show HEAD` shows the commit you want to upload. If not, replace HEAD with 
the commit hash.


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

https://reviews.llvm.org/D17993

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

[PATCH] D89136: Lex: Avoid MemoryBuffer* key in ExcludedPreprocessorDirectiveSkipMapping, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 297648.

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

https://reviews.llvm.org/D89136

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


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -219,8 +219,6 @@
 
   llvm::ErrorOr status() override { return Stat; }
 
-  const llvm::MemoryBuffer *getBufferPtr() const { return Buffer.get(); }
-
   llvm::ErrorOr>
   getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
 bool IsVolatile) override {
@@ -248,7 +246,7 @@
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->getBufferPtr()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -380,7 +380,10 @@
   std::pair HashFileOffset =
   SourceMgr.getDecomposedLoc(HashLoc);
   const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);
-  auto It = ExcludedConditionalDirectiveSkipMappings->find(Buf);
+  if (!Buf)
+return None;
+  auto It =
+  ExcludedConditionalDirectiveSkipMappings->find(Buf->getBufferStart());
   if (It == ExcludedConditionalDirectiveSkipMappings->end())
 return None;
 
Index: 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
===
--- 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
+++ 
clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
@@ -23,8 +23,7 @@
 /// The datastructure that holds the mapping between the active memory buffers
 /// and the individual skip mappings.
 using ExcludedPreprocessorDirectiveSkipMapping =
-llvm::DenseMap;
+llvm::DenseMap;
 
 } // end namespace clang
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -219,8 +219,6 @@
 
   llvm::ErrorOr status() override { return Stat; }
 
-  const llvm::MemoryBuffer *getBufferPtr() const { return Buffer.get(); }
-
   llvm::ErrorOr>
   getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
 bool IsVolatile) override {
@@ -248,7 +246,7 @@
/*RequiresNullTerminator=*/false),
   *Entry->getStatus());
   if (!Entry->getPPSkippedRangeMapping().empty() && PPSkipMappings)
-(*PPSkipMappings)[Result->getBufferPtr()] =
+(*PPSkipMappings)[Result->Buffer->getBufferStart()] =
 >getPPSkippedRangeMapping();
   return llvm::ErrorOr>(
   std::unique_ptr(std::move(Result)));
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -380,7 +380,10 @@
   std::pair HashFileOffset =
   SourceMgr.getDecomposedLoc(HashLoc);
   const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);
-  auto It = ExcludedConditionalDirectiveSkipMappings->find(Buf);
+  if (!Buf)
+return None;
+  auto It =
+  ExcludedConditionalDirectiveSkipMappings->find(Buf->getBufferStart());
   if (It == ExcludedConditionalDirectiveSkipMappings->end())
 return None;
 
Index: clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
===
--- clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
+++ clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
@@ -23,8 +23,7 @@
 /// The datastructure that holds the mapping between the active memory buffers
 /// and the individual skip mappings.
 using ExcludedPreprocessorDirectiveSkipMapping =
-llvm::DenseMap;
+llvm::DenseMap;
 
 } // end namespace clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson added inline comments.



Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:1082
+bool isGFX9Plus(const MCSubtargetInfo ) {
+  return isGFX9(STI) || isGFX10(STI);
+}

arsenm wrote:
> How are these changes related?
This is a mistake. I did not add this and I'm not sure why it is being included 
in the change.


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

https://reviews.llvm.org/D17993

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

[PATCH] D88299: [clang-format] Add MacroUnexpander.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is magnificently difficult :-)
I've sunk a fair few hours into understanding it now, and need to send some 
comments based on the interface+tests.
Some of these will be misguided or answered by the implementation, but I'm 
afraid I can't fit it all into my head (on a reasonable timeframe) until I 
understand it better.




Comment at: clang/lib/Format/FormatToken.h:449
 
+  /// When macro expansion introduces parents, those are marked as
+  /// \c MacroParent, so formatting knows their children need to be formatted.

I can't really understand from the comment when this is supposed to be set, and 
there are no tests of it.

(The comment is vague: is a "parent" the inverse of FormatToken::Children here? 
Is this scenario when the parents in question are new, or their children are 
new, or both? What part of the code is "formatting", and why would it otherwise 
skip the children?)



Comment at: clang/lib/Format/Macros.h:134
 
+/// Matches formatted lines that were created by macro expansion to a format
+/// for the original macro call.

"matches formatted lines" probably describes the hard technical problem it has 
to solve, but not so much what it does for the caller:  what the transformation 
is between its inputs and its outputs.

Is it something like:

```
Rewrites expanded code (containing tokens expanded from macros) into spelled 
code (containing tokens for the macro call itself). Token types are preserved, 
so macro arguments in the output have semantically-correct types from their 
expansion. This is the point of expansion/unexpansion: to allow this 
information to be used in formatting.
```

[Is it just tokentypes? I guess it's also Role and MustBreakBefore and some 
other stuff like that?]



Comment at: clang/lib/Format/Macros.h:142
+/// When getting the formatted lines of the expansion via the \c addLine 
method:
+/// -> class A {
+/// -> public:

I'm a bit confused by these arrows. It doesn't seem that they each point to an 
unwrappedline passed to addLine?



Comment at: clang/lib/Format/Macros.h:142
+/// When getting the formatted lines of the expansion via the \c addLine 
method:
+/// -> class A {
+/// -> public:

sammccall wrote:
> I'm a bit confused by these arrows. It doesn't seem that they each point to 
> an unwrappedline passed to addLine?
This example didn't really help me understand the interface of this class, it 
seems to be a special case:
 - the input is a single block construct (rather than e.g. a whole program), 
but it's not clear why that's the case.
 - the output (unexpanded form) consists of exactly a macro call with no 
leading/trailing tokens, which isn't true in general

If the idea is to provide as input the minimal range of lines that span the 
macro, we should say so somewhere. But I would like to understand why we're not 
simply processing the whole file.



Comment at: clang/lib/Format/Macros.h:147
+///
+/// Creates the unwrapped lines containing the macro call tokens so that
+/// the macro call tokens fit the semantic structure of the expanded formatted

this says "creates the unwrapped lines" but getResult() returns only one.
Does the plural here refer to the tree? Maybe just use singular or "the 
unwrappedlinetree"?



Comment at: clang/lib/Format/Macros.h:154
+/// -> })
+class MacroUnexpander {
+public:

I get the symmetry between the expander/unexpander classes, but the name is 
making it harder for me to follow the code.
 - the extra compound+negation in the name makes it hard/slow to understand, as 
I'm always thinking first about expansion
 - the fact that expansion/unexpansion are not actually opposites completely 
breaks my intuition. It also creates two different meanings of "unexpanded" 
that led me down the garden path a bit (e.g. in the test).

Would you consider `MacroCollapser`? It's artificial, but expand/collapse are 
well-known antonyms without being the same word.

(Incidentally, I just realized this is why I find "UnwrappedLine" so slippery - 
the ambiguity between "this isn't wrapped" and "this was unwrapped")



Comment at: clang/lib/Format/Macros.h:164
+  /// For the given \p Line, match all occurences of tokens expanded from a
+  /// macro and replace them with the original macro call in \c getResult().
+  void addLine(const UnwrappedLine );

I find this hard to follow.
- again, the "match" part is an implementation detail that sounds interesting 
but isn't :-)
- "from a macro" sounds like one in particular, but is actually every macro
- the bit about "getResult" is a separate point that feels shoehorned in

What about:
"Replaces tokens that were expanded from macros with the original macro calls. 
The result is stored and available in getResult()"



Comment at: 

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 297647.
CJ-Johnson added a comment.

Update with full diff context


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

https://reviews.llvm.org/D17993

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CXX/except/except.spec/p14-ir.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGen/temporary-lifetime.cpp
  clang/test/CodeGenCUDA/device-var-init.cu
  clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
  
clang/test/CodeGenCXX/RelativeVTablesABI/child-inheritted-from-parent-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-1.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-2.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/member-function-pointer.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-and-child-in-comdats.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/pass-byval-attributes.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/apple-kext.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/arm64-constructor-return.cpp
  clang/test/CodeGenCXX/array-default-argument.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/attr-disable-tail-calls.cpp
  clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  clang/test/CodeGenCXX/attr.cpp
  clang/test/CodeGenCXX/auto-variable-template.cpp
  clang/test/CodeGenCXX/blocks-cxx11.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/builtin_LINE.cpp
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/conditional-gnu-ext.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/constructors.cpp
  clang/test/CodeGenCXX/copy-constructor-elim-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-constructors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-init-captures.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-copy-omission.cpp
  clang/test/CodeGenCXX/cxx1z-decomposition.cpp
  clang/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx1z-lambda-star-this.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  clang/test/CodeGenCXX/default-arg-temps.cpp
  clang/test/CodeGenCXX/default-arguments.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/derived-to-base-conv.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure.cpp
  clang/test/CodeGenCXX/dllexport-members.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/duplicate-mangled-name.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/empty-nontrivially-copyable.cpp
  clang/test/CodeGenCXX/exceptions-seh-filter-captures.cpp
  clang/test/CodeGenCXX/exceptions-seh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/CodeGenCXX/float128-declarations.cpp
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/CodeGenCXX/global-dtor-no-atexit.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenCXX/goto.cpp
  clang/test/CodeGenCXX/hidden-dllimport.cpp
  

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:1082
+bool isGFX9Plus(const MCSubtargetInfo ) {
+  return isGFX9(STI) || isGFX10(STI);
+}

How are these changes related?


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

https://reviews.llvm.org/D17993

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 297645.
CJ-Johnson added a comment.
Herald added subscribers: llvm-commits, kerbowa, hiraditya, nhaehnle, arsenm.
Herald added a project: LLVM.

Update diff with full context


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

https://reviews.llvm.org/D17993

Files:
  llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
  llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h


Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
===
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -591,6 +591,7 @@
 bool isCI(const MCSubtargetInfo );
 bool isVI(const MCSubtargetInfo );
 bool isGFX9(const MCSubtargetInfo );
+bool isGFX9Plus(const MCSubtargetInfo );
 bool isGFX10(const MCSubtargetInfo );
 bool isGCN3Encoding(const MCSubtargetInfo );
 bool isGFX10_BEncoding(const MCSubtargetInfo );
Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
===
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -1078,6 +1078,10 @@
   return STI.getFeatureBits()[AMDGPU::FeatureGFX9];
 }
 
+bool isGFX9Plus(const MCSubtargetInfo ) {
+  return isGFX9(STI) || isGFX10(STI);
+}
+
 bool isGFX10(const MCSubtargetInfo ) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX10];
 }
Index: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
===
--- llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1190,6 +1190,10 @@
 return AMDGPU::isGFX9(getSTI());
   }
 
+  bool isGFX9Plus() const {
+return AMDGPU::isGFX9Plus(getSTI());
+  }
+
   bool isGFX10() const {
 return AMDGPU::isGFX10(getSTI());
   }
@@ -4699,7 +4703,7 @@
   for (MCRegAliasIterator R(AMDGPU::TTMP12_TTMP13_TTMP14_TTMP15, , true);
R.isValid(); ++R) {
 if (*R == RegNo)
-  return isGFX9() || isGFX10();
+  return isGFX9Plus();
   }
 
   // GFX10 has 2 more SGPRs 104 and 105.


Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
===
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -591,6 +591,7 @@
 bool isCI(const MCSubtargetInfo );
 bool isVI(const MCSubtargetInfo );
 bool isGFX9(const MCSubtargetInfo );
+bool isGFX9Plus(const MCSubtargetInfo );
 bool isGFX10(const MCSubtargetInfo );
 bool isGCN3Encoding(const MCSubtargetInfo );
 bool isGFX10_BEncoding(const MCSubtargetInfo );
Index: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
===
--- llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -1078,6 +1078,10 @@
   return STI.getFeatureBits()[AMDGPU::FeatureGFX9];
 }
 
+bool isGFX9Plus(const MCSubtargetInfo ) {
+  return isGFX9(STI) || isGFX10(STI);
+}
+
 bool isGFX10(const MCSubtargetInfo ) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX10];
 }
Index: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
===
--- llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1190,6 +1190,10 @@
 return AMDGPU::isGFX9(getSTI());
   }
 
+  bool isGFX9Plus() const {
+return AMDGPU::isGFX9Plus(getSTI());
+  }
+
   bool isGFX10() const {
 return AMDGPU::isGFX10(getSTI());
   }
@@ -4699,7 +4703,7 @@
   for (MCRegAliasIterator R(AMDGPU::TTMP12_TTMP13_TTMP14_TTMP15, , true);
R.isValid(); ++R) {
 if (*R == RegNo)
-  return isGFX9() || isGFX10();
+  return isGFX9Plus();
   }
 
   // GFX10 has 2 more SGPRs 104 and 105.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D17993#2325616 , @CJ-Johnson wrote:

> In D17993#2325610 , @jdoerfert wrote:
>
>> Can you please upload again with full context?
>
> My apologies, I am new to LLVM contribution. What is the best way to do that 
> such that it squashes all of my local git commits?

You can use git show HEAD -U99 > mypatch.patch (-U99 for context)

https://llvm.org/docs/Phabricator.html


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

https://reviews.llvm.org/D17993

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson added a comment.

In D17993#2325610 , @jdoerfert wrote:

> Can you please upload again with full context?

My apologies, I am new to LLVM contribution. What is the best way to do that 
such that it squashes all of my local git commits?


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

https://reviews.llvm.org/D17993

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Can you please upload again with full context?


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

https://reviews.llvm.org/D17993

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

Re: [clang] 9dcd96f - Canonicalize declaration pointers when forming APValues.

[Arthur Eubanks via cfe-commits]

Could there be an issue with Objective C property attributes?
https://crbug.com/1134762

On Sun, Oct 11, 2020 at 2:11 PM Richard Smith  wrote:

> Please can you try building with https://reviews.llvm.org/D89212 applied
> first, and see if it produces any warnings? If so, you're probably hitting
> PR47663, and should fix that by moving the 'weak' attribute earlier.
>
> On Sun, 11 Oct 2020 at 11:18, Richard Smith  wrote:
>
>> On Fri, 9 Oct 2020 at 19:12, Arthur Eubanks  wrote:
>>
>>> I think this is the cause of https://crbug.com/1134762. Can we
>>> speculatively revert while coming up with a repro? Or would you like a
>>> repro first?
>>>
>>
>> If you're blocked on this, sure, please go ahead and revert until you
>> have a repro. But the revert will block ongoing development work, so a
>> reproducer would definitely be appreciated! Per PR47663, there are some
>> pretty fundamental problems with adding the 'weak' attribute "too late", so
>> if that's what's going on, the best approach might be to move the 'weak'
>> attribute to an earlier declaration.
>>
>> On Sun, Sep 27, 2020 at 7:06 PM Richard Smith via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>

 Author: Richard Smith
 Date: 2020-09-27T19:05:26-07:00
 New Revision: 9dcd96f728863d40d6f5922ed52732fdd728fb5f

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

 LOG: Canonicalize declaration pointers when forming APValues.

 References to different declarations of the same entity aren't different
 values, so shouldn't have different representations.

 Recommit of e6393ee813178e9d3306b8e3c6949a4f32f8a2cb with fixed handling
 for weak declarations. We now look for attributes on the most recent
 declaration when determining whether a declaration is weak. (Second
 recommit with further fixes for mishandling of weak declarations. Our
 behavior here is fundamentally unsound -- see PR47663 -- but this
 approach attempts to not make things worse.)

 Added:


 Modified:
 clang/include/clang/AST/APValue.h
 clang/lib/AST/APValue.cpp
 clang/lib/AST/Decl.cpp
 clang/lib/AST/DeclBase.cpp
 clang/lib/AST/ExprConstant.cpp
 clang/lib/CodeGen/CGExprConstant.cpp
 clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
 clang/test/CodeGenCXX/weak-external.cpp
 clang/test/OpenMP/ordered_messages.cpp

 Removed:




 
 diff  --git a/clang/include/clang/AST/APValue.h
 b/clang/include/clang/AST/APValue.h
 index 5103cfa8604e..6307f8a92e5a 100644
 --- a/clang/include/clang/AST/APValue.h
 +++ b/clang/include/clang/AST/APValue.h
 @@ -174,6 +174,7 @@ class APValue {
return !(LHS == RHS);
  }
  friend llvm::hash_code hash_value(const LValueBase );
 +friend struct llvm::DenseMapInfo;

private:
  PtrTy Ptr;
 @@ -201,8 +202,7 @@ class APValue {

public:
  LValuePathEntry() : Value() {}
 -LValuePathEntry(BaseOrMemberType BaseOrMember)
 -:
 Value{reinterpret_cast(BaseOrMember.getOpaqueValue())} {}
 +LValuePathEntry(BaseOrMemberType BaseOrMember);
  static LValuePathEntry ArrayIndex(uint64_t Index) {
LValuePathEntry Result;
Result.Value = Index;

 diff  --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
 index 08ae0ff3c67d..32d3ff7ce1d0 100644
 --- a/clang/lib/AST/APValue.cpp
 +++ b/clang/lib/AST/APValue.cpp
 @@ -38,7 +38,7 @@ static_assert(
  "Type is insufficiently aligned");

  APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I,
 unsigned V)
 -: Ptr(P), Local{I, V} {}
 +: Ptr(P ? cast(P->getCanonicalDecl()) : nullptr),
 Local{I, V} {}
  APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V)
  : Ptr(P), Local{I, V} {}

 @@ -82,13 +82,19 @@ bool operator==(const APValue::LValueBase ,
  const APValue::LValueBase ) {
if (LHS.Ptr != RHS.Ptr)
  return false;
 -  if (LHS.is())
 +  if (LHS.is() || LHS.is())
  return true;
return LHS.Local.CallIndex == RHS.Local.CallIndex &&
   LHS.Local.Version == RHS.Local.Version;
  }
  }

 +APValue::LValuePathEntry::LValuePathEntry(BaseOrMemberType
 BaseOrMember) {
 +  if (const Decl *D = BaseOrMember.getPointer())
 +BaseOrMember.setPointer(D->getCanonicalDecl());
 +  Value = reinterpret_cast(BaseOrMember.getOpaqueValue());
 +}
 +
  namespace {
struct LVBase {
  APValue::LValueBase Base;
 @@ -113,14 +119,16 @@ 

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 297640.
CJ-Johnson edited the summary of this revision.
CJ-Johnson added a comment.
Herald added subscribers: jdoerfert, aheejin, sbc100, jvesely, dschuff.

Rebasing on head, removing flag changes since that was added in 
https://reviews.llvm.org/D47894 and fixing broken tests


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

https://reviews.llvm.org/D17993

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CXX/except/except.spec/p14-ir.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp
  clang/test/CodeGen/attr-nomerge.cpp
  clang/test/CodeGen/no-builtin.cpp
  clang/test/CodeGen/temporary-lifetime.cpp
  clang/test/CodeGenCUDA/device-var-init.cu
  clang/test/CodeGenCXX/2011-12-19-init-list-ctor.cpp
  
clang/test/CodeGenCXX/RelativeVTablesABI/child-inheritted-from-parent-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/child-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-1.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/cross-translation-unit-2.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/member-function-pointer.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-and-child-in-comdats.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/parent-vtable-in-comdat.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/pass-byval-attributes.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/simple-vtable-definition.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
  clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  clang/test/CodeGenCXX/amdgcn-automatic-variable.cpp
  clang/test/CodeGenCXX/amdgcn-func-arg.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
  clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
  clang/test/CodeGenCXX/apple-kext.cpp
  clang/test/CodeGenCXX/arm.cpp
  clang/test/CodeGenCXX/arm64-constructor-return.cpp
  clang/test/CodeGenCXX/array-default-argument.cpp
  clang/test/CodeGenCXX/atomicinit.cpp
  clang/test/CodeGenCXX/attr-disable-tail-calls.cpp
  clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  clang/test/CodeGenCXX/attr.cpp
  clang/test/CodeGenCXX/auto-variable-template.cpp
  clang/test/CodeGenCXX/blocks-cxx11.cpp
  clang/test/CodeGenCXX/blocks.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/CodeGenCXX/builtin_LINE.cpp
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  clang/test/CodeGenCXX/cfi-cross-dso.cpp
  clang/test/CodeGenCXX/conditional-gnu-ext.cpp
  clang/test/CodeGenCXX/const-init-cxx11.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp
  clang/test/CodeGenCXX/constructor-init.cpp
  clang/test/CodeGenCXX/constructors.cpp
  clang/test/CodeGenCXX/copy-constructor-elim-2.cpp
  clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
  clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-constructors.cpp
  clang/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx11-initializer-array-new.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-init-captures.cpp
  clang/test/CodeGenCXX/cxx1y-sized-deallocation.cpp
  clang/test/CodeGenCXX/cxx1z-copy-omission.cpp
  clang/test/CodeGenCXX/cxx1z-decomposition.cpp
  clang/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
  clang/test/CodeGenCXX/cxx1z-lambda-star-this.cpp
  clang/test/CodeGenCXX/cxx2a-destroying-delete.cpp
  clang/test/CodeGenCXX/debug-info-class.cpp
  clang/test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  clang/test/CodeGenCXX/default-arg-temps.cpp
  clang/test/CodeGenCXX/default-arguments.cpp
  clang/test/CodeGenCXX/delete.cpp
  clang/test/CodeGenCXX/derived-to-base-conv.cpp
  clang/test/CodeGenCXX/destructors.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
  clang/test/CodeGenCXX/devirtualize-virtual-function-calls.cpp
  clang/test/CodeGenCXX/dllexport-ctor-closure.cpp
  clang/test/CodeGenCXX/dllexport-members.cpp
  clang/test/CodeGenCXX/dllexport.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-members.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/CodeGenCXX/duplicate-mangled-name.cpp
  clang/test/CodeGenCXX/eh.cpp
  clang/test/CodeGenCXX/empty-nontrivially-copyable.cpp
  clang/test/CodeGenCXX/exceptions-seh-filter-captures.cpp
  clang/test/CodeGenCXX/exceptions-seh.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/CodeGenCXX/float128-declarations.cpp
  

[PATCH] D89177: [cmake] Add support for multiple distributions

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.
Herald added a subscriber: rdzhabarov.

We've already considered introducing a similar mechanism so thank you for 
working on this! There's one issue that I haven't figured out how to resolve 
and I'd be interested in your thoughts: building executables and libraries in 
the most optimal may require incompatible flags. For example, when building a 
shared library you have to use `-fPIC` but for executables that's suboptimal; 
similarly, when building executables you may want to use LTO but if you also 
want to create a distribution that contains static libraries, that's a problem 
because those will contain bitcode. So far our solution has been to simply do 
multiple builds with different flags (in GN this can be done in a single build 
by leveraging the "toolchain" concept that also allows sharing as much work as 
possible, but I haven't figured out how to do anything like that in CMake).




Comment at: llvm/cmake/modules/LLVMDistributionSupport.cmake:39
+# default (unnamed) distribution.
+set_property(GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target} " ")
+  endforeach()

Can we use `EMPTY` or something along those lines to make it more obvious?



Comment at: llvm/cmake/modules/LLVMDistributionSupport.cmake:186
+# the latter and fix it in the loop.
+set(distributions " ")
+  endif()

Can we use `EMPTY` or something along those lines to make it more obvious?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89177

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

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson commandeered this revision.
CJ-Johnson added a reviewer: bkramer.
CJ-Johnson added a comment.

The patch is ready! Commandeering this change :)


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

https://reviews.llvm.org/D17993

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

[PATCH] D66782: SourceManager: Prefer Optional over MemoryBuffer*

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:459
 Position P) {
-  llvm::StringRef Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
+  llvm::StringRef Code = SM.getBufferOrFake(SM.getMainFileID()).getBuffer();
   auto Offset =

arphaman wrote:
> I feel like all buffer requests for the main file should succeed and 
> shouldn't need the fake buffer, unless something gone terribly wrong, Do you 
> agree? Do you think it might be valuable to add a method like 
> `SM.getMainFileBuffer` which has an `llvm_unreachable` if buffer is invalid?
I agree the main file should really be there. But I'd rather keep this patch as 
NFC as possible, so I think not in this commit. I also don't see an easy way to 
add that assertion in a prep commit.

WDYT of doing it in a follow-up?



Comment at: clang/include/clang/Basic/SourceManager.h:303
 
+static FileInfo getForRecovery() {
+  FileInfo X = get(SourceLocation(), nullptr, SrcMgr::C_User, "");

arphaman wrote:
> This doesn't seem to be used in this patch, is this dead code?
I'll check and clean it up.



Comment at: clang/lib/Basic/SourceManager.cpp:1179
+*Invalid = !Buffer;
+  return Buffer ? Buffer->getBufferStart() + LocInfo.second : nullptr;
 }

arphaman wrote:
> How come you're returning `nullptr` here instead of `"<<< BUFFER"` like in the error condition above? It seems that clients will 
> not be able to handle this nullptr.
Good question, but I don't think we need to care since this is NFC. See the old 
`return` statement:
```
return Buffer->getBufferStart() + (CharDataInvalid? 0 : LocInfo.second);
```



Comment at: clang/lib/Basic/SourceManager.cpp:1467
+return Buffer->getBufferIdentifier();
+  return "";
 }

arphaman wrote:
> Should you return `""` here to be consistent with the 
> `"invalid loc"` above?
To keep this NFC, this should have the same identifier that the old `getBuffer` 
call would have had. I'll double check it was empty before.



Comment at: clang/lib/Basic/SourceManager.cpp:1706
+  Content->getBuffer(Diag, getFileManager());
   unsigned FilePos = Content->SourceLineCache[Line - 1];
   const char *Buf = Buffer->getBufferStart() + FilePos;

arphaman wrote:
> It appears you're not checking if `Buffer` is `None` here. Previously this 
> should've been fine as it seems `getBuffer` created fake recovery buffers.
That's a good point.

It actually adds a concern for me, since this patch is fairly old (rebased 
after over a year). Could there be calls to `getBuffer` that I've missed?

I can do a more careful audit.

There's a way to do this more safely: adding a new API with a different name, 
migrating everyone over, deleting the old API, and then renaming the new API. 
However, I'm concerned that will create even more churn. Do you think that 
would be better?


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

https://reviews.llvm.org/D66782

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

[PATCH] D88469: [clangd] Heuristic resolution for dependent type and template names

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b962fdd5f36: [clangd] Heuristic resolution for dependent 
type and template names (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88469

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -728,6 +728,54 @@
"template  T convert() const");
 }
 
+TEST_F(TargetDeclTest, DependentTypes) {
+  Flags = {"-fno-delayed-template-parsing"};
+
+  // Heuristic resolution of dependent type name
+  Code = R"cpp(
+template 
+struct A { struct B {}; };
+
+template 
+void foo(typename A::[[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc", "struct B");
+
+  // Heuristic resolution of dependent type name which doesn't get a TypeLoc
+  Code = R"cpp(
+template 
+struct A { struct B { struct C {}; }; };
+
+template 
+void foo(typename A::[[B]]::C);
+  )cpp";
+  EXPECT_DECLS("NestedNameSpecifierLoc", "struct B");
+
+  // Heuristic resolution of dependent type name whose qualifier is also
+  // dependent
+  Code = R"cpp(
+template 
+struct A { struct B { struct C {}; }; };
+
+template 
+void foo(typename A::B::[[C]]);
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc", "struct C");
+
+  // Heuristic resolution of dependent template name
+  Code = R"cpp(
+template 
+struct A {
+  template  struct B {};
+};
+
+template 
+void foo(typename A::template [[B]]);
+  )cpp";
+  EXPECT_DECLS("DependentTemplateSpecializationTypeLoc",
+   "template  struct B");
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -125,6 +125,10 @@
   return !D->isCXXInstanceMember();
 };
 const auto ValueFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TypeFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TemplateFilter = [](const NamedDecl *D) {
+  return isa(D);
+};
 
 // Given the type T of a dependent expression that appears of the LHS of a
 // "->", heuristically find a corresponding pointee type in whose scope we
@@ -219,19 +223,45 @@
   return {};
 }
 
-// Try to heuristically resolve the type of a possibly-dependent expression `E`.
-const Type *resolveExprToType(const Expr *E) {
-  std::vector Decls = resolveExprToDecls(E);
+const Type *resolveDeclsToType(const std::vector ) {
   if (Decls.size() != 1) // Names an overload set -- just bail.
 return nullptr;
   if (const auto *TD = dyn_cast(Decls[0])) {
 return TD->getTypeForDecl();
-  } else if (const auto *VD = dyn_cast(Decls[0])) {
+  }
+  if (const auto *VD = dyn_cast(Decls[0])) {
 return VD->getType().getTypePtrOrNull();
   }
   return nullptr;
 }
 
+// Try to heuristically resolve the type of a possibly-dependent expression `E`.
+const Type *resolveExprToType(const Expr *E) {
+  return resolveDeclsToType(resolveExprToDecls(E));
+}
+
+// Try to heuristically resolve the type of a possibly-dependent nested name
+// specifier.
+const Type *resolveNestedNameSpecifierToType(const NestedNameSpecifier *NNS) {
+  if (!NNS)
+return nullptr;
+
+  switch (NNS->getKind()) {
+  case NestedNameSpecifier::TypeSpec:
+  case NestedNameSpecifier::TypeSpecWithTemplate:
+return NNS->getAsType();
+  case NestedNameSpecifier::Identifier: {
+return resolveDeclsToType(getMembersReferencedViaDependentName(
+resolveNestedNameSpecifierToType(NNS->getPrefix()),
+[&](const ASTContext &) { return NNS->getAsIdentifier(); },
+TypeFilter));
+  }
+  default:
+break;
+  }
+  return nullptr;
+}
+
 const NamedDecl *getTemplatePattern(const NamedDecl *D) {
   if (const CXXRecordDecl *CRD = dyn_cast(D)) {
 if (const auto *Result = CRD->getTemplateInstantiationPattern())
@@ -291,10 +321,8 @@
 //and both are lossy. We must know upfront what the caller ultimately wants.
 //
 // FIXME: improve common dependent scope using name lookup in primary templates.
-// We currently handle DependentScopeDeclRefExpr and
-// CXXDependentScopeMemberExpr, but some other constructs remain to be handled:
-//  - DependentTemplateSpecializationType,
-//  - DependentNameType
+// We currently handle several dependent constructs, but some others remain to
+// be handled:
 //  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = 

[clang-tools-extra] 1b962fd - [clangd] Heuristic resolution for dependent type and template names

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-10-12T13:37:22-04:00
New Revision: 1b962fdd5f365a10684d9f70d703ae101c20d37a

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

LOG: [clangd] Heuristic resolution for dependent type and template names

Fixes https://github.com/clangd/clangd/issues/543

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 4cf62d3d1539..19ffdbb7c7ea 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -125,6 +125,10 @@ const auto StaticFilter = [](const NamedDecl *D) {
   return !D->isCXXInstanceMember();
 };
 const auto ValueFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TypeFilter = [](const NamedDecl *D) { return isa(D); };
+const auto TemplateFilter = [](const NamedDecl *D) {
+  return isa(D);
+};
 
 // Given the type T of a dependent expression that appears of the LHS of a
 // "->", heuristically find a corresponding pointee type in whose scope we
@@ -219,19 +223,45 @@ std::vector resolveExprToDecls(const 
Expr *E) {
   return {};
 }
 
-// Try to heuristically resolve the type of a possibly-dependent expression 
`E`.
-const Type *resolveExprToType(const Expr *E) {
-  std::vector Decls = resolveExprToDecls(E);
+const Type *resolveDeclsToType(const std::vector ) {
   if (Decls.size() != 1) // Names an overload set -- just bail.
 return nullptr;
   if (const auto *TD = dyn_cast(Decls[0])) {
 return TD->getTypeForDecl();
-  } else if (const auto *VD = dyn_cast(Decls[0])) {
+  }
+  if (const auto *VD = dyn_cast(Decls[0])) {
 return VD->getType().getTypePtrOrNull();
   }
   return nullptr;
 }
 
+// Try to heuristically resolve the type of a possibly-dependent expression 
`E`.
+const Type *resolveExprToType(const Expr *E) {
+  return resolveDeclsToType(resolveExprToDecls(E));
+}
+
+// Try to heuristically resolve the type of a possibly-dependent nested name
+// specifier.
+const Type *resolveNestedNameSpecifierToType(const NestedNameSpecifier *NNS) {
+  if (!NNS)
+return nullptr;
+
+  switch (NNS->getKind()) {
+  case NestedNameSpecifier::TypeSpec:
+  case NestedNameSpecifier::TypeSpecWithTemplate:
+return NNS->getAsType();
+  case NestedNameSpecifier::Identifier: {
+return resolveDeclsToType(getMembersReferencedViaDependentName(
+resolveNestedNameSpecifierToType(NNS->getPrefix()),
+[&](const ASTContext &) { return NNS->getAsIdentifier(); },
+TypeFilter));
+  }
+  default:
+break;
+  }
+  return nullptr;
+}
+
 const NamedDecl *getTemplatePattern(const NamedDecl *D) {
   if (const CXXRecordDecl *CRD = dyn_cast(D)) {
 if (const auto *Result = CRD->getTemplateInstantiationPattern())
@@ -291,10 +321,8 @@ const NamedDecl *getTemplatePattern(const NamedDecl *D) {
 //and both are lossy. We must know upfront what the caller ultimately 
wants.
 //
 // FIXME: improve common dependent scope using name lookup in primary 
templates.
-// We currently handle DependentScopeDeclRefExpr and
-// CXXDependentScopeMemberExpr, but some other constructs remain to be handled:
-//  - DependentTemplateSpecializationType,
-//  - DependentNameType
+// We currently handle several dependent constructs, but some others remain to
+// be handled:
 //  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = DeclRelationSet;
@@ -536,6 +564,23 @@ struct TargetFinder {
 if (auto *TD = DTST->getTemplateName().getAsTemplateDecl())
   Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
   }
+  void VisitDependentNameType(const DependentNameType *DNT) {
+for (const NamedDecl *ND : getMembersReferencedViaDependentName(
+ resolveNestedNameSpecifierToType(DNT->getQualifier()),
+ [DNT](ASTContext &) { return DNT->getIdentifier(); },
+ TypeFilter)) {
+  Outer.add(ND, Flags);
+}
+  }
+  void VisitDependentTemplateSpecializationType(
+  const DependentTemplateSpecializationType *DTST) {
+for (const NamedDecl *ND : getMembersReferencedViaDependentName(
+ resolveNestedNameSpecifierToType(DTST->getQualifier()),
+ [DTST](ASTContext &) { return DTST->getIdentifier(); },
+ TemplateFilter)) {
+  Outer.add(ND, Flags);
+}
+  }
   void VisitTypedefType(const TypedefType *TT) {
 Outer.add(TT->getDecl(), Flags);
   }
@@ -591,17 +636,16 @@ struct TargetFinder {
   return;
 debug(*NNS, Flags);
 switch (NNS->getKind()) {
-case 

[clang] 012dd42 - [X86] Support -march=x86-64-v[234]

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-10-12T10:29:46-07:00
New Revision: 012dd42e027e2ff3d183cc9dcf27004cf9711720

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

LOG: [X86] Support -march=x86-64-v[234]

PR47686. These micro-architecture levels are defined in the x86-64 psABI:

https://gitlab.com/x86-psABIs/x86-64-ABI/-/commit/77566eb03bc6a326811cb7e9

GCC 11 will support these levels.

Note, -mtune=x86-64-v[234] are invalid and __builtin_cpu_is cannot be
used on them.

Reviewed By: craig.topper, RKSimon

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

Added: 
clang/test/Preprocessor/predefined-arch-macros-x86.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/docs/UsersManual.rst
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/test/CodeGen/attr-target-x86.c
clang/test/Driver/x86-march.c
clang/test/Driver/x86-mtune.c
clang/test/Misc/target-invalid-cpu-note.c
clang/test/Preprocessor/predefined-arch-macros.c
clang/test/Sema/builtin-cpu-supports.c
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/Support/X86TargetParser.h
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Target/X86/X86.td
llvm/test/CodeGen/X86/cpus-other.ll

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 66427f293775..d0ef03acaa4c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,7 +188,10 @@ X86 Support in Clang
 - The x86 intrinsics ``__rorb``, ``__rorw``, ``__rord``, ``__rorq`, ``_rotr``,
   ``_rotwr`` and ``_lrotr`` may now be used within constant expressions.
 
-- Support for -march=sapphirerapids was added.
+- Support for ``-march=sapphirerapids`` was added.
+
+- Support for ``-march=x86-64-v[234]`` has been added.
+  See :doc:`UsersManual` for details about these micro-architecture levels.
 
 - The -mtune command line option is no longer ignored for X86. This can be used
   to request microarchitectural optimizations independent on -march. 
-march=

diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index ed6c9e3bc341..f313ce72d8ed 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -3201,6 +3201,15 @@ and the ABI remains 32-bit but the assembler emits 
instructions
 appropriate for a CPU running in 16-bit mode, with address-size and
 operand-size prefixes to enable 32-bit addressing and operations.
 
+Several micro-architecture levels as specified by the x86-64 psABI are defined.
+They are cumulative in the sense that features from previous levels are
+implicitly included in later levels.
+
+- ``-march=x86-64``: CMOV, CMPXCHG8B, FPU, FXSR, MMX, FXSR, SCE, SSE, SSE2
+- ``-march=x86-64-v2``: (close to Nehalem) CMPXCHG16B, LAHF-SAHF, POPCNT, 
SSE3, SSE4.1, SSE4.2, SSSE3
+- ``-march=x86-64-v3``: (close to Haswell) AVX, AVX2, BMI1, BMI2, F16C, FMA, 
LZCNT, MOVBE, XSAVE
+- ``-march=x86-64-v4``: AVX512F, AVX512BW, AVX512CD, AVX512DQ, AVX512VL
+
 ARM
 ^^^
 

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index ef83703d6097..98ac13b1ae9b 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -506,6 +506,9 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
,
   case CK_K8:
   case CK_K8SSE3:
   case CK_x86_64:
+  case CK_x86_64_v2:
+  case CK_x86_64_v3:
+  case CK_x86_64_v4:
 defineCPUMacros(Builder, "k8");
 break;
   case CK_AMDFAM10:
@@ -1312,6 +1315,9 @@ Optional X86TargetInfo::getCPUCacheLineSize() 
const {
 case CK_ZNVER2:
 // Deprecated
 case CK_x86_64:
+case CK_x86_64_v2:
+case CK_x86_64_v3:
+case CK_x86_64_v4:
 case CK_Yonah:
 case CK_Penryn:
 case CK_Core2:
@@ -1456,7 +1462,7 @@ void 
X86TargetInfo::fillValidCPUList(SmallVectorImpl ) const {
 }
 
 void X86TargetInfo::fillValidTuneCPUList(SmallVectorImpl ) 
const {
-  llvm::X86::fillValidCPUArchList(Values);
+  llvm::X86::fillValidTuneCPUList(Values);
 }
 
 ArrayRef X86TargetInfo::getGCCRegNames() const {

diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 7b2b7dcf6460..4fc495a09bbb 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -314,7 +314,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
 // Allow 32-bit only CPUs regardless of 64-bit mode unlike isValidCPUName.
 // NOTE: gcc rejects 32-bit mtune CPUs in 64-bit mode. But being lenient
 // since mtune was ignored by clang for so long.
-return llvm::X86::parseArchX86(Name) != llvm::X86::CK_None;
+return llvm::X86::parseTuneCPU(Name) != llvm::X86::CK_None;
   }
 
   void fillValidCPUList(SmallVectorImpl ) const override;

diff  --git a/clang/test/CodeGen/attr-target-x86.c