[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9733
 return true;
 return false;
   };

This is not the right fallback answer if the class is a dependent type -- we 
don't yet know if there's a non-deleted copy or move constructor if one might 
be implicitly declared, so we should conservatively assume that there might be 
one. It'd probably be easiest to return `true` from this function at the very 
start if the type is dependent (because we don't know if there's a non-deleted 
copy or move constructor).



Comment at: clang/test/SemaObjCXX/attr-trivial-abi.mm:59-62
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' 
cannot be applied to 'S10}}'  expected-note {{copy constructors and move 
constructors are all deleted}}
   T p;
 };

We should not diagnose this, because instantiations of this template might have 
non-deleted copy or move constructors. (We don't know yet when parsing the 
template definition.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263



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

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

-fdebug-prefix-map does not make sense to me because coverage is not debug info.
I am on the fence whether we need -fcoverage-prefix-map. I created 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage respect 
-ffile-prefix-map?)




Comment at: clang/include/clang/Driver/Options.td:2066
+  : Joined<["-"], "fcoverage-prefix-map=">, Group,
+Flags<[CC1Option,CC1AsOption]>,
+HelpText<"remap file source paths in coverage info">;

Drop CC1AsOption

The option does not affect -cc1as (assembly input)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1910
+
+auto upgradeAlignmentIfQualified = [&](const BuiltinType *BTy) {
+  if (BTy->getKind() == BuiltinType::Double ||

"Qualified" is a term of art in the context of C/C++ types. Please remove 
"IfQualified" from the name. The lambda just needs to be named for what it 
does. When naming it for the conditions where it should be called, "if" (as 
opposed to "assuming") implies that the function checks the condition itself.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1927
+  assert(RD && "Expected non-null RecordDecl.");
+  if (RD) {
+const ASTRecordLayout  = Context.getASTRecordLayout(RD);

Please don't write a check on a variable right after making an assertion on 
what its value should be.


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

https://reviews.llvm.org/D79719



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

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 275899.
oontvoo added a comment.

updated diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -56,9 +56,8 @@
   int a;
 };
 
-// Do not warn when 'trivial_abi' is used to annotate a template class.
-template
-struct __attribute__((trivial_abi)) S10 {
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
   T p;
 };
 
@@ -76,21 +75,27 @@
   __weak id b;
 };
 
-template
-struct __attribute__((trivial_abi)) S15 : S14 {
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
 };
 
 S15 s15;
 
-template
-struct __attribute__((trivial_abi)) S16 {
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
   S14 a;
 };
 
 S16 s16;
 
-template
+template 
 struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
+  S17();
+  S17(S17 &&);
+  __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
+
+// Should not crash.
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
+
+struct [[clang::trivial_abi]] S0 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S1 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
+  virtual void m();
+};
+
+struct S3_2 {
+  virtual void m();
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}
+
+struct __attribute__((trivial_abi)) S3_3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3_3'}} expected-note {{has a field of a non-trivial class type}}
+  S3_3(S3_3 &&);
+  S3_2 s32;
+};
+
+struct S4 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
+};
+
+struct __attribute__((trivial_abi)) S9 : public S4 {
+};
+
+struct __attribute__((trivial_abi(1))) S8 { // expected-error {{'trivial_abi' attribute takes no arguments}}
+  int a;
+};
+
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
+  T p;
+};
+
+S10 p1;
+
+template 
+struct S14 {
+  T a;
+};
+
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S15 s15;
+
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
+  S14 a;
+};
+
+S16 s16;
+
+template 
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S17 s17;
+
+namespace deletedCopyMoveConstructor {
+struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted a;
+};
+
+struct __attribute__((trivial_abi)) CopyDeleted {
+  CopyDeleted(const CopyDeleted &) = delete;
+  CopyDeleted(CopyDeleted &&) = 

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 275898.
oontvoo added a comment.

updated diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -57,8 +57,8 @@
 };
 
 // Do not warn when 'trivial_abi' is used to annotate a template class.
-template
-struct __attribute__((trivial_abi)) S10 {
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
   T p;
 };
 
@@ -76,21 +76,25 @@
   __weak id b;
 };
 
-template
-struct __attribute__((trivial_abi)) S15 : S14 {
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
 };
 
 S15 s15;
 
-template
-struct __attribute__((trivial_abi)) S16 {
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
   S14 a;
 };
 
 S16 s16;
 
-template
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
+template 
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{copy constructors and move constructors are all deleted}}
+  __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
+
+// Should not crash.
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
+
+struct [[clang::trivial_abi]] S0 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S1 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
+  virtual void m();
+};
+
+struct S3_2 {
+  virtual void m();
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}
+
+struct __attribute__((trivial_abi)) S3_3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3_3'}} expected-note {{has a field of a non-trivial class type}}
+  S3_3(S3_3 &&);
+  S3_2 s32;
+};
+
+struct S4 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
+};
+
+struct __attribute__((trivial_abi)) S9 : public S4 {
+};
+
+struct __attribute__((trivial_abi(1))) S8 { // expected-error {{'trivial_abi' attribute takes no arguments}}
+  int a;
+};
+
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
+  T p;
+};
+
+S10 p1;
+
+template 
+struct S14 {
+  T a;
+};
+
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S15 s15;
+
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
+  S14 a;
+};
+
+S16 s16;
+
+template 
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S17 s17;
+
+namespace deletedCopyMoveConstructor {
+struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted 

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 275895.
oontvoo added a comment.

Add more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaObjCXX/attr-trivial-abi.mm

Index: clang/test/SemaObjCXX/attr-trivial-abi.mm
===
--- clang/test/SemaObjCXX/attr-trivial-abi.mm
+++ clang/test/SemaObjCXX/attr-trivial-abi.mm
@@ -57,8 +57,8 @@
 };
 
 // Do not warn when 'trivial_abi' is used to annotate a template class.
-template
-struct __attribute__((trivial_abi)) S10 {
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
   T p;
 };
 
@@ -76,21 +76,25 @@
   __weak id b;
 };
 
-template
-struct __attribute__((trivial_abi)) S15 : S14 {
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
 };
 
 S15 s15;
 
-template
-struct __attribute__((trivial_abi)) S16 {
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
   S14 a;
 };
 
 S16 s16;
 
-template
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{has a __weak field}}
+template 
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{copy constructors and move constructors are all deleted}}
+  __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{has a __weak field}}
   __weak id a;
 };
 
Index: clang/test/SemaCXX/attr-trivial-abi.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-trivial-abi.cpp
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
+
+// Should not crash.
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
+
+struct [[clang::trivial_abi]] S0 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S1 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{is polymorphic}}
+  virtual void m();
+};
+
+struct S3_2 {
+  virtual void m();
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{is polymorphic}}
+
+struct S4 {
+  int a;
+};
+
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{has a virtual base}}
+};
+
+struct __attribute__((trivial_abi)) S9 : public S4 {
+};
+
+struct __attribute__((trivial_abi(1))) S8 { // expected-error {{'trivial_abi' attribute takes no arguments}}
+  int a;
+};
+
+// Do not warn when 'trivial_abi' is used to annotate a template class.
+template 
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10}}'  expected-note {{copy constructors and move constructors are all deleted}}
+  T p;
+};
+
+S10 p1;
+
+template 
+struct S14 {
+  T a;
+};
+
+template 
+struct __attribute__((trivial_abi)) S15 : S14 { // expected-warning {{'trivial_abi' cannot be applied to 'S15'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S15 s15;
+
+template 
+struct __attribute__((trivial_abi)) S16 { // expected-warning {{'trivial_abi' cannot be applied to 'S16'}} expected-note {{copy constructors and move constructors are all deleted}}
+  S14 a;
+};
+
+S16 s16;
+
+template 
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{copy constructors and move constructors are all deleted}}
+};
+
+S17 s17;
+
+namespace deletedCopyMoveConstructor {
+struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+  CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+};
+
+struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{copy constructors and move constructors are all deleted}}
+  CopyMoveDeleted a;
+};
+
+struct __attribute__((trivial_abi)) CopyDeleted {
+  CopyDeleted(const CopyDeleted &) = delete;
+  

[PATCH] D81315: [analyzer] Warning for default constructed unique pointer dereferences

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:202-219
+ProgramStateRef
+SmartPtrModeling::updateTrackedRegion(const CallEvent , CheckerContext ,
+  const MemRegion *ThisValRegion) const {
+  ProgramStateRef State = C.getState();
+  auto NumArgs = Call.getNumArgs();
+
+  if (NumArgs == 0) {

Szelethus wrote:
> Hmm, this function feels clunky. So, if the call has no arguments, we set the 
> smart pointer to null, otherwise if its a single-argument then we set it to 
> whatever the argument is? 
> 
> How about `operator[]`, that also takes a single argument, but isn't a memory 
> region? `get`, `get_deleter` don't take any arguments, but they don't set the 
> internal pointee to null either. The name `updateTrackedRegion` however 
> suggests that whatever operation was done, this is the 
> one-tool-to-solve-it-all function to take care of it.
> 
> I think this function handles too many things as once, and the name and lack 
> of documentation obfuscates its purpose. How about we put the relevant code 
> to `handleRelease`, and repurpose the rest of the function like this:
> 
> `updateOwnedRegion(CallEvent, CheckerContext, MemRegion of the smart pointer, 
> MemRegion to take ownership of)`
> 
> What do you think?
Yup, I completely agree. I think this structure will naturally evolve into 
something cleaner once more modeling gets added.


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

https://reviews.llvm.org/D81315



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

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

[Artem Dergachev via Phabricator via cfe-commits]

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

!

I vaguely remember that we talked offline about a few nasty test cases that 
with casts and overflows, would it make sense to add them? Like, even if they 
don't pass yet, they may prove valuable.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1472
+  // path compression when dealing with immutability.  That means that we
+  // compress paths every time we do merges.  It also means that we loose
+  // the main amortized complexity benefit from the original data structure.

"lose"!



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1487
+//   we shouldn't generate ranges incompatible with equivalence 
classes.
+//   However, at the moment, due to imperfections in the solver, it is
+//   possible and the merge function can also return infeasible states

"I am the solver!!!"

I guess what you're trying to say is that every time there's an infeasible 
state here, technically one of the `infer()` functions had a chance to prevent 
it (not necessarily easily).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82445



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

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar accepted this revision.
kuhar added a comment.

LGTM modulo accidental formatting changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83087



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2414
+assert(PreferredAlign >= ABIAlign &&
+   "PreferredAlign is at least as large as ABIAlign.");
+return PreferredAlign;

Minor nit: s/is/should be/;



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1207
+  if (!Base->Class->isEmpty())
+HasNonEmptyBaseClass = true;
+

Just set `HandledFirstNonOverlappingEmptyField` to `true` with a comment before 
the `if`:
By handling a base class that is not empty, we're handling the "first 
(inherited) member".



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1799
   if (D->isBitField()) {
+if (FoundNonOverlappingEmptyFieldToHandle)
+  // For a union, the current field does not represent all "firsts".

Add a comment before the `if`:
```
// We're going to handle the "first member" based on
// `FoundNonOverlappingEmptyFieldToHandle` during the current invocation of
// this function; record it as handled for future invocations.
```

Given the rationale from the comment, move the subject `if` to immediately 
after the determination of `FoundNonOverlappingEmptyFieldToHandle`. That way, 
the setting of `HandledFirstNonOverlappingEmptyField` becomes less complicated 
to track.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1832
 const ArrayType* ATy = Context.getAsArrayType(D->getType());
 FieldAlign = Context.getTypeAlignInChars(ATy->getElementType());
   } else if (const ReferenceType *RT = D->getType()->getAs()) {

It seems that the code to set `AlignIsRequired` is missing from this path.

```
typedef double Dbl __attribute__((__aligned__(2)));
typedef Dbl DblArr[];

union U {
  DblArr fam;
  char x;
};

U u[2];
extern char x[sizeof(u)];
extern char x[4];
```



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1908
+   FoundNonOverlappingEmptyFieldToHandle))) {
+HandledFirstNonOverlappingEmptyField = !IsUnion;
+

I think the `if` condition above is too complicated as a filter for setting 
`HandledFirstNonOverlappingEmptyField`. I would prefer if we don't need to set 
`HandledFirstNonOverlappingEmptyField` here. Please see my other comment about 
`HandledFirstNonOverlappingEmptyField`.


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

https://reviews.llvm.org/D79719



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

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: llvm/utils/update_cc_test_checks.py:133
+  parser.add_argument('--include-generated-funcs', action='store_true',
+  help='Output checks for functions not in source')
   parser.add_argument('tests', nargs='+')

greened wrote:
> greened wrote:
> > jdoerfert wrote:
> > > I think this should go into common.py (after D78618). I would also make 
> > > this the default but OK.
> > Yes I suppose it should in case `opt` and friends generate functions.  I 
> > hadn't considered that use-case.
> > 
> > While I would like to make it default unfortunately it would require 
> > updating a bunch of the existing clang tests which doesn't seem too 
> > friendly.  See the patch update comment for details.
> > 
> Just realized it wouldn't necessarily require regeneration of tests, it would 
> just cause regenerated tests to change a lot when they are eventually 
> regenerated.  We should discuss as to whether that's acceptable.  I think for 
> now this should be non-default to at least get the functionality in without 
> disturbing existing users and then we can discuss a separate change to make 
> it default.
> 
> It's also possible we could change how clang orders functions.  I discovered 
> there's a difference in clang 10 vs. 11 in the order functions are output 
> when OpenMP outlining happens.  clang 10 seems to preserve the source order 
> of functions and clang 11 does not.  Perhaps that needs to be fixed as I 
> don't know whether that change was intentional or not.
Best case, without the option the original behavior is preserved. Is that not 
the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004



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

[PATCH] D83111: [X86-64] Support Intel AMX Intrinsic

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

In D83111#2134747 , @craig.topper 
wrote:

> LGTM with all instances of "pointer point" replace with just "pointer"


Done it in commit. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83111



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

[clang] 939d830 - [X86-64] Support Intel AMX Intrinsic

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2020-07-07T10:13:40+08:00
New Revision: 939d8309dbd4ee6cf6e9ef3e8ea26df008b006b4

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

LOG: [X86-64] Support Intel AMX Intrinsic

INTEL ADVANCED MATRIX EXTENSIONS (AMX).
AMX is a new programming paradigm, it has a set of 2-dimensional registers
(TILES) representing sub-arrays from a larger 2-dimensional memory image and
operate on TILES.

These intrinsics use direct TMM register number as its params.

Spec can be found in Chapter 3 here 
https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html

Reviewed By: craig.topper

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

Added: 
clang/lib/Headers/amxintrin.h
clang/test/CodeGen/AMX/amx.c
clang/test/CodeGen/AMX/amx_errors.c
clang/test/CodeGen/AMX/amx_inline_asm.c
clang/test/Preprocessor/x86_amx_target_features.c
llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll
llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll
llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Basic/BuiltinsX86_64.def
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Driver/Options.td
clang/include/clang/Sema/Sema.h
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/cpuid.h
clang/lib/Headers/immintrin.h
clang/lib/Sema/SemaChecking.cpp
clang/test/Driver/x86-target-features.c
llvm/include/llvm/IR/IntrinsicsX86.td
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86InstrAMX.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 67c341feffbb..672c4ae80e73 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -3127,6 +3127,12 @@ X86
 
 .. option:: -maes, -mno-aes
 
+.. option:: -mamx-bf16, -mno-amx-bf16
+
+.. option:: -mamx-int8, -mno-amx-int8
+
+.. option:: -mamx-tile, -mno-amx-tile
+
 .. option:: -mavx, -mno-avx
 
 .. option:: -mavx2, -mno-avx2

diff  --git a/clang/include/clang/Basic/BuiltinsX86_64.def 
b/clang/include/clang/Basic/BuiltinsX86_64.def
index c535f43203e5..7feccd2a81a0 100644
--- a/clang/include/clang/Basic/BuiltinsX86_64.def
+++ b/clang/include/clang/Basic/BuiltinsX86_64.def
@@ -101,6 +101,22 @@ TARGET_BUILTIN(__builtin_ia32_cvtsi2ss64, "V4fV4fOiIi", 
"ncV:128:", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_cvtusi2sd64, "V2dV2dUOiIi", "ncV:128:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_cvtusi2ss64, "V4fV4fUOiIi", "ncV:128:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_directstore_u64, "vULi*ULi", "n", "movdiri")
+
+// AMX
+TARGET_BUILTIN(__builtin_ia32_tile_loadconfig, "vvC*", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tile_storeconfig, "vvC*", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tilerelease, "v", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tilezero, "vUc", "n", "amx-tile")
+
+TARGET_BUILTIN(__builtin_ia32_tileloadd64, "vIUcvC*z", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tileloaddt164, "vIUcvC*z", "n", "amx-tile")
+TARGET_BUILTIN(__builtin_ia32_tilestored64, "vIUcv*z", "n", "amx-tile")
+
+TARGET_BUILTIN(__builtin_ia32_tdpbssd, "vIUcIUcIUc", "n", "amx-int8")
+TARGET_BUILTIN(__builtin_ia32_tdpbsud, "vIUcIUcIUc", "n", "amx-int8")
+TARGET_BUILTIN(__builtin_ia32_tdpbusd, "vIUcIUcIUc", "n", "amx-int8")
+TARGET_BUILTIN(__builtin_ia32_tdpbuud, "vIUcIUcIUc", "n", "amx-int8")
+TARGET_BUILTIN(__builtin_ia32_tdpbf16ps, "vIUcIUcIUc", "n", "amx-bf16")
 TARGET_BUILTIN(__builtin_ia32_ptwrite64, "vUOi", "n", "ptwrite")
 
 #undef BUILTIN

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5b94aa8c4325..c935545610e0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9342,6 +9342,8 @@ def err_x86_builtin_invalid_rounding : Error<
   "invalid rounding argument">;
 def err_x86_builtin_invalid_scale : Error<
   "scale argument must be 1, 2, 4, or 8">;
+def err_x86_builtin_tile_arg_duplicate : Error<
+  "tile arguments must refer to 
diff erent tiles">;
 
 def err_builtin_target_unsupported : Error<
   "builtin is not supported on this target">;

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 50d18343f7d4..745c696bcaa3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3065,6 +3065,12 @@ def m3dnow : Flag<["-"], "m3dnow">, 
Group;
 def mno_3dnow : Flag<["-"], 

[PATCH] D83111: [X86-64] Support Intel AMX Intrinsic

[Xiang Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG939d8309dbd4: [X86-64] Support Intel AMX Intrinsic (authored 
by xiangzhangllvm).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83111?vs=275878=275888#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83111

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/amxintrin.h
  clang/lib/Headers/cpuid.h
  clang/lib/Headers/immintrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/AMX/amx.c
  clang/test/CodeGen/AMX/amx_errors.c
  clang/test/CodeGen/AMX/amx_inline_asm.c
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/x86_amx_target_features.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll
  llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll
  llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll

Index: llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-tile-intrinsics.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -verify-machineinstrs | FileCheck %s
+
+define void @test_amx(i8* %pointer, i8* %base, i64 %stride) {
+; CHECK-LABEL: test_amx:
+; CHECK:   # %bb.0:
+  call void @llvm.x86.ldtilecfg(i8* %pointer)
+; CHECK-NEXT:ldtilecfg (%rdi)
+
+  call void @llvm.x86.sttilecfg(i8* %pointer)
+; CHECK-NEXT:sttilecfg (%rdi)
+
+  call void @llvm.x86.tilerelease()
+; CHECK-NEXT:tilerelease
+
+  call void @llvm.x86.tilezero(i8 3)
+; CHECK-NEXT:tilezero %tmm3
+
+  call void @llvm.x86.tileloadd64(i8 3, i8* %base, i64 %stride)
+; CHECK-NEXT:tileloadd (%rsi,%rdx), %tmm3
+
+  call void @llvm.x86.tileloaddt164(i8 3, i8* %base, i64 %stride)
+; CHECK-NEXT:tileloaddt1 (%rsi,%rdx), %tmm3
+
+  call void @llvm.x86.tilestored64(i8 3, i8* %base, i64 %stride)
+; CHECK-NEXT:tilestored %tmm3, (%rsi,%rdx)
+  ret void
+}
+
+declare void @llvm.x86.tileloadd64(i8 %tile, i8* %base, i64 %stride)
+declare void @llvm.x86.tileloaddt164(i8 %tile, i8* %base, i64 %stride)
+declare void @llvm.x86.tilestored64(i8 %tile, i8* %base, i64 %stride)
+declare void @llvm.x86.ldtilecfg(i8* %pointer)
+declare void @llvm.x86.sttilecfg(i8* %pointer)
+declare void @llvm.x86.tilerelease()
+declare void @llvm.x86.tilezero(i8 %tile)
Index: llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-int8-intrinsics.ll
@@ -0,0 +1,24 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-int8 -verify-machineinstrs | FileCheck %s
+
+define void @test_amx() {
+; CHECK-LABEL: test_amx:
+; CHECK:   # %bb.0:
+  call void @llvm.x86.tdpbssd(i8 3, i8 4, i8 7)
+; CHECK-NEXT:tdpbssd %tmm7, %tmm4, %tmm3
+
+  call void @llvm.x86.tdpbsud(i8 3, i8 4, i8 7)
+; CHECK-NEXT:tdpbsud %tmm7, %tmm4, %tmm3
+
+  call void @llvm.x86.tdpbusd(i8 3, i8 0, i8 7)
+; CHECK-NEXT:tdpbusd %tmm7, %tmm0, %tmm3
+
+  call void @llvm.x86.tdpbuud(i8 3, i8 4, i8 1)
+; CHECK-NEXT:tdpbuud %tmm1, %tmm4, %tmm3
+  ret void
+}
+
+declare void @llvm.x86.tdpbssd(i8 %tile0, i8 %tile1, i8 %tile2)
+declare void @llvm.x86.tdpbsud(i8 %tile0, i8 %tile1, i8 %tile2)
+declare void @llvm.x86.tdpbusd(i8 %tile0, i8 %tile1, i8 %tile2)
+declare void @llvm.x86.tdpbuud(i8 %tile0, i8 %tile1, i8 %tile2)
Index: llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-bf16-intrinsics.ll
@@ -0,0 +1,13 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -mattr=+amx-bf16 -verify-machineinstrs | FileCheck %s
+
+define void @test_amx() {
+; CHECK-LABEL: test_amx:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:tdpbf16ps %tmm7, %tmm4, %tmm3
+; CHECK-NEXT:retq
+  call void @llvm.x86.tdpbf16ps(i8 3, i8 4, i8 7)
+  ret void
+}
+
+declare void @llvm.x86.tdpbf16ps(i8 %tile0, i8 %tile1, i8 %tile2)
Index: llvm/lib/Target/X86/X86InstrAMX.td
===
--- llvm/lib/Target/X86/X86InstrAMX.td
+++ 

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[River Riddle via Phabricator via cfe-commits]

rriddle accepted this revision.
rriddle added a comment.

Approval for anything MLIR related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83087



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

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 275885.
oontvoo added a comment.

Dont skip checking even for templated type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/trivial-abi-templated-type.cpp


Index: clang/test/SemaCXX/trivial-abi-templated-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/trivial-abi-templated-type.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-no-diagnostics
+
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9719,6 +9719,11 @@
 
   // Ill-formed if the copy and move constructors are deleted.
   auto HasNonDeletedCopyOrMoveConstructor = [&]() {
+// If we know there exist users-defined move/copy ctors, don't try to infer
+// the deletion of implicit ones becausee Sema may not have the info yet.
+if (RD.hasUserDeclaredMoveConstructor() ||
+RD.hasUserDeclaredCopyConstructor())
+  return true;
 if (RD.needsImplicitCopyConstructor() &&
 !RD.defaultedCopyConstructorIsDeleted())
   return true;


Index: clang/test/SemaCXX/trivial-abi-templated-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/trivial-abi-templated-type.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-no-diagnostics
+
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9719,6 +9719,11 @@
 
   // Ill-formed if the copy and move constructors are deleted.
   auto HasNonDeletedCopyOrMoveConstructor = [&]() {
+// If we know there exist users-defined move/copy ctors, don't try to infer
+// the deletion of implicit ones becausee Sema may not have the info yet.
+if (RD.hasUserDeclaredMoveConstructor() ||
+RD.hasUserDeclaredCopyConstructor())
+  return true;
 if (RD.needsImplicitCopyConstructor() &&
 !RD.defaultedCopyConstructorIsDeleted())
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81751: [SemaObjC] Fix a -Wobjc-signed-char-bool false-positive with binary conditional operator

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This looks good to me.




Comment at: clang/lib/Sema/SemaChecking.cpp:11825
 
+  Expr *TrueExpr = E->getTrueExpr();
+  if (isa(E))

Can you use `BinaryConditionalOperator::getCommon` here?


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

https://reviews.llvm.org/D81751



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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Eric Christopher via Phabricator via cfe-commits]

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

Works for me :)


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

https://reviews.llvm.org/D83273



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

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 275883.
craig.topper added a comment.

Drop two header includes I was using for debugging


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

https://reviews.llvm.org/D83273

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/X86TargetParser.cpp

Index: llvm/lib/Support/X86TargetParser.cpp
===
--- llvm/lib/Support/X86TargetParser.cpp
+++ llvm/lib/Support/X86TargetParser.cpp
@@ -48,6 +48,14 @@
 return (Bits[I / 32] & Mask) != 0;
   }
 
+  constexpr FeatureBitset |=(const FeatureBitset ) {
+for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I) {
+  uint32_t NewBits = Bits[I] | RHS.Bits[I];
+  Bits[I] = NewBits;
+}
+return *this;
+  }
+
   constexpr FeatureBitset operator&(const FeatureBitset ) const {
 FeatureBitset Result;
 for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I)
@@ -77,6 +85,11 @@
   FeatureBitset Features;
 };
 
+struct FeatureInfo {
+  StringLiteral Name;
+  FeatureBitset ImpliedFeatures;
+};
+
 } // end anonymous namespace
 
 #define X86_FEATURE(ENUM, STRING)  \
@@ -192,13 +205,13 @@
 static constexpr FeatureBitset FeaturesK8SSE3 = FeaturesK8 | FeatureSSE3;
 static constexpr FeatureBitset FeaturesAMDFAM10 =
 FeaturesK8SSE3 | FeatureCMPXCHG16B | FeatureLZCNT | FeaturePOPCNT |
-FeaturePRFCHW | FeatureSAHF | FeatureSSE4A;
+FeaturePRFCHW | FeatureSAHF | FeatureSSE4_A;
 
 // Bobcat architecture processors.
 static constexpr FeatureBitset FeaturesBTVER1 =
 FeatureX87 | FeatureCMPXCHG8B | FeatureCMPXCHG16B | FeatureEM64T |
 FeatureFXSR | FeatureLZCNT | FeatureMMX | FeaturePOPCNT | FeaturePRFCHW |
-FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4A |
+FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4_A |
 FeatureSAHF;
 static constexpr FeatureBitset FeaturesBTVER2 =
 FeaturesBTVER1 | FeatureAES | FeatureAVX | FeatureBMI | FeatureF16C |
@@ -210,7 +223,7 @@
 FeatureCMPXCHG16B | FeatureEM64T | FeatureFMA4 | FeatureFXSR | FeatureLWP |
 FeatureLZCNT | FeatureMMX | FeaturePCLMUL | FeaturePOPCNT | FeaturePRFCHW |
 FeatureSAHF | FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 |
-FeatureSSE4_1 | FeatureSSE4_2 | FeatureSSE4A | FeatureXOP | FeatureXSAVE;
+FeatureSSE4_1 | FeatureSSE4_2 | FeatureSSE4_A | FeatureXOP | FeatureXSAVE;
 static constexpr FeatureBitset FeaturesBDVER2 =
 FeaturesBDVER1 | FeatureBMI | FeatureFMA | FeatureF16C | FeatureTBM;
 static constexpr FeatureBitset FeaturesBDVER3 =
@@ -228,7 +241,7 @@
 FeatureMOVBE | FeatureMWAITX | FeaturePCLMUL | FeaturePOPCNT |
 FeaturePRFCHW | FeatureRDRND | FeatureRDSEED | FeatureSAHF | FeatureSHA |
 FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4_1 |
-FeatureSSE4_2 | FeatureSSE4A | FeatureXSAVE | FeatureXSAVEC |
+FeatureSSE4_2 | FeatureSSE4_A | FeatureXSAVE | FeatureXSAVEC |
 FeatureXSAVEOPT | FeatureXSAVES;
 static constexpr FeatureBitset FeaturesZNVER2 =
 FeaturesZNVER1 | FeatureCLWB | FeatureRDPID | FeatureWBNOINVD;
@@ -376,19 +389,178 @@
   llvm_unreachable("Unable to find CPU kind!");
 }
 
-static const char *FeatureStrings[X86::CPU_FEATURE_MAX] = {
-#define X86_FEATURE(ENUM, STR) STR,
+// Features with no dependencies.
+static constexpr FeatureBitset ImpliedFeaturesADX = {};
+static constexpr FeatureBitset ImpliedFeaturesBMI = {};
+static constexpr FeatureBitset ImpliedFeaturesBMI2 = {};
+static constexpr FeatureBitset ImpliedFeaturesCLDEMOTE = {};
+static constexpr FeatureBitset ImpliedFeaturesCLFLUSHOPT = {};
+static constexpr FeatureBitset ImpliedFeaturesCLWB = {};
+static constexpr FeatureBitset ImpliedFeaturesCLZERO = {};
+static constexpr FeatureBitset ImpliedFeaturesCMOV = {};
+static constexpr FeatureBitset ImpliedFeaturesCMPXCHG16B = {};
+static constexpr FeatureBitset ImpliedFeaturesCMPXCHG8B = {};
+static constexpr FeatureBitset ImpliedFeaturesEM64T = {};
+static constexpr FeatureBitset ImpliedFeaturesENQCMD = {};
+static constexpr FeatureBitset ImpliedFeaturesFSGSBASE = {};
+static constexpr FeatureBitset ImpliedFeaturesFXSR = {};
+static constexpr FeatureBitset ImpliedFeaturesINVPCID = {};
+static constexpr FeatureBitset ImpliedFeaturesLWP = {};
+static constexpr FeatureBitset ImpliedFeaturesLZCNT = {};
+static constexpr FeatureBitset ImpliedFeaturesMWAITX = {};
+static constexpr FeatureBitset ImpliedFeaturesMOVBE = {};
+static constexpr FeatureBitset ImpliedFeaturesMOVDIR64B = {};
+static constexpr FeatureBitset ImpliedFeaturesMOVDIRI = {};
+static constexpr FeatureBitset ImpliedFeaturesPCONFIG = {};
+static constexpr FeatureBitset ImpliedFeaturesPOPCNT = {};
+static constexpr FeatureBitset ImpliedFeaturesPKU = {};
+static constexpr FeatureBitset 

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: RKSimon, spatel, echristo, erichkeane, LuoYuanke, 
LiuChen3, FreddyYe, xiangzhangllvm.
Herald added subscribers: jfb, hiraditya.
Herald added a project: LLVM.

Previously we had to specify the forward and backwards feature dependencies 
separately which was error prone. And as dependencies have gotten more complex 
it was hard to be sure the transitive dependencies were handled correctly. The 
way it was written was also not super readable.

This patch replaces everything with a table that lists what features a feature 
is dependent on directly. Then we just have to recursively walk through the 
table to find the transitive dependencies. This is largely based on how we 
handle subtarget features in the MC layer from the tablegen descriptions.


https://reviews.llvm.org/D83273

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/X86TargetParser.cpp

Index: llvm/lib/Support/X86TargetParser.cpp
===
--- llvm/lib/Support/X86TargetParser.cpp
+++ llvm/lib/Support/X86TargetParser.cpp
@@ -13,6 +13,8 @@
 #include "llvm/Support/X86TargetParser.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::X86;
@@ -48,6 +50,14 @@
 return (Bits[I / 32] & Mask) != 0;
   }
 
+  constexpr FeatureBitset |=(const FeatureBitset ) {
+for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I) {
+  uint32_t NewBits = Bits[I] | RHS.Bits[I];
+  Bits[I] = NewBits;
+}
+return *this;
+  }
+
   constexpr FeatureBitset operator&(const FeatureBitset ) const {
 FeatureBitset Result;
 for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I)
@@ -77,6 +87,11 @@
   FeatureBitset Features;
 };
 
+struct FeatureInfo {
+  StringLiteral Name;
+  FeatureBitset ImpliedFeatures;
+};
+
 } // end anonymous namespace
 
 #define X86_FEATURE(ENUM, STRING)  \
@@ -192,13 +207,13 @@
 static constexpr FeatureBitset FeaturesK8SSE3 = FeaturesK8 | FeatureSSE3;
 static constexpr FeatureBitset FeaturesAMDFAM10 =
 FeaturesK8SSE3 | FeatureCMPXCHG16B | FeatureLZCNT | FeaturePOPCNT |
-FeaturePRFCHW | FeatureSAHF | FeatureSSE4A;
+FeaturePRFCHW | FeatureSAHF | FeatureSSE4_A;
 
 // Bobcat architecture processors.
 static constexpr FeatureBitset FeaturesBTVER1 =
 FeatureX87 | FeatureCMPXCHG8B | FeatureCMPXCHG16B | FeatureEM64T |
 FeatureFXSR | FeatureLZCNT | FeatureMMX | FeaturePOPCNT | FeaturePRFCHW |
-FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4A |
+FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4_A |
 FeatureSAHF;
 static constexpr FeatureBitset FeaturesBTVER2 =
 FeaturesBTVER1 | FeatureAES | FeatureAVX | FeatureBMI | FeatureF16C |
@@ -210,7 +225,7 @@
 FeatureCMPXCHG16B | FeatureEM64T | FeatureFMA4 | FeatureFXSR | FeatureLWP |
 FeatureLZCNT | FeatureMMX | FeaturePCLMUL | FeaturePOPCNT | FeaturePRFCHW |
 FeatureSAHF | FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 |
-FeatureSSE4_1 | FeatureSSE4_2 | FeatureSSE4A | FeatureXOP | FeatureXSAVE;
+FeatureSSE4_1 | FeatureSSE4_2 | FeatureSSE4_A | FeatureXOP | FeatureXSAVE;
 static constexpr FeatureBitset FeaturesBDVER2 =
 FeaturesBDVER1 | FeatureBMI | FeatureFMA | FeatureF16C | FeatureTBM;
 static constexpr FeatureBitset FeaturesBDVER3 =
@@ -228,7 +243,7 @@
 FeatureMOVBE | FeatureMWAITX | FeaturePCLMUL | FeaturePOPCNT |
 FeaturePRFCHW | FeatureRDRND | FeatureRDSEED | FeatureSAHF | FeatureSHA |
 FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4_1 |
-FeatureSSE4_2 | FeatureSSE4A | FeatureXSAVE | FeatureXSAVEC |
+FeatureSSE4_2 | FeatureSSE4_A | FeatureXSAVE | FeatureXSAVEC |
 FeatureXSAVEOPT | FeatureXSAVES;
 static constexpr FeatureBitset FeaturesZNVER2 =
 FeaturesZNVER1 | FeatureCLWB | FeatureRDPID | FeatureWBNOINVD;
@@ -376,19 +391,178 @@
   llvm_unreachable("Unable to find CPU kind!");
 }
 
-static const char *FeatureStrings[X86::CPU_FEATURE_MAX] = {
-#define X86_FEATURE(ENUM, STR) STR,
+// Features with no dependencies.
+static constexpr FeatureBitset ImpliedFeaturesADX = {};
+static constexpr FeatureBitset ImpliedFeaturesBMI = {};
+static constexpr FeatureBitset ImpliedFeaturesBMI2 = {};
+static constexpr FeatureBitset ImpliedFeaturesCLDEMOTE = {};
+static constexpr FeatureBitset ImpliedFeaturesCLFLUSHOPT = {};
+static constexpr FeatureBitset ImpliedFeaturesCLWB = {};
+static constexpr FeatureBitset ImpliedFeaturesCLZERO = {};
+static constexpr FeatureBitset ImpliedFeaturesCMOV = {};
+static constexpr FeatureBitset ImpliedFeaturesCMPXCHG16B = {};
+static constexpr FeatureBitset ImpliedFeaturesCMPXCHG8B = {};

[PATCH] D82617: Disable GCC's -Woverloaded-virtual, which has false positives.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D82617#2119394 , @sammccall wrote:

> Abandoning, we'll do this in clangd or find an acceptable way to silence it 
> (see D82736 ).
>
> In D82617#2119144 , @dblaikie wrote:
>
> > In D82617#2119138 , @sammccall 
> > wrote:
> >
> > > > Guess perhaps a different question: Why don't you want this for clangd? 
> > > > Does it make the codebase better by not adhering to this particular 
> > > > warning?
> > >
> > > Yes, exactly. (Sorry if this wasn't explicit).
> >
> >
> > Sorry - poor phrasing on my part. Seems we disagree on this - I think it's 
> > probably a good thing to adhere to, you don't.
>
>
> Yeah, I think we disagree, and that's OK! We need to stop emitting a warning, 
> and apart from that, this doesn't seem like a terribly important question 
> that needs an LLVM-wide policy.
>
> On stylistic questions like this, I think code owners generally decide, 
> right? So my intent was to drop this proposed change for clang (no consensus) 
> and take these opinions under advisement for clangd.
>
> I didn't re-engage here on the substance as I think all the arguments are 
> played out and nobody's mind is being changed, and I figured it was time to 
> move on.
>
> > I'd like to better understand the difference of opinions.
>
> Sure, and apologies I didn't read it that way. I can summarize the 
> counterargument as:
>
> 1. I agree that this warning flags cases with confusing code, though in some 
> cases (like ThreadsafeFS::view) I think the potential for confusion *due to 
> the name-hiding + overloading pattern* is very low.


Fair - I'd be willing to classify this particular case as pretty close to a 
false positive. But for me it'd be under the bucket of false positive's like 
GCC's -Wparetheses which warns on assert("message" && x || y) (GCC warns about 
the user probably meaning (x || y), Clang doesn't because it notices you get 
the same answer with or without the parens) - a bit annoying, but probably 
harmless enough to add the parens to let GCC users keep getting the warnings (& 
in that case - just means they'll be less likely to break LLVM self-hosting 
buildbots than if we disabled the GCC warning entirely)

> 2. Marking one method of an overload set virtual and implementing others in 
> terms of it is the most direct way to express what we're trying to do here. 
> (e.g. in Java which has no equivalent name-hiding pattern, nobody really 
> takes issue with this). Therefore if the name-hiding is harmless or 
> close-to-it, it's the clearest expression of this pattern.

If there wasn't any name hiding, I wouldn't have a problem with the code as-is. 
For me it's about the call-sites rather than the implementation. Looking at a 
derived class never gives you the whole picture of the class's API anyway.

> 3. The question of whether people like the concrete names or the overload set 
> chosen in ThreadsafeFS is irrelevant here.

Yep - agreed. That's a conversation for other reviews/threads.

> I think there's a fair amount of disagreement about (1), nobody really 
> presented a counterargument to (2), and for some reason we spent a lot of 
> time on (3) which seems to be a total bikeshed on a tangentially related 
> topic that had been settled in the usual way.

Appreciate the summary/thoughts!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82617



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

[PATCH] D82520: [Power10] Implement Vector Splat Immediate Builtins in LLVM/Clang

[Lei Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0c6b6e28e70c: [PowerPC] Implement Vector Splat Immediate 
Builtins in Clang (authored by biplmish, committed by lei).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D82520?vs=275790=275881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82520

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/test/CodeGen/PowerPC/p10-splatImm.ll

Index: llvm/test/CodeGen/PowerPC/p10-splatImm.ll
===
--- llvm/test/CodeGen/PowerPC/p10-splatImm.ll
+++ llvm/test/CodeGen/PowerPC/p10-splatImm.ll
@@ -286,3 +286,21 @@
 entry:
   ret double 0.00e+00
 }
+
+define dso_local <4 x i32> @vec_splati() local_unnamed_addr {
+; CHECK-LABEL: vec_splati:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxspltiw vs34, -17
+; CHECK-NEXT:blr
+entry:
+  ret <4 x i32> 
+}
+
+define dso_local <2 x double> @vec_splatid() local_unnamed_addr {
+; CHECK-LABEL: vec_splatid:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxspltidp vs34, 1065353216
+; CHECK-NEXT:blr
+entry:
+  ret <2 x double> 
+}
Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -512,3 +512,72 @@
   // CHECK-LE-NEXT: ret <4 x i32>
   return vec_inserth(vuia, vuib, uia);
 }
+
+vector signed int test_vec_vec_splati_si(void) {
+  // CHECK-BE: ret <4 x i32> 
+  // CHECK: ret <4 x i32> 
+  return vec_splati(-17);
+}
+
+vector unsigned int test_vec_vec_splati_ui(void) {
+  // CHECK-BE: ret <4 x i32> 
+  // CHECK: ret <4 x i32> 
+  return vec_splati(16U);
+}
+
+vector float test_vec_vec_splati_f(void) {
+  // CHECK-BE: ret <4 x float> 
+  // CHECK: ret <4 x float> 
+  return vec_splati(1.0f);
+}
+
+vector double test_vec_vec_splatid(void) {
+  // CHECK-BE: [[T1:%.+]] = fpext float %{{.+}} to double
+  // CHECK-BE-NEXT: [[T2:%.+]] = insertelement <2 x double> undef, double [[T1:%.+]], i32 0
+  // CHECK-BE-NEXT: [[T3:%.+]] = shufflevector <2 x double> [[T2:%.+]], <2 x double> undef, <2 x i32> zeroinitialize
+  // CHECK-BE-NEXT: ret <2 x double> [[T3:%.+]]
+  // CHECK: [[T1:%.+]] = fpext float %{{.+}} to double
+  // CHECK-NEXT: [[T2:%.+]] = insertelement <2 x double> undef, double [[T1:%.+]], i32 0
+  // CHECK-NEXT: [[T3:%.+]] = shufflevector <2 x double> [[T2:%.+]], <2 x double> undef, <2 x i32> zeroinitialize
+  // CHECK-NEXT: ret <2 x double> [[T3:%.+]]
+  return vec_splatid(1.0);
+}
+
+vector signed int test_vec_vec_splati_ins_si(void) {
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 %{{.+}}
+  // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK-BE: ret <4 x i32>
+  // CHECK:  [[T1:%.+]] = sub i32 1, %{{.+}}
+  // CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK:  [[T2:%.+]] = sub i32 3, %{{.+}}
+  // CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T2]]
+  // CHECK: ret <4 x i32>
+  return vec_splati_ins(vsia, 0, -17);
+}
+
+vector unsigned int test_vec_vec_splati_ins_ui(void) {
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 %{{.+}}
+  // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK-BE: ret <4 x i32>
+  // CHECK:  [[T1:%.+]] = sub i32 1, %{{.+}}
+  // CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK:  [[T2:%.+]] = sub i32 3, %{{.+}}
+  // CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T2]]
+  // CHECK: ret <4 x i32>
+  return vec_splati_ins(vuia, 1, 16U);
+}
+
+vector float test_vec_vec_splati_ins_f(void) {
+  // CHECK-BE: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 %{{.+}}
+  // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
+  // CHECK-BE: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 [[T1]]
+  // CHECK-BE: ret <4 x float>
+  // CHECK:  [[T1:%.+]] = sub i32 1, %{{.+}}
+  // CHECK: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 [[T1]]
+  // CHECK:  [[T2:%.+]] = sub i32 3, %{{.+}}
+  // CHECK: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 [[T2]]
+  // CHECK: ret <4 x float>
+  return vec_splati_ins(vfa, 0, 1.0f);
+}
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -17094,6 +17094,58 @@
vector unsigned long long __c) {
   return __builtin_vsx_xxblendvd(__a, __b, __c);
 }
+
+/* vec_splati */
+
+#define vec_splati(__a)\
+  _Generic((__a), signed int   

[clang] 0c6b6e2 - [PowerPC] Implement Vector Splat Immediate Builtins in Clang

[Lei Huang via cfe-commits]

Author: Biplob Mishra
Date: 2020-07-06T20:29:33-05:00
New Revision: 0c6b6e28e70c06a3cb4704d2d8f90829a689e230

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

LOG: [PowerPC] Implement Vector Splat Immediate Builtins in Clang

Implements builtins for the following prototypes:
  vector signed int vec_splati (const signed int);
  vector float vec_splati (const float);
  vector double vec_splatid (const float);
  vector signed int vec_splati_ins (vector signed int, const unsigned int,
const signed int);
  vector unsigned int vec_splati_ins (vector unsigned int, const unsigned int,
  const unsigned int);
  vector float vec_splati_ins (vector float, const unsigned int, const float);

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

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/test/CodeGen/PowerPC/p10-splatImm.ll

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index a63f2ee359fd..9a4009216930 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -17094,6 +17094,58 @@ vec_blendv(vector double __a, vector double __b,
vector unsigned long long __c) {
   return __builtin_vsx_xxblendvd(__a, __b, __c);
 }
+
+/* vec_splati */
+
+#define vec_splati(__a)
\
+  _Generic((__a), signed int   
\
+   : ((vector signed int)__a), unsigned int
\
+   : ((vector unsigned int)__a), float 
\
+   : ((vector float)__a))
+
+/* vec_spatid */
+
+static __inline__ vector double __ATTRS_o_ai vec_splatid(const float __a) {
+  return ((vector double)((double)__a));
+}
+
+/* vec_splati_ins */
+
+static __inline__ vector signed int __ATTRS_o_ai vec_splati_ins(
+vector signed int __a, const unsigned int __b, const signed int __c) {
+#ifdef __LITTLE_ENDIAN__
+  __a[1 - __b] = __c;
+  __a[3 - __b] = __c;
+#else
+  __a[__b] = __c;
+  __a[2 + __b] = __c;
+#endif
+  return __a;
+}
+
+static __inline__ vector unsigned int __ATTRS_o_ai vec_splati_ins(
+vector unsigned int __a, const unsigned int __b, const unsigned int __c) {
+#ifdef __LITTLE_ENDIAN__
+  __a[1 - __b] = __c;
+  __a[3 - __b] = __c;
+#else
+  __a[__b] = __c;
+  __a[2 + __b] = __c;
+#endif
+  return __a;
+}
+
+static __inline__ vector float __ATTRS_o_ai
+vec_splati_ins(vector float __a, const unsigned int __b, const float __c) {
+#ifdef __LITTLE_ENDIAN__
+  __a[1 - __b] = __c;
+  __a[3 - __b] = __c;
+#else
+  __a[__b] = __c;
+  __a[2 + __b] = __c;
+#endif
+  return __a;
+}
 #endif /* __VSX__ */
 #endif /* __POWER10_VECTOR__ */
 

diff  --git a/clang/test/CodeGen/builtins-ppc-p10vector.c 
b/clang/test/CodeGen/builtins-ppc-p10vector.c
index b0602dd66f53..22b4e7a6f3ec 100644
--- a/clang/test/CodeGen/builtins-ppc-p10vector.c
+++ b/clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -512,3 +512,72 @@ vector unsigned int test_vec_inserth_uiv(void) {
   // CHECK-LE-NEXT: ret <4 x i32>
   return vec_inserth(vuia, vuib, uia);
 }
+
+vector signed int test_vec_vec_splati_si(void) {
+  // CHECK-BE: ret <4 x i32> 
+  // CHECK: ret <4 x i32> 
+  return vec_splati(-17);
+}
+
+vector unsigned int test_vec_vec_splati_ui(void) {
+  // CHECK-BE: ret <4 x i32> 
+  // CHECK: ret <4 x i32> 
+  return vec_splati(16U);
+}
+
+vector float test_vec_vec_splati_f(void) {
+  // CHECK-BE: ret <4 x float> 
+  // CHECK: ret <4 x float> 
+  return vec_splati(1.0f);
+}
+
+vector double test_vec_vec_splatid(void) {
+  // CHECK-BE: [[T1:%.+]] = fpext float %{{.+}} to double
+  // CHECK-BE-NEXT: [[T2:%.+]] = insertelement <2 x double> undef, double 
[[T1:%.+]], i32 0
+  // CHECK-BE-NEXT: [[T3:%.+]] = shufflevector <2 x double> [[T2:%.+]], <2 x 
double> undef, <2 x i32> zeroinitialize
+  // CHECK-BE-NEXT: ret <2 x double> [[T3:%.+]]
+  // CHECK: [[T1:%.+]] = fpext float %{{.+}} to double
+  // CHECK-NEXT: [[T2:%.+]] = insertelement <2 x double> undef, double 
[[T1:%.+]], i32 0
+  // CHECK-NEXT: [[T3:%.+]] = shufflevector <2 x double> [[T2:%.+]], <2 x 
double> undef, <2 x i32> zeroinitialize
+  // CHECK-NEXT: ret <2 x double> [[T3:%.+]]
+  return vec_splatid(1.0);
+}
+
+vector signed int test_vec_vec_splati_ins_si(void) {
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 %{{.+}}
+  // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
+  // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK-BE: ret <4 x i32>
+  // CHECK:  [[T1:%.+]] = sub i32 1, %{{.+}}
+  // CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
+  // CHECK:  [[T2:%.+]] = sub i32 3, 

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

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



Comment at: llvm/utils/update_cc_test_checks.py:133
+  parser.add_argument('--include-generated-funcs', action='store_true',
+  help='Output checks for functions not in source')
   parser.add_argument('tests', nargs='+')

greened wrote:
> jdoerfert wrote:
> > I think this should go into common.py (after D78618). I would also make 
> > this the default but OK.
> Yes I suppose it should in case `opt` and friends generate functions.  I 
> hadn't considered that use-case.
> 
> While I would like to make it default unfortunately it would require updating 
> a bunch of the existing clang tests which doesn't seem too friendly.  See the 
> patch update comment for details.
> 
Just realized it wouldn't necessarily require regeneration of tests, it would 
just cause regenerated tests to change a lot when they are eventually 
regenerated.  We should discuss as to whether that's acceptable.  I think for 
now this should be non-default to at least get the functionality in without 
disturbing existing users and then we can discuss a separate change to make it 
default.

It's also possible we could change how clang orders functions.  I discovered 
there's a difference in clang 10 vs. 11 in the order functions are output when 
OpenMP outlining happens.  clang 10 seems to preserve the source order of 
functions and clang 11 does not.  Perhaps that needs to be fixed as I don't 
know whether that change was intentional or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004



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

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

greened added a comment.

In D83004#2134303 , @greened wrote:

>




> I don't particularly like this mode dichotomy but unifying it would 
> necessitate updating a whole lot of clang tests.

Axtually that's not strictly true,  It would just change the tests 
singificantly when they are updated for some other reason.  Whether that is 
reasonable is something we should discuss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004



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

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

I originally implemented this behavior behind `-fdebug-prefix-map` but there 
was some pushback, more context: 
https://lists.llvm.org/pipermail/cfe-dev/2020-June/065668.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154



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

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

GCC obtained -fdebug-prefix-map first, then r256847 added both 
-fmacro-prefix-map and -ffile-prefix-map. GCC doesn't have 
-fcoverage-prefix-map and I saw your https://github.com/apple/swift/pull/32416
However, do we really need one new -f*-prefix-map for every feature?

If you think `-fcoverage-prefix-map=` is really necessary, you can probably ask 
whether GCC can adopt a -fcoverage-prefix-map for their gcov (.gcno files). If 
they do, reviewers may have more have more confidence to say that we should 
have the option as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154



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

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, 
AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, 
yaxunl, jholewinski.
Herald added projects: clang, OpenMP, LLVM.

There are various runtime calls in the device runtime with unused, or
always fixed, arguments. This is bad for all sorts of reasons. Clean up
two before as we match them in OpenMPOpt now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83268

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/deviceRTLs/common/src/parallel.cu
  openmp/libomptarget/deviceRTLs/interface.h

Index: openmp/libomptarget/deviceRTLs/interface.h
===
--- openmp/libomptarget/deviceRTLs/interface.h
+++ openmp/libomptarget/deviceRTLs/interface.h
@@ -475,10 +475,8 @@
 int16_t RequiresDataSharing);
 EXTERN __attribute__((deprecated)) void __kmpc_spmd_kernel_deinit();
 EXTERN void __kmpc_spmd_kernel_deinit_v2(int16_t RequiresOMPRuntime);
-EXTERN void __kmpc_kernel_prepare_parallel(void *WorkFn,
-   int16_t IsOMPRuntimeInitialized);
-EXTERN bool __kmpc_kernel_parallel(void **WorkFn,
-   int16_t IsOMPRuntimeInitialized);
+EXTERN void __kmpc_kernel_prepare_parallel(void *WorkFn);
+EXTERN bool __kmpc_kernel_parallel(void **WorkFn);
 EXTERN void __kmpc_kernel_end_parallel();
 EXTERN bool __kmpc_kernel_convergent_parallel(void *buffer,
   __kmpc_impl_lanemask_t Mask,
Index: openmp/libomptarget/deviceRTLs/common/src/parallel.cu
===
--- openmp/libomptarget/deviceRTLs/common/src/parallel.cu
+++ openmp/libomptarget/deviceRTLs/common/src/parallel.cu
@@ -227,10 +227,8 @@
 }
 
 // This routine is always called by the team master..
-EXTERN void __kmpc_kernel_prepare_parallel(void *WorkFn,
-   int16_t IsOMPRuntimeInitialized) {
+EXTERN void __kmpc_kernel_prepare_parallel(void *WorkFn) {
   PRINT0(LD_IO, "call to __kmpc_kernel_prepare_parallel\n");
-  ASSERT0(LT_FUSSY, IsOMPRuntimeInitialized, "Expected initialized runtime.");
 
   omptarget_nvptx_workFn = WorkFn;
 
@@ -275,12 +273,9 @@
 // returns True if this thread is active, else False.
 //
 // Only the worker threads call this routine.
-EXTERN bool __kmpc_kernel_parallel(void **WorkFn,
-   int16_t IsOMPRuntimeInitialized) {
+EXTERN bool __kmpc_kernel_parallel(void **WorkFn) {
   PRINT0(LD_IO | LD_PAR, "call to __kmpc_kernel_parallel\n");
 
-  ASSERT0(LT_FUSSY, IsOMPRuntimeInitialized, "Expected initialized runtime.");
-
   // Work function and arguments for L1 parallel region.
   *WorkFn = omptarget_nvptx_workFn;
 
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -584,6 +584,11 @@
 __OMP_RTL(__kmpc_task_allow_completion_event, false, VoidPtr, IdentPtr,
   /* Int */ Int32, /* kmp_task_t */ VoidPtr)
 
+/// Note that device runtime functions (in the following) do not necessarily
+/// need attributes as we expect to see the definitions.
+__OMP_RTL(__kmpc_kernel_parallel, false, Int1, VoidPtrPtr)
+__OMP_RTL(__kmpc_kernel_prepare_parallel, false, Void, VoidPtr)
+
 __OMP_RTL(__last, false, Void, )
 
 #undef __OMP_RTL
Index: clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
===
--- clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
+++ clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
@@ -88,7 +88,7 @@
   // CHECK: [[I_ADDR:%.+]] = getelementptr inbounds [[GLOB_TY]], [[GLOB_TY]]* [[RD]], i32 0, i32 0
   //
   // CHECK: call void @__kmpc_for_static_init_4(
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32)* @{{.+}} to i8*), i16 1)
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32)* @{{.+}} to i8*))
   // CHECK: call void @__kmpc_begin_sharing_variables(i8*** [[SHARED_VARS_PTR:%.+]], i{{64|32}} 1)
   // CHECK: [[SHARED_VARS_BUF:%.+]] = load i8**, i8*** [[SHARED_VARS_PTR]],
   // CHECK: [[VARS_BUF:%.+]] = getelementptr inbounds i8*, i8** [[SHARED_VARS_BUF]], i{{64|32}} 0
Index: clang/test/OpenMP/nvptx_target_teams_codegen.cpp
===

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D82611#2133521 , @erik.pilkington 
wrote:

> In D82611#2125868 , @MadCoder wrote:
>
> >
>




>> I would suggest something like `-Wstrict-direct-dispatch` or something.
> 
> I kinda prefer `-Wpotentially-direct-selector`, since that seems to more 
> closely correspond to what the compiler is complaining about. WDYT?

Yes, that is better, I wasn't dead-set on the name.


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

https://reviews.llvm.org/D82611



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

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 275866.
oontvoo added a comment.

add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/trivial-abi-templated-type.cpp


Index: clang/test/SemaCXX/trivial-abi-templated-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/trivial-abi-templated-type.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-no-diagnostics
+
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6591,7 +6591,7 @@
   }
 
   // See if trivial_abi has to be dropped.
-  if (Record->hasAttr())
+  if (!Record->isDependentType() && Record->hasAttr())
 checkIllFormedTrivialABIStruct(*Record);
 
   // Set HasTrivialSpecialMemberForCall if the record has attribute


Index: clang/test/SemaCXX/trivial-abi-templated-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/trivial-abi-templated-type.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+// expected-no-diagnostics
+
+template 
+class __attribute__((trivial_abi)) a { a(a &&); };
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6591,7 +6591,7 @@
   }
 
   // See if trivial_abi has to be dropped.
-  if (Record->hasAttr())
+  if (!Record->isDependentType() && Record->hasAttr())
 checkIllFormedTrivialABIStruct(*Record);
 
   // Set HasTrivialSpecialMemberForCall if the record has attribute
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D83008#2131776 , @teemperor wrote:

> Thanks for tracking this down, this is a really nasty bug...
>
> The fix itself is obviously fine, but I think I'm out of the loop regarding 
> the testing strategy. We talked about adding a Clang test for this with the 
> help of this layout overwrite JSON file. I assume that extending this to 
> cover virtual bases turned out to be more complicated than expected? FWIW, 
> I'm not necessarily the biggest fan of this Clang test option so I would be 
> fine if we leave it as-is.
>
> I think having an LLDB test is a good idea, but it's not clear why it's a 
> Shell test. If I understand correctly this test requires running on arm64 
> (so, a remote test target), but from what I remember all the remote platform 
> support is only in dotest.py? Also pretty much all other expression 
> evaluation tests and the utilities for that are in the Python/API test 
> infrastructure, so it's a bit out of place.
>
> Also I think the test can be in general much simpler than an arm64-specific 
> test. We get all base class offsets wrong in LLDB, so we can just write a 
> simple test where you change the layout of some structs in a way that it 
> doesn't fit the default layout. E.g., just putting `alignas` on a base class 
> and then reading fields should be enough to trigger the same bug.


Good idea using `alignas` that actually did the trick, I was having trouble 
getting this to reproduce otherwise. I added a second test which should also 
reproduce on non-arm64 cases but I will keep the original test as well since 
they are hitting the bug from slightly different paths.

The goal of using a shell test was to avoid writing a device specific test and 
even though the first case should also pass on all other platforms.


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

https://reviews.llvm.org/D83008



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

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 275865.
shafik added a comment.

Adding a second test that is not arm64 specific.


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

https://reviews.llvm.org/D83008

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  lldb/test/Shell/Expr/Inputs/layout.cpp
  lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test


Index: lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
@@ -0,0 +1,11 @@
+# RUN: %clangxx_host %p/Inputs/layout.cpp -g -o %t
+
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $0 = 12
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $1 = 8
Index: lldb/test/Shell/Expr/Inputs/layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/layout.cpp
@@ -0,0 +1,43 @@
+#include 
+
+struct B1 {
+  uint8_t a;
+};
+
+struct D1 : public B1 {
+  uint8_t a;
+  uint32_t ID;
+  uint8_t b;
+};
+
+struct Mixin : public D1 {
+  uint8_t a;
+  uint32_t *arr[3];
+};
+
+struct B2 {
+  uint32_t a;
+};
+
+class D2 : public B2, public Mixin {};
+
+struct B3 {
+  char f1;
+};
+
+struct alignas(8) B4 {
+  char f2;
+};
+
+struct D3 : B3, B4 {
+};
+
+D2 d2g;
+D3 d3g;
+
+int main() {
+  D2 d2;
+  D3 d3;
+
+  return d2.ID + d3.f2;
+}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1187,11 +1187,10 @@
   // Query the external layout to see if it provides an offset.
   bool HasExternalLayout = false;
   if (UseExternalLayout) {
-// FIXME: This appears to be reversed.
 if (Base->IsVirtual)
-  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, 
Offset);
-else
   HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
+else
+  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, 
Offset);
   }
 
   // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.


Index: lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
===
--- /dev/null
+++ lldb/test/Shell/Expr/TestLayoutNonVirtualBaseClasses.test
@@ -0,0 +1,11 @@
+# RUN: %clangxx_host %p/Inputs/layout.cpp -g -o %t
+
+# RUN: %lldb %t -b -s %s | FileCheck %s
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $0 = 12
+
+expr (intptr_t) - (intptr_t)
+# CHECK: (lldb) expr (intptr_t) - (intptr_t)
+# CHECK: (long) $1 = 8
Index: lldb/test/Shell/Expr/Inputs/layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/Expr/Inputs/layout.cpp
@@ -0,0 +1,43 @@
+#include 
+
+struct B1 {
+  uint8_t a;
+};
+
+struct D1 : public B1 {
+  uint8_t a;
+  uint32_t ID;
+  uint8_t b;
+};
+
+struct Mixin : public D1 {
+  uint8_t a;
+  uint32_t *arr[3];
+};
+
+struct B2 {
+  uint32_t a;
+};
+
+class D2 : public B2, public Mixin {};
+
+struct B3 {
+  char f1;
+};
+
+struct alignas(8) B4 {
+  char f2;
+};
+
+struct D3 : B3, B4 {
+};
+
+D2 d2g;
+D3 d3g;
+
+int main() {
+  D2 d2;
+  D3 d3;
+
+  return d2.ID + d3.f2;
+}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1187,11 +1187,10 @@
   // Query the external layout to see if it provides an offset.
   bool HasExternalLayout = false;
   if (UseExternalLayout) {
-// FIXME: This appears to be reversed.
 if (Base->IsVirtual)
-  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
-else
   HasExternalLayout = External.getExternalVBaseOffset(Base->Class, Offset);
+else
+  HasExternalLayout = External.getExternalNVBaseOffset(Base->Class, Offset);
   }
 
   // Clang <= 6 incorrectly applied the 'packed' attribute to base classes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3c7e8d6 - Fix sdk version test to use 99.99.99 as a max dummy version instead of 10.99.99.

[Amara Emerson via cfe-commits]

Author: Amara Emerson
Date: 2020-07-06T16:53:12-07:00
New Revision: 3c7e8d6d0eb0660fb8fbae98c3e49ca059943416

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

LOG: Fix sdk version test to use 99.99.99 as a max dummy version instead of 
10.99.99.

Was failing on macOS 11 hosts which is > 10.99.99

Added: 


Modified: 
clang/test/Driver/darwin-sdk-vs-os-version.c

Removed: 




diff  --git a/clang/test/Driver/darwin-sdk-vs-os-version.c 
b/clang/test/Driver/darwin-sdk-vs-os-version.c
index 391f4d5a7305..94e52a9036ed 100644
--- a/clang/test/Driver/darwin-sdk-vs-os-version.c
+++ b/clang/test/Driver/darwin-sdk-vs-os-version.c
@@ -2,9 +2,9 @@
 
 // Ensure that we never pick a version that's based on the SDK that's newer 
than
 // the system version:
-// RUN: rm -rf %t/SDKs/MacOSX10.99.99.sdk
-// RUN: mkdir -p %t/SDKs/MacOSX10.99.99.sdk
-// RUN: %clang -target x86_64-apple-darwin -isysroot 
%t/SDKs/MacOSX10.99.99.sdk %s -### 2>&1 \
+// RUN: rm -rf %t/SDKs/MacOSX99.99.99.sdk
+// RUN: mkdir -p %t/SDKs/MacOSX99.99.99.sdk
+// RUN: %clang -target x86_64-apple-darwin -isysroot 
%t/SDKs/MacOSX99.99.99.sdk %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MACOSX-SYSTEM-VERSION %s
 
-// CHECK-MACOSX-SYSTEM-VERSION-NOT: 10.99.99"
+// CHECK-MACOSX-SYSTEM-VERSION-NOT: 99.99.99"



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666
+  /// HasNonEmptyBaseClass - Whether the class has any non-empty class (in the
+  /// sense of (C++11 [meta.unary.prop])) as base.
+  bool HasNonEmptyBaseClass;

Minor nit: s/as base/as a base/;



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:707
+HandledFirstNonOverlappingEmptyField(false),
+   HasNonEmptyBaseClass(false),
+FirstNearlyEmptyVBase(nullptr) {}

Minor nit: Please fix the formatting.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1225
+   Context.getTargetInfo().getTriple().isPS4() ||
+   Context.getTargetInfo().getTriple().isOSAIX()))
+   ? CharUnits::One()

Thanks; verified that this is correct with `xlclang++` from IBM XL C/C++ for 
AIX with:
```
struct A {
  char x;
};
struct B {
  int x;
};
struct __attribute__((__packed__)) C : A, B {} c;
```

Length is 5:
```
[10]m   0x0004  .bss 1  externc
[11]a4  0x0005   00 CM   RW00
```

@Xiangling_L, I suggest adding a case for this to the tests.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1256
+  // space or zero-extent array.
+  if (DefaultsToAIXPowerAlignment && !(getDataSize().isZero() || IsUnion)) {
+PreferredBaseAlign = BaseAlign;

Unions cannot have base classes. Please assert `!IsUnion`.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1260
+
+  // The maximum field alignment overrides base align/preferred base align(AIX
+  // only).

Suggestion:
```
  // The maximum field alignment overrides the base align/(AIX-only) preferred
  // base align.
```



Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:18
+
+// CHECK-DAG: @_ZN5test11aE = global i32 2, align 4
+

Instead of checking the value of `a`, the alignment can be checked more 
directly from the IR for `cc`.


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

https://reviews.llvm.org/D79719



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

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

@ckandeler do you have commit access or do you want me to land the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82629



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

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

@milianw I just approved https://reviews.llvm.org/D82629 - I feel like that 
patch is addressing the core issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82740



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Johel Ernesto Guerrero Peña via Phabricator via cfe-commits]

JohelEGP added a comment.

Yes, it's valid C++ (is my hope). It's from WIP code, which I made available to 
show you: https://github.com/johelegp/jge.

Here it is building, running and passing the tests: 
https://travis-ci.com/github/johelegp/jge/jobs/358137355#L559.

Here are links to the above linked snippets:

- Now looking good:
  - 
https://github.com/johelegp/jge/blob/master/include/jge/cartesian.hpp#L84-L90
  - https://github.com/johelegp/jge/blob/master/include/jge/plane.hpp#L54-L59
  - https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L18-L20.
- Just reported:
  - Inheritance: 
https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L11-L21 
(https://reviews.llvm.org/D79773#2131680)
  - Non list-initialization: 
https://github.com/johelegp/jge/blob/master/tests/jge/cartesian.cpp#L11-L16 
(https://reviews.llvm.org/D79773#2132086)
  - For the `EqualityComparable` case 
(https://reviews.llvm.org/D79773#2131680), I use this macro: 
https://github.com/johelegp/jge/blob/master/include/jge/detail/quantity_wrappers.hpp#L4-L5.
 Example: 
https://github.com/johelegp/jge/blob/master/include/jge/detail/quantity_wrappers.hpp#L59.
 I also used it before the previous fixes besides `//` comments.


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

https://reviews.llvm.org/D79773



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

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

[Jan Korous via Phabricator via cfe-commits]

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

LGTM! Thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82629



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

[PATCH] D82800: [OPENMP50] extend array section for stride (Parsing/Sema/AST)

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 275856.
cchen added a comment.

Fix based on comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82800

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/target_data_messages.c
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_parallel_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_depend_messages.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/task_affinity_messages.cpp
  clang/test/OpenMP/task_depend_messages.cpp

Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -52,7 +52,7 @@
   #pragma omp task depend (in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
   #pragma omp task depend (in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp task depend (in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
-  #pragma omp task depend (in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
+  #pragma omp task depend (in : argv[3:4:1]) // omp50-error {{stride can only be specified in to or from clause}} omp45-error {{expected ']'}} omp45-note {{to match this '['}}
   #pragma omp task depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
   #pragma omp task depend(in:argv[argv[:2]:1]) // expected-error {{OpenMP array section is not allowed here}}
   #pragma omp task depend(in:argv[0:][:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
Index: clang/test/OpenMP/task_affinity_messages.cpp
===
--- clang/test/OpenMP/task_affinity_messages.cpp
+++ clang/test/OpenMP/task_affinity_messages.cpp
@@ -44,7 +44,7 @@
   #pragma omp task affinity (argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
   #pragma omp task affinity (argv[-1:0])
   #pragma omp task affinity (argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
-  #pragma omp task affinity (argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
+  #pragma omp task affinity (argv[3:4:1]) // expected-error {{stride can only be specified in to or from clause}}
   #pragma omp task affinity(a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
   #pragma omp task affinity(argv[argv[:2]:1]) // expected-error {{OpenMP array section is not allowed here}}
   #pragma omp task affinity(argv[0:][:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repro:

- Annotate the unique_ptr class with trivial_abi (eg., 
https://godbolt.org/z/-rprsc)
- ./build/bin/clang++ -stdlib=libc++ -I../libcxx/memory

Crash:

  @ 0x559d129463fc  
clang::CXXRecordDecl::defaultedCopyConstructorIsDeleted()
   @ 0x559d1288d3e5  
clang::Sema::checkIllFormedTrivialABIStruct()::$_7::operator()()
   @ 0x559d12884c34  clang::Sema::checkIllFormedTrivialABIStruct()
   @ 0x559d1288412e  clang::Sema::CheckCompletedCXXClass()
   @ 0x559d1288d843  clang::Sema::ActOnFinishCXXMemberSpecification()
   @ 0x559d12020109  clang::Parser::ParseCXXMemberSpecification()
   @ 0x559d1201e80c  clang::Parser::ParseClassSpecifier()
   @ 0x559d1204e807  clang::Parser::ParseDeclarationSpecifiers()
   @ 0x559d120e9aa9  clang::Parser::ParseSingleDeclarationAfterTemplate()
   @ 0x559d120e8f21  
clang::Parser::ParseTemplateDeclarationOrSpecialization()
   @ 0x559d120e8886  clang::Parser::ParseDeclarationStartingWithTemplate()
   @ 0x559d1204a1d4  clang::Parser::ParseDeclaration()
   @ 0x559d12004b1d  clang::Parser::ParseExternalDeclaration()
   @ 0x559d12017689  clang::Parser::ParseInnerNamespace()
   @ 0x559d12017024  clang::Parser::ParseNamespace()
   @ 0x559d1204a29b  clang::Parser::ParseDeclaration()
   @ 0x559d12004c74  clang::Parser::ParseExternalDeclaration()


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83263

Files:
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9719,6 +9719,11 @@
 
   // Ill-formed if the copy and move constructors are deleted.
   auto HasNonDeletedCopyOrMoveConstructor = [&]() {
+// If we know there exist users-defined move/copy ctors, don't try to infer
+// the deletion of implicit ones becausee Sema may not have the info yet.
+if (RD.hasUserDeclaredMoveConstructor() ||
+RD.hasUserDeclaredCopyConstructor())
+  return true;
 if (RD.needsImplicitCopyConstructor() &&
 !RD.defaultedCopyConstructorIsDeleted())
   return true;


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9719,6 +9719,11 @@
 
   // Ill-formed if the copy and move constructors are deleted.
   auto HasNonDeletedCopyOrMoveConstructor = [&]() {
+// If we know there exist users-defined move/copy ctors, don't try to infer
+// the deletion of implicit ones becausee Sema may not have the info yet.
+if (RD.hasUserDeclaredMoveConstructor() ||
+RD.hasUserDeclaredCopyConstructor())
+  return true;
 if (RD.needsImplicitCopyConstructor() &&
 !RD.defaultedCopyConstructorIsDeleted())
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c359c5d - [X86] Centalize the 'sse4' hack to a single place in X86TargetInfo::setFeatureEnabledImpl. NFCI

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-07-06T15:00:32-07:00
New Revision: c359c5d534429c96f1cebdf8d845b8120e9c2ef0

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

LOG: [X86] Centalize the 'sse4' hack to a single place in 
X86TargetInfo::setFeatureEnabledImpl. NFCI

Instead of detecting the string in 2 places. Just swap the string
to 'sse4.1' or 'sse4.2' at the top of the function.

Prep work for a patch to switch the rest of this function to a
table based system. And I don't want to include 'sse4a' in the
table.

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 30f4570ecc02..2c6742b9042a 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -295,11 +295,18 @@ void X86TargetInfo::setXOPLevel(llvm::StringMap 
, XOPEnum Level,
 
 void X86TargetInfo::setFeatureEnabledImpl(llvm::StringMap ,
   StringRef Name, bool Enabled) {
-  // This is a bit of a hack to deal with the sse4 target feature when used
-  // as part of the target attribute. We handle sse4 correctly everywhere
-  // else. See below for more information on how we handle the sse4 options.
-  if (Name != "sse4")
-Features[Name] = Enabled;
+  if (Name == "sse4") {
+// We can get here via the __target__ attribute since that's not controlled
+// via the -msse4/-mno-sse4 command line alias. Handle this the same way
+// here - turn on the sse4.2 if enabled, turn off the sse4.1 level if
+// disabled.
+if (Enabled)
+  Name = "sse4.2";
+else
+  Name = "sse4.1";
+  }
+
+  Features[Name] = Enabled;
 
   if (Name == "mmx") {
 setMMXLevel(Features, MMX, Enabled);
@@ -381,15 +388,6 @@ void 
X86TargetInfo::setFeatureEnabledImpl(llvm::StringMap ,
   } else if (Name == "sha") {
 if (Enabled)
   setSSELevel(Features, SSE2, Enabled);
-  } else if (Name == "sse4") {
-// We can get here via the __target__ attribute since that's not controlled
-// via the -msse4/-mno-sse4 command line alias. Handle this the same way
-// here - turn on the sse4.2 if enabled, turn off the sse4.1 level if
-// disabled.
-if (Enabled)
-  setSSELevel(Features, SSE42, Enabled);
-else
-  setSSELevel(Features, SSE41, Enabled);
   } else if (Name == "xsave") {
 if (!Enabled)
   Features["xsaveopt"] = Features["xsavec"] = Features["xsaves"] = false;



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

[PATCH] D83055: [clang][Driver] Fix tool path priority test failure

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The description appears to be wrapped at a very small column number. Can you 
please fix it?




Comment at: clang/test/Driver/program-path-priority.c:34
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{\/gcc[^\/]*"}}
 

`\` can be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83055



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

[PATCH] D82767: clang-format: Explicitly use python3

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a subscriber: arsen.
compnerd added a comment.

Thinking a bit more about this, using `/usr/bin/env python` ensures that we 
actually honour the python version that the user specifies because that allows 
the user control over the python symlink pointing to either python2 or python3. 
 @arsen what do you think of just unifying on `/usr/bin/env python` across all 
the scripts?


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

https://reviews.llvm.org/D82767



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2409
+const RecordDecl *RD = RT->getDecl();
+return std::max(ABIAlign, static_cast(toBits(
+  getASTRecordLayout(RD).PreferredAlignment)));

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Please add a comment regarding the situations where the `ABIAlign` value is 
> > greater than the `PreferredAlignment` value. It may be appropriate to 
> > assert that, absent those cases, the `PreferredAlignment` value is at least 
> > that of `ABIAlign`.
> It does not appear that the maximum of the two values is the correct answer:
> ```
> struct C {
>   double x;
> } c;
> typedef struct C __attribute__((__aligned__(2))) CC;
> 
> CC cc;
> extern char x[__alignof__(cc)];
> extern char x[2]; // this is okay with IBM XL C/C++
> ```
> Please add a comment regarding the situations where the ABIAlign value is 
> greater than the PreferredAlignment value.

I added a `if` condition to guard the situation where `ABIAlign` should be 
returned instead of adding a comment. Please let me know if that is sufficient.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1888
+BTy->getKind() == BuiltinType::LongDouble) {
+  PreferredAlign = CharUnits::fromQuantity(8);
+}

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > I believe an assertion that `PreferredAlign` was 4 would be appropriate.
> It seems that overriding the value should only be done after additional 
> checks:
> ```
> typedef double __attribute__((__aligned__(2))) Dbl;
> struct A {
>   Dbl x;
> } a;
> extern char x[__alignof__(a)];
> extern char x[2]; // this is okay with IBM XL C/C++
> ```
> 
> I am getting concerned that the logic here overlaps quite a bit with 
> `getPreferredTypeAlign` and refactoring to make the code here more common 
> with `getPreferredTypeAlign` is necessary.
Fixed the typedef related cases with my new changes, and the overlaps were not 
a lot as I expected. So I didn't do any refactoring yet. Please let me know if 
you still think it's necessary to refactor the code somehow.


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

https://reviews.llvm.org/D79719



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 275840.
Xiangling_L marked 6 inline comments as done.
Xiangling_L added a comment.

Fixed the `typedef` related issues;
Added more testcases;


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

https://reviews.llvm.org/D79719

Files:
  clang/include/clang/AST/RecordLayout.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayout.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.h
  clang/test/Layout/aix-Wpacked.cpp
  clang/test/Layout/aix-double-struct-member.cpp
  clang/test/Layout/aix-no-unique-address-with-double.cpp
  clang/test/Layout/aix-power-alignment-typedef.cpp
  clang/test/Layout/aix-virtual-function-and-base-with-double.cpp

Index: clang/test/Layout/aix-virtual-function-and-base-with-double.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-virtual-function-and-base-with-double.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc64-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+struct A {
+  double d1;
+  virtual void boo() {}
+};
+
+struct B {
+  double d2;
+  A a;
+};
+
+struct C : public A {
+  double d3;
+};
+
+int i = sizeof(B);
+int j = sizeof(C);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::A
+// CHECK-NEXT:0 |   (A vtable pointer)
+// CHECK32-NEXT:  4 |   double d1
+// CHECK32-NEXT:| [sizeof=12, dsize=12, align=4, preferredalign=4,
+// CHECK32-NEXT:|  nvsize=12, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:  8 |   double d1
+// CHECK64-NEXT:| [sizeof=16, dsize=16, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=16, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::B
+// CHECK-NEXT:0 |   double d2
+// CHECK-NEXT:8 |   struct test1::A a
+// CHECK-NEXT:8 | (A vtable pointer)
+// CHECK32-NEXT: 12 | double d1
+// CHECK32-NEXT:| [sizeof=24, dsize=20, align=4, preferredalign=8,
+// CHECK32-NEXT:|  nvsize=20, nvalign=4, preferrednvalign=8]
+// CHECK64-NEXT: 16 | double d1
+// CHECK64-NEXT:| [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=24, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::C
+// CHECK-NEXT:0 |   struct test1::A (primary base)
+// CHECK-NEXT:0 | (A vtable pointer)
+// CHECK32-NEXT:  4 | double d1
+// CHECK32-NEXT: 12 |   double d3
+// CHECK32-NEXT:| [sizeof=20, dsize=20, align=4, preferredalign=4,
+// CHECK32-NEXT:|  nvsize=20, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:  8 | double d1
+// CHECK64-NEXT: 16 |   double d3
+// CHECK64-NEXT:| [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=24, nvalign=8, preferrednvalign=8]
+
+} // namespace test1
+
+namespace test2 {
+struct A {
+  long long l1;
+};
+
+struct B : public virtual A {
+  double d2;
+};
+
+#pragma pack(2)
+struct C : public virtual A {
+  double __attribute__((aligned(4))) d3;
+};
+
+int i = sizeof(B);
+int j = sizeof(C);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::A
+// CHECK-NEXT:0 |   long long l1
+// CHECK-NEXT:  | [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK-NEXT:  |  nvsize=8, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::B
+// CHECK-NEXT:0 |   (B vtable pointer)
+// CHECK32-NEXT:  4 |   double d2
+// CHECK64-NEXT:  8 |   double d2
+// CHECK-NEXT:   16 |   struct test2::A (virtual base)
+// CHECK-NEXT:   16 | long long l1
+// CHECK-NEXT:  | [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK32-NEXT:|  nvsize=12, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:|  nvsize=16, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::C
+// CHECK-NEXT:0 |   (C vtable pointer)
+// CHECK32-NEXT:  4 |   double d3
+// CHECK32-NEXT: 12 |   struct test2::A (virtual base)
+// CHECK32-NEXT: 12 | long long l1
+// CHECK32-NEXT:| [sizeof=20, 

[PATCH] D83250: [clang] Enable errors for undefined TARGET_OS_ macros in Darwin driver

[Zixu Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf47b8851318d: [clang] Enable errors for undefined TARGET_OS_ 
macros in Darwin driver (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83250

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-warning-options.c


Index: clang/test/Driver/darwin-warning-options.c
===
--- /dev/null
+++ clang/test/Driver/darwin-warning-options.c
@@ -0,0 +1,7 @@
+// REQUIRES: system-darwin
+
+// Always error about undefined 'TARGET_OS_*' macros on Darwin.
+// RUN: %clang -### %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: "-Wundef-prefix=TARGET_OS_"
+// CHECK-DAG: "-Werror=undef-prefix"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -954,6 +954,10 @@
 : Darwin(D, Triple, Args) {}
 
 void DarwinClang::addClangWarningOptions(ArgStringList ) const {
+  // Always error about undefined 'TARGET_OS_*' macros.
+  CC1Args.push_back("-Wundef-prefix=TARGET_OS_");
+  CC1Args.push_back("-Werror=undef-prefix");
+
   // For modern targets, promote certain warnings to errors.
   if (isTargetWatchOSBased() || getTriple().isArch64Bit()) {
 // Always enable -Wdeprecated-objc-isa-usage and promote it


Index: clang/test/Driver/darwin-warning-options.c
===
--- /dev/null
+++ clang/test/Driver/darwin-warning-options.c
@@ -0,0 +1,7 @@
+// REQUIRES: system-darwin
+
+// Always error about undefined 'TARGET_OS_*' macros on Darwin.
+// RUN: %clang -### %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: "-Wundef-prefix=TARGET_OS_"
+// CHECK-DAG: "-Werror=undef-prefix"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -954,6 +954,10 @@
 : Darwin(D, Triple, Args) {}
 
 void DarwinClang::addClangWarningOptions(ArgStringList ) const {
+  // Always error about undefined 'TARGET_OS_*' macros.
+  CC1Args.push_back("-Wundef-prefix=TARGET_OS_");
+  CC1Args.push_back("-Werror=undef-prefix");
+
   // For modern targets, promote certain warnings to errors.
   if (isTargetWatchOSBased() || getTriple().isArch64Bit()) {
 // Always enable -Wdeprecated-objc-isa-usage and promote it
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f47b885 - [clang] Enable errors for undefined TARGET_OS_ macros in Darwin driver

[Zixu Wang via cfe-commits]

Author: Zixu Wang
Date: 2020-07-06T14:52:12-07:00
New Revision: f47b8851318d5ec2fa1e7867f3fdb86101cdc1da

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

LOG: [clang] Enable errors for undefined TARGET_OS_ macros in Darwin driver

Add clang option `-Wundef-prefix=TARGET_OS_` and `-Werror=undef-prefix`
to Darwin driver.

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

Added: 
clang/test/Driver/darwin-warning-options.c

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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index ad3b3a955d42..6bf42e6029eb 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -954,6 +954,10 @@ DarwinClang::DarwinClang(const Driver , const 
llvm::Triple ,
 : Darwin(D, Triple, Args) {}
 
 void DarwinClang::addClangWarningOptions(ArgStringList ) const {
+  // Always error about undefined 'TARGET_OS_*' macros.
+  CC1Args.push_back("-Wundef-prefix=TARGET_OS_");
+  CC1Args.push_back("-Werror=undef-prefix");
+
   // For modern targets, promote certain warnings to errors.
   if (isTargetWatchOSBased() || getTriple().isArch64Bit()) {
 // Always enable -Wdeprecated-objc-isa-usage and promote it

diff  --git a/clang/test/Driver/darwin-warning-options.c 
b/clang/test/Driver/darwin-warning-options.c
new file mode 100644
index ..b0a591eac820
--- /dev/null
+++ b/clang/test/Driver/darwin-warning-options.c
@@ -0,0 +1,7 @@
+// REQUIRES: system-darwin
+
+// Always error about undefined 'TARGET_OS_*' macros on Darwin.
+// RUN: %clang -### %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: "-Wundef-prefix=TARGET_OS_"
+// CHECK-DAG: "-Werror=undef-prefix"



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

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D75591#2134301 , @atmnpatel wrote:

> Ping @ABataev.


Rebase, please


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591



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

[PATCH] D82800: [OPENMP50] extend array section for stride (Parsing/Sema/AST)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:1944-1951
+  if (StrideExpr && !StrideExpr->isValueDependent() &&
+  (!StrideExpr->isEvaluatable(Actions.getASTContext()) ||
+   (StrideExpr->EvaluateAsInt(Result, Actions.getASTContext()) 
&&
+!Result.Val.getInt().isOneValue( {
+Diag(Tok, diag::err_omp_array_section_stride_only_zero_or_one)
+  << getOpenMPClauseName(OMPClauseKind)
+  << StrideExpr->getSourceRange();

No, for non-to/from clauses the stride expression should not be allowed at all 
if I read the standard correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82800



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

[PATCH] D83190: [analyzer] Model iterator random incrementation symmetrically

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 5 inline comments as done.
gamesh411 added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:282
+SVal  = IsItOnLHS ? RVal : LVal;
+handlePtrIncrOrDecr(C, ItExpr, BinaryOperator::getOverloadedOperator(OK),
+OffsetVal);

baloghadamsoftware wrote:
> gamesh411 wrote:
> > During the development of this patch, I saw something related. At the 
> > beginning of handlePtrIncrOrDecr, there is a branch on whether the Expr 
> > (2nd argument) is a pointer. I think that branch could just be an 
> > assertion. What do you think? (or maybe I should create a patch to show 
> > what I mean?)
> I wonder whether this should be implemented here in `checkPostStmt()` ot in 
> `handlePtrIncrOrDecr()`. Your current implementation is in `checkPostStmt()`, 
> in this case we can assert in `handlePtrIncrOrDecl()`.
I checked and decided it not really worth it to move the logic inside the 
`handlePtrIncrOrDecr()` function, as that would require us to pass both the 
left and right-hand side as `Expr`-s. This would be fine when we handle a 
binary operator, but if we handle a unary operator, we manually pass the SVal 
of RHS (namely a manually created ConcreteInt with value 1). All this could be 
abstracted with a function wrapping the original `handlePtrIncrOrDecr()`, but 
for now, I don't think it is worth it.



Comment at: clang/test/Analysis/iterator-modeling.cpp:169
+
+  auto i2 = 2 + i1;
+

Note that this line requires the parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83190



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

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

greened added a comment.

In D83004#2129362 , @jdoerfert wrote:

> This is great! Just a few days ago I added a TODO in one of the tests I 
> created asking for this: D82722  :)


Glad to help!

> Will this work for all test scripts?

Obviously right now it's only enabled for clang but it should be 
straightforward to add to the other test scripts.  Al the infrastructure is 
there, the various drivers just have to use it.

> Will this make the `prefix_exclusions` logic obsolete?

Yeah, I think it might.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004



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

[PATCH] D83190: [analyzer] Model iterator random incrementation symmetrically

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 275835.
gamesh411 added a comment.

implement traditional iterator support as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83190

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/iterator-modeling.cpp

Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -149,7 +149,7 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end(){{$
 }
 
-void plus(const std::vector ) {
+void lhs_plus(const std::vector ) {
   auto i1 = v.begin();
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
@@ -161,7 +161,19 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re{{$v.begin() + 2{{$
 }
 
-void plus_negative(const std::vector ) {
+void rhs_plus(const std::vector ) {
+  auto i1 = v.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
+
+  auto i2 = 2 + i1;
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i2) == ); // expected-warning{{TRUE}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re{{$v.begin(){{$
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re{{$v.begin() + 2{{$
+}
+
+void lhs_plus_negative(const std::vector ) {
   auto i1 = v.end();
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
@@ -173,6 +185,18 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end() - 2{{$
 }
 
+void rhs_plus_negative(const std::vector ) {
+  auto i1 = v.end();
+
+  clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
+
+  auto i2 = (-2) + i1;
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i2) == ); // expected-warning{{TRUE}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$v.end(){{$
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$v.end() - 2{{$
+}
+
 void minus(const std::vector ) {
   auto i1 = v.end();
 
@@ -1948,7 +1972,7 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i)); // expected-warning{{$c.end() - 2}}
 }
 
-void plus_ptr_iterator(const cont_with_ptr_iterator ) {
+void lhs_plus_ptr_iterator(const cont_with_ptr_iterator ) {
   auto i1 = c.begin();
 
   clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
@@ -1960,6 +1984,18 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.begin() + 2}}
 }
 
+void rhs_plus_ptr_iterator(const cont_with_ptr_iterator ) {
+  auto i1 = c.begin();
+
+  clang_analyzer_denote(clang_analyzer_container_begin(c), "$c.begin()");
+
+  auto i2 = 2 + i1;
+
+  clang_analyzer_eval(clang_analyzer_iterator_container(i2) == ); // expected-warning{{TRUE}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$c.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.begin() + 2}}
+}
+
 void minus_ptr_iterator(const cont_with_ptr_iterator ) {
   auto i1 = c.end();
 
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -109,7 +109,7 @@
bool Postfix) const;
   void handleRandomIncrOrDecr(CheckerContext , const Expr *CE,
   OverloadedOperatorKind Op, const SVal ,
-  const SVal , const SVal ) const;
+  const SVal , const SVal ) const;
   void handlePtrIncrOrDecr(CheckerContext , const Expr *Iterator,
OverloadedOperatorKind OK, SVal Offset) const;
   void handleAdvance(CheckerContext , const Expr *CE, SVal RetVal, SVal Iter,
@@ -262,18 +262,25 @@
 
 void IteratorModeling::checkPostStmt(const BinaryOperator *BO,
  CheckerContext ) const {
-  ProgramStateRef State = C.getState();
-  BinaryOperatorKind OK = BO->getOpcode();
-  SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
+  const ProgramStateRef State = C.getState();
+  const BinaryOperatorKind OK = BO->getOpcode();
+  const Expr *const LHS = BO->getLHS();
+  const Expr *const RHS = BO->getRHS();
+  const SVal LVal = State->getSVal(LHS, C.getLocationContext());
+  const SVal RVal = State->getSVal(RHS, C.getLocationContext());
 
   if (isSimpleComparisonOperator(BO->getOpcode())) {
-SVal LVal = State->getSVal(BO->getLHS(), C.getLocationContext());
 SVal 

[PATCH] D82930: [HIP] Fix rocm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

yaxunl wrote:
> tra wrote:
> > arsenm wrote:
> > > yaxunl wrote:
> > > > arsenm wrote:
> > > > > arsenm wrote:
> > > > > > yaxunl wrote:
> > > > > > > arsenm wrote:
> > > > > > > > I don't think the detection should fail if this is missing
> > > > > > > why? this file is unique to HIP installation. If it is missing, 
> > > > > > > the installation is broken.
> > > > > > Because I should be able to use this without a complete hip 
> > > > > > installation. Without a specified version, it should assume the 
> > > > > > most modern layout. This will for example break pointing 
> > > > > > --rocm-path at the device library build directory for library tests
> > > > > I also don't see what value checking the version really provides; it 
> > > > > may be informative to print it, but I don't think it's useful to 
> > > > > derive information from it
> > > > what is the directory structure of your most modern layout?
> > > /opt/rocm/amdgcn/bitcode/foo.bc
> > > 
> > > The plan is to remove this and rely on symlinks in the resource directory 
> > > though
> > > I also don't see what value checking the version really provides; it may 
> > > be informative to print it, but I don't think it's useful to derive 
> > > information from it
> > 
> > In CUDA it's used to detect which back-end features to enable (they depend 
> > on what's supported by ptxas supplied by that CUDA version). I don't think 
> > that would be relevant to AMDGPU as it does not need external dependencies 
> > to generate GPU code.
> > 
> > It may be useful for filesystem layout detection. E.g. where to find 
> > bitcode files and what names to expect. This part seems to be relevant for 
> > ROCm, but I'm not sure if the layout is closely tied to the version.
> > 
> We are required to support previous ROCm releases for certain time range. To 
> do that we need to detect HIP version and enable/disable certain HIP features 
> based on HIP version when necessary.
> 
> Therefore if we have a new directory structure for ROCm, that directory 
> structure should contain a HIP version file so that we can detect HIP version.
> 
> 
Currently clang includes some wrapper headers by default, which does not work 
with ROCm 3.5 since those device functions are defined in HIP headers of ROCm 
3.5. To support ROCm 3.5, we have to disable including those wrapper headers. 
This is one example why we need detect HIP version.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

yaxunl wrote:
> yaxunl wrote:
> > tra wrote:
> > > arsenm wrote:
> > > > yaxunl wrote:
> > > > > arsenm wrote:
> > > > > > arsenm wrote:
> > > > > > > yaxunl wrote:
> > > > > > > > arsenm wrote:
> > > > > > > > > I don't think the detection should fail if this is missing
> > > > > > > > why? this file is unique to HIP installation. If it is missing, 
> > > > > > > > the installation is broken.
> > > > > > > Because I should be able to use this without a complete hip 
> > > > > > > installation. Without a specified version, it should assume the 
> > > > > > > most modern layout. This will for example break pointing 
> > > > > > > --rocm-path at the device library build directory for library 
> > > > > > > tests
> > > > > > I also don't see what value checking the version really provides; 
> > > > > > it may be informative to print it, but I don't think it's useful to 
> > > > > > derive information from it
> > > > > what is the directory structure of your most modern layout?
> > > > /opt/rocm/amdgcn/bitcode/foo.bc
> > > > 
> > > > The plan is to remove this and rely on symlinks in the resource 
> > > > directory though
> > > > I also don't see what value checking the version really provides; it 
> > > > may be informative to print it, but I don't think it's useful to derive 
> > > > information from it
> > > 
> > > In CUDA it's used to detect which back-end features to enable (they 
> > > depend on what's supported by ptxas supplied by that CUDA version). I 
> > > don't think that would be relevant to AMDGPU as it does not need external 
> > > dependencies to generate GPU code.
> > > 
> > > It may be useful for filesystem layout detection. E.g. where to find 
> > > bitcode files and what names to expect. This part seems to be relevant 
> > > for ROCm, but I'm not sure if the layout is closely tied to the version.
> > > 
> > We are required to support previous ROCm releases for certain time range. 
> > To do that we need to detect HIP version and enable/disable certain HIP 
> > features based on HIP version when necessary.
> > 
> > Therefore if we have a new directory structure 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:10068
+isa(D) && cast(D)->isFileVarDecl() &&
+cast(D)->getStorageClass() == SC_Static) {
+  return GVA_StrongExternal;

yaxunl wrote:
> rjmccall wrote:
> > Are you sure this doesn't apply to e.g. local statics?  Can't you have 
> > kernel lambdas, or am I confusing HIP with another language?
> function-scope static var in a device function is only visible to the device 
> function. Host code cannot access it, therefore no need to externalize it.
This doesn't sound right. An inline function can return a pointer to a function 
scope static variable, e.g. to implement a singleton in a header file.  I think 
host code can then access said variable.


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

https://reviews.llvm.org/D80858



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

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

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



Comment at: llvm/utils/update_cc_test_checks.py:133
+  parser.add_argument('--include-generated-funcs', action='store_true',
+  help='Output checks for functions not in source')
   parser.add_argument('tests', nargs='+')

jdoerfert wrote:
> I think this should go into common.py (after D78618). I would also make this 
> the default but OK.
Yes I suppose it should in case `opt` and friends generate functions.  I hadn't 
considered that use-case.

While I would like to make it default unfortunately it would require updating a 
bunch of the existing clang tests which doesn't seem too friendly.  See the 
patch update comment for details.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004



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

[PATCH] D82800: [OPENMP50] extend array section for stride (Parsing/Sema/AST)

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 275834.
cchen added a comment.

Move analysis of stride from Sema to Parse


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82800

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/target_data_messages.c
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_parallel_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_depend_messages.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/task_affinity_messages.cpp
  clang/test/OpenMP/task_depend_messages.cpp

Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -52,7 +52,7 @@
   #pragma omp task depend (in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
   #pragma omp task depend (in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp task depend (in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
-  #pragma omp task depend (in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
+  #pragma omp task depend (in : argv[3:4:1]) // omp45-error {{expected ']'}} omp45-note {{to match this '['}}
   #pragma omp task depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
   #pragma omp task depend(in:argv[argv[:2]:1]) // expected-error {{OpenMP array section is not allowed here}}
   #pragma omp task depend(in:argv[0:][:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
Index: clang/test/OpenMP/task_affinity_messages.cpp
===
--- clang/test/OpenMP/task_affinity_messages.cpp
+++ clang/test/OpenMP/task_affinity_messages.cpp
@@ -44,7 +44,7 @@
   #pragma omp task affinity (argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
   #pragma omp task affinity (argv[-1:0])
   #pragma omp task affinity (argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
-  #pragma omp task affinity (argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
+  #pragma omp task affinity (argv[3:4:1])
   #pragma omp task affinity(a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
   #pragma omp task affinity(argv[argv[:2]:1]) // expected-error {{OpenMP array section is not allowed here}}
   #pragma omp task affinity(argv[0:][:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s 

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via cfe-commits]

greened updated this revision to Diff 275831.
greened added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixed various bugs, added tests.

This now has two modes because generated function output can't be ordered with 
respect to source file functions.  When clang generates functions it can 
sometimes output original source functions in a different (non-source) order so 
checks can't be placed next to their definitions in the source file.

I don't particularly like this mode dichotomy but unifying it would necessitate 
updating a whole lot of clang tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004

Files:
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.generated.expected
  
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs.c.no-generated.expected
  clang/test/utils/update_cc_test_checks/generated-funcs.test
  llvm/utils/UpdateTestChecks/asm.py
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -137,9 +137,11 @@
   prefix_list.append((check_prefixes, tool_cmd_args))
 
 func_dict = {}
+func_order = {}
 for prefixes, _ in prefix_list:
   for prefix in prefixes:
 func_dict.update({prefix: dict()})
+func_order.update({prefix: []})
 for prefixes, opt_args in prefix_list:
   common.debug('Extracted opt cmd: ' + opt_basename + ' ' + opt_args)
   common.debug('Extracted FileCheck prefixes: ' + str(prefixes))
@@ -147,7 +149,7 @@
   raw_tool_output = common.invoke_tool(args.opt_binary, opt_args, test)
   common.build_function_body_dictionary(
   common.OPT_FUNCTION_RE, common.scrub_body, [],
-  raw_tool_output, prefixes, func_dict, args.verbose,
+  raw_tool_output, prefixes, func_dict, func_order, args.verbose,
   args.function_signature)
 
 is_in_function = False
Index: llvm/utils/update_llc_test_checks.py
===
--- llvm/utils/update_llc_test_checks.py
+++ llvm/utils/update_llc_test_checks.py
@@ -123,10 +123,12 @@
 autogenerated_note = (comment_sym + ADVERT + 'utils/' + script_name)
 
 func_dict = {}
+func_order = {}
 for p in run_list:
   prefixes = p[0]
   for prefix in prefixes:
 func_dict.update({prefix: dict()})
+func_order.update({prefix: []})
 for prefixes, llc_args, triple_in_cmd, march_in_cmd in run_list:
   common.debug('Extracted LLC cmd:', llc_tool, llc_args)
   common.debug('Extracted FileCheck prefixes:', str(prefixes))
@@ -138,7 +140,7 @@
 triple = asm.get_triple_from_march(march_in_cmd)
 
   asm.build_function_body_dictionary_for_triple(args, raw_tool_output,
-  triple, prefixes, func_dict)
+  triple, prefixes, func_dict, func_order)
 
 is_in_function = False
 is_in_function_start = False
Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -129,6 +129,8 @@
   help='Use more regex for x86 matching to reduce diffs between various subtargets')
   parser.add_argument('--function-signature', action='store_true',
   help='Keep function signature information around for the check line')
+  parser.add_argument('--include-generated-funcs', action='store_true',
+  help='Output checks for functions not in source')
   parser.add_argument('tests', nargs='+')
   args = common.parse_commandline_args(parser)
   args.clang_args = shlex.split(args.clang_args or '')
@@ -168,7 +170,8 @@
   return args
 
 
-def get_function_body(args, filename, clang_args, extra_commands, prefixes, triple_in_cmd, func_dict):
+def get_function_body(args, filename, clang_args, extra_commands, prefixes,
+  triple_in_cmd, func_dict, func_order):
   # TODO Clean up duplication of asm/common build_function_body_dictionary
   # Invoke external tool and extract function bodies.
   raw_tool_output = common.invoke_tool(args.clang, clang_args, filename)
@@ -188,7 +191,8 @@
   if '-emit-llvm' in clang_args:
 common.build_function_body_dictionary(
 common.OPT_FUNCTION_RE, common.scrub_body, [],
-raw_tool_output, prefixes, func_dict, args.verbose, args.function_signature)
+raw_tool_output, prefixes, func_dict, func_order, args.verbose,
+args.function_signature)
   else:
 print('The clang command line should include -emit-llvm as asm 

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel added a comment.
Herald added a subscriber: aaron.ballman.

Ping @ABataev.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591



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

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 275830.
MaskRay retitled this revision from "[Driver] Add -fld-path= and deprecate 
-fuse-ld=/abs/path and -fuse-ld=rel/path" to "[Driver] Add --ld-path= and 
deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path".
MaskRay edited the summary of this revision.
MaskRay added a reviewer: phosek.
MaskRay added a comment.

Rename -fld-path= to --ld-path=


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fuse-ld.c
  clang/test/Driver/ld-path.c
  clang/test/Misc/warning-flags.c

Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (68):
+CHECK: Warnings without flags (69):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -47,6 +47,7 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
+CHECK-NEXT:   warn_drv_use_ld_non_word
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: clang/test/Driver/ld-path.c
===
--- /dev/null
+++ clang/test/Driver/ld-path.c
@@ -0,0 +1,65 @@
+/// This tests uses the PATH environment variable.
+// UNSUPPORTED: system-windows
+
+// RUN: cd %S
+
+/// If --ld-path= specifies a word (without /), PATH is consulted to locate the linker.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// BFD: Inputs/basic_freebsd_tree/usr/bin/ld.bfd
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=ld.gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=GOLD
+
+// GOLD: Inputs/basic_freebsd_tree/usr/bin/ld.gold
+
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=not_exist \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST
+
+// NOT_EXIST: error: invalid linker name in argument '--ld-path=not_exist'
+
+// RUN: %clang %s -### --ld-path= \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=EMPTY
+
+// EMPTY: error: invalid linker name in argument '--ld-path='
+
+/// If --ld-path= contains a slash, PATH is not consulted.
+// RUN: env PATH=%S/Inputs/basic_freebsd_tree/usr/bin %clang %s -### --ld-path=./ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NO_BFD
+
+// NO_BFD: error: invalid linker name in argument '--ld-path=./ld.bfd'
+
+/// --ld-path's argument is searched in PATH. --sysroot and -B have no effect.
+// RUN: env PATH= %clang %s -### --ld-path=ld.bfd -B %S/Inputs/basic_freebsd_tree/usr/bin \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=NOT_EXIST2
+
+// NOT_EXIST2: error: invalid linker name in argument '--ld-path=ld.bfd'
+
+/// --ld-path can specify a path
+// RUN: %clang %s -### --ld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+// RUN: %clang %s -### --ld-path=Inputs/basic_freebsd_tree/usr/bin/ld.bfd \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD
+
+/// --ld-path= and -fuse-ld= can be used together. --ld-path= takes precedence.
+/// -fuse-ld= can be used to specify the linker flavor.
+// RUN: %clang %s -### -Werror --ld-path=%S/Inputs/basic_freebsd_tree/usr/bin/ld.bfd -fuse-ld=gold \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=BFD --implicit-check-not=error:
+
+/// --ld-path= respects -working-directory.
+// RUN: %clang %s -### --ld-path=usr/bin/ld.bfd -working-directory=%S/Inputs/basic_freebsd_tree \
+// RUN:   --target=x86_64-unknown-freebsd --sysroot=%S/Inputs/basic_freebsd_tree 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=USR_BIN_BFD
+
+// USR_BIN_BFD: "usr/bin/ld.bfd"
Index: clang/test/Driver/fuse-ld.c

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6069
+llvm::raw_ostream ) const {
+  OS << ".static." << getLangOpts().CUID;
+}

I suspect that will have interesting issues if CUID is an arbitrary 
user-supplied string. We may want to impose some sort of sanity check or 
filtering on the cuid value. Considering that it's a CC1 flag, it's not a 
critical problem, but some safeguards would be useful there, too. Should we 
limit allowed character set?



Comment at: clang/test/Driver/hip-cuid.hip:35
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -cuid=abcd \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \

Nit: `abcd` could potentially match the value generated by hash. I'd change it 
to contain characters other than hex.


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

https://reviews.llvm.org/D80858



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

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

[Ellis Hoag via Phabricator via cfe-commits]

ellis added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:34
+  const BlockDecl *EscapingBlockDecl = MatchedEscapingBlock->getBlockDecl();
+  for (const BlockDecl::Capture  : EscapingBlockDecl->captures()) {
+const VarDecl *Var = CapturedVar.getVariable();

aaron.ballman wrote:
> This makes me think we should extend the `hasAnyCaptures()` AST matcher so it 
> works with blocks as well as lambdas. It would be nice to do all of this from 
> the matcher interface.
Should I add a TODO for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82904



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

[PATCH] D83183: [clang] Rework how and when APValues are dumped

[Bruno Ricci via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf63e3ea558bb: [clang] Rework how and when APValues are 
dumped (authored by riccibruno).

Changed prior to commit:
  https://reviews.llvm.org/D83183?vs=275783=275828#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83183

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ASTDumper.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-arithmetic.cpp
  clang/test/AST/ast-dump-APValue-array.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-todo.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-APValue-vector.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/AST/ast-dump-color.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/pr43983.cpp
  clang/test/Import/switch-stmt/test.cpp
  clang/test/Tooling/clang-check-ast-dump.cpp

Index: clang/test/Tooling/clang-check-ast-dump.cpp
===
--- clang/test/Tooling/clang-check-ast-dump.cpp
+++ clang/test/Tooling/clang-check-ast-dump.cpp
@@ -33,6 +33,7 @@
 // CHECK-ATTR-NEXT: FieldDecl{{.*}}n
 // CHECK-ATTR-NEXT:   AlignedAttr
 // CHECK-ATTR-NEXT: ConstantExpr
+// CHECK-ATTR-NEXT:   value: Int 2
 // CHECK-ATTR-NEXT:   BinaryOperator
 //
 // RUN: clang-check -ast-dump -ast-dump-filter test_namespace::AfterNullNode "%s" -- 2>&1 | FileCheck -check-prefix CHECK-AFTER-NULL %s
Index: clang/test/Import/switch-stmt/test.cpp
===
--- clang/test/Import/switch-stmt/test.cpp
+++ clang/test/Import/switch-stmt/test.cpp
@@ -5,20 +5,26 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 4
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
@@ -30,16 +36,20 @@
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 1
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 2
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 // CHECK-NEXT: CaseStmt
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 3
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ConstantExpr
+// CHECK-NEXT: value: Int 5
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: BreakStmt
 
Index: clang/test/AST/pr43983.cpp
===
--- clang/test/AST/pr43983.cpp
+++ clang/test/AST/pr43983.cpp
@@ -9,6 +9,7 @@
 
 struct B { _Alignas(64) struct { int b; };   };
 
-// CHECK: AlignedAttr {{.*}} _Alignas
-// CHECK: ConstantExpr {{.*}} 64
-// CHECK: IntegerLiteral {{.*}} 64
+// CHECK:  | `-AlignedAttr {{.*}}  _Alignas
+// CHECK-NEXT:  |   `-ConstantExpr {{.*}}  'int'
+// CHECK-NEXT:  | |-value: Int 64
+// CHECK-NEXT:  | `-IntegerLiteral {{.*}}  'int' 64
Index: clang/test/AST/ast-dump-stmt.cpp
===
--- clang/test/AST/ast-dump-stmt.cpp
+++ clang/test/AST/ast-dump-stmt.cpp
@@ -130,6 +130,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}} 
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
@@ -144,6 +145,7 @@
 ;
   // CHECK: IfStmt 0x{{[^ ]*}}  has_else
   // CHECK-NEXT: ConstantExpr 0x{{[^ ]*}}  'bool'
+  // CHECK-NEXT: value: Int 1
   // CHECK-NEXT: BinaryOperator
   // CHECK-NEXT: UnaryExprOrTypeTraitExpr
   // CHECK-NEXT: ParenExpr
Index: clang/test/AST/ast-dump-records.cpp
===
--- clang/test/AST/ast-dump-records.cpp
+++ clang/test/AST/ast-dump-records.cpp
@@ -15,7 +15,7 @@
 // CHECK: CXXRecordDecl 0x{{[^ ]*}}  col:8 referenced struct B
 
 struct A {
-  // CHECK: 

[clang] f63e3ea - [clang] Rework how and when APValues are dumped

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-06T22:03:08+01:00
New Revision: f63e3ea558bbe14826b5b775367eac617b35e041

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

LOG: [clang] Rework how and when APValues are dumped

Currently APValues are dumped as a single string. This becomes quickly
completely unreadable since APValue is a tree-like structure. Even a simple
example is not pretty:

  struct S { int arr[4]; float f; };
  constexpr S s = { .arr = {1,2}, .f = 3.1415f };
  // Struct  fields: Array: Int: 1, Int: 2, 2 x Int: 0, Float: 3.141500e+00

With this patch this becomes:

  -Struct
   |-field: Array size=4
   | |-elements: Int 1, Int 2
   | `-filler: 2 x Int 0
   `-field: Float 3.141500e+00

Additionally APValues are currently only dumped as part of visiting a
ConstantExpr. This patch also dump the value of the initializer of constexpr
variable declarations:

  constexpr int foo(int a, int b) { return a + b - 42; }
  constexpr int a = 1, b = 2;
  constexpr int c = foo(a, b) > 0 ? foo(a, b) : foo(b, a);
  // VarDecl 0x6218aec8  col:17 c 'const int' constexpr cinit
  // |-value: Int -39
  // `-ConditionalOperator 0x6218b4d0  'int'
  // 

Do the above by moving the dump functions to TextNodeDumper which already has
the machinery to display trees. The cases APValue::LValue, 
APValue::MemberPointer
and APValue::AddrLabelDiff are left as they were before (unimplemented).

We try to display multiple elements on the same line if they are considered to
be "simple". This is to avoid wasting large amounts of vertical space in an
example like:

  constexpr int arr[8] = {0,1,2,3,4,5,6,7};
  // VarDecl 0x6218bb78  col:17 arr 'int const[8]' constexpr 
cinit
  // |-value: Array size=8
  // | |-elements: Int 0, Int 1, Int 2, Int 3
  // | `-elements: Int 4, Int 5, Int 6, Int 7

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

Reviewed By: aaron.ballman

Added: 
clang/test/AST/ast-dump-APValue-anon-union.cpp
clang/test/AST/ast-dump-APValue-arithmetic.cpp
clang/test/AST/ast-dump-APValue-array.cpp
clang/test/AST/ast-dump-APValue-struct.cpp
clang/test/AST/ast-dump-APValue-todo.cpp
clang/test/AST/ast-dump-APValue-union.cpp
clang/test/AST/ast-dump-APValue-vector.cpp

Modified: 
clang/include/clang/AST/APValue.h
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/JSONNodeDumper.h
clang/include/clang/AST/TextNodeDumper.h
clang/lib/AST/APValue.cpp
clang/lib/AST/ASTDumper.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/test/AST/alignas_maybe_odr_cleanup.cpp
clang/test/AST/ast-dump-attr.cpp
clang/test/AST/ast-dump-color.cpp
clang/test/AST/ast-dump-constant-expr.cpp
clang/test/AST/ast-dump-decl.cpp
clang/test/AST/ast-dump-records.cpp
clang/test/AST/ast-dump-stmt.cpp
clang/test/AST/pr43983.cpp
clang/test/Import/switch-stmt/test.cpp
clang/test/Tooling/clang-check-ast-dump.cpp

Removed: 




diff  --git a/clang/include/clang/AST/APValue.h 
b/clang/include/clang/AST/APValue.h
index c69974c63c71..cca92b5f8235 100644
--- a/clang/include/clang/AST/APValue.h
+++ b/clang/include/clang/AST/APValue.h
@@ -372,7 +372,7 @@ class APValue {
   bool isAddrLabelDiff() const { return Kind == AddrLabelDiff; }
 
   void dump() const;
-  void dump(raw_ostream , const ASTContext *Context) const;
+  void dump(raw_ostream , const ASTContext ) const;
 
   void printPretty(raw_ostream , const ASTContext , QualType Ty) const;
   std::string getAsString(const ASTContext , QualType Ty) const;

diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index f1c98193df6c..26656b7162b6 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -22,10 +22,13 @@
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TemplateArgumentVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeVisitor.h"
 
 namespace clang {
 
+class APValue;
+
 /**
 
 ASTNodeTraverser traverses the Clang AST for dumping purposes.
@@ -50,6 +53,7 @@ struct {
   void Visit(const OMPClause *C);
   void Visit(const BlockDecl::Capture );
   void Visit(const GenericSelectionExpr::ConstAssociation );
+  void Visit(const APValue , QualType Ty);
 };
 */
 template 
@@ -211,6 +215,10 @@ class ASTNodeTraverser
 });
   }
 
+  void Visit(const APValue , QualType Ty) {
+getNodeDelegate().AddChild([=] { getNodeDelegate().Visit(Value, Ty); });
+  }
+
   void Visit(const comments::Comment *C, const comments::FullComment *FC) {
 getNodeDelegate().AddChild([=] {
   getNodeDelegate().Visit(C, FC);

diff  --git a/clang/include/clang/AST/JSONNodeDumper.h 

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

[Ellis Hoag via Phabricator via cfe-commits]

ellis updated this revision to Diff 275826.
ellis marked 9 inline comments as done.
ellis added a comment.

Use `LangOpts.Blocks` instead of `LangOpts.ObjC` and add FIXME about grabbing 
the use location of a `CapturedVar`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82904

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s bugprone-no-escape %t
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef struct dispatch_time_s *dispatch_time_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+
+void test_noescape_attribute(__attribute__((noescape)) int *p, int *q) {
+  dispatch_async(queue, ^{
+*p = 123;
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+// CHECK-MESSAGES: :[[@LINE-4]]:30: note: the 'noescape' attribute is declared here.
+  });
+
+  dispatch_after(456, queue, ^{
+*p = 789;
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+  });
+
+  dispatch_async(queue, ^{
+*q = 0;
+// CHECK-MESSAGES-NOT: :[[@LINE-2]]:25: warning: pointer 'q' with attribute 'noescape' is captured by an asynchronously-executed block
+  });
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -70,6 +70,7 @@
`bugprone-misplaced-widening-cast `_,
`bugprone-move-forwarding-reference `_, "Yes"
`bugprone-multiple-statement-macro `_,
+   `bugprone-no-escape `_, "Yes"
`bugprone-not-null-terminated-result `_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - bugprone-no-escape
+
+bugprone-no-escape
+==
+
+Finds pointers with the ``noescape`` attribute that are captured by an
+asynchronously-executed block. The block arguments in ``dispatch_async()`` and
+``dispatch_after()`` are guaranteed to escape, so it is an error if a pointer with the
+``noescape`` attribute is captured by one of these blocks.
+
+The following is an example of an invalid use of the ``noescape`` attribute.
+
+  .. code-block:: objc
+void foo(__attribute__((noescape)) int *p) {
+  dispatch_async(queue, ^{
+*p = 123;
+  });
+});
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,12 @@
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
+- New :doc:`bugprone-no-escape
+  ` check.
+
+  Finds pointers with the ``noescape`` attribute that are captured by an
+  asynchronously-executed block.
+
 - New :doc:`bugprone-spuriously-wake-up-functions
   ` check.
 
@@ -201,14 +207,14 @@
   Now checks ``std::basic_string_view`` by default.
 
 - Improved :doc:`readability-else-after-return
-  ` check now supports a 
+  ` check now supports a
   `WarnOnConditionVariables` option to control whether to refactor condition
   variables where possible.
 
 - Improved :doc:`readability-identifier-naming
   ` check.
 
-  Now able to rename member references in class template definitions with 
+  Now able to rename member references in class template definitions with
   explicit access.
 
 - Improved :doc:`readability-qualified-auto
Index: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
@@ -0,0 

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChain.cpp:556-557
+  // -fld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. PATH is consulted if the value does not contain /. -B is
+  // intentionally ignored.
+  if (const Arg *A = Args.getLastArg(options::OPT_fld_path_EQ)) {

MaskRay wrote:
> jyknight wrote:
> > Shouldn't this use -B paths? Why do we want to ignore them?
> If the value is relative, respect -Bprefix and COMPILER_PATH? (COMPILER is 
> currently not well supported in clang IIUC)
D80660's need is an option relative to the current working directory...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015



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

[PATCH] D82930: [HIP] Fix rocm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

tra wrote:
> arsenm wrote:
> > yaxunl wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > yaxunl wrote:
> > > > > > arsenm wrote:
> > > > > > > I don't think the detection should fail if this is missing
> > > > > > why? this file is unique to HIP installation. If it is missing, the 
> > > > > > installation is broken.
> > > > > Because I should be able to use this without a complete hip 
> > > > > installation. Without a specified version, it should assume the most 
> > > > > modern layout. This will for example break pointing --rocm-path at 
> > > > > the device library build directory for library tests
> > > > I also don't see what value checking the version really provides; it 
> > > > may be informative to print it, but I don't think it's useful to derive 
> > > > information from it
> > > what is the directory structure of your most modern layout?
> > /opt/rocm/amdgcn/bitcode/foo.bc
> > 
> > The plan is to remove this and rely on symlinks in the resource directory 
> > though
> > I also don't see what value checking the version really provides; it may be 
> > informative to print it, but I don't think it's useful to derive 
> > information from it
> 
> In CUDA it's used to detect which back-end features to enable (they depend on 
> what's supported by ptxas supplied by that CUDA version). I don't think that 
> would be relevant to AMDGPU as it does not need external dependencies to 
> generate GPU code.
> 
> It may be useful for filesystem layout detection. E.g. where to find bitcode 
> files and what names to expect. This part seems to be relevant for ROCm, but 
> I'm not sure if the layout is closely tied to the version.
> 
We are required to support previous ROCm releases for certain time range. To do 
that we need to detect HIP version and enable/disable certain HIP features 
based on HIP version when necessary.

Therefore if we have a new directory structure for ROCm, that directory 
structure should contain a HIP version file so that we can detect HIP version.




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

https://reviews.llvm.org/D82930



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

[PATCH] D82930: [HIP] Fix rocm detection

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

arsenm wrote:
> yaxunl wrote:
> > arsenm wrote:
> > > arsenm wrote:
> > > > yaxunl wrote:
> > > > > arsenm wrote:
> > > > > > I don't think the detection should fail if this is missing
> > > > > why? this file is unique to HIP installation. If it is missing, the 
> > > > > installation is broken.
> > > > Because I should be able to use this without a complete hip 
> > > > installation. Without a specified version, it should assume the most 
> > > > modern layout. This will for example break pointing --rocm-path at the 
> > > > device library build directory for library tests
> > > I also don't see what value checking the version really provides; it may 
> > > be informative to print it, but I don't think it's useful to derive 
> > > information from it
> > what is the directory structure of your most modern layout?
> /opt/rocm/amdgcn/bitcode/foo.bc
> 
> The plan is to remove this and rely on symlinks in the resource directory 
> though
> I also don't see what value checking the version really provides; it may be 
> informative to print it, but I don't think it's useful to derive information 
> from it

In CUDA it's used to detect which back-end features to enable (they depend on 
what's supported by ptxas supplied by that CUDA version). I don't think that 
would be relevant to AMDGPU as it does not need external dependencies to 
generate GPU code.

It may be useful for filesystem layout detection. E.g. where to find bitcode 
files and what names to expect. This part seems to be relevant for ROCm, but 
I'm not sure if the layout is closely tied to the version.



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

https://reviews.llvm.org/D82930



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

[PATCH] D83088: Introduce CfgTraits abstraction

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138
+  // Prefer to avoid support for bundled instructions as long as we
+  // don't really need it.
+  assert(!m_instr->isBundle());

nhaehnle wrote:
> arsenm wrote:
> > I've been thinking about more aggressively using bundles around call sites 
> > to handle waterfall looping around divergent calls with SGPR arguments
> Hmm, so what's the correct iteration behavior in the presence of bundles? 
> Iterate over all instructions in the bundle (which is that 
> MachineBasicBlock::instr_iterator does) and only iterate over explicit defs? 
> I think that's what makes the most sense, and what I'm going with for now...
I don't think this actually needs to specially consider bundles. The BUNDLE 
itself is supposed to have the uses/defs that cover all the uses/defs inside 
the bundle. You shouldn't need to worry about the individual instructions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



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

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChain.cpp:556-557
+  // -fld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. PATH is consulted if the value does not contain /. -B is
+  // intentionally ignored.
+  if (const Arg *A = Args.getLastArg(options::OPT_fld_path_EQ)) {

jyknight wrote:
> Shouldn't this use -B paths? Why do we want to ignore them?
If the value is relative, respect -Bprefix and COMPILER_PATH? (COMPILER is 
currently not well supported in clang IIUC)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015



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

[PATCH] D82930: [HIP] Fix rocm detection

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:167
+llvm::ErrorOr> VersionFile =
+FS.getBufferForFile(BinPath + "/.hipVersion");
+if (!VersionFile)

yaxunl wrote:
> arsenm wrote:
> > arsenm wrote:
> > > yaxunl wrote:
> > > > arsenm wrote:
> > > > > I don't think the detection should fail if this is missing
> > > > why? this file is unique to HIP installation. If it is missing, the 
> > > > installation is broken.
> > > Because I should be able to use this without a complete hip installation. 
> > > Without a specified version, it should assume the most modern layout. 
> > > This will for example break pointing --rocm-path at the device library 
> > > build directory for library tests
> > I also don't see what value checking the version really provides; it may be 
> > informative to print it, but I don't think it's useful to derive 
> > information from it
> what is the directory structure of your most modern layout?
/opt/rocm/amdgcn/bitcode/foo.bc

The plan is to remove this and rely on symlinks in the resource directory though


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

https://reviews.llvm.org/D82930



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

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Thanks for working on this.

There seems to be many places in this check where names are confusing, `Var` 
referring to a `MemberExpr`, `DeclRef` refering to a `MemberExpr`. I'd prefer 
if the names that you have added were renamed to something more in line with 
what they represent.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp:82
+
+  if (const auto *Var = Result.Nodes.getNodeAs("expr")) {
+const Decl *D = Var->getMemberDecl();

This should be an `else if` or the previous `if` should return. Saves redundant 
checking for `MemberExpr` when `DeclRefExpr` already matched.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp:108-126
+  if (d.b) {
+*d.b = true; // no-warning
+  }
+
+  if (d.b) {
+d.b[0] = false; // no-warning
+  }

nit: You don't need to recycle all the test cases, the logic for that is 
already checked above, just a few showing member expr being detected properly 
will suffice.

Don't have huge preference on this so feel free to ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83188



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

[PATCH] D83242: [RFC][BPF] support expr with typedef type for FIELD_EXISTENCE reloc

[Andrii Nakryiko via Phabricator via cfe-commits]

anakryiko added a comment.

Awesome, that's exactly what we need for BPF helper availability checks!

Can you please also add test that this pattern works:

  return __builtin_preserve_field_info((btf_bpf_read_branch_records)0, 
FIELD_EXISTENCE);

Also for non-func pointer typedefs, something like this would work as well, 
right?

  return __builtin_preserve_field_info(*(T *)0, FIELD_EXISTENCE);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83242



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGDecl.cpp:103-118
+  case Decl::CXXRecord: // struct/union/class X; [C++]
+if (CGDebugInfo *DI = getDebugInfo())
+  if (CGM.getCodeGenOpts().hasMaybeUnusedDebugInfo() &&
+  cast(D).hasDefinition())
+DI->completeUnusedClass(cast(D));
+return;
   case Decl::Record:// struct/union/class X;

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > dblaikie wrote:
> > > All of these might be able to be collapsed together - using 
> > > "cast(D).hasDefinition()" ... to have a common implementation.
> > `TagDecl` doesn't have a method `hasDefinition`.  Only `CXXRecordDecl` does.
> Also, how to get the `QualType` differs between `Decl::Enum` an 
> `Decl::Record`; `getEnumType` vs `getRecordType` respectively.
Looks like ASTContext::getTypeDeclType could be used, if it makes things 
especially tidier. But perhaps the TagDecl/hasDefinition difference is enough 
for it not to be especially significant.



Comment at: clang/lib/CodeGen/CGDecl.cpp:116
+if (CGDebugInfo *DI = getDebugInfo())
+  if (cast()->getDefinition())
+DI->EmitAndRetainType(getContext().getEnumType(cast()));

nickdesaulniers wrote:
> dblaikie wrote:
> > I think the right thing to call here (& the other places in this patch) is 
> > "hasDefinition" rather than "getDefinition" (this will avoid the definition 
> > being deserialized when it's not needed yet (then if we don't end up 
> > emitting the type because the flag hasn't been given, it won't be 
> > deserialized unnecessarily))
> `TagDecl` doesn't have a method `hasDefinition`.  Only `CXXRecordDecl` does.
Ah, fair enough!



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5385
+  if (getCodeGenOpts().hasMaybeUnusedDebugInfo() && CRD->hasDefinition())
+DI->completeUnusedClass(*CRD);
+  else if (auto *ES = D->getASTContext().getExternalSource())

nickdesaulniers wrote:
> dblaikie wrote:
> > I think this might not work as intended if the type trips over the other 
> > class deduplication optimizations (try manually testing with a type that 
> > has an external VTable?) - and perhaps this codepath should use the 
> > EmitAndRetain functionality.
> completeUnusedClass eventually calls into CGDebugInfo::completeClass which 
> makes use of the TypeCache.  What does it mean for a class to have "an 
> external vtable?"  Can you give me an example?
Here's some sample code that uses a type in the DWARF but due to the vtable 
being external (an external vtable is one that's guaranteed to be emitted in 
some other object file - due to the key function optimization in the Itanium 
ABI - since all virtual functions must be defined if a type is ODR used, the 
compiler can make a shared assumption that the vtable will only be emitted with 
a single consistently chosen virtual function (ideally: the first non-inline 
one) and emit the vtable only in that object and nowhere else) the type is 
emitted as a declaration (when using "-fno-standalone-debug"):

```
struct t1 {
  virtual void f1();
};
t1 v1;
```



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3825-3828
+  if (EmitDwarf &&
+  Args.hasFlag(options::OPT_fno_eliminate_unused_debug_types,
+   options::OPT_feliminate_unused_debug_types, false))
+DebugInfoKind = codegenoptions::UnusedTypeInfo;

How does GCC compose this with -gmlt, for instance? I'd suspect that the 
desired behavior might be for -gmlt to override this -f flag? So maybe this 
should be predicated on "if (EmitDwarf && DebugInfoKind >= 
codegenoptions::LimitedDebugInfo && ... "?

(so if someone wants to use only -gmlt in certain parts of their build that 
doesn't get upgraded by the presence of this -f flag)



Comment at: clang/test/CodeGen/debug-info-unused-types.c:46-60
+struct ;
+enum ;
+union ;
+void b0(void) {
+  struct ;
+  enum ;
+  union ;

Maybe give these more specific names (similarly in the other test) - and/or you 
could use a pattern in the naming that might make the -NOT lines easier?

eg:

```
struct decl_struct;
enum decl_enum;
...
// NODBG-NOT: name: "decl_
```

(also best to include a bit more context in the -NOT checks, otherwise there's 
risk that a users build directory (even with these longer names) or other paths 
might accidentally fail the checks)



Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:15-21
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// CHECK: !DIEnumerator(name: "BAZ"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: !DICompositeType(tag: 

[PATCH] D78655: [CUDA][HIP] Let lambda be host device by default

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1783
+
+  if (LangOpts.CUDA && LangOpts.CUDAIsDevice)
+CUDACheckLambdaCapture(CallOperator, From);

yaxunl wrote:
> tra wrote:
> > I would expect Sema-level diags to be produced during both host and device 
> > compilation. Some of the diags for HD may need to be postponed until after 
> > things have been CodeGen'ed, but the checks should happen nevertheless.
> > 
> > 
> A host device function could be emitted in both host and device compilation. 
> The way the deferred diags works is that they are only emitted when the 
> function is sure to be emitted in a specific compilation. In host 
> compilation, when a host device function is sure to be emitted, it is emitted 
> as host function, therefore diags for host compilation and only diags for 
> host compilation should be emitted. The same with device compilation.
> 
> This makes sense since we do not know if a host device function will be 
> emitted in device compilation when we are doing host compilation, since to do 
> that we have to go through the whole device compilation whereas currently 
> device compilation and host compilation are separate process.
> 
> That said, when we emit diags for captures by reference, we should only emit 
> them when the lambdas are emitted as device functions. When they are emitted 
> as host functions in host compilation, these captures are valid and should 
> not be diagnosed.
HD lambda capturing something that's side-specific is similar to HD function 
calling side-specific function. I still think that the general principle 
applies to both sides of the compilation.

We may be saved by the fact that functions seem to be the only things that are 
truly side-specific and wrong-side access will already produce postponed 
diagnostics. For variables we'll end up capturing shadows, which is in the gray 
area -- we're allowed to use pointers & sizeof(), but not the non-const values.

Perhaps this condition check should be folded into the `CUDACheckLambdaCapture` 
and add few comments about the reasons for particular OK/not-OK choices we make 
there.



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

https://reviews.llvm.org/D78655



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

[PATCH] D83250: [clang] Enable errors for undefined TARGET_OS_ macros in Darwin driver

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 275821.
zixuw added a comment.

Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83250

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-warning-options.c


Index: clang/test/Driver/darwin-warning-options.c
===
--- /dev/null
+++ clang/test/Driver/darwin-warning-options.c
@@ -0,0 +1,7 @@
+// REQUIRES: system-darwin
+
+// Always error about undefined 'TARGET_OS_*' macros on Darwin.
+// RUN: %clang -### %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: "-Wundef-prefix=TARGET_OS_"
+// CHECK-DAG: "-Werror=undef-prefix"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -954,6 +954,10 @@
 : Darwin(D, Triple, Args) {}
 
 void DarwinClang::addClangWarningOptions(ArgStringList ) const {
+  // Always error about undefined 'TARGET_OS_*' macros.
+  CC1Args.push_back("-Wundef-prefix=TARGET_OS_");
+  CC1Args.push_back("-Werror=undef-prefix");
+
   // For modern targets, promote certain warnings to errors.
   if (isTargetWatchOSBased() || getTriple().isArch64Bit()) {
 // Always enable -Wdeprecated-objc-isa-usage and promote it


Index: clang/test/Driver/darwin-warning-options.c
===
--- /dev/null
+++ clang/test/Driver/darwin-warning-options.c
@@ -0,0 +1,7 @@
+// REQUIRES: system-darwin
+
+// Always error about undefined 'TARGET_OS_*' macros on Darwin.
+// RUN: %clang -### %s 2>&1 | FileCheck %s
+
+// CHECK-DAG: "-Wundef-prefix=TARGET_OS_"
+// CHECK-DAG: "-Werror=undef-prefix"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -954,6 +954,10 @@
 : Darwin(D, Triple, Args) {}
 
 void DarwinClang::addClangWarningOptions(ArgStringList ) const {
+  // Always error about undefined 'TARGET_OS_*' macros.
+  CC1Args.push_back("-Wundef-prefix=TARGET_OS_");
+  CC1Args.push_back("-Werror=undef-prefix");
+
   // For modern targets, promote certain warnings to errors.
   if (isTargetWatchOSBased() || getTriple().isArch64Bit()) {
 // Always enable -Wdeprecated-objc-isa-usage and promote it
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81751: [SemaObjC] Fix a -Wobjc-signed-char-bool false-positive with binary conditional operator

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a reviewer: ahatanak.
erik.pilkington added a comment.

Ping!


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

https://reviews.llvm.org/D81751



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

[PATCH] D83254: [X86] Enabled a bunch of 64-bit Interlocked* functions intrinsics on 32-bit Windows to match recent MSVC

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: RKSimon, spatel, rnk, erichkeane.
Herald added a subscriber: jfb.

This enables 
_InterlockedAnd64/_InterlockedOr64/_InterlockedXor64/_InterlockedDecrement64/_InterlockedIncrement64/_InterlockedExchange64/_InterlockedExchangeAdd64/_InterlockedExchangeSub64
 on 32-bit Windows

The backend already knows how to expand these to a loop using cmpxchg8b on 
32-bit targets.

Fixes PR46595


https://reviews.llvm.org/D83254

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/ms-intrinsics.c

Index: clang/test/CodeGen/ms-intrinsics.c
===
--- clang/test/CodeGen/ms-intrinsics.c
+++ clang/test/CodeGen/ms-intrinsics.c
@@ -523,72 +523,72 @@
 // CHECK: store volatile i64 %v, i64* %p
 
 
-#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)
+#if defined(__i386__) || defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)
 __int64 test_InterlockedExchange64(__int64 volatile *value, __int64 mask) {
   return _InterlockedExchange64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedExchange64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw xchg i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedExchange64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw xchg i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedExchangeAdd64(__int64 volatile *value, __int64 mask) {
   return _InterlockedExchangeAdd64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedExchangeAdd64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw add i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedExchangeAdd64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw add i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedExchangeSub64(__int64 volatile *value, __int64 mask) {
   return _InterlockedExchangeSub64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedExchangeSub64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw sub i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedExchangeSub64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw sub i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedOr64(__int64 volatile *value, __int64 mask) {
   return _InterlockedOr64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedOr64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw or i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedOr64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw or i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedXor64(__int64 volatile *value, __int64 mask) {
   return _InterlockedXor64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedXor64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw xor i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedXor64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw xor i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedAnd64(__int64 volatile *value, __int64 mask) {
   return _InterlockedAnd64(value, mask);
 }
-// CHECK-ARM-X64: define{{.*}}i64 @test_InterlockedAnd64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
-// CHECK-ARM-X64:   [[RESULT:%[0-9]+]] = atomicrmw and i64* %value, i64 %mask seq_cst
-// CHECK-ARM-X64:   ret i64 [[RESULT:%[0-9]+]]
-// CHECK-ARM-X64: }
+// CHECK: define{{.*}}i64 @test_InterlockedAnd64(i64*{{[a-z_ ]*}}%value, i64{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK:   [[RESULT:%[0-9]+]] = atomicrmw and i64* %value, i64 %mask seq_cst
+// CHECK:   ret i64 [[RESULT:%[0-9]+]]
+// CHECK: }
 
 __int64 test_InterlockedIncrement64(__int64 volatile *Addend) {
   return _InterlockedIncrement64(Addend);
 }
-// 

[PATCH] D82728: [clang] Add -Wsuggest-override

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D82728#2133951 , @Quuxplusone wrote:

> In D82728#2133720 , @dblaikie wrote:
>
> > (the presence of at least one "override" being a signal the user intended 
> > to use override and missed some [...]
>
>
> I'm in favor of `-Wsuggest-override`, and would definitely use it if it 
> existed.


I've no doubt a non-trivial number of folks would - we'd probably enable it in 
LLVM itself.

> The problem I see with `-Wmissing-override`, as it's been implemented, is 
> that it uses the wrong granularity for "intent": it looks only across the 
> methods of a single class, rather than across all the classes of a single 
> header, or across a single translation unit, or across my entire codebase. In 
> real life, I //always// want to look across my entire codebase (excluding 
> system headers). If //any// class in my project uses `override`, I want Clang 
> to take that as a clear declaration of intent to use `override` throughout; I 
> don't want Clang treating class A differently from class B. But of course 
> Clang can't look at my whole codebase simultaneously.

Right - Clang's doing its best (well, debatable - that's what we're debating) 
with what it's got.

> So the next best thing is to give the user a simple way to "preload the 
> intent flag": to say "As soon as you start processing //any// class, please 
> act as if intent has been declared for that class." Adding 
> `-Wsuggest-override` to my build line seems like a perfect way to implement 
> that "preload" facility.

The issue is that such a warning then needs to be off by default, because we 
can't assume the user's intent. And Clang's historically been fairly averse to 
off-by-default warnings due to low user-penetration (not zero, like I said, I 
imagine LLVM itself would use such a warning, were it implemented) & so 
concerns about the cost/benefit tradeoff of the added complexity (source code 
and runtime) of the feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82728



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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 275811.
nhaehnle marked 4 inline comments as done.
nhaehnle added a comment.

- fix MachineCfgTraits::blockdef_iterator and allow it to iterate over the 
instructions in a bundle
- use MachineBasicBlock::printName


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/CodeGen/MachineCfgTraits.h
  llvm/include/llvm/IR/CFG.h
  llvm/include/llvm/Support/CfgTraits.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/MachineCfgTraits.cpp
  llvm/lib/IR/CFG.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CfgTraits.cpp
  llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
  mlir/include/mlir/IR/Dominance.h

Index: mlir/include/mlir/IR/Dominance.h
===
--- mlir/include/mlir/IR/Dominance.h
+++ mlir/include/mlir/IR/Dominance.h
@@ -10,8 +10,37 @@
 #define MLIR_IR_DOMINANCE_H
 
 #include "mlir/IR/RegionGraphTraits.h"
+#include "llvm/Support/CfgTraits.h"
 #include "llvm/Support/GenericDomTree.h"
 
+namespace mlir {
+
+/// Partial CFG traits for MLIR's CFG, without a value type.
+class CfgTraitsBase : public llvm::CfgTraitsBase {
+public:
+  using ParentType = Region;
+  using BlockRef = Block *;
+  using ValueRef = void;
+
+  static llvm::CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(llvm::CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class CfgTraits : public llvm::CfgTraits {
+  static Region *getBlockParent(Block *block) { return block->getParent(); }
+};
+
+} // namespace mlir
+
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;
+};
+
 extern template class llvm::DominatorTreeBase;
 extern template class llvm::DominatorTreeBase;
 
Index: llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
===
--- llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
+++ llvm/lib/Transforms/Vectorize/VPlanDominatorTree.h
@@ -18,9 +18,33 @@
 #include "VPlan.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/Support/CfgTraits.h"
 
 namespace llvm {
 
+/// Partial CFG traits for VPlan's CFG, without a value type.
+class VPCfgTraitsBase : public CfgTraitsBase {
+public:
+  using ParentType = VPRegionBlock;
+  using BlockRef = VPBlockBase *;
+  using ValueRef = void;
+
+  static CfgBlockRef toGeneric(BlockRef block) {
+return makeOpaque(block);
+  }
+  static BlockRef fromGeneric(CfgBlockRef block) {
+return static_cast(getOpaque(block));
+  }
+};
+
+class VPCfgTraits : public CfgTraits {
+  static VPRegionBlock *getBlockParent(VPBlockBase *block) {
+return block->getParent();
+  }
+};
+
+template <> struct CfgTraitsFor { using CfgTraits = VPCfgTraits; };
+
 /// Template specialization of the standard LLVM dominator tree utility for
 /// VPBlockBases.
 using VPDominatorTree = DomTreeBase;
Index: llvm/lib/Support/CfgTraits.cpp
===
--- /dev/null
+++ llvm/lib/Support/CfgTraits.cpp
@@ -0,0 +1,14 @@
+//===- CfgTraits.cpp - Traits for generically working on CFGs ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/CfgTraits.h"
+
+using namespace llvm;
+
+void CfgInterface::anchor() {}
+void CfgPrinter::anchor() {}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -71,6 +71,7 @@
   BranchProbability.cpp
   BuryPointer.cpp
   CachePruning.cpp
+  CfgTraits.cpp
   circular_raw_ostream.cpp
   Chrono.cpp
   COM.cpp
Index: llvm/lib/IR/CMakeLists.txt
===
--- llvm/lib/IR/CMakeLists.txt
+++ llvm/lib/IR/CMakeLists.txt
@@ -4,6 +4,7 @@
   Attributes.cpp
   AutoUpgrade.cpp
   BasicBlock.cpp
+  CFG.cpp
   Comdat.cpp
   ConstantFold.cpp
   ConstantRange.cpp
Index: llvm/lib/IR/CFG.cpp
===
--- /dev/null
+++ llvm/lib/IR/CFG.cpp
@@ -0,0 +1,56 @@
+//===- CFG.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/IR/CFG.h"

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:133
+  ++m_def;
+  if (m_def == m_instr->defs().end()) {
+++m_instr;

arsenm wrote:
> != return early?
The logic is actually subtly broken in the presence of instructions without 
defs, I just didn't notice it because it currently affects only debug printing 
logic. Going to fix it.



Comment at: llvm/include/llvm/CodeGen/MachineCfgTraits.h:136-138
+  // Prefer to avoid support for bundled instructions as long as we
+  // don't really need it.
+  assert(!m_instr->isBundle());

arsenm wrote:
> I've been thinking about more aggressively using bundles around call sites to 
> handle waterfall looping around divergent calls with SGPR arguments
Hmm, so what's the correct iteration behavior in the presence of bundles? 
Iterate over all instructions in the bundle (which is that 
MachineBasicBlock::instr_iterator does) and only iterate over explicit defs? I 
think that's what makes the most sense, and what I'm going with for now...



Comment at: llvm/lib/CodeGen/MachineCfgTraits.cpp:27-29
+void MachineCfgTraits::Printer::printBlockName(raw_ostream ,
+   MachineBasicBlock *block) const 
{
+  out << "bb." << block->getNumber();

arsenm wrote:
> I think this should be added to MachineBasicBlock. The same logic is already 
> repeated in MIRPrinter (and the MBB dump function uses a different prefix)
D83253


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088



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

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

BTW, I just noticed recently that we have a -mlinker-version= flag, too, which 
is only used on darwin at the moment. That's another instance of "we need to 
condition behavior based on what linker we're invoking", but in this case, 
between multiple versions of apple's linker, rather than which brand of linker. 
That doesn't impact this directly, but just thought I'd mention it as it's in 
the same area of concern.




Comment at: clang/lib/Driver/ToolChain.cpp:556-557
+  // -fld-path= takes precedence over -fuse-ld= and specifies the executable
+  // name. PATH is consulted if the value does not contain /. -B is
+  // intentionally ignored.
+  if (const Arg *A = Args.getLastArg(options::OPT_fld_path_EQ)) {

Shouldn't this use -B paths? Why do we want to ignore them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015



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

[PATCH] D83144: Allow to specify macro names for android-comparison-in-temp-failure-retry.

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a reviewer: alexfh.
george.burgess.iv added a comment.

Concept and implementation LGTM. Please wait for comment from +alexfh before 
landing, since I think they have more ownership over clang-tidy in general than 
I do :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83144



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

[PATCH] D83084: DomTree: Remove the releaseMemory() method

[Nicolai Hähnle via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG723a44c9b5d6: DomTree: Remove the releaseMemory() method 
(authored by nhaehnle).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83084

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  llvm/include/llvm/Analysis/PostDominators.h
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/GenericDomTree.h


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 +167,7 @@
   }
 
   /// Releases the memory held by the dominator tree.
-  virtual void releaseMemory() {
-DT.releaseMemory();
-  }
+  virtual void releaseMemory() { DT.reset(); }
 
   /// Converts the dominator tree to human readable form.
   virtual void print(raw_ostream , const llvm::Module* M= nullptr) const {


Index: llvm/include/llvm/Support/GenericDomTree.h
===
--- llvm/include/llvm/Support/GenericDomTree.h
+++ llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 
Index: llvm/include/llvm/IR/Dominators.h
===
--- llvm/include/llvm/IR/Dominators.h
+++ llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };
Index: llvm/include/llvm/Analysis/PostDominators.h
===
--- llvm/include/llvm/Analysis/PostDominators.h
+++ llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 

[clang] 723a44c - DomTree: Remove the releaseMemory() method

[Nicolai Hähnle via cfe-commits]

Author: Nicolai Hähnle
Date: 2020-07-06T21:58:11+02:00
New Revision: 723a44c9b5d654ec791720fc450757ae00f9e631

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

LOG: DomTree: Remove the releaseMemory() method

Summary:
It is fully redundant with reset().

Change-Id: I25850b9f08eace757cf03cbb8780e970aca7f51a

Reviewers: arsenm, RKSimon, mehdi_amini, courbet

Subscribers: wdng, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/Dominators.h
llvm/include/llvm/Analysis/PostDominators.h
llvm/include/llvm/IR/Dominators.h
llvm/include/llvm/Support/GenericDomTree.h

Removed: 




diff  --git a/clang/include/clang/Analysis/Analyses/Dominators.h 
b/clang/include/clang/Analysis/Analyses/Dominators.h
index 061c98137da2..51d86f6e4540 100644
--- a/clang/include/clang/Analysis/Analyses/Dominators.h
+++ b/clang/include/clang/Analysis/Analyses/Dominators.h
@@ -167,9 +167,7 @@ class CFGDominatorTreeImpl : public ManagedAnalysis {
   }
 
   /// Releases the memory held by the dominator tree.
-  virtual void releaseMemory() {
-DT.releaseMemory();
-  }
+  virtual void releaseMemory() { DT.reset(); }
 
   /// Converts the dominator tree to human readable form.
   virtual void print(raw_ostream , const llvm::Module* M= nullptr) const {

diff  --git a/llvm/include/llvm/Analysis/PostDominators.h 
b/llvm/include/llvm/Analysis/PostDominators.h
index 801eb3d59673..296110d8d03b 100644
--- a/llvm/include/llvm/Analysis/PostDominators.h
+++ b/llvm/include/llvm/Analysis/PostDominators.h
@@ -88,9 +88,7 @@ struct PostDominatorTreeWrapperPass : public FunctionPass {
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override {
-DT.releaseMemory();
-  }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module*) const override;
 };

diff  --git a/llvm/include/llvm/IR/Dominators.h 
b/llvm/include/llvm/IR/Dominators.h
index a79be8779b7e..0084ac0b655a 100644
--- a/llvm/include/llvm/IR/Dominators.h
+++ b/llvm/include/llvm/IR/Dominators.h
@@ -277,7 +277,7 @@ class DominatorTreeWrapperPass : public FunctionPass {
 AU.setPreservesAll();
   }
 
-  void releaseMemory() override { DT.releaseMemory(); }
+  void releaseMemory() override { DT.reset(); }
 
   void print(raw_ostream , const Module *M = nullptr) const override;
 };

diff  --git a/llvm/include/llvm/Support/GenericDomTree.h 
b/llvm/include/llvm/Support/GenericDomTree.h
index d93293864a65..e83e7aa39e7a 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -325,8 +325,6 @@ class DominatorTreeBase {
 return false;
   }
 
-  void releaseMemory() { reset(); }
-
   /// getNode - return the (Post)DominatorTree node for the specified basic
   /// block.  This is the same as using operator[] on this class.  The result
   /// may (but is not required to) be null for a forward (backwards)
@@ -760,9 +758,6 @@ class DominatorTreeBase {
 return DomTreeBuilder::Verify(*this, VL);
   }
 
-protected:
-  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
-
   void reset() {
 DomTreeNodes.clear();
 Roots.clear();
@@ -772,6 +767,9 @@ class DominatorTreeBase {
 SlowQueries = 0;
   }
 
+protected:
+  void addRoot(NodeT *BB) { this->Roots.push_back(BB); }
+
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template 



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

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:82
 "bugprone-bool-pointer-implicit-conversion");
-CheckFactories.registerCheck(
-"bugprone-branch-clone");
+CheckFactories.registerCheck("bugprone-branch-clone");
 CheckFactories.registerCheck(

It looks like unrelated formatting changes may have snuck in?



Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:121
 "bugprone-narrowing-conversions");
+CheckFactories.registerCheck("bugprone-no-escape");
 CheckFactories.registerCheck(

Given that this is limited to Objective-C, why register this under `bugprone` 
as opposed to the `objc` module? Alternatively, why limit to Objective-C when 
blocks can be used in other modes like C or C++ with `-fblocks`?



Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:34
+  const BlockDecl *EscapingBlockDecl = MatchedEscapingBlock->getBlockDecl();
+  for (const BlockDecl::Capture  : EscapingBlockDecl->captures()) {
+const VarDecl *Var = CapturedVar.getVariable();

This makes me think we should extend the `hasAnyCaptures()` AST matcher so it 
works with blocks as well as lambdas. It would be nice to do all of this from 
the matcher interface.



Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:37
+if (Var && Var->hasAttr()) {
+  diag(MatchedEscapingBlock->getBeginLoc(),
+   "pointer %0 with attribute 'noescape' is captured by an "

Given that the capture is the issue (not the block), why not point to the use 
of the captured variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82904



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

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In D82611#2125868 , @MadCoder wrote:

> So the risk with that one is that if someone had say the `-didSomething` 
> direct selector and that some UIKit/Apple SDK API now adds this as a thing 
> that you use with CFNotification center for example, where you _need_ 
> dynamism, then the uses of the API would warn all the time when the internal 
> to the client project `-didSomething` is in scope.
>
> So we ideally need a way to silence this warning, so at the very least this 
> needs to be a new `-W` flag (on by default (?)) so that callers that have an 
> issue can use a `_Pragma` at the call site to ignore the error for when it's 
> legal.


The `DiagGroup<"potentially-direct-selector">` creates a command line flag, so 
the pragma thing works. I couldn't think of a decent way of suppressing this 
without pragmas, but ISTM that this will have a low enough false-positive rate 
that it won't be a big problem.

> I would suggest something like `-Wstrict-direct-dispatch` or something.

I kinda prefer `-Wpotentially-direct-selector`, since that seems to more 
closely correspond to what the compiler is complaining about. WDYT?




Comment at: clang/test/SemaObjC/potentially-direct-selector.m:84
+  (void)@selector(inBaseCat);
+  (void)@selector(inBaseCatImpl);
+  (void)@selector(inDerived);

MadCoder wrote:
> I think this might be too weak, if we add a warning then maybe what we want 
> is a tri-state:
> 
> `-Wstrict-direct-dispatch=none|self|all`
> 
> because I can see cases where the heuristic here would be too weak
Sure - new patch adds an off-by-default "strict" variant, since I think that's 
a bit more common in clang then the `-Wx=y` approach.


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

https://reviews.llvm.org/D82611



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

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 275759.
erik.pilkington marked 3 inline comments as done.
erik.pilkington added a comment.

Add an off-by-default variant of this warning.


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

https://reviews.llvm.org/D82611

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/potentially-direct-selector.m

Index: clang/test/SemaObjC/potentially-direct-selector.m
===
--- /dev/null
+++ clang/test/SemaObjC/potentially-direct-selector.m
@@ -0,0 +1,157 @@
+// RUN: %clang_cc1 %s -Wpotentially-direct-selector -verify
+// RUN: %clang_cc1 %s -Wstrict-potentially-direct-selector -verify=expected,strict
+
+#define NS_DIRECT __attribute__((objc_direct))
+
+__attribute__((objc_root_class))
+@interface Dummies
+-(void)inBase;
+-(void)inBaseImpl;
+-(void)inBaseCat;
+-(void)inBaseCatImpl;
+-(void)inDerived;
+-(void)inDerivedImpl;
+-(void)inDerivedCat;
+-(void)inDerivedCatImpl;
++(void)inBaseClass;
++(void)inDerivedClass;
++(void)inDerivedCatClass;
+@end
+
+__attribute__((objc_root_class))
+@interface Base
+-(void)inBase NS_DIRECT; // expected-note + {{direct method}}
++(void)inBaseClass NS_DIRECT;  // expected-note + {{direct method}}
+@end
+
+@implementation Base
+-(void)inBaseImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inBase {}
++(void)inBaseClass {}
+@end
+
+@interface Base (Cat)
+-(void)inBaseCat NS_DIRECT; // expected-note + {{direct method}}
+@end
+
+@implementation Base (Cat)
+-(void)inBaseCatImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inBaseCat {}
+@end
+
+@interface Derived : Base
+-(void)inDerived NS_DIRECT; // expected-note + {{direct method}}
++(void)inDerivedClass NS_DIRECT;  // expected-note + {{direct method}}
+@end
+
+@implementation Derived
+-(void)inDerivedImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inDerived {}
++(void)inDerivedClass {}
+@end
+
+@interface Derived (Cat)
+-(void)inDerivedCat NS_DIRECT; // expected-note + {{direct method}}
++(void)inDerivedCatClass NS_DIRECT; // expected-note + {{direct method}}
+@end
+
+@implementation Derived (Cat)
+-(void)inDerivedCatImpl NS_DIRECT { // expected-note + {{direct method}}
+}
+-(void)inDerivedCat {}
++(void)inDerivedCatClass {}
+
+-(void)test1 {
+  (void)@selector(inBase); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseImpl); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseCat); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseCatImpl); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerived); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedImpl); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCat); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCatImpl); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedClass); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseClass); // expected-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCatClass); // expected-warning{{@selector expression formed with potentially direct selector}}
+}
+@end
+
+void test2() {
+  (void)@selector(inBase); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseImpl); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseCat); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerived); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedImpl); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCat); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCatImpl); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedClass); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inBaseClass); // strict-warning{{@selector expression formed with potentially direct selector}}
+  (void)@selector(inDerivedCatClass); // strict-warning{{@selector expression formed 

[PATCH] D82800: [OPENMP50] extend array section for stride (Parsing/Sema/AST)

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 8 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:1933
 }
+if (getLangOpts().OpenMP >= 50 && Tok.is(tok::colon)) {
+  // Consume ':'

ABataev wrote:
> Seems to me, it is too broad. According to the standard, it must be allowed 
> only in to/from clauses. 
I did the check in Sema, I'll move the check here. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82800



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

[PATCH] D83071: Add support for options with two flags for controlling the same field.

[Daniel Grumberg via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3931-3932
   if (((FLAGS)::CC1Option) &&  
\
   (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {
\
-if (Option::KIND##Class == Option::FlagClass) {
\
-  Args.push_back(SPELLING);
\
-}  
\
-if (Option::KIND##Class == Option::SeparateClass) {
\
-  Args.push_back(SPELLING);
\
-  DENORMALIZER(Args, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   
\
-}  
\
+DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   
\
   }

dang wrote:
> dexonsmith wrote:
> > I realize this commit doesn't introduce it, but it seems unfortunate to 
> > call `EXTRACTOR` twice. Maybe in a follow-up or prep commit you can fix 
> > that... maybe something like this?
> > ```
> >   if ((FLAGS)::CC1Option) {
> > const auto  = EXTRACTOR(this->KEYPATH);
> > if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)
> >   DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, 
> > EXTRACTOR(this->KEYPATH));
> >   }
> > ```
> Yes I can do that of course. Although EXTRACTOR is meant to be very cheap and 
> in most cases it expands to just `this->KEYPATH`
See D83211


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83071



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

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid SourceLocation

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:118
 
+syntax::NodeKind getOperatorNodeKind(const CXXOperatorCallExpr ) {
+  switch (E.getOperator()) {

eduucaldas wrote:
> # Where to put this logic? 
> The pro of having this function inside `BuildTree.cpp` is that it is closer 
> to it's *only* usage. And also for now `BuildTree.cpp` agglomerates 
> everything that takes a semantic AST as an input, so it would be coherent.
> 
> Another option is to put it in `Nodes`, as it might serve as documentation 
> for `*OperatorExpression`s. 
> For example, it would document that the semantic node `CXXOperatorCallExpr` 
> can also generate the syntax node `BinaryOperatorExpression`, which was 
> previously only generated by a semantic `BinaryOperator`
> 
> Another option is to add put this function as a lambda inside 
> `WalkUpFromCXXOperatorCallExpr` as probably it will only be used there. 
Placing this helper here makes sense to me. However, please mark it static.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:178
+return syntax::NodeKind::BinaryOperatorExpression;
+  // Not supported by SyntaxTree
+  case OO_New:

"Not supported by syntax tree *yet*"



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:741
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
+if (S->getLocation().isInvalid())
+  return true;

WDYT about overriding `TraverseCXXOperatorCallExpr`, so that we don't even 
visit the synthetic argument of the postfix unary `++`? I would prefer to not 
introduce blanket "if invalid then ignore" checks in the code.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:852
+default:
+  llvm_unreachable("getKind does not implement that");
 }

"getOperatorNodeKind() does not return this value"



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2333
+struct X {
+  X operator++(int);
+};

Could you also add tests for a prefix unary operator?



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:2376
+| | |   `-x
+| | `-IdExpression
+| |   `-UnqualifiedId

I'm not sure about this part where `++` is wrapped in IdExpression -- shouldn't 
the syntax tree look identical to a builtin postfix `++` (see another test in 
this file)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82954



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2409
+const RecordDecl *RD = RT->getDecl();
+return std::max(ABIAlign, static_cast(toBits(
+  getASTRecordLayout(RD).PreferredAlignment)));

Please add a comment regarding the situations where the `ABIAlign` value is 
greater than the `PreferredAlignment` value. It may be appropriate to assert 
that, absent those cases, the `PreferredAlignment` value is at least that of 
`ABIAlign`.



Comment at: clang/lib/AST/ASTContext.cpp:2409
+const RecordDecl *RD = RT->getDecl();
+return std::max(ABIAlign, static_cast(toBits(
+  getASTRecordLayout(RD).PreferredAlignment)));

hubert.reinterpretcast wrote:
> Please add a comment regarding the situations where the `ABIAlign` value is 
> greater than the `PreferredAlignment` value. It may be appropriate to assert 
> that, absent those cases, the `PreferredAlignment` value is at least that of 
> `ABIAlign`.
It does not appear that the maximum of the two values is the correct answer:
```
struct C {
  double x;
} c;
typedef struct C __attribute__((__aligned__(2))) CC;

CC cc;
extern char x[__alignof__(cc)];
extern char x[2]; // this is okay with IBM XL C/C++
```



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1888
+BTy->getKind() == BuiltinType::LongDouble) {
+  PreferredAlign = CharUnits::fromQuantity(8);
+}

hubert.reinterpretcast wrote:
> I believe an assertion that `PreferredAlign` was 4 would be appropriate.
It seems that overriding the value should only be done after additional checks:
```
typedef double __attribute__((__aligned__(2))) Dbl;
struct A {
  Dbl x;
} a;
extern char x[__alignof__(a)];
extern char x[2]; // this is okay with IBM XL C/C++
```

I am getting concerned that the logic here overlaps quite a bit with 
`getPreferredTypeAlign` and refactoring to make the code here more common with 
`getPreferredTypeAlign` is necessary.


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

https://reviews.llvm.org/D79719



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1885
+  Context.getBaseElementType(CTy->getElementType())
+  ->getAs())
+if (BTy->getKind() == BuiltinType::Double ||

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > I believe `castAs` should be expected to succeed here.
> `castAs` is not declared in current context, do we really want to use it by 
> introducing one more header?
`castAs` should be declared:
https://clang.llvm.org/doxygen/classclang_1_1Type.html#a436b8b08ae7f2404b4712d37986194ce

Also, the additional point is that the `if` here should be unnecessary. `BTy` 
should not be null. We should use `castAs` to gain the guarantee (with 
assertions) that `BTy` is not null.


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

https://reviews.llvm.org/D79719



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

[PATCH] D83242: [RFC][BPF] support expr with typedef type for FIELD_EXISTENCE reloc

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added reviewers: anakryiko, ast.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

[The patch needs more work e.g. to create proper test, to be agreed on 
interface, etc.]

This patch added support of expression with typedef type
for FIELD_EXISTENCE relocation. This tries to address the 
following use case:

  enum { FIELD_EXISTENCE = 2, };
  typedef unsigned long u64;
  typedef unsigned int u32;
  struct bpf_perf_event_data_kern;
  typedef u64 (*btf_bpf_read_branch_records)(struct bpf_perf_event_data_kern *, 
void *, u32, u64);
  int test() {
 btf_bpf_read_branch_records a;
 return __builtin_preserve_field_info(a, FIELD_EXISTENCE);
  }

A relocation with a type of typedef 'btf_bpf_read_branch_records' will be
recorded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83242

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp


Index: llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
===
--- llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
+++ llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
@@ -277,7 +277,7 @@
   }
   if (GV->getName().startswith("llvm.bpf.preserve.field.info")) {
 CInfo.Kind = BPFPreserveFieldInfoAI;
-CInfo.Metadata = nullptr;
+CInfo.Metadata = Call->getMetadata(LLVMContext::MD_preserve_access_index);
 // Check validity of info_kind as clang did not check this.
 uint64_t InfoKind = getConstant(Call->getArgOperand(1));
 if (InfoKind >= BPFCoreSharedInfo::MAX_FIELD_RELOC_KIND)
@@ -742,6 +742,13 @@
   break;
 }
 
+if (CInfo.Kind == BPFPreserveFieldInfoAI) {
+  // typedef type.
+  TypeName = std::string(PossibleTypeDef->getName());
+  TypeMeta = PossibleTypeDef;
+  break;
+}
+
 assert(CInfo.Kind == BPFPreserveArrayAI);
 
 // Array entries will always be consumed for accumulative initial index.
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2582,6 +2582,15 @@
 return false;
   }
 
+  // The second argument needs to be a constant int
+  Arg = TheCall->getArg(1);
+  llvm::APSInt Value;
+  if (!Arg->isIntegerConstantExpr(Value, Context)) {
+Diag(Arg->getBeginLoc(), diag::err_preserve_field_info_not_const)
+<< 2 << Arg->getSourceRange();
+return true;
+  }
+
   // The first argument needs to be a record field access.
   // If it is an array element access, we delay decision
   // to BPF backend to check whether the access is a
@@ -2590,21 +2599,14 @@
   if (Arg->getType()->getAsPlaceholderType() ||
   (Arg->IgnoreParens()->getObjectKind() != OK_BitField &&
!dyn_cast(Arg->IgnoreParens()) &&
-   !dyn_cast(Arg->IgnoreParens( {
+   !dyn_cast(Arg->IgnoreParens()) &&
+   // Value 2 here represents reloc kind FIELD_EXISTENCE.
+   (Value != 2 || !Arg->getType()->getAs( {
 Diag(Arg->getBeginLoc(), diag::err_preserve_field_info_not_field)
 << 1 << Arg->getSourceRange();
 return true;
   }
 
-  // The second argument needs to be a constant int
-  Arg = TheCall->getArg(1);
-  llvm::APSInt Value;
-  if (!Arg->isIntegerConstantExpr(Value, Context)) {
-Diag(Arg->getBeginLoc(), diag::err_preserve_field_info_not_const)
-<< 2 << Arg->getSourceRange();
-return true;
-  }
-
   TheCall->setType(Context.UnsignedIntTy);
   return false;
 }
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -10966,11 +10966,17 @@
 ConstantInt *C = cast(EmitScalarExpr(E->getArg(1)));
 Value *InfoKind = ConstantInt::get(Int64Ty, C->getSExtValue());
 
+llvm::DIType *DbgInfo =
+getDebugInfo()->getOrCreateStandaloneType(E->getArg(0)->getType(),
+  E->getArg(0)->getExprLoc());
+
 // Built the IR for the preserve_field_info intrinsic.
 llvm::Function *FnGetFieldInfo = llvm::Intrinsic::getDeclaration(
 (), llvm::Intrinsic::bpf_preserve_field_info,
 {FieldAddr->getType()});
-return Builder.CreateCall(FnGetFieldInfo, {FieldAddr, InfoKind});
+CallInst *Fn = Builder.CreateCall(FnGetFieldInfo, {FieldAddr, InfoKind});
+Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
+return Fn;
   }
   case BPF::BI__builtin_btf_type_id: {
 Value *FieldVal = nullptr;


Index: llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
===
--- llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
+++ llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
@@ -277,7 +277,7 @@
   }
   if 

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82118



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

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Do you think there should be a hidden command line option to disable disabling 
blacklisted checks, purely to prevent hindering attempts to fix these 
problematic checks.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115
+// either due to crashes or false positives.
+const char *getClangTidyBlacklist() {
+  static const std::string FalsePositives =

Return by StringRef?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83224



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Bumping for review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



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

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Fixed here to get the bot running again: 
https://github.com/llvm/llvm-project/commit/7308e1432624f02d4e652ffa70e40d0eaa89fdb3


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82585



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

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

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

Szelethus added a comment.

In D82585#2133260 , @teemperor wrote:

> Fixed here to get the bot running again: 
> https://github.com/llvm/llvm-project/commit/7308e1432624f02d4e652ffa70e40d0eaa89fdb3


Thank you so much! Kind of ironic considering that this entire patch was all 
about fixing them in the first place :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82585



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

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 275728.
Xiangling_L marked 3 inline comments as done.
Xiangling_L added a comment.

Fixed -Wpacked warning issue;
Fixed EmptySubobjects related offset issue;
Fixed zero-extent array in a base class related issue;
Addressed other comments;


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

https://reviews.llvm.org/D79719

Files:
  clang/include/clang/AST/RecordLayout.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayout.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.h
  clang/test/Layout/aix-Wpacked.cpp
  clang/test/Layout/aix-double-struct-member.cpp
  clang/test/Layout/aix-no-unique-address-with-double.cpp
  clang/test/Layout/aix-virtual-function-and-base-with-double.cpp

Index: clang/test/Layout/aix-virtual-function-and-base-with-double.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-virtual-function-and-base-with-double.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc64-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+namespace test1 {
+struct A {
+  double d1;
+  virtual void boo() {}
+};
+
+struct B {
+  double d2;
+  A a;
+};
+
+struct C : public A {
+  double d3;
+};
+
+int i = sizeof(B);
+int j = sizeof(C);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::A
+// CHECK-NEXT:0 |   (A vtable pointer)
+// CHECK32-NEXT:  4 |   double d1
+// CHECK32-NEXT:| [sizeof=12, dsize=12, align=4, preferredalign=4,
+// CHECK32-NEXT:|  nvsize=12, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:  8 |   double d1
+// CHECK64-NEXT:| [sizeof=16, dsize=16, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=16, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::B
+// CHECK-NEXT:0 |   double d2
+// CHECK-NEXT:8 |   struct test1::A a
+// CHECK-NEXT:8 | (A vtable pointer)
+// CHECK32-NEXT: 12 | double d1
+// CHECK32-NEXT:| [sizeof=24, dsize=20, align=4, preferredalign=8,
+// CHECK32-NEXT:|  nvsize=20, nvalign=4, preferrednvalign=8]
+// CHECK64-NEXT: 16 | double d1
+// CHECK64-NEXT:| [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=24, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test1::C
+// CHECK-NEXT:0 |   struct test1::A (primary base)
+// CHECK-NEXT:0 | (A vtable pointer)
+// CHECK32-NEXT:  4 | double d1
+// CHECK32-NEXT: 12 |   double d3
+// CHECK32-NEXT:| [sizeof=20, dsize=20, align=4, preferredalign=4,
+// CHECK32-NEXT:|  nvsize=20, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:  8 | double d1
+// CHECK64-NEXT: 16 |   double d3
+// CHECK64-NEXT:| [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=24, nvalign=8, preferrednvalign=8]
+
+} // namespace test1
+
+namespace test2 {
+struct A {
+  long long l1;
+};
+
+struct B : public virtual A {
+  double d2;
+};
+
+#pragma pack(2)
+struct C : public virtual A {
+  double __attribute__((aligned(4))) d3;
+};
+
+int i = sizeof(B);
+int j = sizeof(C);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::A
+// CHECK-NEXT:0 |   long long l1
+// CHECK-NEXT:  | [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK-NEXT:  |  nvsize=8, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::B
+// CHECK-NEXT:0 |   (B vtable pointer)
+// CHECK32-NEXT:  4 |   double d2
+// CHECK64-NEXT:  8 |   double d2
+// CHECK-NEXT:   16 |   struct test2::A (virtual base)
+// CHECK-NEXT:   16 | long long l1
+// CHECK-NEXT:  | [sizeof=24, dsize=24, align=8, preferredalign=8,
+// CHECK32-NEXT:|  nvsize=12, nvalign=4, preferrednvalign=4]
+// CHECK64-NEXT:|  nvsize=16, nvalign=8, preferrednvalign=8]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:0 | struct test2::C
+// CHECK-NEXT:0 |   (C vtable pointer)
+// CHECK32-NEXT:  4 |   double d3
+// CHECK32-NEXT: 12 |   struct test2::A (virtual base)
+// CHECK32-NEXT: 12 | long long l1
+// 

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

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

Szelethus added a comment.

I'm on it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82585



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