[PATCH] D75159: [clang-tidy] Fix PR#37210 'Out-of-class template constructor only half-fixed with modernize-pass-by-value'

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

Can you commit it please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75159



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

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D75271#1896724 , @Szelethus wrote:

> Let's change this to either `CheckerManger` or `AnalysisManager`.


Did you read my comments? I tried `CheckerManager` first, but it does not work, 
because `CheckerRegistry` does not have it. If I pass it to `CheckerRegistry`, 
then the `printXXX()` functions will not compile, because they do not have it.

In D75271#1896182 , @Szelethus wrote:

> Well, I like to say that "Any manager can be retrieved in clang at arbitrary 
> places if you try hard enough", so I think either that //actually// satisfies 
> this would be fine :)


This is not always true, especially if the manager does not exist at that 
particular place. There is not `CheckerManager` in the `printXXX()` functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271



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

[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Stefanos Baziotis via Phabricator via cfe-commits]

baziotis added a comment.

In D74801#1897267 , @lattner wrote:

> Seems fine to me.


Thank you! Please note that I don't have commit access.


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

https://reviews.llvm.org/D74801



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

[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

[Chris Lattner via Phabricator via cfe-commits]

lattner accepted this revision.
lattner added a comment.

Seems fine to me.


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

https://reviews.llvm.org/D74801



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D75285#1896458 , @rjmccall wrote:

> Unfortunately, `const` also doesn't mean that the memory doesn't change.   It 
> does mean it can't be changed through this pointer, but `restrict` allows you 
> to derive more pointers from it within the `restrict` scope, and those 
> pointers can remove the `const` qualifier.


If users derive a non-const pointer from the const pointer and modify it, 
doesn't that result in UB? Thanks.


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

https://reviews.llvm.org/D75285



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

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

I still got assertion when I use the built clang with check-mlir. The reduced 
testcase is

  class A {
  public:
int foo();
  };
  
  static A a;
  
  struct B {
B(int x = a.foo());
  };
  
  void test() {
B x;
  }

The assertion I got is:

  clang: /home/yaxunl/git/llvm/llvm/tools/clang/lib/CodeGen/CGExpr.cpp:2628: 
clang::CodeGen::LValue clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(const 
clang::DeclRefExpr *): Assertion `(ND->isUsed(false) || !isa(ND) || 
E->isNonOdrUse() || !E->getLocation().isValid()) && "Should not use decl 
without marking it used!"' failed.
  Stack dump:
  
  
   #0 0x0258c614 PrintStackTraceSignalHandler(void*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x258c614)
   #1 0x0258a1ae llvm::sys::RunSignalHandlers() 
(/home/yaxunl/git/llvm/assert/bin/clang+0x258a1ae)
   #2 0x0258b7a2 llvm::sys::CleanupOnSignal(unsigned long) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x258b7a2)
   #3 0x0251d0c3 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x251d0c3)
   #4 0x0251d1fc CrashRecoverySignalHandler(int) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x251d1fc)
   #5 0x7f0dde3bf390 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x11390)
   #6 0x7f0ddcf29428 raise 
/build/glibc-LK5gWL/glibc-2.23/signal/../sysdeps/unix/sysv/linux/raise.c:54:0
   #7 0x7f0ddcf2b02a abort 
/build/glibc-LK5gWL/glibc-2.23/stdlib/abort.c:91:0
   #8 0x7f0ddcf21bd7 __assert_fail_base 
/build/glibc-LK5gWL/glibc-2.23/assert/assert.c:92:0
   #9 0x7f0ddcf21c82 (/lib/x86_64-linux-gnu/libc.so.6+0x2dc82)
  #10 0x02a1a5df 
clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(clang::DeclRefExpr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a1a5df)
  #11 0x02a0dfb6 
clang::CodeGen::CodeGenFunction::EmitLValue(clang::Expr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a0dfb6)
  #12 0x02a39973 
clang::CodeGen::CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(clang::CallExpr
 const*, clang::CXXMethodDecl const*, clang::CodeGen::ReturnValueSlot, bool, 
clang::NestedNameSpecifier*, bool, clang::Expr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a39973)
  #13 0x02a389b9 
clang::CodeGen::CodeGenFunction::EmitCXXMemberCallExpr(clang::CXXMemberCallExpr 
const*, clang::CodeGen::ReturnValueSlot) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a389b9)
  #14 0x02a28f95 
clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*, 
clang::CodeGen::ReturnValueSlot) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a28f95)
  #15 0x02a5be29 (anonymous 
namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a5be29)
  #16 0x02a55b19 clang::StmtVisitorBase::Visit(clang::Stmt*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a55b19)
  #17 0x02a4b615 
clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*, bool) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a4b615)
  #18 0x02a0da30 
clang::CodeGen::CodeGenFunction::EmitAnyExpr(clang::Expr const*, 
clang::CodeGen::AggValueSlot, bool) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a0da30)
  #19 0x02a0edde 
clang::CodeGen::CodeGenFunction::EmitAnyExprToTemp(clang::Expr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a0edde)
  #20 0x029cdd6b 
clang::CodeGen::CodeGenFunction::EmitCallArg(clang::CodeGen::CallArgList&, 
clang::Expr const*, clang::QualType) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x29cdd6b)
  #21 0x029ccc41 
clang::CodeGen::CodeGenFunction::EmitCallArgs(clang::CodeGen::CallArgList&, 
llvm::ArrayRef, 
llvm::iterator_range >, 
clang::CodeGen::CodeGenFunction::AbstractCallee, unsigned int, 
clang::CodeGen::CodeGenFunction::EvaluationOrder) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x29ccc41)
  #22 0x028d8e7b void 
clang::CodeGen::CodeGenFunction::EmitCallArgs(clang::CodeGen::CallArgList&,
 clang::FunctionProtoType const*, 
llvm::iterator_range >, 
clang::CodeGen::CodeGenFunction::AbstractCallee, unsigned int, 
clang::CodeGen::CodeGenFunction::EvaluationOrder) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x28d8e7b)
  #23 0x029de431 
clang::CodeGen::CodeGenFunction::EmitCXXConstructorCall(clang::CXXConstructorDecl
 const*, clang::CXXCtorType, bool, bool, clang::CodeGen::AggValueSlot, 
clang::CXXConstructExpr const*) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x29de431)
  #24 0x02a3b84e 
clang::CodeGen::CodeGenFunction::EmitCXXConstructExpr(clang::CXXConstructExpr 
const*, clang::CodeGen::AggValueSlot) 
(/home/yaxunl/git/llvm/assert/bin/clang+0x2a3b84e)
  #25 0x02a32a8b (anonymous 
namespace)::AggExprEmitter::VisitCXXConstructExpr(clang::CXXConstructExpr 
const*) (/home/yaxunl/git/llvm/assert/bin/clang+0x2a32a8b)
  #26 0x02a2d44f 
clang::CodeGen::CodeGenFunction::EmitAggExpr(clang::Expr const*, 

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Here's what I think we should do: continue to have this method just return the 
cache line size. Then have another method that returns `true` or `false` for 
whether the given architecture supports aligned pairs of cache lines then, 
users of this (either in clang or libc++) can decide what they want to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/test/CodeGenCXX/attr-no-zero-initializer.cpp:40
+// CHECK: @unt = global %struct.nontrivial undef
+nontrivial unt [[clang::no_zero_initializer]];

Can you explain a bit about how this interacts with C++ constructors? Will this 
object not have its constructor called at startup; or is it that the 
constructor will be called but the memory simply won't have been zeroed 
//before// calling the constructor?

For [P1144 relocation](https://wg21.link/p1144) (D50114, D61761, etc), Anton 
Zhilin and I have been discussing the possibility of a similar-sounding 
attribute that would skip the constructor of a local variable altogether, 
allowing someone to write e.g.
```
T relocate(T *source) {
[[unconstructed]] T result;
memcpy(result, source, sizeof(T));
return result;
}
```
If your attribute does exactly that, then I'm interested. If your attribute 
doesn't do that, but is occupying real estate that //implies// that it does, 
then I'm also interested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361



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

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

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 247156.
JonChesterfield added a comment.

- clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/DeclBase.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-no-zero-initializer.cpp

Index: clang/test/Sema/attr-no-zero-initializer.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-no-zero-initializer.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+int good __attribute__((no_zero_initializer));
+const int still_cant_be_const __attribute__((no_zero_initializer)); // expected-error {{default initialization of an object of const type}}
+extern int external __attribute__((no_zero_initializer));
+
+void func() __attribute__((no_zero_initializer)) // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+{
+  int local __attribute__((no_zero_initializer)); // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+
+  static int sl __attribute__((no_zero_initializer));
+}
+
+struct s {
+  __attribute__((no_zero_initializer)) int field; // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+
+  static __attribute__((no_zero_initializer)) int sfield;
+
+} __attribute__((no_zero_initializer)); // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -94,6 +94,7 @@
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
 // CHECK-NEXT: NoThreadSafetyAnalysis (SubjectMatchRule_function)
 // CHECK-NEXT: NoThrow (SubjectMatchRule_hasType_functionType)
+// CHECK-NEXT: NoZeroInitializer (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function)
 // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
Index: clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @_ZZ4funcvE4data = internal global i32 undef
+int* func(void)
+{
+  static int data [[clang::no_zero_initializer]];
+  return 
+}
+
+// No code emitted
+extern int extern_unhelpful_but_harmless [[clang::no_zero_initializer]];
+
+// CHECK: @tentative = global i32 undef
+int tentative  [[clang::no_zero_initializer]];
+
+// CHECK: @_ZL16tentative_static = internal global i32 undef
+static int tentative_static [[clang::no_zero_initializer]] __attribute__((used));
+
+// CHECK: @nominally_zero_init = global i32 undef
+int nominally_zero_init  [[clang::no_zero_initializer]] = 0;
+
+// CHECK: @nominally_value_init = global i32 undef
+int nominally_value_init  [[clang::no_zero_initializer]] = 4;
+
+class trivial
+{
+  float x;
+};
+
+// CHECK: @ut = global %class.trivial undef
+trivial ut [[clang::no_zero_initializer]];
+
+struct nontrivial
+{
+  nontrivial() : x(3.14) {}
+  double x;
+};
+
+// CHECK: @unt = global %struct.nontrivial undef
+nontrivial unt [[clang::no_zero_initializer]];
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6505,6 +6505,11 @@
   D->addAttr(::new (S.Context) UninitializedAttr(S.Context, AL));
 }
 
+static void handleNoZeroInitializerAttr(Sema , Decl *D,
+const ParsedAttr ) {
+  D->addAttr(::new (S.Context) NoZeroInitializerAttr(S.Context, AL));
+}
+
 static bool tryMakeVariablePseudoStrong(Sema , VarDecl *VD,
 bool DiagnoseFailure) {
   QualType Ty = VD->getType();
@@ -7427,6 +7432,10 @@
 handleUninitializedAttr(S, D, AL);
 break;
 
+  case ParsedAttr::AT_NoZeroInitializer:
+handleNoZeroInitializerAttr(S, D, AL);
+break;
+
   case ParsedAttr::AT_ObjCExternallyRetained:
 handleObjCExternallyRetainedAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D75323: Support relative dest paths in headermap files

[Takuto Ikuta via Phabricator via cfe-commits]

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

add test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75323



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

[PATCH] D75323: Support relative dest paths in headermap files

[Simon Que via Phabricator via cfe-commits]

sque created this revision.
sque added reviewers: vsapsai, arphaman, akyrtzi.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Existing behavior:

- If hmap dest path is absolute: = Look for the file and return it as result. = 
If file not found, return None.
- If hmap dest path is relative: = Look for entry in headermap with the dest 
path as key, where the dest is a file that exists. = If key or file not found, 
return None.

New behavior:

- If hmap dest path is absolute: (unchanged) = look for the file and return it 
as result. = If not found, return None.
- If hmap dest path is relative: (changed) = Look for entry in headermap with 
the dest path as key, where the dest is a file that exists. = If key or file 
not found, fall back to looking for the file directly. = If file not found, 
return none.

Test case that uses a relative include:

1. Set up input files. mkdir headers touch headers/foo.h echo '#include 
"foo/foo.h"' > foo.cpp echo 'int main() { return 0; }' >> foo.cpp
2. Create hmap file using hmaptool:
3. https://github.com/llvm-mirror/clang/blob/master/utils/hmaptool/hmaptool
4. The hmap file has the mapping "foo/foo.h" -> "headers/foo.h" echo 
'{"mappings":{"foo/foo.h":"headers/foo.h"}}' > foo.json hmaptool write foo.json 
foo.hmap
5. Attempt to compile: clang foo.cpp -o foo -I foo.hmap

Without patch: "fatal error: 'foo/foo.h' file not found"
With patch: successfully compiles.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75323

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -415,7 +415,9 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
+// If lookup failed, fall back to file lookup using relative path directly.
+  }
+  if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75310: [CUDA, clang-cl] Filter out unsupported arguments for device-side compilation.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a subscriber: hans.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, but let us ask @hans for a second opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75310



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

[clang] b6f605c - Change test to use -S so it works when an external assembler is used that is not present in the testing environment.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2020-02-27T16:51:57-08:00
New Revision: b6f605cec5af0c4191de63de4b30ad984716f8a5

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

LOG: Change test to use -S so it works when an external assembler is used that 
is not present in the testing environment.

Added: 


Modified: 
clang/test/OpenMP/PR44893.c

Removed: 




diff  --git a/clang/test/OpenMP/PR44893.c b/clang/test/OpenMP/PR44893.c
index 1ba1127e874b..07b879b831e7 100644
--- a/clang/test/OpenMP/PR44893.c
+++ b/clang/test/OpenMP/PR44893.c
@@ -1,4 +1,4 @@
-// RUN: %clang -fopenmp -O -g -x c %s -c -disable-output -o %t
+// RUN: %clang -fopenmp -O -g -x c %s -S -disable-output -o %t
 
 // Do not crash ;)
 



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

[PATCH] D75319: Remove unused parameter from CXXRecordDecl::forallBases

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Apparently all users of the function were fine with short-circuiting
and none cared to override the default argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75319

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/CXXInheritance.cpp


Index: clang/lib/AST/CXXInheritance.cpp
===
--- clang/lib/AST/CXXInheritance.cpp
+++ clang/lib/AST/CXXInheritance.cpp
@@ -147,37 +147,27 @@
   return false;
 }
 
-bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches,
-bool AllowShortCircuit) const {
+bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches) const {
   SmallVector Queue;
 
   const CXXRecordDecl *Record = this;
-  bool AllMatches = true;
   while (true) {
 for (const auto  : Record->bases()) {
   const RecordType *Ty = I.getType()->getAs();
-  if (!Ty) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
-  }
+  if (!Ty)
+return false;
 
   CXXRecordDecl *Base =
 cast_or_null(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
+return false;
   }
 
   Queue.push_back(Base);
-  if (!BaseMatches(Base)) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
-  }
+  if (!BaseMatches(Base))
+return false;
 }
 
 if (Queue.empty())
@@ -185,7 +175,7 @@
 Record = Queue.pop_back_val(); // not actually a queue.
   }
 
-  return AllMatches;
+  return true;
 }
 
 bool CXXBasePaths::lookupInBases(ASTContext ,
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -1519,12 +1519,7 @@
   /// \param BaseMatches Callback invoked for each (direct or indirect) base
   /// class of this type, or if \p AllowShortCircuit is true then until a call
   /// returns false.
-  ///
-  /// \param AllowShortCircuit if false, forces the callback to be called
-  /// for every base class, even if a dependent or non-matching base was
-  /// found.
-  bool forallBases(ForallBasesCallback BaseMatches,
-   bool AllowShortCircuit = true) const;
+  bool forallBases(ForallBasesCallback BaseMatches) const;
 
   /// Function type used by lookupInBases() to determine whether a
   /// specific base class subobject matches the lookup criteria.


Index: clang/lib/AST/CXXInheritance.cpp
===
--- clang/lib/AST/CXXInheritance.cpp
+++ clang/lib/AST/CXXInheritance.cpp
@@ -147,37 +147,27 @@
   return false;
 }
 
-bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches,
-bool AllowShortCircuit) const {
+bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches) const {
   SmallVector Queue;
 
   const CXXRecordDecl *Record = this;
-  bool AllMatches = true;
   while (true) {
 for (const auto  : Record->bases()) {
   const RecordType *Ty = I.getType()->getAs();
-  if (!Ty) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
-  }
+  if (!Ty)
+return false;
 
   CXXRecordDecl *Base =
 cast_or_null(Ty->getDecl()->getDefinition());
   if (!Base ||
   (Base->isDependentContext() &&
!Base->isCurrentInstantiation(Record))) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
+return false;
   }
 
   Queue.push_back(Base);
-  if (!BaseMatches(Base)) {
-if (AllowShortCircuit) return false;
-AllMatches = false;
-continue;
-  }
+  if (!BaseMatches(Base))
+return false;
 }
 
 if (Queue.empty())
@@ -185,7 +175,7 @@
 Record = Queue.pop_back_val(); // not actually a queue.
   }
 
-  return AllMatches;
+  return true;
 }
 
 bool CXXBasePaths::lookupInBases(ASTContext ,
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -1519,12 +1519,7 @@
   /// \param BaseMatches Callback invoked for each (direct or indirect) base
   /// class of this type, or if \p AllowShortCircuit is true then until a call
   /// returns false.
-  ///
-  /// \param AllowShortCircuit if false, forces the callback to be called
-  /// for every base class, even if a dependent or non-matching base was
-  /// found.
-  bool 

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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

Sounds great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75311



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

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

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D74361#1879559 , @rjmccall wrote:

> This will need to impact static analysis and the AST, I think.  Local 
> variables can't be redeclared, but global variables can, so disallowing 
> initializers syntactically when the attribute is present doesn't necessarily 
> stop other declarations from defining the variable.  Also, you need to make 
> the attribute mark something as a definition, the same way that e.g. the 
> alias attribute does.  Also, this probably ought to disable the 
> default-initialization of non-trivial types in C++, in case that's not 
> already done.


I would like this to be the case but am having a tough time understanding how 
sema works. VarDecl::hasInit() looked promising but doesn't appear to indicate 
whether there is a syntactic initialiser (e.g. = 10) present. I will continue 
to investigate. Codegen appears to be working fine.

In D74361#1880904 , @jfb wrote:

> The current uninitialized attribute fits the model C and C++ follow for 
> attributes: they have no semantic meaning for a valid program, in that a 
> compiler can just ignore them and (barring undefined behavior) there's no 
> observable effect to the program. This updated semantic changes the behavior 
> of a valid C and C++ program, because the standards specify the value of 
> uninitialized globals and TLS. I'd much rather have a separate attribute, say 
> `no_zero_init`, to explicitly say what this does.


This proposed attribute can similarly be ignored without observable semantic 
effect. Instead of an IR undef variable, we would have an IR zeroinitialized 
variable, which is a legitimate implementation choice for undef. Adding the 
attribute to an existing program will, in general, change its meaning - but 
that's also true of other attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361



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

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

[Jon Chesterfield via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6509
+static void handleNoZeroInitializerAttr(Sema , Decl *D, const ParsedAttr 
) {
+  D->addAttr(::new (S.Context) NoZeroInitializerAttr(S.Context, AL));
+}

cast(D)->hasInit() seems to always return true here - presumably the 
error needs to be emitted sometime before this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361



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

[PATCH] D74361: [Clang] Uninitialize attribute on global variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield updated this revision to Diff 247127.
JonChesterfield added a comment.

- Rename attribute, add to hasDefiningAttr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/DeclBase.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-no-zero-initializer.cpp

Index: clang/test/Sema/attr-no-zero-initializer.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-no-zero-initializer.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+int good __attribute__((no_zero_initializer));
+const int still_cant_be_const __attribute__((no_zero_initializer)); // expected-error {{default initialization of an object of const type}}
+extern int external __attribute__((no_zero_initializer));
+
+void func() __attribute__((no_zero_initializer)) // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+{
+  int local __attribute__((no_zero_initializer)); // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+
+  static int sl __attribute__((no_zero_initializer));
+}
+
+struct s {
+  __attribute__((no_zero_initializer)) int field; // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
+
+  static __attribute__((no_zero_initializer)) int sfield;
+
+} __attribute__((no_zero_initializer)); // expected-warning {{'no_zero_initializer' attribute only applies to global variables}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -94,6 +94,7 @@
 // CHECK-NEXT: NoStackProtector (SubjectMatchRule_function)
 // CHECK-NEXT: NoThreadSafetyAnalysis (SubjectMatchRule_function)
 // CHECK-NEXT: NoThrow (SubjectMatchRule_hasType_functionType)
+// CHECK-NEXT: NoZeroInitializer (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function)
 // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
Index: clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/attr-no-zero-initializer.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @_ZZ4funcvE4data = internal global i32 undef
+int* func(void)
+{
+  static int data [[clang::no_zero_initializer]];
+  return 
+}
+
+// No code emitted
+extern int extern_unhelpful_but_harmless [[clang::no_zero_initializer]];
+
+// CHECK: @tentative = global i32 undef
+int tentative  [[clang::no_zero_initializer]];
+
+// CHECK: @_ZL16tentative_static = internal global i32 undef
+static int tentative_static [[clang::no_zero_initializer]] __attribute__((used));
+
+// CHECK: @nominally_zero_init = global i32 undef
+int nominally_zero_init  [[clang::no_zero_initializer]] = 0;
+
+// CHECK: @nominally_value_init = global i32 undef
+int nominally_value_init  [[clang::no_zero_initializer]] = 4;
+
+class trivial
+{
+  float x;
+};
+
+// CHECK: @ut = global %class.trivial undef
+trivial ut [[clang::no_zero_initializer]];
+
+struct nontrivial
+{
+  nontrivial() : x(3.14) {}
+  double x;
+};
+
+// CHECK: @unt = global %struct.nontrivial undef
+nontrivial unt [[clang::no_zero_initializer]];
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6505,6 +6505,10 @@
   D->addAttr(::new (S.Context) UninitializedAttr(S.Context, AL));
 }
 
+static void handleNoZeroInitializerAttr(Sema , Decl *D, const ParsedAttr ) {
+  D->addAttr(::new (S.Context) NoZeroInitializerAttr(S.Context, AL));
+}
+
 static bool tryMakeVariablePseudoStrong(Sema , VarDecl *VD,
 bool DiagnoseFailure) {
   QualType Ty = VD->getType();
@@ -7427,6 +7431,10 @@
 handleUninitializedAttr(S, D, AL);
 break;
 
+  case ParsedAttr::AT_NoZeroInitializer:
+handleNoZeroInitializerAttr(S, D, AL);
+break;
+
   case ParsedAttr::AT_ObjCExternallyRetained:
 handleObjCExternallyRetainedAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ 

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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

Looks like other module-related tracking like `LoadedModuleMaps`, 
`DirectoryHasModuleMap` works without extra changes. And `ModuleMapParser` 
relies on a file name to decide if a module map is private or not, so the 
presence of a public module map doesn't matter for it.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1463-1470
   StringRef Filename = llvm::sys::path::filename(File->getName());
   SmallString<128>  PrivateFilename(File->getDir()->getName());
   if (Filename == "module.map")
 llvm::sys::path::append(PrivateFilename, "module_private.map");
   else if (Filename == "module.modulemap")
 llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
   else

We don't try to strip/add file extensions, so won't try to load a module map 
like 'module.private.private.modulemap'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75311



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

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: bruno, Bigcheese.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.
vsapsai marked an inline comment as done.
vsapsai added a comment.

Looks like other module-related tracking like `LoadedModuleMaps`, 
`DirectoryHasModuleMap` works without extra changes. And `ModuleMapParser` 
relies on a file name to decide if a module map is private or not, so the 
presence of a public module map doesn't matter for it.




Comment at: clang/lib/Lex/HeaderSearch.cpp:1463-1470
   StringRef Filename = llvm::sys::path::filename(File->getName());
   SmallString<128>  PrivateFilename(File->getDir()->getName());
   if (Filename == "module.map")
 llvm::sys::path::append(PrivateFilename, "module_private.map");
   else if (Filename == "module.modulemap")
 llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
   else

We don't try to strip/add file extensions, so won't try to load a module map 
like 'module.private.private.modulemap'.


Support only preferred spelling 'Modules/module.private.modulemap' and
not the deprecated 'module_private.map'.

rdar://problem/57715533


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75311

Files:
  clang/lib/Lex/HeaderSearch.cpp
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
  
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
  
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
  clang/test/Modules/implicit-private-without-public.m


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN:   -F%S/Inputs/implicit-private-without-public \
+// RUN:   -fsyntax-only %s -verify
+
+@import Foo_Private;
+
+// Private module map without a public one isn't supported for deprecated 
module map locations.
+@import DeprecatedModuleMapLocation_Private;
+// expected-error@-1{{module 'DeprecatedModuleMapLocation_Private' not found}}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/PrivateHeaders/Foo_Priv.h
@@ -0,0 +1 @@
+void foo_private(void);
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/Foo.framework/Modules/module.private.modulemap
@@ -0,0 +1,4 @@
+framework module Foo_Private {
+  header "Foo_Priv.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/module_private.map
@@ -0,0 +1,4 @@
+framework module DeprecatedModuleMapLocation_Private {
+  header "A.h"
+  export *
+}
Index: 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
===
--- /dev/null
+++ 
clang/test/Modules/Inputs/implicit-private-without-public/DeprecatedModuleMapLocation.framework/PrivateHeaders/A.h
@@ -0,0 +1 @@
+void a(void);
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1568,6 +1568,16 @@
   llvm::sys::path::append(ModuleMapFileName, "module.map");
   if (auto F = FileMgr.getFile(ModuleMapFileName))
 return *F;
+
+  // For frameworks, allow to have a private module map with a preferred
+  // spelling when a public module map is absent.
+  if (IsFramework) {
+ModuleMapFileName = Dir->getName();
+llvm::sys::path::append(ModuleMapFileName, "Modules",
+"module.private.modulemap");
+if (auto F = FileMgr.getFile(ModuleMapFileName))
+  return *F;
+  }
   return nullptr;
 }
 


Index: clang/test/Modules/implicit-private-without-public.m
===
--- /dev/null
+++ clang/test/Modules/implicit-private-without-public.m
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// RUN: rm -rf 

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72872



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

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D75289#1896925 , @njames93 wrote:

> In D75289#1896902 , @Eugene.Zelenko 
> wrote:
>
> > Language and/or its standard is checked in other places too. Should all 
> > similar places be refactored?
>
>
> They should but I feel they should be in follow up patches, the only reason 
> MakeSmartPtrCheck is in here is because by coincidence it used the same name 
> as what I planned and I got a compile warning about it. 
>  I also don't know exactly where all occurrences are.


Just grep for //getLangOpts()//.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75289



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

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[Olivier Giroux via Phabricator via cfe-commits]

__simt__ added a comment.

(//I assume I'm not seeing a code review being used to veto a C++ Standard 
feature, but actually the other points are the reason for the red flag.//)

I can see a desire for hyper-precise definitions to achieve the best possible 
performance, but we really need a healthy does of conservatism here.

The C++ values can't change as a result of selecting between microarchitecture 
variations that are supposed to link, it takes an ABI break to change these.

We specified two numbers here so we could conservatively set them (e.g. 
constructive: smallest; destructive: largest) if we want, but then they are 
fixed.

I think there's just two plausible answers for x86_64:

1. constructive=64, destructive=64 (the only plausible answer for X86 classic 
edition)
2. constructive=64, destructive=128 (reserve some room for line-pair designs)

Recapping: precision is nice but we need to fix this in the ABI so ultimate 
precision isn't required nor desirable; we should choose one of these pairs (or 
debate if another pair is viable that I'm not thinking of); then we should ship 
C++17.




Comment at: clang/lib/Basic/Targets/X86.cpp:1748
+// | Haswell|  64 | 
https://www.7-cpu.com/cpu/Haswell.html  
 |
+// | Bradwell   |  64 | 
https://www.7-cpu.com/cpu/Broadwell.html
 |
+// | Skylake (including skylake-avx512) |  64 | 
https://www.nas.nasa.gov/hecc/support/kb/skylake-processors_550.html "Cache 
Hierarchy"  
 |

Broadwell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D75289#1896902 , @Eugene.Zelenko 
wrote:

> Language and/or its standard is checked in other places too. Should all 
> similar places be refactored?


They should but I feel they should be in follow up patches, the only reason 
MakeSmartPtrCheck is in here is because by coincidence it used the same name as 
what I planned and I got a compile warning about it. 
I also don't know exactly where all occurrences are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75289



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

[PATCH] D75310: [CUDA, clang-cl] Filter out unsupported arguments for device-side compilation.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added a reviewer: rnk.
Herald added subscribers: sanjoy.google, bixia.
Herald added a project: clang.

Device-side compilation does not support some features and we need to
filter them out when command line options enable them for the host.

We're already doing this in various places in the regular clang driver,
but clang-cl mode constructs cc1 options independently and needs to
implement the filtering, too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75310

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.cu


Index: clang/test/Driver/cl-options.cu
===
--- /dev/null
+++ clang/test/Driver/cl-options.cu
@@ -0,0 +1,27 @@
+// Verify that we don't pass unwanted options to device-side compilation when
+// clang-cl is used for CUDA compilation.
+// Note: %s must be preceded by --, otherwise it may be interpreted as a
+// command-line option, e.g. on Mac where %s is commonly under /Users.
+
+// -stack-protector should not be passed to device-side CUDA compilation
+// RUN: %clang_cl -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=GS-default %s
+// GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GS-default-NOT: "-stack-protector"
+// GS-default: "-cc1" "-triple"
+// GS-default: "-stack-protector" "2"
+
+// -exceptions should be passed to device-side compilation.
+// RUN: %clang_cl /c /GX -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=GX %s
+// GX: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GX-NOT: "-fcxx-exceptions"
+// GX-NOT: "-fexceptions"
+// GX: "-cc1" "-triple"
+// GX: "-fcxx-exceptions" "-fexceptions"
+
+// /Gd should not override default calling convention on device side.
+// RUN: %clang_cl /c /Gd -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck 
-check-prefix=Gd %s
+// Gd: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// Gd-NOT: "-fcxx-exceptions"
+// Gd-NOT: "-fdefault-calling-conv=cdecl"
+// Gd: "-cc1" "-triple"
+// Gd: "-fdefault-calling-conv=cdecl"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6395,6 +6395,7 @@
codegenoptions::DebugInfoKind *DebugInfoKind,
bool *EmitCodeView) const {
   unsigned RTOptionID = options::OPT__SLASH_MT;
+  bool isNVPTX = getToolChain().getTriple().isNVPTX();
 
   if (Args.hasArg(options::OPT__SLASH_LDd))
 // The /LDd option implies /MTd. The dependent lib part can be overridden,
@@ -6462,8 +6463,8 @@
 
   // This controls whether or not we emit stack-protector instrumentation.
   // In MSVC, Buffer Security Check (/GS) is on by default.
-  if (Args.hasFlag(options::OPT__SLASH_GS, options::OPT__SLASH_GS_,
-   /*Default=*/true)) {
+  if (!isNVPTX && Args.hasFlag(options::OPT__SLASH_GS, options::OPT__SLASH_GS_,
+   /*Default=*/true)) {
 CmdArgs.push_back("-stack-protector");
 CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong)));
   }
@@ -6483,7 +6484,7 @@
 
   const Driver  = getToolChain().getDriver();
   EHFlags EH = parseClangCLEHFlags(D, Args);
-  if (EH.Synch || EH.Asynch) {
+  if (!isNVPTX && (EH.Synch || EH.Asynch)) {
 if (types::isCXX(InputType))
   CmdArgs.push_back("-fcxx-exceptions");
 CmdArgs.push_back("-fexceptions");
@@ -6552,7 +6553,7 @@
   options::OPT__SLASH_Gregcall)) {
 unsigned DCCOptId = CCArg->getOption().getID();
 const char *DCCFlag = nullptr;
-bool ArchSupported = true;
+bool ArchSupported = !isNVPTX;
 llvm::Triple::ArchType Arch = getToolChain().getArch();
 switch (DCCOptId) {
 case options::OPT__SLASH_Gd:


Index: clang/test/Driver/cl-options.cu
===
--- /dev/null
+++ clang/test/Driver/cl-options.cu
@@ -0,0 +1,27 @@
+// Verify that we don't pass unwanted options to device-side compilation when
+// clang-cl is used for CUDA compilation.
+// Note: %s must be preceded by --, otherwise it may be interpreted as a
+// command-line option, e.g. on Mac where %s is commonly under /Users.
+
+// -stack-protector should not be passed to device-side CUDA compilation
+// RUN: %clang_cl -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck -check-prefix=GS-default %s
+// GS-default: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GS-default-NOT: "-stack-protector"
+// GS-default: "-cc1" "-triple"
+// GS-default: "-stack-protector" "2"
+
+// -exceptions should be passed to device-side compilation.
+// RUN: %clang_cl /c /GX -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck -check-prefix=GX %s
+// GX: "-cc1" "-triple" "nvptx64-nvidia-cuda"
+// GX-NOT: "-fcxx-exceptions"
+// GX-NOT: "-fexceptions"
+// GX: "-cc1" "-triple"
+// GX: "-fcxx-exceptions" "-fexceptions"
+
+// /Gd should not override 

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Language and/or its standard is checked in other places too. Should all similar 
places be refactored?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75289



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

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 247123.
tstellar marked 4 inline comments as done.
tstellar added a comment.

Address most recent review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875

Files:
  clang/docs/AttributeReference.rst
  clang/docs/CMakeLists.txt
  llvm/cmake/modules/AddSphinxTarget.cmake


Index: llvm/cmake/modules/AddSphinxTarget.cmake
===
--- llvm/cmake/modules/AddSphinxTarget.cmake
+++ llvm/cmake/modules/AddSphinxTarget.cmake
@@ -18,6 +18,7 @@
 #
 # ``project`` should be the project name
 function (add_sphinx_target builder project)
+  cmake_parse_arguments(ARG "" "SOURCE_DIR" "" ${ARGN})
   set(SPHINX_BUILD_DIR "${CMAKE_CURRENT_BINARY_DIR}/${builder}")
   set(SPHINX_DOC_TREE_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/_doctrees-${project}-${builder}")
   set(SPHINX_TARGET_NAME docs-${project}-${builder})
@@ -28,13 +29,17 @@
 set(SPHINX_WARNINGS_AS_ERRORS_FLAG "")
   endif()
 
+  if (NOT ARG_SOURCE_DIR)
+set(ARG_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+
   add_custom_target(${SPHINX_TARGET_NAME}
 COMMAND ${SPHINX_EXECUTABLE}
 -b ${builder}
 -d "${SPHINX_DOC_TREE_DIR}"
 -q # Quiet: no output other than errors and 
warnings.
 ${SPHINX_WARNINGS_AS_ERRORS_FLAG} # Treat warnings 
as errors if requested
-"${CMAKE_CURRENT_SOURCE_DIR}" # Source
+"${ARG_SOURCE_DIR}" # Source
 "${SPHINX_BUILD_DIR}" # Output
 COMMENT
 "Generating ${builder} Sphinx documentation for ${project} 
into \"${SPHINX_BUILD_DIR}\"")
Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,15 +90,43 @@
 endif()
 endif()
 
+function (gen_rst_file_from_td output_file td_option source docs_target)
+  if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
+message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+  get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
+  list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
+  clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
+  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
   if (SPHINX_FOUND)
 if (${SPHINX_OUTPUT_HTML})
-  add_sphinx_target(html clang)
+  add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+
+  # Copy rst files to build directory before generating the html
+  # documentation.  Some of the rst files are generated, so they
+  # only exist in the build directory.  Sphinx needs all files in
+  # the same directory in order to genrate the html, so we need to
+  # copy all the non-gnerated rst files from the source to the build
+  # directory before we run sphinx.
+  add_custom_target(copy-clang-rst-docs
+COMMAND "${CMAKE_COMMAND}" -E copy_directory
+"${CMAKE_CURRENT_SOURCE_DIR}"
+"${CMAKE_CURRENT_BINARY_DIR}")
+  add_dependencies(docs-clang-html copy-clang-rst-docs)
+
   add_custom_command(TARGET docs-clang-html POST_BUILD
-COMMAND ${CMAKE_COMMAND} -E copy
+COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
+
+  # Generated files
+  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
+  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
+  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man clang)
Index: clang/docs/AttributeReference.rst
===
--- clang/docs/AttributeReference.rst
+++ /dev/null
@@ -1,13 +0,0 @@
-..
-  ---
-  NOTE: This file is automatically generated by running clang-tblgen
-  -gen-attr-docs. Do not edit this file by hand!! The contents for
-  this file are automatically generated by a server-side process.
-  
-  Please do not commit this file. The file exists for local testing
-  purposes only.
-  ---
-
-===
-Attributes in Clang
-===
\ No newline at end of file


Index: 

Re: [clang] 83f4372 - [CodeGen] fix clang test that runs the optimizer pipeline; NFC

[Eric Christopher via cfe-commits]

Sure. That sounds great. Thanks!

On Wed, Feb 26, 2020 at 10:45 AM Sanjay Patel 
wrote:

> To be clear - the test is checking IR instructions, but it's checking -O1
> IR for various targets.
> So there must be different expectations per target...
> But I just tried a test of turning everything down to -O0, and it all
> passed except for the "fast-math" run for AArch64.
> I can tweak that to not be so specific if that sounds like a reasonable
> solution.
>
> On Wed, Feb 26, 2020 at 1:05 PM Eric Christopher 
> wrote:
>
>> I mean anything that's testing assembly output out of clang is less than
>> ideal. There are some circumstances, but this doesn't seem like one of
>> them.
>>
>> On Wed, Feb 26, 2020, 9:10 AM Sanjay Patel 
>> wrote:
>>
>>> The test file dates back to:
>>> https://reviews.llvm.org/D5698
>>> ...and I'm not familiar with _Complex enough to say how to fix this
>>> properly (seems like the check lines are already limited such that -O0
>>> rather than -O1 would work?).
>>>
>>> But this file keeps wiggling unexpectedly, it's going to move again with
>>> https://reviews.llvm.org/D75130
>>>
>>> On Tue, Feb 25, 2020 at 1:15 PM Eric Christopher 
>>> wrote:
>>>
 Is there any way to pull this test out of clang and as an opt test?
 What's it trying to test?

 -eric

 On Tue, Feb 25, 2020 at 6:15 AM Sanjay Patel via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

>
> Author: Sanjay Patel
> Date: 2020-02-25T09:13:49-05:00
> New Revision: 83f4372f3a708ceaa800feff8b1bd92ae2c3be5f
>
> URL:
> https://github.com/llvm/llvm-project/commit/83f4372f3a708ceaa800feff8b1bd92ae2c3be5f
> DIFF:
> https://github.com/llvm/llvm-project/commit/83f4372f3a708ceaa800feff8b1bd92ae2c3be5f.diff
>
> LOG: [CodeGen] fix clang test that runs the optimizer pipeline; NFC
>
> There's already a FIXME note on this file; it can break when the
> underlying LLVM behavior changes independently of anything in clang.
>
> Added:
>
>
> Modified:
> clang/test/CodeGen/complex-math.c
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/test/CodeGen/complex-math.c
> b/clang/test/CodeGen/complex-math.c
> index e42418ad72c2..54dee473a364 100644
> --- a/clang/test/CodeGen/complex-math.c
> +++ b/clang/test/CodeGen/complex-math.c
> @@ -93,14 +93,15 @@ float _Complex mul_float_rc(float a, float
> _Complex b) {
>// X86: ret
>return a * b;
>  }
> +
>  float _Complex mul_float_cc(float _Complex a, float _Complex b) {
>// X86-LABEL: @mul_float_cc(
>// X86: %[[AC:[^ ]+]] = fmul
>// X86: %[[BD:[^ ]+]] = fmul
>// X86: %[[AD:[^ ]+]] = fmul
>// X86: %[[BC:[^ ]+]] = fmul
> -  // X86: %[[RR:[^ ]+]] = fsub float %[[AC]], %[[BD]]
> -  // X86: %[[RI:[^ ]+]] = fadd float
> +  // X86: %[[RR:[^ ]+]] = fsub
> +  // X86: %[[RI:[^ ]+]] = fadd
>// X86-DAG: %[[AD]]
>// X86-DAG: ,
>// X86-DAG: %[[BC]]
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>

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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,22 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->p)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->s6[0].pp)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
+{}
+  }
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +111,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected addressable lvalue in 'to' clause}} le45-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
 #pragma omp target update to(S2::S2s)
 #pragma omp target update to(S2::S2sc)
 #pragma omp target update to(to)
 #pragma omp target update to(y x) // 

Re: [clang] d930ed1 - Disallow use of __has_c_attribute in C++ mode.

[James Y Knight via cfe-commits]

That all makes sense -- especially the bits about the dates needing to be
different.

But, even with all that, I'm not sure why we shouldn't implement both
__has_cpp_attribute AND __has_c_attribute in C++ mode?

The subset of C attributes which retain their C-defined semantics in C++
(which, hopefully, is all of them, but doesn't need to be) should return
the appropriate C-standard date with which the (C++ mode) implementation is
compatible. That is, in -std=c++11, we may have
__has_cpp_attribute(deprecated) == 201309,
while __has_c_attribute(deprecated) == 2020XXYY.

I'd hope/expect that whichever version of the C++ standard updates
its baseline to C2x will adopt this feature "automatically", and if it
doesn't, that it'll be included explicitly. But, ISTM we probably don't
need to wait for that to occur in order to implement that behavior in clang?



On Mon, Nov 25, 2019, 7:07 PM Aaron Ballman  wrote:

> On Mon, Nov 25, 2019 at 5:59 PM James Y Knight 
> wrote:
> >
> > Isn't this unnecessarily annoying to users? You have the same syntax to
> use the attributes, and the attributes are expected to be compatible when
> named the same way, but you can't use the same #if conditional to check for
> availability, when writing a header intended to work in both modes?
>
> Yes, I do think it's needlessly annoying to users.
>
> WG21 adopted __has_cpp_attribute for C++2a based on the existing Clang
> implementation. WG14 is looking to add a similar feature testing
> macro, but cannot use __has_cpp_attribute due to the name and so I
> proposed __has_c_attribute for parity with __has_cpp_attribute and
> implemented it in Clang. After a lot of discussions with users,
> committee members, and other implementers, I realized this is silly:
> we could have a single macro that handles both C and C++ depending on
> the language mode being compiled for. Because of that, as a liaison I
> filed an NB comment with WG21 to have them reconsider the name
> __has_cpp_attribute before it shipped in a standard so that we could
> use a language-neutral name for both languages. In Belfast, EWG
> reaffirmed that they want the name __has_cpp_attribute. They found my
> arguments to be unpersuasive and didn't think it would be possible for
> the semantics of attributes to align between the languages, or to
> commit to such a guarantee (despite WG14 having a chartered mandate to
> be compatible with features adopted from C++).
>
> Rather than leaving C out in the cold because EWG came to the decision
> they did, we're kind of stuck with __has_c_attribute.
>
> One problem this does solve is that __has_cpp_attribute and
> __has_c_attribute both return a numeric value that is date-like for
> standard attributes, but those standard attributes are adopted into
> different standards at different times. Because returned values are
> different, it makes use harder when dealing with attribute
> modifications over time. This isn't impossible to solve, we could
> introduce macros for the return values and base their values on which
> language mode is being preprocessed, but we no longer have to.
>
> ~Aaron
>
> >
> > On Mon, Nov 25, 2019 at 5:35 PM Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >>
> >> Author: Aaron Ballman
> >> Date: 2019-11-25T17:35:12-05:00
> >> New Revision: d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/d930ed1acc0ea49d4b3aae7e95b4c6d9cd310578.diff
> >>
> >> LOG: Disallow use of __has_c_attribute in C++ mode.
> >>
> >> __has_cpp_attribute is not available in C mode, and __has_c_attribute
> >> should not be available in C++ mode. This also adds a test to
> >> demonstrate that we properly handle scoped attribute tokens even in C
> >> mode.
> >>
> >> Added:
> >> clang/test/Preprocessor/has_c_attribute.cpp
> >>
> >> Modified:
> >> clang/lib/Lex/PPMacroExpansion.cpp
> >> clang/test/Preprocessor/has_c_attribute.c
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> 
> >> diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp
> b/clang/lib/Lex/PPMacroExpansion.cpp
> >> index e6e00b1c1700..a69c4dbb3a2a 100644
> >> --- a/clang/lib/Lex/PPMacroExpansion.cpp
> >> +++ b/clang/lib/Lex/PPMacroExpansion.cpp
> >> @@ -370,7 +370,11 @@ void Preprocessor::RegisterBuiltinMacros() {
> >>Ident__has_extension= RegisterBuiltinMacro(*this,
> "__has_extension");
> >>Ident__has_builtin  = RegisterBuiltinMacro(*this,
> "__has_builtin");
> >>Ident__has_attribute= RegisterBuiltinMacro(*this,
> "__has_attribute");
> >> -  Ident__has_c_attribute  = RegisterBuiltinMacro(*this,
> "__has_c_attribute");
> >> +  if (!LangOpts.CPlusPlus)
> >> +Ident__has_c_attribute = RegisterBuiltinMacro(*this,
> "__has_c_attribute");
> >> +  else
> >> +Ident__has_c_attribute = nullptr;
> >> +
> >>

[PATCH] D69591: Devirtualize a call on alloca without waiting for post inline cleanup and next DevirtSCCRepeatedPass iteration.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

yamauchi added a comment.

Here's a fix: https://reviews.llvm.org/D75307


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69591



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

[clang] 4c2a656 - Avoid ASTContext.h -> TargetInfo.h dep

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-02-27T14:35:00-08:00
New Revision: 4c2a6567bb12559cfc091bca2b25ae907cbd4e0f

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

LOG: Avoid ASTContext.h -> TargetInfo.h dep

This has been done before in 2008: ab13857072
But these things regress easily.
Move some things out of line.

Saves 316 includes + transitive stuff:
316 -../clang/include/clang/Basic/TargetOptions.h
316 -../clang/include/clang/Basic/TargetInfo.h
316 -../clang/include/clang/Basic/TargetCXXABI.h
316 -../clang/include/clang/Basic/OpenCLOptions.h
316 -../clang/include/clang/Basic/OpenCLExtensions.def
302 -../llvm/include/llvm/Target/TargetOptions.h
302 -../llvm/include/llvm/Support/CodeGen.h
302 -../llvm/include/llvm/MC/MCTargetOptions.h
302 -../llvm/include/llvm/ADT/FloatingPointMode.h
302 -../clang/include/clang/Basic/XRayInstr.h
302 -../clang/include/clang/Basic/DebugInfoOptions.h
302 -../clang/include/clang/Basic/CodeGenOptions.h
302 -../clang/include/clang/Basic/CodeGenOptions.def
257 -../llvm/include/llvm/Support/Regex.h
 79 -../llvm/include/llvm/ADT/SmallSet.h
 68 -MSVCSTL/include/set
 66 -../llvm/include/llvm/ADT/SmallPtrSet.h
 62 -../llvm/include/llvm/ADT/StringSwitch.h

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/Context.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/PatternInit.cpp
clang/unittests/CodeGen/TBAAMetadataTest.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 2640c66cc8dd..92e5921fa375 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -39,7 +39,6 @@
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
-#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/XRayLists.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -74,6 +73,7 @@
 namespace llvm {
 
 struct fltSemantics;
+template  class SmallPtrSet;
 
 } // namespace llvm
 
@@ -131,7 +131,6 @@ class VarTemplateDecl;
 class VTableContextBase;
 struct BlockVarCopyInit;
 
-
 namespace Builtin {
 
 class Context;
@@ -139,6 +138,7 @@ class Context;
 } // namespace Builtin
 
 enum BuiltinTemplateKind : int;
+enum OpenCLTypeKind : uint8_t;
 
 namespace comments {
 
@@ -1205,7 +1205,7 @@ class ASTContext : public RefCountedBase {
   QualType getBlockDescriptorExtendedType() const;
 
   /// Map an AST Type to an OpenCLTypeKind enum value.
-  TargetInfo::OpenCLTypeKind getOpenCLTypeKind(const Type *T) const;
+  OpenCLTypeKind getOpenCLTypeKind(const Type *T) const;
 
   /// Get address space for OpenCL type.
   LangAS getOpenCLTypeAddrSpace(const Type *T) const;
@@ -1635,23 +1635,9 @@ class ASTContext : public RefCountedBase {
 return NSCopyingName;
   }
 
-  CanQualType getNSUIntegerType() const {
-assert(Target && "Expected target to be initialized");
-const llvm::Triple  = Target->getTriple();
-// Windows is LLP64 rather than LP64
-if (T.isOSWindows() && T.isArch64Bit())
-  return UnsignedLongLongTy;
-return UnsignedLongTy;
-  }
+  CanQualType getNSUIntegerType() const;
 
-  CanQualType getNSIntegerType() const {
-assert(Target && "Expected target to be initialized");
-const llvm::Triple  = Target->getTriple();
-// Windows is LLP64 rather than LP64
-if (T.isOSWindows() && T.isArch64Bit())
-  return LongLongTy;
-return LongTy;
-  }
+  CanQualType getNSIntegerType() const;
 
   /// Retrieve the identifier 'bool'.
   IdentifierInfo *getBoolName() const {
@@ -2129,9 +2115,7 @@ class ASTContext : public RefCountedBase {
   /// Return the alignment (in bytes) of the thrown exception object. This is
   /// only meaningful for targets that allocate C++ exceptions in a system
   /// runtime, such as those using the Itanium C++ ABI.
-  CharUnits getExnObjectAlignment() const {
-return toCharUnitsFromBits(Target->getExnObjectAlignment());
-  }
+  CharUnits getExnObjectAlignment() const;
 
   /// Get or compute information about the layout of the specified
   /// record (struct/union/class) 

[clang] 0f6959f - Add some missing header dependencies

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-02-27T14:32:12-08:00
New Revision: 0f6959f3632a1131fe74f7dde33cc52778b02280

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

LOG: Add some missing header dependencies

Unit tests are not part of `all` O_O, and I tested on Windows with
-fdelayed-template-parsing.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/Yaml.h
clang/unittests/AST/ASTVectorTest.cpp
clang/unittests/CodeGen/TBAAMetadataTest.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/Yaml.h 
b/clang/lib/StaticAnalyzer/Checkers/Yaml.h
index 968c50e33f6d..ec612dde3b8b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Yaml.h
+++ b/clang/lib/StaticAnalyzer/Checkers/Yaml.h
@@ -15,6 +15,7 @@
 #define LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H
 
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/YAMLTraits.h"
 
 namespace clang {

diff  --git a/clang/unittests/AST/ASTVectorTest.cpp 
b/clang/unittests/AST/ASTVectorTest.cpp
index f5b208ab1687..7953acb1b4db 100644
--- a/clang/unittests/AST/ASTVectorTest.cpp
+++ b/clang/unittests/AST/ASTVectorTest.cpp
@@ -10,9 +10,11 @@
 //
 
//===--===//
 
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTVector.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceManager.h"
 #include "gtest/gtest.h"
 
 using namespace clang;

diff  --git a/clang/unittests/CodeGen/TBAAMetadataTest.cpp 
b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
index 6535fe27b3c7..9726838508c1 100644
--- a/clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -9,12 +9,13 @@
 #include "IRMatchers.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Parse/ParseAST.h"
 #include "llvm/ADT/Triple.h"
-#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"



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

Re: Buildbot cleaning for zorg upgrade

[Galina Kistanova via cfe-commits]

Hello Michael,

Thanks for adding those polly bots!

Please feel free to stage the new bots. Everything is ready on the staging
build bot.
You can point them to lab.llvm.org:9994 and make sure they are reliably
green.

Thanks

Galina

On Tue, Feb 25, 2020 at 9:44 PM Michael Kruse 
wrote:

> Looking forward to the upgrade. Thanks for investing the time.
>
> I am trying to re-add buildbots for polly-x86_64
> (https://reviews.llvm.org/D75127) and I am not use whether you
> received my messages about it. In any case, would you prefer to wait
> until the new instance of buildbot-master is available?
>
> Michael
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Selection.cpp:261
+  // consider it selected.
+  if (!SeenMacroCalls.insert(ArgStart).second) {
+return NoTokens;

sammccall wrote:
> sammccall wrote:
> > Given the following program:
> > ```
> > #define SQUARE(x) x * x;
> > int four = [[SQUARE(2)]];
> > ```
> > We're going to now report the binary operator and one of the operands as 
> > selected and not the other, which doesn't seem desirable.
> > 
> > I think we want to accept macro-selected || arg-selected, so probably doing 
> > the current "non-argument macro expansion" first unconditionally or 
> > factoring it out into a function.
> > 
> > This will change the behavior of `int four = [[SQUARE]](2)` to consider the 
> > literal children selected too, I think this is fine.
> I don't think it's a good idea to add hidden state and side-effects to 
> testChunk() - it breaks a lot of assumptions that help reason about the code, 
> and using `mutable` hides the violation of them.
> (And a possible source of bugs - this is first in traversal order rather than 
> first in source order - these are mostly but IIRC not always the same).
> 
> Instead I think you can do this statelessly: from the top-level spelling 
> location, walk down with `SM.getMacroArgExpandedLocation` until you hit the 
> target FileID (this is the first-expansion of first-expansion of 
> first-expansion...) or the FileID stops changing (you've reached the 
> innermost macro invocation, and your target location was on a different 
> branch).
I agree that adding state is not great. I thought it was icky as I was writing 
it, I just couldn't think of an alternative. Thank you for suggesting one!

I implemented what you suggested, and it seems to work. I did want to ask a 
clarifying question to make sure I understand correctly: when an argument 
occurs multiple times in a macro exapnsion, the occurrences will have distinct 
`FileID`s (as opposed just different offsets in the same macro `FileID`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72041



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

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 247106.
nridge marked an inline comment as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72041

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -329,7 +329,18 @@
#define ADDRESSOF(X) 
int *j = ADDRESSOF(^i);
   )cpp",
-
+  R"cpp(// Macro argument appearing multiple times in expansion
+#define VALIDATE_TYPE(x) (void)x;
+#define ASSERT(expr)   \
+  do { \
+VALIDATE_TYPE(expr);   \
+if (!expr);\
+  } while (false)
+bool [[waldo]]() { return true; }
+void foo() {
+  ASSERT(wa^ldo());
+}
+  )cpp",
   R"cpp(// Symbol concatenated inside macro (not supported)
int *pi;
#define POINTER(X) p ## X;
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -415,7 +415,7 @@
 
 // Regression test: this used to match the injected X, not the outer X.
 TEST(SelectionTest, InjectedClassName) {
-  const char* Code = "struct ^X { int x; };";
+  const char *Code = "struct ^X { int x; };";
   auto AST = TestTU::withCode(Annotations(Code).code()).build();
   auto T = makeSelectionTree(Code, AST);
   ASSERT_EQ("CXXRecordDecl", nodeKind(T.commonAncestor())) << T;
@@ -508,7 +508,8 @@
 }
 
 TEST(SelectionTest, MacroArgExpansion) {
-  // If a macro arg is expanded several times, we consider them all selected.
+  // If a macro arg is expanded several times, we only consider the first one
+  // selected.
   const char *Case = R"cpp(
 int mul(int, int);
 #define SQUARE(X) mul(X, X);
@@ -517,15 +518,8 @@
   Annotations Test(Case);
   auto AST = TestTU::withCode(Test.code()).build();
   auto T = makeSelectionTree(Case, AST);
-  // Unfortunately, this makes the common ancestor the CallExpr...
-  // FIXME: hack around this by picking one?
-  EXPECT_EQ("CallExpr", T.commonAncestor()->kind());
-  EXPECT_FALSE(T.commonAncestor()->Selected);
-  EXPECT_EQ(2u, T.commonAncestor()->Children.size());
-  for (const auto* N : T.commonAncestor()->Children) {
-EXPECT_EQ("IntegerLiteral", N->kind());
-EXPECT_TRUE(N->Selected);
-  }
+  EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
+  EXPECT_TRUE(T.commonAncestor()->Selected);
 
   // Verify that the common assert() macro doesn't suffer from this.
   // (This is because we don't associate the stringified token with the arg).
@@ -542,7 +536,7 @@
 }
 
 TEST(SelectionTest, Implicit) {
-  const char* Test = R"cpp(
+  const char *Test = R"cpp(
 struct S { S(const char*); };
 int f(S);
 int x = f("^");
Index: clang-tools-extra/clangd/Selection.h
===
--- clang-tools-extra/clangd/Selection.h
+++ clang-tools-extra/clangd/Selection.h
@@ -64,6 +64,9 @@
 //
 // SelectionTree tries to behave sensibly in the presence of macros, but does
 // not model any preprocessor concepts: the output is a subset of the AST.
+// When a macro argument is specifically selected, only its first expansion is
+// selected in the AST. (Returning a selection forest is unreasonably difficult
+// for callers to handle correctly.)
 //
 // Comments, directives and whitespace are completely ignored.
 // Semicolons are also ignored, as the AST generally does not model them well.
@@ -127,15 +130,15 @@
 Selection Selected;
 // Walk up the AST to get the DeclContext of this Node,
 // which is not the node itself.
-const DeclContext& getDeclContext() const;
+const DeclContext () const;
 // Printable node kind, like "CXXRecordDecl" or "AutoTypeLoc".
 std::string kind() const;
 // If this node is a wrapper with no syntax (e.g. implicit cast), return
 // its contents. (If multiple wrappers are present, unwraps all of them).
-const Node& ignoreImplicit() const;
+const Node () const;
 // If this node is inside a wrapper with no syntax (e.g. implicit cast),
 // return that wrapper. (If multiple are present, unwraps all of them).
-const Node& outerImplicit() const;
+const Node () const;
   };
   // The most specific common ancestor of all the selected nodes.
   // Returns nullptr if the common ancestor is the root.
Index: clang-tools-extra/clangd/Selection.cpp

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D73245#1894420 , @EricWF wrote:

> @ldionne Since this has the possibility of breaking existing users of 
> `std::max_align_t`, can you test this against your c++03 codebases?


I have no opinion about this patch, but I generally agree we shouldn't make 
efforts to provide C++03 conformance. It's a vain effort and we should be 
looking towards the future. On the other hand, this patch does seem to simplify 
the code a bit, in the sense that we now don't provide anything unless we have 
a conforming C++11 implementation, in which case we don't need to go through 
platform specific hoops anymore. I like that.

Regardless, I'm running this change on a code base to see whether it causes 
problems.


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

https://reviews.llvm.org/D73245



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

[PATCH] D75162: [X86][F16C] Remove cvtph2ps intrinsics and use generic half2float conversion (PR37554)

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D75162#1896469 , @RKSimon wrote:

> In D75162#1896365 , @craig.topper 
> wrote:
>
> > Should we check that this generates constrained.fpext in strict mode?
>
>
> Sure - where is the best place(s) to test this?


For the intrinsics we've tested this way so far, they're in 
sse-builtins-constrained.c and similar. I think we have separate files for 
compares that I should merge into the others. They got commited separately as I 
was working on compares when someone else was working on the others so I made a 
separate file to avoid the merge conflict.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75162



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

[PATCH] D75162: [X86][F16C] Remove cvtph2ps intrinsics and use generic half2float conversion (PR37554)

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I've posted https://reviews.llvm.org/D75304 to make the backend recognize the 
strict version of this pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75162



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

Re: [PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[Eric Christopher via cfe-commits]

I'm agreed with James on all points fwiw.

On Thu, Feb 27, 2020, 2:00 PM James Y Knight via Phabricator <
revi...@reviews.llvm.org> wrote:

> jyknight requested changes to this revision.
> jyknight added a comment.
> This revision now requires changes to proceed.
>
> In D74918#1885869 , @zoecarver
> wrote:
>
> > @jyknight It would probably be best if we could account for CPUs who
> like to use aligned pairs when getting a cache line. It's probably also
> important that we don't change the value `getCPUCacheLineSize` returns, so,
> if we are going to account for that, we should probably update
> `getCPUCacheLineSize ` before this patch lands.
>
>
> It would be extremely unfortunate to go to all the trouble of attempting
> to return accurate results from the P0154 interfaces, and then to return an
> answer insufficient to actually achieve the performance benefit it's
> intended for, and then be unable to fix it due to ABI concerns. So, yes, I
> do believe that this issue must be decided.
>
> Additionally, my opinion here has really not changed from a couple of
> years ago. I continue to believe it was a mistake to standardize these
> constexpr values, and that absolutely the best course of action would be to
> continue to decline to implement this part of the standard, forever. (And
> that GCC should similarly also continue to decline to implement it).
>
> That said, the list of cacheline sizes collected in this review is still
> useful in any case, since it needs to be copied into LLVM's X86Subtarget to
> implement the backend getCacheLineSize function.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D74918/new/
>
> https://reviews.llvm.org/D74918
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75215: [DebugInfo] Fix for adding "returns cxx udt" option to functions in CodeView.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 247104.
akhuang added a comment.

Just change CodeViewDebug to add CxxReturnUdt to all methods that return a 
record type,
which appears to be consistent with what msvc does and avoids emitting two 
versions of
the method in the pdb file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215

Files:
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/test/DebugInfo/COFF/function-options.ll

Index: llvm/test/DebugInfo/COFF/function-options.ll
===
--- llvm/test/DebugInfo/COFF/function-options.ll
+++ llvm/test/DebugInfo/COFF/function-options.ll
@@ -33,7 +33,15 @@
 ; DEFINE_FUNCTION(DClass); // Expect: FO = CxxReturnUdt
 ;
 ; class FClass { static int x; };
-; DEFINE_FUNCTION(FClass); // Expect FO = None
+; DEFINE_FUNCTION(FClass); // Expect: FO = None
+;
+; class GClass;
+; class HClass {
+;   AClass Member_AClass(AClass &); // Expect: FO = CxxReturnUdt
+;   BClass Member_BClass(BClass &); // Expect: FO = CxxReturnUdt
+;   GClass Member_GClass(GClass &); // Expect: FO = CxxReturnUdt
+; }
+; DEFINE_FUNCTION(HClass);
 ; 
 ; struct AStruct {};
 ; DEFINE_FUNCTION(AStruct); // Expect FO = None
@@ -229,6 +237,66 @@
 ; CHECK: FunctionType: FClass (FClass&) ([[SP6]])
 ; CHECK: Name: Func_FClass
 ; CHECK:   }
+; CHECK:   MemberFunction ([[MF_A:.*]]) {
+; CHECK: TypeLeafKind: LF_MFUNCTION (0x1009)
+; CHECK: ReturnType: AClass ({{.*}})
+; CHECK: ClassType: HClass ({{.*}})
+; CHECK: ThisType: HClass* const ({{.*}})
+; CHECK: CallingConvention: NearC (0x0)
+; CHECK: FunctionOptions [ (0x1)
+; CHECK:   CxxReturnUdt (0x1)
+; CHECK: ]
+; CHECK: NumParameters: 1
+; CHECK: ArgListType: (AClass&) ({{.*}})
+; CHECK: ThisAdjustment: 0
+; CHECK:   }
+; CHECK:   MemberFunction ([[MF_B:.*]]) {
+; CHECK: TypeLeafKind: LF_MFUNCTION (0x1009)
+; CHECK: ReturnType: BClass ({{.*}})
+; CHECK: ClassType: HClass ({{.*}})
+; CHECK: ThisType: HClass* const ({{.*}})
+; CHECK: CallingConvention: NearC (0x0)
+; CHECK: FunctionOptions [ (0x1)
+; CHECK:   CxxReturnUdt (0x1)
+; CHECK: ]
+; CHECK: NumParameters: 1
+; CHECK: ArgListType: (BClass&) ({{.*}})
+; CHECK: ThisAdjustment: 0
+; CHECK:   }
+; CHECK:   MemberFunction ([[MF_G:.*]]) {
+; CHECK: TypeLeafKind: LF_MFUNCTION (0x1009)
+; CHECK: ReturnType: GClass ({{.*}})
+; CHECK: ClassType: HClass ({{.*}})
+; CHECK: ThisType: HClass* const ({{.*}})
+; CHECK: CallingConvention: NearC (0x0)
+; CHECK: FunctionOptions [ (0x1)
+; CHECK:   CxxReturnUdt (0x1)
+; CHECK: ]
+; CHECK: NumParameters: 1
+; CHECK: ArgListType: (GClass&) ({{.*}})
+; CHECK: ThisAdjustment: 0
+; CHECK:   }
+; CHECK:   FieldList (0x1043) {
+; CHECK: TypeLeafKind: LF_FIELDLIST (0x1203)
+; CHECK: OneMethod {
+; CHECK:   TypeLeafKind: LF_ONEMETHOD (0x1511)
+; CHECK:   AccessSpecifier: Private (0x1)
+; CHECK:   Type: AClass HClass::(AClass&) ([[MF_A]])
+; CHECK:   Name: Member_AClass
+; CHECK: }
+; CHECK: OneMethod {
+; CHECK:   TypeLeafKind: LF_ONEMETHOD (0x1511)
+; CHECK:   AccessSpecifier: Private (0x1)
+; CHECK:   Type: BClass HClass::(BClass&) ([[MF_B]])
+; CHECK:   Name: Member_BClass
+; CHECK: }
+; CHECK: OneMethod {
+; CHECK:   TypeLeafKind: LF_ONEMETHOD (0x1511)
+; CHECK:   AccessSpecifier: Private (0x1)
+; CHECK:   Type: GClass HClass::(GClass&) ([[MF_G]])
+; CHECK:   Name: Member_GClass
+; CHECK: }
+; CHECK:   }
 ; CHECK:   Procedure ([[SP7:.*]]) {
 ; CHECK: TypeLeafKind: LF_PROCEDURE (0x1008)
 ; CHECK: ReturnType: AStruct ({{.*}})
@@ -335,11 +403,10 @@
 ; CHECK:   }
 ; CHECK: ]
 
-
 ; ModuleID = 'function-options.cpp'
 source_filename = "function-options.cpp"
-target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-windows-msvc19.15.26729"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.23.28106"
 
 %class.AClass = type { i8 }
 %class.BClass = type { i8 }
@@ -347,6 +414,7 @@
 %class.C2Class = type { i8 }
 %class.DClass = type { i8 }
 %class.FClass = type { i8 }
+%class.HClass = type { i8 }
 %struct.AStruct = type { i8 }
 %struct.BStruct = type { i8 }
 %union.AUnion = type { i8 }
@@ -358,228 +426,278 @@
   %retval = alloca %class.AClass, align 1
   %arg.addr = alloca %class.AClass*, align 8
   store %class.AClass* %arg, %class.AClass** %arg.addr, align 8
-  call void @llvm.dbg.declare(metadata %class.AClass** %arg.addr, metadata !14, metadata !DIExpression()), !dbg !15
-  %0 = load %class.AClass*, %class.AClass** %arg.addr, align 8, !dbg !15
-  %coerce.dive = getelementptr inbounds %class.AClass, %class.AClass* %retval, i32 0, i32 0, !dbg !15
-  %1 = load i8, i8* %coerce.dive, align 1, !dbg !15
-  ret 

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[James Y Knight via Phabricator via cfe-commits]

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

In D74918#1885869 , @zoecarver wrote:

> @jyknight It would probably be best if we could account for CPUs who like to 
> use aligned pairs when getting a cache line. It's probably also important 
> that we don't change the value `getCPUCacheLineSize` returns, so, if we are 
> going to account for that, we should probably update `getCPUCacheLineSize ` 
> before this patch lands.


It would be extremely unfortunate to go to all the trouble of attempting to 
return accurate results from the P0154 interfaces, and then to return an answer 
insufficient to actually achieve the performance benefit it's intended for, and 
then be unable to fix it due to ABI concerns. So, yes, I do believe that this 
issue must be decided.

Additionally, my opinion here has really not changed from a couple of years 
ago. I continue to believe it was a mistake to standardize these constexpr 
values, and that absolutely the best course of action would be to continue to 
decline to implement this part of the standard, forever. (And that GCC should 
similarly also continue to decline to implement it).

That said, the list of cacheline sizes collected in this review is still useful 
in any case, since it needs to be copied into LLVM's X86Subtarget to implement 
the backend getCacheLineSize function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

[PATCH] D75279: Avoid SourceManager.h include in RawCommentList.h, add missing incs

[Reid Kleckner via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG86565c130942: Avoid SourceManager.h include in 
RawCommentList.h, add missing incs (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75279

Files:
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTDumper.h
  clang/include/clang/AST/RawCommentList.h
  clang/lib/ARCMigrate/TransProtectedScope.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DataCollection.cpp
  clang/lib/AST/ExternalASTSource.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RawCommentList.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CloneDetection.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/lib/Index/FileIndexRecord.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/lib/Tooling/Core/Lookup.cpp
  clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
  clang/lib/Tooling/Transformer/SourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -21,6 +21,7 @@
 #include "lldb/Utility/Log.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/Basic/SourceManager.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
Index: clang/lib/Tooling/Transformer/SourceCode.cpp
===
--- clang/lib/Tooling/Transformer/SourceCode.cpp
+++ clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/Errc.h"
 
Index: clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
Index: clang/lib/Tooling/Core/Lookup.cpp
===
--- clang/lib/Tooling/Core/Lookup.cpp
+++ clang/lib/Tooling/Core/Lookup.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/SmallVector.h"
 using namespace clang;
 using namespace clang::tooling;
Index: clang/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- clang/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ clang/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -11,9 +11,9 @@
 //===--===//
 
 #include "clang/Tooling/ASTDiff/ASTDiff.h"
-
 #include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -8,6 +8,7 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
+
 #include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
@@ -19,6 +20,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Template.h"
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ 

[clang] 86565c1 - Avoid SourceManager.h include in RawCommentList.h, add missing incs

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-02-27T13:49:40-08:00
New Revision: 86565c13094236e022d2238f5653641aaca7d31f

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

LOG: Avoid SourceManager.h include in RawCommentList.h, add missing incs

SourceManager.h includes FileManager.h, which is expensive due to
dependencies on LLVM FS headers.

Remove dead BeforeThanCompare specialization.

Sink ASTContext::addComment to cpp file.

This reduces the time to compile a file that does nothing but include
ASTContext.h from ~3.4s to ~2.8s for me.

Saves these includes:
219 -../clang/include/clang/Basic/SourceManager.h
204 -../clang/include/clang/Basic/FileSystemOptions.h
204 -../clang/include/clang/Basic/FileManager.h
165 -../llvm/include/llvm/Support/VirtualFileSystem.h
164 -../llvm/include/llvm/Support/SourceMgr.h
164 -../llvm/include/llvm/Support/SMLoc.h
161 -../llvm/include/llvm/Support/Path.h
141 -../llvm/include/llvm/ADT/BitVector.h
128 -../llvm/include/llvm/Support/MemoryBuffer.h
124 -../llvm/include/llvm/Support/FileSystem.h
124 -../llvm/include/llvm/Support/Chrono.h
124 -.../MSVCSTL/include/stack
122 -../llvm/include/llvm-c/Types.h
122 -../llvm/include/llvm/Support/NativeFormatting.h
122 -../llvm/include/llvm/Support/FormatProviders.h
122 -../llvm/include/llvm/Support/CBindingWrapping.h
122 -.../MSVCSTL/include/xtimec.h
122 -.../MSVCSTL/include/ratio
122 -.../MSVCSTL/include/chrono
121 -../llvm/include/llvm/Support/FormatVariadicDetails.h
118 -../llvm/include/llvm/Support/MD5.h
109 -.../MSVCSTL/include/deque
105 -../llvm/include/llvm/Support/Host.h
105 -../llvm/include/llvm/Support/Endian.h

Reviewed By: aaron.ballman, hans

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/ASTDumper.h
clang/include/clang/AST/RawCommentList.h
clang/lib/ARCMigrate/TransProtectedScope.cpp
clang/lib/AST/ASTContext.cpp
clang/lib/AST/DataCollection.cpp
clang/lib/AST/ExternalASTSource.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/AST/RawCommentList.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Analysis/CloneDetection.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/SanitizerMetadata.cpp
clang/lib/Index/CommentToXML.cpp
clang/lib/Index/FileIndexRecord.cpp
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Tooling/ASTDiff/ASTDiff.cpp
clang/lib/Tooling/Core/Lookup.cpp
clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
clang/lib/Tooling/Transformer/SourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp 
b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index 79d2499fcb93..ca8b1b59f89e 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "LexerUtils.h"
+#include "clang/Basic/SourceManager.h"
 
 namespace clang {
 namespace tidy {

diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index c944e09d5a75..2640c66cc8dd 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -721,15 +721,7 @@ class ASTContext : public RefCountedBase {
   RawComment *getRawCommentForDeclNoCache(const Decl *D) const;
 
 public:
-  RawCommentList () {
-return Comments;
-  }
-
-  void addComment(const RawComment ) {
-assert(LangOpts.RetainCommentsFromSystemHeaders ||
-   !SourceMgr.isInSystemHeader(RC.getSourceRange().getBegin()));
-Comments.addComment(RC, LangOpts.CommentOpts, BumpAlloc);
-  }
+  void addComment(const RawComment );
 
   /// Return the documentation comment attached to a given declaration.
   /// Returns nullptr if no comment is attached.

diff  --git a/clang/include/clang/AST/ASTDumper.h 
b/clang/include/clang/AST/ASTDumper.h
index 61202f057a80..f46ffb960db6 100644
--- a/clang/include/clang/AST/ASTDumper.h
+++ b/clang/include/clang/AST/ASTDumper.h
@@ -11,6 +11,7 @@
 
 #include "clang/AST/ASTNodeTraverser.h"
 #include "clang/AST/TextNodeDumper.h"
+#include "clang/Basic/SourceManager.h"
 
 namespace 

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D75077#1896708 , @cchen wrote:

> Add back the check of UO_Deref


Haven't added tests for some prohibited test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

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

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

Let's change this to either `CheckerManger` or `AnalysisManager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271



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

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:1786
+// i386
+case CK_i386:
+// Netburst

I found the documentation for the 82385 cache controller chip for the 386. It's 
a bit weird. The tags for the cache are based on 16 byte lines, but there are 
valid bits for every 2 bytes within that line. So its possible for only a 
portion of a line to be valid.



Comment at: clang/lib/Basic/Targets/X86.cpp:1840
+// Core
+case CK_Core2:
+case CK_Generic:

If Yonah and Penryn are 64, then Core2 should be as well. It's the generation 
between them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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

Add back the check of UO_Deref


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,21 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->p)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->s6[0].pp)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+  }
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +110,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected addressable lvalue in '#pragma omp target update' and '#pragma omp target map'}} le45-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
 #pragma omp target update to(S2::S2s)
 #pragma omp target update to(S2::S2sc)
 #pragma omp target update to(to)
 #pragma omp target update to(y x) // expected-error {{expected ',' or ')' in 'to' clause}}
-#pragma omp target update to(argc > 0 ? x : y) // expected-error 2 {{expected expression containing only member 

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Also, would be good to add more tests for the situations that should not be 
handled, like just `(a+b)`, explicit casts, etc.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9966
+def err_omp_non_lvalue_in_map_or_motion_clauses: Error<
+  "expected addressable lvalue in '#pragma omp target update' and '#pragma omp 
target map'"
+  >;

I still would change this message to something like `expected addressable 
lvalue in '%0' clause`, where `%0` must be one of `map|to|from` to be more 
specific and correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,21 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->p)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->s6[0].pp)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+  }
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +110,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected addressable lvalue in '#pragma omp target update' and '#pragma omp target map'}} le45-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
 #pragma omp target update to(S2::S2s)
 #pragma omp target update to(S2::S2sc)
 #pragma omp target update to(to)
 #pragma omp target update to(y x) // expected-error {{expected ',' or ')' in 'to' clause}}
-#pragma omp target update to(argc > 0 ? x : y) // expected-error 2 {{expected expression containing only member accesses 

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15684-15685
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+if (SemaRef.getLangOpts().OpenMP < 50 || UO->getOpcode() != UO_Deref ||
+!UO->isLValue()) {
+  emitErrorMsg();

ABataev wrote:
> `UO_Deref` is always r-value. So, `!UO->isLValue()` will be always triggered 
> for `UO_Deref`. You can just exclude it from the condition.
Oops, sorry, wrong comment, thought about Addr_of, but you removed it already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Robert Richmond via Phabricator via cfe-commits]

RobRich999 added a comment.

Based upon the description, I think this patch is more applicable than just 
targeting a specific AMD proc family since it allows the end-user a choice for 
maximizing threading with both CMT and SMT on all supported platforms.

BTW, until if/when this patch lands, I just set a static value in source as a 
local workaround for now.

llvm/lib/Support/Host.cpp

#elif defined(_WIN32)
// Defined in llvm/lib/Support/Windows/Threading.inc
static int computeHostNumPhysicalCores() { return 32; }
#else


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D75285#1896400 , @hfinkel wrote:

> Unfortunately, we cannot do this kind of thing just because it seems to make 
> sense. The language semantics must be exactly satisfied by the IR-level 
> semantics. I certainly agree that it would make sense for users to be able to 
> mark invariant loads, but this mechanism simply might not be the right one.
>
> One problem here is that, with something like:
>
>   char test2(X *x) {
> const char* __restrict p = &(x->b);
> return *p;
>   }
>   
>
> what happens when the function is inlined? Does the "invariantness" only 
> still apply to accesses within the scope of the local restrict pointer? I 
> believe that it would not, and that would be a problem because later code 
> might legally modify the relevant data.


How about inserting llvm.invariant.end at the end of scope of the variable?


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

https://reviews.llvm.org/D75285



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

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

[Alexandre Rames via Phabricator via cfe-commits]

arames updated this revision to Diff 247087.
arames added a comment.

Rename and clarify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72100

Files:
  clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/test/Frontend/verify-any-file.c
  clang/test/Frontend/verify-any-file.h

Index: clang/test/Frontend/verify-any-file.h
===
--- /dev/null
+++ clang/test/Frontend/verify-any-file.h
@@ -0,0 +1 @@
+unexpected var;
Index: clang/test/Frontend/verify-any-file.c
===
--- /dev/null
+++ clang/test/Frontend/verify-any-file.c
@@ -0,0 +1,14 @@
+// RUN: not %clang_cc1 -verify %s 2>&1 | FileCheck %s
+
+#include "verify-any-file.h"
+// expected-error@*:* {{unknown type name 'unexpected'}}
+
+// expected-error@*:* {{missing error}}
+
+// expected-error@*:123 {{invalid line : "*" required}}
+//
+//  CHECK: error: 'error' diagnostics expected but not seen:
+// CHECK-NEXT:   File * Line * (directive at {{.*}}verify-any-file.c:6): missing error
+// CHECK-NEXT: error: 'error' diagnostics seen but not expected:
+// CHECK-NEXT:   File {{.*}}verify-any-file.c Line 8: missing or invalid line number following '@' in expected '*'
+// CHECK-NEXT: 2 errors generated.
Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -89,9 +89,10 @@
 class StandardDirective : public Directive {
 public:
   StandardDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
-bool MatchAnyLine, StringRef Text, unsigned Min,
-unsigned Max)
-  : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max) {}
+bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+unsigned Min, unsigned Max)
+  : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine, MatchAnyLine, Text,
+  Min, Max) {}
 
   bool isValid(std::string ) override {
 // all strings are considered valid; even empty ones
@@ -107,9 +108,10 @@
 class RegexDirective : public Directive {
 public:
   RegexDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
- bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max,
- StringRef RegexStr)
-  : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max),
+ bool MatchAnyFileAndLine, bool MatchAnyLine, StringRef Text,
+ unsigned Min, unsigned Max, StringRef RegexStr)
+  : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyFileAndLine, MatchAnyLine, Text,
+  Min, Max),
 Regex(RegexStr) {}
 
   bool isValid(std::string ) override {
@@ -294,11 +296,12 @@
 // Attach the specified directive to the line of code indicated by
 // \p ExpectedLoc.
 void attachDirective(DiagnosticsEngine , const UnattachedDirective ,
- SourceLocation ExpectedLoc, bool MatchAnyLine = false) {
+ SourceLocation ExpectedLoc, bool MatchAnyFileAndLine = false,
+ bool MatchAnyLine = false) {
   // Construct new directive.
   std::unique_ptr D =
   Directive::create(UD.RegexKind, UD.DirectivePos, ExpectedLoc,
-MatchAnyLine, UD.Text, UD.Min, UD.Max);
+MatchAnyFileAndLine, MatchAnyLine, UD.Text, UD.Min, UD.Max);
 
   std::string Error;
   if (!D->isValid(Error)) {
@@ -498,6 +501,7 @@
 // Next optional token: @
 SourceLocation ExpectedLoc;
 StringRef Marker;
+bool MatchAnyFileAndLine = false;
 bool MatchAnyLine = false;
 if (!PH.Next("@")) {
   ExpectedLoc = Pos;
@@ -526,26 +530,39 @@
 StringRef Filename(PH.C, PH.P-PH.C);
 PH.Advance();
 
-// Lookup file via Preprocessor, like a #include.
-const DirectoryLookup *CurDir;
-Optional File =
-PP->LookupFile(Pos, Filename, false, nullptr, nullptr, CurDir,
-   nullptr, nullptr, nullptr, nullptr, nullptr);
-if (!File) {
-  Diags.Report(Pos.getLocWithOffset(PH.C-PH.Begin),
-   diag::err_verify_missing_file) << Filename << KindStr;
-  continue;
-}
-
-const FileEntry *FE = >getFileEntry();
-if (SM.translateFile(FE).isInvalid())
-  SM.createFileID(FE, Pos, SrcMgr::C_User);
-
-if (PH.Next(Line) && Line > 0)
-  ExpectedLoc = SM.translateFileLineCol(FE, Line, 1);
-else if (PH.Next("*")) {
+if (Filename == "*") {
+  MatchAnyFileAndLine = true;
+  if (!PH.Next("*")) {
+Diags.Report(Pos.getLocWithOffset(PH.C - 

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15684-15685
+  bool VisitUnaryOperator(UnaryOperator *UO) {
+if (SemaRef.getLangOpts().OpenMP < 50 || UO->getOpcode() != UO_Deref ||
+!UO->isLValue()) {
+  emitErrorMsg();

`UO_Deref` is always r-value. So, `!UO->isLValue()` will be always triggered 
for `UO_Deref`. You can just exclude it from the condition.



Comment at: clang/test/OpenMP/target_update_codegen.cpp:375
+  // CK6-DAG: [[L_VAL]] = load i32, i32* [[L_ADDR:%.+]]
+  // CK6-DAG: store i32 [[L:%.+]], i32* [[L_ADDR]]
+  #pragma omp target update to(*(B+l))

Seems to me, `L` is not used in the test, so you can just use `{{%.+}}` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Are you sure `restrict` alone isn't good enough?  It doesn't directly tell you 
that the memory is invariant, but it's usually simple to prove that the memory 
isn't modified within the `restrict` scope, which might be sufficient.


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

https://reviews.llvm.org/D75285



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:44
+  RawStringRecommendedExtensions(Options.getLocalOrGlobal(
+  "RecommendedExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseFileExtensions(RawStringSuspiciousExtensions,

Follow the convention of the other checks and use `HeaderFileExtensions` as the 
option name. Probably call the other option `ImplementationFileExtensions`



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:42
+StringRef Extension = Suffix.trim();
+for (StringRef::const_iterator it = Extension.begin();
+ it != Extension.end(); ++it) {

This could be changed to 
```
if (!llvm::all_of(Extension, isAlphanumeric))
  return false;
```



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38
+/// extensions.
+inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+

A lot of configuration options for clang tidy use semi-colon `;` seperated 
lists. Would it not be wise to carry on the convention and use semi colon here 
(and below) as well. It could break a few peoples configuration but that would 
be realised  fairly quickly and updated


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

https://reviews.llvm.org/D74669



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

If this is not the right way to tell the compiler a memory pointed to by a 
pointer is invariant, what is the recommended way?

Can we introduce clang builtins for llvm.invariant.start and llvm.invariant.end 
to allow user to specify that?

Thanks.


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

https://reviews.llvm.org/D75285



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

[PATCH] D67537: [clangd] Client-side support for inactive regions

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:193
+  if (scopes[0] == "meta.disabled") {
+this.inactiveDecorationIndex = index;
+return vscode.window.createTextEditorDecorationType({

hokein wrote:
> hmm, looks like clangd 
> [omits](https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clangd/SemanticHighlighting.cpp#L454)
>  the `InactiveCode` token.
> 
> I'd suggest we send it like other tokens, that will simplify the patch, we 
> don't need the `inactiveDecorationIndex` and `isInactive`, I think it also 
> helps when we migrate to the official implementation. WDYT?
> 
> 
This was discussed fairly extensively in the server-side review 
(https://reviews.llvm.org/D67536).

In the end, we decided to go with this approach of having a separate field in 
the protocol for inactive lines, for a variety of reasons including the one 
discussed in https://reviews.llvm.org/D67536#inline-616346


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67537



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

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen created this revision.
sdesmalen added reviewers: efriedma, rjmccall, rovka, rsandifo-arm.
Herald added subscribers: psnobl, rkruppe, kristof.beyls, tschuett, mgorny.
Herald added a reviewer: rengolin.
Herald added a project: clang.

This patch adds 'q' to mean 'scalable vector' in the builtin
type string, and for SVE will return the matching builtin
type as defined in the C/C++ language extensions for SVE.

This patch also adds some scaffolding to generate the arm_sve.h
header file, and some builtin definitions (+CodeGen) to be able
to implement some simple masked load intrinsics that use the
ACLE types, such as:

svint8_t test_svld1_s8(svbool_t pg, const int8_t *base) {

  return svld1_s8(pg, base);

}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75298

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/AArch64SVEACLETypes.def
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/include/clang/Basic/arm_sve.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/module.modulemap
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1.c
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/SveEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -91,6 +91,8 @@
 void EmitNeonSema2(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitNeonTest2(llvm::RecordKeeper , llvm::raw_ostream );
 
+void EmitSveHeader(llvm::RecordKeeper , llvm::raw_ostream );
+
 void EmitMveHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitMveBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitMveBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -70,6 +70,7 @@
   GenArmMveBuiltinSema,
   GenArmMveBuiltinCG,
   GenArmMveBuiltinAliases,
+  GenArmSveHeader,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -180,6 +181,8 @@
"Generate ARM NEON sema support for clang"),
 clEnumValN(GenArmNeonTest, "gen-arm-neon-test",
"Generate ARM NEON tests for clang"),
+clEnumValN(GenArmSveHeader, "gen-arm-sve-header",
+   "Generate arm_sve.h for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",
"Generate arm_mve.h for clang"),
 clEnumValN(GenArmMveBuiltinDef, "gen-arm-mve-builtin-def",
@@ -351,6 +354,9 @@
   case GenArmMveBuiltinAliases:
 EmitMveBuiltinAliases(Records, OS);
 break;
+  case GenArmSveHeader:
+EmitSveHeader(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/SveEmitter.cpp
===
--- /dev/null
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -0,0 +1,157 @@
+//===- SveEmitter.cpp - Generate arm_sve.h for use with clang -*- 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
+//
+//===--===//
+//
+// This tablegen backend is responsible for emitting arm_sve.h, which includes
+// a declaration and definition of each function specified by the ARM C/C++
+// Language Extensions (ACLE).
+//
+// For details, visit:
+//  https://developer.arm.com/architectures/system-architectures/software-standards/acle
+//
+// Each SVE instruction is implemented in terms of 1 or more functions which
+// are suffixed with the element type of the input vectors.  Functions may be
+// implemented in terms of generic vector operations such as +, *, -, etc. or
+// by calling a __builtin_-prefixed function which will be handled by clang's
+// CodeGen library.
+//
+// See also the documentation in include/clang/Basic/arm_sve.td.
+//
+//===--===//
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/Error.h"
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+
+//===--===//
+// SVEEmitter
+//===--===//
+
+namespace {
+
+class SVEEmitter {
+private:
+  RecordKeeper 
+
+public:

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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

Fix test and add check for unary operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,21 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->p)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->s6[0].pp)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+  }
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +110,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected addressable lvalue in '#pragma omp target update' and '#pragma omp target map'}} le45-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
 #pragma omp target update to(S2::S2s)
 #pragma omp target update to(S2::S2sc)
 #pragma omp target update to(to)
 #pragma omp target update to(y x) // expected-error {{expected ',' or ')' in 'to' clause}}
-#pragma omp target update to(argc > 0 ? x : y) // expected-error 2 {{expected expression containing 

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 247075.
cchen marked 14 inline comments as done.
cchen added a comment.

Remove redundant else


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_messages.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_from_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -1,6 +1,12 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=40 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wno-openmp-mapping -Wuninitialized
 
 void foo() {
 }
@@ -59,6 +65,21 @@
   double *p;
   unsigned bfa : 4;
 };
+struct S8 {
+  int *ptr;
+  int a;
+  struct S7* S;
+  void foo() {
+#pragma omp target update to(*this) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+#pragma omp target update to(*(this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->p)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(this->S->i+this->S->s6[0].pp)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+#pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
+  }
+};
 
 S3 h;
 #pragma omp threadprivate(h) // expected-note 2 {{defined as threadprivate or thread local}}
@@ -89,12 +110,12 @@
 #pragma omp target update to(x)
 #pragma omp target update to(t[:I])
 #pragma omp target update to(T) // expected-error {{'T' does not refer to a value}} expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
-#pragma omp target update to(I) // expected-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
+#pragma omp target update to(I) // le50-error 2 {{expected addressable lvalue in '#pragma omp target update' and '#pragma omp target map'}} le45-error 2 {{expected expression containing only member accesses and/or array sections based on named variables}}
 #pragma omp target update to(S2::S2s)
 #pragma omp target update to(S2::S2sc)
 #pragma omp target update to(to)
 #pragma omp target update to(y x) // expected-error {{expected ',' or ')' in 'to' clause}}
-#pragma omp target update to(argc > 0 ? x : y) // expected-error 2 {{expected 

[PATCH] D67537: [clangd] Client-side support for inactive regions

[Nathan Ridge via Phabricator via cfe-commits]

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

Update to use 'clangd.preprocessor.inactive' as the scope name for inactive 
lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67537

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
  clang-tools-extra/clangd/test/semantic-highlighting.test

Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -62,7 +62,7 @@
 # CHECK-NEXT:"entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
-# CHECK-NEXT:"meta.disabled"
+# CHECK-NEXT:"clangd.preprocessor.inactive"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -63,7 +63,8 @@
   test('Colorizer groups decorations correctly', async () => {
 const scopeTable = [
   [ 'variable' ], [ 'entity.type.function' ],
-  [ 'entity.type.function.method' ]
+  [ 'entity.type.function.method' ],
+  [ 'clangd.preprocessor.inactive' ]
 ];
 // Create the scope source ranges the highlightings should be highlighted
 // at. Assumes the scopes used are the ones in the "scopeTable" variable.
@@ -80,6 +81,12 @@
   new vscode.Position(line.line,
   token.character + token.length)));
 });
+if (line.isInactive) {
+  scopeRanges[scopeRanges.length - 1].push(new vscode.Range(
+new vscode.Position(line.line, 0),
+new vscode.Position(line.line, 0)
+  ));
+}
   });
   return scopeRanges;
 };
@@ -121,7 +128,8 @@
 tokens : [
   {character : 1, length : 2, scopeIndex : 1},
   {character : 10, length : 2, scopeIndex : 2},
-]
+],
+isInactive: false
   },
   {
 line : 2,
@@ -129,7 +137,8 @@
   {character : 3, length : 2, scopeIndex : 1},
   {character : 6, length : 2, scopeIndex : 1},
   {character : 8, length : 2, scopeIndex : 2},
-]
+],
+isInactive: true
   },
 ];
 
@@ -144,7 +153,8 @@
   line : 1,
   tokens : [
 {character : 2, length : 1, scopeIndex : 0},
-  ]
+  ],
+  isInactive: false
 };
 highlighter.highlight(fileUri2, [ highlightingsInLine1 ]);
 assert.deepEqual(
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -23,6 +23,8 @@
   // with its start position, length and the "lookup table" index of of the
   // semantic highlighting Text Mate scopes.
   tokens?: string;
+  // Whether the line is part of an inactive preprocessor branch.
+  isInactive?: boolean;
 }
 
 // A SemanticHighlightingToken decoded from the base64 data sent by clangd.
@@ -40,6 +42,8 @@
   line: number;
   // All SemanticHighlightingTokens on the line.
   tokens: SemanticHighlightingToken[];
+  // Whether the line is part of an inactive preprocessor branch.
+  isInactive: boolean;
 }
 
 // Language server push notification providing the semantic highlighting
@@ -122,7 +126,10 @@
 
   handleNotification(params: SemanticHighlightingParams) {
 const lines: SemanticHighlightingLine[] = params.lines.map(
-(line) => ({line : line.line, tokens : decodeTokens(line.tokens)}));
+  (line) => ({
+line: line.line, tokens: decodeTokens(line.tokens),
+isInactive: line.isInactive || false
+  }));
 this.highlighter.highlight(vscode.Uri.parse(params.textDocument.uri),
lines);
   }
@@ -161,6 +168,7 @@
   // SemanticHighlightingToken with scopeIndex i should have the decoration at
   // index i in this list.
   private decorationTypes: vscode.TextEditorDecorationType[] = [];
+  private inactiveDecorationIndex: number;
   // The clangd TextMate scope lookup table.
   private scopeLookupTable: string[][];
   constructor(scopeLookupTable: string[][]) {
@@ -180,7 +188,22 @@
   // theme is loaded.
   public 

[PATCH] D75223: [clang-offload-wrapper] Lower priority of __tgt_register_lib in favor of __tgt_register_requires

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75223



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

[PATCH] D75292: [clangd] Remove the word 'toy' from the description of vscode-clangd

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

We could go a step further and describe it as the "reference client" for 
clangd, but I'm not sure how accurate / desirable that is at this stage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75292



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

[PATCH] D75289: [clang-tidy] Added virtual isLanguageVersionSupported to ClangTidyCheck

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, Eugene.Zelenko, JonasToth, 
alexfh, hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
xazax.hun.
Herald added a project: clang.

Motivated by Tune inspections to a specific C++ standard. 

Moves the isLanguageVersionSupported virtual function from `MakeSmartPtrCheck` 
to the base `ClangTidyCheck` class.
This will disable registering matchers or pp callbacks on unsupported language 
versions for a check.
Having it as a standalone function is cleaner than manually disabling the check 
in the register function and should hopefully
encourage check developers to actually restrict the check based on language 
version.
As an added bonus this could enable automatic detection of what language 
version a check runs on for the purpose of documentation generation


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75289

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -67,6 +67,8 @@
 // `getLangOpts()`).
 CheckFactory::createChecks(, Checks);
 for (auto  : Checks) {
+  if (!Check->isLanguageVersionSupported(Context.getLangOpts()))
+continue;
   Check->registerMatchers();
   Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
 }
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -301,6 +301,8 @@
 });
 Preprocessor *PP = >getPreprocessor();
 for (const auto  : CTChecks) {
+  if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
+continue;
   // FIXME: the PP callbacks skip the entire preamble.
   // Checks that want to see #includes in the main file do not see them.
   Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -41,7 +41,7 @@
   virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0;
 
   /// Returns whether the C++ version is compatible with current check.
-  virtual bool isLanguageVersionSupported(const LangOptions ) const;
+  bool isLanguageVersionSupported(const LangOptions ) const override;
 
   static const char PointerType[];
 
Index: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -68,17 +68,12 @@
 void MakeSmartPtrCheck::registerPPCallbacks(const SourceManager ,
 Preprocessor *PP,
 Preprocessor *ModuleExpanderPP) {
-  if (isLanguageVersionSupported(getLangOpts())) {
 Inserter = std::make_unique(SM, getLangOpts(),
  IncludeStyle);
 PP->addPPCallbacks(Inserter->CreatePPCallbacks());
-  }
 }
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  if (!isLanguageVersionSupported(getLangOpts()))
-return;
-
   // Calling make_smart_ptr from within a member function of a type with a
   // private or protected constructor would be ill-formed.
   auto CanCallCtor = unless(has(ignoringImpCasts(
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -100,6 +100,16 @@
   /// whether it has the default value or it has been overridden.
   virtual void storeOptions(ClangTidyOptions::OptionMap ) {}
 
+  /// Override this to disable registering matchers and PP callbacks if an
+  /// invalid language version is being used.
+  ///
+  /// For example if a check is examining overloaded functions then this should
+  /// be overridden to return false when the CPlusPlus flag is not set in 
+  /// \p LangOpts.
+  virtual bool isLanguageVersionSupported(const LangOptions ) const {
+return true;
+  }
+
   /// Provides access to the 

[PATCH] D75162: [X86][F16C] Remove cvtph2ps intrinsics and use generic half2float conversion (PR37554)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

In D75162#1896365 , @craig.topper 
wrote:

> Should we check that this generates constrained.fpext in strict mode?


Sure - where is the best place(s) to test this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75162



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

[PATCH] D75292: [clangd] Remove the word 'toy' from the description of vscode-clangd

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
nridge added a comment.

We could go a step further and describe it as the "reference client" for 
clangd, but I'm not sure how accurate / desirable that is at this stage.


vscode-clangd today is a relatively mature and usable extension.
The word "toy" in its description may discourage some from using it
or contributing to it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75292

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
===
--- clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
+++ clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
@@ -1,4 +1,4 @@
-# Toy VS Code Extension for clangd
+# VS Code Extension for clangd
 
 ## What's in the folder
 * This folder contains all of the files necessary for your extension
@@ -30,4 +30,4 @@
 * see the output of the test result in the debug console
 * make changes to `test/extension.test.ts` or create new test files inside the 
`test` folder
 * by convention, the test runner will only consider files matching the 
name pattern `**.test.ts`
-* you can create folders inside the `test` folder to structure your tests 
any way you want
\ No newline at end of file
+* you can create folders inside the `test` folder to structure your tests 
any way you want


Index: clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
===
--- clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
+++ clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
@@ -1,4 +1,4 @@
-# Toy VS Code Extension for clangd
+# VS Code Extension for clangd
 
 ## What's in the folder
 * This folder contains all of the files necessary for your extension
@@ -30,4 +30,4 @@
 * see the output of the test result in the debug console
 * make changes to `test/extension.test.ts` or create new test files inside the `test` folder
 * by convention, the test runner will only consider files matching the name pattern `**.test.ts`
-* you can create folders inside the `test` folder to structure your tests any way you want
\ No newline at end of file
+* you can create folders inside the `test` folder to structure your tests any way you want
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Unfortunately, `const` also doesn't mean that the memory doesn't change.   It 
does mean it can't be changed through this pointer, but `restrict` allows you 
to derive more pointers from it within the `restrict` scope, and those pointers 
can remove the `const` qualifier.


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

https://reviews.llvm.org/D75285



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

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Friendly ping. Any other comments on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_update_codegen.cpp:374
+  // CK6-64-DAG: [[IDX_EXT]] = sext i32 [[TWO:%.+]] to i64
+  // CK6-DAG: [[TWO:%.+]] = load i32, i32* [[L_ADDR:%.+]]
+  // CK6-DAG: store i32 [[L:%.+]], i32* [[L_ADDR]]

cchen wrote:
> ABataev wrote:
> > Must be just `[[TWO]]`?
> You mean use a more representable name?
No, I mean that here you should not redefine the variable but check that the 
previously defined `TWO` is used in the check line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15710
+// know the other subtree is just an offset)
+assert(BO->getType()->isPointerType() &&
+   "Expect the binary operator be pointer type");

cchen wrote:
> cchen wrote:
> > ABataev wrote:
> > > Not sure this should be an assert. What if you have something like 
> > > `*((a+b) + c)` and `a+b` is not pointer arithmetic?
> > Good point, I'll fix it. Thanks
> Well, for this case, the type of binop expr `*((a+b) + c)` is `int *`, and 
> the type of LHS is `int`, while the type of RHS is `int *`, therefore, expr 
> `(a+b)` is not going to be visited. I'm using assert here since before the 
> `checkMapClauseExpressionBase` be called, we have already check for whether 
> the expression is lvalue. Maybe I'll just error out if binop here is not a 
> pointer type instead of `assert`?
Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D74387: [SYCL] Do not diagnose use of __float128

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D74387#1895264 , @Fznamznon wrote:

> @rjmccall, Thank you very much for so detailed response, It really helps. I 
> started working on implementation and I have a couple of questions/problems 
> with this particular appoach.
>
> > - If `foo` uses `__float128` (whether in its signature or internally), that 
> > is invalid in device mode, but the diagnostic will be delayed by the 
> > forbidden-type mechanism, meaning that it will become an `unavailable` 
> > attribute on `foo`.
>
> So, for example if some variable is declared with `__float128` type, we are 
> adding to parent function Unavaliable attribute, right?


That's how it's supposed to work.  I can't guarantee that it will actually 
always work that way, because I'm sure you'll be pushing on this code in some 
new ways.

>> - If `bar` uses `foo`, that use is invalid in device mode (because of the 
>> `unavailable` attribute), but the diagnostic will be delayed via the 
>> standard CUDA/OMP mechanism because we don't know yet whether `bar` should 
>> be emitted as a device function.
>> - If `kernel` uses `bar`, that will trigger the emission of the delayed 
>> diagnostics of `bar`, including the use-of-`unavailable`-function diagnostic 
>> where it uses `foo`.  It should be straightforward to specialize this 
>> diagnostic so that it reports the error by actually diagnosing the use of 
>> `__float128` at the original location (which is recorded in the 
>> `unavailable` attribute) and then just adding a note about how `foo` is used 
>> by `bar`.
> 
> Consider following example (this is absolutely valid SYCL code, except 
> __float128 usage):
> 
>   // Host code:
>   __float128 A;
>   // Everything what lambda passed to `sycl_kernel` calls becomes device 
> code. Capturing of host variables means that these variables will be passed 
> to device by value, so using of A in this lambda is invalid.
>   sycl_kernel([=]() {auto B = A});
>
>
> 
> In this case we add unavailable attribute to parent function for variable `A` 
> declaration. But this function is not called from device code. Please correct 
> me if I'm wrong but it seems that we need to diagnose not only functions, but 
> also  usages of any declarations with unavailable attribute including 
> variable declarations, right?

Right.  The diagnosis side of that should already happen — `unavailable` 
diagnostics apply to uses of any kind of declaration, not just functions or 
variables.  Current delayed diagnostics should be enough to make the 
`unavailable` attribute get applied to the global  variable `A` in your 
example,  since it's a use from the declarator.   If SYCL supports C++-style 
dynamic global initializers, you'll probably need to add code so that uses of 
`__float128` within a global initializer get associated with the global, which 
currently won't happen because the initializer isn't "in scope".  But there are 
at least two other patches underway that are dealing with similar issues: 
https://reviews.llvm.org/D71227 and https://reviews.llvm.org/D70172.

> In addition, there are a couple of problems with this approach, for example 
> with unevaluated `sizeof` context, i.e. code like this:
> 
>   sycl_kernel([=]() {int A = sizeof(__float128);});
> 
> 
> is diagnosed too, I think this is not correct.

Okay, that's a much thornier problem if you want to allow that.  What you're 
talking about is essentially the difference between an evaluated and an 
unevaluated context, but we don't generally track that for uses of *types*.  
It's much easier to set things up so that you only complain about uses of 
*values* like the global variable A if they're in evaluated expressions, but 
types are never really "evaluated" in the same way that expressions are, so 
it's complicated.

I think that's a very separable question, so I would recommend you focus on the 
first problem right now, and then if you really care about allowing 
`sizeof(__float128)`, we can approach that later.

> I can upload what I have now to this review if it will help better (or maybe 
> we will understand that I'm doing something wrong).
> 
> I'm also thinking about a bit another approach:
> 
> - If some declaration uses `__float128` it will become an unavailable 
> attribute on this declaration. That means we don't always add unavailable 
> attribute to the function which uses `__float128` internally.
> - In the place where clang actually emits `use-of-unavailable-declaration` 
> diagnostics (somewhere in `DoEmitAvailabilityWarning` function, defined in 
> `SemaAvailability.cpp`) for SYCL, we make these diagnostics deferred using 
> CUDA/OMP deferred diagnostics mechanism (using SYCL-specific analog of 
> function like `diagIfOpenMPDeviceCode`/`CUDADiagIfDeviceCode`).

Sure, but you'll have to write a custom walk of the body looking for uses of 
the type that you don't like; that seems like a lot of work to get right, and 
it'll tend to fail 

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso added a comment.

> ! In D75271#1896223 , @Szelethus 
> wrote:
>  Thinking back, I did have a number of failed attempts to make something a 
> bit less ugly, but the sharp divide between the 2 libraries makes is 
> really-really difficult, and I don't recall alternative solutions being that 
> much better. Either the checker interface gets worse, or the checker 
> registration interface gets so messy that it would severely hurt further 
> improvements in terms of checker dependency development.

Well, if you cannot solve this problem, most likely we neither, but keep in 
mind:

> If you can't solve a problem, then there is an easier problem you can't 
> solve: find it.

- George Polya

---

As far as you guys keep updating your API I will buy it, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271



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

[PATCH] D75279: Avoid SourceManager.h include in RawCommentList.h, add missing incs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75279



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15710
+// know the other subtree is just an offset)
+assert(BO->getType()->isPointerType() &&
+   "Expect the binary operator be pointer type");

cchen wrote:
> ABataev wrote:
> > Not sure this should be an assert. What if you have something like `*((a+b) 
> > + c)` and `a+b` is not pointer arithmetic?
> Good point, I'll fix it. Thanks
Well, for this case, the type of binop expr `*((a+b) + c)` is `int *`, and the 
type of LHS is `int`, while the type of RHS is `int *`, therefore, expr `(a+b)` 
is not going to be visited. I'm using assert here since before the 
`checkMapClauseExpressionBase` be called, we have already check for whether the 
expression is lvalue. Maybe I'll just error out if binop here is not a pointer 
type instead of `assert`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[Hal Finkel via Phabricator via cfe-commits]

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

Unfortunately, we cannot do this kind of thing just because it seems to make 
sense. The language semantics must be exactly satisfied by the IR-level 
semantics. I certainly agree that it would make sense for users to be able to 
mark invariant loads, but this mechanism simply might not be the right one.

One problem here is that, with something like:

  char test2(X *x) {
const char* __restrict p = &(x->b);
return *p;
  }

what happens when the function is inlined? Does the "invariantness" only still 
apply to accesses within the scope of the local restrict pointer? I believe 
that it would not, and that would be a problem because later code might legally 
modify the relevant data.


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

https://reviews.llvm.org/D75285



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

[PATCH] D31343: Add an attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/examples/Attribute/Attribute.cpp:35
+  }
+  virtual bool handleDeclAttribute(Sema , Decl *D,
+   const ParsedAttr ) const {

john.brawn wrote:
> aaron.ballman wrote:
> > It is unclear to me how a user would add an attribute that needs to follow 
> > the merge pattern we use in `mergeDeclAttribute()`.
> I'm guessing it's probably not possible, from a quick look at 
> mergeDeclAttribute.
That... seems like a problem that we should fix (or at least have a plan for 
fixing). Merging attributes is important so that we can have attributes with 
consistency checks between declarations for anything other than basic attribute 
inheritance. FWIW, there are 20+ attributes that already need this 
functionality, so it does get used relatively often.



Comment at: clang/examples/Attribute/Attribute.cpp:35
+Spellings.push_back({ParsedAttr::AS_GNU, "example"});
+Spellings.push_back({ParsedAttr::AS_CXX11, "::example"});
+Spellings.push_back({ParsedAttr::AS_CXX11, "plugin::example"});

This is not a valid spelling for an attribute (we should probably assert that 
an attribute-scoped-token does not have an empty scope).



Comment at: clang/examples/Attribute/Attribute.cpp:42
+// This attribute appertains to functions only.
+if (!D || !isa(D)) {
+  S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)

I don't think `D` can ever be null, can it?


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

https://reviews.llvm.org/D31343



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

[PATCH] D31342: Add ParsedAttrInfo::handleDeclAttribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:105
+   target.
+
 Putting it all together

It might also be nice to link to the example code from D31343 from the 
documentation to help get users started. e.g., "To see a working example of an 
attribute plugin, see clang/examples/Attribute/Attribute.cpp" with a link to 
the git repo source viewer for that file. WDYT?


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

https://reviews.llvm.org/D31342



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15710
+// know the other subtree is just an offset)
+assert(BO->getType()->isPointerType() &&
+   "Expect the binary operator be pointer type");

ABataev wrote:
> Not sure this should be an assert. What if you have something like `*((a+b) + 
> c)` and `a+b` is not pointer arithmetic?
Good point, I'll fix it. Thanks



Comment at: clang/test/OpenMP/target_update_codegen.cpp:374
+  // CK6-64-DAG: [[IDX_EXT]] = sext i32 [[TWO:%.+]] to i64
+  // CK6-DAG: [[TWO:%.+]] = load i32, i32* [[L_ADDR:%.+]]
+  // CK6-DAG: store i32 [[L:%.+]], i32* [[L_ADDR]]

ABataev wrote:
> Must be just `[[TWO]]`?
You mean use a more representable name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75162: [X86][F16C] Remove cvtph2ps intrinsics and use generic half2float conversion (PR37554)

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Should we check that this generates constrained.fpext in strict mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75162



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

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

Not handling this was a side-effect of being overly cautious when trying
to avoid reading files for which clangd doesn't have the source mapped.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -274,7 +274,9 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -290,9 +290,10 @@
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(
+  DiagLevel, Info, *CTContext,
+  Info.getSourceManager().getMainFileID())) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -212,15 +212,14 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If a valid FileID is passed as `FileFilter`, the function does not attempt
+/// to read source files other than the one identified by the filter. A degree
+/// of control over this is needed because in some use cases we cannot rely on
+/// files other than the one containing the diagnostic being mapped in the
+/// SourceManager.
 bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
   const Diagnostic , ClangTidyContext ,
-  bool CheckMacroExpansion = true);
+  FileID FileFilter = FileID{});
 
 /// A diagnostic consumer that turns each \c Diagnostic into a
 /// \c SourceManager-independent \c ClangTidyError.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -120,7 +120,6 @@
 : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
   IsWarningAsError(IsWarningAsError) {}
 
-
 class ClangTidyContext::CachedGlobList {
 public:
   CachedGlobList(StringRef Globs) : Globs(Globs) {}
@@ -341,13 +340,18 @@
 
 static bool LineIsMarkedWithNOLINTinMacro(const SourceManager ,
   SourceLocation Loc, unsigned DiagID,
-  const ClangTidyContext ) {
+  const ClangTidyContext ,
+  FileID FileFilter) {
   while (true) {
 if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
   return true;
 if (!Loc.isMacroID())
   return false;
 Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+if (FileFilter.isValid() && Loc.isFileID() &&
+SM.getFileID(Loc) != FileFilter) {
+  return false;
+}
   }
   return false;
 }
@@ -357,14 +361,13 @@
 
 bool 

[PATCH] D75279: Avoid SourceManager.h include in RawCommentList.h, add missing incs

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

Nice, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75279



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

[PATCH] D75285: Mark restrict pointer or reference to const as invariant

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: rjmccall.

We saw users intend to use `const int* restrict` to indicate the memory pointed 
to by the pointer is invariant.

This makes sense since restrict means the memory is not aliased by any other 
pointers whereas const
means the memory does not change.

Mark such pointer or reference as invariant allows more optimization 
opportunities.

This gives users a way to mark something as invariant.


https://reviews.llvm.org/D75285

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGenCXX/invariant.cpp

Index: clang/test/CodeGenCXX/invariant.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/invariant.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -O3 -emit-llvm -o - %s | FileCheck %s
+
+typedef struct {
+  int a;
+  char b;
+} X;
+
+const int* __restrict p;
+
+// CHECK-LABEL: test1
+// CHECK: llvm.invariant.start
+int test1() {
+  return *p;
+}
+
+// CHECK-LABEL: test2
+// CHECK: llvm.invariant.start
+char test2(X *x) {
+  const char* __restrict p = &(x->b);
+  return *p;
+}
+
+// CHECK-LABEL: test3
+// CHECK: llvm.invariant.start
+char test3(X ) {
+  const char& __restrict p = x.b;
+  return p;
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4070,7 +4070,7 @@
 llvm::GlobalVariable *GV);
 
   // Emit an @llvm.invariant.start call for the given memory region.
-  void EmitInvariantStart(llvm::Constant *Addr, CharUnits Size);
+  void EmitInvariantStart(llvm::Value *Addr, CharUnits Size);
 
   /// EmitCXXGlobalVarDeclInit - Create the initializer for a C++
   /// variable with global storage.
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -308,6 +308,20 @@
 E->getExprLoc());
 
 EmitLValueAlignmentAssumption(E, V);
+
+// Mark a restrict pointer or to const as invariant.
+if (const auto *DRE = dyn_cast(E)) {
+  const ValueDecl *VD = DRE->getDecl();
+  auto QT = VD->getType();
+  if (QT->isPointerType()) {
+auto PointeeTy = QT->getPointeeType();
+if (PointeeTy.isConstQualified() && QT.isRestrictQualified()) {
+  CGF.EmitInvariantStart(
+  V, CGF.getContext().getTypeSizeInChars(PointeeTy));
+}
+  }
+}
+
 return V;
   }
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1252,62 +1252,88 @@
 ///
 LValue CodeGenFunction::EmitLValue(const Expr *E) {
   ApplyDebugLocation DL(*this, E);
+  LValue Ret;
   switch (E->getStmtClass()) {
-  default: return EmitUnsupportedLValue(E, "l-value expression");
+  default:
+Ret = EmitUnsupportedLValue(E, "l-value expression");
+break;
 
   case Expr::ObjCPropertyRefExprClass:
 llvm_unreachable("cannot emit a property reference directly");
+break;
 
   case Expr::ObjCSelectorExprClass:
-return EmitObjCSelectorLValue(cast(E));
+Ret = EmitObjCSelectorLValue(cast(E));
+break;
   case Expr::ObjCIsaExprClass:
-return EmitObjCIsaExpr(cast(E));
+Ret = EmitObjCIsaExpr(cast(E));
+break;
   case Expr::BinaryOperatorClass:
-return EmitBinaryOperatorLValue(cast(E));
+Ret = EmitBinaryOperatorLValue(cast(E));
+break;
   case Expr::CompoundAssignOperatorClass: {
 QualType Ty = E->getType();
 if (const AtomicType *AT = Ty->getAs())
   Ty = AT->getValueType();
 if (!Ty->isAnyComplexType())
-  return EmitCompoundAssignmentLValue(cast(E));
-return EmitComplexCompoundAssignmentLValue(cast(E));
+  Ret = EmitCompoundAssignmentLValue(cast(E));
+else
+  Ret =
+  EmitComplexCompoundAssignmentLValue(cast(E));
+break;
   }
   case Expr::CallExprClass:
   case Expr::CXXMemberCallExprClass:
   case Expr::CXXOperatorCallExprClass:
   case Expr::UserDefinedLiteralClass:
-return EmitCallExprLValue(cast(E));
+Ret = EmitCallExprLValue(cast(E));
+break;
   case Expr::CXXRewrittenBinaryOperatorClass:
-return EmitLValue(cast(E)->getSemanticForm());
+Ret = EmitLValue(cast(E)->getSemanticForm());
+break;
   case Expr::VAArgExprClass:
-return EmitVAArgExprLValue(cast(E));
+Ret = EmitVAArgExprLValue(cast(E));
+break;
   case Expr::DeclRefExprClass:
-return EmitDeclRefLValue(cast(E));
+Ret = EmitDeclRefLValue(cast(E));
+break;
   case Expr::ConstantExprClass:
-return EmitLValue(cast(E)->getSubExpr());
+Ret = EmitLValue(cast(E)->getSubExpr());
+break;
   case Expr::ParenExprClass:
-return 

[PATCH] D31342: Add ParsedAttrInfo::handleDeclAttribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ClangPlugins.rst:89
+The members of ``ParsedAttrInfo`` that a plugin attribute must define are:
+ * ``AttrKind``, which must be set to ``ParsedAttr::NoSemaHandlerAttribute``.
+ * ``Spellings``, which must be populated with the attribute syntaxes that are

If it must always be this one specific value, why do we make users set it 
manually instead of setting it automatically for them?



Comment at: clang/docs/ClangPlugins.rst:90
+ * ``AttrKind``, which must be set to ``ParsedAttr::NoSemaHandlerAttribute``.
+ * ``Spellings``, which must be populated with the attribute syntaxes that are
+   allowed and how the attribute name is spelled for each syntax.

The documentation should mention what type this is -- the example above doesn't 
name the type, just uses braced initialization, so it may be unclear how to 
specify a scope, for instance.



Comment at: clang/docs/ClangPlugins.rst:92
+   allowed and how the attribute name is spelled for each syntax.
+ * ``handleDeclAttribute``, which is the function that applies the attribute to
+   a declaration.

This returns a value, but there's no mention of what `true`/`false` means, and 
the example doesn't make it clear how one would "handle" the attribute (e.g., 
how one creates the semantic attribute and attaches it to the declaration).


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

https://reviews.llvm.org/D31342



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

[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15690
+  return false;
+if (!UO->isLValue()) {
+  emitErrorMsg();

Hmm, addr_of creates rvalues.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15693
+  return false;
+} else {
+  if (!RelevantExpr) {

No need for `else` here



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15710
+// know the other subtree is just an offset)
+assert(BO->getType()->isPointerType() &&
+   "Expect the binary operator be pointer type");

Not sure this should be an assert. What if you have something like `*((a+b) + 
c)` and `a+b` is not pointer arithmetic?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:15720
+  return Visit(LE);
+else
+  return Visit(RE);

No need for `else` here



Comment at: clang/test/OpenMP/target_update_codegen.cpp:374
+  // CK6-64-DAG: [[IDX_EXT]] = sext i32 [[TWO:%.+]] to i64
+  // CK6-DAG: [[TWO:%.+]] = load i32, i32* [[L_ADDR:%.+]]
+  // CK6-DAG: store i32 [[L:%.+]], i32* [[L_ADDR]]

Must be just `[[TWO]]`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077



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

[PATCH] D75261: [clang-format] Recognize C# nullable types

[Krasimir Georgiev via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:609
+
+  verifyFormat(R"(public float? Value;)", Style); // no space before `?`.
+}

Could you also try/add a test for an array of nullable type (from language ref 
docs [[ 
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/nullable-value-types
 | here ]]) (also an example of using `new` with a nullable type):
```// An array of a nullable type:
int?[] arr = new int?[10];```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75261



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:76
+
+  const auto SE = utils::getFileExtension(FileName, 
Check.SuspiciousExtensions);
+  if (!SE)

Please don't use `auto` here (the type isn't spelled out in the initialization).



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:36
+bool parseFileExtensions(StringRef AllFileExtensions,
+ FileExtensionsSet , char delimiter) {
+  SmallVector Suffixes;

`Delimiter` here as well.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:54
+getFileExtension(StringRef FileName, const FileExtensionsSet ) {
+  StringRef extension = llvm::sys::path::extension(FileName);
+  if (extension.empty())

`Extension`



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:9
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H

Can drop the `HEADER` from these macros.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:48
+bool parseFileExtensions(StringRef AllFileExtensions,
+ FileExtensionsSet , char delimiter);
+

delimiter -> Delimiter per our usual naming conventions.


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

https://reviews.llvm.org/D74669



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

[PATCH] D68887: [clang-tidy] Copy the Ranges field from the Diagnostic when creating the ClangTidyError

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D68887



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

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

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

Szelethus added a comment.

In D75271#1896182 , @Szelethus wrote:

> > Also this entire callback should be removed ideally: it has to be a virtual 
> > function defaulting to `return true;` and if someone needs this feature 
> > could rewrite the behavior. I guess there was some debate whether it should 
> > be on by default or not, but for a checker writer and future changes this 
> > patch shows that how weak this API is.
>
> Yup, that is a very good criticism of the the API. I would prefer to see 
> something a lot less ugly, and I strongly share you sentiment that making 
> changes to it is about as ugly as the initial patch itself. Virtual functions 
> would be okay, the reason why I didn't do that is because where do you put 
> them? Checker objects can't house them, because the point of the entire 
> `shouldRegister` function is to never create them in the first place. But 
> thinking about it again, the problem isn't really their **creation** as much 
> as their **registration**. If we were to create a checker object and ask it 
> whether its fine to do analysis, and //then// store it, we would be able to 
> get rid of the `shouldRegister` function. I can see from a mile away that due 
> to the library layout, cyclic linking dependencies, and whatever else would 
> make this seemingly trivial task a lot more invasive and difficult, and I'm 
> just not too sure whether its worth the effort to change an arguably bearable 
> inconvenience.


Thinking back, I did have a number of failed attempts to make something a bit 
less ugly, but the sharp divide between the 2 libraries makes is really-really 
difficult, and I don't recall alternative solutions being that much better. 
Either the checker interface gets worse, or the checker registration interface 
gets so messy that it would severely hurt further improvements in terms of 
checker dependency development.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271



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

[PATCH] D75279: Avoid SourceManager.h include in RawCommentList.h, add missing incs

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: aaron.ballman, hans.
Herald added a reviewer: teemperor.
Herald added subscribers: lldb-commits, arphaman.
Herald added projects: clang, LLDB.

SourceManager.h includes FileManager.h, which is expensive due to
dependencies on LLVM FS headers.

Remove dead BeforeThanCompare specialization.

Sink ASTContext::addComment to cpp file.

This reduces the time to compile a file that does nothing but include
ASTContext.h from ~3.4s to ~2.8s for me.

Saves these includes:

  219 -../clang/include/clang/Basic/SourceManager.h
  204 -../clang/include/clang/Basic/FileSystemOptions.h
  204 -../clang/include/clang/Basic/FileManager.h
  165 -../llvm/include/llvm/Support/VirtualFileSystem.h
  164 -../llvm/include/llvm/Support/SourceMgr.h
  164 -../llvm/include/llvm/Support/SMLoc.h
  161 -../llvm/include/llvm/Support/Path.h
  141 -../llvm/include/llvm/ADT/BitVector.h
  128 -../llvm/include/llvm/Support/MemoryBuffer.h
  124 -../llvm/include/llvm/Support/FileSystem.h
  124 -../llvm/include/llvm/Support/Chrono.h
  124 -.../MSVCSTL/include/stack
  122 -../llvm/include/llvm-c/Types.h
  122 -../llvm/include/llvm/Support/NativeFormatting.h
  122 -../llvm/include/llvm/Support/FormatProviders.h
  122 -../llvm/include/llvm/Support/CBindingWrapping.h
  122 -.../MSVCSTL/include/xtimec.h
  122 -.../MSVCSTL/include/ratio
  122 -.../MSVCSTL/include/chrono
  121 -../llvm/include/llvm/Support/FormatVariadicDetails.h
  118 -../llvm/include/llvm/Support/MD5.h
  109 -.../MSVCSTL/include/deque
  105 -../llvm/include/llvm/Support/Host.h
  105 -../llvm/include/llvm/Support/Endian.h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75279

Files:
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTDumper.h
  clang/include/clang/AST/RawCommentList.h
  clang/lib/ARCMigrate/TransProtectedScope.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DataCollection.cpp
  clang/lib/AST/ExternalASTSource.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RawCommentList.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CloneDetection.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/lib/Index/FileIndexRecord.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/lib/Tooling/Core/Lookup.cpp
  clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
  clang/lib/Tooling/Transformer/SourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -21,6 +21,7 @@
 #include "lldb/Utility/Log.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/Basic/SourceManager.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
Index: clang/lib/Tooling/Transformer/SourceCode.cpp
===
--- clang/lib/Tooling/Transformer/SourceCode.cpp
+++ clang/lib/Tooling/Transformer/SourceCode.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/Errc.h"
 
Index: clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFinder.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
Index: clang/lib/Tooling/Core/Lookup.cpp
===
--- clang/lib/Tooling/Core/Lookup.cpp
+++ clang/lib/Tooling/Core/Lookup.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/SmallVector.h"
 using namespace clang;
 using namespace clang::tooling;
Index: clang/lib/Tooling/ASTDiff/ASTDiff.cpp

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, sunfish, aheejin, jgravelle-google, 
dschuff.
Herald added a project: clang.

Emscripten now handles/supports this new mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75277

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -448,10 +448,8 @@
 CodeGenFunction(*this).EmitCfiCheckStub();
   }
   emitAtAvailableLinkGuard();
-  if (Context.getTargetInfo().getTriple().isWasm() &&
-  !Context.getTargetInfo().getTriple().isOSEmscripten()) {
+  if (Context.getTargetInfo().getTriple().isWasm())
 EmitMainVoidAlias();
-  }
   emitLLVMUsed();
   if (SanStats)
 SanStats->finish();
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -67,9 +67,7 @@
 
   // On wasm, the argc/argv form of "main" is renamed so that the startup code
   // can call it with the correct function signature.
-  // On Emscripten, users may be exporting "main" and expecting to call it
-  // themselves, so we can't mangle it.
-  if (Triple.isWasm() && !Triple.isOSEmscripten())
+  if (Triple.isWasm())
 if (const FunctionDecl *FD = dyn_cast(ND))
   if (FD->isMain() && FD->hasPrototype() && FD->param_size() == 2)
 return CCM_WasmMainArgcArgv;


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -448,10 +448,8 @@
 CodeGenFunction(*this).EmitCfiCheckStub();
   }
   emitAtAvailableLinkGuard();
-  if (Context.getTargetInfo().getTriple().isWasm() &&
-  !Context.getTargetInfo().getTriple().isOSEmscripten()) {
+  if (Context.getTargetInfo().getTriple().isWasm())
 EmitMainVoidAlias();
-  }
   emitLLVMUsed();
   if (SanStats)
 SanStats->finish();
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -67,9 +67,7 @@
 
   // On wasm, the argc/argv form of "main" is renamed so that the startup code
   // can call it with the correct function signature.
-  // On Emscripten, users may be exporting "main" and expecting to call it
-  // themselves, so we can't mangle it.
-  if (Triple.isWasm() && !Triple.isOSEmscripten())
+  if (Triple.isWasm())
 if (const FunctionDecl *FD = dyn_cast(ND))
   if (FD->isMain() && FD->hasPrototype() && FD->param_size() == 2)
 return CCM_WasmMainArgcArgv;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66564: [clang-tidy] new altera struct pack align check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66564#1895980 , @Eugene.Zelenko 
wrote:

> In D66564#1895916 , @aaron.ballman 
> wrote:
>
> > Are you aware of plans that you or others have for adding additional checks 
> > to the new `altera` module?
>
>
> There are several patches for `altera` module already.


Excellent, thank you for the confirmation!




Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:23
+void StructPackAlignCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(recordDecl(isStruct(), isDefinition()).bind("struct"),
+ this);

I don't think we want this check to trigger on template instantiations because 
the packing and alignment requirements may be different there for each 
instantiation type. e.g.,
```
template 
struct S {
  Ty One;
  int Two;
  Uy Three;
};
```



Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:87
+  if (NeedsPacking && !IsPacked) {
+diag(Struct->getLocation(),
+ "accessing fields in struct %0 is inefficient due to padding; only "

Should this be diagnosing structures declared in system headers? My intuition 
is that we don't want to diagnose those because they're outside of the user's 
control. This can be handled when registering the matchers for the check, if we 
want to ignore those (`unless(isExpansionInSystemHeader())`).


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

https://reviews.llvm.org/D66564



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

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

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

Szelethus added a reviewer: Charusso.
Szelethus added a comment.

In D75271#1895900 , @Charusso wrote:

> I am so sorry to mention, but we need the `AnalysisManager` to pass around 
> which manages the analysis, therefore it knows both the `LangOptions` and 
> `AnalyzerOptions`.




In D75271#1895915 , @Charusso wrote:

> PS: The `CheckerManager` also could serve this behavior as `registerXXX()` 
> already passing around that manager, but I believe the `AnalysisManager` 
> supposed to manage the analysis.


Well, I like to say that "Any manager can be retrieved in clang at arbitrary 
places if you try hard enough", so I think either that //actually// satisfies 
this would be fine :)

> Also this entire callback should be removed ideally: it has to be a virtual 
> function defaulting to `return true;` and if someone needs this feature could 
> rewrite the behavior. I guess there was some debate whether it should be on 
> by default or not, but for a checker writer and future changes this patch 
> shows that how weak this API is.

Yup, that is a very good criticism of the the API. I would prefer to see 
something a lot less ugly, and I strongly share you sentiment that making 
changes to it is about as ugly as the initial patch itself. Virtual functions 
would be okay, the reason why I didn't do that is because where do you put 
them? Checker objects can't house them, because the point of the entire 
`shouldRegister` function is to never create them in the first place. But 
thinking about it again, the problem isn't really their **creation** as much as 
their **registration**. If we were to create a checker object and ask it 
whether its fine to do analysis, and //then// store it, we would be able to get 
rid of the `shouldRegister` function. I can see from a mile away that due to 
the library layout, cyclic linking dependencies, and whatever else would make 
this seemingly trivial task a lot more invasive and difficult, and I'm just not 
too sure whether its worth the effort to change an arguably bearable 
inconvenience.

In D75271#1896098 , @Charusso wrote:

> In D75271#1895949 , 
> @baloghadamsoftware wrote:
>
> > It is impossible to use `CheckerManager` as parameter for 
> > `shouldRegisterXXX()` because `CheckerRegistry` does not have it. If I add 
> > `CheckerManager` to `CheckerRegistry` then the `printXXX()` functions will 
> > not work because they do not have `CheckerManager` at all. This patch does 
> > not help in printing error message, see D75171 
> > . I need a way to solve 44998 
> >  by rejecting the checker if 
> > the option is disabled **and** printing an error message.
>
>
> Aha, I see as of D75171 . Well, @Szelethus 
> felt the same to pass around the `CheckerManager`. Let us see:
>
>   std::unique_ptr ento::createCheckerManager(
>   ASTContext , 
>   AnalyzerOptions ,
>   ArrayRef plugins,
>   ArrayRef> checkerRegistrationFns,
>   DiagnosticsEngine ) { 
>   
> auto checkerMgr = std::make_unique(context, opts);
>
> CheckerRegistry allCheckers(plugins, diags, opts, context.getLangOpts(),
> checkerRegistrationFns);  
>  
> allCheckers.initializeManager(*checkerMgr);
> allCheckers.validateCheckerOptions();
> checkerMgr->finishedCheckerRegistration();
>
> return checkerMgr;
>   }
>
>
> - from `StaticAnalyzer/Frontend/CheckerRegistration.cpp`.
>
>   Are you sure we cannot pass around the manager like 
> `allCheckers.initializeManager(*checkerMgr);`? I am also thinking of a 
> glue-layer between the two managers, like `CheckerRegistryManager` or 
> something, which holds all your needs, like `AnalyzerOptions` and 
> `LangOptions` for now.


`CheckerManager` has complete control of `CheckerRegistry`. You can for sure 
cascade it down as a parameter. The worry is, again, how `CheckerManager` is in 
`libStaticAnalysisCore` and `CheckerRegistry` is in 
`libStaticAnalysisFrontend`, but given that we wouldn't need to use it one but 
more then how we use it for registry functions, this should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271



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

[PATCH] D75057: Syndicate, test and fix base64 implementation

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 3 inline comments as done.
serge-sans-paille added inline comments.



Comment at: llvm/unittests/Support/Base64Test.cpp:30
+  // from: https://tools.ietf.org/html/rfc4648#section-10
+  TestBase64("", "");
+  TestBase64("f", "Zg==");

hokein wrote:
> nit: i would just inline the `TestBase64`.
I wouldn't :-) I find it easier to read like that, if that's okay with you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75057



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

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Next step is to plumb this to clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69782



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

[PATCH] D75057: Syndicate, test and fix base64 implementation

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 247039.
serge-sans-paille added a comment.

Add a test case that exhibits the overflow in previous implementation + minor 
nits from @hokein


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75057

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  compiler-rt/lib/fuzzer/FuzzerUtil.cpp
  llvm/include/llvm/Support/Base64.h
  llvm/unittests/Support/Base64Test.cpp
  llvm/unittests/Support/CMakeLists.txt

Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -9,6 +9,7 @@
   AnnotationsTest.cpp
   ARMAttributeParser.cpp
   ArrayRecyclerTest.cpp
+  Base64Test.cpp
   BinaryStreamTest.cpp
   BlockFrequencyTest.cpp
   BranchProbabilityTest.cpp
Index: llvm/unittests/Support/Base64Test.cpp
===
--- /dev/null
+++ llvm/unittests/Support/Base64Test.cpp
@@ -0,0 +1,53 @@
+//===- llvm/unittest/Support/Base64Test.cpp - Base64 tests
+//===//
+//
+// 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
+//
+//===--===//
+//
+// This file implements unit tests for the Base64 functions.
+//
+//===--===//
+
+#include "llvm/Support/Base64.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+/// Tests an arbitrary set of bytes passed as \p Input.
+void TestBase64(StringRef Input, StringRef Final) {
+  auto Res = encodeBase64(Input);
+  EXPECT_EQ(Res, Final);
+}
+
+} // namespace
+
+TEST(Base64Test, Base64) {
+  // from: https://tools.ietf.org/html/rfc4648#section-10
+  TestBase64("", "");
+  TestBase64("f", "Zg==");
+  TestBase64("fo", "Zm8=");
+  TestBase64("foo", "Zm9v");
+  TestBase64("foob", "Zm9vYg==");
+  TestBase64("fooba", "Zm9vYmE=");
+  TestBase64("foobar", "Zm9vYmFy");
+
+  // With non-printable values.
+  char NonPrintableVector[] = {0x00, 0x00, 0x00,   0x46,
+   0x00, 0x08, (char)0xff, (char)0xee};
+  TestBase64(StringRef(NonPrintableVector, sizeof(NonPrintableVector)),
+ "RgAI/+4=");
+
+  // Large test case
+  char LargeVector[] = {0x54, 0x68, 0x65, 0x20, 0x71, 0x75, 0x69, 0x63, 0x6b,
+0x20, 0x62, 0x72, 0x6f, 0x77, 0x6e, 0x20, 0x66, 0x6f,
+0x78, 0x20, 0x6a, 0x75, 0x6d, 0x70, 0x73, 0x20, 0x6f,
+0x76, 0x65, 0x72, 0x20, 0x31, 0x33, 0x20, 0x6c, 0x61,
+0x7a, 0x79, 0x20, 0x64, 0x6f, 0x67, 0x73, 0x2e};
+  TestBase64(LargeVector,
+ "VGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIDEzIGxhenkgZG9ncy4=");
+}
Index: llvm/include/llvm/Support/Base64.h
===
--- /dev/null
+++ llvm/include/llvm/Support/Base64.h
@@ -0,0 +1,53 @@
+//===--- Base64.h - Base64 Encoder/Decoder --*- 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
+//
+//===--===//
+//
+// This file provides generic base64 encoder/decoder.
+//
+//===--===//
+
+#ifndef LLVM_SUPPORT_BASE64_H
+#define LLVM_SUPPORT_BASE64_H
+
+#include 
+
+namespace llvm {
+
+template  std::string encodeBase64(InputBytes const ) {
+  static const char Table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+  "abcdefghijklmnopqrstuvwxyz"
+  "0123456789+/";
+  std::string Buffer;
+  Buffer.resize(((Bytes.size() + 2) / 3) * 4);
+
+  size_t i = 0, j = 0;
+  for (size_t n = Bytes.size() / 3 * 3; i < n; i += 3, j += 4) {
+uint32_t x = (Bytes[i] << 16) | (Bytes[i + 1] << 8) | Bytes[i + 2];
+Buffer[j + 0] = Table[(x >> 18) & 63];
+Buffer[j + 1] = Table[(x >> 12) & 63];
+Buffer[j + 2] = Table[(x >> 6) & 63];
+Buffer[j + 3] = Table[x & 63];
+  }
+  if (i + 1 == Bytes.size()) {
+uint32_t x = (Bytes[i] << 16);
+Buffer[j + 0] = Table[(x >> 18) & 63];
+Buffer[j + 1] = Table[(x >> 12) & 63];
+Buffer[j + 2] = '=';
+Buffer[j + 3] = '=';
+  } else if (i + 2 == Bytes.size()) {
+uint32_t x = (Bytes[i] << 16) | (Bytes[i + 1] << 8);
+Buffer[j + 0] = Table[(x >> 18) & 63];
+Buffer[j + 1] = Table[(x >> 12) & 63];
+Buffer[j + 2] = Table[(x >> 6) & 63];
+Buffer[j + 3] = '=';
+  }
+  return Buffer;
+}

Re: r355698 - Re-fix _lrotl/_lrotr to always take Long, no matter the platform.

[Michael Spencer via cfe-commits]

On Wed, Feb 26, 2020 at 8:39 PM James Y Knight  wrote:

> I'm not clear as to what you're saying is broken.
>
> On MS platforms, long is 32-bit, so either way this function takes a 32bit
> value, right? And, Microsoft's docs say this function takes a long.
>

As per the original commit that changed this:

Support MS builtins using 'long' on LP64 platforms
>


This allows for -fms-extensions to work the same on LP64. For example,
> _BitScanReverse is expected to be 32-bit, matching Windows/LLP64, even
> though long is 64-bit on x86_64 Darwin or Linux (LP64).
>


Implement this by adding a new character code 'N', which is 'int' if
> the target is LP64 and the same 'L' otherwise
>


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


The point of this change was to aid porting Windows code to LP64 targets
when using -fms-extensions.

In the future I would ask that before changing something that has an
explicit, documented test like this that you ping the original author for
review.

- Michael Spencer


>
>
> On Wed, Feb 26, 2020, 5:09 PM Michael Spencer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I believe this commit is wrong. These intrinsics were originally
>> implemented to support Windows code which expects _lrot{l,r}'s first
>> argument to be 32 bits, it's a breaking change to change these definitions.
>> I'm fine with adding the Intel version of these intrinsics, but you can't
>> just break the Microsoft version of them.
>>
>> - Michael Spencer
>>
>> On Fri, Mar 8, 2019 at 7:09 AM Erich Keane via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: erichkeane
>>> Date: Fri Mar  8 07:10:07 2019
>>> New Revision: 355698
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=355698=rev
>>> Log:
>>> Re-fix _lrotl/_lrotr to always take Long, no matter the platform.
>>>
>>> r355322 fixed this, however is being reverted due to concerns with
>>> enabling it in other modes.
>>>
>>> Change-Id: I6a939b7469b8fa196d5871a627eb2330dbd30f29
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Basic/Builtins.def
>>> cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>>>
>>> Modified: cfe/trunk/include/clang/Basic/Builtins.def
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=355698=355697=355698=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/Basic/Builtins.def (original)
>>> +++ cfe/trunk/include/clang/Basic/Builtins.def Fri Mar  8 07:10:07 2019
>>> @@ -831,12 +831,12 @@ LANGBUILTIN(_ReturnAddress, "v*", "n", A
>>>  LANGBUILTIN(_rotl8,  "UcUcUc","n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotl16, "UsUsUc","n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotl,   "UiUii", "n", ALL_MS_LANGUAGES)
>>> -LANGBUILTIN(_lrotl,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
>>> +LANGBUILTIN(_lrotl,  "ULiULii",   "n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotl64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotr8,  "UcUcUc","n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotr16, "UsUsUc","n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotr,   "UiUii", "n", ALL_MS_LANGUAGES)
>>> -LANGBUILTIN(_lrotr,  "UNiUNii",   "n", ALL_MS_LANGUAGES)
>>> +LANGBUILTIN(_lrotr,  "ULiULii",   "n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(_rotr64, "UWiUWii",   "n", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(__va_start,   "vc**.", "nt", ALL_MS_LANGUAGES)
>>>  LANGBUILTIN(__fastfail, "vUi","nr", ALL_MS_LANGUAGES)
>>>
>>> Modified: cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c?rev=355698=355697=355698=diff
>>>
>>> ==
>>> --- cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c (original)
>>> +++ cfe/trunk/test/CodeGen/ms-intrinsics-rotations.c Fri Mar  8 07:10:07
>>> 2019
>>> @@ -12,17 +12,10 @@
>>>  // RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>>>  // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility
>>> -fms-compatibility-version=17.00 \
>>>  // RUN: -triple x86_64--linux -emit-llvm %s -o - \
>>> -// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>>> +// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
>>>  // RUN: %clang_cc1 -ffreestanding -fms-extensions \
>>>  // RUN: -triple x86_64--darwin -emit-llvm %s -o - \
>>> -// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
>>> -
>>> -// LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
>>> -#ifdef __LP64__
>>> -#define LONG int
>>> -#else
>>> -#define LONG long
>>> -#endif
>>> +// RUN: | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
>>>
>>>  // rotate left
>>>
>>> @@ -47,12 +40,15 @@ unsigned int test_rotl(unsigned int valu
>>>  // CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32
>>> [[X]], i32 [[Y:%.*]])
>>>  // CHECK:   ret i32 

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTContext.cpp:4020
+
+  auto *New = new (*this, TypeAlignment) ExtIntType(IsUnsigned, NumBits);
+  ExtIntTypes.InsertNode(New, InsertPos);

keryell wrote:
> Why no just `auto` without a `*` everywhere?
The LLVM coding standard requires it.


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

https://reviews.llvm.org/D73967



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

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

[Ronan Keryell via Phabricator via cfe-commits]

keryell added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:4020
+
+  auto *New = new (*this, TypeAlignment) ExtIntType(IsUnsigned, NumBits);
+  ExtIntTypes.InsertNode(New, InsertPos);

Why no just `auto` without a `*` everywhere?


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

https://reviews.llvm.org/D73967



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