[PATCH] D76119: [Coroutines] Insert lifetime intrinsics even O0 is used

[JunMa 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 rGd0f4af8f3088: [Coroutines] Insert lifetime intrinsics even 
O0 is used (authored by junparser).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76119

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp


Index: clang/test/CodeGenCoroutines/coro-always-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-always-inline.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) {}
+  bool await_resume() { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
+void foo() { co_return; }
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -579,8 +579,9 @@
   // At O0 and O1 we only run the always inliner which is more efficient. At
   // higher optimization levels we run the normal inliner.
   if (CodeGenOpts.OptimizationLevel <= 1) {
-bool InsertLifetimeIntrinsics = (CodeGenOpts.OptimizationLevel != 0 &&
- !CodeGenOpts.DisableLifetimeMarkers);
+bool InsertLifetimeIntrinsics = ((CodeGenOpts.OptimizationLevel != 0 &&
+  !CodeGenOpts.DisableLifetimeMarkers) ||
+ LangOpts.Coroutines);
 PMBuilder.Inliner = 
createAlwaysInlinerLegacyPass(InsertLifetimeIntrinsics);
   } else {
 // We do not want to inline hot callsites for SamplePGO module-summary 
build
@@ -1176,7 +1177,10 @@
   // which is just that always inlining occurs. Further, disable generating
   // lifetime intrinsics to avoid enabling further optimizations during
   // code generation.
-  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));
+  // However, we need to insert lifetime intrinsics to avoid invalid access
+  // caused by multithreaded coroutines.
+  MPM.addPass(
+  AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/LangOpts.Coroutines));
 
   // At -O0, we can still do PGO. Add all the requested passes for
   // instrumentation PGO, if requested.


Index: clang/test/CodeGenCoroutines/coro-always-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-always-inline.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts \
+// RUN:   -fexperimental-new-pass-manager -O0 %s 

[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

Ping.


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

https://reviews.llvm.org/D73307



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

[clang] d0f4af8 - [Coroutines] Insert lifetime intrinsics even O0 is used

[Jun Ma via cfe-commits]

Author: Jun Ma
Date: 2020-03-24T13:41:55+08:00
New Revision: d0f4af8f3088f72df7fea9983127cbeeebbef6a1

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

LOG: [Coroutines] Insert lifetime intrinsics even O0 is used

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

Added: 
clang/test/CodeGenCoroutines/coro-always-inline.cpp

Modified: 
clang/lib/CodeGen/BackendUtil.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index e8f2524a25d5..97b97276521d 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -579,8 +579,9 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
,
   // At O0 and O1 we only run the always inliner which is more efficient. At
   // higher optimization levels we run the normal inliner.
   if (CodeGenOpts.OptimizationLevel <= 1) {
-bool InsertLifetimeIntrinsics = (CodeGenOpts.OptimizationLevel != 0 &&
- !CodeGenOpts.DisableLifetimeMarkers);
+bool InsertLifetimeIntrinsics = ((CodeGenOpts.OptimizationLevel != 0 &&
+  !CodeGenOpts.DisableLifetimeMarkers) ||
+ LangOpts.Coroutines);
 PMBuilder.Inliner = 
createAlwaysInlinerLegacyPass(InsertLifetimeIntrinsics);
   } else {
 // We do not want to inline hot callsites for SamplePGO module-summary 
build
@@ -1176,7 +1177,10 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   // which is just that always inlining occurs. Further, disable generating
   // lifetime intrinsics to avoid enabling further optimizations during
   // code generation.
-  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));
+  // However, we need to insert lifetime intrinsics to avoid invalid access
+  // caused by multithreaded coroutines.
+  MPM.addPass(
+  AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/LangOpts.Coroutines));
 
   // At -O0, we can still do PGO. Add all the requested passes for
   // instrumentation PGO, if requested.

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
new file mode 100644
index ..a2e4bba45c0c
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) {}
+  bool await_resume() { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
+void foo() { co_return; }



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20d704a75ed5: [objc_direct] also go through implementations 
when looking for clashes (authored by MadCoder).

Changed prior to commit:
  https://reviews.llvm.org/D76643?vs=252162=252214#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -12,6 +12,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (void)unavailableInChild;
 - (void)rootRegular;  // expected-note {{previous declaration is here}}
 + (void)classRootRegular; // expected-note {{previous declaration is here}}
 - (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
@@ -52,6 +53,7 @@
 __attribute__((objc_direct_members))
 @interface SubDirectMembers : Root
 @property int foo; // expected-note {{previous declaration is here}}
+- (void)unavailableInChild __attribute__((unavailable)); // should not warn
 - (instancetype)init;
 @end
 
@@ -81,6 +83,8 @@
 
 __attribute__((objc_direct_members))
 @implementation Root
+- (void)unavailableInChild {
+}
 - (void)rootRegular {
 }
 + (void)classRootRegular {
Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,62 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema , Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && !Method->hasAttr() &&
+  CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema , ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking it up in the @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = 

[clang] 20d704a - [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via cfe-commits]

Author: Pierre Habouzit
Date: 2020-03-23T20:49:09-07:00
New Revision: 20d704a75ed51c7a9a155aa3978d0c02671c3f69

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

LOG: [objc_direct] also go through implementations when looking for clashes

Some methods are sometimes declared in the @implementation blocks which
can cause undiagnosed clashes.

Just write a checkObjCDirectMethodClashes() for this purpose.

Also make sure that "unavailable" selectors do not inherit
objc_direct_members.

Differential Revision: https://reviews.llvm.org/D76643
Signed-off-by: Pierre Habouzit 
Radar-ID: rdar://problem/59332804, rdar://problem/59782963

Added: 


Modified: 
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/SemaObjC/method-direct-one-definition.m
clang/test/SemaObjC/method-direct.m

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 369b04ed8e25..934e1a23141c 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,62 @@ static void checkObjCMethodX86VectorTypes(Sema ,
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema , Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && !Method->hasAttr() &&
+  CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema , ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << 
IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking it up in the @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = Cat->getImplementation())
+  if (CatImpl != ImpDecl)
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+}
+
 Decl *Sema::ActOnMethodDeclaration(
 Scope *S, SourceLocation MethodLoc, SourceLocation EndLoc,
 tok::TokenKind MethodType, ObjCDeclSpec , ParsedType ReturnType,
@@ -4809,9 +4865,9 @@ Decl *Sema::ActOnMethodDeclaration(
   Diag(ObjCMethod->getLocation(), diag::warn_dealloc_in_category)
 << ObjCMethod->getDeclName();
 }
-  } else if (ImpDecl->hasAttr()) {
-ObjCMethod->addAttr(
-ObjCDirectAttr::CreateImplicit(Context, 
ObjCMethod->getLocation()));
+  } else {
+mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod);
+checkObjCDirectMethodClashes(*this, IDecl, ObjCMethod, ImpDecl);
   }
 
   // Warn if a method declared in a protocol to which a category or
@@ -4832,39 +4888,16 @@ Decl *Sema::ActOnMethodDeclaration(
 }
   } else {
 if (!isa(ClassDecl)) {
-  if (!ObjCMethod->isDirectMethod() &&
-  ClassDecl->hasAttr()) {
-ObjCMethod->addAttr(
-ObjCDirectAttr::CreateImplicit(Context, 
ObjCMethod->getLocation()));
-  }
+  mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod);
 
-  // There can be a single declaration in any @interface container
-  // for a given direct method, look for clashes as we add them.
-  //
-  // For valid code, we should always know the primary interface
-  // declaration by now, however for invalid code we'll keep parsing
-  

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D76229#1929453 , @martong wrote:

> Please note about the alignment requirements that they are not checked in the 
> static analyzer for a reason. To track the alignment data across the data 
> flow, probably we should modify the analyzer's core structure (e.g. 
> `MemRegion`) and some modeling checkers like the `MallocChecker`. That is not 
> an impossible task but also far from easy IMHO (would require several patches 
> built on top of each other). Just as an example for the subtleties: think 
> about the nuances of `operator new`s default alignment and specific C++17 
> rules (https://reviews.llvm.org/D67545).


This checker's tests only cover `VarRegion`-based regions. It should be easy to 
pick up the alignment from the AST for the `VarDecl` it corresponds to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76510: [analyzer] Change the default output type to PD_TEXT_MINIMAL, error if an output loc is missing for PathDiagConsumers that need it

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:209
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
-  AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
+  AnalysisDiagClients AnalysisDiagOpt = PD_TEXT_MINIMAL;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;

This patch most likely changes almost nothing because the default output mode 
of `clang --analyze` has always been Plist:

```lang=c++
lib/Driver/ToolChains/Clang.cpp
2938:  // Set the output format. The default is plist, for (lame) historical 
reasons.
2939-  CmdArgs.push_back("-analyzer-output");
2940-  if (Arg *A = Args.getLastArg(options::OPT__analyzer_output))
2941-CmdArgs.push_back(A->getValue());
2942-  else
2943-CmdArgs.push_back("plist");
```
I'll double check on historical reasons; i suspect we should be able to 
transition out of it eventually because this is indeed the worst possible 
output mode for `--analyze`.

That said, i wonder how/why do our tests currently work without flooding 
everything with html reports.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76510



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

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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

Fix build errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1599,6 +1599,8 @@
   const HeaderFileInfo 
   ArrayRef KnownHeaders;
   UnresolvedModule Unresolved;
+  uint32_t UID;
+  uint32_t FileIndex;
 };
 using data_type_ref = const data_type &;
 
@@ -1676,6 +1678,10 @@
   }
   LE.write(Offset);
 
+  // Write this file UID and its index into the array where it was written.
+  LE.write(Data.UID);
+  LE.write(Data.FileIndex);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1800,7 +1806,8 @@
   Filename, File->getSize(), getTimestampForOutput(File)
 };
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {},
+  File->getUID(), UID,
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,18 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty() ) {
+RecordData Record;
+for (auto FileUID : it->second.IncludedFiles) {
+  Record.push_back(FileUID);
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,13 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  {
+HFI.UID = endian::readNext(d);
+//TODO: seems the index is not needed
+const uint32_t OldIndex = endian::readNext(d);
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -9271,6 +9278,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // Fix up the HeaderSearchInfo UIDs.
+  if (!PendingImportedHeaders.empty()){
+std::map OldToNewUID;
+
+// These HFIs were deserialized and assigned their "old"
+// UID.
+// We need to update them and populate the OldToNewUID map
+// for use next.
+HeaderSearch& HS = PP.getHeaderSearchInfo();
+for (unsigned NewUID = 0; NewUID < HS.FileInfo.size(); ++NewUID) {
+  OldToNewUID[HS.FileInfo[NewUID].UID] = NewUID;
+  HS.FileInfo[NewUID].UID = NewUID;
+}
+
+const auto& Iter = PendingImportedHeaders.begin();
+for (unsigned I = 0; I < PendingImportedHeaders.size(); ++I){
+  ModuleFile* ModFile = Iter[I].first;
+  auto& HeaderUIDs = Iter[I].second;
+  Module *M = HS.lookupModule(ModFile->ModuleName);
+
+  Preprocessor::SubmoduleState&  SubState = PP.Submodules[M];
+  for (unsigned OldUid : HeaderUIDs) {
+auto NewUIDIt = OldToNewUID.find(OldUid);
+assert(NewUIDIt != OldToNewUID.end());
+
+SubState.IncludedFiles.insert(NewUIDIt->second);
+  }
+}
+PendingImportedHeaders.clear();
+  }
 }
 
 void ASTReader::diagnoseOdrViolations() {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,21 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-// After module map parsing, 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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

Clang format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1599,6 +1599,8 @@
   const HeaderFileInfo 
   ArrayRef KnownHeaders;
   UnresolvedModule Unresolved;
+  uint32_t UID;
+  uint32_t FileIndex;
 };
 using data_type_ref = const data_type &;
 
@@ -1676,6 +1678,10 @@
   }
   LE.write(Offset);
 
+  // Write this file UID and its index into the array where it was written.
+  LE.write(Data.UID);
+  LE.write(Data.FileIndex);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1800,7 +1806,9 @@
   Filename, File->getSize(), getTimestampForOutput(File)
 };
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI, HS.getModuleMap().findAllModulesForHeader(File),
+{},   File->getUID(),
+UID,
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2642,18 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (auto FileUID : it->second.IncludedFiles) {
+  Record.push_back(FileUID);
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,13 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  {
+HFI.UID = endian::readNext(d);
+// TODO: seems the index is not needed
+const uint32_t OldIndex = endian::readNext(d);
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -9271,6 +9278,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // Fix up the HeaderSearchInfo UIDs.
+  if (!PendingImportedHeaders.empty()) {
+std::map OldToNewUID;
+
+// These HFIs were deserialized and assigned their "old"
+// UID.
+// We need to update them and populate the OldToNewUID map
+// for use next.
+HeaderSearch  = PP.getHeaderSearchInfo();
+for (unsigned NewUID = 0; NewUID < HS.FileInfo.size(); ++NewUID) {
+  OldToNewUID[HS.FileInfo[NewUID].UID] = NewUID;
+  HS.FileInfo[NewUID].UID = NewUID;
+}
+
+const auto  = PendingImportedHeaders.begin();
+for (unsigned I = 0; I < PendingImportedHeaders.size(); ++I) {
+  ModuleFile *ModFile = Iter[I].first;
+  auto  = Iter[I].second;
+  Module *M = HS.lookupModule(ModFile->ModuleName);
+
+  Preprocessor::SubmoduleState  = PP.Submodules[M];
+  for (unsigned OldUid : HeaderUIDs) {
+auto NewUIDIt = OldToNewUID.find(OldUid);
+assert(NewUIDIt != OldToNewUID.end());
+
+SubState.IncludedFiles.insert(NewUIDIt->second);
+  }
+}
+PendingImportedHeaders.clear();
+  }
 }
 
 void ASTReader::diagnoseOdrViolations() {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,22 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-// After module map parsing, this expands to:

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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

(hopefully) Final revision ... running out of idea for edit comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1599,6 +1599,8 @@
   const HeaderFileInfo 
   ArrayRef KnownHeaders;
   UnresolvedModule Unresolved;
+  uint32_t UID;
+  uint32_t FileIndex;
 };
 using data_type_ref = const data_type &;
 
@@ -1676,6 +1678,10 @@
   }
   LE.write(Offset);
 
+  // Write this file UID and its index into the array where it was written.
+  LE.write(Data.UID);
+  LE.write(Data.FileIndex);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1800,7 +1806,8 @@
   Filename, File->getSize(), getTimestampForOutput(File)
 };
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {},
+  File->getUID(), UID,
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,18 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty() ) {
+RecordData Record;
+for (auto FileUID : it->second.IncludedFiles) {
+  Record.push_back(FileUID);
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,13 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  {
+HFI.UID = endian::readNext(d);
+//TODO: seems the index is not needed
+const uint32_t OldIndex = endian::readNext(d);
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -9271,6 +9278,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // Fix up the HeaderSearchInfo UIDs.
+  if (!PendingImportedHeaders.empty()){
+std::map OldToNewUID;
+
+// These HFIs were deserialized and assigned their "old"
+// UID.
+// We need to update them and populate the OldToNewUID map
+// for use next.
+HeaderSearch& HS = PP->getHeaderSearchInfo();
+for (unsigned NewUID = 0; NewUID < HS.FileInfo.size(); ++NewUID) {
+  OldToNewUID[HS.FileInfo[NewUID].UID] = NewUID;
+  HS.FileInfo[NewUID].UID = NewUID;
+}
+
+auto& Iter = PendingImportedHeaders.begin();
+for (unsigned I = 0; I < PendingImportedHeaders.size(); ++I){
+  ModuleFile* ModFile = Iter[I].first;
+  auto& HeaderUIDs = Iter[I].second;
+  Module *M = HS.lookupModule(ModFile->ModuleName);
+
+  SubmoduleState& SubState = PP.Submodulels[M];
+  for (unsigned OldUid : HeaderUIDs) {
+auto NewUIDIt = OldToNewUID.find(OldUid);
+assert(NewUIDIt != OldToNewUID.end());
+
+SubState.IncludedFiles.insert(NewUIDIt->second);
+  }
+}
+PendingImportedHeaders.clear();
+  }
 }
 
 void ASTReader::diagnoseOdrViolations() {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,21 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-   

[PATCH] D70411: [analyzer] CERT: STR31-C

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 5 inline comments as done.
Charusso added a comment.

Thanks for the feedback! Given that it will remain an `alpha` checker for a 
long time (~1 year), no one really should use it.




Comment at: clang/docs/analyzer/checkers.rst:1935
+
+alpha.security.cert.str.31c
+"""

Szelethus wrote:
> balazske wrote:
> > There are already more checkers that can check for CERT related problems 
> > but not specially made for these. These checkers do not reside in this new 
> > `cert` group. And generally a checker does not check for specifically a 
> > CERT rule, instead for more of them or other things too, or more checkers 
> > can detect a single rule. (And the user can think that only these CERT 
> > rules are checkable that exist in this package, that is not true.) So I do 
> > not like the introduction of this new `cert` package. (The documentation of 
> > existing checkers lists if the checker is designed for a CERT rule.)
> I disagree to some extent. I think it would be great to have a `cert` package 
> that houses all checkers for each of the rules with the addition of checker 
> aliases. Clang-tidy has something similar as well!
I designed the checker only for the rule STR31-C that is why I have picked 
package `cert`. Clang-Tidy could aliasing checks. For example the check could 
be in the `bugprone` category aliased to `cert` and both could trigger the 
analysis.

> the user can think that only these CERT rules are checkable
We only need to move `security.FloatLoopCounter` under the `cert` package. What 
else SEI CERT rules are finished off other than package `cert`? It is not my 
fault if someone could not package well or could not catch most of the issues 
of a SEI CERT rule or could not reach the SEI CERT group to note the fact the 
Analyzer catch a very tiny part of a rule. However, this patch package well and 
could catch most of the STR31-C rule.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h:22
+extern const char *const CXXObjectLifecycle;
+extern const char *const SecurityError;
+} // namespace categories

balazske wrote:
> Are there already not other checkers that find security related bugs (the 
> taint checker?)? Why do these not use a `SecurityError`? It is not bad to 
> have a `SecurityError` but maybe there is a reason why was it not there 
> already. If these categories are exclusive it is hard to find out what 
> problem (probably already existing bug type in other checkers) belongs to 
> what category (it can be for this checker `UnixAPI` or `MemoryError` too?). 
We have only three pure security checkers: `security.insecureAPI`, 
`security.FloatLoopCounter` (FLP30-C, FLP30-CPP) and 
`alpha.security.cert.pos.34c`. The non-alpha checkers are very old, but Ted 
definitely made that category:
```
  const char *bugType = "Floating point variable used as loop counter";
  BR.EmitBasicReport(bugType, "Security", os.str().c_str(),
 FS->getLocStart(), ranges.data(), ranges.size());
```
- 
https://github.com/llvm/llvm-project/commit/9c49762776b4d2cb16911dec0c873c90a6508968

Every other security-ish checker does not catch the CERT rule's examples. May 
if the checker evolves I would pick MemoryError, but it will not evolve soon.


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

https://reviews.llvm.org/D70411



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: ASDenysPetrov.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto  = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> NoQ wrote:
> > I'm opposed to this code for the same reason that i'm opposed to it in the 
> > debug checker. Parent region is an undocumented implementation detail of 
> > `RegionStore`. It is supposed to be immaterial to the user. You should not 
> > rely on its exact value.
> > 
> > @baloghadamsoftware Can we eliminate all such code from iterator checkers 
> > and instead identify all iterators by regions in which they're stored? Does 
> > my improved C++ support help with this anyhow whenever it kicks in?
> How to find the region where it is stored? I am open to find better 
> solutions, but it was the only one I could find so far. If we ignore 
> `LazyCompoundVal` then everything falls apart, we can remove all the 
> iterator-related checkers.
It's the region from which you loaded it. If you obtained it as 
`Call.getArgSVal()` then it's the parameter region for the call. If you 
obtained it as `Call.getReturnValue()` then it's the target region can be 
obtained by looking at the //construction context// for the call.


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

https://reviews.llvm.org/D75677



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

NoQ wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> > > > non-anonymous `VarRegion` or a `FieldRegion` or something like that (in 
> > > > other patches as well). It would work more often and it'll 
> > > > transparently handle references.
> > > Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
> > > `getMostDerivedRegion()` does not help.)
> > You mean the first checking form `SymbolicRegion`, then get its symbol, 
> > check for `SymbolRegionValue`, then get its `TypedValueRegion`, check for 
> > `DeclRegion` and use its `Decl`? This sound waaay more complicated and less 
> > readable. I am not sure which are the side cases: is it always 
> > `SymbolicRegion`? Is the `Symbol` of `SymbolicRegion` always a 
> > `SymbolRegionValue`? Is ithe `TypedValueRegion` (the return value of its 
> > `getRegion()`) always a `DeclRegion`?
> > Unfortunately it is a `SymRegion`
> 
> Emm, that's rarely the case. Only if it's a reference passed into a top-level 
> function as a parameter.
(or to another unknown location) (please learn what `SymbolicRegion` is, it is 
very important for your work)

I guess you should do both then, because when the analyzer is able to resolve 
the reference it's better in this case to point out what is the actual 
container that's being modified.


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

https://reviews.llvm.org/D73720



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

[PATCH] D74615: [Analyzer] Add visitor to track iterator invalidation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: ASDenysPetrov.

> However, if //something// happens to the value in a transition which is done 
> by another checker (or the core engine) then

... then //that// checker (or the engine) adds a tag in order to explain 
itself, because from its point of view it

> //does// something with a value when adding a new transition

.

-

> This may result in double notes like `All iterators of 'V' after 'i1' are 
> invalidated.` and then the next is `Iterator 'i0' is invalidated`.

Not if interestingness checks are performed correctly. Before emitting such 
note, the note tag should ask the bug report whether it's interested in all 
iterators being invalidated, or in how a particular iterator is invalidated. If 
`PathSensitiveBugReport` is not flexible enough to answer such questions, we 
should improve it.

-

> We already have such double notes: Assuming the condition is 'true'. then 
> Taking 'true' branch..

These are not double notes. The former describes the arbitrary state split, 
thus indicating that there is no concrete knowledge about the branch condition 
up to this point, while the latter describes the control flow in the program. 
Also the "Taking..." notes are displayed as arrows rather than as bubbles by 
GUIs that read plists, so basically everything except scan-build.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74615



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

[PATCH] D74541: [Analyzer] Use note tags to track iterator increments and decrements

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: ASDenysPetrov.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:489-490
+StringRef ChangeText =
+  ((Op == OO_Plus || Op == OO_PlusEqual) != (ChangeVal <= 0)) ?
+  "incremented" : "decremented";
+const NoteTag *ChangeTag = getChangeTag(C, ChangeText, ItE,

baloghadamsoftware wrote:
> NoQ wrote:
> > NoQ wrote:
> > > Can we assert out the `ChangeVal == 0` case or make a better message for 
> > > it (`"unchanged"` or something like that)?
> > Another important thing to do here is to track the RHS value via 
> > `trackExpressionValue()`. I.e., why do we think that iterator is 
> > incremented by 1 and not by 2?
> I agree, that is important, but where should I call it? This is the modeling 
> checker, which models the increments and the decrements, but 
> `trackExpressionValue()` can only be invoked for bug reports. Should I put it 
> into the `NoteTag` lambda?
Yup, it should be possible to invoke `trackExpressionValue()`, and generally 
attach more visitors, inside a `NoteTag`.


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

https://reviews.llvm.org/D74541



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: ASDenysPetrov.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> > > non-anonymous `VarRegion` or a `FieldRegion` or something like that (in 
> > > other patches as well). It would work more often and it'll transparently 
> > > handle references.
> > Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
> > `getMostDerivedRegion()` does not help.)
> You mean the first checking form `SymbolicRegion`, then get its symbol, check 
> for `SymbolRegionValue`, then get its `TypedValueRegion`, check for 
> `DeclRegion` and use its `Decl`? This sound waaay more complicated and less 
> readable. I am not sure which are the side cases: is it always 
> `SymbolicRegion`? Is the `Symbol` of `SymbolicRegion` always a 
> `SymbolRegionValue`? Is ithe `TypedValueRegion` (the return value of its 
> `getRegion()`) always a `DeclRegion`?
> Unfortunately it is a `SymRegion`

Emm, that's rarely the case. Only if it's a reference passed into a top-level 
function as a parameter.


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

https://reviews.llvm.org/D73720



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

[PATCH] D76592: [Parser] Fix the assertion crash in ActOnStartOfSwitch stmt.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:709
   } SwitchDiagnoser(Cond);
+  // The TypoExpr might be corrected to a non-intergral-or-enum type in the
+  // later stage without the proper type check, which is invalid for switch

How do we know Cond is a TypoExpr directly rather than containing one?

I think the usual strategy when code can't deal with typo correction being 
delayed further is to call CorrectDelayedTyposInExpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76592



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

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

[JF Bastien via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76528



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

[PATCH] D76663: [clangd] Support new semanticTokens request from LSP 3.16.

[Sam McCall via Phabricator via cfe-commits]

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

This is a simpler request/response protocol.

Reference: 
https://github.com/microsoft/vscode-languageserver-node/blob/master/protocol/src/protocol.semanticTokens.proposed.ts

No attempt to support incremental formatting (yet).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76663

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -720,6 +720,41 @@
   ASSERT_EQ(Counter.Count, 1);
 }
 
+TEST(SemanticHighlighting, toSemanticTokens) {
+  auto CreatePosition = [](int Line, int Character) -> Position {
+Position Pos;
+Pos.line = Line;
+Pos.character = Character;
+return Pos;
+  };
+
+  std::vector Tokens = {
+  {HighlightingKind::Variable,
+   Range{CreatePosition(1, 1), CreatePosition(1, 5)}},
+  {HighlightingKind::Function,
+   Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
+  {HighlightingKind::Variable,
+   Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+  };
+
+  std::vector Results = toSemanticTokens(Tokens);
+  EXPECT_EQ(Tokens.size(), Results.size());
+  EXPECT_EQ(Results[0].tokenType, unsigned(HighlightingKind::Variable));
+  EXPECT_EQ(Results[0].deltaLine, 1u);
+  EXPECT_EQ(Results[0].deltaStart, 1u);
+  EXPECT_EQ(Results[0].length, 4u);
+
+  EXPECT_EQ(Results[1].tokenType, unsigned(HighlightingKind::Function));
+  EXPECT_EQ(Results[1].deltaLine, 2u);
+  EXPECT_EQ(Results[1].deltaStart, 4u);
+  EXPECT_EQ(Results[1].length, 3u);
+
+  EXPECT_EQ(Results[2].tokenType, unsigned(HighlightingKind::Variable));
+  EXPECT_EQ(Results[2].deltaLine, 0u);
+  EXPECT_EQ(Results[2].deltaStart, 4u);
+  EXPECT_EQ(Results[2].length, 4u);
+}
+
 TEST(SemanticHighlighting, toTheiaSemanticHighlightingInformation) {
   auto CreatePosition = [](int Line, int Character) -> Position {
 Position Pos;
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -0,0 +1,22 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","text":"int x = 2;"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/semanticTokens","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:   "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"data": [
+#  First line, char 5, variable, no modifiers.
+# CHECK-NEXT:  0,
+# CHECK-NEXT:  4,
+# CHECK-NEXT:  1,
+# CHECK-NEXT:  0,
+# CHECK-NEXT:  0
+# CHECK-NEXT:]
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -38,6 +38,15 @@
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
+# CHECK-NEXT:  "semanticTokensProvider": {
+# CHECK-NEXT:"documentProvider": true,
+# CHECK-NEXT:"legend": {
+# CHECK-NEXT:  "tokenModifiers": [],
+# CHECK-NEXT:  "tokenTypes": [
+# CHECK-NEXT:"variable",
+# CHECK:   ]
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -75,6 +75,9 @@
 // main AST.
 std::vector getSemanticHighlightings(ParsedAST );
 
+std::vector toSemanticTokens(llvm::ArrayRef);

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:140
+  if (llvm::sys::path::is_relative(SysRoot)) {
+SysRoot = GetResourcesPath(ClangExecutable, SysRoot);
+  }

I don't think this is an intended use for `GetResourcesPath` since this is not 
a resource path. Since you set custom resource dir, this is equivalent to:

```
llvm::sys::path::append(llvm::sys::path::parent_path(ClangExecutable), SysRoot)
```

On line 144 we already set `Dir = 
llvm::sys::path::parent_path(ClangExecutable)`, so if you move this below you 
can just use `SysRoot = llvm::sys::path::append(Dir, SysRoot)` which should be 
sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653



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

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

I have no idea how to write a test for this, but I tested it locally with wasi 
SDK.

Hopefully fucia will find this useful too as a generic version of 
https://reviews.llvm.org/D42019


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653



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

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4621
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking up int he @implementation block if the
+  //   translation unit sees it to find more clashes.

"in the"


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

https://reviews.llvm.org/D76643



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

[PATCH] D60748: Adds an option "malign-pass-aggregate" to make the alignment of the struct and union parameters compatible with the default gcc

[LiuChen via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1570
+return 4;
+} else if (Align < 16)
+  return MinABIStackAlignInBytes;

LiuChen3 wrote:
> jyknight wrote:
> > If I understood GCC's algorithm correctly, I think this needs to come first?
> You mean it should be ?
> 
> 
> ```
> if (MaxAlignment < 16)
>   retrun 4
> else
>  return std::max(MaxAlignment, Align);
> ```
I found that the test I wrote above was wrong. Sorry for the noise.


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

https://reviews.llvm.org/D60748



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

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, sunfish, aheejin, mgorny, dschuff.
Herald added a project: clang.
sbc100 retitled this revision from "Allow -DDEFAULT_SYSROOT to be a relative 
path" to "[clang] Allow -DDEFAULT_SYSROOT to be a relative path".
sbc100 added reviewers: sunfish, phosek.

In this case we interpret the path as relative the llvm installation
root.

This allows SDKs to be build that include clang along with a custom
sysroot without requiring users to specify --sysroot to point to the
directory where they install the SDK.

See https://github.com/WebAssembly/wasi-sdk/issues/58


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76653

Files:
  clang/CMakeLists.txt
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -136,6 +136,10 @@
   if (!this->VFS)
 this->VFS = llvm::vfs::getRealFileSystem();
 
+  if (llvm::sys::path::is_relative(SysRoot)) {
+SysRoot = GetResourcesPath(ClangExecutable, SysRoot);
+  }
+
   Name = std::string(llvm::sys::path::filename(ClangExecutable));
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -136,6 +136,10 @@
   if (!this->VFS)
 this->VFS = llvm::vfs::getRealFileSystem();
 
+  if (llvm::sys::path::is_relative(SysRoot)) {
+SysRoot = GetResourcesPath(ClangExecutable, SysRoot);
+  }
+
   Name = std::string(llvm::sys::path::filename(ClangExecutable));
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76605: [analyzer] Display the checker name in the text output

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sounds great!

Does the checker name get printed on the final note as well in text output? We 
should probably test it.




Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:313-316
+ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
+"Display the checker name for textual outputs",
+true)
+

Why do we need an option? Is it just for tests? Is it for clang-tidy to avoid 
printing the checker name twice?




Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:112
 reportPiece(NoteID, Piece->getLocation().asLocation(),
-Piece->getString(), Piece->getRanges(), 
Piece->getFixits());
+Piece->getString().str(), Piece->getRanges(),
+Piece->getFixits());

Szelethus wrote:
> martong wrote:
> > Why the `.str()` ?
> `StringRef` no longer converts to `std::string` implicitly.
But it seems to have been fine before(?)



Comment at: clang/test/Analysis/incorrect-checker-names.cpp:6
+  int x = 0;
+  // FIXME: This shouldn't be tied to a modeling checker.
+  return  // expected-warning{{Address of stack memory associated with 
local variable 'x' returned to caller [core.StackAddrEscapeBase]}}

Do you have a plan to address this FIXME? Does clang-tidy have the same problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76605



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

[PATCH] D76379: [Analyzer] IteratorRangeChecker verify `std::advance()`, `std::prev()` and `std::next()`

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D76379#1930917 , 
@baloghadamsoftware wrote:

> In D76379#1929698 , @Szelethus wrote:
>
> > The patch looks great, though I'd kindly ask you to wait a bit for someone 
> > with a bit more experience on `SVal`-smithing ;)
>
>
> Do you mean the change from `const SVal &` to `SVal`? It was done according 
> to this comment .


I think @Szelethus means my inline comment.




Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp:131
+C, Call.getArgSVal(0),
+nonloc::ConcreteInt(BVF.getValue(llvm::APSInt::get(1;
+  }

Please think about the type of the integer. You most likely want 
`SValBuilder::makeArrayIndex()`. There's also `SValBuilder::makeIntVal()` with 
a bunch of handy overloads. You almost never need to access `BasicValueFactory` 
directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76379



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

[PATCH] D76622: [analyzer] ConstraintManager - use EXPENSIVE_CHECKS instead of (gcc specific) __OPTIMIZE__ guard

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sounds good but eventually i hope we re-enable this assert in release+assert 
builds (D57062 ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76622



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

[clang-tools-extra] edf6a19 - [clangd] Rename theia-derived semantic highlighting protocol. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-03-24T00:39:47+01:00
New Revision: edf6a19adf7acf54f96e78718fb2339e5fcbc444

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

LOG: [clangd] Rename theia-derived semantic highlighting protocol. NFC

This feature is being added to LSP with a different (request-response)
protocol in 3.16, so this should avoid some confusion.

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 748269d5aef4..d0e8d139a40e 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -485,8 +485,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
,
   }
   }
 
-  ClangdServerOpts.SemanticHighlighting =
-  Params.capabilities.SemanticHighlighting;
+  ClangdServerOpts.TheiaSemanticHighlighting =
+  Params.capabilities.TheiaSemanticHighlighting;
   if (Params.rootUri && *Params.rootUri)
 ClangdServerOpts.WorkspaceRoot = std::string(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
@@ -611,7 +611,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
,
 ;
   if (NegotiatedOffsetEncoding)
 Result["offsetEncoding"] = *NegotiatedOffsetEncoding;
-  if (Params.capabilities.SemanticHighlighting)
+  if (Params.capabilities.TheiaSemanticHighlighting)
 Result.getObject("capabilities")
 ->insert(
 {"semanticHighlighting",
@@ -1169,8 +1169,8 @@ void ClangdLSPServer::applyConfiguration(
   reparseOpenedFiles(ModifiedFiles);
 }
 
-void ClangdLSPServer::publishSemanticHighlighting(
-const SemanticHighlightingParams ) {
+void ClangdLSPServer::publishTheiaSemanticHighlighting(
+const TheiaSemanticHighlightingParams ) {
   notify("textDocument/semanticHighlighting", Params);
 }
 
@@ -1376,12 +1376,12 @@ void ClangdLSPServer::onHighlightingsReady(
   // LSP allows us to send incremental edits of highlightings. Also need to 
diff 
   // to remove highlightings from tokens that should no longer have them.
   std::vector Diffed = 
diff Highlightings(Highlightings, Old);
-  SemanticHighlightingParams Notification;
+  TheiaSemanticHighlightingParams Notification;
   Notification.TextDocument.uri =
   URIForFile::canonicalize(File, /*TUPath=*/File);
   Notification.TextDocument.version = decodeVersion(Version);
-  Notification.Lines = toSemanticHighlightingInformation(Diffed);
-  publishSemanticHighlighting(Notification);
+  Notification.Lines = toTheiaSemanticHighlightingInformation(Diffed);
+  publishTheiaSemanticHighlighting(Notification);
 }
 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index e70b7b56a4f2..c4e9e5fb679c 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -135,7 +135,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   void applyConfiguration(const ConfigurationSettings );
 
   /// Sends a "publishSemanticHighlighting" notification to the LSP client.
-  void publishSemanticHighlighting(const SemanticHighlightingParams &);
+  void
+  publishTheiaSemanticHighlighting(const TheiaSemanticHighlightingParams &);
 
   /// Sends a "publishDiagnostics" notification to the LSP client.
   void publishDiagnostics(const PublishDiagnosticsParams &);

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5d2bfa7c8c57..3d68f85b6487 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -58,9 +58,9 @@ namespace {
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex,
ClangdServer::Callbacks *ServerCallbacks,
-   bool SemanticHighlighting)
+   bool TheiaSemanticHighlighting)
   : FIndex(FIndex), ServerCallbacks(ServerCallbacks),
-SemanticHighlighting(SemanticHighlighting) {}
+TheiaSemanticHighlighting(TheiaSemanticHighlighting) {}
 
   void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext ,
  std::shared_ptr PP,
@@ -75,14 +75,14 @@ 

[clang] 5bd0611 - Update documentation for __builtin_operator_new and

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-23T16:31:10-07:00
New Revision: 5bd06118c2a798f1f87b9251953bae8a27f21e5f

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

LOG: Update documentation for __builtin_operator_new and
__builtin_operator_delete to match r328134.

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index aee0b82880ba..f9511dc1a02f 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2129,21 +2129,32 @@ object that overloads ``operator&``.
 ``__builtin_operator_new`` and ``__builtin_operator_delete``
 
 
-``__builtin_operator_new`` allocates memory just like a non-placement non-class
-*new-expression*. This is exactly like directly calling the normal
-non-placement ``::operator new``, except that it allows certain optimizations
+A call to ``__builtin_operator_new(args)`` is exactly the same as a call to
+``::operator new(args)``, except that it allows certain optimizations
 that the C++ standard does not permit for a direct function call to
 ``::operator new`` (in particular, removing ``new`` / ``delete`` pairs and
-merging allocations).
+merging allocations), and that the call is required to resolve to a
+`replaceable global allocation function
+`_.
 
-Likewise, ``__builtin_operator_delete`` deallocates memory just like a
-non-class *delete-expression*, and is exactly like directly calling the normal
-``::operator delete``, except that it permits optimizations. Only the unsized
-form of ``__builtin_operator_delete`` is currently available.
+Likewise, ``__builtin_operator_delete`` is exactly the same as a call to
+``::operator delete(args)``, except that it permits optimizations
+and that the call is required to resolve to a
+`replaceable global deallocation function
+`_.
 
 These builtins are intended for use in the implementation of ``std::allocator``
 and other similar allocation libraries, and are only available in C++.
 
+Query for this feature with ``__has_builtin(__builtin_operator_new)`` or
+``__has_builtin(__builtin_operator_delete)``:
+
+  * If the value is at least ``201802L``, the builtins behave as described 
above.
+
+  * If the value is non-zero, the builtins may not support calling arbitrary
+replaceable global (de)allocation functions, but do support calling at 
least
+``::operator new(size_t)`` and ``::operator delete(void*)``.
+
 ``__builtin_preserve_access_index``
 ---
 



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

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

[Alon Zakai via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/AttrDocs.td:4173
+attribute for the WebAssembly target. This attribute may be attached to a
+function declaration, where it causes the be exported from the linked
+WebAssembly module.

"the be" => "the function to be"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76547



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

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Actually looking again, I'm not sure there's any way to make clang generate a 
bool vector at the moment.  Added tests for the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76528



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

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

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

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:6364-6371
+const llvm::fltSemantics  = Info.Ctx.getFloatTypeSemantics(Ty);
+unsigned NumBits = APFloat::semanticsSizeInBits(Semantics);
+assert(NumBits % 8 == 0);
+CharUnits Width = CharUnits::fromQuantity(NumBits / 8);
+SmallVector Bytes(Width.getQuantity());
+llvm::StoreIntToMemory(AsInt, &*Bytes.begin(), Width.getQuantity());
+Buffer.writeObject(Offset, Bytes);

This appears more complex than necessary. `bitcastToAPInt` already returned the 
correct number of bits (eg, 80 for x86 fp80) so you don't need to compute that 
yourself. How about leaving this function alone and changing `visitInt` to 
store the number of (value representation) bits in the `APSInt` rather than the 
full (object representation) width of the type? As is, `visitInt` would also be 
wrong in the same way if the integer type had any full bytes of padding.



Comment at: clang/lib/AST/ExprConstant.cpp:6462
+APSInt Truncated = Val.trunc(IntWidth);
+if (Truncated.zext(Val.getBitWidth()) != Val)
+  return unrepresentableValue(QualType(T, 0), Val);

If you want this to be a general check for integer types with padding, you 
should zext or sext as appropriate for the signedness of the truncated value. 
(Eg, you could set the signedness of `Truncated` to match the integer type, and 
use `extend` rather than `zext` here.)


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

https://reviews.llvm.org/D76323



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

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

[Eli Friedman via Phabricator via cfe-commits]

efriedma updated this revision to Diff 252169.
efriedma edited the summary of this revision.
efriedma added a comment.

Add testcase to show missing vector handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76528

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp


Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1610,5 +1610,24 @@
 // CHECK-NEXT:  store <4 x double> , <4 x 
double>* %custom, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+// TODO: This vector has tail padding
+TEST_UNINIT(doublevec24, double  __attribute__((vector_size(24;
+// CHECK-LABEL: @test_doublevec24_uninit()
+// CHECK:   %uninit = alloca <3 x double>, align
+// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-LABEL: @test_doublevec24_uninit()
+// PATTERN: store <3 x double> , <3 x double>* %uninit, align 32
+// ZERO-LABEL: @test_doublevec24_uninit()
+// ZERO: store <3 x double> zeroinitializer, <3 x double>* %uninit, align 32
+
+// TODO: This vector has tail padding
+TEST_UNINIT(longdoublevec32, long double  
__attribute__((vector_size(sizeof(long double)*2;
+// CHECK-LABEL: @test_longdoublevec32_uninit()
+// CHECK:   %uninit = alloca <2 x x86_fp80>, align
+// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-LABEL: @test_longdoublevec32_uninit()
+// PATTERN: store <2 x x86_fp80> , <2 x x86_fp80>* %uninit, align 32
+// ZERO-LABEL: @test_longdoublevec32_uninit()
+// ZERO: store <2 x x86_fp80> zeroinitializer, <2 x x86_fp80>* %uninit, align 
32
 
 } // extern "C"
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1050,13 +1050,13 @@
   llvm::Type *OrigTy = constant->getType();
   if (const auto STy = dyn_cast(OrigTy))
 return constStructWithPadding(CGM, isPattern, STy, constant);
-  if (auto *STy = dyn_cast(OrigTy)) {
+  if (auto *ArrayTy = dyn_cast(OrigTy)) {
 llvm::SmallVector Values;
-unsigned Size = STy->getNumElements();
+uint64_t Size = ArrayTy->getNumElements();
 if (!Size)
   return constant;
-llvm::Type *ElemTy = STy->getElementType();
-bool ZeroInitializer = constant->isZeroValue();
+llvm::Type *ElemTy = ArrayTy->getElementType();
+bool ZeroInitializer = constant->isNullValue();
 llvm::Constant *OpValue, *PaddedOp;
 if (ZeroInitializer) {
   OpValue = llvm::Constant::getNullValue(ElemTy);
@@ -1072,13 +1072,10 @@
 auto *NewElemTy = Values[0]->getType();
 if (NewElemTy == ElemTy)
   return constant;
-if (OrigTy->isArrayTy()) {
-  auto *ArrayTy = llvm::ArrayType::get(NewElemTy, Size);
-  return llvm::ConstantArray::get(ArrayTy, Values);
-} else {
-  return llvm::ConstantVector::get(Values);
-}
+auto *NewArrayTy = llvm::ArrayType::get(NewElemTy, Size);
+return llvm::ConstantArray::get(NewArrayTy, Values);
   }
+  // FIXME: Do we need to handle tail padding in vectors?
   return constant;
 }
 


Index: clang/test/CodeGenCXX/auto-var-init.cpp
===
--- clang/test/CodeGenCXX/auto-var-init.cpp
+++ clang/test/CodeGenCXX/auto-var-init.cpp
@@ -1610,5 +1610,24 @@
 // CHECK-NEXT:  store <4 x double> , <4 x double>* %custom, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%custom)
 
+// TODO: This vector has tail padding
+TEST_UNINIT(doublevec24, double  __attribute__((vector_size(24;
+// CHECK-LABEL: @test_doublevec24_uninit()
+// CHECK:   %uninit = alloca <3 x double>, align
+// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-LABEL: @test_doublevec24_uninit()
+// PATTERN: store <3 x double> , <3 x double>* %uninit, align 32
+// ZERO-LABEL: @test_doublevec24_uninit()
+// ZERO: store <3 x double> zeroinitializer, <3 x double>* %uninit, align 32
+
+// TODO: This vector has tail padding
+TEST_UNINIT(longdoublevec32, long double  __attribute__((vector_size(sizeof(long double)*2;
+// CHECK-LABEL: @test_longdoublevec32_uninit()
+// CHECK:   %uninit = alloca <2 x x86_fp80>, align
+// CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
+// PATTERN-LABEL: @test_longdoublevec32_uninit()
+// PATTERN: store <2 x x86_fp80> , <2 x x86_fp80>* %uninit, align 32
+// ZERO-LABEL: @test_longdoublevec32_uninit()
+// ZERO: store <2 x x86_fp80> zeroinitializer, <2 x x86_fp80>* %uninit, align 32
 
 } // extern "C"
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1050,13 +1050,13 @@
   llvm::Type *OrigTy = constant->getType();
   if (const auto STy = dyn_cast(OrigTy))
 return 

[clang] 2b4027f - [analyzer] Delete unneeded headers and using after D76509 for layering check

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-03-23T16:11:15-07:00
New Revision: 2b4027f2b8d8224266cc2a423d9ccd74f6800711

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

LOG: [analyzer] Delete unneeded headers and using after D76509 for layering 
check

Otherwise it is incorrect to remove clangStaticAnalyzerFrontend's
dependency on clangRewrite and clangToolingCore.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp 
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 7e1e090afa60..2e3aa0669061 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -34,8 +34,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h"
-#include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/FileSystem.h"
@@ -49,7 +47,6 @@
 
 using namespace clang;
 using namespace ento;
-using namespace tooling;
 
 #define DEBUG_TYPE "AnalysisConsumer"
 



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

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 252162.
MadCoder edited the summary of this revision.

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

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -12,6 +12,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (void)unavailableInChild;
 - (void)rootRegular;  // expected-note {{previous declaration is here}}
 + (void)classRootRegular; // expected-note {{previous declaration is here}}
 - (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
@@ -52,6 +53,7 @@
 __attribute__((objc_direct_members))
 @interface SubDirectMembers : Root
 @property int foo; // expected-note {{previous declaration is here}}
+- (void)unavailableInChild __attribute__((unavailable)); // should not warn
 - (instancetype)init;
 @end
 
@@ -81,6 +83,8 @@
 
 __attribute__((objc_direct_members))
 @implementation Root
+- (void)unavailableInChild {
+}
 - (void)rootRegular {
 }
 + (void)classRootRegular {
Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,62 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema , Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && !Method->hasAttr() &&
+  CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema , ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking up int he @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = Cat->getImplementation())
+  if (CatImpl != ImpDecl)
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+}
+
 Decl *Sema::ActOnMethodDeclaration(
 Scope *S, SourceLocation MethodLoc, 

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D76184#1927159 , @rnk wrote:

> I patched this in with it's dependency, and to compile just Attr.h by itself, 
> it reduces the time from ~2.420s to ~1.920s, so it is worth pushing this 
> through. :)
>
> However, I noticed that there are still some targets broken by the removal of 
> the SmallSet.h include. I'm happy to push that through if you want to hand it 
> off.


I was about to push all three changes but I wans't aware of broken targets. Thx 
for telling me. I would not mind you talking over this patch. I'll merged the 
other two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76184



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

[Marcus Johnson via Phabricator via cfe-commits]

MarcusJohnson91 added a comment.

Rebased on Master again, recompiling and re-running all the tests.

I'll update this comment when it passes, or create a new diff if it doesn't but 
nothing had to be changed so it'll probably work.

@krasimir I noticed that you've been active recently, can you review my patch?

Not sure if tagging is considered rude, I figure that @MyDeveloperDay's 
notification fell off your radar.


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

https://reviews.llvm.org/D75791



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

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added a reviewer: rsmith.
Herald added subscribers: cfe-commits, jfb.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

There is a version that just tests (also called
isIntegerConstantExpression) & whereas this version is specifically used
when the value is of interest (a few call sites were actually refactored
to calling the test-only version) so let's make the API look more like
it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76646

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2417,8 +2417,8 @@
 return Context.getDependentVectorType(CurType, SizeExpr, AttrLoc,
VectorType::GenericVector);
 
-  llvm::APSInt VecSize(32);
-  if (!SizeExpr->isIntegerConstantExpr(VecSize, Context)) {
+  Optional VecSize = SizeExpr->getIntegerConstantExpr(Context);
+  if (!VecSize) {
 Diag(AttrLoc, diag::err_attribute_argument_type)
 << "vector_size" << AANT_ArgumentIntegerConstant
 << SizeExpr->getSourceRange();
@@ -2429,7 +2429,7 @@
 return Context.getDependentVectorType(CurType, SizeExpr, AttrLoc,
VectorType::GenericVector);
 
-  unsigned VectorSize = static_cast(VecSize.getZExtValue() * 8);
+  unsigned VectorSize = static_cast(VecSize->getZExtValue() * 8);
   unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
 
   if (VectorSize == 0) {
@@ -2474,8 +2474,8 @@
   }
 
   if (!ArraySize->isTypeDependent() && !ArraySize->isValueDependent()) {
-llvm::APSInt vecSize(32);
-if (!ArraySize->isIntegerConstantExpr(vecSize, Context)) {
+Optional vecSize = ArraySize->getIntegerConstantExpr(Context);
+if (!vecSize) {
   Diag(AttrLoc, diag::err_attribute_argument_type)
 << "ext_vector_type" << AANT_ArgumentIntegerConstant
 << ArraySize->getSourceRange();
@@ -2484,7 +2484,7 @@
 
 // Unlike gcc's vector_size attribute, the size is specified as the
 // number of elements, not the number of bytes.
-unsigned vectorSize = static_cast(vecSize.getZExtValue());
+unsigned vectorSize = static_cast(vecSize->getZExtValue());
 
 if (vectorSize == 0) {
   Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -6073,13 +6073,15 @@
const Expr *AddrSpace,
SourceLocation AttrLoc) {
   if (!AddrSpace->isValueDependent()) {
-llvm::APSInt addrSpace(32);
-if (!AddrSpace->isIntegerConstantExpr(addrSpace, S.Context)) {
+Optional OptAddrSpace =
+AddrSpace->getIntegerConstantExpr(S.Context);
+if (!OptAddrSpace) {
   S.Diag(AttrLoc, diag::err_attribute_argument_type)
   << "'address_space'" << AANT_ArgumentIntegerConstant
   << AddrSpace->getSourceRange();
   return false;
 }
+llvm::APSInt  = *OptAddrSpace;
 
 // Bounds checking.
 if (addrSpace.isSigned()) {
@@ -7510,9 +7512,9 @@
   }
   // The number of elements must be an ICE.
   Expr *numEltsExpr = static_cast(Attr.getArgAsExpr(0));
-  llvm::APSInt numEltsInt(32);
+  Optional numEltsInt;
   if (numEltsExpr->isTypeDependent() || numEltsExpr->isValueDependent() ||
-  !numEltsExpr->isIntegerConstantExpr(numEltsInt, S.Context)) {
+  !(numEltsInt = numEltsExpr->getIntegerConstantExpr(S.Context))) {
 S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
 << Attr << AANT_ArgumentIntegerConstant
 << numEltsExpr->getSourceRange();
@@ -7528,7 +7530,7 @@
 
   // The total size of the vector must be 64 or 128 bits.
   unsigned typeSize = static_cast(S.Context.getTypeSize(CurType));
-  unsigned numElts = static_cast(numEltsInt.getZExtValue());
+  unsigned numElts = static_cast(numEltsInt->getZExtValue());
   unsigned vecSize = typeSize * numElts;
   if (vecSize != 64 && vecSize != 128) {
 S.Diag(Attr.getLoc(), diag::err_attribute_bad_neon_vector_size) << CurType;
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -296,15 +296,15 @@
 
   if (NumArgs == 1) {
 Expr *E = A.getArgAsExpr(0);
-llvm::APSInt ArgVal(32);
+Optional ArgVal;
 
-if (!E->isIntegerConstantExpr(ArgVal, S.Context)) {
+if (!(ArgVal = 

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

The test files I added checked initialization had stores to each padding 
location, I think that's what would be needed here as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76528



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

[PATCH] D76094: [clangd] Change line break behaviour for hoverinfo

[Lorenz Junglas via Phabricator via cfe-commits]

lolleko updated this revision to Diff 252159.
lolleko added a comment.

Final Cleanup
Removed markdown linebreak parsing for now.

No I don't have commit rights yet. Feel fre to merge this for me.

Thanks for the thorough review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76094

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1889,6 +1889,26 @@
 llvm::StringRef ExpectedRenderMarkdown;
 llvm::StringRef ExpectedRenderPlainText;
   } Cases[] = {{
+   " \n foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar \n  ",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo  \nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
"foo\n\n\nbar",
"foo  \nbar",
"foo\nbar",
@@ -1908,79 +1928,11 @@
"foo.  \nbar",
"foo.\nbar",
},
-   {
-   "foo:\nbar",
-   "foo:  \nbar",
-   "foo:\nbar",
-   },
-   {
-   "foo,\nbar",
-   "foo,  \nbar",
-   "foo,\nbar",
-   },
-   {
-   "foo;\nbar",
-   "foo;  \nbar",
-   "foo;\nbar",
-   },
-   {
-   "foo!\nbar",
-   "foo!  \nbar",
-   "foo!\nbar",
-   },
-   {
-   "foo?\nbar",
-   "foo?  \nbar",
-   "foo?\nbar",
-   },
-   {
-   "foo\n-bar",
-   "foo  \n-bar",
-   "foo\n-bar",
-   },
{
"foo\n*bar",
-   // TODO `*` should probably not be escaped after line break
"foo  \n\\*bar",
"foo\n*bar",
},
-   {
-   "foo\n@bar",
-   "foo  \n@bar",
-   "foo\n@bar",
-   },
-   {
-   "foo\n\\bar",
-   // TODO `\` should probably not be escaped after line break
-   "foo  \nbar",
-   "foo\n\\bar",
-   },
-   {
-   "foo\n>bar",
-   // TODO `>` should probably not be escaped after line break
-   "foo  \n\\>bar",
-   "foo\n>bar",
-   },
-   {
-   "foo\n#bar",
-   "foo  \n#bar",
-   "foo\n#bar",
-   },
-   {
-   "foo  \nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\\\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
{
"foo\nbar",
"foo bar",
@@ -1989,7 +1941,7 @@
 
   for (const auto  : Cases) {
 markup::Document Output;
-parseLineBreaks(C.Documentation, Output);
+parseDocumentation(C.Documentation, Output);
 
 EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
 EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -75,7 +75,7 @@
   markup::Document present() const;
 };
 
-void parseLineBreaks(llvm::StringRef Input, markup::Document );
+void parseDocumentation(llvm::StringRef Input, markup::Document );
 
 llvm::raw_ostream <<(llvm::raw_ostream &, const HoverInfo::Param &);
 inline bool operator==(const HoverInfo::Param ,
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -522,26 +522,15 @@
 }
 
 bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
-  if (LineBreakIndex 

Re: [clang] 43606ef - Suppress an "unused variable" warning in release build

[David Blaikie via cfe-commits]

Sent  https://reviews.llvm.org/D76646 to hopefully make this a bit more
legible

On Mon, Mar 23, 2020 at 2:38 PM David Blaikie  wrote:

> Ah, thanks. (I'm going to see about renaming/restructuring that API - to
> return Optional and be called 'get' rather than 'is' to make this
> more clear)
>
> On Mon, Mar 23, 2020 at 5:04 AM Mikhail Maltsev 
> wrote:
>
>> Yes, it does. isIntegerConstantExpr assigns a value to CoprocNoAP.
>>
>> --
>> Regards,
>>Mikhail Maltsev
>>
>> 
>> From: David Blaikie 
>> Sent: Sunday, March 22, 2020 03:32
>> To: Mikhail Maltsev; Mikhail Maltsev
>> Cc: cfe-commits
>> Subject: Re: [clang] 43606ef - Suppress an "unused variable" warning in
>> release build
>>
>> Does "isIntegerConstantExpr" have side effects that are
>> desired/necessary? Otherwise please change this to roll the
>> isIntegerConstantExpr into the assert (so that it is only executed when
>> asserts are enabled)
>>
>> On Tue, Mar 10, 2020 at 10:11 AM Mikhail Maltsev via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> Author: Mikhail Maltsev
>> Date: 2020-03-10T17:10:52Z
>> New Revision: 43606efb6847fc9c79e7d93760a2a6191e8a8539
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/43606efb6847fc9c79e7d93760a2a6191e8a8539
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/43606efb6847fc9c79e7d93760a2a6191e8a8539.diff
>>
>> LOG: Suppress an "unused variable" warning in release build
>>
>> Added:
>>
>>
>> Modified:
>> clang/lib/Sema/SemaChecking.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/lib/Sema/SemaChecking.cpp
>> b/clang/lib/Sema/SemaChecking.cpp
>> index 24d0d9209a1d..8a2b4b019663 100644
>> --- a/clang/lib/Sema/SemaChecking.cpp
>> +++ b/clang/lib/Sema/SemaChecking.cpp
>> @@ -2094,6 +2094,7 @@ bool Sema::CheckARMCoprocessorImmediate(const Expr
>> *CoprocArg, bool WantCDE) {
>>
>>llvm::APSInt CoprocNoAP;
>>bool IsICE = CoprocArg->isIntegerConstantExpr(CoprocNoAP, Context);
>> +  (void)IsICE;
>>assert(IsICE && "Coprocossor immediate is not a constant expression");
>>int64_t CoprocNo = CoprocNoAP.getExtValue();
>>assert(CoprocNo >= 0 && "Coprocessor immediate must be non-negative");
>>
>>
>>
>> ___
>> 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

Re: [clang] 4a0267e - Convert a reachable llvm_unreachable into an assert.

[David Blaikie via cfe-commits]

On Mon, Mar 23, 2020 at 5:24 AM Aaron Ballman 
wrote:

> On Sun, Mar 22, 2020 at 3:31 PM David Blaikie  wrote:
> >
> > On Sun, Mar 22, 2020 at 9:34 AM Aaron Ballman 
> wrote:
> >>
> >> On Sun, Mar 22, 2020 at 12:19 PM David Blaikie 
> wrote:
> >> >
> >> > On Sun, Mar 22, 2020 at 6:34 AM Aaron Ballman 
> wrote:
> >> >>
> >> >> On Sat, Mar 21, 2020 at 11:31 PM David Blaikie 
> wrote:
> >> >> >
> >> >> > Why the change? this seems counter to LLVM's style which pretty
> consistently uses unreachable rather than assert(false), so far as I know?
> >> >>
> >> >> We're not super consistent (at least within Clang), but the rules as
> >> >> I've generally understood them are to use llvm_unreachable only for
> >> >> truly unreachable code and to use assert(false) when the code is
> >> >> technically reachable but is a programmer mistake to have gotten
> >> >> there.
> >> >
> >> >
> >> > I don't see those as two different things personally -
> llvm_unreachable is used when the programmer believes it to be unreachable
> (not that it must be proven to be unreachable - we have message text there
> so it's informative if the assumption turns out not to hold)
> >>
> >> The message text doesn't help when the code is reached in release
> >> mode, which was the issue. Asserts + release you still get some
> >> helpful text saying "you screwed up". llvm_unreachable in release
> >> mode, you may get lucky or you may not (in this case, I didn't get
> >> lucky -- there was no text, just a crash).
> >
> >
> > That doesn't seem to be what it's documented to do:
> >
> > /// Marks that the current location is not supposed to be reachable.
> > /// In !NDEBUG builds, prints the message and location info to stderr.
> > /// In NDEBUG builds, becomes an optimizer hint that the current location
> > /// is not supposed to be reachable.  On compilers that don't support
> > /// such hints, prints a reduced message instead.
> >
> > & certainly I think the documentation is what it /should/ be doing.
> >
> > /maybe/
> https://reviews.llvm.org/rG5f4535b974e973d52797945fbf80f19ffba8c4ad broke
> that contract on Windows, but I'm not sure how? (an unreachable at the end
> of that function shouldn't cause the whole function to be unreachable -
> because abort could have side effects and halt the program before the
> unreachable is reached)
>
> Agreed. It could also be that my machine is in a weird state (I'm
> currently battling a situation where the command line parser appears
> to be totally broken on Windows due to misuse of a ManagedStatic
> somewhere but I've not seen any commits that relate to the issues).
>
> >> >> In this particular case, the code is very much reachable
> >> >
> >> >
> >> > In what sense? If it is actually reachable - shouldn't it be tested?
> (& in which case the assert(false) will get in the way of that testing)
> >>
> >> In the sense that normal code paths reach that code easily. Basically,
> >> that code is checking to see whether a plugin you've written properly
> >> sets up its options or not. When you're developing a plugin, it's
> >> quite reasonable to expect you won't get it just right on the first
> >> try, so you hit the code path but only as a result of you not writing
> >> the plugin quite right. So under normal conditions (once the plugin is
> >> working), the code path should not be reached but under development
> >> the code path gets reached accidentally.
> >>
> >> >> and I
> >> >> spent a lot more time debugging than I should have because I was
> using
> >> >> a release + asserts build and the semantics of llvm_unreachable made
> >> >> unfortunate codegen (switching to an assert makes the issue
> >> >> immediately obvious).
> >> >
> >> >
> >> > I think it might be reasonable to say that release+asserts to have
> unreachable behave the same way unreachable behaves at -O0 (which is to
> say, much like assert(false)). Clearly release+asserts effects code
> generation, so there's nothing like the "debug info invariance" goal that
> this would be tainting, etc.
> >> >
> >> > Certainly opinions vary here, but here are some commits that show the
> sort of general preference:
> >> > http://llvm.org/viewvc/llvm-project?view=revision=259302
> >> > http://llvm.org/viewvc/llvm-project?view=revision=253005
> >> > http://llvm.org/viewvc/llvm-project?view=revision=251266
> >> >
> >> > And some counter examples:
> >> > http://llvm.org/viewvc/llvm-project?view=revision=225043
> >> > Including this thread where Chandler originally (not sure what his
> take on it is these days) expressed some ideas more along your lines:
> >> >
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20110919/thread.html#46583
> >> >
> >> > But I'm always pretty concerned about the idea that assertions should
> be used in places where the behavior of the program has any constraints
> when the assertion is false...
> >>
> >> I'm opposed to using unreachability hints on control flow paths you
> >> expect to reach -- the semantics are just 

[clang] 1236eb6 - [OPENMP50]Add 'default' modifier in reduction clauses.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-03-23T18:18:08-04:00
New Revision: 1236eb6c31ff0c1c9b69c544d735a3de4e0fc687

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

LOG: [OPENMP50]Add 'default' modifier in reduction clauses.

Added full support for 'default' modifier in the reduction clauses.

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/Basic/OpenMPKinds.def
clang/include/clang/Basic/OpenMPKinds.h
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/OpenMP/parallel_ast_print.cpp
clang/test/OpenMP/parallel_reduction_codegen.cpp
clang/test/OpenMP/parallel_reduction_messages.c

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 8b92cb62634a..29c251ef7ee6 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -2703,6 +2703,12 @@ class OMPReductionClause final
   friend OMPVarListClause;
   friend TrailingObjects;
 
+  /// Reduction modifier.
+  OpenMPReductionClauseModifier Modifier = OMPC_REDUCTION_unknown;
+
+  /// Reduction modifier location.
+  SourceLocation ModifierLoc;
+
   /// Location of ':'.
   SourceLocation ColonLoc;
 
@@ -2716,18 +2722,22 @@ class OMPReductionClause final
   ///
   /// \param StartLoc Starting location of the clause.
   /// \param LParenLoc Location of '('.
-  /// \param EndLoc Ending location of the clause.
+  /// \param ModifierLoc Modifier location.
   /// \param ColonLoc Location of ':'.
+  /// \param EndLoc Ending location of the clause.
   /// \param N Number of the variables in the clause.
   /// \param QualifierLoc The nested-name qualifier with location information
   /// \param NameInfo The full name info for reduction identifier.
   OMPReductionClause(SourceLocation StartLoc, SourceLocation LParenLoc,
- SourceLocation ColonLoc, SourceLocation EndLoc, unsigned 
N,
+ SourceLocation ModifierLoc, SourceLocation ColonLoc,
+ SourceLocation EndLoc,
+ OpenMPReductionClauseModifier Modifier, unsigned N,
  NestedNameSpecifierLoc QualifierLoc,
  const DeclarationNameInfo )
   : OMPVarListClause(OMPC_reduction, StartLoc,
  LParenLoc, EndLoc, N),
-OMPClauseWithPostUpdate(this), ColonLoc(ColonLoc),
+OMPClauseWithPostUpdate(this), Modifier(Modifier),
+ModifierLoc(ModifierLoc), ColonLoc(ColonLoc),
 QualifierLoc(QualifierLoc), NameInfo(NameInfo) {}
 
   /// Build an empty clause.
@@ -2739,6 +2749,12 @@ class OMPReductionClause final
  N),
 OMPClauseWithPostUpdate(this) {}
 
+  /// Sets reduction modifier.
+  void setModifier(OpenMPReductionClauseModifier M) { Modifier = M; }
+
+  /// Sets location of the modifier.
+  void setModifierLoc(SourceLocation Loc) { ModifierLoc = Loc; }
+
   /// Sets location of ':' symbol in clause.
   void setColonLoc(SourceLocation CL) { ColonLoc = CL; }
 
@@ -2808,6 +2824,7 @@ class OMPReductionClause final
   ///
   /// \param StartLoc Starting location of the clause.
   /// \param LParenLoc Location of '('.
+  /// \param ModifierLoc Modifier location.
   /// \param ColonLoc Location of ':'.
   /// \param EndLoc Ending location of the clause.
   /// \param VL The variables in the clause.
@@ -2838,8 +2855,9 @@ class OMPReductionClause final
   /// OpenMP region with this clause.
   static OMPReductionClause *
   Create(const ASTContext , SourceLocation StartLoc, SourceLocation 
LParenLoc,
- SourceLocation ColonLoc, SourceLocation EndLoc, ArrayRef VL,
- NestedNameSpecifierLoc QualifierLoc,
+ SourceLocation ModifierLoc, SourceLocation ColonLoc,
+ SourceLocation EndLoc, OpenMPReductionClauseModifier Modifier,
+ ArrayRef VL, NestedNameSpecifierLoc QualifierLoc,
  const DeclarationNameInfo , ArrayRef Privates,
  ArrayRef LHSExprs, ArrayRef RHSExprs,
  ArrayRef ReductionOps, Stmt *PreInit, Expr *PostUpdate);
@@ -2850,6 +2868,12 @@ class OMPReductionClause final
   /// \param N The number of variables.
   static OMPReductionClause *CreateEmpty(const ASTContext , unsigned N);
 
+  /// Returns modifier.
+  OpenMPReductionClauseModifier getModifier() const { return Modifier; }
+
+  /// Returns modifier location.
+  SourceLocation getModifierLoc() const { return ModifierLoc; }
+
   /// 

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: erik.pilkington, ahatanak, dexonsmith, arphaman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some methods are sometimes declared in the @implementation blocks which can 
cause undiagnosed clashes.

Just write a checkObjCDirectMethodClashes() for this purpose.

Signed-off-by: Pierre Habouzit 
Radar-ID: rdar://problem/59332804


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,61 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema , Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema , ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking up int he @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = Cat->getImplementation())
+  if (CatImpl != ImpDecl)
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+}
+
 Decl *Sema::ActOnMethodDeclaration(
 Scope *S, SourceLocation MethodLoc, SourceLocation EndLoc,
 tok::TokenKind MethodType, ObjCDeclSpec , ParsedType ReturnType,
@@ -4809,9 +4864,9 @@
   Diag(ObjCMethod->getLocation(), diag::warn_dealloc_in_category)
 << ObjCMethod->getDeclName();
 }
-  } else if (ImpDecl->hasAttr()) {
-ObjCMethod->addAttr(
-ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation()));
+  } else {
+mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod);
+checkObjCDirectMethodClashes(*this, IDecl, ObjCMethod, ImpDecl);
   }
 
   // Warn if a method declared in a protocol to which a category or
@@ -4832,39 +4887,16 @@
 }
   } else {
 if (!isa(ClassDecl)) {
-  if (!ObjCMethod->isDirectMethod() &&
-  ClassDecl->hasAttr()) {
-ObjCMethod->addAttr(
-

[PATCH] D76094: [clangd] Change line break behaviour for hoverinfo

[Lorenz Junglas via Phabricator via cfe-commits]

lolleko updated this revision to Diff 252153.
lolleko added a comment.

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76094

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1889,6 +1889,26 @@
 llvm::StringRef ExpectedRenderMarkdown;
 llvm::StringRef ExpectedRenderPlainText;
   } Cases[] = {{
+   " \n foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar \n  ",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo  \nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
"foo\n\n\nbar",
"foo  \nbar",
"foo\nbar",
@@ -1908,79 +1928,11 @@
"foo.  \nbar",
"foo.\nbar",
},
-   {
-   "foo:\nbar",
-   "foo:  \nbar",
-   "foo:\nbar",
-   },
-   {
-   "foo,\nbar",
-   "foo,  \nbar",
-   "foo,\nbar",
-   },
-   {
-   "foo;\nbar",
-   "foo;  \nbar",
-   "foo;\nbar",
-   },
-   {
-   "foo!\nbar",
-   "foo!  \nbar",
-   "foo!\nbar",
-   },
-   {
-   "foo?\nbar",
-   "foo?  \nbar",
-   "foo?\nbar",
-   },
-   {
-   "foo\n-bar",
-   "foo  \n-bar",
-   "foo\n-bar",
-   },
{
"foo\n*bar",
-   // TODO `*` should probably not be escaped after line break
"foo  \n\\*bar",
"foo\n*bar",
},
-   {
-   "foo\n@bar",
-   "foo  \n@bar",
-   "foo\n@bar",
-   },
-   {
-   "foo\n\\bar",
-   // TODO `\` should probably not be escaped after line break
-   "foo  \nbar",
-   "foo\n\\bar",
-   },
-   {
-   "foo\n>bar",
-   // TODO `>` should probably not be escaped after line break
-   "foo  \n\\>bar",
-   "foo\n>bar",
-   },
-   {
-   "foo\n#bar",
-   "foo  \n#bar",
-   "foo\n#bar",
-   },
-   {
-   "foo  \nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\\\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
{
"foo\nbar",
"foo bar",
@@ -1989,7 +1941,7 @@
 
   for (const auto  : Cases) {
 markup::Document Output;
-parseLineBreaks(C.Documentation, Output);
+parseDoucmentation(C.Documentation, Output);
 
 EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
 EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -75,7 +75,7 @@
   markup::Document present() const;
 };
 
-void parseLineBreaks(llvm::StringRef Input, markup::Document );
+void parseDocumentation(llvm::StringRef Input, markup::Document );
 
 llvm::raw_ostream <<(llvm::raw_ostream &, const HoverInfo::Param &);
 inline bool operator==(const HoverInfo::Param ,
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -522,26 +522,15 @@
 }
 
 bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
-  if (LineBreakIndex + 1 >= Str.size()) {
-return false;
-  }
-  auto NextNonSpaceCharIndex = Str.find_first_not_of(' ', LineBreakIndex + 1);
-
-  return 

[PATCH] D76094: [clangd] Change line break behaviour for hoverinfo

[Lorenz Junglas via Phabricator via cfe-commits]

lolleko updated this revision to Diff 252151.
lolleko added a comment.

Final Cleanup
Removed markdown linebreak parsing for now.

No I don't have commit rights yet. Feel fre to merge this for me.

Thanks for the thorough review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76094

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1889,6 +1889,26 @@
 llvm::StringRef ExpectedRenderMarkdown;
 llvm::StringRef ExpectedRenderPlainText;
   } Cases[] = {{
+   " \n foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar \n  ",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo  \nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
+   "foo\nbar",
+   "foo bar",
+   "foo bar",
+   },
+   {
"foo\n\n\nbar",
"foo  \nbar",
"foo\nbar",
@@ -1908,79 +1928,11 @@
"foo.  \nbar",
"foo.\nbar",
},
-   {
-   "foo:\nbar",
-   "foo:  \nbar",
-   "foo:\nbar",
-   },
-   {
-   "foo,\nbar",
-   "foo,  \nbar",
-   "foo,\nbar",
-   },
-   {
-   "foo;\nbar",
-   "foo;  \nbar",
-   "foo;\nbar",
-   },
-   {
-   "foo!\nbar",
-   "foo!  \nbar",
-   "foo!\nbar",
-   },
-   {
-   "foo?\nbar",
-   "foo?  \nbar",
-   "foo?\nbar",
-   },
-   {
-   "foo\n-bar",
-   "foo  \n-bar",
-   "foo\n-bar",
-   },
{
"foo\n*bar",
-   // TODO `*` should probably not be escaped after line break
"foo  \n\\*bar",
"foo\n*bar",
},
-   {
-   "foo\n@bar",
-   "foo  \n@bar",
-   "foo\n@bar",
-   },
-   {
-   "foo\n\\bar",
-   // TODO `\` should probably not be escaped after line break
-   "foo  \nbar",
-   "foo\n\\bar",
-   },
-   {
-   "foo\n>bar",
-   // TODO `>` should probably not be escaped after line break
-   "foo  \n\\>bar",
-   "foo\n>bar",
-   },
-   {
-   "foo\n#bar",
-   "foo  \n#bar",
-   "foo\n#bar",
-   },
-   {
-   "foo  \nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
-   {
-   "foo\\\nbar",
-   "foo  \nbar",
-   "foo\nbar",
-   },
{
"foo\nbar",
"foo bar",
@@ -1989,7 +1941,7 @@
 
   for (const auto  : Cases) {
 markup::Document Output;
-parseLineBreaks(C.Documentation, Output);
+parseDoucmentation(C.Documentation, Output);
 
 EXPECT_EQ(Output.asMarkdown(), C.ExpectedRenderMarkdown);
 EXPECT_EQ(Output.asPlainText(), C.ExpectedRenderPlainText);
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -75,7 +75,7 @@
   markup::Document present() const;
 };
 
-void parseLineBreaks(llvm::StringRef Input, markup::Document );
+void parseDoucmentation(llvm::StringRef Input, markup::Document );
 
 llvm::raw_ostream <<(llvm::raw_ostream &, const HoverInfo::Param &);
 inline bool operator==(const HoverInfo::Param ,
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -522,26 +522,15 @@
 }
 
 bool isParagraphLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
-  if (LineBreakIndex 

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

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@craig.topper that would mean that it couldn't be constexpr though, right?


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] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 252147.
ZarkoCA added a comment.

Simplified testcases.
Added testcases for variadic arguments being passed directly to the stack when 
all registers are used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76130

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
  llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll

Index: llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
@@ -0,0 +1,224 @@
+; RUN: llc -O2 -mtriple powerpc64-ibm-aix-xcoff -stop-after=machine-cp -verify-machineinstrs < %s | \
+; RUN: FileCheck --check-prefixes=CHECK,64BIT %s
+
+; RUN: llc -O2 -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec \
+; RUN:  -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefixes=CHECKASM,ASM64PWR4 %s
+  
+  target datalayout = "E-m:e-i64:64-n32:64"
+  target triple = "powerpc64-ibm-aix-xcoff"
+
+  define i32 @int_va_arg(i32 %a, ...) local_unnamed_addr  {
+  entry:
+%arg = alloca i8*, align 8
+%0 = bitcast i8** %arg to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %0, i8* nonnull %0)
+%1 = va_arg i8** %arg, i32
+%add = add nsw i32 %1, %a
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+ret i32 %add
+  }
+
+  declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+  declare void @llvm.va_start(i8*)
+  declare void @llvm.va_copy(i8*, i8*)
+  declare void @llvm.va_end(i8*)
+  declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+  define i32 @int_stack_va_arg(i32 %one, i32 %two, i32 %three, i32 %four, i32 %five, i32 %six, i32 %seven, i32 %eight, ...) local_unnamed_addr {
+  entry:
+%arg = alloca i8*, align 8
+%0 = bitcast i8** %arg to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+call void @llvm.va_start(i8* nonnull %0)
+%add = add nsw i32 %two, %one
+%add3 = add nsw i32 %add, %three
+%add4 = add nsw i32 %add3, %four
+%add5 = add nsw i32 %add4, %five
+%add6 = add nsw i32 %add5, %six
+%add7 = add nsw i32 %add6, %seven
+%add8 = add nsw i32 %add7, %eight
+%1 = va_arg i8** %arg, i32
+%add9 = add nsw i32 %add8, %1
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+ret i32 %add9
+  }
+
+  define double @double_va_arg(double %a, ...) local_unnamed_addr {
+  entry:
+%arg = alloca i8*, align 8
+%0 = bitcast i8** %arg to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %0, i8* nonnull %0)
+%1 = va_arg i8** %arg, double
+%add = fadd double %1, %a
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+ret double %add
+  }
+
+  define double @double_stack_va_arg(double %one, double %two, double %three, double %four, double %five, double %six, double %seven, double %eight, double %nine, double %ten, double %eleven, double %twelve, double %thirtheen, ...) local_unnamed_addr {
+  entry:
+%arg = alloca i8*, align 8
+%0 = bitcast i8** %arg to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+call void @llvm.va_start(i8* nonnull %0)
+%add = fadd double %one, %two
+%add3 = fadd double %add, %three
+%add4 = fadd double %add3, %four
+%add5 = fadd double %add4, %five
+%add6 = fadd double %add5, %six
+%add7 = fadd double %add6, %seven
+%add8 = fadd double %add7, %eight
+%1 = va_arg i8** %arg, double
+%add9 = fadd double %add8, %1
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+ret double %add9
+  }
+
+  declare void @llvm.stackprotector(i8*, i8**)
+
+; CHECK-LABEL:   name:int_va_arg
+; CHECK-LABEL:   liveins:
+; 64BIT-NEXT:- { reg: '$x3', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x4', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x5', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x6', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x7', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x8', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x9', virtual-reg: '' }
+; 64BIT-NEXT:- { reg: '$x10', virtual-reg: '' }
+
+; CHECK-LABEL:   fixedStack:
+; 64BIT: - { id: 0, type: default, offset: 56, size: 8
+; CHECK-LABEL:   body: |
+; 64BIT: liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+; 64BIT-DAG: STD killed renamable $x4, 0, %fixed-stack.0 :: (store 8 into %fixed-stack.0)
+; 64BIT-DAG: STD killed renamable $x5, 8, %fixed-stack.0 :: (store 8 into %fixed-stack.0 + 8)
+; 

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Not sure what sort of test would catch this; it's not crashing, and the types 
with the tail padding issue are not naturally exposed by target intrinsic 
headers (so it would only show up for code using ext_vector_type, or maybe 
OpenCL code).  I only tripped over the issue by inspection.

The way LLVM currently works, vectors never have internal padding, only tail 
padding.  (This could possibly change in the future, I guess, but there aren't 
any specific plans.)

For the types clang can generate, tail padding can show up a few different ways:

1. A vector can have an element count that's not a power of two: <5 x double>
2. A vector can partially cover a byte: <4 x i1>
3. A vector can have an element type that naturally has padding, like x86 long 
double

I can write testcases for the above, if you would find it helpful.  (Actually 
figuring out how to pad a vector correctly is more work than I really want to 
invest into this right now.  In the tricky cases, it might make sense to 
convert the constant to an integer, and pad that.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76528



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

[clang] 502915c - PR45142: 'template ~X' is ill-formed; reject it rather than crashing.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-23T15:07:06-07:00
New Revision: 502915c619a32972ddc525be585794371bfbd27b

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

LOG: PR45142: 'template ~X' is ill-formed; reject it rather than crashing.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/lib/Parse/ParseExprCXX.cpp
clang/test/CXX/drs/dr4xx.cpp
clang/test/SemaCXX/pseudo-destructors.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index acb229225ea1..dcbb0d3f8799 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -706,6 +706,8 @@ def err_id_after_template_in_nested_name_spec : Error<
   "expected template name after 'template' keyword in nested name specifier">;
 def err_unexpected_template_in_unqualified_id : Error<
   "'template' keyword not permitted here">;
+def err_unexpected_template_in_destructor_name : Error<
+  "'template' keyword not permitted in destructor name">;
 def err_unexpected_template_after_using : Error<
   "'template' keyword not permitted after 'using' keyword">;
 def err_two_right_angle_brackets_need_space : Error<

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index b8d91c19228f..a0b97ea7514d 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -2884,6 +2884,22 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec , 
ParsedType ObjectType,
 // Parse the '~'.
 SourceLocation TildeLoc = ConsumeToken();
 
+if (TemplateSpecified) {
+  // C++ [temp.names]p3:
+  //   A name prefixed by the keyword template shall be a template-id [...]
+  //
+  // A template-id cannot begin with a '~' token. This would never work
+  // anyway: x.~A() would specify that the destructor is a template,
+  // not that 'A' is a template.
+  //
+  // FIXME: Suggest replacing the attempted destructor name with a correct
+  // destructor name and recover. (This is not trivial if this would become
+  // a pseudo-destructor name).
+  Diag(*TemplateKWLoc, diag::err_unexpected_template_in_destructor_name)
+<< Tok.getLocation();
+  return true;
+}
+
 if (SS.isEmpty() && Tok.is(tok::kw_decltype)) {
   DeclSpec DS(AttrFactory);
   SourceLocation EndLoc = ParseDecltypeSpecifier(DS);
@@ -2903,7 +2919,7 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec , 
ParsedType ObjectType,
 
 // If the user wrote ~T::T, correct it to T::~T.
 DeclaratorScopeObj DeclScopeObj(*this, SS);
-if (!TemplateSpecified && NextToken().is(tok::coloncolon)) {
+if (NextToken().is(tok::coloncolon)) {
   // Don't let ParseOptionalCXXScopeSpecifier() "correct"
   // `int A; struct { ~A::A(); };` to `int A; struct { ~A:A(); };`,
   // it will confuse this recovery logic.

diff  --git a/clang/test/CXX/drs/dr4xx.cpp b/clang/test/CXX/drs/dr4xx.cpp
index 35fd15f0cc64..2c762237037d 100644
--- a/clang/test/CXX/drs/dr4xx.cpp
+++ b/clang/test/CXX/drs/dr4xx.cpp
@@ -297,13 +297,11 @@ namespace dr420 { // dr420: yes
   void test2(T p) {
 p->template Y::~Y();
 p->~Y();
-// FIXME: This is ill-formed, but this diagnostic is terrible. We should
-// reject this in the parser.
-p->template ~Y(); // expected-error 2{{no member named '~typename 
Y'}}
+p->template ~Y(); // expected-error {{'template' keyword not 
permitted in destructor name}}
   }
   template struct Y {};
-  template void test2(Y*); // expected-note {{instantiation}}
-  template void test2(ptr >); // expected-note {{instantiation}}
+  template void test2(Y*);
+  template void test2(ptr >);
 
   void test3(int *p, ptr q) {
 typedef int Int;

diff  --git a/clang/test/SemaCXX/pseudo-destructors.cpp 
b/clang/test/SemaCXX/pseudo-destructors.cpp
index 0cd139047432..b71b523de683 100644
--- a/clang/test/SemaCXX/pseudo-destructors.cpp
+++ b/clang/test/SemaCXX/pseudo-destructors.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -emit-llvm-only -verify -std=c++11 %s
 struct A {};
 
 enum Foo { F };
@@ -92,6 +92,9 @@ namespace PR11339 {
 template using Id = T;
 void AliasTemplate(int *p) {
   p->~Id();
+  p->template ~Id(); // expected-error {{'template' keyword not permitted 
in destructor name}}
+  (0).~Id();
+  (0).template ~Id(); // expected-error {{'template' keyword not 
permitted in destructor name}}
 }
 
 namespace dotPointerAccess {



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

[PATCH] D62627: [NFC] Do not run CGProfilePass when not using integrated assembler

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 252143.
zhizhouy marked 4 inline comments as done.
zhizhouy retitled this revision from "[NFC] Do not run CGProfilePass when 
-fno-integrated-as is on" to "[NFC] Do not run CGProfilePass when not using 
integrated assembler".
zhizhouy added a comment.

Thanks for the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62627

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/new-pm-cgprofile.ll

Index: llvm/test/Other/new-pm-cgprofile.ll
===
--- /dev/null
+++ llvm/test/Other/new-pm-cgprofile.ll
@@ -0,0 +1,11 @@
+; RUN: opt -debug-pass-manager -passes='default' %s 2>&1 |FileCheck %s --check-prefixes=DEFAULT
+; RUN: opt -debug-pass-manager -passes='default' -enable-npm-call-graph-profile=0 %s 2>&1 |FileCheck %s --check-prefixes=OFF
+; RUN: opt -debug-pass-manager -passes='default' -enable-npm-call-graph-profile=1 %s 2>&1 |FileCheck %s --check-prefixes=ON
+;
+; DEFAULT: Running pass: CGProfilePass
+; OFF-NOT: Running pass: CGProfilePass
+; ON: Running pass: CGProfilePass
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -237,6 +237,10 @@
 EnableCHR("enable-chr-npm", cl::init(true), cl::Hidden,
   cl::desc("Enable control height reduction optimization (CHR)"));
 
+static cl::opt EnableCallGraphProfile(
+"enable-npm-call-graph-profile", cl::init(true), cl::Hidden,
+cl::desc("Enable call graph profile pass for the new PM (default = on)"));
+
 PipelineTuningOptions::PipelineTuningOptions() {
   LoopInterleaving = EnableLoopInterleaving;
   LoopVectorization = EnableLoopVectorization;
@@ -246,6 +250,7 @@
   Coroutines = false;
   LicmMssaOptCap = SetLicmMssaOptCap;
   LicmMssaNoAccForPromotionCap = SetLicmMssaNoAccForPromotionCap;
+  CallGraphProfile = EnableCallGraphProfile;
 }
 
 extern cl::opt EnableHotColdSplit;
@@ -1060,7 +1065,8 @@
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
-  MPM.addPass(CGProfilePass());
+  if (PTO.CallGraphProfile)
+MPM.addPass(CGProfilePass());
 
   // Now we need to do some global optimization transforms.
   // FIXME: It would seem like these should come first in the optimization
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -105,6 +105,10 @@
   /// Tuning option to disable promotion to scalars in LICM with MemorySSA, if
   /// the number of access is too large.
   unsigned LicmMssaNoAccForPromotionCap;
+
+  /// Tuning option to enable/disable call graph profile. Its default value is
+  /// that of the flag: `-enable-npm-call-graph-profile`.
+  bool CallGraphProfile;
 };
 
 /// This class provides access to building LLVM's passes.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -823,6 +823,7 @@
   Opts.RerollLoops = Args.hasArg(OPT_freroll_loops);
 
   Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
+  Opts.CallGraphProfile = !Opts.DisableIntegratedAS;
   Opts.Autolink = !Args.hasArg(OPT_fno_autolink);
   Opts.SampleProfileFile =
   std::string(Args.getLastArgValue(OPT_fprofile_sample_use_EQ));
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1107,6 +1107,7 @@
   PTO.LoopInterleaving = CodeGenOpts.UnrollLoops;
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
+  PTO.CallGraphProfile = CodeGenOpts.CallGraphProfile;
   PTO.Coroutines = LangOpts.Coroutines;
 
   PassInstrumentationCallbacks PIC;
@@ -1514,6 +1515,7 @@
   Conf.PTO.LoopInterleaving = CGOpts.UnrollLoops;
   Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
   Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
+  Conf.PTO.CallGraphProfile = CGOpts.CallGraphProfile;
 
   // Context sensitive profile.
   if (CGOpts.hasProfileCSIRInstr()) {
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -254,6 +254,7 @@
 CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP  , 1, 0) 

[PATCH] D62627: [NFC] Do not run CGProfilePass when not using integrated assembler

[Rong Xu via Phabricator via cfe-commits]

xur added a comment.

look good to me with the fix of comments.




Comment at: llvm/include/llvm/Passes/PassBuilder.h:110
+  /// Tuning option to enable/disable call graph profile. Its default value is
+  /// true.
+  bool CallGrpahProfile;

The default value is specified by -enable-npm-call-graph-profile, rather than 
true, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62627



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

Re: [clang] 43606ef - Suppress an "unused variable" warning in release build

[David Blaikie via cfe-commits]

Ah, thanks. (I'm going to see about renaming/restructuring that API - to
return Optional and be called 'get' rather than 'is' to make this
more clear)

On Mon, Mar 23, 2020 at 5:04 AM Mikhail Maltsev 
wrote:

> Yes, it does. isIntegerConstantExpr assigns a value to CoprocNoAP.
>
> --
> Regards,
>Mikhail Maltsev
>
> 
> From: David Blaikie 
> Sent: Sunday, March 22, 2020 03:32
> To: Mikhail Maltsev; Mikhail Maltsev
> Cc: cfe-commits
> Subject: Re: [clang] 43606ef - Suppress an "unused variable" warning in
> release build
>
> Does "isIntegerConstantExpr" have side effects that are desired/necessary?
> Otherwise please change this to roll the isIntegerConstantExpr into the
> assert (so that it is only executed when asserts are enabled)
>
> On Tue, Mar 10, 2020 at 10:11 AM Mikhail Maltsev via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: Mikhail Maltsev
> Date: 2020-03-10T17:10:52Z
> New Revision: 43606efb6847fc9c79e7d93760a2a6191e8a8539
>
> URL:
> https://github.com/llvm/llvm-project/commit/43606efb6847fc9c79e7d93760a2a6191e8a8539
> DIFF:
> https://github.com/llvm/llvm-project/commit/43606efb6847fc9c79e7d93760a2a6191e8a8539.diff
>
> LOG: Suppress an "unused variable" warning in release build
>
> Added:
>
>
> Modified:
> clang/lib/Sema/SemaChecking.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Sema/SemaChecking.cpp
> b/clang/lib/Sema/SemaChecking.cpp
> index 24d0d9209a1d..8a2b4b019663 100644
> --- a/clang/lib/Sema/SemaChecking.cpp
> +++ b/clang/lib/Sema/SemaChecking.cpp
> @@ -2094,6 +2094,7 @@ bool Sema::CheckARMCoprocessorImmediate(const Expr
> *CoprocArg, bool WantCDE) {
>
>llvm::APSInt CoprocNoAP;
>bool IsICE = CoprocArg->isIntegerConstantExpr(CoprocNoAP, Context);
> +  (void)IsICE;
>assert(IsICE && "Coprocossor immediate is not a constant expression");
>int64_t CoprocNo = CoprocNoAP.getExtValue();
>assert(CoprocNo >= 0 && "Coprocessor immediate must be non-negative");
>
>
>
> ___
> 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] D76622: [analyzer] ConstraintManager - use EXPENSIVE_CHECKS instead of (gcc specific) __OPTIMIZE__ guard

[Simon Pilgrim via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a4421a5e860: [analyzer] ConstraintManager - use 
EXPENSIVE_CHECKS instead of (gcc specific)… (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76622

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -96,11 +96,7 @@
 // If StTrue is infeasible, asserting the falseness of Cond is unnecessary
 // because the existing constraints already establish this.
 if (!StTrue) {
-#ifndef __OPTIMIZE__
-  // This check is expensive and should be disabled even in Release+Asserts
-  // builds.
-  // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC
-  // does not. Is there a good equivalent there?
+#ifdef EXPENSIVE_CHECKS
   assert(assume(State, Cond, false) && "System is over constrained.");
 #endif
   return ProgramStatePair((ProgramStateRef)nullptr, State);


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -96,11 +96,7 @@
 // If StTrue is infeasible, asserting the falseness of Cond is unnecessary
 // because the existing constraints already establish this.
 if (!StTrue) {
-#ifndef __OPTIMIZE__
-  // This check is expensive and should be disabled even in Release+Asserts
-  // builds.
-  // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC
-  // does not. Is there a good equivalent there?
+#ifdef EXPENSIVE_CHECKS
   assert(assume(State, Cond, false) && "System is over constrained.");
 #endif
   return ProgramStatePair((ProgramStateRef)nullptr, State);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

[Alexandre Ganea via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5896e2df45da: [Clang] Fix HIP tests when running on Windows 
with the LLVM toolchain is in the… (authored by aganea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76631

Files:
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-rdc.hip


Index: clang/test/Driver/hip-toolchain-rdc.hip
===
--- clang/test/Driver/hip-toolchain-rdc.hip
+++ clang/test/Driver/hip-toolchain-rdc.hip
@@ -44,7 +44,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV1:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV1:.*out]]" [[OBJ_DEV1]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -38,7 +38,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 //
@@ -67,7 +67,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 //
@@ -112,7 +112,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_803:.*out]]" [[OBJ_DEV_B_803]]
 
 //
@@ -141,7 +141,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_900:.*out]]" [[OBJ_DEV_B_900]]
 
 //
Index: clang/test/Driver/hip-device-compile.hip
===
--- clang/test/Driver/hip-device-compile.hip
+++ clang/test/Driver/hip-device-compile.hip
@@ -42,7 +42,7 @@
 // CHECK-NOT: {{"*.llvm-link"}}
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
-// CHECK-NOT: {{".*lld"}}
+// CHECK-NOT: {{".*lld.*"}}
 // CHECK-NOT: {{".*clang-offload-bundler"}}
 // CHECK-NOT: {{".*ld.*"}}
 
@@ -67,6 +67,6 @@
 // BUNDLE: {{"*.llvm-link"}}
 // BUNDLE: {{".*opt"}}
 // BUNDLE: {{".*llc"}}
-// BUNDLE: {{".*lld"}}
+// BUNDLE: {{".*lld.*"}}
 // BUNDLE: {{".*clang-offload-bundler"}}
 


Index: clang/test/Driver/hip-toolchain-rdc.hip
===
--- clang/test/Driver/hip-toolchain-rdc.hip
+++ clang/test/Driver/hip-toolchain-rdc.hip
@@ -44,7 +44,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV1:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV1:.*out]]" [[OBJ_DEV1]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -38,7 +38,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 //
@@ -67,7 +67,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 //
@@ -112,7 +112,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_803:.*out]]" [[OBJ_DEV_B_803]]
 
 //
@@ -141,7 +141,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_900:.*out]]" [[OBJ_DEV_B_900]]
 
 //
Index: clang/test/Driver/hip-device-compile.hip
===
--- 

[PATCH] D76509: [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

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

This revision was automatically updated to reflect the committed changes.
Szelethus marked 6 inline comments as done.
Closed by commit rG7bf871c39f73: [analyzer][NFC] Move the text output type to 
its own file, move code to… (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D76509?vs=251973=252138#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76509

Files:
  clang/include/clang/StaticAnalyzer/Core/Analyses.def
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

Index: clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
@@ -21,6 +21,4 @@
   clangLex
   clangStaticAnalyzerCheckers
   clangStaticAnalyzerCore
-  clangRewrite
-  clangToolingCore
   )
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -65,126 +65,6 @@
 STATISTIC(MaxCFGSize, "The maximum number of basic blocks in a function.");
 
 //===--===//
-// Special PathDiagnosticConsumers.
-//===--===//
-
-void ento::createPlistHTMLDiagnosticConsumer(
-AnalyzerOptions , PathDiagnosticConsumers ,
-const std::string , const Preprocessor ,
-const cross_tu::CrossTranslationUnitContext ) {
-  createHTMLDiagnosticConsumer(
-  AnalyzerOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
-  CTU);
-  createPlistMultiFileDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
-}
-
-void ento::createTextPathDiagnosticConsumer(
-AnalyzerOptions , PathDiagnosticConsumers ,
-const std::string , const clang::Preprocessor ,
-const cross_tu::CrossTranslationUnitContext ) {
-  llvm_unreachable("'text' consumer should be enabled on ClangDiags");
-}
-
-namespace {
-class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
-  DiagnosticsEngine 
-  LangOptions LO;
-
-  bool IncludePath = false;
-  bool ShouldEmitAsError = false;
-  bool ApplyFixIts = false;
-
-public:
-  ClangDiagPathDiagConsumer(DiagnosticsEngine , LangOptions LO)
-  : Diag(Diag), LO(LO) {}
-  ~ClangDiagPathDiagConsumer() override {}
-  StringRef getName() const override { return "ClangDiags"; }
-
-  bool supportsLogicalOpControlFlow() const override { return true; }
-  bool supportsCrossFileDiagnostics() const override { return true; }
-
-  PathGenerationScheme getGenerationScheme() const override {
-return IncludePath ? Minimal : None;
-  }
-
-  void enablePaths() { IncludePath = true; }
-  void enableWerror() { ShouldEmitAsError = true; }
-  void enableApplyFixIts() { ApplyFixIts = true; }
-
-  void FlushDiagnosticsImpl(std::vector ,
-FilesMade *filesMade) override {
-unsigned WarnID =
-ShouldEmitAsError
-? Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0")
-: Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
-unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
-SourceManager  = Diag.getSourceManager();
-
-Replacements Repls;
-auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
-   ArrayRef Ranges,
-   ArrayRef Fixits) {
-  if (!ApplyFixIts) {
-Diag.Report(Loc, ID) << String << Ranges << Fixits;
-return;
-  }
-
-  Diag.Report(Loc, ID) << String << Ranges;
-  for (const FixItHint  : Fixits) {
-Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
-
-if (llvm::Error Err = Repls.add(Repl)) {
-  llvm::errs() << "Error applying replacement " << Repl.toString()
-   << ": " << Err << "\n";
-}
-  }
-};
-
-for (std::vector::iterator I = Diags.begin(),
- E = Diags.end();
- I != E; ++I) {
-  const PathDiagnostic *PD = *I;
-  reportPiece(WarnID, PD->getLocation().asLocation(),
-  PD->getShortDescription(), PD->path.back()->getRanges(),
-  PD->path.back()->getFixits());
-
-  // First, add extra notes, even if paths should not be included.
-  for (const auto  : PD->path) {
-if (!isa(Piece.get()))
-  continue;
-
-reportPiece(NoteID, Piece->getLocation().asLocation(),
-   

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D76631#1937681 , @aganea wrote:

> In D76631#1937428 , @yaxunl wrote:
>
> > I am curious why opt and llc is not affected
>
>
> In one case (opt, llc, clang-offload-bundler) it finds those programs in the 
> "program paths", ie. the build folder: 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L4733
>
> Whereas in another cases (lld) when it doesn't find the program in the 
> "program paths", it will go search the env.var. %PATH% and fall back to a 
> platform-specific search, which includes searching using 
> `{program_name}.exe`: 
> https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Windows/Program.inc#L65
>
> This is the output I was getting:
>
>   
> D:\llvm-project\buildninjaRel\tools\clang\test\Driver>"d:\llvm-project\buildninjarel\bin\clang.exe"
>  "-c" "--cuda-device-only" "-###" "-target" "x86_64-linux-gnu" "-o" "a.s" 
> "-x" "hip" "--cuda-gpu-arch=gfx900" "--hip-device-lib=lib1.bc" 
> "--hip-device-lib-path=D:\llvm-project\clang\test\Driver/Inputs/hip_multiple_inputs/lib1"
>  "D:\llvm-project\clang\test\Driver/Inputs/hip_multiple_inputs/a.cu"
>   clang version 11.0.0 (https://github.com/llvm/llvm-project.git 
> c1f8595fe5b856222418e2de547f0e346d84ac84)
>   Target: x86_64-unknown-linux-gnu
>   Thread model: posix
>   InstalledDir: d:\llvm-project\buildninjarel\bin
>"d:\\llvm-project\\buildninjarel\\bin\\clang.exe" "-cc1" "-triple" 
> "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc" 
> "-emit-llvm-uselists" "-disable-free" "-disable-llvm-verifier" 
> "-discard-value-names" "-main-file-name" "a.cu" "-mrelocation-model" "pic" 
> "-pic-level" "1" "-mthread-model" "posix" "-mframe-pointer=all" 
> "-fno-rounding-math" "-mconstructor-aliases" "-aux-target-cpu" "x86-64" 
> "-target-cpu" "gfx900" "-fcuda-is-device" "-fcuda-allow-variadic-functions" 
> "-fvisibility" "hidden" "-fapply-global-visibility-to-externs" 
> "-mlink-builtin-bitcode" 
> "D:\\llvm-project\\clang\\test\\Driver/Inputs/hip_multiple_inputs/lib1\\lib1.bc"
>  "-dwarf-column-info" "-fno-split-dwarf-inlining" "-debugger-tuning=gdb" 
> "-resource-dir" "d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0" 
> "-internal-isystem" "/usr/local/include" "-internal-isystem" 
> "d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0\\include" 
> "-internal-externc-isystem" "/include" "-internal-externc-isystem" 
> "/usr/include" "-internal-isystem" "/usr/local/include" "-internal-isystem" 
> "d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0\\include" 
> "-internal-externc-isystem" "/include" "-internal-externc-isystem" 
> "/usr/include" "-fdeprecated-macro" "-fno-autolink" "-fdebug-compilation-dir" 
> "D:\\llvm-project\\buildninjaRel\\tools\\clang\\test\\Driver" "-ferror-limit" 
> "19" "-fmessage-length" "138" "-fgnuc-version=4.2.1" "-fobjc-runtime=gcc" 
> "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" 
> "-fcolor-diagnostics" "-fcuda-allow-variadic-functions" "-faddrsig" "-o" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940.bc" "-x" "hip" 
> "D:\\llvm-project\\clang\\test\\Driver/Inputs/hip_multiple_inputs/a.cu"
>"d:\\llvm-project\\buildninjarel\\bin\\llvm-link" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940.bc" "-o" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-linked-fa625c.bc"
>"d:\\llvm-project\\buildninjarel\\bin\\opt" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-linked-fa625c.bc" 
> "-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx900" "-o" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-optimized-227a21.bc"
>"d:\\llvm-project\\buildninjarel\\bin\\llc" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-optimized-227a21.bc"
>  "-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx900" "-filetype=obj" "-o" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-03076f.o"
>"C:\\Program Files\\LLVM\\bin\\lld.exe" "-flavor" "gnu" "-shared" "-o" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-630562.out" 
> "C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-03076f.o"
>"d:\\llvm-project\\buildninjarel\\bin\\clang-offload-bundler" "-type=o" 
> "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa-gfx900" 
> "-inputs=nul,C:\\Users\\aganea\\AppData\\Local\\Temp\\a-630562.out" 
> "-outputs=a.s"
>
>
> If LLD is built at the same time as Clang, this error doesn't happen.


Thanks for the explanation. It makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76631



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

[PATCH] D76472: AMDGPU: Emit llvm.fshr for __builtin_amdgcn_alignbit

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

3f533006ba8c8ae6f3596f49f480aa794ed4e347 



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

https://reviews.llvm.org/D76472



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

[PATCH] D62627: [NFC] Do not run CGProfilePass when -fno-integrated-as is on

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Thanks,

Noticed a few typos. Rest lgtm but deferring to other reviewers for now for 
approval.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1110
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
+  PTO.CallGrpahProfile = CodeGenOpts.CallGraphProfile;
   PTO.Coroutines = LangOpts.Coroutines;

Typo: PTO.CallGrpahProfile -> PTO.CallGraphProfile



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1518
   Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
+  Conf.PTO.CallGrpahProfile = CGOpts.CallGraphProfile;
 

Same typo.



Comment at: llvm/include/llvm/Passes/PassBuilder.h:111
+  /// true.
+  bool CallGrpahProfile;
 };

CallGrpahProfile -> CallGraphProfile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62627



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

[clang] 1a4421a - [analyzer] ConstraintManager - use EXPENSIVE_CHECKS instead of (gcc specific) __OPTIMIZE__ guard

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-03-23T21:03:14Z
New Revision: 1a4421a5e860ffc63c77594c9fb40787f84241aa

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

LOG: [analyzer] ConstraintManager - use EXPENSIVE_CHECKS instead of (gcc 
specific) __OPTIMIZE__ guard

This was noticed on D71817, which removed another use of __OPTIMIZE__

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
index f85c37379158..935b2bb7b937 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -96,11 +96,7 @@ class ConstraintManager {
 // If StTrue is infeasible, asserting the falseness of Cond is unnecessary
 // because the existing constraints already establish this.
 if (!StTrue) {
-#ifndef __OPTIMIZE__
-  // This check is expensive and should be disabled even in Release+Asserts
-  // builds.
-  // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC
-  // does not. Is there a good equivalent there?
+#ifdef EXPENSIVE_CHECKS
   assert(assume(State, Cond, false) && "System is over constrained.");
 #endif
   return ProgramStatePair((ProgramStateRef)nullptr, State);



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

[clang] 5896e2d - [Clang] Fix HIP tests when running on Windows with the LLVM toolchain is in the path

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2020-03-23T16:54:48-04:00
New Revision: 5896e2df45dab7680be321ee63903431a673817b

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

LOG: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain is 
in the path

On Windows, when the LLVM toolchain is in the current path (%PATH%), fusing the 
linker yields c:\{path}\lld.exe whereas the hip tests did not expect the .exe 
part.
This only happens if LLD is not present in the build folder, which consequently 
makes the code in Driver::GetProgramPath() to fall back to %PATH% and 
platform-specific search, which includes the .exe part (see 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L4733).

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

Added: 


Modified: 
clang/test/Driver/hip-device-compile.hip
clang/test/Driver/hip-toolchain-no-rdc.hip
clang/test/Driver/hip-toolchain-rdc.hip

Removed: 




diff  --git a/clang/test/Driver/hip-device-compile.hip 
b/clang/test/Driver/hip-device-compile.hip
index b6f1f1cf218e..c442d23581ce 100644
--- a/clang/test/Driver/hip-device-compile.hip
+++ b/clang/test/Driver/hip-device-compile.hip
@@ -42,7 +42,7 @@
 // CHECK-NOT: {{"*.llvm-link"}}
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
-// CHECK-NOT: {{".*lld"}}
+// CHECK-NOT: {{".*lld.*"}}
 // CHECK-NOT: {{".*clang-offload-bundler"}}
 // CHECK-NOT: {{".*ld.*"}}
 
@@ -67,6 +67,6 @@
 // BUNDLE: {{"*.llvm-link"}}
 // BUNDLE: {{".*opt"}}
 // BUNDLE: {{".*llc"}}
-// BUNDLE: {{".*lld"}}
+// BUNDLE: {{".*lld.*"}}
 // BUNDLE: {{".*clang-offload-bundler"}}
 

diff  --git a/clang/test/Driver/hip-toolchain-no-rdc.hip 
b/clang/test/Driver/hip-toolchain-no-rdc.hip
index cda852e2d8c7..4371334577f7 100644
--- a/clang/test/Driver/hip-toolchain-no-rdc.hip
+++ b/clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -38,7 +38,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 //
@@ -67,7 +67,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 //
@@ -112,7 +112,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_803:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_803:.*out]]" [[OBJ_DEV_B_803]]
 
 //
@@ -141,7 +141,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV_B_900:".*-gfx900-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_900:.*out]]" [[OBJ_DEV_B_900]]
 
 //

diff  --git a/clang/test/Driver/hip-toolchain-rdc.hip 
b/clang/test/Driver/hip-toolchain-rdc.hip
index 18fd1d7b09a0..203784f2ab43 100644
--- a/clang/test/Driver/hip-toolchain-rdc.hip
+++ b/clang/test/Driver/hip-toolchain-rdc.hip
@@ -44,7 +44,7 @@
 // CHECK-SAME: "-filetype=obj"
 // CHECK-SAME: "-o" [[OBJ_DEV1:".*-gfx803-.*o"]]
 
-// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV1:.*out]]" [[OBJ_DEV1]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"



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

[clang] 3f53300 - AMDGPU: Emit llvm.fshr for __builtin_amdgcn_alignbit

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2020-03-23T16:51:25-04:00
New Revision: 3f533006ba8c8ae6f3596f49f480aa794ed4e347

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

LOG: AMDGPU: Emit llvm.fshr for __builtin_amdgcn_alignbit

These are equivalent. The generic rotate builtins do not directly map
to the fshr intrinsic.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGenOpenCL/builtins-amdgcn.cl
llvm/include/llvm/IR/IntrinsicsAMDGPU.td

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 8d86424c60a9..754d95d1ab81 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13609,6 +13609,13 @@ Value *CodeGenFunction::EmitAMDGPUBuiltinExpr(unsigned 
BuiltinID,
 return emitRangedBuiltin(*this, Intrinsic::r600_read_tidig_y, 0, 1024);
   case AMDGPU::BI__builtin_r600_read_tidig_z:
 return emitRangedBuiltin(*this, Intrinsic::r600_read_tidig_z, 0, 1024);
+  case AMDGPU::BI__builtin_amdgcn_alignbit: {
+llvm::Value *Src0 = EmitScalarExpr(E->getArg(0));
+llvm::Value *Src1 = EmitScalarExpr(E->getArg(1));
+llvm::Value *Src2 = EmitScalarExpr(E->getArg(2));
+Function *F = CGM.getIntrinsic(Intrinsic::fshr, Src0->getType());
+return Builder.CreateCall(F, { Src0, Src1, Src2 });
+  }
   default:
 return nullptr;
   }

diff  --git a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl 
b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
index 85e921cbe12a..0aa3e4144c52 100644
--- a/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ b/clang/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -596,7 +596,7 @@ kernel void test_mbcnt_hi(global uint* out, uint src0, uint 
src1) {
 }
 
 // CHECK-LABEL: @test_alignbit(
-// CHECK: tail call i32 @llvm.amdgcn.alignbit(i32 %src0, i32 %src1, i32 %src2)
+// CHECK: tail call i32 @llvm.fshr.i32(i32 %src0, i32 %src1, i32 %src2)
 kernel void test_alignbit(global uint* out, uint src0, uint src1, uint src2) {
   *out = __builtin_amdgcn_alignbit(src0, src1, src2);
 }

diff  --git a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td 
b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
index 3f962cc667c5..c01db52b1622 100644
--- a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -1371,8 +1371,8 @@ def int_amdgcn_writelane :
   [IntrNoMem, IntrConvergent]
 >;
 
-def int_amdgcn_alignbit :
-  GCCBuiltin<"__builtin_amdgcn_alignbit">, Intrinsic<[llvm_i32_ty],
+// FIXME: Deprecated. This is equivalent to llvm.fshr
+def int_amdgcn_alignbit : Intrinsic<[llvm_i32_ty],
   [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty],
   [IntrNoMem, IntrSpeculatable]
 >;



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

[clang] 7bf871c - [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-03-23T21:50:40+01:00
New Revision: 7bf871c39f739a7382ba89c97f50fd047f36f894

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

LOG: [analyzer][NFC] Move the text output type to its own file, move code to 
PathDiagnosticConsumer creator functions

TableGen and .def files (which are meant to be used with the preprocessor) come
with obvious downsides. One of those issues is that generated switch-case
branches have to be identical. This pushes corner cases either to an outer code
block, or into the generated code.

Inspect the removed code in AnalysisConsumer::DigestAnalyzerOptions. You can see
how corner cases like a not existing output file, the analysis output type being
set to PD_NONE, or whether to complement the output with additional diagnostics
on stderr lay around the preprocessor generated code. This is a bit problematic,
as to how to deal with such errors is not in the hands of the users of this
interface (those implementing output types, like PlistDiagnostics etc).

This patch changes this by moving these corner cases into the generated code,
more specifically, into the called functions. In addition, I introduced a new
output type for convenience purposes, PD_TEXT_MINIMAL, which always existed
conceptually, but never in the actual Analyses.def file. This refactoring
allowed me to move TextDiagnostics (renamed from ClangDiagPathDiagConsumer) to
its own file, which it really deserved.

Also, those that had the misfortune to gaze upon Analyses.def will probably
enjoy the sight that a clang-format did on it.

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

Added: 
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/Analyses.def
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/Analyses.def 
b/clang/include/clang/StaticAnalyzer/Core/Analyses.def
index 377451576148..c4e5f5be6fd7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Analyses.def
+++ b/clang/include/clang/StaticAnalyzer/Core/Analyses.def
@@ -14,41 +14,80 @@
 #define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store", 
CreateRegionStoreManager)
+ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store",
+   CreateRegionStoreManager)
 
 #ifndef ANALYSIS_CONSTRAINTS
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of 
concrete value ranges", CreateRangeConstraintManager)
-ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", 
CreateZ3ConstraintManager)
+ANALYSIS_CONSTRAINTS(RangeConstraints, "range",
+ "Use constraint tracking of concrete value ranges",
+ CreateRangeConstraintManager)
+
+ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver",
+ CreateZ3ConstraintManager)
 
 #ifndef ANALYSIS_DIAGNOSTICS
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
 #endif
 
-ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML", 
createHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "html-single-file", "Output analysis 
results using HTML (not allowing for multi-file bugs)", 
createHTMLSingleFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", 
createPlistDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis 
results using Plists (allowing for multi-file bugs)", 
createPlistMultiFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using 
HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF 
file", createSarifDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", 
createTextPathDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
+ createHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+HTML_SINGLE_FILE, "html-single-file",
+"Output analysis results using HTML (not allowing for multi-file bugs)",
+createHTMLSingleFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists",
+ 

[PATCH] D76472: AMDGPU: Emit llvm.fshr for __builtin_amdgcn_alignbit

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D76472



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

[PATCH] D62627: [NFC] Do not run CGProfilePass when -fno-integrated-as is on

[Zhizhou Yang via Phabricator via cfe-commits]

zhizhouy updated this revision to Diff 252129.
zhizhouy marked 2 inline comments as done.
zhizhouy edited the summary of this revision.
zhizhouy added a reviewer: manojgupta.
zhizhouy removed a subscriber: manojgupta.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62627

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/new-pm-cgprofile.ll

Index: llvm/test/Other/new-pm-cgprofile.ll
===
--- /dev/null
+++ llvm/test/Other/new-pm-cgprofile.ll
@@ -0,0 +1,11 @@
+; RUN: opt -debug-pass-manager -passes='default' %s 2>&1 |FileCheck %s --check-prefixes=DEFAULT
+; RUN: opt -debug-pass-manager -passes='default' -enable-npm-call-graph-profile=0 %s 2>&1 |FileCheck %s --check-prefixes=OFF
+; RUN: opt -debug-pass-manager -passes='default' -enable-npm-call-graph-profile=1 %s 2>&1 |FileCheck %s --check-prefixes=ON
+;
+; DEFAULT: Running pass: CGProfilePass
+; OFF-NOT: Running pass: CGProfilePass
+; ON: Running pass: CGProfilePass
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -237,6 +237,10 @@
 EnableCHR("enable-chr-npm", cl::init(true), cl::Hidden,
   cl::desc("Enable control height reduction optimization (CHR)"));
 
+static cl::opt EnableCallGraphProfile(
+"enable-npm-call-graph-profile", cl::init(true), cl::Hidden,
+cl::desc("Enable call graph profile pass for the new PM (default = on)"));
+
 PipelineTuningOptions::PipelineTuningOptions() {
   LoopInterleaving = EnableLoopInterleaving;
   LoopVectorization = EnableLoopVectorization;
@@ -246,6 +250,7 @@
   Coroutines = false;
   LicmMssaOptCap = SetLicmMssaOptCap;
   LicmMssaNoAccForPromotionCap = SetLicmMssaNoAccForPromotionCap;
+  CallGrpahProfile = EnableCallGraphProfile;
 }
 
 extern cl::opt EnableHotColdSplit;
@@ -1060,7 +1065,8 @@
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
-  MPM.addPass(CGProfilePass());
+  if (PTO.CallGrpahProfile)
+MPM.addPass(CGProfilePass());
 
   // Now we need to do some global optimization transforms.
   // FIXME: It would seem like these should come first in the optimization
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -105,6 +105,10 @@
   /// Tuning option to disable promotion to scalars in LICM with MemorySSA, if
   /// the number of access is too large.
   unsigned LicmMssaNoAccForPromotionCap;
+
+  /// Tuning option to enable/disable call graph profile. Its default value is
+  /// true.
+  bool CallGrpahProfile;
 };
 
 /// This class provides access to building LLVM's passes.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -823,6 +823,7 @@
   Opts.RerollLoops = Args.hasArg(OPT_freroll_loops);
 
   Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
+  Opts.CallGraphProfile = !Opts.DisableIntegratedAS;
   Opts.Autolink = !Args.hasArg(OPT_fno_autolink);
   Opts.SampleProfileFile =
   std::string(Args.getLastArgValue(OPT_fprofile_sample_use_EQ));
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1107,6 +1107,7 @@
   PTO.LoopInterleaving = CodeGenOpts.UnrollLoops;
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
+  PTO.CallGrpahProfile = CodeGenOpts.CallGraphProfile;
   PTO.Coroutines = LangOpts.Coroutines;
 
   PassInstrumentationCallbacks PIC;
@@ -1514,6 +1515,7 @@
   Conf.PTO.LoopInterleaving = CGOpts.UnrollLoops;
   Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
   Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
+  Conf.PTO.CallGrpahProfile = CGOpts.CallGraphProfile;
 
   // Context sensitive profile.
   if (CGOpts.hasProfileCSIRInstr()) {
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -254,6 +254,7 @@
 CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP  , 1, 0) ///< Run SLP vectorizer.
 CODEGENOPT(ProfileSampleAccurate, 1, 0) ///< 

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

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

rsmith added a comment.

In D76572#1937641 , @Quuxplusone wrote:

> [...] take my word for it, you **don't** want to start with the code I wrote 
> the other day. :)


Understood, thanks :) (FWIW, I had a stab at implementing essentially this last 
year and hit exactly the issues you highlighted.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



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

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:110-114
+case 26:
+  return 0;
+__attribute__((fallthrough)); // expected-warning{{fallthrough annotation 
in unreachable code}}
+case 27:
+  break;

tbaeder wrote:
> aaron.ballman wrote:
> > This test seems unrelated to the patch, or am I misunderstanding something?
> This is the test that prompted me to write this patch - before this patch, 
> the error message has an invalid loc. I guess I should remove this here and 
> move it over to the source ranges tests and also test that is has the right 
> source range.
Ah, yeah, you should probably add this to the source range tests. If the 
current test here would have failed without the patch, it's also perfectly 
reasonable to leave this test here as well.


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

https://reviews.llvm.org/D75844



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

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7727
 
+// Move function type attribute to the declarator.
+case ParsedAttr::AT_LifetimeContract:

xazax.hun wrote:
> aaron.ballman wrote:
> > xazax.hun wrote:
> > > aaron.ballman wrote:
> > > > This is not an okay thing to do for C++ attributes. They have a 
> > > > specific meaning as to what they appertain to and do not move around to 
> > > > better subjects like GNU-style attributes.
> > > Yeah, I know that. But this is problematic in the standard. See the 
> > > contracts proposal which also kind of violates this rule by adding an 
> > > exception. Basically, this is the poor man's version of emulating the 
> > > contracts proposal.  In the long term we plan to piggyback on contracts 
> > > rather than hacking the C++ attributes.
> > The contracts proposal was not adopted and I am opposed to adding this 
> > change for any standard attribute. We've done a lot of work to ensure that 
> > those attributes attach to the correct thing based on lexical placement and 
> > I don't want to start to undo that effort.
> I am not sure how to proceed in this case. As far as I understand this wasn't 
> the main reason why didn't we adopt the contracts proposal.
> 
> While I do understand why is it good to make the lexical placement matter, 
> but the current placement hinders non-trivial use-cases. Currently, we either 
> have a type attribute or we cannot mention any of the formal parameters. This 
> is why the contracts cannot work without changing attributes in the standard, 
> and the same reason why this patch will never work without doing this. Or do 
> you have an alternative in mind?
> 
> Do you think introducing a frontend flag for this attribute would help? This 
> would make it clear that we are currently using a non-standard feature that 
> needs to be enabled explicitly. Again, this is a temporary solution, that 
> will go away as soon as we have contracts. Since contracts need to solve the 
> same problem, whatever the solution will be, this patch can piggyback on that.
> 
> What do you think?
> I am not sure how to proceed in this case. As far as I understand this wasn't 
> the main reason why didn't we adopt the contracts proposal.

There were a lot of flaws, and this was one of them. I don't think you could 
point to any one flaw and say "this is why we didn't get contracts" though.

> Or do you have an alternative in mind?

I agree that this is an existing deficiency in both the C and C++ standards 
with the attribute syntax for functions. There simply is no way to make a 
function declaration attribute that can refer to the parameters currently. The 
only alternative that comes to mind is to use only a GNU-style spelling rather 
than trying to use a [[]] spelling. 

> Do you think introducing a frontend flag for this attribute would help? This 
> would make it clear that we are currently using a non-standard feature that 
> needs to be enabled explicitly. Again, this is a temporary solution, that 
> will go away as soon as we have contracts. Since contracts need to solve the 
> same problem, whatever the solution will be, this patch can piggyback on that.

There's no guarantee that contracts will come back with the same syntax used 
previously, so I wouldn't want to rely on that to solve the issue until we have 
a better idea of what contracts will look like coming out of the new study 
group. My preference is to use the GNU-style spelling and not provide a [[]] 
spelling. We've done that for other attributes (like `enable_if` and 
`diagnose_if`.


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

https://reviews.llvm.org/D72810



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

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D76631#1937428 , @yaxunl wrote:

> I am curious why opt and llc is not affected


In one case (opt, llc, clang-offload-bundler) it finds those programs in the 
"program paths", ie. the build folder: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L4733

Whereas in another cases (lld) when it doesn't find the program in the "program 
paths", it will go search the env.var. %PATH% and fall back to a 
platform-specific search, which includes searching using `{program_name}.exe`: 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Windows/Program.inc#L65

This is the output I was getting:

  
D:\llvm-project\buildninjaRel\tools\clang\test\Driver>"d:\llvm-project\buildninjarel\bin\clang.exe"
 "-c" "--cuda-device-only" "-###" "-target" "x86_64-linux-gnu" "-o" "a.s" "-x" 
"hip" "--cuda-gpu-arch=gfx900" "--hip-device-lib=lib1.bc" 
"--hip-device-lib-path=D:\llvm-project\clang\test\Driver/Inputs/hip_multiple_inputs/lib1"
 "D:\llvm-project\clang\test\Driver/Inputs/hip_multiple_inputs/a.cu"
  clang version 11.0.0 (https://github.com/llvm/llvm-project.git 
c1f8595fe5b856222418e2de547f0e346d84ac84)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: d:\llvm-project\buildninjarel\bin
   "d:\\llvm-project\\buildninjarel\\bin\\clang.exe" "-cc1" "-triple" 
"amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc" 
"-emit-llvm-uselists" "-disable-free" "-disable-llvm-verifier" 
"-discard-value-names" "-main-file-name" "a.cu" "-mrelocation-model" "pic" 
"-pic-level" "1" "-mthread-model" "posix" "-mframe-pointer=all" 
"-fno-rounding-math" "-mconstructor-aliases" "-aux-target-cpu" "x86-64" 
"-target-cpu" "gfx900" "-fcuda-is-device" "-fcuda-allow-variadic-functions" 
"-fvisibility" "hidden" "-fapply-global-visibility-to-externs" 
"-mlink-builtin-bitcode" 
"D:\\llvm-project\\clang\\test\\Driver/Inputs/hip_multiple_inputs/lib1\\lib1.bc"
 "-dwarf-column-info" "-fno-split-dwarf-inlining" "-debugger-tuning=gdb" 
"-resource-dir" "d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0" 
"-internal-isystem" "/usr/local/include" "-internal-isystem" 
"d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0\\include" 
"-internal-externc-isystem" "/include" "-internal-externc-isystem" 
"/usr/include" "-internal-isystem" "/usr/local/include" "-internal-isystem" 
"d:\\llvm-project\\buildninjarel\\lib\\clang\\11.0.0\\include" 
"-internal-externc-isystem" "/include" "-internal-externc-isystem" 
"/usr/include" "-fdeprecated-macro" "-fno-autolink" "-fdebug-compilation-dir" 
"D:\\llvm-project\\buildninjaRel\\tools\\clang\\test\\Driver" "-ferror-limit" 
"19" "-fmessage-length" "138" "-fgnuc-version=4.2.1" "-fobjc-runtime=gcc" 
"-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" 
"-fcolor-diagnostics" "-fcuda-allow-variadic-functions" "-faddrsig" "-o" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940.bc" "-x" "hip" 
"D:\\llvm-project\\clang\\test\\Driver/Inputs/hip_multiple_inputs/a.cu"
   "d:\\llvm-project\\buildninjarel\\bin\\llvm-link" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940.bc" "-o" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-linked-fa625c.bc"
   "d:\\llvm-project\\buildninjarel\\bin\\opt" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-linked-fa625c.bc" 
"-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx900" "-o" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-optimized-227a21.bc"
   "d:\\llvm-project\\buildninjarel\\bin\\llc" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-optimized-227a21.bc" 
"-mtriple=amdgcn-amd-amdhsa" "-mcpu=gfx900" "-filetype=obj" "-o" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-03076f.o"
   "C:\\Program Files\\LLVM\\bin\\lld.exe" "-flavor" "gnu" "-shared" "-o" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-630562.out" 
"C:\\Users\\aganea\\AppData\\Local\\Temp\\a-152940-gfx900-03076f.o"
   "d:\\llvm-project\\buildninjarel\\bin\\clang-offload-bundler" "-type=o" 
"-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa-gfx900" 
"-inputs=nul,C:\\Users\\aganea\\AppData\\Local\\Temp\\a-630562.out" 
"-outputs=a.s"

If LLD is built at the same time as Clang, this error doesn't happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76631



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

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D74619#1936336 , @pratlucas wrote:

> Hi @leonardchan ,
>
> I've double-checked the Neon intrinsics reference and it indeed confirms that 
> the only allowed value for the lane argument for `vdupq_lane_f64` is `0`:
>
>   Argument Preparation
>  
>   vec → Vn.1D 
>   0 << lane << 0
>
>
> (https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vdupq_lane_f64)
>
> I believe the observed behaviour is expected.


Thanks for the update. Able to confirm this was an "us" issue, not a clang one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74619



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

[clang] cfaa84e - Fix "previously declared as a struct" warning

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2020-03-23T12:59:16-07:00
New Revision: cfaa84e1a679b2e2eb409eb57292f7caecb642d4

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

LOG: Fix "previously declared as a struct" warning

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/Serialization/ASTRecordReader.h

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index d9e2945ebbea..ca0f991c24e3 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -116,7 +116,7 @@ class ObjCPropertyDecl;
 class ObjCPropertyImplDecl;
 class ObjCProtocolDecl;
 class ObjCTypeParamDecl;
-struct OMPTraitInfo;
+class OMPTraitInfo;
 struct ParsedTargetAttr;
 class Preprocessor;
 class Stmt;

diff  --git a/clang/include/clang/Serialization/ASTRecordReader.h 
b/clang/include/clang/Serialization/ASTRecordReader.h
index 362296024a97..5bdc9ca2ddbf 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -22,7 +22,7 @@
 #include "llvm/ADT/APSInt.h"
 
 namespace clang {
-struct OMPTraitInfo;
+class OMPTraitInfo;
 
 /// An object for streaming information from a record.
 class ASTRecordReader



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

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

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

In D75360#1937557 , @JDevlieghere 
wrote:

> Oh, I'm sorry, I just reverted it like a minute ago.


Its fine, I'll recommit when I have a fix as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D74183#1937426 , @efriedma wrote:

> I think we should remove the LangRef rule that says "sret" pointers have to 
> be dereferenceable/naturally aligned, and let the frontend add explicit 
> aligned/dereferenceable markings where appropriate.  (At that point, sret has 
> no target-independent meaning; it's just to manipulate the target ABI.)  It 
> would make the IR easier to understand, and resolves the interaction with 
> opaque pointers.


That might be reasonable, yeah.  And yeah, making the `dereferenceable` 
assumption explicit is going to be necessary with opaque pointer types 
eventually.


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

https://reviews.llvm.org/D74183



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

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

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

Quuxplusone added a comment.

In D76572#1937558 , @rsmith wrote:

> In D76572#1935791 , @lebedev.ri 
> wrote:
>
> > Passing-by remark:
> >
> > > I wrote a Clang warning [not pictured] to diagnose any use of `T(x)` 
> > > which was not equivalent to `static_cast(x)`.
> >
> > I'm not sure whether or not this will pass the bar for a clang diagnostic
>
>
> I'd like to try it out on a larger codebase, but it sounds at least 
> potentially good to me. There's a simple syntactic workaround (use `(T)x` 
> instead of `T(x)`), and there's a high likelihood that the code doesn't mean 
> what the programmer intended.
>
> Does the warning catch the cases where the code is equivalent to 
> `static_cast(x)` except that it ignores access? That seems like a really 
> good thing to warn on regardless of whether we warn on the general case.


Yes, I verified with some simple test cases that it catches the equivalents of 
(1) reinterpret_cast, (2) const_cast, (3) const_cast plus static_cast, (4) 
upcast to private base, and (5) downcast to private base.
However, the thing I wrote is 100% **not** ready for prime time. It basically 
just inserts a call to `TryStaticCast` from inside `CheckCXXCStyleCast`. 
Unfortunately `TryStaticCast` is one of these functions that Clang is full of, 
where it both "checks if X is possible" and also "does X" and also "prints 
error messages to stdout," all from within the same function. So my patch ends 
up being morally equivalent to just treating `T(x)` as `static_cast(x)`; it 
emits some errors that shouldn't be there if it's supposed to just warn, and I 
have no idea how to shut up those errors except by completely refactoring 
`TryCast`. (E.g., it could take an extra parameter `Mode` that indicated 
whether to actually do the conversion, or just report whether it was possible, 
or emit notes about why it was impossible.) I wrestled unsuccessfully with 
similar issues when I did `-Wreturn-std-move`.

I 100% want to see someone like you tackle this issue, but take my word for it, 
you **don't** want to start with the code I wrote the other day. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



___
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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D31343#1937588 , @aaron.ballman 
wrote:

> In D31343#1936810 , @Jasmin wrote:
>
> > Hello John!
> >
> > I was encouraged by Erich after a talk I had with him and Aaron on IRC to 
> > contact you about an use case for this plugin.
> >
> > I wanted to use user-specified/unknown attributes to annotate code and was 
> > told that they are not propagated through
> >  the AST and therefore it is not possible at the moment to use them for 
> > that purpose. Aaron's and Erich's idea was to
> >  allow kind of no-op attributes in the plugin so that the compiler would 
> > not issue a warning about the unknown attribute.
> >  This would be helpful for the user being able to define a list of 
> > attributes the user knows and expects, so that a compilation
> >  would not be spammed unnecessarily and misspelled attributes would still 
> > stand out.
> >
> > Being able to get the cursor or displayName of the attribute would 
> > essentially be enough to use it in tooling.
> >
> > We can use __attribute__((annotate(smth))) at the time being to achieve the 
> > same goal, but it is much more writing to do
> >  and also vendor specific. Being able to do the same with attributes would 
> > give them a real purpose, other than having
> >  to be accepted and not causing an error. Also they have to be supported by 
> > the language and we don't have to use
> >  macros to annotate the code.
> >
> > To summarize it would be nice to have:
> >
> > - user supplied unknown attributes to suppress a warning
> > - unknown attributes propagated in the AST
> >
> >   I hope I summarized this correctly and could get through the gist of this 
> > idea.
> >
> >   Looking forward to hearing from you.
>
>
> For reference: the current set of patches John has been working on allow you 
> to load a plugin to let the frontend parse an attribute with an arbitrary 
> spelling, but it does not have a way for you to register a new attribute AST 
> node that gets attached to anything in the AST. Instead, you create an 
> existing semantic attribute (such as an `AnnotateAttr`) and add that to the 
> AST.
>
> I think this is a great request to keep in mind for John or anyone else who 
> wants to extend this plugin functionality to allow for generating custom 
> attribute AST nodes (which is of interest for tools like clang static 
> analyzer or clang tidy plugins that may consume the clang AST). This 
> functionality would also have to keep the libclang C indexing interface in 
> mind, as that's another area of the compiler where this functionality would 
> be useful.


Hmm... I wonder if there is value to having these plugin-attributes auto-create 
a PluginAnnotateAttr or something (some otherwise bikeshedded attribute) for 
exactly Jasmin's use case.  That way the plugin author simply needs to list the 
possible spellings.


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] D76527: Don't export symbols from clang/opt/llc if plugins are disabled.

[Eli Friedman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG896335bfb8ea: Dont export symbols from clang/opt/llc 
if plugins are disabled. (authored by efriedma).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76527

Files:
  clang/tools/driver/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/llc/CMakeLists.txt
  llvm/tools/llvm-stress/CMakeLists.txt
  llvm/tools/opt/CMakeLists.txt
  llvm/unittests/Passes/CMakeLists.txt


Index: llvm/unittests/Passes/CMakeLists.txt
===
--- llvm/unittests/Passes/CMakeLists.txt
+++ llvm/unittests/Passes/CMakeLists.txt
@@ -16,7 +16,7 @@
   add_llvm_unittest(PluginsTests
 PluginsTest.cpp
 )
-  export_executable_symbols(PluginsTests)
+  export_executable_symbols_for_plugins(PluginsTests)
   target_link_libraries(PluginsTests PRIVATE LLVMTestingSupport)
 
   set(LLVM_LINK_COMPONENTS)
Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -39,7 +39,7 @@
   intrinsics_gen
   SUPPORT_PLUGINS
   )
-export_executable_symbols(opt)
+export_executable_symbols_for_plugins(opt)
 
 if(LLVM_BUILD_EXAMPLES)
 target_link_libraries(opt PRIVATE ExampleIRTransforms)
Index: llvm/tools/llvm-stress/CMakeLists.txt
===
--- llvm/tools/llvm-stress/CMakeLists.txt
+++ llvm/tools/llvm-stress/CMakeLists.txt
@@ -10,4 +10,3 @@
   DEPENDS
   intrinsics_gen
   )
-export_executable_symbols(llvm-stress)
Index: llvm/tools/llc/CMakeLists.txt
===
--- llvm/tools/llc/CMakeLists.txt
+++ llvm/tools/llc/CMakeLists.txt
@@ -27,4 +27,4 @@
   SUPPORT_PLUGINS
   )
 
-export_executable_symbols(llc)
+export_executable_symbols_for_plugins(llc)
Index: llvm/tools/bugpoint/CMakeLists.txt
===
--- llvm/tools/bugpoint/CMakeLists.txt
+++ llvm/tools/bugpoint/CMakeLists.txt
@@ -38,4 +38,4 @@
   intrinsics_gen
   SUPPORT_PLUGINS
   )
-export_executable_symbols(bugpoint)
+export_executable_symbols_for_plugins(bugpoint)
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1029,6 +1029,13 @@
   endif()
 endfunction()
 
+# Export symbols if LLVM plugins are enabled.
+function(export_executable_symbols_for_plugins target)
+  if(LLVM_ENABLE_PLUGINS)
+export_executable_symbols(${target})
+  endif()
+endfunction()
+
 if(NOT LLVM_TOOLCHAIN_TOOLS)
   set (LLVM_TOOLCHAIN_TOOLS
 llvm-ar
Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -57,7 +57,7 @@
 
 # Support plugins.
 if(CLANG_PLUGIN_SUPPORT)
-  export_executable_symbols(clang)
+  export_executable_symbols_for_plugins(clang)
 endif()
 
 add_dependencies(clang clang-resource-headers)


Index: llvm/unittests/Passes/CMakeLists.txt
===
--- llvm/unittests/Passes/CMakeLists.txt
+++ llvm/unittests/Passes/CMakeLists.txt
@@ -16,7 +16,7 @@
   add_llvm_unittest(PluginsTests
 PluginsTest.cpp
 )
-  export_executable_symbols(PluginsTests)
+  export_executable_symbols_for_plugins(PluginsTests)
   target_link_libraries(PluginsTests PRIVATE LLVMTestingSupport)
 
   set(LLVM_LINK_COMPONENTS)
Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -39,7 +39,7 @@
   intrinsics_gen
   SUPPORT_PLUGINS
   )
-export_executable_symbols(opt)
+export_executable_symbols_for_plugins(opt)
 
 if(LLVM_BUILD_EXAMPLES)
 target_link_libraries(opt PRIVATE ExampleIRTransforms)
Index: llvm/tools/llvm-stress/CMakeLists.txt
===
--- llvm/tools/llvm-stress/CMakeLists.txt
+++ llvm/tools/llvm-stress/CMakeLists.txt
@@ -10,4 +10,3 @@
   DEPENDS
   intrinsics_gen
   )
-export_executable_symbols(llvm-stress)
Index: llvm/tools/llc/CMakeLists.txt
===
--- llvm/tools/llc/CMakeLists.txt
+++ llvm/tools/llc/CMakeLists.txt
@@ -27,4 +27,4 @@
   SUPPORT_PLUGINS
   )
 
-export_executable_symbols(llc)
+export_executable_symbols_for_plugins(llc)
Index: llvm/tools/bugpoint/CMakeLists.txt
===
--- llvm/tools/bugpoint/CMakeLists.txt
+++ llvm/tools/bugpoint/CMakeLists.txt
@@ -38,4 +38,4 @@
   intrinsics_gen
   SUPPORT_PLUGINS
   )

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG55eca2853e4f: [OpenMP][NFC] Minimize memory usage and 
copying of `OMPTraitInfo`s (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D76173?vs=250347=252123#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76173

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Serialization/ASTRecordReader.h
  clang/include/clang/Serialization/ASTRecordWriter.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -107,7 +107,7 @@
   .Case("IdentifierInfo *", "Record.readIdentifier()")
   .Case("StringRef", "Record.readString()")
   .Case("ParamIdx", "ParamIdx::deserialize(Record.readInt())")
-  .Case("OMPTraitInfo", "Record.readOMPTraitInfo()")
+  .Case("OMPTraitInfo *", "Record.readOMPTraitInfo()")
   .Default("Record.readInt()");
 }
 
@@ -131,7 +131,7 @@
  .Case("StringRef", "AddString(" + std::string(name) + ");\n")
  .Case("ParamIdx",
"push_back(" + std::string(name) + ".serialize());\n")
- .Case("OMPTraitInfo",
+ .Case("OMPTraitInfo *",
"writeOMPTraitInfo(" + std::string(name) + ");\n")
  .Default("push_back(" + std::string(name) + ");\n");
 }
@@ -363,7 +363,7 @@
   OS << "if (SA->get" << getUpperName() << "().isValid())\n  ";
 OS << "OS << \" \" << SA->get" << getUpperName()
<< "().getSourceIndex();\n";
-  } else if (type == "OMPTraitInfo") {
+  } else if (type == "OMPTraitInfo *") {
 OS << "OS << \" \" << SA->get" << getUpperName() << "();\n";
   } else {
 llvm_unreachable("Unknown SimpleArgument type!");
@@ -1331,7 +1331,7 @@
   else if (ArgName == "VersionArgument")
 Ptr = std::make_unique(Arg, Attr);
   else if (ArgName == "OMPTraitInfoArgument")
-Ptr = std::make_unique(Arg, Attr, "OMPTraitInfo");
+Ptr = std::make_unique(Arg, Attr, "OMPTraitInfo *");
 
   if (!Ptr) {
 // Search in reverse order so that the most-derived type is handled first.
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6624,9 +6624,9 @@
   Record.AddSourceLocation(C->getKindKwLoc());
 }
 
-void ASTRecordWriter::writeOMPTraitInfo(const OMPTraitInfo ) {
-  writeUInt32(TI.Sets.size());
-  for (const auto  : TI.Sets) {
+void ASTRecordWriter::writeOMPTraitInfo(const OMPTraitInfo *TI) {
+  writeUInt32(TI->Sets.size());
+  for (const auto  : TI->Sets) {
 writeEnum(Set.Kind);
 writeUInt32(Set.Selectors.size());
 for (const auto  : Set.Selectors) {
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2744,7 +2744,7 @@
 return Reader.readVersionTuple();
   }
 
-  OMPTraitInfo readOMPTraitInfo() { return Reader.readOMPTraitInfo(); }
+  OMPTraitInfo *readOMPTraitInfo() { return Reader.readOMPTraitInfo(); }
 
   template  T *GetLocalDeclAs(uint32_t LocalID) {
 return Reader.GetLocalDeclAs(LocalID);
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -12684,8 +12684,8 @@
   C->setKindKwLoc(Record.readSourceLocation());
 }
 
-OMPTraitInfo ASTRecordReader::readOMPTraitInfo() {
-  OMPTraitInfo TI;
+OMPTraitInfo *ASTRecordReader::readOMPTraitInfo() {
+  OMPTraitInfo  = getContext().getNewOMPTraitInfo();
   TI.Sets.resize(readUInt32());
   for (auto  : TI.Sets) {
 Set.Kind = readEnum();
@@ -12700,5 +12700,5 @@
 Property.Kind = readEnum();
 }
   }
-  return TI;
+  return 
 }
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -398,7 +398,8 @@
 
   // Copy the template version of the OMPTraitInfo and run substitute on all
   // score and 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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

Clear pending action.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1676,6 +1676,14 @@
   }
   LE.write(Offset);
 
+  // Writes the number of importers.
+  LE.write(Data.HFI.Importers.size());
+  if (!Data.HFI.Importers.empty()){
+for (const FileEntry* F : Data.HFI.Importers) {
+  LE.write(F->getUID());
+}
+  }
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,20 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  {
+const uint32_t ImportersCount = endian::readNext(d));
+if (ImportersCount > 0) {
+  // Read the file UIDs and temporarily save it because we don't have
+  // the FileEntry for these yet.
+  for (int i = 0; i < ImportersCount; ++i) {
+HFI.ImportersUIDs.push_back(endian::readNext(d));
+  }
+  // TODO: This is a little inefficient. But don't know of a way to
+  // stores only this HFI.
+  Reader.addPendingHeaderSearch(HS);
+}
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -9068,7 +9082,7 @@
   while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
  !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||
  !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
- !PendingUpdateRecords.empty()) {
+ !PendingUpdateRecords.empty() || !PendingHeaderSearch.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -9143,6 +9157,18 @@
 }
 PendingMacroIDs.clear();
 
+for (auto PendingHS : PendingHeaderSearch) {
+  for (HeaderFileInfo& HFI : PendingHS->FileInfo){
+if (!HFI.FinishedLoadingImporters) {
+  for (uint32_t FUID : HFI.ImporterUIDs) {
+// TODO: find the FileEntry based on UID??
+  }
+  HFI.FinishedLoadingImporters = true;
+}
+  }
+}
+PendingHeaderSearch.clear();
+
 // Wire up the DeclContexts for Decls that we delayed setting until
 // recursive loading is completed.
 while (!PendingDeclContextInfos.empty()) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,22 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-// After module map parsing, this expands to:
-//
-//module stddef {
-//  header "/path_to_builtin_dirs/stddef.h"
-//  textual "stddef.h"
-//}
-//
-// It's common that libc++ and system modules will both define such
-// submodules. Make sure cached results for a builtin header won't
-// prevent other builtin modules to potentially enter the builtin header.
-// Note that builtins are header guarded and the decision to actually
-// enter them is postponed to the controlling macros logic below.
-bool TryEnterHdr = false;
-if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
-  TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
-ModuleMap::isBuiltinHeader(
-llvm::sys::path::filename(File->getName()));
-
-// Textual headers can be #imported from different modules. Since ObjC
-// headers find in the wild might rely only on #import and do not contain
-// controlling 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 252117.
oontvoo marked an inline comment as done.
oontvoo added a comment.

Add a PendingHeaderSearch set


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1676,6 +1676,14 @@
   }
   LE.write(Offset);
 
+  // Writes the number of importers.
+  LE.write(Data.HFI.Importers.size());
+  if (!Data.HFI.Importers.empty()){
+for (const FileEntry* F : Data.HFI.Importers) {
+  LE.write(F->getUID());
+}
+  }
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,20 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  {
+const uint32_t ImportersCount = endian::readNext(d));
+if (ImportersCount > 0) {
+  // Read the file UIDs and temporarily save it because we don't have
+  // the FileEntry for these yet.
+  for (int i = 0; i < ImportersCount; ++i) {
+HFI.ImportersUIDs.push_back(endian::readNext(d));
+  }
+  // TODO: This is a little inefficient. But don't know of a way to
+  // stores only this HFI.
+  Reader.addPendingHeaderSearch(HS);
+}
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -9068,7 +9082,7 @@
   while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
  !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||
  !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
- !PendingUpdateRecords.empty()) {
+ !PendingUpdateRecords.empty() || !PendingHeaderSearch.empty()) {
 // If any identifiers with corresponding top-level declarations have
 // been loaded, load those declarations now.
 using TopLevelDeclsMap =
@@ -9143,6 +9157,17 @@
 }
 PendingMacroIDs.clear();
 
+for (auto PendingHS : PendingHeaderSearch) {
+  for (HeaderFileInfo& HFI : PendingHS->FileInfo){
+if (!HFI.FinishedLoadingImporters) {
+  for (uint32_t FUID : HFI.ImporterUIDs) {
+// TODO: find the FileEntry based on UID??
+  }
+  HFI.FinishedLoadingImporters = true;
+}
+  }
+}
+
 // Wire up the DeclContexts for Decls that we delayed setting until
 // recursive loading is completed.
 while (!PendingDeclContextInfos.empty()) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,22 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-// After module map parsing, this expands to:
-//
-//module stddef {
-//  header "/path_to_builtin_dirs/stddef.h"
-//  textual "stddef.h"
-//}
-//
-// It's common that libc++ and system modules will both define such
-// submodules. Make sure cached results for a builtin header won't
-// prevent other builtin modules to potentially enter the builtin header.
-// Note that builtins are header guarded and the decision to actually
-// enter them is postponed to the controlling macros logic below.
-bool TryEnterHdr = false;
-if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
-  TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
-ModuleMap::isBuiltinHeader(
-llvm::sys::path::filename(File->getName()));
-
-// Textual headers can be #imported from different modules. Since ObjC
-// headers find in the wild might rely only on #import and do not contain
-

[PATCH] D31343: Add an attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D31343#1936810 , @Jasmin wrote:

> Hello John!
>
> I was encouraged by Erich after a talk I had with him and Aaron on IRC to 
> contact you about an use case for this plugin.
>
> I wanted to use user-specified/unknown attributes to annotate code and was 
> told that they are not propagated through
>  the AST and therefore it is not possible at the moment to use them for that 
> purpose. Aaron's and Erich's idea was to
>  allow kind of no-op attributes in the plugin so that the compiler would not 
> issue a warning about the unknown attribute.
>  This would be helpful for the user being able to define a list of attributes 
> the user knows and expects, so that a compilation
>  would not be spammed unnecessarily and misspelled attributes would still 
> stand out.
>
> Being able to get the cursor or displayName of the attribute would 
> essentially be enough to use it in tooling.
>
> We can use __attribute__((annotate(smth))) at the time being to achieve the 
> same goal, but it is much more writing to do
>  and also vendor specific. Being able to do the same with attributes would 
> give them a real purpose, other than having
>  to be accepted and not causing an error. Also they have to be supported by 
> the language and we don't have to use
>  macros to annotate the code.
>
> To summarize it would be nice to have:
>
> - user supplied unknown attributes to suppress a warning
> - unknown attributes propagated in the AST
>
>   I hope I summarized this correctly and could get through the gist of this 
> idea.
>
>   Looking forward to hearing from you.


For reference: the current set of patches John has been working on allow you to 
load a plugin to let the frontend parse an attribute with an arbitrary 
spelling, but it does not have a way for you to register a new attribute AST 
node that gets attached to anything in the AST. Instead, you create an existing 
semantic attribute (such as an `AnnotateAttr`) and add that to the AST.

I think this is a great request to keep in mind for John or anyone else who 
wants to extend this plugin functionality to allow for generating custom 
attribute AST nodes (which is of interest for tools like clang static analyzer 
or clang tidy plugins that may consume the clang AST). This functionality would 
also have to keep the libclang C indexing interface in mind, as that's another 
area of the compiler where this functionality would be useful.


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] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76631



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

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

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

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

Changes LGTM on the Clang side.

In D76572#1935791 , @lebedev.ri wrote:

> Passing-by remark:
>
> > I wrote a Clang warning [not pictured] to diagnose any use of `T(x)` which 
> > was not equivalent to `static_cast(x)`.
>
> I'm not sure whether or not this will pass the bar for a clang diagnostic


I'd like to try it out on a larger codebase, but it sounds at least potentially 
good to me. There's a simple syntactic workaround (use `(T)x` instead of 
`T(x)`), and there's a high likelihood that the code doesn't mean what the 
programmer intended.

Does the warning catch the cases where the code is equivalent to 
`static_cast(x)` except that it ignores access? That seems like a really 
good thing to warn on regardless of whether we warn on the general case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D75360#1937556 , @Szelethus wrote:

> In D75360#1937415 , @JDevlieghere 
> wrote:
>
> > It appears this broke the modules build: 
> > http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12905/console
> >
> > Can you please take a look?
>
>
> Yup, on it.


Oh, I'm sorry, I just reverted it like a minute ago.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

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

Szelethus added a comment.

In D75360#1937415 , @JDevlieghere 
wrote:

> It appears this broke the modules build: 
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12905/console
>
> Can you please take a look?


Yup, on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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

[clang] 55eca28 - [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2020-03-23T14:23:46-05:00
New Revision: 55eca2853e4f096be189208770034a8a4fb72666

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

LOG: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

See rational here: https://reviews.llvm.org/D71830#1922656

Reviewed By: rnk

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Serialization/ASTRecordReader.h
clang/include/clang/Serialization/ASTRecordWriter.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/AttrImpl.cpp
clang/lib/AST/OpenMPClause.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index d74edb8a8adb..d9e2945ebbea 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -116,6 +116,7 @@ class ObjCPropertyDecl;
 class ObjCPropertyImplDecl;
 class ObjCProtocolDecl;
 class ObjCTypeParamDecl;
+struct OMPTraitInfo;
 struct ParsedTargetAttr;
 class Preprocessor;
 class Stmt;
@@ -2962,6 +2963,14 @@ OPT_LIST(V)
   };
 
   llvm::StringMap SectionInfos;
+
+  /// Return a new OMPTraitInfo object owned by this context.
+  OMPTraitInfo ();
+
+private:
+  /// All OMPTraitInfo objects live in this collection, one per
+  /// `pragma omp [begin] declare variant` directive.
+  SmallVector OMPTraitInfoVector;
 };
 
 /// Utility function for constructing a nullary selector.

diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index acf450f07e66..8b92cb62634a 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7116,22 +7116,27 @@ class OMPClausePrinter final : public 
OMPClauseVisitor {
 /// collection of selector sets, each with an associated kind and an ordered
 /// collection of selectors. A selector has a kind, an optional 
score/condition,
 /// and an ordered collection of properties.
-struct OMPTraitInfo {
+class OMPTraitInfo {
+  /// Private constructor accesible only by ASTContext.
+  OMPTraitInfo() {}
+  friend class ASTContext;
+
+public:
   struct OMPTraitProperty {
 llvm::omp::TraitProperty Kind = llvm::omp::TraitProperty::invalid;
   };
   struct OMPTraitSelector {
 Expr *ScoreOrCondition = nullptr;
 llvm::omp::TraitSelector Kind = llvm::omp::TraitSelector::invalid;
-llvm::SmallVector Properties;
+llvm::SmallVector Properties;
   };
   struct OMPTraitSet {
 llvm::omp::TraitSet Kind = llvm::omp::TraitSet::invalid;
-llvm::SmallVector Selectors;
+llvm::SmallVector Selectors;
   };
 
   /// The outermost level of selector sets.
-  llvm::SmallVector Sets;
+  llvm::SmallVector Sets;
 
   bool anyScoreOrCondition(
   llvm::function_ref Cond) {
@@ -7157,6 +7162,7 @@ struct OMPTraitInfo {
   void print(llvm::raw_ostream , const PrintingPolicy ) const;
 };
 llvm::raw_ostream <<(llvm::raw_ostream , const OMPTraitInfo );
+llvm::raw_ostream <<(llvm::raw_ostream , const OMPTraitInfo *TI);
 
 } // namespace clang
 

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index c8c35cbe5b70..db1589bea814 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3314,7 +3314,7 @@ def OMPDeclareVariant : InheritableAttr {
 OMPTraitInfoArgument<"TraitInfos">,
   ];
   let AdditionalMembers = [{
-OMPTraitInfo () { return traitInfos; }
+OMPTraitInfo () { return *traitInfos; }
 void printPrettyPragma(raw_ostream & OS, const PrintingPolicy )
 const;
   }];

diff  --git a/clang/include/clang/Serialization/ASTRecordReader.h 
b/clang/include/clang/Serialization/ASTRecordReader.h
index 7d4ef7c43a9d..362296024a97 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -260,7 +260,7 @@ class ASTRecordReader
   }
 
   /// Read an OMPTraitInfo object, advancing Idx.
-  OMPTraitInfo readOMPTraitInfo();
+  OMPTraitInfo *readOMPTraitInfo();
 
   /// Read an OpenMP clause, advancing Idx.
   OMPClause *readOMPClause();

diff  --git a/clang/include/clang/Serialization/ASTRecordWriter.h 
b/clang/include/clang/Serialization/ASTRecordWriter.h
index 924aa5d4b758..491207c9de90 100644
--- a/clang/include/clang/Serialization/ASTRecordWriter.h
+++ 

[clang] 896335b - Don't export symbols from clang/opt/llc if plugins are disabled.

[Eli Friedman via cfe-commits]

Author: Eli Friedman
Date: 2020-03-23T12:17:09-07:00
New Revision: 896335bfb8ea2c09c361c4f1e5a9aa6fb78caf88

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

LOG: Don't export symbols from clang/opt/llc if plugins are disabled.

The only reason we export symbols from these tools is to support
plugins; if we don't have plugins, exporting symbols just bloats the
executable and makes LTO less effective.

See review of D75879 for the discussion that led to this patch.

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

Added: 


Modified: 
clang/tools/driver/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/tools/bugpoint/CMakeLists.txt
llvm/tools/llc/CMakeLists.txt
llvm/tools/llvm-stress/CMakeLists.txt
llvm/tools/opt/CMakeLists.txt
llvm/unittests/Passes/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/driver/CMakeLists.txt 
b/clang/tools/driver/CMakeLists.txt
index 2b783cff0955..c53485ef1957 100644
--- a/clang/tools/driver/CMakeLists.txt
+++ b/clang/tools/driver/CMakeLists.txt
@@ -57,7 +57,7 @@ endif()
 
 # Support plugins.
 if(CLANG_PLUGIN_SUPPORT)
-  export_executable_symbols(clang)
+  export_executable_symbols_for_plugins(clang)
 endif()
 
 add_dependencies(clang clang-resource-headers)

diff  --git a/llvm/cmake/modules/AddLLVM.cmake 
b/llvm/cmake/modules/AddLLVM.cmake
index 7b95d8be1b60..8cd71eef2332 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1029,6 +1029,13 @@ function(export_executable_symbols target)
   endif()
 endfunction()
 
+# Export symbols if LLVM plugins are enabled.
+function(export_executable_symbols_for_plugins target)
+  if(LLVM_ENABLE_PLUGINS)
+export_executable_symbols(${target})
+  endif()
+endfunction()
+
 if(NOT LLVM_TOOLCHAIN_TOOLS)
   set (LLVM_TOOLCHAIN_TOOLS
 llvm-ar

diff  --git a/llvm/tools/bugpoint/CMakeLists.txt 
b/llvm/tools/bugpoint/CMakeLists.txt
index 0b5998e181eb..8d2d5a06e664 100644
--- a/llvm/tools/bugpoint/CMakeLists.txt
+++ b/llvm/tools/bugpoint/CMakeLists.txt
@@ -38,4 +38,4 @@ add_llvm_tool(bugpoint
   intrinsics_gen
   SUPPORT_PLUGINS
   )
-export_executable_symbols(bugpoint)
+export_executable_symbols_for_plugins(bugpoint)

diff  --git a/llvm/tools/llc/CMakeLists.txt b/llvm/tools/llc/CMakeLists.txt
index 479bc6b55b27..2eecfca2e075 100644
--- a/llvm/tools/llc/CMakeLists.txt
+++ b/llvm/tools/llc/CMakeLists.txt
@@ -27,4 +27,4 @@ add_llvm_tool(llc
   SUPPORT_PLUGINS
   )
 
-export_executable_symbols(llc)
+export_executable_symbols_for_plugins(llc)

diff  --git a/llvm/tools/llvm-stress/CMakeLists.txt 
b/llvm/tools/llvm-stress/CMakeLists.txt
index 139ab9e0d8f9..e4d1ae65ee76 100644
--- a/llvm/tools/llvm-stress/CMakeLists.txt
+++ b/llvm/tools/llvm-stress/CMakeLists.txt
@@ -10,4 +10,3 @@ add_llvm_tool(llvm-stress
   DEPENDS
   intrinsics_gen
   )
-export_executable_symbols(llvm-stress)

diff  --git a/llvm/tools/opt/CMakeLists.txt b/llvm/tools/opt/CMakeLists.txt
index 79613c836c53..8caa1b78b729 100644
--- a/llvm/tools/opt/CMakeLists.txt
+++ b/llvm/tools/opt/CMakeLists.txt
@@ -39,7 +39,7 @@ add_llvm_tool(opt
   intrinsics_gen
   SUPPORT_PLUGINS
   )
-export_executable_symbols(opt)
+export_executable_symbols_for_plugins(opt)
 
 if(LLVM_BUILD_EXAMPLES)
 target_link_libraries(opt PRIVATE ExampleIRTransforms)

diff  --git a/llvm/unittests/Passes/CMakeLists.txt 
b/llvm/unittests/Passes/CMakeLists.txt
index c04aa9f84458..823bc56851fa 100644
--- a/llvm/unittests/Passes/CMakeLists.txt
+++ b/llvm/unittests/Passes/CMakeLists.txt
@@ -16,7 +16,7 @@ if (NOT WIN32)
   add_llvm_unittest(PluginsTests
 PluginsTest.cpp
 )
-  export_executable_symbols(PluginsTests)
+  export_executable_symbols_for_plugins(PluginsTests)
   target_link_libraries(PluginsTests PRIVATE LLVMTestingSupport)
 
   set(LLVM_LINK_COMPONENTS)



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

Re: [clang] 4a0267e - Convert a reachable llvm_unreachable into an assert.

[Aaron Ballman via cfe-commits]

On Mon, Mar 23, 2020 at 1:41 PM Craig Topper  wrote:
>
> If its relatively common for someone making a plugin to mess this up, maybe 
> it should be report_fatal_error instead of only catching it in asserts build?

I think that would also be a reasonable solution if that's the preference.

~Aaron

>
> ~Craig
>
>
> On Mon, Mar 23, 2020 at 5:24 AM Aaron Ballman  wrote:
>>
>> On Sun, Mar 22, 2020 at 3:31 PM David Blaikie  wrote:
>> >
>> > On Sun, Mar 22, 2020 at 9:34 AM Aaron Ballman  
>> > wrote:
>> >>
>> >> On Sun, Mar 22, 2020 at 12:19 PM David Blaikie  wrote:
>> >> >
>> >> > On Sun, Mar 22, 2020 at 6:34 AM Aaron Ballman  
>> >> > wrote:
>> >> >>
>> >> >> On Sat, Mar 21, 2020 at 11:31 PM David Blaikie  
>> >> >> wrote:
>> >> >> >
>> >> >> > Why the change? this seems counter to LLVM's style which pretty 
>> >> >> > consistently uses unreachable rather than assert(false), so far as I 
>> >> >> > know?
>> >> >>
>> >> >> We're not super consistent (at least within Clang), but the rules as
>> >> >> I've generally understood them are to use llvm_unreachable only for
>> >> >> truly unreachable code and to use assert(false) when the code is
>> >> >> technically reachable but is a programmer mistake to have gotten
>> >> >> there.
>> >> >
>> >> >
>> >> > I don't see those as two different things personally - llvm_unreachable 
>> >> > is used when the programmer believes it to be unreachable (not that it 
>> >> > must be proven to be unreachable - we have message text there so it's 
>> >> > informative if the assumption turns out not to hold)
>> >>
>> >> The message text doesn't help when the code is reached in release
>> >> mode, which was the issue. Asserts + release you still get some
>> >> helpful text saying "you screwed up". llvm_unreachable in release
>> >> mode, you may get lucky or you may not (in this case, I didn't get
>> >> lucky -- there was no text, just a crash).
>> >
>> >
>> > That doesn't seem to be what it's documented to do:
>> >
>> > /// Marks that the current location is not supposed to be reachable.
>> > /// In !NDEBUG builds, prints the message and location info to stderr.
>> > /// In NDEBUG builds, becomes an optimizer hint that the current location
>> > /// is not supposed to be reachable.  On compilers that don't support
>> > /// such hints, prints a reduced message instead.
>> >
>> > & certainly I think the documentation is what it /should/ be doing.
>> >
>> > /maybe/ 
>> > https://reviews.llvm.org/rG5f4535b974e973d52797945fbf80f19ffba8c4ad broke 
>> > that contract on Windows, but I'm not sure how? (an unreachable at the end 
>> > of that function shouldn't cause the whole function to be unreachable - 
>> > because abort could have side effects and halt the program before the 
>> > unreachable is reached)
>>
>> Agreed. It could also be that my machine is in a weird state (I'm
>> currently battling a situation where the command line parser appears
>> to be totally broken on Windows due to misuse of a ManagedStatic
>> somewhere but I've not seen any commits that relate to the issues).
>>
>> >> >> In this particular case, the code is very much reachable
>> >> >
>> >> >
>> >> > In what sense? If it is actually reachable - shouldn't it be tested? (& 
>> >> > in which case the assert(false) will get in the way of that testing)
>> >>
>> >> In the sense that normal code paths reach that code easily. Basically,
>> >> that code is checking to see whether a plugin you've written properly
>> >> sets up its options or not. When you're developing a plugin, it's
>> >> quite reasonable to expect you won't get it just right on the first
>> >> try, so you hit the code path but only as a result of you not writing
>> >> the plugin quite right. So under normal conditions (once the plugin is
>> >> working), the code path should not be reached but under development
>> >> the code path gets reached accidentally.
>> >>
>> >> >> and I
>> >> >> spent a lot more time debugging than I should have because I was using
>> >> >> a release + asserts build and the semantics of llvm_unreachable made
>> >> >> unfortunate codegen (switching to an assert makes the issue
>> >> >> immediately obvious).
>> >> >
>> >> >
>> >> > I think it might be reasonable to say that release+asserts to have 
>> >> > unreachable behave the same way unreachable behaves at -O0 (which is to 
>> >> > say, much like assert(false)). Clearly release+asserts effects code 
>> >> > generation, so there's nothing like the "debug info invariance" goal 
>> >> > that this would be tainting, etc.
>> >> >
>> >> > Certainly opinions vary here, but here are some commits that show the 
>> >> > sort of general preference:
>> >> > http://llvm.org/viewvc/llvm-project?view=revision=259302
>> >> > http://llvm.org/viewvc/llvm-project?view=revision=253005
>> >> > http://llvm.org/viewvc/llvm-project?view=revision=251266
>> >> >
>> >> > And some counter examples:
>> >> > http://llvm.org/viewvc/llvm-project?view=revision=225043
>> >> > Including this thread where Chandler 

[clang] 56abcfa - Revert "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes"

[Jonas Devlieghere via cfe-commits]

Author: Jonas Devlieghere
Date: 2020-03-23T12:09:24-07:00
New Revision: 56abcfad70ee679ad95ab41d934491ebcaebdf7d

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

LOG: Revert "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow 
CheckerManager to be constructed for non-analysis purposes"

Temporarily reverting this patch because it breaks the modules build.

Added: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 6faf0f2e0afd..4454d7603b27 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -14,11 +14,9 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
-#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
-#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
@@ -42,6 +40,7 @@ class BugReporter;
 class CallEvent;
 class CheckerBase;
 class CheckerContext;
+class CheckerRegistry;
 class ExplodedGraph;
 class ExplodedNode;
 class ExplodedNodeSet;
@@ -122,42 +121,14 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext *Context;
+  ASTContext 
   const LangOptions LangOpts;
   AnalyzerOptions 
   CheckerNameRef CurrentCheckerName;
-  DiagnosticsEngine 
-  CheckerRegistry Registry;
 
 public:
-  CheckerManager(
-  ASTContext , AnalyzerOptions ,
-  ArrayRef plugins,
-  ArrayRef> checkerRegistrationFns)
-  : Context(), LangOpts(Context.getLangOpts()), AOptions(AOptions),
-Diags(Context.getDiagnostics()),
-Registry(plugins, Context.getDiagnostics(), AOptions,
- checkerRegistrationFns) {
-Registry.initializeRegistry(*this);
-Registry.initializeManager(*this);
-finishedCheckerRegistration();
-  }
-
-  /// Constructs a CheckerManager that ignores all non TblGen-generated
-  /// checkers. Useful for unit testing, unless the checker infrastructure
-  /// itself is tested.
   CheckerManager(ASTContext , AnalyzerOptions )
-  : CheckerManager(Context, AOptions, {}, {}) {}
-
-  /// Constructs a CheckerManager without requiring an AST. No checker
-  /// registration will take place. Only useful for retrieving the
-  /// CheckerRegistry and print for help flags where the AST is unavalaible.
-  CheckerManager(AnalyzerOptions , const LangOptions ,
- DiagnosticsEngine , ArrayRef plugins)
-  : LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
-Registry(plugins, Diags, AOptions) {
-Registry.initializeRegistry(*this);
-  }
+  : Context(Context), LangOpts(Context.getLangOpts()), AOptions(AOptions) 
{}
 
   ~CheckerManager();
 
@@ -170,12 +141,7 @@ class CheckerManager {
 
   const LangOptions () const { return LangOpts; }
   AnalyzerOptions () const { return AOptions; }
-  const CheckerRegistry () const { return Registry; }
-  DiagnosticsEngine () const { return Diags; }
-  ASTContext () const {
-assert(Context);
-return *Context;
-  }
+  ASTContext () const { return Context; }
 
   /// Emits an error through a DiagnosticsEngine about an invalid user supplied
   /// checker option value.

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
index bcc29a60ad70..2d24e6a9586b 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
@@ -55,7 +55,7 @@ class AnalysisASTConsumer : public ASTConsumer {
 std::unique_ptr
 CreateAnalysisConsumer(CompilerInstance );
 
-} // namespace ento
+} // end GR namespace
 
 } // 

[PATCH] D76520: [CUDA][HIP] Add -Xarch_device and -Xarch_host options

[Yaxun Liu via Phabricator via cfe-commits]

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

add TODO for fixing space separated arguments


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

https://reviews.llvm.org/D76520

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Compilation.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-options.hip

Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -13,3 +13,16 @@
 // RUN:   -mllvm -amdgpu-early-inline-all=true  %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=MLLVM %s
 // MLLVM-NOT: "-mllvm"{{.*}}"-amdgpu-early-inline-all=true"{{.*}}"-mllvm"{{.*}}"-amdgpu-early-inline-all=true"
+
+// RUN: %clang -### -Xarch_device -g -nogpulib --cuda-gpu-arch=gfx900 \
+// RUN:   -Xarch_device -fcf-protection=branch \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=DEV %s
+// DEV: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
+// DEV: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
+// DEV-NOT: clang{{.*}} {{.*}} "-debug-info-kind={{.*}}"
+
+// RUN: %clang -### -Xarch_host -g -nogpulib --cuda-gpu-arch=gfx900 \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=HOST %s
+// HOST-NOT: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
+// HOST-NOT: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
+// HOST: clang{{.*}} "-debug-info-kind={{.*}}"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -378,12 +378,6 @@
   const OptTable  = getDriver().getOpts();
 
   for (Arg *A : Args) {
-if (A->getOption().matches(options::OPT_Xarch__)) {
-  // Skip this argument unless the architecture matches BoundArch.
-  if (BoundArch.empty() || A->getValue(0) != BoundArch)
-continue;
-  TranslateXarchArgs(Args, A, DAL);
-}
 DAL->append(A);
   }
 
Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -800,12 +800,6 @@
   }
 
   for (Arg *A : Args) {
-if (A->getOption().matches(options::OPT_Xarch__)) {
-  // Skip this argument unless the architecture matches BoundArch
-  if (BoundArch.empty() || A->getValue(0) != BoundArch)
-continue;
-  TranslateXarchArgs(Args, A, DAL);
-}
 DAL->append(A);
   }
 
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1103,11 +1103,20 @@
   return nullptr;
 }
 
-void ToolChain::TranslateXarchArgs(const llvm::opt::DerivedArgList ,
-   llvm::opt::Arg *,
-   llvm::opt::DerivedArgList *DAL) const {
+/ TODO: Currently argument values separated by space e.g.
+// -Xclang -mframe-pointer=no cannot be passed by -Xarch_. This should be
+// fixed.
+void ToolChain::TranslateXarchArgs(
+const llvm::opt::DerivedArgList , llvm::opt::Arg *,
+llvm::opt::DerivedArgList *DAL,
+SmallVectorImpl *AllocatedArgs) const {
   const OptTable  = getDriver().getOpts();
-  unsigned Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
+  unsigned ValuePos = 1;
+  if (A->getOption().matches(options::OPT_Xarch_device) ||
+  A->getOption().matches(options::OPT_Xarch_host))
+ValuePos = 0;
+
+  unsigned Index = Args.getBaseArgs().MakeIndex(A->getValue(ValuePos));
   unsigned Prev = Index;
   std::unique_ptr XarchArg(Opts.ParseOneArg(Args, Index));
 
@@ -1130,5 +1139,49 @@
   }
   XarchArg->setBaseArg(A);
   A = XarchArg.release();
-  DAL->AddSynthesizedArg(A);
+  if (!AllocatedArgs)
+DAL->AddSynthesizedArg(A);
+  else
+AllocatedArgs->push_back(A);
+}
+
+llvm::opt::DerivedArgList *ToolChain::TranslateXarchArgs(
+const llvm::opt::DerivedArgList , StringRef BoundArch,
+Action::OffloadKind OFK,
+SmallVectorImpl *AllocatedArgs) const {
+  DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
+  bool Modified = false;
+
+  bool IsGPU = OFK == Action::OFK_Cuda || OFK == Action::OFK_HIP;
+  for (Arg *A : Args) {
+bool NeedTrans = false;
+bool Skip = false;
+if (A->getOption().matches(options::OPT_Xarch_device)) {
+  NeedTrans = IsGPU;
+  Skip = !IsGPU;
+} else if (A->getOption().matches(options::OPT_Xarch_host)) {
+  NeedTrans = !IsGPU;
+  Skip = IsGPU;
+} else if (A->getOption().matches(options::OPT_Xarch__) && IsGPU) {
+  // Do not translate -Xarch_ options 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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

Updated the reader to read the UIDs first, then we'll build up the Importers 
list at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1676,6 +1676,14 @@
   }
   LE.write(Offset);
 
+  // Writes the number of importers.
+  LE.write(Data.HFI.Importers.size());
+  if (!Data.HFI.Importers.empty()){
+for (const FileEntry* F : Data.HFI.Importers) {
+  LE.write(F->getUID());
+}
+  }
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1898,6 +1898,21 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  {
+const uint32_t ImportersCount = endian::readNext(d));
+if (ImportersCount > 0) {
+
+  // TODO: I don't think this is right because these importers
+  // are NOT guaranteed to be read/loaded before this header???
+  //
+  // Read the file UIDs and temporarily save it because we don't have
+  // the FileEntry for these yet.
+  for (int i = 0; i < ImportersCount; ++i) {
+HFI.ImportersUIDs.push_back(endian::readNext(d));
+  }
+}
+  }
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1253,60 +1253,22 @@
   // Get information about this file.
   HeaderFileInfo  = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-if (!ModulesEnabled)
-  return false;
-// Ensure FileInfo bits are up to date.
-ModMap.resolveHeaderDirectives(File);
-// Modules with builtins are special; multiple modules use builtins as
-// modular headers, example:
-//
-//module stddef { header "stddef.h" export * }
-//
-// After module map parsing, this expands to:
-//
-//module stddef {
-//  header "/path_to_builtin_dirs/stddef.h"
-//  textual "stddef.h"
-//}
-//
-// It's common that libc++ and system modules will both define such
-// submodules. Make sure cached results for a builtin header won't
-// prevent other builtin modules to potentially enter the builtin header.
-// Note that builtins are header guarded and the decision to actually
-// enter them is postponed to the controlling macros logic below.
-bool TryEnterHdr = false;
-if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
-  TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
-ModuleMap::isBuiltinHeader(
-llvm::sys::path::filename(File->getName()));
-
-// Textual headers can be #imported from different modules. Since ObjC
-// headers find in the wild might rely only on #import and do not contain
-// controlling macros, be conservative and only try to enter textual headers
-// if such macro is present.
-if (!FileInfo.isModuleHeader &&
-FileInfo.getControllingMacro(ExternalLookup))
-  TryEnterHdr = true;
-return TryEnterHdr;
-  };
-
   // If this is a #import directive, check that we have not already imported
   // this header.
   if (isImport) {
 // If this has already been imported, don't import it again.
 FileInfo.isImport = true;
+  }
 
-// Has this already been #import'ed or #include'd?
-if (FileInfo.NumIncludes && !TryEnterImported())
-  return false;
-  } else {
-// Otherwise, if this is a #include of a file that was previously #import'd
-// or if this is the second #include of a #pragma once file, ignore it.
-if (FileInfo.isImport && !TryEnterImported())
-  return false;
+  if (FileInfo.isPragmaOnce || FileInfo.isImport) {
+if (PP.isIncludeVisible(File))return false;
+else {
+  // Mark as 'included'.
+  PP.setIncludeVisible(File);
+
+  // Also record the importer.
+  

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D76184#1927159 , @rnk wrote:

> I patched this in with it's dependency, and to compile just Attr.h by itself, 
> it reduces the time from ~2.420s to ~1.920s, so it is worth pushing this 
> through. :)
>
> However, I noticed that there are still some targets broken by the removal of 
> the SmallSet.h include. I'm happy to push that through if you want to hand it 
> off.


I was about to push all three changes but I wans't aware of broken targets. Thx 
for telling me. I would not mind you talking over this patch. I'll merge the 
other two in a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76184



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

[PATCH] D76594: [clang][AST] User relative offsets inside AST file

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

In D76594#1937289 , @sammccall wrote:

> This looks reasonable to me, though I'm not familiar enough with the code to 
> do a good review.
>
> Just wanted to confirm my understanding: the problematic offsets that are now 
> section-relative instead of file-relative are only used in two sections of 
> the file (macro directives and sloc entries). These sections seem likely to 
> be small relative to others that hold e.g. the AST. So this should raise the 
> effective size limit by a lot, more likely 100x than 5?


Yes, problematic offsets are now section relative and the sections several 
times smaller than the whole file. In suspect in some artificial cases these 
section can be pretty large. For example, you can create source file with only 
macro and you can exceed limit for on macro size alone. Moreover I'm not sure 
that files bigger than 4G can be processed correctly. Because there are other 
places where 32 bit numbers are used for some IDs or byte offsets.

> (We've coincidentally started to see large crashing preambles this week, so 
> thanks for this! I think we probably don't need to add any detection/recovery 
> if this limit is high enough)

I added asserts for all new places where overflow can happen but perhaps we 
also need something for release builds. If you have cases when you reach size 
limit, it would be very helpful if you can try this patch on these cases. I 
tested it on couple ~700M files but my examples relatively unified and follow 
the same pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594



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

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think we should remove the LangRef rule that says "sret" pointers have to be 
dereferenceable/naturally aligned, and let the frontend add explicit 
aligned/dereferenceable markings where appropriate.  (At that point, sret has 
no target-independent meaning; it's just to manipulate the target ABI.)  It 
would make the IR easier to understand, and resolves the interaction with 
opaque pointers.

That isn't to say we shouldn't make this change in clang; clang should do this, 
but at that point it would just be a performance enhancement, not required for 
correctness.


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

https://reviews.llvm.org/D74183



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

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

I am curious why opt and llc is not affected


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76631



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

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wuzish.

I'm just going to assume the test changes look good; thank you for taking the 
time to do this.


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

https://reviews.llvm.org/D74183



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

It appears this broke the modules build: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12905/console

Can you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D76384#1936769 , @mibintc wrote:

> In D76384#1934785 , @rjmccall wrote:
>
> > Let's make this patch be purely a representation change.  I wouldn't expect 
> > *any* test changes from that; if there are, we should understand why.  You 
> > can then add the stuff about tracking whether there's a pragma in a 
> > separate patch.
>
>
> OK, some of the tests show ConditionalOperator in the IR matching. Since that 
> operator is necessarily getting merged into BinaryOperator those tests will 
> have to change. I'll see about changing the srcpos for the conditional 
> assignment operator so i can undo the srcpos changes in those tests.


I assume you mean CompoundAssignOperator.  Sure, if there's some sort of debug 
output / AST dump in a test that includes expression node names, obviously 
that's okay to change; and it's possible that the new behavior with 
CompoundAssignOperator not split out is better in some way.  We should just 
make sure we understand any changes we see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384



___
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 a comment.

In D74918#1937291 , @lebedev.ri wrote:

> In D74918#1937237 , @zoecarver wrote:
>
> > @lebedev.ri Where should I put this? Could you maybe point me to another 
> > LLVM target API that clang uses that I could base this off? Maybe it should 
> > go inside the triple or just be a static function?
>
>
> I would have guessed `TargetLoweringInfo`/`TargetTransformInfo`, but i'm not 
> sure if we can access it here without violating layering.


I don't think can be done unless the proposed builtin for this lowered to an 
llvm intrinsic and was handled completely in llvm CodeGen.


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] D76520: [CUDA][HIP] Add -Xarch_device and -Xarch_host options

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D76520#1937294 , @yaxunl wrote:

> -Xarch_ does not work for passing -cc1 options in the beginning. This patch 
> does not change that.
>
> This requires some further changes about how the options after -Xarch_ are 
> handled. I would suggest to do that in another patch.


I'm OK with a separate patch.
Meanwhile we should document that it only works for driver args. Perhaps add a 
TODO comment in the TranslateXarchArgs describing its limitations.


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

https://reviews.llvm.org/D76520



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

[PATCH] D76534: [clang/docs] Fix various sphinx warnings/errors in docs.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Sure.  I'm not asking you to fix this, I'm just asking your opinion about the 
right thing to do.  Is the Sphinx warning reasonable, such that we should fix 
our (autogenerated, IIRC) input to avoid the "redundant" spellings, or should 
we ask Sphinx to handle it more gracefully in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76534



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