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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov marked 12 inline comments as done.
mizvekov added inline comments.



Comment at: clang-tools-extra/clangd/unittests/ASTTests.cpp:180
   )cpp",
-  // FIXME: it'd be nice if this resolved to the alias instead
-  "struct Foo",
+  // It's so nice that this is resolved to the alias instead :-D
+  "Bar",

rsmith wrote:
> Very true. But probably not worth keeping the comment. :)
Such a party pooper =)



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1350-1362
+  QualType ParDesug;
+  auto updatePar = [, , ](QualType T) {
+Param = T;
+ParDesug = T.getDesugaredType(S.Context);
+  };
+  updatePar(Param);
+

rsmith wrote:
> rsmith wrote:
> > `Par` is an unusual name for a parameter type; `Parm` or `Param` would be 
> > more common and I'd find either of those to be clearer. (Ideally use the 
> > same spelling for `Param` and `ParDesug`.) Given that the description in 
> > the standard uses `P` and `A` as the names of these types, those names 
> > would be fine here too if you want something short.
> Instead of tracking two types here, I think it'd be clearer to follow the 
> more usual pattern in Clang: track only `Param` and `Arg`, use 
> `Arg->getAs()` instead of `dyn_cast(ArgDesug)` to identify if `Arg` is 
> sugar for a `T`, and use `Arg->getAs()` instead of `cast(ArgDesug)` to 
> get a minimally-desugared `T*` from `Arg`.
> 
> The only place where I think we'd want something different from that is in 
> the `switch (Param->getTypeClass())` below, where I think we should switch on 
> the type class of the canonical type (or equivalently on the type class of 
> the desugared type, but it's cheaper and more obviously correct to ask for 
> the type class of the canonical type).
There was one small catch in your suggestion:
Using getAs on TemplateSpecializationType is a bit tricky because that type can 
be sugar as well, so you might end up with a type alias instead of the 
underlying thing you wanted. I think that was the only tricky type though.
And it was productive that I ran into this problem, because I ended up 
discovering some cases where we were losing some sugar there, and there was a 
tiny diagnostic improvement in one of the test cases as a result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D110656: [clang][Sema] Warn on uninitialized array elments

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D110656#3034181 , @nathanchance 
wrote:

> I can test this on the Linux kernel tonight and report the results tomorrow 
> morning.

My apologies, I did not have time to get to this last night or today and I am 
going to be on vacation starting today for a week and a half. I can report back 
once I am back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110656

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

Maintenance works at llvm lab today at 7PM PST

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM lab and build bot will be unavailable for about an hour starting from
7:00 PM PST today for some maintenance work.

Thank you for understanding.

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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D49864#2857630 , @janosimas wrote:

> That makes sense.
> Should I add it somewhere? Or do I need to talk to someone?

Please update clang-tools-extra/docs/ReleaseNotes.rst. It looks like there's no 
other docs for this script.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D49864

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

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

I'll take a look at this over the weekend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:54
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+

cchen wrote:
> Do we really expect `sa[1].f.b` be the same as `[0]`? Shouldn't `sa[1].f.b` 
> be the same as `[0]`?
Yes, looks so, missed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added inline comments.



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:54
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+

Do we really expect `sa[1].f.b` be the same as `[0]`? Shouldn't `sa[1].f.b` 
be the same as `[0]`?



Comment at: 
openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers_array.cpp:68
+  printf("%d %d %d %4.5f %d\n", sa[1].e, sa[1].f.a, sa[1].f.c.a, sa[1].f.b[1],
+ sa[1].f.b == [0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100673

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D110745#3035975 , @nikic wrote:

> Sorry, but the fact that there is still no way to opt-in to the old behavior 
> is still a blocker from my side. If we can't use `dereferenceable + nofree` 
> arguments for that purpose, then we need to provide a different way to do 
> that. Like `dereferenceable + really_nofree`. It looks like the current 
> implementation doesn't even accept the `dereferenceable + nofree + noalias` 
> case originally proposed (which is pretty bad from a design perspective, but 
> would at least work fairly well for rustc in practice). I don't think that 
> our current analysis capabilities are sufficient to land this change at this 
> time.

@nikic Do you have any specific examples of where this causes a workload to 
regress?  At this point, I really need something specific as opposed to a 
general concern.  We're at the point where perfection is very much the enemy of 
the good here.  As noted, I've already spent a lot of time trying to minimize 
impact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Josh Stone via cfe-commits]

On 10/1/21 2:59 PM, Josh Stone wrote:
> On 9/30/21 4:15 PM, Mehdi AMINI via llvm-dev wrote:
>> On Thu, Sep 30, 2021 at 4:09 PM Brian Cain  wrote:
>>>
>>>
>>>
>>> On Thu, Sep 30, 2021, 6:04 PM Brian Cain  wrote:

 Does something like Rust's "bors" bot satisfy the herald rules need?
>>>
>>>
>>>
>>> sorry, maybe I was thinking of the high-five bot. And it looks like that's 
>>> not quite a match for herald.
>>
>> Actually high-five may be a good starting point!
>> In practice it may still be a bit limited by the GitHub integration:
>> for example I suspect you may not be able to "subscribe" someone to a
>> pull-request?
>> Also what the user will receive as an email may be quite unhelpful
>> (you have been subscribed to "" instead of the
>> current more comprehensive emails).
> 
> You can configure path-based "mentions" like these:
> https://github.com/rust-lang/highfive/blob/6e2c21639aaeafaeae423b244d353247c507d46a/highfive/configs/rust-lang/rust.json#L129
> 
> It will mention those users in a comment, which subscribes them, like:
> https://github.com/rust-lang/rust/pull/89266#issuecomment-927275025
> 
> That one demonstrates both an individual and an org team, and note that
> people can choose whether their team membership is publicly visible.

Hmm, maybe you can only choose your org visibility, not each team.

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

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Josh Stone via cfe-commits]

On 9/30/21 4:15 PM, Mehdi AMINI via llvm-dev wrote:
> On Thu, Sep 30, 2021 at 4:09 PM Brian Cain  wrote:
>>
>>
>>
>> On Thu, Sep 30, 2021, 6:04 PM Brian Cain  wrote:
>>>
>>> Does something like Rust's "bors" bot satisfy the herald rules need?
>>
>>
>>
>> sorry, maybe I was thinking of the high-five bot. And it looks like that's 
>> not quite a match for herald.
> 
> Actually high-five may be a good starting point!
> In practice it may still be a bit limited by the GitHub integration:
> for example I suspect you may not be able to "subscribe" someone to a
> pull-request?
> Also what the user will receive as an email may be quite unhelpful
> (you have been subscribed to "" instead of the
> current more comprehensive emails).

You can configure path-based "mentions" like these:
https://github.com/rust-lang/highfive/blob/6e2c21639aaeafaeae423b244d353247c507d46a/highfive/configs/rust-lang/rust.json#L129

It will mention those users in a comment, which subscribes them, like:
https://github.com/rust-lang/rust/pull/89266#issuecomment-927275025

That one demonstrates both an individual and an org team, and note that
people can choose whether their team membership is publicly visible.

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

[PATCH] D110954: [clangd] Improve PopulateSwitch tweak

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 376635.
dgoldman marked an inline comment as done.
dgoldman added a comment.

Minor fixes for review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110954

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -23,6 +23,7 @@
 CodeContext Context;
 llvm::StringRef TestSource;
 llvm::StringRef ExpectedSource;
+llvm::StringRef FileName = "TestTU.cpp";
   };
 
   Case Cases[]{
@@ -206,10 +207,43 @@
   R""(template void f() {enum Enum {A}; ^switch (A) {}})"",
   "unavailable",
   },
+  {// C: Only filling in missing enumerators
+   Function,
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+^switch (val) {case B:break;}
+  )"",
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+switch (val) {case B:break;case A:case C:break;}
+  )"",
+   "TestTU.c"},
+  {// C: Only filling in missing enumerators w/ typedefs
+   Function,
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (^controlState) {case A:break;}
+  )"",
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (controlState) {case A:break;case B:case C:break;}
+  )"",
+   "TestTU.c"},
   };
 
   for (const auto  : Cases) {
 Context = Case.Context;
+FileName = Case.FileName;
 EXPECT_EQ(apply(Case.TestSource), Case.ExpectedSource);
   }
 }
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -113,7 +113,8 @@
 return false;
   // Ignore implicit casts, since enums implicitly cast to integer types.
   Cond = Cond->IgnoreParenImpCasts();
-  EnumT = Cond->getType()->getAsAdjusted();
+  // Get the canonical type to handle typedefs.
+  EnumT = Cond->getType().getCanonicalType()->getAsAdjusted();
   if (!EnumT)
 return false;
   EnumD = EnumT->getDecl();
@@ -152,14 +153,30 @@
 if (CS->caseStmtIsGNURange())
   return false;
 
+// Support for direct references to enum constants. This is required to
+// support C and ObjC which don't contain values in their ConstantExprs.
+// The general way to get the value of a case is EvaluateAsRValue, but we'd
+// rather not deal with that in case the AST is broken.
+if (auto *DRE = dyn_cast(CS->getLHS()->IgnoreParenCasts())) {
+  if (auto *Enumerator = dyn_cast(DRE->getDecl())) {
+auto Iter = ExpectedCases.find(Normalize(Enumerator->getInitVal()));
+if (Iter != ExpectedCases.end())
+  Iter->second.setCovered();
+continue;
+  }
+}
+
+// ConstantExprs with values are expected for C++, otherwise the storage
+// kind will be None.
+
 // Case expression is not a constant expression or is value-dependent,
 // so we may not be able to work out which cases are covered.
 const ConstantExpr *CE = dyn_cast(CS->getLHS());
 if (!CE || CE->isValueDependent())
   return false;
 
-// Unsure if this case could ever come up, but prevents an unreachable
-// executing in getResultAsAPSInt.
+// We need a stored value in order to continue; currently both C and ObjC
+// enums won't have one.
 if (CE->getResultStorageKind() == ConstantExpr::RSK_None)
   return false;
 auto Iter = ExpectedCases.find(Normalize(CE->getResultAsAPSInt()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106409: [PowerPC] Truncate exponent parameter for vec_cts,vec_ctf

[Zarko Todorovski via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/altivec.h:3178
+  : (__builtin_vsx_xvcvuxdsp((vector unsigned long long)(__a)) *   
\
+ (vector float)(vector unsigned)((0x7f - (__b)) << 23 & 0x1F)),
\
+vector signed long long
\

nemanjai wrote:
> Hmm... aren't you truncating the wrong thing here (and for all the other 
> shifted ones? Shouldn't it be something like:
> ```
> (vector float)(vector unsigned)((0x7f - ((__b) & 0x1F)) << 23)),
> ```
Thanks, I wasn't doing the truncation correctly. Redid to what I think is 
correct now. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106409

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

[PATCH] D109632: [clang] de-duplicate methods from AST files

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D109632#3033489 , @rmaz wrote:

> In D109632#3032381 , @vsapsai wrote:
>
>> What's interesting, I was able to trigger more diagnostic. Specifically, I 
>> got warnings about `length` ambiguity because in NSStatusItem it is CGFloat 
>> ,
>>  while in NSString it is NSUInteger. I also had more diagnostic that's 
>> unclear how it is triggered. It can be a project issue or a bug somewhere 
>> else, need to check it more carefully.
>
> Yes, I also had a couple of files fail to compile due to mismatched (or 
> differently matched) selectors.

Diagnostic about `-length` is due to isAcceptableMethodMismatch 

 returning different results depending on the order of methods. Probably other 
diagnostic is caused by a similar issue. Will need to add tests and to fix 
`isAcceptableMethodMismatch`.

>> In theory, "set dedupe" approach should be slower than "no external" as we 
>> are iterating through shared dependencies. But in practice it isn't, which 
>> is puzzling. My current theory is that "no external" also feeds into 
>> correctness, so we might be doing more [correct] method overloading checks.
>
> Well, wouldn't the tradeoff there be that we now have to descend into all 
> dependent modules during selector lookup when we didn't have to previously? 
> And this would do more work as only a small subset of those modules would 
> have a selector match, which in the current case has already been handled and 
> serialized on a single method list.

My assumption was that all dependent modules are in memory at this point. And 
we visit transitive modules only once, so I don't expect it to be a big 
performance hit (though I can be wrong). And deserializing methods from 
different modules shouldn't be more work because we are deserializing fewer 
methods than with "set dedupe". Maybe the order of methods matters? I can see 
us short circuiting in some cases but not the others. Though that's not a very 
plausible explanation for 20-25% discrepancy in compilation time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109632

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

[PATCH] D106409: [PowerPC] Truncate results for out of range values for vec_cts,vec_ctf

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 376630.
ZarkoCA edited the summary of this revision.
ZarkoCA added a comment.
Herald added a subscriber: steven.zhang.

- Only truncate **b** and not result
- Add a proper test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106409

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/ppc-vec_ct-truncate.c

Index: clang/test/CodeGen/ppc-vec_ct-truncate.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc-vec_ct-truncate.c
@@ -0,0 +1,90 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+// RUN:   -triple powerpc64-ibm-aix-xcoff -emit-llvm %s -o - \
+// RUN:   -target-cpu pwr8 | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+// RUN:   -triple powerpc64-unknown-linux-gnu -emit-llvm %s -o - \
+// RUN:   -target-cpu pwr8 | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -target-feature +vsx \
+// RUN:   -triple powerpc64le-unknown-linux-gnu -emit-llvm %s -o - \
+// RUN:   -target-cpu pwr8 | FileCheck %s
+#include 
+vector double a1 = {-1.234e-5, 1.2345};
+vector double a2 = {-1.234e-5, 1.2345};
+vector signed int vd1, vd2;
+vector double vd3 = {0.234, 1.234};
+vector float vf1 = {0.234, 1.234, 2.345, 3.456};
+vector float vf2 = {0.234, 1.234, 2.345, 3.456};
+vector float vf3 = {0.234, 1.234, 2.345, 3.456};
+vector float vf4 = {0.234, 1.234, 2.345, 3.456};
+vector float vf5 = {0.234, 1.234, 2.345, 3.456};
+vector signed int vsi1 = {1, 2, 3, 4};
+vector signed int vsi2 = {1, 2, 3, 4};
+vector signed int vsi3 = {1, 2, 3, 4};
+vector signed int vsi4 = {1, 2, 3, 4};
+vector long long int vl1 = {50, 10};
+vector long long int vl2 = {50, 10};
+vector double vd;
+vector float vf;
+vector signed long long vsll;
+vector unsigned int vsi5;
+vector unsigned int vsi6;
+vector unsigned long long vull;
+
+void test() {
+  // CHECK-LABEL: @test(
+  // CHECK-NEXT:  entry:
+  // CHECK-LE-LABEL: @test(
+  // CHECK-LE-NEXT:  entry:
+
+  vd1 = vec_cts(a1, 31);
+  //  CHECK:   [[TMP0:%.*]] = load <2 x double>, <2 x double>* @a1, align 16
+  //  CHECK-NEXT:  fmul <2 x double> [[TMP0]], 
+
+  vd2 = vec_cts(a2, 500);
+  // CHECK:[[TMP4:%.*]] = load <2 x double>, <2 x double>* @a2, align 16
+  // CHECK-NEXT:   fmul <2 x double> [[TMP4]], 
+
+  vsi3 = vec_ctu(vf1, 31);
+  // CHECK:[[TMP8:%.*]] = load <4 x float>, <4 x float>* @vf1, align 16
+  // CHECK-NEXT:   call <4 x i32> @llvm.ppc.altivec.vctuxs(<4 x float> [[TMP8]], i32 31)
+
+  vsi4 = vec_ctu(vf2, 500);
+  // CHECK:[[TMP10:%.*]] = load <4 x float>, <4 x float>* @vf2, align 16
+  // CHECK-NEXT:   call <4 x i32> @llvm.ppc.altivec.vctuxs(<4 x float> [[TMP10]], i32 20)
+
+  vull = vec_ctul(vf1, 31);
+  // CHECK:[[TMP12:%.*]] = load <4 x float>, <4 x float>* @vf1, align 16
+  // CHECK-NEXT:   fmul <4 x float> [[TMP12]], 
+
+  vull = vec_ctul(vf3, 500);
+  // CHECK:[[TMP21:%.*]] = load <4 x float>, <4 x float>* @vf3, align 16
+  // CHECK-NEXT:   fmul <4 x float> [[TMP21]], 
+
+  vsll = vec_ctsl(vf4, 31);
+  // CHECK:[[TMP30:%.*]] = load <4 x float>, <4 x float>* @vf4, align 16
+  // CHECK-NEXT:   fmul <4 x float> [[TMP30]], 
+
+  vsll = vec_ctsl(vf5, 500);
+  // CHECK:[[TMP39:%.*]] = load <4 x float>, <4 x float>* @vf5, align 16
+  // CHECK-NEXT:   fmul <4 x float> [[TMP39]], 
+
+  vf = vec_ctf(vsi1, 31);
+  // CHECK:[[TMP48:%.*]] = load <4 x i32>, <4 x i32>* @vsi1, align 16
+  // CHECK-NEXT:   call <4 x float> @llvm.ppc.altivec.vcfsx(<4 x i32> [[TMP48]], i32 31)
+
+  vf = vec_ctf(vsi2, 500);
+  // CHECK:[[TMP50:%.*]] = load <4 x i32>, <4 x i32>* @vsi2, align 16
+  // CHECK-NEXT:   call <4 x float> @llvm.ppc.altivec.vcfsx(<4 x i32> [[TMP50]], i32 20)
+
+  vd = vec_ctd(vsi3, 31);
+  // CHECK:[[TMP53:%.*]] = load <4 x i32>, <4 x i32>* @vsi3, align 16
+  // CHECK:[[TMP83:%.*]] = call <2 x double> @llvm.ppc.vsx.xvcvsxwdp(<4 x i32> [[TMP82:%.*]])
+  // CHECK-NEXT:   fmul <2 x double> [[TMP83]], 
+
+  vd = vec_ctd(vsi4, 500);
+
+  // CHECK:[[TMP84:%.*]] = load <4 x i32>, <4 x i32>* @vsi4, align 16
+  // CHECK:[[TMP115:%.*]] = call <2 x double> @llvm.ppc.vsx.xvcvsxwdp(<4 x i32> [[TMP114:%.*]])
+  // CHECK-NEXT:   fmul <2 x double> [[TMP115]], 
+}
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -3178,64 +3178,69 @@
 #ifdef __XL_COMPAT_ALTIVEC__
 #define vec_ctf(__a, __b)  \
   _Generic((__a), vector int   \
-   : (vector float)__builtin_altivec_vcfsx((vector int)(__a), (__b)),  \
+   : (vector 

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

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added inline comments.



Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:212
+  DynTypedNode Node = DynTypedNode::create(tl);
+  EXPECT_TRUE(Node == Node);
+  EXPECT_FALSE(Node < Node);

I don't know what we are trying to check with this self equivalence.
Did you mean `Node == tl`?



Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:225
+  const auto ptl =
+  matches[0].getNodeAs("ptl")->castAs();
+  DynTypedNode Node = DynTypedNode::create(ptl);

We should check that we can do getNodeAs and getNodeAs.
That is the goal of having the hierarchy in DynTypedNode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

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

[PATCH] D110954: [clangd] Improve PopulateSwitch tweak

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp:178
 
-// Unsure if this case could ever come up, but prevents an unreachable
-// executing in getResultAsAPSInt.
+// We need a storage kind in order to be able to fetch the value type,
+// currently both C and ObjC enums will return none.

"we need a storage kind" doesn't seem quite right, rather "we need a stored 
value"



Comment at: 
clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp:210
   },
+  {// C: Only filling in missing enumerators
+   Function,

There's nothing ObjC-specific about these tests, I don't think we should 
duplicate them between C and ObjC.

(Unlike C vs C++, it's very rare for plain-c code to have different semantics 
in ObjC)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110954

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

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

[James King via Phabricator via cfe-commits]

jcking1034 updated this revision to Diff 376610.
jcking1034 added a comment.

Clean up unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

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

Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -199,5 +199,34 @@
   EXPECT_FALSE(Node < Node);
 }
 
+TEST(DynTypedNode, TypeLoc) {
+  std::string code = R"cc(void example() { int abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto matches =
+  match(traverse(TK_AsIs,
+ varDecl(hasName("abc"), hasTypeLoc(typeLoc().bind("tl",
+AST->getASTContext());
+  EXPECT_EQ(matches.size(), 1u);
+  const auto  = *matches[0].getNodeAs("tl");
+  DynTypedNode Node = DynTypedNode::create(tl);
+  EXPECT_TRUE(Node == Node);
+  EXPECT_FALSE(Node < Node);
+}
+
+TEST(DynTypedNode, PointerTypeLoc) {
+  std::string code = R"cc(void example() { int* abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto matches =
+  match(traverse(TK_AsIs, varDecl(hasName("abc"),
+  hasTypeLoc(typeLoc().bind("ptl",
+AST->getASTContext());
+  EXPECT_EQ(matches.size(), 1u);
+  const auto ptl =
+  matches[0].getNodeAs("ptl")->castAs();
+  DynTypedNode Node = DynTypedNode::create(ptl);
+  EXPECT_TRUE(Node == Node);
+  EXPECT_FALSE(Node < Node);
+}
+
 } // namespace
 }  // namespace clang
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/TypeLoc.h"
 
 using namespace clang;
 
@@ -28,6 +29,8 @@
 {NKI_None, "TemplateName"},
 {NKI_None, "NestedNameSpecifierLoc"},
 {NKI_None, "QualType"},
+#define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"},
+#include "clang/AST/TypeLocNodes.def"
 {NKI_None, "TypeLoc"},
 {NKI_None, "CXXBaseSpecifier"},
 {NKI_None, "CXXCtorInitializer"},
@@ -127,6 +130,17 @@
   llvm_unreachable("invalid type kind");
  }
 
+ ASTNodeKind ASTNodeKind::getFromNode(const TypeLoc ) {
+   switch (T.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  case TypeLoc::CLASS: \
+return ASTNodeKind(NKI_##CLASS##TypeLoc);
+#include "clang/AST/TypeLocNodes.def"
+   }
+   llvm_unreachable("invalid typeloc kind");
+ }
+
 ASTNodeKind ASTNodeKind::getFromNode(const OMPClause ) {
   switch (C.getClauseKind()) {
 #define GEN_CLANG_CLAUSE_CLASS
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -63,6 +63,7 @@
   static ASTNodeKind getFromNode(const Decl );
   static ASTNodeKind getFromNode(const Stmt );
   static ASTNodeKind getFromNode(const Type );
+  static ASTNodeKind getFromNode(const TypeLoc );
   static ASTNodeKind getFromNode(const OMPClause );
   static ASTNodeKind getFromNode(const Attr );
   /// \}
@@ -133,6 +134,8 @@
 NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
+#define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc,
+#include "clang/AST/TypeLocNodes.def"
 NKI_TypeLoc,
 NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc,
 NKI_CXXBaseSpecifier,
@@ -198,6 +201,8 @@
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
+#define TYPELOC(CLASS, PARENT) KIND_TO_KIND_ID(CLASS##TypeLoc)
+#include "clang/AST/TypeLocNodes.def"
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
@@ -304,7 +309,7 @@
   return getUnchecked().getAsOpaquePtr() <
  Other.getUnchecked().getAsOpaquePtr();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind)) {
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind)) {
   auto TLA = getUnchecked();
   auto TLB = Other.getUnchecked();
   return std::make_pair(TLA.getType().getAsOpaquePtr(),
@@ -336,7 +341,7 @@
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
@@ -365,7 +370,7 @@
 }
 static unsigned getHashValue(const 

[PATCH] D110955: [AIX] Don't pass namedsects in LTO mode

[Jinsong Ji via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c31969e8df2: [AIX] Dont pass namedsects in LTO mode 
(authored by jsji).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110955

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,58 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-NON-LTO-NOT: "-lc++"
+// CHECK-PGO-NON-LTO-NOT: "-lc++abi"
+// CHECK-PGO-NON-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-NON-LTO-NOT: "--as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lunwind"
+// CHECK-PGO-NON-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lm"
+// CHECK-PGO-NON-LTO: "-lc"
+
+// Check powerpc-ibm-aix7.2.5.3, -fprofile-generate, -flto
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-flto\
+// RUN:-target powerpc-ibm-aix7.2.5.3 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-LTO %s
+// CHECK-PGO-LTO-NOT: warning:
+// CHECK-PGO-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.2.5.3"
+// CHECK-PGO-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-LTO-NOT: "-bdbg:namedsects"
+// CHECK-PGO-LTO: "-b32"
+// CHECK-PGO-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-LTO-NOT: "-lc++"
+// CHECK-PGO-LTO-NOT: "-lc++abi"
+// CHECK-PGO-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-LTO-NOT: "--as-needed"
+// CHECK-PGO-LTO-NOT: "-lunwind"
+// CHECK-PGO-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-LTO-NOT: "-lm"
+// CHECK-PGO-LTO: "-lc"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -98,8 +98,9 @@
 CmdArgs.push_back("-bnoentry");
   }
 
-  // Specify PGO linker option
-  if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
+  // Specify PGO linker option without LTO
+  if (!D.isUsingLTO() &&
+  (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
 options::OPT_fno_profile_generate, false) ||


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,58 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: 

[clang] 9c31969 - [AIX] Don't pass namedsects in LTO mode

[Jinsong Ji via cfe-commits]

Author: Jinsong Ji
Date: 2021-10-01T19:22:40Z
New Revision: 9c31969e8df2f4b41b05e415fc9a66ff0bfa0802

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

LOG: [AIX] Don't pass namedsects in LTO mode

LTO don't need binder option , don't pass it in LTO mode.

Reviewed By: Whitney

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AIX.cpp
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index 053f6389dc0e..e4bbf498b9cd 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -98,8 +98,9 @@ void aix::Linker::ConstructJob(Compilation , const 
JobAction ,
 CmdArgs.push_back("-bnoentry");
   }
 
-  // Specify PGO linker option
-  if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
+  // Specify PGO linker option without LTO
+  if (!D.isUsingLTO() &&
+  (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
 options::OPT_fno_profile_generate, false) ||

diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index de70c2f03046..fc40e50d4509 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -649,3 +649,58 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-NON-LTO-NOT: "-lc++"
+// CHECK-PGO-NON-LTO-NOT: "-lc++abi"
+// CHECK-PGO-NON-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-NON-LTO-NOT: "--as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lunwind"
+// CHECK-PGO-NON-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lm"
+// CHECK-PGO-NON-LTO: "-lc"
+
+// Check powerpc-ibm-aix7.2.5.3, -fprofile-generate, -flto
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-flto\
+// RUN:-target powerpc-ibm-aix7.2.5.3 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-LTO %s
+// CHECK-PGO-LTO-NOT: warning:
+// CHECK-PGO-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.2.5.3"
+// CHECK-PGO-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-LTO-NOT: "-bdbg:namedsects"
+// CHECK-PGO-LTO: "-b32"
+// CHECK-PGO-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-LTO-NOT: "-lc++"
+// CHECK-PGO-LTO-NOT: "-lc++abi"
+// CHECK-PGO-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-LTO-NOT: "--as-needed"
+// CHECK-PGO-LTO-NOT: "-lunwind"
+// CHECK-PGO-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-LTO-NOT: "-lm"
+// CHECK-PGO-LTO: "-lc"



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

[PATCH] D110955: [AIX] Don't pass namedsects in LTO mode

[Jinsong Ji via Phabricator via cfe-commits]

jsji updated this revision to Diff 376595.
jsji added a comment.

Update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110955

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,58 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-NON-LTO-NOT: "-lc++"
+// CHECK-PGO-NON-LTO-NOT: "-lc++abi"
+// CHECK-PGO-NON-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-NON-LTO-NOT: "--as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lunwind"
+// CHECK-PGO-NON-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lm"
+// CHECK-PGO-NON-LTO: "-lc"
+
+// Check powerpc-ibm-aix7.2.5.3, -fprofile-generate, -flto
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-flto\
+// RUN:-target powerpc-ibm-aix7.2.5.3 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-LTO %s
+// CHECK-PGO-LTO-NOT: warning:
+// CHECK-PGO-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.2.5.3"
+// CHECK-PGO-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-LTO-NOT: "-bdbg:namedsects"
+// CHECK-PGO-LTO: "-b32"
+// CHECK-PGO-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-LTO-NOT: "-lc++"
+// CHECK-PGO-LTO-NOT: "-lc++abi"
+// CHECK-PGO-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-LTO-NOT: "--as-needed"
+// CHECK-PGO-LTO-NOT: "-lunwind"
+// CHECK-PGO-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-LTO-NOT: "-lm"
+// CHECK-PGO-LTO: "-lc"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -98,8 +98,9 @@
 CmdArgs.push_back("-bnoentry");
   }
 
-  // Specify PGO linker option
-  if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
+  // Specify PGO linker option without LTO
+  if (!D.isUsingLTO() &&
+  (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
 options::OPT_fno_profile_generate, false) ||


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,58 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" 

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D110927#3036647 , @ASDenysPetrov 
wrote:

> In D110927#3036436 , @steakhal 
> wrote:
>
>> I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in 
>> your summary.
>> I think it's allowed by the standard to access any valid object via a 
>> `char*` - according to the strict aliasing rule.
>> @shafik WDYT?
>
> As I found we can legaly treat `char*` as the object of type `char` but not 
> as an array of objects. This is mentioned in 
> http://eel.is/c++draft/basic.compound#3.4 //For purposes of pointer 
> arithmetic ... an object of type T that is not an array element is considered 
> to belong to an array with one element of type T.// That means that we can 
> get only the first element of `char*`, otherwise it would be an UB. There is 
> also a paper to overcome this constraint 
> http://open-std.org/JTC1/SC22/WG21/docs/papers/2019/p1839r0.pdf
>
> @aaron.ballman I would like you join the discussion, as we have similar one 
> in D104285 .

IIUC the object is `const int arr[42]` and the `(char *)arr` is an expression 
of pointer type and adding `1` to this is valid. The case you refer to in 
D104285  ended up being a pointer to an array 
of 2 ints and therefore accessing the third element was out of bounds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110927

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

[PATCH] D110934: [NFC] Update return type of vec_popcnt to vector unsigned.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

I think the commit message needs to be updated.

> This patch updates the vec_popcnt builtins to take a signed int as the second 
> parameter...

Should be: This patch updates the return type of the vec_popcnt builtins to 
vector unsigned...

Other than that, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110934

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

[PATCH] D110955: [AIX] Don't pass namedsects in LTO mode

[Jinsong Ji via Phabricator via cfe-commits]

jsji created this revision.
jsji added reviewers: PowerPC, Whitney.
Herald added a subscriber: inglorion.
jsji requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

LTO don't need binder option , don't pass it in LTO mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110955

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,60 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-NON-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-NON-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-NON-LTO: "-bdbg:namedsects"
+// CHECK-PGO-NON-LTO: "-b32"
+// CHECK-PGO-NON-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-NON-LTO: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-PGO-NON-LTO-NOT: "-lc++"
+// CHECK-PGO-NON-LTO-NOT: "-lc++abi"
+// CHECK-PGO-NON-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-NON-LTO-NOT: "--as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lunwind"
+// CHECK-PGO-NON-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-NON-LTO-NOT: "-lm"
+// CHECK-PGO-NON-LTO: "-lc"
+
+// Check powerpc-ibm-aix7.2.5.3, -fprofile-generate, -flto
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-flto\
+// RUN:-target powerpc-ibm-aix7.2.5.3 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-LTO %s
+// CHECK-PGO-LTO-NOT: warning:
+// CHECK-PGO-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.2.5.3"
+// CHECK-PGO-LTO: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-PGO-LTO: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-PGO-LTO: "{{.*}}ld{{(.exe)?}}"
+// CHECK-PGO-LTO-NOT: "-bdbg:namedsects"
+// CHECK-PGO-LTO: "-b32"
+// CHECK-PGO-LTO: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-PGO-LTO: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-PGO-LTO-NOT: "-lc++"
+// CHECK-PGO-LTO-NOT: "-lc++abi"
+// CHECK-PGO-LTO: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-PGO-LTO-NOT: "--as-needed"
+// CHECK-PGO-LTO-NOT: "-lunwind"
+// CHECK-PGO-LTO-NOT: "--no-as-needed"
+// CHECK-PGO-LTO-NOT: "-lm"
+// CHECK-PGO-LTO: "-lc"
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -98,8 +98,9 @@
 CmdArgs.push_back("-bnoentry");
   }
 
-  // Specify PGO linker option
-  if ((Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
+  // Specify PGO linker option without LTO
+  if (!D.isUsingLTO() &&
+  (Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
 false) ||
Args.hasFlag(options::OPT_fprofile_generate,
 options::OPT_fno_profile_generate, false) ||


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -649,3 +649,60 @@
 // CHECK-LD64-SHARED-NOT: "--no-as-needed"
 // CHECK-LD64-SHARED: "-lm"
 // CHECK-LD64-SHARED: "-lc"
+
+// Check powerpc-ibm-aix7.3.0.0, -fprofile-generate
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-static \
+// RUN:-fprofile-generate\
+// RUN:-target powerpc-ibm-aix7.3.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-PGO-NON-LTO %s
+// CHECK-PGO-NON-LTO-NOT: warning:
+// CHECK-PGO-NON-LTO: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.3.0.0"
+// CHECK-PGO-NON-LTO: "-resource-dir" 

[PATCH] D110954: [clangd] Improve PopulateSwitch tweak

[David Goldman via Phabricator via cfe-commits]

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

- Support enums in C and ObjC as their AST representations differ slightly.

- Add support for typedef'ed enums.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110954

Files:
  clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
  clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/PopulateSwitchTests.cpp
@@ -23,6 +23,7 @@
 CodeContext Context;
 llvm::StringRef TestSource;
 llvm::StringRef ExpectedSource;
+llvm::StringRef FileName = "TestTU.cpp";
   };
 
   Case Cases[]{
@@ -206,10 +207,56 @@
   R""(template void f() {enum Enum {A}; ^switch (A) {}})"",
   "unavailable",
   },
+  {// C: Only filling in missing enumerators
+   Function,
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+^switch (val) {case B:break;}
+  )"",
+   R""(
+enum CEnum {A,B,C};
+enum CEnum val = A;
+switch (val) {case B:break;case A:case C:break;}
+  )"",
+   "TestTU.c"},
+  {// ObjC: Only filling in missing enumerators
+   Function,
+   R""(
+enum ObjCEnum {A,B,C};
+enum ObjCEnum val = B;
+^switch (val) {case A:break;}
+  )"",
+   R""(
+enum ObjCEnum {A,B,C};
+enum ObjCEnum val = B;
+switch (val) {case A:break;case B:case C:break;}
+  )"",
+   "TestTU.m"},
+  {// ObjC: Only filling in missing enumerators w/ typedefs
+   Function,
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (^controlState) {case A:break;}
+  )"",
+   R""(
+typedef unsigned long UInteger;
+enum ControlState : UInteger;
+typedef enum ControlState ControlState;
+enum ControlState : UInteger {A,B,C};
+ControlState controlState = A;
+switch (controlState) {case A:break;case B:case C:break;}
+  )"",
+   "TestTU.m"},
   };
 
   for (const auto  : Cases) {
 Context = Case.Context;
+FileName = Case.FileName;
 EXPECT_EQ(apply(Case.TestSource), Case.ExpectedSource);
   }
 }
Index: clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -113,7 +113,8 @@
 return false;
   // Ignore implicit casts, since enums implicitly cast to integer types.
   Cond = Cond->IgnoreParenImpCasts();
-  EnumT = Cond->getType()->getAsAdjusted();
+  // Get the canonical type to handle typedefs.
+  EnumT = Cond->getType().getCanonicalType()->getAsAdjusted();
   if (!EnumT)
 return false;
   EnumD = EnumT->getDecl();
@@ -152,14 +153,30 @@
 if (CS->caseStmtIsGNURange())
   return false;
 
+// Support for direct references to enum constants. This is required to
+// support C and ObjC which don't contain values in their ConstantExprs.
+// The general way to get the value of a case is EvaluateAsRValue, but we'd
+// rather not deal with that in case the AST is broken.
+if (auto *DRE = dyn_cast(CS->getLHS()->IgnoreParenCasts())) {
+  if (auto *Enumerator = dyn_cast(DRE->getDecl())) {
+auto Iter = ExpectedCases.find(Normalize(Enumerator->getInitVal()));
+if (Iter != ExpectedCases.end())
+  Iter->second.setCovered();
+continue;
+  }
+}
+
+// ConstantExprs with values are expected for C++, otherwise the storage
+// kind will be None.
+
 // Case expression is not a constant expression or is value-dependent,
 // so we may not be able to work out which cases are covered.
 const ConstantExpr *CE = dyn_cast(CS->getLHS());
 if (!CE || CE->isValueDependent())
   return false;
 
-// Unsure if this case could ever come up, but prevents an unreachable
-// executing in getResultAsAPSInt.
+// We need a storage kind in order to be able to fetch the value type,
+// currently both C and ObjC enums will return none.
 if (CE->getResultStorageKind() == ConstantExpr::RSK_None)
   return false;
 auto Iter = 

[PATCH] D110935: [NFC] Update vec_extract builtin signatures to take signed int.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110935

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult ) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

mtrofin wrote:
> aeubanks wrote:
> > can we add a test for these?
> I think that would be tricky, because they should not actually happen - the 
> way we determine whether a site is alwaysinlinable checks (but not 
> thoroughly) for legality. Let me see if I can find a regression test. It may 
> be we can synthesize such a case in IR only, though, so not much of a help 
> for the frontend tests?
yeah some IR tests is what I was thinking


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110665: [clang] Don't use the AST to display backend diagnostics

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110665

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

[PATCH] D77598: Integral template argument suffix and cast printing

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D77598#3035591 , @v.g.vassilev 
wrote:

> In D77598#3035449 , @dblaikie wrote:
>
>> Came across this while trying to do "simplified template names" - producing 
>> template names in DWARF without template parameter lists as part of the 
>> textual name, then rebuilding that name from the structural representation 
>> of template parameters in DWARF (DW_TAG_template_*_parameter, etc). The 
>> roundtripping is implemented to ensure that the simplified names are 
>> non-lossy - that all the data is still available through the structural 
>> representation. (some names are harder or not currently possible to rebuild)
>>
>> The selective use of suffixes, especially contextually (overloading) seems 
>> like something I'll probably want to avoid entirely for DWARF to keep the 
>> names consistent across different contexts - but I am also just a bit 
>> confused about some things I'm seeing with this change.
>>
>> For instance, it looks like 
>> https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/Decl.cpp#L2900
>>  doesn't pass the list of template parameters, so function template names 
>> always get suffixes on their integer parameters.
>>
>> Whereas the equivalent calls for printing class template specialization 
>> names here: 
>> https://github.com/llvm/llvm-project/blob/fcdefc8575866a36e80e91024b876937ae6a9965/clang/lib/AST/DeclTemplate.cpp#L949-L959
>>  - is that just a minor bug/missing functionality?
>>
>> I'm testing a fix for that:
>>
>>   diff --git clang/lib/AST/Decl.cpp clang/lib/AST/Decl.cpp
>>   index 60ca8633224b..11cf068d2c4c 100644
>>   --- clang/lib/AST/Decl.cpp
>>   +++ clang/lib/AST/Decl.cpp
>>   @@ -2897,7 +2897,7 @@ void FunctionDecl::getNameForDiagnostic(
>>  NamedDecl::getNameForDiagnostic(OS, Policy, Qualified);
>>  const TemplateArgumentList *TemplateArgs = 
>> getTemplateSpecializationArgs();
>>  if (TemplateArgs)
>>   -printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy);
>>   +printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy, 
>> getPrimaryTemplate()->getTemplateParameters());
>>}
>>
>>bool FunctionDecl::isVariadic() const {
>>
>> I've sent out a patch with ^ applied/cleanups applied: D77598 
>> 
>
> This seems an oversight on our end, thanks! You probably meant to link 
> https://reviews.llvm.org/D110898

Yep, thanks for the correction!

Do you have any interest/bandwidth to look into the one with nested namespaces 
(the other code I pointed to, in NestedNameSpecifier.cpp, above) case? At a 
quick glance I wasn't able to quite figure out how to get the template 
parameters from the TemplateSpecializationType or 
DependentTemplateSpecializationType to pass in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3;>P1073R3
-  No
+  Partial
 

cor3ntin wrote:
> erichkeane wrote:
> > I'm trying to evaluate our consteval support, and I am having trouble 
> > finding any unsuperceded part of P1073R3 that is not implemented in 
> > Clang14.  Can you share what Delta you know of that has you considering 
> > this as 'partial'?  Is it something in a different patch awaiting review?
> This thing https://reviews.llvm.org/D74130 is what I believe the last bit of 
> missing functionality.
> Note that I only implemented P1937 myself
Ah, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3;>P1073R3
-  No
+  Partial
 

erichkeane wrote:
> I'm trying to evaluate our consteval support, and I am having trouble finding 
> any unsuperceded part of P1073R3 that is not implemented in Clang14.  Can you 
> share what Delta you know of that has you considering this as 'partial'?  Is 
> it something in a different patch awaiting review?
This thing https://reviews.llvm.org/D74130 is what I believe the last bit of 
missing functionality.
Note that I only implemented P1937 myself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D110428: [AIX] Define WCHAR_T_TYPE as unsigned short on AIX for wchar.c test case.

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

This doesn't appear to be true for 64-bit AIX:

  extern "C" int printf(const char *, ...);
  int main() {
  printf("wchar_t: %ld\nunsigned short: 
%ld\n",sizeof(wchar_t),sizeof(unsigned short));
  return 0;
  }
  $ clang++ -m64 foo.cc
  $ ./a.out
  wchar_t: 4
  unsigned short: 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110428

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

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 376559.
gandhi21299 added a comment.

- Since callees may alias to a function pointer, it makes sense for 
`getCalleeFunction(...)` to return a `Function` which is a cast of the operand 
of a `GlobalAlias`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
  llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/inline-calls.ll

Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/SIISelLowering.cpp
===
--- llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -3007,6 +3007,7 @@
   bool IsSibCall = false;
   bool IsThisReturn = false;
   MachineFunction  = DAG.getMachineFunction();
+  GlobalAddressSDNode *GSD = dyn_cast(Callee);
 
   if (Callee.isUndef() || isNullConstant(Callee)) {
 if (!CLI.IsTailCall) {
@@ -3264,7 +3265,7 @@
   Ops.push_back(Callee);
   // Add a redundant copy of the callee global which will not be legalized, as
   // we need direct access to the callee later.
-  if (GlobalAddressSDNode *GSD = dyn_cast(Callee)) {
+  if (GSD) {
 const GlobalValue *GV = GSD->getGlobal();
 Ops.push_back(DAG.getTargetGlobalAddress(GV, DL, MVT::i64));
   } else {
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/GlobalAlias.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
@@ -61,7 +63,8 @@
 assert(Op.getImm() == 0);
 return nullptr;
   }
-
+  if (auto *GA = dyn_cast(Op.getGlobal()))
+return cast(GA->getOperand(0));
   return cast(Op.getGlobal());
 }
 
Index: llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
@@ -913,14 +913,17 @@
   if (Info.Callee.isReg()) {
 CallInst.addReg(Info.Callee.getReg());
 CallInst.addImm(0);
-  } else if (Info.Callee.isGlobal() && Info.Callee.getOffset() == 0) {
-// The call lowering lightly assumed we can directly encode a call target in
-// the instruction, which is not the case. Materialize the address here.
+  } else if (Info.Callee.isGlobal()) {
 const GlobalValue *GV = Info.Callee.getGlobal();
-auto Ptr = MIRBuilder.buildGlobalValue(
-  LLT::pointer(GV->getAddressSpace(), 64), GV);
-CallInst.addReg(Ptr.getReg(0));
-CallInst.add(Info.Callee);
+if (Info.Callee.getOffset() == 0) {
+  // The call lowering lightly assumed we can directly encode a call target
+  // in the instruction, which is not the case. Materialize the address
+  // here.
+  auto Ptr = MIRBuilder.buildGlobalValue(
+  LLT::pointer(GV->getAddressSpace(), 64), GV);
+  CallInst.addReg(Ptr.getReg(0));
+  CallInst.add(Info.Callee);
+}
   } else
 return false;
 
Index: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
@@ -93,6 +93,8 @@
 
   for (GlobalAlias  : M.aliases()) {
 if (Function* F = dyn_cast(A.getAliasee())) {
+  if (A.getLinkage() != GlobalValue::InternalLinkage)
+continue;
   A.replaceAllUsesWith(F);
   AliasesToRemove.push_back();
 }
Index: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
@@ -0,0 +1,17 @@
+// 

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 376564.
gandhi21299 added a comment.

- eliminated changes in SIISelLowering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
  llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
  llvm/test/CodeGen/AMDGPU/inline-calls.ll


Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  
%s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/GlobalAlias.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
@@ -61,7 +63,8 @@
 assert(Op.getImm() == 0);
 return nullptr;
   }
-
+  if (auto *GA = dyn_cast(Op.getGlobal()))
+return cast(GA->getOperand(0));
   return cast(Op.getGlobal());
 }
 
Index: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
@@ -93,6 +93,8 @@
 
   for (GlobalAlias  : M.aliases()) {
 if (Function* F = dyn_cast(A.getAliasee())) {
+  if (A.getLinkage() != GlobalValue::InternalLinkage)
+continue;
   A.replaceAllUsesWith(F);
   AliasesToRemove.push_back();
 }
Index: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu
@@ -0,0 +1,17 @@
+// REQUIRES: amdgpu-registered-target, clang-driver
+
+// RUN: %clang --offload-arch=gfx906 --cuda-device-only -x hip -emit-llvm -S 
-o - %s \
+// RUN:   -fgpu-rdc -O3 -mllvm -amdgpu-early-inline-all=true -mllvm 
-amdgpu-function-calls=false | \
+// RUN:   FileCheck %s
+
+#include "Inputs/cuda.h"
+
+// CHECK: %struct.B = type { i8 }
+struct B {
+
+  // CHECK: @_ZN1BC1Ei = hidden unnamed_addr alias void (%struct.B*, i32), 
void (%struct.B*, i32)* @_ZN1BC2Ei
+  __device__ B(int x);
+};
+
+__device__ B::B(int x) {
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5102,9 +5102,9 @@
   }
 
   // Enable -mconstructor-aliases except on darwin, where we have to work 
around
-  // a linker bug (see ), and CUDA/AMDGPU device code,
-  // where aliases aren't supported.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())
+  // a linker bug (see ), and CUDA device code, where
+  // aliases aren't supported.
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we


Index: llvm/test/CodeGen/AMDGPU/inline-calls.ll
===
--- llvm/test/CodeGen/AMDGPU/inline-calls.ll
+++ llvm/test/CodeGen/AMDGPU/inline-calls.ll
@@ -1,6 +1,5 @@
 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck  %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck  %s
-; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s
 
 ; ALL-NOT: {{^}}func:
 define internal i32 @func(i32 %a) {
@@ -18,8 +17,8 @@
   ret void
 }
 
-; CHECK-NOT: func_alias
-; ALL-NOT: func_alias
+; CHECK: func_alias
+; ALL: func_alias
 @func_alias = alias i32 (i32), i32 (i32)* @func
 
 ; ALL: {{^}}kernel3:
Index: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -29,6 +29,8 @@
 #include "SIMachineFunctionInfo.h"
 #include "llvm/Analysis/CallGraph.h"
 

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

mtrofin wrote:
> wenlei wrote:
> > mtrofin wrote:
> > > wenlei wrote:
> > > > curious why do we need anonymous namespace here?
> > > iiuc it's preferred we place file-local types inside an anonymous 
> > > namespace. 
> > > 
> > > Looking now at the [[ 
> > > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> > > guideline ]], it touts their benefits but also says I should have only 
> > > placed de decl there and the impl of those members out... but the members 
> > > are quite trivial. Happy to move them out though.
> > Thanks for the pointer. I don't have a strong opinion but slightly leaning 
> > towards moving out of anonymous namespace be consistent with how other 
> > InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in 
> > anonymous namespace).
> Ah, those are public (i.e. in a .h file)
Generally if a type is declared/defined inside a .cpp file it should be in an 
anonymous namespace so it can't collide with other implementation type names in 
other .cpp files. (& .cpp-local functions should be static or in an anonymous 
namespace for the same reason) 

Looks like the other two (`DefaultInlineAdvice` and `MLInlineAdvice`) are 
defined in headers, so they must not be in anonymous namespaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin marked 3 inline comments as done.
mtrofin added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

wenlei wrote:
> mtrofin wrote:
> > wenlei wrote:
> > > curious why do we need anonymous namespace here?
> > iiuc it's preferred we place file-local types inside an anonymous 
> > namespace. 
> > 
> > Looking now at the [[ 
> > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> > guideline ]], it touts their benefits but also says I should have only 
> > placed de decl there and the impl of those members out... but the members 
> > are quite trivial. Happy to move them out though.
> Thanks for the pointer. I don't have a strong opinion but slightly leaning 
> towards moving out of anonymous namespace be consistent with how other 
> InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in 
> anonymous namespace).
Ah, those are public (i.e. in a .h file)



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult ) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

aeubanks wrote:
> can we add a test for these?
I think that would be tricky, because they should not actually happen - the way 
we determine whether a site is alwaysinlinable checks (but not thoroughly) for 
legality. Let me see if I can find a regression test. It may be we can 
synthesize such a case in IR only, though, so not much of a help for the 
frontend tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:207
+  auto matches = match(traverse(TK_AsIs, typeLoc().bind("tl")), context);
+  for (auto  : matches) {
+const auto  = *nodes.getNodeAs("tl");

let's assert that `matches.size() == 1` and then just use `nodes[0]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89
+  void recordUnsuccessfulInliningImpl(const InlineResult ) override {
+if (IsInliningRecommended)
+  ORE.emit([&]() {
+return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+   << "'" << NV("Callee", Callee) << "' is not AlwaysInline into '"
+   << NV("Caller", Caller)
+   << "': " << NV("Reason", Result.getFailureReason());

can we add a test for these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[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, thanks.




Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

mtrofin wrote:
> wenlei wrote:
> > curious why do we need anonymous namespace here?
> iiuc it's preferred we place file-local types inside an anonymous namespace. 
> 
> Looking now at the [[ 
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
> guideline ]], it touts their benefits but also says I should have only placed 
> de decl there and the impl of those members out... but the members are quite 
> trivial. Happy to move them out though.
Thanks for the pointer. I don't have a strong opinion but slightly leaning 
towards moving out of anonymous namespace be consistent with how other 
InlineAdvice is organized (DefaultInlineAdvice, MLInlineAdvice not in anonymous 
namespace).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D106302: Implement P1937 consteval in unevaluated contexts

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/www/cxx_status.html:1106
   https://wg21.link/p1073r3;>P1073R3
-  No
+  Partial
 

I'm trying to evaluate our consteval support, and I am having trouble finding 
any unsuperceded part of P1073R3 that is not implemented in Clang14.  Can you 
share what Delta you know of that has you considering this as 'partial'?  Is it 
something in a different patch awaiting review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106302

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Mircea Trofin via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

wenlei wrote:
> curious why do we need anonymous namespace here?
iiuc it's preferred we place file-local types inside an anonymous namespace. 

Looking now at the [[ 
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | style 
guideline ]], it touts their benefits but also says I should have only placed 
de decl there and the impl of those members out... but the members are quite 
trivial. Happy to move them out though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

[Wenlei He via Phabricator via cfe-commits]

wenlei added inline comments.



Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52
 
+namespace {
+using namespace llvm::ore;

curious why do we need anonymous namespace here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110891

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

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

[James King via Phabricator via cfe-commits]

jcking1034 updated this revision to Diff 376573.
jcking1034 marked an inline comment as done.
jcking1034 added a comment.

Fix typing for `create` and add unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110586

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

Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -199,5 +199,34 @@
   EXPECT_FALSE(Node < Node);
 }
 
+TEST(DynTypedNode, TypeLoc) {
+  std::string code = R"cc(void example() { int abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto  = AST->getASTContext();
+  auto matches = match(traverse(TK_AsIs, typeLoc().bind("tl")), context);
+  for (auto  : matches) {
+const auto  = *nodes.getNodeAs("tl");
+DynTypedNode Node = DynTypedNode::create(tl);
+EXPECT_TRUE(Node == Node);
+EXPECT_FALSE(Node < Node);
+  }
+}
+
+TEST(DynTypedNode, PointerTypeLoc) {
+  std::string code = R"cc(void example() { int* abc; })cc";
+  auto AST = clang::tooling::buildASTFromCode(code);
+  auto  = AST->getASTContext();
+  auto matches =
+  match(traverse(TK_AsIs, varDecl(hasName("abc"),
+  hasTypeLoc(typeLoc().bind("ptl",
+context);
+  for (auto  : matches) {
+const auto ptl = nodes.getNodeAs("ptl")->castAs();
+DynTypedNode Node = DynTypedNode::create(ptl);
+EXPECT_TRUE(Node == Node);
+EXPECT_FALSE(Node < Node);
+  }
+}
+
 } // namespace
 }  // namespace clang
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/TypeLoc.h"
 
 using namespace clang;
 
@@ -28,6 +29,8 @@
 {NKI_None, "TemplateName"},
 {NKI_None, "NestedNameSpecifierLoc"},
 {NKI_None, "QualType"},
+#define TYPELOC(CLASS, PARENT) {NKI_##PARENT, #CLASS "TypeLoc"},
+#include "clang/AST/TypeLocNodes.def"
 {NKI_None, "TypeLoc"},
 {NKI_None, "CXXBaseSpecifier"},
 {NKI_None, "CXXCtorInitializer"},
@@ -127,6 +130,17 @@
   llvm_unreachable("invalid type kind");
  }
 
+ ASTNodeKind ASTNodeKind::getFromNode(const TypeLoc ) {
+   switch (T.getTypeLocClass()) {
+#define ABSTRACT_TYPELOC(CLASS, PARENT)
+#define TYPELOC(CLASS, PARENT) \
+  case TypeLoc::CLASS: \
+return ASTNodeKind(NKI_##CLASS##TypeLoc);
+#include "clang/AST/TypeLocNodes.def"
+   }
+   llvm_unreachable("invalid typeloc kind");
+ }
+
 ASTNodeKind ASTNodeKind::getFromNode(const OMPClause ) {
   switch (C.getClauseKind()) {
 #define GEN_CLANG_CLAUSE_CLASS
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -63,6 +63,7 @@
   static ASTNodeKind getFromNode(const Decl );
   static ASTNodeKind getFromNode(const Stmt );
   static ASTNodeKind getFromNode(const Type );
+  static ASTNodeKind getFromNode(const TypeLoc );
   static ASTNodeKind getFromNode(const OMPClause );
   static ASTNodeKind getFromNode(const Attr );
   /// \}
@@ -133,6 +134,8 @@
 NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
+#define TYPELOC(CLASS, PARENT) NKI_##CLASS##TypeLoc,
+#include "clang/AST/TypeLocNodes.def"
 NKI_TypeLoc,
 NKI_LastKindWithoutPointerIdentity = NKI_TypeLoc,
 NKI_CXXBaseSpecifier,
@@ -198,6 +201,8 @@
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
+#define TYPELOC(CLASS, PARENT) KIND_TO_KIND_ID(CLASS##TypeLoc)
+#include "clang/AST/TypeLocNodes.def"
 KIND_TO_KIND_ID(TypeLoc)
 KIND_TO_KIND_ID(Decl)
 KIND_TO_KIND_ID(Stmt)
@@ -304,7 +309,7 @@
   return getUnchecked().getAsOpaquePtr() <
  Other.getUnchecked().getAsOpaquePtr();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind)) {
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind)) {
   auto TLA = getUnchecked();
   auto TLB = Other.getUnchecked();
   return std::make_pair(TLA.getType().getAsOpaquePtr(),
@@ -336,7 +341,7 @@
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
-if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
+if (ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind))
   return getUnchecked() == Other.getUnchecked();
 
 if (ASTNodeKind::getFromNodeKind().isSame(NodeKind))
@@ -365,7 +370,7 @@
 }
 static 

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

steakhal wrote:
> aaron.ballman wrote:
> > steakhal wrote:
> > > If it can be used only once, we should mark this function as an `r-value` 
> > > function.
> > > There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> > > 
> > > Later, when you would actually call this function, you need to 
> > > `std::move()` the object, signifying that the original object gets 
> > > destroyed in the process.
> > > 
> > > @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or 
> > > we can simply use the `&&` in the function declaration?
> > > I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros 
> > > in the past. What's the status on this?
> > The expected pattern is:
> > ```
> > #if LLVM_HAS_RVALUE_REFERENCE_THIS
> >   void func() && {
> >   }
> > #endif
> > ```
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
> >  (and elsewhere). However, I note that there are some unprotected uses 
> > (such as 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
> >  and so it may be a sign that we're fine to remove 
> > `LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
> > right thing?
> You tried to eliminate that on Jan 22, 2020 with 
> rGdfe9f130e07c929d21f8122272077495de531a38.
> But according to git blame, the BuildTree.cpp#L437 was committed on Jul 9, 
> 2019 with rG9b3f38f99085e2bbf9db01bb00d4c6d837f0fc00.
> I'm confused.
> I'm confused.

As am I. I went back and looked at the history here, and as best I can tell, I 
tried to get rid of lvalue functions, we threw it at bots, and for reasons I 
didn't capture anywhere I can find, had to revert it. Sorry for the troubles 
with not tracking that information! :-(

However, in my simple testing on godbolt, I can't find a version of MSVC that 
has issues with lvalue or rvalue reference overloads. We claim to still support 
MSVC 2017 (perhaps it's time to bump that to something more recent now that 
MSVC has changed its release schedule), so maybe there's an older version 
that's still an issue, but I would expect us to have spotted that given there 
are unprotected uses of rvalue ref overloads.

My gut feeling is that we should explore getting rid of `LLVM_LVALUE_FUNCTION` 
and `LLVM_HAS_RVALUE_REFERENCE_THIS` again as it's been almost two years since 
the last try. If there's a problematic version of MSVC, we might want to 
consider dropping support for it unless it persists in newer MSVC versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110670: [Sema] Allow comparisons between different ms ptr size address space types.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/MicrosoftExtensions.cpp:9-10
+  return (p32u == p32s) +
+ (p32u == p64) +
+ (p32s == p64);
+}

akhuang wrote:
> aaron.ballman wrote:
> > (Side question, not directly about this patch but sorta related.) Should 
> > there be a diagnostic about conversion potentially causing a possible loss 
> > of data (on the 64-bit target)?
> Hmm, it seems reasonable, but I also don't know how motivated I am to add a 
> diagnostic -- 
I don't insist -- we are missing that warning in general here (we don't warn on 
assignment yet). But it might be a nice follow-up for anyone who's interested 
in working on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110670

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

[PATCH] D109652: [PowerPC] Restrict various P10 options to P10 only.

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109652

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

[PATCH] D110428: [AIX] Define WCHAR_T_TYPE as unsigned short on AIX for wchar.c test case.

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110428

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

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@ASDenysPetrov This looks promising! Please address the nits which @steakhal 
found, than I think it is good to land.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641
+  // FIXME: Take array dimension into account to prevent exceeding its size.
+  const int64_t I = Idx.getExtValue();
+  uint32_t Code =

steakhal wrote:
> You could use the `uint64_t` type here, and spare the subsequent explicit 
> cast. This operation would be safe since `Idx` must be non-negative here.
+1 for using `uint64_t` if possible


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

https://reviews.llvm.org/D107339

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

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110927#3036436 , @steakhal wrote:

> I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in 
> your summary.
> I think it's allowed by the standard to access any valid object via a `char*` 
> - according to the strict aliasing rule.
> @shafik WDYT?

As I found we can legaly treat `char*` as the object of type `char` but not as 
an array of objects. This is mentioned in 
http://eel.is/c++draft/basic.compound#3.4 //For purposes of pointer arithmetic 
... an object of type T that is not an array element is considered to belong to 
an array with one element of type T.// That means that we can get only the 
first element of `char*`, otherwise it would be an UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110927

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3d0b5805e5f: [analyzer] Fix deprecated plistlib functions 
(authored by manas, committed by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

Files:
  clang/utils/analyzer/SATestBuild.py


Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:


Index: clang/utils/analyzer/SATestBuild.py
===
--- clang/utils/analyzer/SATestBuild.py
+++ clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a3d0b58 - [analyzer] Fix deprecated plistlib functions

[Balazs Benics via cfe-commits]

Author: Manas
Date: 2021-10-01T17:07:24+02:00
New Revision: a3d0b5805e5ff2fd870df5be5c3197eee0bb74a0

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

LOG: [analyzer] Fix deprecated plistlib functions

It replaces the usage of readPlist,writePlist functions with load,dump
in plistlib package.

This fixes deprecation issues when analyzer reports are being generated
outside of docker.

Patch by Manas!

Reviewed By: steakhal

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

Added: 


Modified: 
clang/utils/analyzer/SATestBuild.py

Removed: 




diff  --git a/clang/utils/analyzer/SATestBuild.py 
b/clang/utils/analyzer/SATestBuild.py
index 1977a8fc2aeff..cf02f26ef267b 100644
--- a/clang/utils/analyzer/SATestBuild.py
+++ b/clang/utils/analyzer/SATestBuild.py
@@ -856,7 +856,8 @@ def normalize_reference_results(directory: str, output_dir: 
str,
 continue
 
 plist = os.path.join(dir_path, filename)
-data = plistlib.readPlist(plist)
+with open(plist, "rb") as plist_file:
+data = plistlib.load(plist_file)
 path_prefix = directory
 
 if build_mode == 1:
@@ -875,7 +876,8 @@ def normalize_reference_results(directory: str, output_dir: 
str,
 if 'clang_version' in data:
 data.pop('clang_version')
 
-plistlib.writePlist(data, plist)
+with open(plist, "wb") as plist_file:
+plistlib.dump(data, plist_file)
 
 
 def get_build_log_path(output_dir: str) -> str:



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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Orlando Cazalet-Hyams via Phabricator via cfe-commits]

Orlando added a comment.

In D110783#3036403 , @thakis wrote:

>> Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 
>> 
>
> Your bot cycled green after that change: 
> https://lab.llvm.org/buildbot/#/builders/139/builds/10976

Thanks for the fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

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

steakhal added a comment.

I'm gonna get back to this on Monday.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1699-1701
+  ProgramStateRef OldState;
+  do {
+OldState = State;

IMO we should have a `llvm::Statistic` here, tracking the maximum iteration 
count to reach the fixed point and an average iteration count.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1732
+  if (!State)
+return false;
+}

I'd love to see a coverage report of the tests you add with this patch.



Comment at: clang/test/Analysis/expr-inspection-printState-eq-classes.c:11
 return;
-  if (b != 0)
+  if (a != c)
 return;

Why do you need to change this?



Comment at: 
clang/test/Analysis/symbol-simplification-fixpoint-iteration-unreachable-code.cpp:16
+return;
+  if (c + b != 0)
+return;

Is it important to have this instead of `b + c`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

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

steakhal added a comment.

It seems like it doesn't build with GCC 8.3.0 on a Debian system.
Could you investigate?

  
llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1029:13: 
error: explicit specialization in non-namespace scope 'class 
{anonymous}::SymbolicRangeInferrer'
 template <>
   ^
  
llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1030:77: 
error: template-id 'VisitBinaryOperator' in declaration of primary 
template
 RangeSet VisitBinaryOperator(RangeSet LHS, RangeSet RHS, QualType 
T) {
   ^

I think the issue is this: 
https://stackoverflow.com/questions/3052579/explicit-specialization-in-non-namespace-scope


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[libunwind] 532783f - [libunwind] Fix cfi_register for float registers.

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2021-10-01T16:51:51+02:00
New Revision: 532783f9e1e65c7bd48b1592d2376e9dd47c5a73

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

LOG: [libunwind] Fix cfi_register for float registers.

Fixes D110144.
registers.getFloatRegister is not const in ARM therefor can't be called here.

Reviewed By: mstorsjo, #libunwind

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

Added: 


Modified: 
libunwind/src/DwarfInstructions.hpp

Removed: 




diff  --git a/libunwind/src/DwarfInstructions.hpp 
b/libunwind/src/DwarfInstructions.hpp
index 53baf6a148f33..b58c51bb7a604 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -115,10 +115,12 @@ double DwarfInstructions::getSavedFloatRegister(
 return addressSpace.getDouble(
 evaluateExpression((pint_t)savedReg.value, addressSpace,
 registers, cfa));
-  case CFI_Parser::kRegisterInRegister:
-return registers.getFloatRegister((int)savedReg.value);
   case CFI_Parser::kRegisterUndefined:
 return 0.0;
+  case CFI_Parser::kRegisterInRegister:
+#ifndef _LIBUNWIND_TARGET_ARM
+return registers.getFloatRegister((int)savedReg.value);
+#endif
   case CFI_Parser::kRegisterIsExpression:
   case CFI_Parser::kRegisterUnused:
   case CFI_Parser::kRegisterOffsetFromCFA:



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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D107312#3036421 , @steakhal wrote:

> I'm not using this script. I'm assuming you run it and verified that it works.
> Thanks for cleaning this up.

I have run it. It is working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D106102#3036474 , @steakhal wrote:

> In D106102#3036220 , @manas wrote:
>
>> I do not have landing rights.
>
> Please add your name and email on whom behalf I should commit this patch. 
> Mine is `Balazs Benics`

It is `Manas `

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

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

steakhal added a comment.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 376526.
vabridgers added a comment.

Use canonical types for comparison


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/ptr-arith.c

Index: clang/test/Analysis/ptr-arith.c
===
--- clang/test/Analysis/ptr-arith.c
+++ clang/test/Analysis/ptr-arith.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.FixedAddr,alpha.core.PointerArithm,alpha.core.PointerSub,debug.ExprInspection -analyzer-store=region -Wno-pointer-to-int-cast -verify -triple i686-apple-darwin9 -Wno-tautological-pointer-compare -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(int);
+void clang_analyzer_dump(int);
 
 void f1() {
   int a[10];
@@ -330,3 +331,59 @@
   simd_float2 x = {0, 1};
   return x[1]; // no-warning
 }
+
+struct s {
+  int v;
+};
+
+// These three expressions should produce the same sym vals.
+void struct_pointer_canon(struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim(struct s **ps) {
+  struct s ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+typedef struct s T1;
+typedef struct s T2;
+void struct_pointer_canon_typedef(T1 *ps) {
+  T2 ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_bidim_typedef(T1 **ps) {
+  T2 ss = **ps;
+  clang_analyzer_eval(&(ps[0][0].v) == &((*ps)->v)); // expected-warning{{TRUE}}
+}
+
+void struct_pointer_canon_const(const struct s *ps) {
+  struct s ss = *ps;
+  clang_analyzer_dump((*ps).v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps[0].v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_dump(ps->v);
+  // expected-warning-re@-1{{reg_${{[[:digit:]]+}}}.v>}}
+  clang_analyzer_eval((*ps).v == ps[0].v); // expected-warning{{TRUE}}
+  clang_analyzer_eval((*ps).v == ps->v);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ps[0].v == ps->v);   // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -442,6 +442,26 @@
 
 SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
 SVal Base) {
+
+  // Special case, if index is 0, return the same type as if
+  // this was not an array dereference.
+  if (Offset.isZeroConstant()) {
+ASTContext  = StateMgr.getContext();
+QualType BT = Base.getType(this->Ctx);
+if (!BT.isNull() && !elementType.isNull()) {
+  if (!BT->getPointeeType().isNull()) {
+QualType CanonBT = BT->getPointeeType().isCanonical()
+   ? BT->getPointeeType()
+   : Ctx.getCanonicalType(BT->getPointeeType());
+QualType CanonEleTy = elementType.isCanonical()
+  ? elementType
+  : Ctx.getCanonicalType(elementType);
+if (CanonBT == CanonEleTy)
+  return Base;
+  }
+}
+  }
+
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

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

steakhal added a comment.

In D106102#3036220 , @manas wrote:

> I do not have landing rights.

Please add your name and email on whom behalf I should commit this patch. Mine 
is `Balazs Benics`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D110935: [NFC] Update vec_extract builtin signatures to take signed int.

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: PowerPC, nemanjai, stefanp.
amyk added projects: LLVM, PowerPC, clang.
amyk requested review of this revision.

This patch updates the vec_extract builtins to take a signed int as the second 
parameter, as defined by the Power Vector Intrinsics Programming Reference.
This patch is NFC and all existing tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110935

Files:
  clang/lib/Headers/altivec.h


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -13444,74 +13444,74 @@
 /* vec_extract */
 
 static __inline__ signed char __ATTRS_o_ai vec_extract(vector signed char __a,
-   unsigned int __b) {
+   signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai
-vec_extract(vector unsigned char __a, unsigned int __b) {
+vec_extract(vector unsigned char __a, signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai vec_extract(vector bool char __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ signed short __ATTRS_o_ai vec_extract(vector signed short 
__a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ unsigned short __ATTRS_o_ai
-vec_extract(vector unsigned short __a, unsigned int __b) {
+vec_extract(vector unsigned short __a, signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ unsigned short __ATTRS_o_ai vec_extract(vector bool short 
__a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x7];
 }
 
 static __inline__ signed int __ATTRS_o_ai vec_extract(vector signed int __a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x3];
 }
 
 static __inline__ unsigned int __ATTRS_o_ai vec_extract(vector unsigned int 
__a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x3];
 }
 
 static __inline__ unsigned int __ATTRS_o_ai vec_extract(vector bool int __a,
-unsigned int __b) {
+signed int __b) {
   return __a[__b & 0x3];
 }
 
 #ifdef __VSX__
 static __inline__ signed long long __ATTRS_o_ai
-vec_extract(vector signed long long __a, unsigned int __b) {
+vec_extract(vector signed long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ unsigned long long __ATTRS_o_ai
-vec_extract(vector unsigned long long __a, unsigned int __b) {
+vec_extract(vector unsigned long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ unsigned long long __ATTRS_o_ai
-vec_extract(vector bool long long __a, unsigned int __b) {
+vec_extract(vector bool long long __a, signed int __b) {
   return __a[__b & 0x1];
 }
 
 static __inline__ double __ATTRS_o_ai vec_extract(vector double __a,
-  unsigned int __b) {
+  signed int __b) {
   return __a[__b & 0x1];
 }
 #endif
 
 static __inline__ float __ATTRS_o_ai vec_extract(vector float __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0x3];
 }
 


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -13444,74 +13444,74 @@
 /* vec_extract */
 
 static __inline__ signed char __ATTRS_o_ai vec_extract(vector signed char __a,
-   unsigned int __b) {
+   signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai
-vec_extract(vector unsigned char __a, unsigned int __b) {
+vec_extract(vector unsigned char __a, signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ unsigned char __ATTRS_o_ai vec_extract(vector bool char __a,
- unsigned int __b) {
+ signed int __b) {
   return __a[__b & 0xf];
 }
 
 static __inline__ 

[PATCH] D110934: [NFC] Update return type of vec_popcnt to vector unsigned.

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: PowerPC, nemanjai, stefanp.
amyk added projects: LLVM, PowerPC, clang.
amyk requested review of this revision.

This patch updates the vec_popcnt builtins to take a signed int as the second 
parameter, as defined by the Power Vector Intrinsics Programming Reference.
This patch is NFC and all existing tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110934

Files:
  clang/lib/Headers/altivec.h


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -2482,7 +2482,7 @@
 #ifdef __POWER8_VECTOR__
 /* vec_popcnt */
 
-static __inline__ vector signed char __ATTRS_o_ai
+static __inline__ vector unsigned char __ATTRS_o_ai
 vec_popcnt(vector signed char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
@@ -2490,7 +2490,7 @@
 vec_popcnt(vector unsigned char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
-static __inline__ vector signed short __ATTRS_o_ai
+static __inline__ vector unsigned short __ATTRS_o_ai
 vec_popcnt(vector signed short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
@@ -2498,7 +2498,7 @@
 vec_popcnt(vector unsigned short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
-static __inline__ vector signed int __ATTRS_o_ai
+static __inline__ vector unsigned int __ATTRS_o_ai
 vec_popcnt(vector signed int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
@@ -2506,7 +2506,7 @@
 vec_popcnt(vector unsigned int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
-static __inline__ vector signed long long __ATTRS_o_ai
+static __inline__ vector unsigned long long __ATTRS_o_ai
 vec_popcnt(vector signed long long __a) {
   return __builtin_altivec_vpopcntd(__a);
 }


Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -2482,7 +2482,7 @@
 #ifdef __POWER8_VECTOR__
 /* vec_popcnt */
 
-static __inline__ vector signed char __ATTRS_o_ai
+static __inline__ vector unsigned char __ATTRS_o_ai
 vec_popcnt(vector signed char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
@@ -2490,7 +2490,7 @@
 vec_popcnt(vector unsigned char __a) {
   return __builtin_altivec_vpopcntb(__a);
 }
-static __inline__ vector signed short __ATTRS_o_ai
+static __inline__ vector unsigned short __ATTRS_o_ai
 vec_popcnt(vector signed short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
@@ -2498,7 +2498,7 @@
 vec_popcnt(vector unsigned short __a) {
   return __builtin_altivec_vpopcnth(__a);
 }
-static __inline__ vector signed int __ATTRS_o_ai
+static __inline__ vector unsigned int __ATTRS_o_ai
 vec_popcnt(vector signed int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
@@ -2506,7 +2506,7 @@
 vec_popcnt(vector unsigned int __a) {
   return __builtin_altivec_vpopcntw(__a);
 }
-static __inline__ vector signed long long __ATTRS_o_ai
+static __inline__ vector unsigned long long __ATTRS_o_ai
 vec_popcnt(vector signed long long __a) {
   return __builtin_altivec_vpopcntd(__a);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

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

steakhal added a subscriber: shafik.
steakhal added a comment.

I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in 
your summary.
I think it's allowed by the standard to access any valid object via a `char*` - 
according to the strict aliasing rule.
@shafik WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110927

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

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

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

I'm not using this script. I'm assuming you run it and verified that it works.
Thanks for cleaning this up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2108
+LLVM_NODISCARD ProgramStateRef
+EquivalenceClass::removeMember(ProgramStateRef State, const SymbolRef Old) {
+

Remove `const `



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2123
+  ClsMembers = F.remove(ClsMembers, Old);
+  assert(!ClsMembers.isEmpty() &&
+ "Class should have had at least two members before member removal");

Comment that this is a precondition.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2156
+  assert(find(State, MemberSym) == find(State, SimplifiedMemberSym));
+  // Remove the old and more complex symbol.
+  State = find(State, MemberSym).removeMember(State, MemberSym);

TODO add Performance and complexity essay here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106823

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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 
> 

Your bot cycled green after that change: 
https://lab.llvm.org/buildbot/#/builders/139/builds/10976


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[PATCH] D110783: [clang] Make crash reproducer work with clang-cl

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D110783#3035158 , @dyung wrote:

> Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot 
> after your change. Can you take a look?
>
> https://lab.llvm.org/buildbot/#/builders/139/builds/10939

Hopefully better after ec4a82286674c44c9216e9585235b0fa5df4ae9f 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110783

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

[clang] ec4a822 - [clang] Try to unbreak crash-report.cpp on PS4 bot after 8dfbe9b0a

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-10-01T09:33:13-04:00
New Revision: ec4a82286674c44c9216e9585235b0fa5df4ae9f

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

LOG: [clang] Try to unbreak crash-report.cpp on PS4 bot after 8dfbe9b0a

Looks like exceptions are off-by-default with the PS4 triple.
Since adding -fexceptions defeats the purpose of the test change
in 8dfbe9b0a, pass an explicit triple instead.

Added: 


Modified: 
clang/test/Driver/crash-report.cpp

Removed: 




diff  --git a/clang/test/Driver/crash-report.cpp 
b/clang/test/Driver/crash-report.cpp
index 293bc1f0ad9f0..7da94885080be 100644
--- a/clang/test/Driver/crash-report.cpp
+++ b/clang/test/Driver/crash-report.cpp
@@ -7,7 +7,7 @@
 // RUN:  -Xclang -internal-isystem -Xclang /tmp/ \
 // RUN:  -Xclang -internal-externc-isystem -Xclang /tmp/ \
 // RUN:  -Xclang -main-file-name -Xclang foo.cpp \
-// RUN:  -DFOO=BAR -DBAR="BAZ QUX"' > %t.rsp
+// RUN:  -DFOO=BAR -DBAR="BAZ QUX"' --target=x86_64-linux-gnu > %t.rsp
 
 // RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1  \
 // RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \



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

[clang] 369d785 - [PowerPC] Optimal sequence for doubleword vec_all_{eq|ne} on Power7

[Nemanja Ivanovic via cfe-commits]

Author: Nemanja Ivanovic
Date: 2021-10-01T08:27:15-05:00
New Revision: 369d785574f5a22c086d0c40268a39a64bdd7217

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

LOG: [PowerPC] Optimal sequence for doubleword vec_all_{eq|ne} on Power7

These builtins produce inefficient code for CPU's prior to Power8
due to vcmpequd being unavailable. The predicate forms can actually
leverage the available vcmpequw along with xxlxor to produce a better
sequence.

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-vsx.c

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 6a179d86d71f9..5da4fbf72ce97 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -14815,42 +14815,43 @@ static __inline__ int __ATTRS_o_ai vec_all_eq(vector 
bool int __a,
 #ifdef __VSX__
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector signed long long __a,
   vector signed long long __b) {
+#ifdef __POWER8_VECTOR__
   return __builtin_altivec_vcmpequd_p(__CR6_LT, __a, __b);
+#else
+  // No vcmpequd on Power7 so we xor the two vectors and compare against zero 
as
+  // 32-bit elements.
+  return vec_all_eq((vector signed int)vec_xor(__a, __b), (vector signed 
int)0);
+#endif
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, __a, (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector unsigned long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector unsigned long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_all_eq(vector bool long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT, (vector long long)__a,
-  (vector long long)__b);
+  return vec_all_eq((vector signed long long)__a, (vector signed long 
long)__b);
 }
 #endif
 
@@ -17038,43 +17039,43 @@ static __inline__ int __ATTRS_o_ai vec_any_ne(vector 
bool int __a,
 #ifdef __VSX__
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector signed long long __a,
   vector signed long long __b) {
+#ifdef __POWER8_VECTOR__
   return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, __a, __b);
+#else
+  // Take advantage of the optimized sequence for vec_all_eq when vcmpequd is
+  // not available.
+  return !vec_all_eq(__a, __b);
+#endif
 }
 
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector unsigned long long __a,
   vector unsigned long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, (vector long long)__a,
-  (vector long long)__b);
+  return vec_any_ne((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static __inline__ int __ATTRS_o_ai vec_any_ne(vector signed long long __a,
   vector bool long long __b) {
-  return __builtin_altivec_vcmpequd_p(__CR6_LT_REV, __a,
-  (vector signed long long)__b);
+  return vec_any_ne((vector signed long long)__a, (vector signed long 
long)__b);
 }
 
 static 

[PATCH] D110668: [clang-cl] Accept `#pragma warning(disable : N)` for some N

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D110668#3034576 , @xbolva00 wrote:

> Please next time give a bit more time to potential reviewers / other folks 
> outside your org. The whole lifecycle of this patch (posted - landed) took < 
> 24h.

Is there anything wrong with the patch?

I agree that it's good to let larger changes sit for a bit, but this seems like 
a fairly small and inconsequential change to me. Many patches land with a 
review time < 24h.

In any case, happy to address post-commit review comments too of course.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110668

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

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Sam McCall via Phabricator via cfe-commits]

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

Thanks! Just nits

(This review addresses comments on 
https://reviews.llvm.org/rGdea48079b90d40f2087435b778544dffb0ab1793)




Comment at: clang-tools-extra/clangd/Headers.cpp:175
+  if (Entry == MainFileEntry) {
+RealPathNames[0] = MainFileEntry->tryGetRealPathName().str();
+return static_cast(0u);

if RealPathNames.front().empty...



Comment at: clang-tools-extra/clangd/Headers.h:123
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());

This is a strange public member, and would need to be documented.

But it seems better yet to make collectIncludeStructureCallback a member 
(`IncludeStructure::collect()`?) so we can just encapsulate this.



Comment at: clang-tools-extra/clangd/Headers.h:125
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;

nit: this assertion feels a little out-of-place/unneccesary



Comment at: clang-tools-extra/clangd/Headers.h:150
+  llvm::DenseMap
+  includeDepth(HeaderID Root = static_cast(0u)) const;
 

if we're going to expose the root headerID, we should do it in a named constant 
and reference that here.



Comment at: clang-tools-extra/clangd/Headers.h:158
 private:
+  const FileEntry *MainFileEntry;
+

this member should be documented (I think you can just move the bit about 
HeaderID(0) from below to here.



Comment at: clang-tools-extra/clangd/Headers.h:158
 private:
+  const FileEntry *MainFileEntry;
+

sammccall wrote:
> this member should be documented (I think you can just move the bit about 
> HeaderID(0) from below to here.
`= nullptr`



Comment at: clang-tools-extra/clangd/Headers.h:168
+  // We reserve HeaderID(0) for the main file and will manually check for that
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.

nit: it's the *value* that's unstable, not the type. So just "the UniqueID"



Comment at: clang-tools-extra/clangd/Headers.h:169
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.
   llvm::DenseMap UIDToIndex;

their -> the



Comment at: llvm/include/llvm/Support/FileSystem/UniqueID.h:17
 
+#include "llvm/ADT/DenseMap.h"
 #include 

you only need DenseMapInfo here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110925

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

[PATCH] D110798: [NFC] Use CHECK-NEXT instead of CHECK-SAME in target-invalid-cpu-note.c

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> Can you read the latest example I comment? I think you misunderstand the 
> extra outputs I mentioned. Or if I'm wrong, can you give an inline example?

I understood but yes your example is one that a SAME cannot catch, that's true. 
My point was that SAME does catch some changes, it's not like it's useless.

The only drawback to a single NEXT is when it fails the output from FileCheck 
isn't useful until you manually compare the lines it found, since they're so 
long. Then again a lot of FileCheck output can be like that and using NEXT 
makes the test more robust so sure, let's go with NEXT.

> Yeah, I suppose this is the only one test to check this valid CPU list? Then 
> I suppose to add check whole list for Arm targets.

Correct. (though I think downstream Arm Compiler tests this in other ways but 
that's beside the point)




Comment at: clang/test/Misc/target-invalid-cpu-note.c:1
 // RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix ARM
 // ARM: error: unknown target CPU 'not-a-cpu'

Please add a comment as the first line explaining that why we use NEXT and 
check `{{$}}` ending.



Comment at: clang/test/Misc/target-invalid-cpu-note.c:27
 // TUNE_X86_64: error: unknown target CPU 'not-a-cpu'
-// TUNE_X86_64: note: valid target CPU values are: i386, i486, winchip-c6, 
winchip2, c3,
-// TUNE_X86_64-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, 
pentium3,
-// TUNE_X86_64-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, 
prescott,
-// TUNE_X86_64-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, 
goldmont, goldmont-plus, tremont,
-// TUNE_X86_64-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, 
ivybridge,
-// TUNE_X86_64-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, 
skylake-avx512,
-// TUNE_X86_64-SAME: skx, cascadelake, cooperlake, cannonlake, icelake-client, 
rocketlake, icelake-server, tigerlake, sapphirerapids, alderlake, knl, knm, 
lakemont, k6, k6-2, k6-3,
-// TUNE_X86_64-SAME: athlon, athlon-tbird, athlon-xp, athlon-mp, athlon-4, k8, 
athlon64,
-// TUNE_X86_64-SAME: athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, 
amdfam10,
-// TUNE_X86_64-SAME: barcelona, btver1, btver2, bdver1, bdver2, bdver3, 
bdver4, znver1, znver2, znver3,
-// TUNE_X86_64-SAME: x86-64, geode{{$}}
+// TUNE_X86_64-NEXT: note: valid target CPU values are: i386, i486, 
winchip-c6, winchip2, c3, i586, pentium, pentium-mmx, pentiumpro, i686, 
pentium2, pentium3, pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, 
prescott, nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, 
goldmont-plus, tremont, nehalem, corei7, westmere, sandybridge, corei7-avx, 
ivybridge, core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, 
skx, cascadelake, cooperlake, cannonlake, icelake-client, rocketlake, 
icelake-server, tigerlake, sapphirerapids, alderlake, knl, knm, lakemont, k6, 
k6-2, k6-3, athlon, athlon-tbird, athlon-xp, athlon-mp, athlon-4, k8, athlon64, 
athlon-fx, opteron, k8-sse3, athlon64-sse3, opteron-sse3, amdfam10, barcelona, 
btver1, btver2, bdver1, bdver2, bdver3, bdver4, znver1, znver2, znver3, x86-64, 
geode{{$}}
 

This one uses `{{$}}` which I think (please check) means that anything extra on 
the end of the line will be a mistmatch. I'd add that to the rest as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110798

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

[PATCH] D107312: [analyzer] Fix deprecated plistlib functions

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

Gentle ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107312

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

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

In D106102#3035598 , @steakhal wrote:

> Good work. Land it.

I do not have landing rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106102

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

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Denys Petrov via Phabricator via cfe-commits]

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

Fixed cases in which RegionStore is able to get a stored value of a constant 
array through a pointer of inappropriate type. Adjust 
`RegionStoreManager::getBindingForElement` to the C++20 Standard.
Example:

  const int arr[42] = {1,2,3};
  int x1 = ((unsigned*)arr)[0];  // valid
  int x2 = ((short*)arr)[0]; // invalid
  int x3 = ((char*)arr)[0];  // valid
  int x4 = ((char*)arr)[1];  // invalid


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110927

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -2,6 +2,10 @@
 
 void clang_analyzer_eval(int);
 
+namespace std {
+enum class byte : unsigned char {};
+};
+
 struct S {
   int a = 3;
 };
@@ -48,6 +52,46 @@
   auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
+void glob_cast_same() {
+  auto *ptr = (int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // no-warning
+}
+
+void glob_cast_char() {
+  const auto *ptr = (char *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_uchar() {
+  auto *ptr = (unsigned char *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_byte() {
+  auto *ptr = (const std::byte *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_opposite_sign() {
+  auto *ptr = (unsigned int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_invalid1() {
+  auto *ptr = (signed char *)glob_arr2;
+  auto x = ptr[0]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_cast_invalid2() {
+  using T = short *;
+  auto x = ((T)glob_arr2)[0]; // expected-warning{{garbage or undefined}}
+}
+
 const float glob_arr3[] = {
 0., 0.0235, 0.0470, 0.0706, 0.0941, 0.1176};
 float no_warn_garbage_value() {
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -437,6 +437,8 @@
 
   RegionBindingsRef removeSubRegionBindings(RegionBindingsConstRef B,
 const SubRegion *R);
+  bool canAccessStoredValue(QualType OrigT, QualType ThroughT,
+uint64_t Index) const;
 
 public: // Part of public interface to class.
 
@@ -1625,6 +1627,56 @@
   return Result;
 }
 
+/// Returns true if the stored value can be accessed through the pointer to
+/// another type:
+///  const int arr[42] = {};
+///  auto* pchar = (char*)arr;
+///  auto* punsigned = (unsigned int*)arr;
+///  auto* pshort= (short*)arr;
+///  auto x1 = pchar[0]; // valid
+///  auto x2 = pchar[1]; // invalid
+///  auto x3 = punsigned[0]; // valid
+///  auto x4 = pshort;   // invalid
+bool RegionStoreManager::canAccessStoredValue(QualType OrigT, QualType ThroughT,
+  uint64_t Index) const {
+  // Remove cv-qualifiers.
+  OrigT = OrigT->getCanonicalTypeUnqualified();
+  ThroughT = ThroughT->getCanonicalTypeUnqualified();
+
+  // C++20 7.2.1.11 [basic.lval] (excerpt):
+  //  A program can access the stored value of an object through:
+  //  - the same type of the object;
+  //  - a signed or unsigned type corresponding to the type of the
+  //object;
+  //  - a char, unsigned char, std::byte. (NOTE:
+  //  Otherwise, the behavior is undefined.
+  return
+  // - is same
+  (OrigT == ThroughT) ||
+  // - is another sign
+  (((OrigT == Ctx.CharTy && ThroughT == Ctx.UnsignedCharTy) ||
+(OrigT == Ctx.SignedCharTy && ThroughT == Ctx.UnsignedCharTy) ||
+(OrigT == Ctx.ShortTy && ThroughT == Ctx.UnsignedShortTy) ||
+(OrigT == Ctx.IntTy && ThroughT == Ctx.UnsignedIntTy) ||
+(OrigT == Ctx.LongTy && ThroughT == Ctx.UnsignedLongTy) ||
+(OrigT == Ctx.LongLongTy && ThroughT == Ctx.UnsignedLongLongTy) ||
+(ThroughT == Ctx.CharTy && OrigT == Ctx.UnsignedCharTy) ||
+(ThroughT == Ctx.SignedCharTy && OrigT == Ctx.UnsignedCharTy) ||
+(ThroughT == Ctx.ShortTy && OrigT == Ctx.UnsignedShortTy) ||
+(ThroughT == Ctx.IntTy && OrigT == 

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Get rid of SM in getOrCreateID & getID


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110925

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  llvm/include/llvm/Support/FileSystem/UniqueID.h

Index: llvm/include/llvm/Support/FileSystem/UniqueID.h
===
--- llvm/include/llvm/Support/FileSystem/UniqueID.h
+++ llvm/include/llvm/Support/FileSystem/UniqueID.h
@@ -14,7 +14,9 @@
 #ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 #define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 namespace llvm {
 namespace sys {
@@ -47,6 +49,31 @@
 
 } // end namespace fs
 } // end namespace sys
+
+// Support UniqueIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline llvm::sys::fs::UniqueID getEmptyKey() {
+auto EmptyKey = DenseMapInfo>::getEmptyKey();
+return {EmptyKey.first, EmptyKey.second};
+  }
+
+  static inline llvm::sys::fs::UniqueID getTombstoneKey() {
+auto TombstoneKey =
+DenseMapInfo>::getTombstoneKey();
+return {TombstoneKey.first, TombstoneKey.second};
+  }
+
+  static unsigned getHashValue(const llvm::sys::fs::UniqueID ) {
+return hash_value(
+std::pair(Tag.getDevice(), Tag.getFile()));
+  }
+
+  static bool isEqual(const llvm::sys::fs::UniqueID ,
+  const llvm::sys::fs::UniqueID ) {
+return LHS == RHS;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -516,10 +516,10 @@
   IncludeStructure Includes = PatchedAST->getIncludeStructure();
   auto MainFE = FM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(MainFE);
-  auto MainID = Includes.getID(*MainFE, SM);
+  auto MainID = Includes.getID(*MainFE);
   auto AuxFE = FM.getFile(testPath("sub/aux.h"));
   ASSERT_TRUE(AuxFE);
-  auto AuxID = Includes.getID(*AuxFE, SM);
+  auto AuxID = Includes.getID(*AuxFE);
   EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
 }
 
@@ -560,12 +560,12 @@
   IncludeStructure Includes = ExpectedAST.getIncludeStructure();
   auto MainFE = FM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(MainFE);
-  auto MainID = Includes.getOrCreateID(*MainFE, SM);
+  auto MainID = Includes.getOrCreateID(*MainFE);
   auto  = PatchedAST->getSourceManager().getFileManager();
   IncludeStructure PatchedIncludes = PatchedAST->getIncludeStructure();
   auto PatchedMainFE = PatchedFM.getFile(testPath("foo.cpp"));
   ASSERT_TRUE(PatchedMainFE);
-  auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE, SM);
+  auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE);
   EXPECT_EQ(Includes.includeDepth(MainID)[MainID],
 PatchedIncludes.includeDepth(PatchedMainID)[PatchedMainID]);
 }
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -72,7 +72,7 @@
 auto  = Clang->getSourceManager();
 auto Entry = SM.getFileManager().getFile(Filename);
 EXPECT_TRUE(Entry);
-return Includes.getOrCreateID(*Entry, SM);
+return Includes.getOrCreateID(*Entry);
   }
 
   IncludeStructure collectIncludes() {
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -14,6 +14,7 @@
 #include "index/Symbol.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -119,15 +120,19 @@
 RealPathNames.emplace_back();
   }
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;
+  }
+
   // HeaderID identifies file in the include graph. It corresponds to a
   // FileEntry rather than a FileID, but stays stable across preamble & main
   // file builds.
   enum class HeaderID : unsigned {};
 
-  llvm::Optional getID(const FileEntry *Entry,
- const SourceManager ) const;
-  HeaderID getOrCreateID(const FileEntry *Entry,
- const SourceManager );
+  llvm::Optional getID(const FileEntry *Entry) const;
+  

[PATCH] D110925: [clangd] Follow-up on rGdea48079b90d

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110925

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  llvm/include/llvm/Support/FileSystem/UniqueID.h

Index: llvm/include/llvm/Support/FileSystem/UniqueID.h
===
--- llvm/include/llvm/Support/FileSystem/UniqueID.h
+++ llvm/include/llvm/Support/FileSystem/UniqueID.h
@@ -14,7 +14,9 @@
 #ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 #define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
 
+#include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 namespace llvm {
 namespace sys {
@@ -47,6 +49,31 @@
 
 } // end namespace fs
 } // end namespace sys
+
+// Support UniqueIDs as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline llvm::sys::fs::UniqueID getEmptyKey() {
+auto EmptyKey = DenseMapInfo>::getEmptyKey();
+return {EmptyKey.first, EmptyKey.second};
+  }
+
+  static inline llvm::sys::fs::UniqueID getTombstoneKey() {
+auto TombstoneKey =
+DenseMapInfo>::getTombstoneKey();
+return {TombstoneKey.first, TombstoneKey.second};
+  }
+
+  static unsigned getHashValue(const llvm::sys::fs::UniqueID ) {
+return hash_value(
+std::pair(Tag.getDevice(), Tag.getFile()));
+  }
+
+  static bool isEqual(const llvm::sys::fs::UniqueID ,
+  const llvm::sys::fs::UniqueID ) {
+return LHS == RHS;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -14,6 +14,7 @@
 #include "index/Symbol.h"
 #include "support/Logger.h"
 #include "support/Path.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -119,6 +120,12 @@
 RealPathNames.emplace_back();
   }
 
+  void setMainFileEntry(const FileEntry *Entry) {
+assert(Entry && Entry->isValid());
+assert(!RealPathNames.empty());
+this->MainFileEntry = Entry;
+  }
+
   // HeaderID identifies file in the include graph. It corresponds to a
   // FileEntry rather than a FileID, but stays stable across preamble & main
   // file builds.
@@ -126,8 +133,7 @@
 
   llvm::Optional getID(const FileEntry *Entry,
  const SourceManager ) const;
-  HeaderID getOrCreateID(const FileEntry *Entry,
- const SourceManager );
+  HeaderID getOrCreateID(const FileEntry *Entry, const SourceManager );
 
   StringRef getRealPath(HeaderID ID) const {
 assert(static_cast(ID) <= RealPathNames.size());
@@ -141,8 +147,8 @@
   // All transitive includes (absolute paths), with their minimum include depth.
   // Root --> 0, #included file --> 1, etc.
   // Root is the ID of the header being visited first.
-  // Usually it is getID(SM.getFileEntryForID(SM.getMainFileID()), SM).
-  llvm::DenseMap includeDepth(HeaderID Root) const;
+  llvm::DenseMap
+  includeDepth(HeaderID Root = static_cast(0u)) const;
 
   // Maps HeaderID to the ids of the files included from it.
   llvm::DenseMap> IncludeChildren;
@@ -150,16 +156,18 @@
   std::vector MainFileIncludes;
 
 private:
+  const FileEntry *MainFileEntry;
+
   std::vector RealPathNames; // In HeaderID order.
-  // HeaderID maps the FileEntry::UniqueID to the internal representation.
+  // FileEntry::UniqueID is mapped to the internal representation (HeaderID).
   // Identifying files in a way that persists from preamble build to subsequent
-  // builds is surprisingly hard. FileID is unavailable in
-  // InclusionDirective(), and RealPathName and UniqueID are not preserved in
+  // builds is surprisingly hard. FileID is unavailable in InclusionDirective(),
+  // and RealPathName and UniqueID are not preserved in
   // the preamble.
   //
-  // We reserve 0 to the main file and will manually check for that in getID
-  // and getOrCreateID because llvm::sys::fs::UniqueID is not stable when their
-  // content of the main file changes.
+  // We reserve HeaderID(0) for the main file and will manually check for that
+  // in getID and getOrCreateID because llvm::sys::fs::UniqueID is not stable
+  // when their content of the main file changes.
   llvm::DenseMap UIDToIndex;
 };
 
@@ -228,7 +236,7 @@
 
 namespace llvm {
 
-// Support Tokens as DenseMap keys.
+// Support HeaderIDs as DenseMap keys.
 template <> struct DenseMapInfo {
   static inline clang::clangd::IncludeStructure::HeaderID getEmptyKey() {
 return static_cast(
@@ -251,30 +259,6 @@
   }
 };
 
-// Support 

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

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

steakhal added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

aaron.ballman wrote:
> steakhal wrote:
> > If it can be used only once, we should mark this function as an `r-value` 
> > function.
> > There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> > 
> > Later, when you would actually call this function, you need to 
> > `std::move()` the object, signifying that the original object gets 
> > destroyed in the process.
> > 
> > @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we 
> > can simply use the `&&` in the function declaration?
> > I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in 
> > the past. What's the status on this?
> The expected pattern is:
> ```
> #if LLVM_HAS_RVALUE_REFERENCE_THIS
>   void func() && {
>   }
> #endif
> ```
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
>  (and elsewhere). However, I note that there are some unprotected uses (such 
> as 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
>  and so it may be a sign that we're fine to remove 
> `LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
> right thing?
You tried to eliminate that on Jan 22, 2020 with 
rGdfe9f130e07c929d21f8122272077495de531a38.
But according to git blame, the BuildTree.cpp#L437 was committed on Jul 9, 2019 
with rG9b3f38f99085e2bbf9db01bb00d4c6d837f0fc00.
I'm confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

[PATCH] D110810: [clang][ASTImporter] Simplify code of attribute import [NFC].

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8530-8531
 
+  // Get the result of the previous import attempt (can be used only once).
+  llvm::Expected getResult() {
+if (Err)

steakhal wrote:
> If it can be used only once, we should mark this function as an `r-value` 
> function.
> There is a similar macro already defined as `LLVM_LVALUE_FUNCTION`.
> 
> Later, when you would actually call this function, you need to `std::move()` 
> the object, signifying that the original object gets destroyed in the process.
> 
> @aaron.ballman Do you think we need to define `LLVM_RVALUE_FUNCTION` or we 
> can simply use the `&&` in the function declaration?
> I've seen that you tried to substitute all `LLVM_LVALUE_FUNCTION` macros in 
> the past. What's the status on this?
The expected pattern is:
```
#if LLVM_HAS_RVALUE_REFERENCE_THIS
  void func() && {
  }
#endif
```
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchersInternal.h#L609
 (and elsewhere). However, I note that there are some unprotected uses (such as 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Syntax/BuildTree.cpp#L437)
 and so it may be a sign that we're fine to remove 
`LLVM_HAS_RVALUE_REFERENCE_THIS` because all our supported compilers do the 
right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110810

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

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Christian Kühnel via cfe-commits]

Hi folks,

Arcanist not working any longer is really unfortunate. Phroge also has a
fork of Arcanist, however I haven't seen any SSL-related patches:
https://we.phorge.it/source/arcanist/browse/master/

1) Replacement for Herald rules.


I suppose the notification problem is solvable. I just took a quick look at
the GitHub APIs and wrote a proposal for a quick-and-dirty solution:
https://github.com/ChristianKuehnel/llvm-iwg/blob/gerald_design/pull_request_migration/gerald_design.md

Concerning privacy and storing user emails this solution is less than
ideal. However contributors are already sharing their email addresses in
the git logs. If we want to hide all personal information, a simple config
file is not enough, then we would need to have some UI where users
configure their rules. However this would be an even larger effort (web UI,
database, user authentication, Github SSO, ...).

I just checked GitHUb's public roadmap and did not find anything that
sounds like Herald:
https://github.com/github/roadmap/projects/1

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

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Christian Kühnel via cfe-commits]

I would like to add a third blocker to Mehdi's list:

3) We first would need to finish our ongoing Bugzilla to GitHub Issues
migration: At the moment the plan is to use the old bug ID in bugzilla as
issue ID on GitHub. However issues and Pull Requests share the same
namespace. So once we start using Pull Requests this would eat up the
numbers for the bugs/issues.

This is certainly not a show stopper, but something to decide on. I see two
options here:

1) We delay using Pull Requests until the Bugzilla to Issues migration is
completed.
2) We give up the requirement to keep the bug IDs and assign the new IDs as
they are available.


Best,
Christian


On Fri, Oct 1, 2021 at 12:56 AM Mehdi AMINI via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> We talked about this with the IWG (Infrastructure Working Group) just
> last week coincidentally.
> Two major blocking tracks that were identified at the roundtable
> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> today:
>
> 1) Replacement for Herald rules. This is what allows us to subscribe
> and track new revisions or commits based on paths in the repo or other
> criteria. We could build a replacement based on GitHub action or any
> other kind of service, but this is a bit tricky (how do you store
> emails privately? etc.). I have looked around online but I didn't find
> another OSS project (or external company) providing a similar service
> for GitHub unfortunately, does anyone know of any?
>
> 2) Support for stacked commits. I can see how to structure this
> somehow assuming we would push pull-request branches in the main repo
> (with one new commit per branch and cascading the pull-requests from
> one branch to the other), otherwise this will be a major regression
> compared to the current workflow.
>
> What remains unknown to me is the current state of GitHub management
> of comments across `git commit --amend` and force push to update a
> branch.
>
> Others may have other items to add!
>
> --
> Mehdi
>
> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>  wrote:
> >
> > How far are we from a workflow that leverages Github's Pull Requests?
> Is there some consensus that it's a desired end goal, but some features are
> missing?  Or do we prefer to use a workflow like this for the long term?
> >
> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
> >>
> >> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
> >>
> >>
> >>
> >> I know that arc is not universally used, but I think it’s a serious
> blow to many people’s workflows. I think that MyDeveloperDay’s question
> might have just become a bit more urgent.
> >>
> >>
> >>
> >> I suppose in the short-term, we could fork the phabricator repos in
> order to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
> >>
> >>
> >>
> >> Thanks,
> >>
> >>Chris Tetreault
> >>
> >>
> >>
> >> From: llvm-dev  On Behalf Of
> MyDeveloper Day via llvm-dev
> >> Sent: Wednesday, August 18, 2021 10:17 AM
> >> To: llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
> >>
> >>
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> >>
> >> All
> >>
> >>
> >>
> >> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
> >>
> >>
> >>
> >> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
> >>
> >>
> >>
> >> See:
> >>
> >>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
> >>
> >> https://www.phacility.com/phabricator/
> >>
> >>
> >>
> >> Personally I'm excited by the concept of a community driven replacement
> ( https://we.phorge.it/) .
> >>
> >> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
> >>
> >>
> >>
> >> MyDeveloperDay
> >>
> >> ___
> >> LLVM Developers mailing list
> >> llvm-...@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > ___
> > LLVM Developers mailing list
> > llvm-...@lists.llvm.org
> > 

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

On Thu, Sep 30, 2021, 6:04 PM Brian Cain  wrote:

> Does something like Rust's "bors" bot satisfy the herald rules need?
>


sorry, maybe I was thinking of the high-five bot. And it looks like that's
not quite a match for herald.



> re: #2 I have done this on GHE and it's mildly awkward but it does work.
>
> And yes normalizing force pushes is the unfortunate state of GitHub PRs.
> Comments are preserved. Code-anchored comments like review comments are
> marked as referring to out-of-date code, IIRC.
>
> On Thu, Sep 30, 2021, 5:56 PM Mehdi AMINI  wrote:
>
>> We talked about this with the IWG (Infrastructure Working Group) just
>> last week coincidentally.
>> Two major blocking tracks that were identified at the roundtable
>> during the LLVM Dev Meeting exactly 2 years ago are still an issue
>> today:
>>
>> 1) Replacement for Herald rules. This is what allows us to subscribe
>> and track new revisions or commits based on paths in the repo or other
>> criteria. We could build a replacement based on GitHub action or any
>> other kind of service, but this is a bit tricky (how do you store
>> emails privately? etc.). I have looked around online but I didn't find
>> another OSS project (or external company) providing a similar service
>> for GitHub unfortunately, does anyone know of any?
>>
>> 2) Support for stacked commits. I can see how to structure this
>> somehow assuming we would push pull-request branches in the main repo
>> (with one new commit per branch and cascading the pull-requests from
>> one branch to the other), otherwise this will be a major regression
>> compared to the current workflow.
>>
>> What remains unknown to me is the current state of GitHub management
>> of comments across `git commit --amend` and force push to update a
>> branch.
>>
>> Others may have other items to add!
>>
>> --
>> Mehdi
>>
>> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>>  wrote:
>> >
>> > How far are we from a workflow that leverages Github's Pull Requests?
>> Is there some consensus that it's a desired end goal, but some features are
>> missing?  Or do we prefer to use a workflow like this for the long term?
>> >
>> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
>> llvm-...@lists.llvm.org> wrote:
>> >>
>> >> As I, and others have noticed, it seems that as of today, there’s some
>> certificate issue with arcanist. (See:
>> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html)
>> The fix seems simple, and a PR is up, but looking through the PR activity,
>> it seems that the PR will not be accepted because Phabricator is no longer
>> being maintained. It seems that arc has become the first casualty of the
>> discontinuation of maintenance of phabricator.
>> >>
>> >>
>> >>
>> >> I know that arc is not universally used, but I think it’s a serious
>> blow to many people’s workflows. I think that MyDeveloperDay’s question
>> might have just become a bit more urgent.
>> >>
>> >>
>> >>
>> >> I suppose in the short-term, we could fork the phabricator repos in
>> order to fix little issues like this. Alternately, we should probably stop
>> recommending arcanist (unless we want to provide instructions on how to fix
>> any breakages that come along).
>> >>
>> >>
>> >>
>> >> Thanks,
>> >>
>> >>Chris Tetreault
>> >>
>> >>
>> >>
>> >> From: llvm-dev  On Behalf Of
>> MyDeveloper Day via llvm-dev
>> >> Sent: Wednesday, August 18, 2021 10:17 AM
>> >> To: llvm-dev ; cfe-commits <
>> cfe-commits@lists.llvm.org>
>> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
>> >>
>> >>
>> >>
>> >> WARNING: This email originated from outside of Qualcomm. Please be
>> wary of any links or attachments, and do not enable macros.
>> >>
>> >> All
>> >>
>> >>
>> >>
>> >> I'm a massive fan of Phabricator, and I know there is often lots of
>> contentious discussion about its relative merits vs github,
>> >>
>> >>
>> >>
>> >> But unless I missed this, was there any discussion regarding the
>> recent "Winding Down" announcement of Phabricator? and what it might mean
>> for us in LLVM
>> >>
>> >>
>> >>
>> >> See:
>> >>
>> >>
>> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
>> >>
>> >> https://www.phacility.com/phabricator/
>> >>
>> >>
>> >>
>> >> Personally I'm excited by the concept of a community driven
>> replacement ( https://we.phorge.it/) .
>> >>
>> >> epriestley did a truly amazing job, it wasn't open to public
>> contributions. Perhaps more open development could lead to closing some of
>> the github gaps that were of concern.
>> >>
>> >>
>> >>
>> >> MyDeveloperDay
>> >>
>> >> ___
>> >> LLVM Developers mailing list
>> >> llvm-...@lists.llvm.org
>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> >
>> > ___
>> > LLVM Developers mailing list
>> > llvm-...@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

Does something like Rust's "bors" bot satisfy the herald rules need?

re: #2 I have done this on GHE and it's mildly awkward but it does work.

And yes normalizing force pushes is the unfortunate state of GitHub PRs.
Comments are preserved. Code-anchored comments like review comments are
marked as referring to out-of-date code, IIRC.

On Thu, Sep 30, 2021, 5:56 PM Mehdi AMINI  wrote:

> We talked about this with the IWG (Infrastructure Working Group) just
> last week coincidentally.
> Two major blocking tracks that were identified at the roundtable
> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> today:
>
> 1) Replacement for Herald rules. This is what allows us to subscribe
> and track new revisions or commits based on paths in the repo or other
> criteria. We could build a replacement based on GitHub action or any
> other kind of service, but this is a bit tricky (how do you store
> emails privately? etc.). I have looked around online but I didn't find
> another OSS project (or external company) providing a similar service
> for GitHub unfortunately, does anyone know of any?
>
> 2) Support for stacked commits. I can see how to structure this
> somehow assuming we would push pull-request branches in the main repo
> (with one new commit per branch and cascading the pull-requests from
> one branch to the other), otherwise this will be a major regression
> compared to the current workflow.
>
> What remains unknown to me is the current state of GitHub management
> of comments across `git commit --amend` and force push to update a
> branch.
>
> Others may have other items to add!
>
> --
> Mehdi
>
> On Thu, Sep 30, 2021 at 3:39 PM Brian Cain via llvm-dev
>  wrote:
> >
> > How far are we from a workflow that leverages Github's Pull Requests?
> Is there some consensus that it's a desired end goal, but some features are
> missing?  Or do we prefer to use a workflow like this for the long term?
> >
> > On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
> llvm-...@lists.llvm.org> wrote:
> >>
> >> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
> >>
> >>
> >>
> >> I know that arc is not universally used, but I think it’s a serious
> blow to many people’s workflows. I think that MyDeveloperDay’s question
> might have just become a bit more urgent.
> >>
> >>
> >>
> >> I suppose in the short-term, we could fork the phabricator repos in
> order to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
> >>
> >>
> >>
> >> Thanks,
> >>
> >>Chris Tetreault
> >>
> >>
> >>
> >> From: llvm-dev  On Behalf Of
> MyDeveloper Day via llvm-dev
> >> Sent: Wednesday, August 18, 2021 10:17 AM
> >> To: llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> >> Subject: [llvm-dev] Phabricator Creator Pulling the Plug
> >>
> >>
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> >>
> >> All
> >>
> >>
> >>
> >> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
> >>
> >>
> >>
> >> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
> >>
> >>
> >>
> >> See:
> >>
> >>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
> >>
> >> https://www.phacility.com/phabricator/
> >>
> >>
> >>
> >> Personally I'm excited by the concept of a community driven replacement
> ( https://we.phorge.it/) .
> >>
> >> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
> >>
> >>
> >>
> >> MyDeveloperDay
> >>
> >> ___
> >> LLVM Developers mailing list
> >> llvm-...@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > ___
> > LLVM Developers mailing list
> > llvm-...@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [llvm-dev] Phabricator Creator Pulling the Plug

[Brian Cain via cfe-commits]

How far are we from a workflow that leverages Github's Pull Requests?  Is
there some consensus that it's a desired end goal, but some features are
missing?  Or do we prefer to use a workflow like this for the long term?

On Thu, Sep 30, 2021, 4:54 PM Chris Tetreault via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> As I, and others have noticed, it seems that as of today, there’s some
> certificate issue with arcanist. (See:
> https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The
> fix seems simple, and a PR is up, but looking through the PR activity, it
> seems that the PR will not be accepted because Phabricator is no longer
> being maintained. It seems that arc has become the first casualty of the
> discontinuation of maintenance of phabricator.
>
>
>
> I know that arc is not universally used, but I think it’s a serious blow
> to many people’s workflows. I think that MyDeveloperDay’s question might
> have just become a bit more urgent.
>
>
>
> I suppose in the short-term, we could fork the phabricator repos in order
> to fix little issues like this. Alternately, we should probably stop
> recommending arcanist (unless we want to provide instructions on how to fix
> any breakages that come along).
>
>
>
> Thanks,
>
>Chris Tetreault
>
>
>
> *From:* llvm-dev  *On Behalf Of *MyDeveloper
> Day via llvm-dev
> *Sent:* Wednesday, August 18, 2021 10:17 AM
> *To:* llvm-dev ; cfe-commits <
> cfe-commits@lists.llvm.org>
> *Subject:* [llvm-dev] Phabricator Creator Pulling the Plug
>
>
>
> *WARNING:* This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
>
> All
>
>
>
> I'm a massive fan of Phabricator, and I know there is often lots of
> contentious discussion about its relative merits vs github,
>
>
>
> But unless I missed this, was there any discussion regarding the recent
> "Winding Down" announcement of Phabricator? and what it might mean for us
> in LLVM
>
>
>
> See:
>
>
> https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
>
> https://www.phacility.com/phabricator/
>
>
>
> Personally I'm excited by the concept of a community driven replacement (
> https://we.phorge.it/) .
>
> epriestley did a truly amazing job, it wasn't open to public
> contributions. Perhaps more open development could lead to closing some of
> the github gaps that were of concern.
>
>
>
> MyDeveloperDay
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [llvm-dev] Phabricator Creator Pulling the Plug

[Chris Tetreault via cfe-commits]

As I, and others have noticed, it seems that as of today, there’s some 
certificate issue with arcanist. (See: 
https://lists.llvm.org/pipermail/llvm-dev/2021-September/153019.html) The fix 
seems simple, and a PR is up, but looking through the PR activity, it seems 
that the PR will not be accepted because Phabricator is no longer being 
maintained. It seems that arc has become the first casualty of the 
discontinuation of maintenance of phabricator.

I know that arc is not universally used, but I think it’s a serious blow to 
many people’s workflows. I think that MyDeveloperDay’s question might have just 
become a bit more urgent.

I suppose in the short-term, we could fork the phabricator repos in order to 
fix little issues like this. Alternately, we should probably stop recommending 
arcanist (unless we want to provide instructions on how to fix any breakages 
that come along).

Thanks,
   Chris Tetreault

From: llvm-dev  On Behalf Of MyDeveloper Day 
via llvm-dev
Sent: Wednesday, August 18, 2021 10:17 AM
To: llvm-dev ; cfe-commits 
Subject: [llvm-dev] Phabricator Creator Pulling the Plug


WARNING: This email originated from outside of Qualcomm. Please be wary of any 
links or attachments, and do not enable macros.
All

I'm a massive fan of Phabricator, and I know there is often lots of contentious 
discussion about its relative merits vs github,

But unless I missed this, was there any discussion regarding the recent 
"Winding Down" announcement of Phabricator? and what it might mean for us in 
LLVM

See:
https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/
https://www.phacility.com/phabricator/

Personally I'm excited by the concept of a community driven replacement ( 
https://we.phorge.it/) .
epriestley did a truly amazing job, it wasn't open to public contributions. 
Perhaps more open development could lead to closing some of the github gaps 
that were of concern.

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Just adding a few more clang-format big guns as reviewers, its probably good to 
get some more input before embarking on what is going to probably be a 
reasonably sized change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The use of "Custom" would kind of match the BraceWrapping, as for 
`SpaceBeforeParensCustom` either that or `SpaceBeforeParensStyles` , 
`SpaceBeforeParensSettings`, `SpaceBeforeParensOptions` I guess I don't really 
mind, (I always find naming things hard!) maybe the others might chip in on 
their preference of names

  BreakBeforeBraces: Custom
  BraceWrapping:
  AfterClass: true
  AfterControlStatement: false
  AfterEnum: true
  BeforeElse: true
  BeforeCatch: true
  AfterFunction: true
  AfterNamespace: true
  IndentBraces: false
  AfterStruct: true
  AfterUnion: true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D109740: [OpenCL] Add atomic_half type builtins

[Yang Haonan via Phabricator via cfe-commits]

haonanya updated this revision to Diff 376467.
haonanya added a comment.

Fix format issue


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

https://reviews.llvm.org/D109740

Files:
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/Sema.cpp
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -19,7 +19,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
+   atomic_intptr_t *p, atomic_float *f, atomic_double *d, atomic_half *h,
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -367,6 +367,11 @@
 AddPointerSizeDependentTypes();
   }
 
+  if (getOpenCLOptions().isSupported("cl_khr_fp16", getLangOpts())) {
+auto AtomicHalfT = Context.getAtomicType(Context.HalfTy);
+addImplicitTypedef("atomic_half", AtomicHalfT);
+  }
+
   std::vector Atomic64BitTypes;
   if (getOpenCLOptions().isSupported("cl_khr_int64_base_atomics",
  getLangOpts()) &&
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -85,16 +85,25 @@
 
 def FuncExtOpenCLCPipes  : FunctionExtension<"__opencl_c_pipes">;
 def FuncExtOpenCLCWGCollectiveFunctions  : FunctionExtension<"__opencl_c_work_group_collective_functions">;
+def FuncExtFloatAtomicsFp16GlobalLoadStore  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_load_store">;
+def FuncExtFloatAtomicsFp16LocalLoadStore   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_load_store">;
+def FuncExtFloatAtomicsFp16GenericLoadStore : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_load_store __opencl_c_ext_fp16_local_atomic_load_store">;
+def FuncExtFloatAtomicsFp16GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_add">;
 def FuncExtFloatAtomicsFp32GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_global_atomic_add">;
 def FuncExtFloatAtomicsFp64GlobalAdd : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_global_atomic_add">;
+def FuncExtFloatAtomicsFp16LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_add">;
 def FuncExtFloatAtomicsFp32LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_add">;
 def FuncExtFloatAtomicsFp64LocalAdd  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_add">;
+def FuncExtFloatAtomicsFp16GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_add __opencl_c_ext_fp16_global_atomic_add">;
 def FuncExtFloatAtomicsFp32GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_add __opencl_c_ext_fp32_global_atomic_add">;
 def FuncExtFloatAtomicsFp64GenericAdd: FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_add __opencl_c_ext_fp64_global_atomic_add">;
+def FuncExtFloatAtomicsFp16GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp32GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp64GlobalMinMax  : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_global_atomic_min_max">;
+def FuncExtFloatAtomicsFp16LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_min_max">;
 def FuncExtFloatAtomicsFp32LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_min_max">;
 def FuncExtFloatAtomicsFp64LocalMinMax   : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_min_max">;
+def FuncExtFloatAtomicsFp16GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp16_local_atomic_min_max __opencl_c_ext_fp16_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp32GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp32_local_atomic_min_max __opencl_c_ext_fp32_global_atomic_min_max">;
 def FuncExtFloatAtomicsFp64GenericMinMax : FunctionExtension<"cl_ext_float_atomics __opencl_c_ext_fp64_local_atomic_min_max __opencl_c_ext_fp64_global_atomic_min_max">;
 
@@ -362,6 +371,7 @@
 def AtomicULong   

[PATCH] D110825: [clangd] Handle members of anon structs in SelectionTree

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG512aa8485010: [clangd] Handle members of anon structs in 
SelectionTree (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110825

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), 
Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), 
Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 512aa84 - [clangd] Handle members of anon structs in SelectionTree

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-10-01T12:38:18+02:00
New Revision: 512aa8485010009f6ec1b8d9deea3effe67e0106

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

LOG: [clangd] Handle members of anon structs in SelectionTree

References to fields inside anon structs contain an implicit children
for the container, which has the same SourceLocation with the field.
This was resulting in SelectionTree always picking the anon-struct rather than
the field as the selection.

This patch prevents that by claiming the range for the field early.

https://github.com/clangd/clangd/issues/877.

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index e3a2e31997669..a53673e074804 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -443,6 +443,15 @@ bool isImplicit(const Stmt *S) {
   if (auto *CTI = llvm::dyn_cast(S))
 if (CTI->isImplicit())
   return true;
+  // Make sure implicit access of anonymous structs don't end up owning tokens.
+  if (auto *ME = llvm::dyn_cast(S)) {
+if (auto *FD = llvm::dyn_cast(ME->getMemberDecl()))
+  if (FD->isAnonymousStructOrUnion())
+// If Base is an implicit CXXThis, then the whole MemberExpr has no
+// tokens. If it's a normal e.g. DeclRef, we treat the MemberExpr like
+// an implicit cast.
+return isImplicit(ME->getBase());
+  }
   // Refs to operator() and [] are (almost?) always implicit as part of calls.
   if (auto *DRE = llvm::dyn_cast(S)) {
 if (auto *FD = llvm::dyn_cast(DRE->getDecl())) {

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 6a9d355792a67..e8b64e9200bb5 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -365,6 +365,27 @@ TEST(LocateSymbol, WithIndex) {
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), 
Test.range(;
 }
 
+TEST(LocateSymbol, AnonymousStructFields) {
+  auto Code = Annotations(R"cpp(
+struct $2[[Foo]] {
+  struct { int $1[[x]]; };
+  void foo() {
+// Make sure the implicit base is skipped.
+$1^x = 42;
+  }
+};
+// Check that we don't skip explicit bases.
+int a = $2^Foo{}.x;
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  auto AST = TU.build();
+  EXPECT_THAT(locateSymbolAt(AST, Code.point("1"), TU.index().get()),
+  UnorderedElementsAre(Sym("x", Code.range("1"), 
Code.range("1";
+  EXPECT_THAT(
+  locateSymbolAt(AST, Code.point("2"), TU.index().get()),
+  UnorderedElementsAre(Sym("Foo", Code.range("2"), Code.range("2";
+}
+
 TEST(LocateSymbol, FindOverrides) {
   auto Code = Annotations(R"cpp(
 class Foo {



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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

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

steakhal added a comment.

In D110625#3035974 , @ASDenysPetrov 
wrote:

> In D110625#3035929 , @steakhal 
> wrote:
>
>> I thought that `SVal::getType` should return an already canonical 
>> `QualType`. If it doesn't do that we would need to do canonicalization at 
>> each callsite, which is less than ideal IMO.
>> If it's not yet canonical, we should probably canonicalize within the 
>> `getType()` function probably. WDYT?
>
> I believe that it returns non-canonical type. Yes, we shall get canonical 
> versions separately for most cases in practice. This is not ideal but it 
> would be worse to return canonical types at once along with loss of extra 
> information.

Okay, if this is the case, we will need to use `getCanonicalType()` on both 
`BT` and `elementType`.
Vince, could you update your change, just to make sure it will work in the 
future, even if the test doesn't show much difference with the current behavior?
Note that you cannot call `getCanonicalType()` on a null-type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Nikita Popov via Phabricator via cfe-commits]

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

Sorry, but the fact that there is still no way to opt-in to the old behavior is 
still a blocker from my side. If we can't use `dereferenceable + nofree` 
arguments for that purpose, then we need to provide a different way to do that. 
Like `dereferenceable + really_nofree`. It looks like the current 
implementation doesn't even accept the `dereferenceable + nofree + noalias` 
case originally proposed (which is pretty bad from a design perspective, but 
would at least work fairly well for rustc in practice). I don't think that our 
current analysis capabilities are sufficient to land this change at this time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110625#3035929 , @steakhal wrote:

> I thought that `SVal::getType` should return an already canonical `QualType`. 
> If it doesn't do that we would need to do canonicalization at each callsite, 
> which is less than ideal IMO.
> If it's not yet canonical, we should probably canonicalize within the 
> `getType()` function probably. WDYT?

I believe that it returns non-canonical type. Yes, we shall get canonical 
versions separately for most cases in practice. This is not ideal but it would 
be worse to return canonical types at once along with loss of extra information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

MyDeveloperDay wrote:
> Now I look back here, why where these Macro considered the same as for loops? 
> why would we want
> 
> ```
> for ()
> Q_FOREACH(...)
> ```
> 
> So this really does need a struct or we'll be eventually be adding
> 
> `SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`
> 
> ```
> SpaceBeforeParen:
>  AfterFunctionDefinitionName: false
>  AfterFunctionDeclarationName: true
>  AfterSwitch: true
>  AfterForeachMacros: false
>   (there are likely others)
> ```
>  
> `
> 
> 
> 
> 
> 
Indeed replacing the enum with a struct as suggested is better to support the 
different possible combinations, compare to the current version of 
SpaceBeforeParens that results in very long names.

To support existing configuration files, I propose to keep the enum and to add 
a struct to handle the custom use cases and to cleanup the code. What do you 
think ?

```
SpaceBeforeParens: Custom
SpaceBeforeParensCustom:
 AfterFunctionDefinitionName: false
 AfterFunctionDeclarationName: true
 AfterSwitch: true
 AfterForeachMacros: false
…
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

Now I look back here, why where these Macro considered the same as for loops? 
why would we want

```
for ()
Q_FOREACH(...)
```

So this really does need a struct or we'll be eventually be adding

`SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`

```
SpaceBeforeParen:
 AfterFunctionDefinitionName: false
 AfterFunctionDeclarationName: true
 AfterSwitch: true
 AfterForeachMacros: false
  (there are likely others)
```
 
`







Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110913: [analyzer][solver] Handle simplification to ConcreteInt

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

steakhal added a comment.

The patch seems reasonable, but I will need slightly more time to digest it.
I'll get back to this.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:390-396
 /// Try to simplify a given symbolic expression's associated value based on the
 /// constraints in State. This is needed because the Environment bindings are
 /// not getting updated when a new constraint is added to the State.
+SVal simplifyToSVal(ProgramStateRef State, SymbolRef Sym);
+/// If the symbol is simplified to a constant then it returns the original
+/// symbol.
 SymbolRef simplify(ProgramStateRef State, SymbolRef Sym);

I'm confused about which comment corresponds to which function.
You should also signify the difference between the two APIs.

Shouldn't be one of them an implementation detail? If so, why do we have the 
same visibility?



Comment at: clang/test/Analysis/solver-sym-simplification-concreteint.c:22
+  // c == 0 --> 0 + 1 == 0 contradiction
+  clang_analyzer_eval(c == 0);// expected-warning{{FALSE}}
+

Could we have an `eval(c == -1) // TRUE`?
Also, please disable eager bifurcation to prevent state-splits in the eval 
arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110913

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

[PATCH] D110911: [analyzer][NFC] Add RangeSet::dump

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D110911#3035911 , @steakhal wrote:

> Please mark both of them `LLVM_DUMP_METHOD`s. This way they will be stripped 
> from release builds according to their documentation.

And `Range::dump` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110911

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

[PATCH] D110065: [AArch64] Add support for the 'R' architecture profile.

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:62
   std::string ABI;
+  StringRef getArchProfile() const;
 

tmatheson wrote:
> The equivalent in the ARM backend is named `getCPUProfile`
That's arguably a worse name though.


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

https://reviews.llvm.org/D110065

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

[PATCH] D110065: [AArch64] Add support for the 'R' architecture profile.

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:62
   std::string ABI;
+  StringRef getArchProfile() const;
 

The equivalent in the ARM backend is named `getCPUProfile`


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

https://reviews.llvm.org/D110065

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

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

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

steakhal added a comment.

In D110625#3035843 , @ASDenysPetrov 
wrote:

> In D110625#3035616 , @steakhal 
> wrote:
>
>> WDYT Denys? Btw does the SVval::getType return a canonical type in all cases?
>
> `SVal::getType` returns a `QualType` which can be aliased and cv-qualified. 
> So, it may mismatch in comparison `BT->getPointeeType() == elementType`. That 
> is my concern.

I thought that `SVal::getType` should return an already canonical `QualType`. 
If it doesn't do that we would need to do canonicalization at each callsite, 
which is less than ideal IMO.
If it's not yet canonical, we should probably canonicalize within the 
`getType()` function probably. WDYT?

---

In D110625#3035866 , @vabridgers 
wrote:

> I moved the test cases from ptr-arith.cpp to ptr-arith - using to typedef, 
> and created an extra test case that uses const. I believe this may address 
> all concerns discussed thus far? Please let me know if we need anything more 
> and I'll get it done. Best!

I think your change is good, but we need to sort some things out before moving 
forward with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110625

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Nuno Lopes via Phabricator via cfe-commits]

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

LGTM. We have discussed this before. It's important to fix the reference types 
in C++.




Comment at: llvm/test/Analysis/BasicAA/dereferenceable.ll:66
 ; CHECK: Function: local_and_deref_ret_2: 2 pointers, 2 call sites
-; CHECK-NEXT:  NoAlias:i32* %obj, i32* %ret
+; CHECK-NEXT:  MayAlias:   i32* %obj, i32* %ret
 bb:

this one is unfortunate.. They can't alias because objsize(%obj) < 
dereferenceable size of %ret. This case should be trivial to catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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