[PATCH] D81420: Fix size for _ExtInt types with builtins

[Mott, Jeffrey T via Phabricator via cfe-commits]

jtmott-intel updated this revision to Diff 270482.
jtmott-intel added a comment.

- Removed sign/unsign select.
- Added test and support for placeholder types in builtins.


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

https://reviews.llvm.org/D81420

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-overflow.c
  clang/test/Sema/builtins-overflow.c
  clang/test/Sema/builtins-overflow.m

Index: clang/test/Sema/builtins-overflow.m
===
--- /dev/null
+++ clang/test/Sema/builtins-overflow.m
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+void test() {
+  int z[1];
+  __builtin_add_overflow(1, 1, z);
+}
Index: clang/test/Sema/builtins-overflow.c
===
--- clang/test/Sema/builtins-overflow.c
+++ clang/test/Sema/builtins-overflow.c
@@ -19,4 +19,23 @@
   __builtin_add_overflow(1, 1, 3);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('int' invalid)}}
   __builtin_add_overflow(1, 1, &f);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('float *' invalid)}}
   __builtin_add_overflow(1, 1, &q);  // expected-error {{result argument to overflow builtin must be a pointer to a non-const integer ('const unsigned int *' invalid)}}
+
+  {
+_ExtInt(128) x = 1;
+_ExtInt(128) y = 1;
+_ExtInt(128) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+unsigned _ExtInt(129) x = 1;
+unsigned _ExtInt(129) y = 1;
+unsigned _ExtInt(129) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expect ok
+  }
+  {
+_ExtInt(129) x = 1;
+_ExtInt(129) y = 1;
+_ExtInt(129) result;
+_Bool status = __builtin_mul_overflow(x, y, &result); // expected-error {{__builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits}}
+  }
 }
Index: clang/test/CodeGen/builtins-overflow.c
===
--- clang/test/CodeGen/builtins-overflow.c
+++ clang/test/CodeGen/builtins-overflow.c
@@ -41,6 +41,20 @@
   return r;
 }
 
+int test_add_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_add_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.sadd.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_add_overflow(x, y, &r))
+overflowed();
+  return r;
+}
+
 unsigned test_sub_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_uint_uint_uint
   // CHECK-NOT: ext
@@ -69,6 +83,20 @@
   return r;
 }
 
+int test_sub_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_sub_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.ssub.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_sub_overflow(x, y, &r))
+overflowed();
+  return r;
+}
+
 unsigned test_mul_overflow_uint_uint_uint(unsigned x, unsigned y) {
   // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_uint_uint_uint
   // CHECK-NOT: ext
@@ -97,6 +125,48 @@
   return r;
 }
 
+int test_mul_overflow_xint31_xint31_xint31(_ExtInt(31) x, _ExtInt(31) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint31_xint31_xint31({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i31, i1 } @llvm.smul.with.overflow.i31(i31 %{{.+}}, i31 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i31, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i31, i1 } [[S]], 0
+  // CHECK: store i31 [[Q]], i31*
+  // CHECK: br i1 [[C]]
+  _ExtInt(31) r;
+  if (__builtin_mul_overflow(x, y, &r))
+overflowed();
+  return r;
+}
+
+int test_mul_overflow_xint127_xint127_xint127(_ExtInt(127) x, _ExtInt(127) y) {
+  // CHECK-LABEL: define {{(dso_local )?}}i32 @test_mul_overflow_xint127_xint127_xint127({{.+}})
+  // CHECK-NOT: ext
+  // CHECK: [[S:%.+]] = call { i127, i1 } @llvm.smul.with.overflow.i127(i127 %{{.+}}, i127 %{{.+}})
+  // CHECK-DAG: [[C:%.+]] = extractvalue { i127, i1 } [[S]], 1
+  // CHECK-DAG: [[Q:%.+]] = extractvalue { i127, i1 } [[S]], 0
+  // CHECK: store i127 [[Q]], i127*
+  // CHECK: br i1 [[C]]
+  _ExtInt(

[PATCH] D81757: [WebAssembly] Add intrinsic for i64x2.mul

[Thomas Lively via Phabricator via cfe-commits]

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

This instruction was implemented in 3181273be7 
, but that 
commit did
not add an intrinsic for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81757

Files:
  clang/lib/Headers/wasm_simd128.h


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -864,6 +864,11 @@
   return (v128_t)((__u64x2)__a - (__u64x2)__b);
 }
 
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_mul(v128_t __a,
+   v128_t __b) {
+  return (v128_t)((__u64x2)__a * (__u64x2)__b);
+}
+
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_abs(v128_t __a) {
   return (v128_t)__builtin_wasm_abs_f32x4((__f32x4)__a);
 }


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -864,6 +864,11 @@
   return (v128_t)((__u64x2)__a - (__u64x2)__b);
 }
 
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_mul(v128_t __a,
+   v128_t __b) {
+  return (v128_t)((__u64x2)__a * (__u64x2)__b);
+}
+
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_abs(v128_t __a) {
   return (v128_t)__builtin_wasm_abs_f32x4((__f32x4)__a);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81420: Fix size for _ExtInt types with builtins

[Mott, Jeffrey T via Phabricator via cfe-commits]

jtmott-intel marked an inline comment as done.
jtmott-intel added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:327
   return true;
 TheCall->setArg(2, Arg.get());
   }

rjmccall wrote:
> jtmott-intel wrote:
> > rjmccall wrote:
> > > I know this is existing code, but this is a broken mess.  Please change 
> > > this to:
> > > 
> > > ```
> > >   ExprResult Arg = 
> > > DefaultFunctionArrayLvalueConversion(TheCall->getArg(2));
> > >   if (Arg.isInvalid()) return true;
> > >   TheCall->setArg(2, Arg.get());
> > > 
> > >   QualType Ty = Arg.get()->getType();
> > >   const auto *PtrTy = Ty->getAs();
> > >   if (!PtrTy ||
> > >   !PtrTy->getPointeeType()->isIntegerType() ||
> > >   PtrTy->getPointeeType().isConstQualified()) {
> > > S.Diag(Arg.get()->getBeginLoc(),
> > >diag::err_overflow_builtin_must_be_ptr_int)
> > >   << Ty << Arg.get()->getSourceRange();
> > > return true;
> > >   }
> > > ```
> > > 
> > > Test case would be something like (in ObjC):
> > > 
> > > ```
> > > @interface HasPointer
> > > @property int *pointer;
> > > @end
> > > 
> > > void test(HasPointer *hp) {
> > >   __builtin_add_overflow(x, y, hp.pointer);
> > > }
> > > ```
> > > 
> > > And the earlier block needs essentially the same change (but obviously 
> > > checking for a different type).  You can't check types before doing 
> > > placeholder conversions, and once you do l-value conversions and a 
> > > type-check, you really don't need the full parameter-initialization logic.
> > I've implemented this locally but I have a quick question about the test. 
> > It passed even before I applied this code change. Is this test expected to 
> > fail before this change and pass after? Or is it just to prove that the 
> > feature (placeholder argument types?) works in general?
> Oh, we seem to handle placeholders specially, nevermind.  Well, what I said 
> about doing conversions before checking the type still holds, but the test 
> case is just this:
> 
> ```
> int z[1];
> __builtin_add_overflow(x, y, z);
> ```
> 
> where we should decay the argument to a pointer instead of rejecting it.
Done. Latest updated patch includes these code changes.


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

https://reviews.llvm.org/D81420



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

[PATCH] D81752: Revert "[analyzer][NFC] Don't allow dependency checkers to emit diagnostics"

[Sterling Augustine via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe64059828f19: Revert "[analyzer][NFC] Don't allow 
dependency checkers to emit diagnostics" (authored by saugustine).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81752

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -94,10 +94,6 @@
 // Methods of CmdLineOption, PackageInfo and CheckerInfo.
 //===--===//
 
-LLVM_DUMP_METHOD void CheckerRegistry::CmdLineOption::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::CmdLineOption::dumpToStream(llvm::raw_ostream &Out) const {
   // The description can be just checked in Checkers.inc, the point here is to
@@ -119,10 +115,6 @@
   llvm_unreachable("Unhandled CheckerRegistry::StateFromCmdLine enum");
 }
 
-LLVM_DUMP_METHOD void CheckerRegistry::CheckerInfo::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream &Out) const {
   // The description can be just checked in Checkers.inc, the point here is to
@@ -145,10 +137,6 @@
   }
 }
 
-LLVM_DUMP_METHOD void CheckerRegistry::PackageInfo::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::PackageInfo::dumpToStream(llvm::raw_ostream &Out) const {
   Out << FullName << "\n";
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -40,7 +40,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -2107,32 +2106,6 @@
 // Methods for BugReport and subclasses.
 //===--===//
 
-static bool isDependency(const CheckerRegistry &Registry,
- StringRef CheckerName) {
-  for (const std::pair &Pair : Registry.Dependencies) {
-if (Pair.second == CheckerName)
-  return true;
-  }
-  return false;
-}
-
-PathSensitiveBugReport::PathSensitiveBugReport(
-const BugType &bt, StringRef shortDesc, StringRef desc,
-const ExplodedNode *errorNode, PathDiagnosticLocation LocationToUnique,
-const Decl *DeclToUnique)
-: BugReport(Kind::PathSensitive, bt, shortDesc, desc), ErrorNode(errorNode),
-  ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : SourceRange()),
-  UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
-  assert(!isDependency(ErrorNode->getState()
-   ->getAnalysisManager()
-   .getCheckerManager()
-   ->getCheckerRegistry(),
-   bt.getCheckerName()) &&
- "Some checkers depend on this one! We don't allow dependency "
- "checkers to emit warnings, because checkers should depend on "
- "*modeling*, not *diagnostics*.");
-}
-
 void PathSensitiveBugReport::addVisitor(
 std::unique_ptr visitor) {
   if (!visitor)
@@ -2221,12 +2194,12 @@
   return;
 case bugreporter::TrackingKind::Condition:
   return;
-}
+  }
 
-llvm_unreachable(
-"BugReport::markInteresting currently can only handle 2 different "
-"tracking kinds! Please define what tracking kind should this entitiy"
-"have, if it was already marked as interesting with a different kind!");
+  llvm_unreachable(
+  "BugReport::markInteresting currently can only handle 2 different "
+  "tracking kinds! Please define what tracking kind should this entitiy"
+  "have, if it was already marked as interesting with a different kind!");
 }
 
 void PathSensitiveBugReport::markInteresting(SymbolRef sym,
Index: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -135,7 +135,7 @@
  "Invalid development status!");
 }
 
-LLVM_DUMP_METHOD void dump() const;
+L

[PATCH] D81627: [HIP] Do not call opt/llc for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D81627#2090499 , @tra wrote:

> LGTM.  Good to go if @arsenm is OK with fixing -fgpu-rdc in a separate patch.


@arsenm Are you OK with fixing -fgpu-rdc in a separate patch? The fix for that 
is orthogonal to the current patch. Mixing them together will clutter things 
up. Also the need for -fgpu-rdc is not so urgent since only rccl needs it. 
Requesting this patch to fix -fgpu-rdc will delay fixing issues for all other 
math libs and frameworks.


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

https://reviews.llvm.org/D81627



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

[PATCH] D81761: [analyzer] Force dependency checkers to be hidden

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, vsavchenko, dcoughlin, xazax.hun, balazske, 
martong, baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
whisperity.
Szelethus edited the summary of this revision.

Since strong dependencies aren't user-facing (its hardly ever legal to disable 
them), lets enforce that they are hidden. Modeling checkers that aren't 
dependencies are of course not impacted, but there is only so much you can do 
against developers shooting themselves in the foot :^)

I also made some changes to the test files, reversing the "test" package for, 
well, testing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81761

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  
clang/lib/Analysis/plugins/CheckerDependencyHandling/CheckerDependencyHandling.cpp
  clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
  clang/lib/Analysis/plugins/SampleAnalyzer/MainCallChecker.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/test/Analysis/checker-plugins.c
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -41,17 +41,16 @@
 
 void addCustomChecker(AnalysisASTConsumer &AnalysisConsumer,
   AnalyzerOptions &AnOpts) {
-  AnOpts.CheckersAndPackages = {{"custom.CustomChecker", true}};
+  AnOpts.CheckersAndPackages = {{"test.CustomChecker", true}};
   AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-Registry.addChecker("custom.CustomChecker", "Description",
-   "");
+Registry.addChecker("test.CustomChecker", "Description", "");
   });
 }
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
-  EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description\n");
+  EXPECT_EQ(Diags, "test.CustomChecker:Custom diagnostic description\n");
 }
 
 //===--===//
@@ -120,7 +119,7 @@
 void addCheckerRegistrationOrderPrinter(CheckerRegistry &Registry) {
   Registry.addChecker(registerCheckerRegistrationOrderPrinter,
   shouldRegisterCheckerRegistrationOrderPrinter,
-  "custom.RegistrationOrder", "Description", "", false);
+  "test.RegistrationOrder", "Description", "", false);
 }
 
 #define UNITTEST_CHECKER(CHECKER_NAME, DIAG_MSG)   \
@@ -141,7 +140,7 @@
   }\
   void add##CHECKER_NAME(CheckerRegistry &Registry) {  \
 Registry.addChecker(register##CHECKER_NAME, shouldRegister##CHECKER_NAME,  \
-"custom." #CHECKER_NAME, "Description", "", false);\
+"test." #CHECKER_NAME, "Description", "", false);  \
   }
 
 UNITTEST_CHECKER(StrongDep, "Strong")
@@ -154,22 +153,22 @@
 
 void addDep(AnalysisASTConsumer &AnalysisConsumer,
   AnalyzerOptions &AnOpts) {
-  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
-{"custom.RegistrationOrder", true}};
+  AnOpts.CheckersAndPackages = {{"test.Dep", true},
+{"test.RegistrationOrder", true}};
   AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
 Registry.addChecker(registerStrongDep, shouldRegisterStrongFALSE,
-"custom.Strong", "Description", "", false);
+"test.Strong", "Description", "", false);
 addStrongDep(Registry);
 addDep(Registry);
 addCheckerRegistrationOrderPrinter(Registry);
-Registry.addDependency("custom.Dep", "custom.Strong");
+Registry.addDependency("test.Dep", "test.Strong");
   });
 }
 
 TEST(RegisterDeps, UnsatisfiedDependency) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "custom.RegistrationOrder:custom.RegistrationOrder\n");
+  EXPECT_EQ(Diags, "test.RegistrationOrder:test.RegistrationOrder\n");
 }
 
 //===--===//
@@ -180,52 +179,52 @@
 
 void addWeakDepCheckerBothEnabled(AnalysisASTConsumer &AnalysisConsumer,
   AnalyzerOptions &AnOpts) {
-  AnOpts.CheckersAndPackages = {{"custom.Dep", true},
-{"custom.WeakDep", true},
-{"custom.RegistrationOrder", true}};
+  AnOp

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

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

Reverted in rGe64059828f19f629081220bffeb3ef7582870111 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78126



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

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

Yeah, the restriction to the header file is a bug.  It was a misconception on 
my part when I originally wrote the matcher.

See https://bugs.llvm.org/show_bug.cgi?id=26332 which should also be fixed by 
the change here.

I had attempted to make progress on fixing that bug (26332) but some retooling 
of the testing framework is needed to validate
that the **header** has a fixit applied to it.  I made some progress by 
refactoring the python script, but as usual it got bogged
down in clang review hell and I haven't had time to make any more forward 
progress on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81336



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

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553
+/// \endcode
+AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher,
+  InnerMatcher) {

aaron.ballman wrote:
> njames93 wrote:
> > jkorous wrote:
> > > aaron.ballman wrote:
> > > > jkorous wrote:
> > > > > Nit: while "[base specifier] `hasType`" sounds natural to me for some 
> > > > > reason `hasClass` doesn't. English is not my first language though.
> > > > I agree that `hasClass` seems unnatural here. Out of curiosity, could 
> > > > we modify the `hasName` matcher to work on base specifiers so you can 
> > > > write: `cxxRecordDecl(hasAnyBase(hasName("Base")))` as shorthand for 
> > > > the more wordy version 
> > > > `cxxRecordDecl(hasAnyBase(hasType(cxxRecordDecl(hasName("Base")`?
> > > Wouldn't it be strange to treat `hasName` differently than all the other 
> > > narrowing matchers? Honest question - I feel that `hasName` might be the 
> > > most commonly used, just don't know if that's enough to justify this.
> > > https://clang.llvm.org/docs/LibASTMatchersReference.html#narrowing-matchers
> > Repurposing `hasName` would be a pain especially considering 99% of its use 
> > cases wont be for base class matching.
> > Wouldn't it be strange to treat hasName differently than all the other 
> > narrowing matchers? Honest question - I feel that hasName might be the most 
> > commonly used, just don't know if that's enough to justify this. 
> > https://clang.llvm.org/docs/LibASTMatchersReference.html#narrowing-matchers
> 
> Different how? I'm suggesting to overload `hasName` to work on a 
> `CXXBaseSpecifier` since those have a name.
> 
> > Repurposing hasName would be a pain especially considering 99% of its use 
> > cases wont be for base class matching.
> 
> I'm asking what the right API is for users, though, which is a bit different. 
> Base specifiers have names (there are no unnamed base specifiers), so to me, 
> it makes more sense for `hasName` to work with them directly since that is 
> the thing that does name matching.
> 
> I think you can accomplish this by using a `PolymorphicMatcherWithParam1` 
> like we do for `hasOverloadedOperatorName` which can narrow to either a 
> `CXXOperatorCallExpr` or a `FunctionDecl`.
>> Wouldn't it be strange to treat hasName differently than all the other 
>> narrowing matchers? Honest question - I feel that hasName might be the most 
>> commonly used, just don't know if that's enough to justify this. 
>> https://clang.llvm.org/docs/LibASTMatchersReference.html#narrowing-matchers

> Different how? I'm suggesting to overload hasName to work on a 
> CXXBaseSpecifier since those have a name.

What I meant is that technically we can overload any `Matcher` 
matcher in the same fashion so having the overloaded version of `hasName` only 
makes it somewhat special (and someone might argue that it'd impact consistency 
of matchers composability). Anyway, I'm fine with your suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81552



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

[clang] d2c394e - [WebAssembly] Add intrinsic for i64x2.mul

[Thomas Lively via cfe-commits]

Author: Thomas Lively
Date: 2020-06-12T14:08:18-07:00
New Revision: d2c394e74fc5eb33581b58f238a745d7dd18f219

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

LOG: [WebAssembly] Add intrinsic for i64x2.mul

Summary:
This instruction was implemented in 3181273be7, but that commit did
not add an intrinsic for it.

Reviewers: aheejin

Subscribers: dschuff, sbc100, jgravelle-google, sunfish, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Headers/wasm_simd128.h

Removed: 




diff  --git a/clang/lib/Headers/wasm_simd128.h 
b/clang/lib/Headers/wasm_simd128.h
index 258198a7de34..b78123834b64 100644
--- a/clang/lib/Headers/wasm_simd128.h
+++ b/clang/lib/Headers/wasm_simd128.h
@@ -864,6 +864,11 @@ static __inline__ v128_t __DEFAULT_FN_ATTRS 
wasm_i64x2_sub(v128_t __a,
   return (v128_t)((__u64x2)__a - (__u64x2)__b);
 }
 
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_mul(v128_t __a,
+   v128_t __b) {
+  return (v128_t)((__u64x2)__a * (__u64x2)__b);
+}
+
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_abs(v128_t __a) {
   return (v128_t)__builtin_wasm_abs_f32x4((__f32x4)__a);
 }



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

[PATCH] D81420: Fix size for _ExtInt types with builtins

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.

LGTM, thanks!


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

https://reviews.llvm.org/D81420



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

[PATCH] D81757: [WebAssembly] Add intrinsic for i64x2.mul

[Thomas Lively via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2c394e74fc5: [WebAssembly] Add intrinsic for i64x2.mul 
(authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81757

Files:
  clang/lib/Headers/wasm_simd128.h


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -864,6 +864,11 @@
   return (v128_t)((__u64x2)__a - (__u64x2)__b);
 }
 
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_mul(v128_t __a,
+   v128_t __b) {
+  return (v128_t)((__u64x2)__a * (__u64x2)__b);
+}
+
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_abs(v128_t __a) {
   return (v128_t)__builtin_wasm_abs_f32x4((__f32x4)__a);
 }


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -864,6 +864,11 @@
   return (v128_t)((__u64x2)__a - (__u64x2)__b);
 }
 
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i64x2_mul(v128_t __a,
+   v128_t __b) {
+  return (v128_t)((__u64x2)__a * (__u64x2)__b);
+}
+
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_f32x4_abs(v128_t __a) {
   return (v128_t)__builtin_wasm_abs_f32x4((__f32x4)__a);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 270517.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961

Files:
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
  clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTDumper.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/AST/ASTContextParentMapTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/AST/SourceLocationTest.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
  clang/unittests/Sema/GslOwnerPointerInference.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -695,6 +695,70 @@
   EXPECT_EQ(ErrorCount, 0);
 }
 
+TEST_F(TransformerTest, TemplateInstantiation) {
+
+  std::string NonTemplatesInput = R"cpp(
+struct S {
+  int m_i;
+};
+)cpp";
+  std::string NonTemplatesExpected = R"cpp(
+struct S {
+  safe_int m_i;
+};
+)cpp";
+
+  std::string TemplatesInput = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  auto MatchedField = fieldDecl(hasType(asString("int"))).bind("theField");
+
+  // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + TemplatesInput);
+
+  // In AsIs mode, template instantiations are modified, which is
+  // often not desired:
+
+  std::string IncorrectTemplatesExpected = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  safe_int m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_AsIs, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + IncorrectTemplatesExpected);
+}
+
 // Transformation of macro source text when the change encompasses the entirety
 // of the expanded text.
 TEST_F(TransformerTest, SimpleMacro) {
Index: clang/unittests/Sema/GslOwnerPointerInference.cpp
===
--- clang/unittests/Sema/GslOwnerPointerInference.cpp
+++ clang/unittests/Sema/GslOwnerPointerInference.cpp
@@ -14,42 +14,45 @@
 using namespace ast_matchers;
 
 TEST(OwnerPointer, BothHaveAttributes) {
-  EXPECT_TRUE(matches("template"
-  "class [[gsl::Owner]] C;"
+  EXPECT_TRUE(matches(
+  "template"
+  "class [[gsl::Owner]] C;"
 
-  "template"
-  "class [[gsl::Owner]] C {};"
+  "template"
+  "class [[gsl::Owner]] C {};"
 
-  "C c;",
-  classTemplateSpecializationDecl(
-  hasName("C"), hasAttr(clang::attr::Owner;
+

[PATCH] D80751: [clang][diagnostics] Add '-Wundef-prefix' warning option

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:108
+def Undefined : DiagGroup<"undef">;
+def UndefinedPrefix : DiagGroup<"undef-prefix", [Undefined]>;
 def UnsupportedNan : DiagGroup<"unsupported-nan">;

zixuw wrote:
> arphaman wrote:
> > It seems like you're not using the `UndefinedPrefix` group in this patch. 
> > Is that intentional, or was this left in the patch by mistake?
> This is intentional to map both `Wundef` and `Wundef-prefix` to the same 
> warning. And since `Wundef` is changed to an alias, the `Undefined` subgroup 
> is used to keep existing outputs/behaviors consistent (to make 
> `-Werror=undef` work, and to keep the `[-Wundef]` option name in the 
> diagnostic message).
Got it, thanks!



Comment at: clang/lib/Lex/PPExpressions.cpp:262
+  // string to UndefPrefixes as an explicit "-Wundef" does.
+  if (UndefPrefixes.empty() ||
+  llvm::any_of(UndefPrefixes,

zixuw wrote:
> ributzka wrote:
> > What happens when you use `-Werror=undef` and `-Wundef-prefix=`?
> Then we only get errors for `undef-prefix=`. Yes this needs to be fixed.
> Perhaps a better way to handle the `-Werror=undef` implication of `-Wundef`.
We should try to fix this in this PR then. Can you check the severity of the 
diagnostic, and if it's an error always emit it here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80751



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

[PATCH] D81769: Repair various issues with modernize-avoid-bind

[Jeff Trull via Phabricator via cfe-commits]

jaafar created this revision.
jaafar added reviewers: aaron.ballman, alexfh, hokein.
jaafar added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In the process of running this check on a large codebase 
 I found a number of limitations, and 
thought I would pass on my fixes for possible integration upstream:

1. Templated function call operators are not supported
2. Function object constructors are always used directly in the lambda body, 
even if their arguments are not captured
3. Placeholders with namespace qualifiers (`std::placeholders::_1`) are not 
detected
4. Lambda arguments should be forwarded to the stored function
5. Data members from other classes still get captured with `this`
6. Expressions (as opposed to variables) inside `std::ref` are not captured 
properly
7. Function object templates sometimes have their template arguments replaced 
with concrete types

This patch resolves all those issues and adds suitable unit tests.

If desired, I can separate these commits out into 7 separate diffs, but it 
seemed like it might be easier to evaluate them all at once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81769

Files:
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind-permissive-parameter-list.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
@@ -7,11 +7,11 @@
 
 template 
 bind_rt bind(Fp &&, Arguments &&...);
-}
+} // namespace impl
 
 template 
 T ref(T &t);
-}
+} // namespace std
 
 namespace boost {
 template 
@@ -58,12 +58,33 @@
 
 void UseF(F);
 
+struct G {
+  G() : _member(0) {}
+  G(int m) : _member(m) {}
+
+  template 
+  void operator()(T) const {}
+
+  int _member;
+};
+
+template 
+struct H {
+  void operator()(T) const {};
+};
+
 struct placeholder {};
 placeholder _1;
 placeholder _2;
 
+namespace placeholders {
+using ::_1;
+using ::_2;
+} // namespace placeholders
+
 int add(int x, int y) { return x + y; }
 int addThree(int x, int y, int z) { return x + y + z; }
+void sub(int &x, int y) { x += y; }
 
 // Let's fake a minimal std::function-like facility.
 namespace std {
@@ -107,6 +128,7 @@
   int MemberVariable;
   static int StaticMemberVariable;
   F MemberStruct;
+  G MemberStructWithData;
 
   void testCaptureByValue(int Param, F f) {
 int x = 3;
@@ -145,6 +167,11 @@
 auto GGG = boost::bind(UseF, MemberStruct);
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to boost::bind [modernize-avoid-bind]
 // CHECK-FIXES: auto GGG = [this] { return UseF(MemberStruct); };
+
+auto HHH = std::bind(add, MemberStructWithData._member, 1);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind
+// Correctly distinguish data members of other classes
+// CHECK-FIXES: auto HHH = [capture0 = MemberStructWithData._member] { return add(capture0, 1); };
   }
 };
 
@@ -217,17 +244,38 @@
   auto EEE = std::bind(*D::create(), 1, 2);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
   // CHECK-FIXES: auto EEE = [Func = *D::create()] { return Func(1, 2); };
+
+  auto FFF = std::bind(G(), 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Templated function call operators may be used
+  // CHECK-FIXES: auto FFF = [] { return G()(1); };
+
+  int CTorArg = 42;
+  auto GGG = std::bind(G(CTorArg), 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Function objects with constructor arguments should be captured
+  // CHECK-FIXES: auto GGG = [Func = G(CTorArg)] { return Func(1); };
 }
 
+template 
+void testMemberFnOfClassTemplate(T) {
+  auto HHH = std::bind(H(), 42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+  // Ensure function class template arguments are preserved
+  // CHECK-FIXES: auto HHH = [] { return H()(42); };
+}
+
+template void testMemberFnOfClassTemplate(int);
+
 void testPlaceholders() {
   int x = 2;
   auto AAA = std::bind(add, x, _1);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto AAA = [x](auto && PH1) { return add(x, PH1); };
+  // CHECK-FIXES: auto AAA = [x](auto && PH1) { return add(x, std::forward(PH1)); };
 
   auto BBB = std::bind(add, _2, _1);
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
-  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && PH2) { return add(PH2, PH1); };
+  // CHECK-FIXES: auto BBB = [](auto && PH1, auto && PH2) { return add(std::forward(PH2), std::forward(PH1)); };
 
   // No fix is 

[PATCH] D81769: Repair various issues with modernize-avoid-bind

[Jeff Trull via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:38
 enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
-enum CaptureMode { CM_None, CM_ByRef, CM_ByValue, CM_InitExpression };
+enum CaptureMode { CM_None, CM_ByRef, CM_ByValue };
+enum CaptureExpr { CE_None, CE_Var, CE_InitExpression };

"Captured by reference" and "uses init expression" are not mutually exclusive - 
that gave rise to item 6 above.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:150
 
+static bool tryCaptureAsLocalVariable(const MatchFinder::MatchResult &Result,
+  BindArgument &B, const Expr *E);

These forward declarations seemed to make the diffs a lot easier to read. It's 
also possible to move the two functions into this position, of course.



Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:243
+  if ((std::distance(ME->child_begin(), ME->child_end()) == 1) &&
+  isa(*ME->children().begin())) {
+// reference to data member without explicit "this"

Is there a better way to express this? "a single child of type ThisExpr" was 
what I was going for...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81769



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

[PATCH] D81242: [StackSafety] Run ThinLTO

[Evgenii Stepanov via Phabricator via cfe-commits]

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

LGTM with 2 notes




Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:618
+ConstantRange Access = Found->sextOrTrunc(Use.Range.getBitWidth());
+if (Access.signedAddMayOverflow(C.Offset) !=
+ConstantRange::OverflowResult::NeverOverflows)

vitalybuka wrote:
> eugenis wrote:
> > Do we have a test for this overflow check?
> yes
> Example, in bit width 8
> [-128,-127)+[-128,-127) = [0,1)
> both non-wrapped and result is non-wrapped so we have no way to spot overflow
Sure. I was asking if we have a testcase that covers the overflow check.



Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:926
+  }
+  // Reset data for all sammaries. Alive and DSO local will be set back 
from
+  // of data flow results below. Anything else will not be accessed

typo: summaries


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81242



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

[PATCH] D81771: [CUDA,HIP] Use VFS for SDK detection.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
tra added reviewers: yaxunl, arsenm.
Herald added subscribers: kerbowa, sanjoy.google, bixia, nhaehnle, wdng, 
jvesely.
Herald added a project: clang.
tra updated this revision to Diff 270533.
tra edited the summary of this revision.
tra added a comment.

Replaced another use of D.getVFS.


It's useful for using clang from tools that may need need to provide SDK files
from non-standard locations.

Clang CLI only provides `-ivfsoverlay` for include files, but not for the 
driver's use in general, so there's no
good way to test this yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81771

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -78,6 +78,7 @@
 
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
+  auto &FS = D.getVFS();
 
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 Candidates.emplace_back(
@@ -114,7 +115,7 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-Distro Dist(D.getVFS(), llvm::Triple(llvm::sys::getProcessTriple()));
+Distro Dist(FS, llvm::Triple(llvm::sys::getProcessTriple()));
 if (Dist.IsDebian() || Dist.IsUbuntu())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505
@@ -125,14 +126,13 @@
 
   for (const auto &Candidate : Candidates) {
 InstallPath = Candidate.Path;
-if (InstallPath.empty() || !D.getVFS().exists(InstallPath))
+if (InstallPath.empty() || !FS.exists(InstallPath))
   continue;
 
 BinPath = InstallPath + "/bin";
 IncludePath = InstallPath + "/include";
 LibDevicePath = InstallPath + "/nvvm/libdevice";
 
-auto &FS = D.getVFS();
 if (!(FS.exists(IncludePath) && FS.exists(BinPath)))
   continue;
 bool CheckLibDevice = (!NoCudaLib || Candidate.StrictChecking);
@@ -177,7 +177,8 @@
   }
 } else {
   std::error_code EC;
-  for (llvm::sys::fs::directory_iterator LI(LibDevicePath, EC), LE;
+  for (llvm::vfs::directory_iterator LI = FS.dir_begin(LibDevicePath, EC),
+ LE;
!EC && LI != LE; LI = LI.increment(EC)) {
 StringRef FilePath = LI->path();
 StringRef FileName = llvm::sys::path::filename(FilePath);
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -27,7 +27,9 @@
   const StringRef Suffix(".bc");
 
   std::error_code EC;
-  for (llvm::sys::fs::directory_iterator LI(LibDevicePath, EC), LE;
+  for (llvm::vfs::directory_iterator
+   LI = D.getVFS().dir_begin(LibDevicePath, EC),
+   LE;
!EC && LI != LE; LI = LI.increment(EC)) {
 StringRef FilePath = LI->path();
 StringRef FileName = llvm::sys::path::filename(FilePath);
@@ -137,11 +139,12 @@
 LibDevicePath = LibPathEnv;
   }
 
+  auto &FS = D.getVFS();
   if (!LibDevicePath.empty()) {
 // Maintain compatability with HIP flag/envvar pointing directly at the
 // bitcode library directory. This points directly at the library path 
instead
 // of the rocm root installation.
-if (!D.getVFS().exists(LibDevicePath))
+if (!FS.exists(LibDevicePath))
   return;
 
 scanLibDevicePath();
@@ -151,7 +154,7 @@
 
   for (const auto &Candidate : Candidates) {
 InstallPath = Candidate.Path;
-if (InstallPath.empty() || !D.getVFS().exists(InstallPath))
+if (InstallPath.empty() || !FS.exists(InstallPath))
   continue;
 
 // The install path situation in old versions of ROCm is a real mess, and
@@ -167,8 +170,6 @@
 llvm::sys::path::append(IncludePath, InstallPath, "include");
 llvm::sys::path::append(LibDevicePath, InstallPath, "amdgcn", "bitcode");
 
-auto &FS = D.getVFS();
-
 // We don't need the include path for OpenCL, since clang already ships 
with
 // the default header.
 


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -78,6 +78,7 @@
 
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
+  auto &FS = D.getVFS();
 
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 Candidates.emplace_back(
@@ -114,7 +115,7 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-Distro Dist(D.getVFS(), llvm::Triple(llvm::sys::getProcessTriple(

[PATCH] D81771: [CUDA,HIP] Use VFS for SDK detection.

[Artem Belevich via Phabricator via cfe-commits]

tra updated this revision to Diff 270533.
tra edited the summary of this revision.
tra added a comment.

Replaced another use of D.getVFS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81771

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -78,6 +78,7 @@
 
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
+  auto &FS = D.getVFS();
 
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 Candidates.emplace_back(
@@ -114,7 +115,7 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-Distro Dist(D.getVFS(), llvm::Triple(llvm::sys::getProcessTriple()));
+Distro Dist(FS, llvm::Triple(llvm::sys::getProcessTriple()));
 if (Dist.IsDebian() || Dist.IsUbuntu())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505
@@ -125,14 +126,13 @@
 
   for (const auto &Candidate : Candidates) {
 InstallPath = Candidate.Path;
-if (InstallPath.empty() || !D.getVFS().exists(InstallPath))
+if (InstallPath.empty() || !FS.exists(InstallPath))
   continue;
 
 BinPath = InstallPath + "/bin";
 IncludePath = InstallPath + "/include";
 LibDevicePath = InstallPath + "/nvvm/libdevice";
 
-auto &FS = D.getVFS();
 if (!(FS.exists(IncludePath) && FS.exists(BinPath)))
   continue;
 bool CheckLibDevice = (!NoCudaLib || Candidate.StrictChecking);
@@ -177,7 +177,8 @@
   }
 } else {
   std::error_code EC;
-  for (llvm::sys::fs::directory_iterator LI(LibDevicePath, EC), LE;
+  for (llvm::vfs::directory_iterator LI = FS.dir_begin(LibDevicePath, EC),
+ LE;
!EC && LI != LE; LI = LI.increment(EC)) {
 StringRef FilePath = LI->path();
 StringRef FileName = llvm::sys::path::filename(FilePath);
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -27,7 +27,9 @@
   const StringRef Suffix(".bc");
 
   std::error_code EC;
-  for (llvm::sys::fs::directory_iterator LI(LibDevicePath, EC), LE;
+  for (llvm::vfs::directory_iterator
+   LI = D.getVFS().dir_begin(LibDevicePath, EC),
+   LE;
!EC && LI != LE; LI = LI.increment(EC)) {
 StringRef FilePath = LI->path();
 StringRef FileName = llvm::sys::path::filename(FilePath);
@@ -137,11 +139,12 @@
 LibDevicePath = LibPathEnv;
   }
 
+  auto &FS = D.getVFS();
   if (!LibDevicePath.empty()) {
 // Maintain compatability with HIP flag/envvar pointing directly at the
 // bitcode library directory. This points directly at the library path 
instead
 // of the rocm root installation.
-if (!D.getVFS().exists(LibDevicePath))
+if (!FS.exists(LibDevicePath))
   return;
 
 scanLibDevicePath();
@@ -151,7 +154,7 @@
 
   for (const auto &Candidate : Candidates) {
 InstallPath = Candidate.Path;
-if (InstallPath.empty() || !D.getVFS().exists(InstallPath))
+if (InstallPath.empty() || !FS.exists(InstallPath))
   continue;
 
 // The install path situation in old versions of ROCm is a real mess, and
@@ -167,8 +170,6 @@
 llvm::sys::path::append(IncludePath, InstallPath, "include");
 llvm::sys::path::append(LibDevicePath, InstallPath, "amdgcn", "bitcode");
 
-auto &FS = D.getVFS();
-
 // We don't need the include path for OpenCL, since clang already ships 
with
 // the default header.
 


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -78,6 +78,7 @@
 
   // In decreasing order so we prefer newer versions to older versions.
   std::initializer_list Versions = {"8.0", "7.5", "7.0"};
+  auto &FS = D.getVFS();
 
   if (Args.hasArg(clang::driver::options::OPT_cuda_path_EQ)) {
 Candidates.emplace_back(
@@ -114,7 +115,7 @@
 for (const char *Ver : Versions)
   Candidates.emplace_back(D.SysRoot + "/usr/local/cuda-" + Ver);
 
-Distro Dist(D.getVFS(), llvm::Triple(llvm::sys::getProcessTriple()));
+Distro Dist(FS, llvm::Triple(llvm::sys::getProcessTriple()));
 if (Dist.IsDebian() || Dist.IsUbuntu())
   // Special case for Debian to have nvidia-cuda-toolkit work
   // out of the box. More info on http://bugs.debian.org/882505
@@ -125,14 +126,13 @@
 
   for (const auto &Candidate : Candidates) {
 InstallPath = Candidate.Path;
-i

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth accepted this revision.
amccarth added a comment.

My only real concern was about the possible reproducibility impact of 
introducing absolute paths into the binaries.  If @thakis and @hans are 
satisfied this is OK, then LGTM.




Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:8
+// CHECK: 
+// CHECK: 0x[[PWD:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: 
[[PWDVAL:.+]]
+// CHECK: 0x[[FILEPATH:.+]] | LF_STRING_ID [size = {{.+}}] ID: , 
String: [[FILEPATHVAL:.+[\\/]debug-info-codeview-buildinfo.c]]

aganea wrote:
> amccarth wrote:
> > PWD?  Did you mean CWD (current working directory)?
> I meant the current working directory when the program starts. I was under 
> the impression that the nomenclature for that is PWD = CWD at startup. While 
> CWD is the current directory at any point in time during the execution, and 
> can be different of PWD.
> Would you prefer if I changed the regex capture name to CWD?
Sorry, I always misread "pwd" as "password."




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D80751: [clang][diagnostics] Add '-Wundef-prefix' warning option

[Zixu Wang via Phabricator via cfe-commits]

zixuw added inline comments.



Comment at: clang/lib/Lex/PPExpressions.cpp:262
+  // string to UndefPrefixes as an explicit "-Wundef" does.
+  if (UndefPrefixes.empty() ||
+  llvm::any_of(UndefPrefixes,

arphaman wrote:
> zixuw wrote:
> > ributzka wrote:
> > > What happens when you use `-Werror=undef` and `-Wundef-prefix=`?
> > Then we only get errors for `undef-prefix=`. Yes this needs to be 
> > fixed.
> > Perhaps a better way to handle the `-Werror=undef` implication of `-Wundef`.
> We should try to fix this in this PR then. Can you check the severity of the 
> diagnostic, and if it's an error always emit it here?
That is an option. But then we lose the ability to intentionally turning just 
the prefixes into errors, like `-Wundef-prefix=FOO -Werror` or 
`-Wundef-prefix=BAR -Werror=undef-prefix`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80751



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7821
+  Context.getTypeSizeInChars(ElementType).getQuantity();
+} else if (VAT) {
+  ElementType = VAT->getElementType().getTypePtr();

ABataev wrote:
> What if the base is a pointer, not an array?
The `if (ElementType)` condition only push back stride when base is not 
pointer. I'm now allowing one dimension size to be unknown (pointer as base) 
and sema has analysis to check if more than one indirection as base. My last 
codegen test case is for testing pointer as base.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7831-7838
+  llvm::Value *SizeV = nullptr;
+  if (CAT) {
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {
+const Expr *Size = VAT->getSizeExpr();
+SizeV = CGF.EmitScalarExpr(Size);

ABataev wrote:
> The code for `SizeV` must be under the control of the next `if`:
> ```
> if (DimSizes.size() < Components.size() - 1) {
>  
> }
> ```
I don't think I understand this one. Why do you remove SizeV in the if 
condition?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7834
+llvm::APInt Size = CAT->getSize();
+SizeV = llvm::ConstantInt::get(CGF.SizeTy, Size);
+  } else if (VAT) {

ABataev wrote:
> Create directly as of `CGF.Int64Ty` type.
Doing this I'll get assertion error in this exact line if on a 32-bits target.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7872-7873
+  } else {
+Offset = CGF.Builder.CreateIntCast(CGF.EmitScalarExpr(OffsetExpr),
+   CGF.Int64Ty,
+   /*isSigned=*/false);

ABataev wrote:
> Do you really to pass real offsets here? Can we use pointers instead?
Do you mean I should set the type of Offset to Expr*?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D81772: Reduce -Wregister from DefaultError to a default-on warning.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is a lot of C++ code in the wild still using 'register', which
will become broken if we switch the default compilation mode to
C++17. A default-on warning seems to get the message across
sufficiently, without unnecessarily breaking code that hasn't asked
for it (e.g. with -Werror). GCC has also chosen to make -Wregister an
on-by-default warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81772

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/deprecated.cpp


Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -27,13 +27,13 @@
 
 void stuff(register int q) {
 #if __cplusplus > 201402L
-  // expected-error@-2 {{ISO C++17 does not allow 'register' storage class 
specifier}}
+  // expected-warning@-2 {{ISO C++17 does not allow 'register' storage class 
specifier}}
 #elif __cplusplus >= 201103L && !defined(NO_DEPRECATED_FLAGS)
   // expected-warning@-4 {{'register' storage class specifier is deprecated}}
 #endif
   register int n;
 #if __cplusplus > 201402L
-  // expected-error@-2 {{ISO C++17 does not allow 'register' storage class 
specifier}}
+  // expected-warning@-2 {{ISO C++17 does not allow 'register' storage class 
specifier}}
 #elif __cplusplus >= 201103L && !defined(NO_DEPRECATED_FLAGS)
   // expected-warning@-4 {{'register' storage class specifier is deprecated}}
 #endif
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -240,7 +240,7 @@
   "and incompatible with C++17">, InGroup;
 def ext_register_storage_class : ExtWarn<
   "ISO C++17 does not allow 'register' storage class specifier">,
-  DefaultError, InGroup;
+  InGroup;
 
 def err_invalid_decl_spec_combination : Error<
   "cannot combine with previous '%0' declaration specifier">;


Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -27,13 +27,13 @@
 
 void stuff(register int q) {
 #if __cplusplus > 201402L
-  // expected-error@-2 {{ISO C++17 does not allow 'register' storage class specifier}}
+  // expected-warning@-2 {{ISO C++17 does not allow 'register' storage class specifier}}
 #elif __cplusplus >= 201103L && !defined(NO_DEPRECATED_FLAGS)
   // expected-warning@-4 {{'register' storage class specifier is deprecated}}
 #endif
   register int n;
 #if __cplusplus > 201402L
-  // expected-error@-2 {{ISO C++17 does not allow 'register' storage class specifier}}
+  // expected-warning@-2 {{ISO C++17 does not allow 'register' storage class specifier}}
 #elif __cplusplus >= 201103L && !defined(NO_DEPRECATED_FLAGS)
   // expected-warning@-4 {{'register' storage class specifier is deprecated}}
 #endif
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -240,7 +240,7 @@
   "and incompatible with C++17">, InGroup;
 def ext_register_storage_class : ExtWarn<
   "ISO C++17 does not allow 'register' storage class specifier">,
-  DefaultError, InGroup;
+  InGroup;
 
 def err_invalid_decl_spec_combination : Error<
   "cannot combine with previous '%0' declaration specifier">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

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

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-

[PATCH] D81041: Use existing path sep style in clang::FileManager::FixupRelativePath

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

In D81041#2072396 , @ctetreau wrote:

> After some further investigation, I have come to believe that the root cause 
> of the issue I am seeing is on line 783 of clang/lib/Lex/HeaderSearch.cpp. A 
> path is constructed using string concatenation (dir + '/' + file), which is 
> obviously not robust to the various issues in path construction. A fix had 
> been committed and reverted back in 2015.


When I was fixing portability problems with VFS paths, I started out by trying 
to make paths canonical, and that always led to roadblocks.  A clang developer 
told me that clang philosophically does not try to do any regularization of 
paths.  It turns out that's actually key to making VFS paths viable.  Since 
they can truly consist of a mix of styles, there is no "correct" canonical 
form.  Once I took that approach, most of the VFS portability problems were 
simple to fix without inflicting collateral damage.  So I'm not surprised that 
the 2015 "fix" causes problems.

I'm happy to look at future proposals, and I'll CC myself on that bug report.  
But since you've said you have other priorities now, I'll treat this patch as 
dormant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81041



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

[PATCH] D81721: [SVE] Ensure proper mangling of ACLE tuple types

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81721



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

[PATCH] D81774: [PowerPC][Power10] Implement VSX PCV Generate Operations in LLVM/Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: nemanjai, saghir, power-llvm-team, PowerPC, hfinkel.
amyk added projects: LLVM, clang, PowerPC.
Herald added subscribers: shchenz, hiraditya.

This patch implements builtins for the following prototypes for the VSX Permute 
Control Vector Generate with Mask Instructions:

  vector unsigned char vec_genpcvm (vector unsigned char, const int);
  vector unsigned short vec_genpcvm (vector unsigned short, const int);
  vector unsigned int vec_genpcvm (vector unsigned int, const int);
  vector unsigned long long vec_genpcvm (vector unsigned long long, const int);

Depends on D80935 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81774

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vsx-pcv.ll
  llvm/test/MC/Disassembler/PowerPC/p10insts.txt
  llvm/test/MC/PowerPC/p10.s

Index: llvm/test/MC/PowerPC/p10.s
===
--- llvm/test/MC/PowerPC/p10.s
+++ llvm/test/MC/PowerPC/p10.s
@@ -15,3 +15,15 @@
 # CHECK-BE: pextd 1, 2, 4 # encoding: [0x7c,0x41,0x21,0x78]
 # CHECK-LE: pextd 1, 2, 4 # encoding: [0x78,0x21,0x41,0x7c]
 pextd 1, 2, 4
+# CHECK-BE: xxgenpcvbm 0, 1, 2# encoding: [0xf0,0x02,0x0f,0x28]
+# CHECK-LE: xxgenpcvbm 0, 1, 2# encoding: [0x28,0x0f,0x02,0xf0]
+xxgenpcvbm 0, 1, 2
+# CHECK-BE: xxgenpcvhm 0, 1, 2# encoding: [0xf0,0x02,0x0f,0x2a]
+# CHECK-LE: xxgenpcvhm 0, 1, 2# encoding: [0x2a,0x0f,0x02,0xf0]
+xxgenpcvhm 0, 1, 2
+# CHECK-BE: xxgenpcvwm 0, 1, 2# encoding: [0xf0,0x02,0x0f,0x68]
+# CHECK-LE: xxgenpcvwm 0, 1, 2# encoding: [0x68,0x0f,0x02,0xf0]
+xxgenpcvwm 0, 1, 2
+# CHECK-BE: xxgenpcvdm 0, 1, 2# encoding: [0xf0,0x02,0x0f,0x6a]
+# CHECK-LE: xxgenpcvdm 0, 1, 2# encoding: [0x6a,0x0f,0x02,0xf0]
+xxgenpcvdm 0, 1, 2
Index: llvm/test/MC/Disassembler/PowerPC/p10insts.txt
===
--- llvm/test/MC/Disassembler/PowerPC/p10insts.txt
+++ llvm/test/MC/Disassembler/PowerPC/p10insts.txt
@@ -12,3 +12,15 @@
 
 # CHECK: pextd 1, 2, 4
 0x7c 0x41 0x21 0x78
+
+# CHECK xxgenpcvbm 0, 1, 2
+0xf0 0x02 0x0f 0x28
+
+# CHECK xxgenpcvhm 0, 1, 2
+0xf0 0x02 0x0f 0x2a
+
+# CHECK xxgenpcvwm 0, 1, 2
+0xf0 0x02 0x0f 0x68
+
+# CHECK xxgenpcvdm 0, 1, 2
+0xf0 0x02 0x0f 0x6a
Index: llvm/test/CodeGen/PowerPC/p10-vsx-pcv.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vsx-pcv.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s
+
+; These test cases aim to test the VSX PCV Generate Operations on Power10.
+
+declare <16 x i8> @llvm.ppc.vsx.xxgenpcvbm(<16 x i8>, i32)
+declare <8 x i16> @llvm.ppc.vsx.xxgenpcvhm(<8 x i16>, i32)
+declare <4 x i32> @llvm.ppc.vsx.xxgenpcvwm(<4 x i32>, i32)
+declare <2 x i64> @llvm.ppc.vsx.xxgenpcvdm(<2 x i64>, i32)
+
+define <16 x i8> @test_xxgenpcvbm(<16 x i8> %a) {
+; CHECK-LABEL: test_xxgenpcvbm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxgenpcvbm v2, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %gen = tail call <16 x i8> @llvm.ppc.vsx.xxgenpcvbm(<16 x i8> %a, i32 1)
+  ret <16 x i8> %gen
+}
+
+define <8 x i16> @test_xxgenpcvhm(<8 x i16> %a) {
+; CHECK-LABEL: test_xxgenpcvhm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxgenpcvhm v2, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %gen = tail call <8 x i16> @llvm.ppc.vsx.xxgenpcvhm(<8 x i16> %a, i32 1)
+  ret <8 x i16> %gen
+}
+
+define <4 x i32> @test_xxgenpcvwm(<4 x i32> %a) {
+; CHECK-LABEL: test_xxgenpcvwm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxgenpcvwm v2, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %gen = tail call <4 x i32> @llvm.ppc.vsx.xxgenpcvwm(<4 x i32> %a, i32 1)
+  ret <4 x i32> %gen
+}
+
+define <2 x i64> @test_xxgenpcvdm(<2 x i64> %a) {
+; CHECK-LABEL: test_xxgenpcvdm:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xxgenpcvdm v2, v2, 1
+; CHECK-NEXT:blr
+entry:
+  %gen = tail call <2 x i64> @llvm.ppc.vsx.xxgenpcvdm(<2 x i64> %a, i32 1)
+  ret <2 x i64> %gen
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -161,6 +161,22 @@
   let Inst{48-63} = D_RA{15-0}; // d1
 }
 
+// X-Form: [PO T IMM VRB XO TX]
+class XF

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 270548.
Xiangling_L marked 35 inline comments as done.
Xiangling_L edited the summary of this revision.
Xiangling_L added a comment.

Address another round of reviews;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-constructor-attribute.cpp
  clang/test/CodeGen/aix-destructor-attribute.cpp
  clang/test/CodeGen/aix-init-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,139 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ \
+// RUN: -fno-use-cxa-atexit -std=c++2a < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
 
-struct test {
-  test();
-  ~test();
-} t;
+namespace test1 {
+  struct Test1 {
+Test1();
+~Test1();
+  } t1, t2;
+}; // namespace test1
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+namespace test2 {
+  int foo() { return 3; }
+  int x = foo();
+}; // namespace test2
+
+namespace test3 {
+  struct Test {
+constexpr Test() {};
+~Test() {};
+  };
+
+  constinit Test t;
+}; // namespace test3
+
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry: 
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*)
+
+// CHECK: define internal void @__finalize__ZN5test12t1E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t1E)
+// CHECK:   %needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t1E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*)
+
+// CHECK: define internal void @__cxx_global_var_init.1() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1C1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__finalize__ZN5test12t2E() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor__ZN5test12t2E)
+// CHECK:   %needsDestruct = icmp eq i32 %0, 0
+// CHECK:   br i1 %needsDestruct, label %destruct.call, label %destruct.end
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor__ZN5test12t2E()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.2() #0 {
+// CHECK: entry:
+// CHECK32: %call = call i32 @_ZN5test23fooEv()
+// CHECK64: %call = call signext i32 @_ZN5test23fooEv()
+// CHECK:   store i32 %call, i32* @_ZN5test21xE
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__cxx_global_var_init.3() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor__ZN5test31tE)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define internal void @__dtor__ZN5test31tE() #0 {
+// CHECK: e

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+return;
 

hubert.reinterpretcast wrote:
> Can this part be committed in a separate patch? It does not appear to have 
> dependencies on other parts of this patch and has the appearance of being a 
> possible change for other platforms.
Sure. I will create a NFC patch for this part and try to commit it first. But 
so far, I think I can keep this part in this patch just for review purpose to 
make the patch look nicer?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:704
   AddGlobalDtor(Fn);
+  CXXGlobalDtors.clear();
 }

hubert.reinterpretcast wrote:
> If this is another drive-by fix, can we put it in a separate patch?
Sure, I will put it in the above mentioned clean-up NFC patch as well.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

hubert.reinterpretcast wrote:
> The called function is to be renamed.
Any suggestions here about how to represent the functionality of `__sterm` and 
`_GLOBAL__D_a` into one word? Cannot think of a good name...

Actually I am wondering do we need to rename the `Destruct` part? The `__sterm` 
and `_GLOBAL__D_a` do destruct the object by calling sterm finalizers and 
object destructors?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
 llvm::Function *Fn,

hubert.reinterpretcast wrote:
> This function is to be renamed.
 I will try `GenerateCXXGlobalDestructFunc` based on the thoughts as I also 
mentioned elsewhere that the `__sterm` and `_GLOBAL__D_` function do destruct 
the object by calling sterm finalizers and object destructors.

Any suggestions or concerns about this renaming? Or do we really need to rename 
this one?



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();

hubert.reinterpretcast wrote:
> Suggestion:
> Create the finalization action associated with a variable.
I don't get your point, can you elaborate on this a little bit?



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

jasonliu wrote:
> Looks like the non-inline comments are easier to get ignored and missed, so I 
> will copy paste the non-inlined comment I previously had:
> ```
> -fregister_global_dtors_with_atexit does not seem to work properly in current 
> implementation.
> We should consider somehow disabling/report_fatal_error it instead of letting 
> it generate invalid code on AIX.
> ```
Thanks for doing so!

The semantic of `global_dtors` here on AIX is `__sterm` function, which we are 
never gonna register with `__atexit`. I am thinking we can disable it in a 
follow-up driver patch with the handling of `-fno-use-cxa-atexit`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166



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

[clang] c1e47b4 - [StackSafety] Run ThinLTO

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2020-06-12T18:11:29-07:00
New Revision: c1e47b47f8848bb4c1ead8c530940d783c1fbf46

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

LOG: [StackSafety] Run ThinLTO

Summary:
ThinLTO linking runs dataflow processing on collected
function parameters. Then StackSafetyGlobalInfoWrapperPass
in ThinLTO backend will run as usual looking up to external
symbol in the summary if needed.

Depends on D80985.

Reviewers: eugenis, pcc

Reviewed By: eugenis

Subscribers: inglorion, hiraditya, steven_wu, dexonsmith, cfe-commits, 
llvm-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/test/Driver/memtag_lto.c
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/LTO/LTO.cpp
llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
llvm/test/Analysis/StackSafetyAnalysis/ipa.ll

Removed: 




diff  --git a/clang/test/Driver/memtag_lto.c b/clang/test/Driver/memtag_lto.c
index 81a6f7b842e1..3421e4028db1 100644
--- a/clang/test/Driver/memtag_lto.c
+++ b/clang/test/Driver/memtag_lto.c
@@ -85,7 +85,7 @@
 // RUN:  -r %t.ltonewpm2.bc,use,plx \
 // RUN:  -r %t.ltonewpm2.bc,z, 2>&1 | FileCheck %s 
-check-prefixes=SSI,XSAFE,YSAFE
 
-// FIXME: Thin LTO: both are safe.
+// Thin LTO: both are safe.
 // RUN: %clang -fno-experimental-new-pass-manager -O1 -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -c %s -flto=thin -o 
%t.thinlto1.bc
 // RUN: %clang -fno-experimental-new-pass-manager -O1 -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -c -DBUILD2 %s 
-flto=thin -o %t.thinlto2.bc
 // RUN: llvm-lto2 run -o %t.thinlto %t.thinlto1.bc %t.thinlto2.bc -save-temps 
-stack-safety-print -thinlto-threads 1 -O1 \
@@ -94,9 +94,9 @@
 // RUN:  -r %t.thinlto1.bc,use_local,plx \
 // RUN:  -r %t.thinlto1.bc,w, \
 // RUN:  -r %t.thinlto2.bc,use,plx \
-// RUN:  -r %t.thinlto2.bc,z, 2>&1 | FileCheck %s 
-check-prefixes=SSI,XUNSAFE,YSAFE
+// RUN:  -r %t.thinlto2.bc,z, 2>&1 | FileCheck %s 
-check-prefixes=SSI,XSAFE,YSAFE
 
-// FIXME: Thin LTO, new PM: both are safe.
+// Thin LTO, new PM: both are safe.
 // RUN: %clang -fexperimental-new-pass-manager -O1 -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -c %s -flto=thin -o 
%t.thinltonewpm1.bc
 // RUN: %clang -fexperimental-new-pass-manager -O1 -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -c -DBUILD2 %s 
-flto=thin -o %t.thinltonewpm2.bc
 // RUN: llvm-lto2 run -use-new-pm -o %t.thinltonewpm %t.thinltonewpm1.bc 
%t.thinltonewpm2.bc -save-temps -stack-safety-print -thinlto-threads 1 -O1 \
@@ -105,7 +105,7 @@
 // RUN:  -r %t.thinltonewpm1.bc,use_local,plx \
 // RUN:  -r %t.thinltonewpm1.bc,w, \
 // RUN:  -r %t.thinltonewpm2.bc,use,plx \
-// RUN:  -r %t.thinltonewpm2.bc,z, 2>&1 | FileCheck %s 
-check-prefixes=SSI,XUNSAFE,YSAFE
+// RUN:  -r %t.thinltonewpm2.bc,z, 2>&1 | FileCheck %s 
-check-prefixes=SSI,XSAFE,YSAFE
 
 void use(int *p);
 

diff  --git a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h 
b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
index 0b37decfccca..3ee520eb0411 100644
--- a/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
+++ b/llvm/include/llvm/Analysis/StackSafetyAnalysis.h
@@ -151,6 +151,8 @@ class StackSafetyGlobalInfoWrapperPass : public ModulePass {
 
 bool needsParamAccessSummary(const Module &M);
 
+void generateParamAccessSummary(ModuleSummaryIndex &Index);
+
 } // end namespace llvm
 
 #endif // LLVM_ANALYSIS_STACKSAFETYANALYSIS_H

diff  --git a/llvm/lib/Analysis/StackSafetyAnalysis.cpp 
b/llvm/lib/Analysis/StackSafetyAnalysis.cpp
index 3fb5c294acf6..a59d564cd1cc 100644
--- a/llvm/lib/Analysis/StackSafetyAnalysis.cpp
+++ b/llvm/lib/Analysis/StackSafetyAnalysis.cpp
@@ -544,6 +544,22 @@ StackSafetyDataFlowAnalysis::run() {
   return Functions;
 }
 
+FunctionSummary *resolveCallee(GlobalValueSummary *S) {
+  while (S) {
+if (!S->isLive() || !S->isDSOLocal())
+  return nullptr;
+if (FunctionSummary *FS = dyn_cast(S))
+  return FS;
+AliasSummary *AS = dyn_cast(S);
+if (!AS)
+  return nullptr;
+S = AS->getBaseObject();
+if (S == AS)
+  return nullptr;
+  }
+  return nullptr;
+}
+
 const Function *findCalleeInModule(const GlobalValue *GV) {
   while (GV) {
 if (GV->isDeclaration() || GV->isInterposable() || !GV->isDSOLocal())
@@ -560,7 +576,28 @@ const Function *findCalleeInModule(const GlobalValue *GV) {
   return nullptr;
 }
 
-template  void resolveAllCalls(UseInfo &Use) {
+GlobalValueSummary *getGlobalValueSummary(const ModuleSummaryIndex *Index,
+  uint64_t ValueGUID) {
+  auto VI = Index->getValueInfo(ValueGUID);
+  if (!VI || VI.getSummaryList(

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

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

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3845
+  IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE)  
\
+Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+#include "clang/Driver/Options.inc"

Bigcheese wrote:
> It's a little sad that we need to allocation every string just because of the 
> `-`. We definitely need to be able to allocate strings for options with data, 
> but it would be good if we could just have the strings with `-` prefixed in 
> the option table if that's reasonable to do.
I want to highlight @Bigcheese's comment again so it doesn't get lost. I think 
a reasonable name would be `SPELLING`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3903
+  Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));   
\
+  Args.push_back(StringAllocator(DENORMALIZER(this->KEYPATH)));
\
+}  
\

The denormalizer should take the `StringAllocator` as an argument, and only 
allocate when necessary.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:327
+  OS << "\n";
+  OS << "#ifdef AUTO_NORMALIZING_OPTIONS";
+  OS << "\n\n";

Since this is meant to be included exactly once (as opposed to being an 
x-macro), perhaps this should be split out into a separate `.inc`.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:351-354
+OS << "#define " << MacroName << "(V, D) .Case(V, D)\n";
+emitValueTable(OS, OptionName, R.getValueAsString("Values"),
+   NormalizedValuesScope, NormalizedValues);
+OS << "#undef " << MacroName << "\n";

It seems unnecessary to generate macros here; you can just generate the code 
directly...

But looking more closely, I wonder if we really need this generated code at all.

Instead, can we create a table like this in Options.inc?
```
struct EnumValue {
  const char *Name;
  unsigned Value;
};

struct EnumValueTable {
  const EnumValue *Table;
  unsigned Size;
};

static const EnumValue RelocationModelTable[] = {
  {"static", static_cast(llvm::Reloc::Static)},
  {"pic", static_cast(llvm::Reloc::_PIC)},
  // ...
};

static const EnumValue SomeOtherTable[] = {
  {"name", static_cast(clang::SomeOtherEnumValue)},
  // ...
};

static const EnumValueTable *EnumValueTables[] = {
  {&RelocationModelTable, sizeof(RelocationModelTable) / sizeof(EnumValue)},
  {&SomeOtherTable, sizeof(SomeOtherTable) / sizeof(EnumValue)},
};
static const unsigned EnumValueTablesSize =
sizeof(EnumValueTables) / sizeof(EnumValueTable);
```

We can then have this code directly in Options.cpp:
```
static llvm::Optional extractSimpleEnum(
llvm::opt::OptSpecifier Opt, int TableIndex,
const ArgList &ArgList, DiagnosticsEngine &Diags) {
  assert(TableIndex >= 0);
  assert(TableIndex < EnumValueTablesSize);
  const EnumValueTable &Table = *EnumValueTables[TableIndex];

  auto Arg = Args.getLastArg(Opt);
  if (!Arg)
return None;

  StringRef ArgValue = Arg->getValue();
  for (int I = 0, E = Table.Size; I != E; ++I)
if (ArgValue == Table.Table[I].Name)
  return Table.Table[I].Value;

  Diags.Report(diag::err_drv_invalid_value)
  << Arg->getAsString(ArgList) << ArgValue;
  return None;
}
```
and change the normalizer contract to:
- return an `Optional`,
- take an `llvm::opt::OptSpecifier`,
- take an index into a table (set to `-1` if there's no relevant table), and
- be responsible for the boilerplate call to `getLastArg`.
Simple enums would have `NORMALIZER` hardcoded to `extractSimpleEnum`. The 
handler code that calls it needs new arguments `TABLE_INDEX` and `TYPE` but 
also looks simpler this way:
```
if (auto MaybeValue = NORMALIZER(Opt##ID, TABLE_INDEX, Args, Diags)) \
  this->KEYPATH = static_cast(*MaybeValue);\
else \
  this->KEYPATH = DEFAULT_VALUE;
```

We could similarly have a single `serializeSimpleEnum` function in Options.cpp 
for a hardcoded `DENORMALIZER` for simple enums:
```
static const char *serializeSimpleEnum(int TableIndex, unsigned Value) {
  assert(TableIndex >= 0);
  assert(TableIndex < EnumValueTablesSize);
  const EnumValueTable &Table = *EnumValueTables[TableIndex];
  for (int I = 0, E = Table.Size; I != E; ++I)
if (Value == Table.Table[I].Value)
  return Table.Table[I].Name;

  llvm::report_fatal_error("good error message");
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79796



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

[PATCH] D81242: [StackSafety] Run ThinLTO

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 270553.
vitalybuka marked 3 inline comments as done.
vitalybuka added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81242

Files:
  clang/test/Driver/memtag_lto.c
  llvm/include/llvm/Analysis/StackSafetyAnalysis.h
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll

Index: llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
===
--- llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
+++ llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
@@ -10,6 +10,111 @@
 ; RUN: opt -S -analyze -stack-safety %t.combined.bc | FileCheck %s --check-prefixes=CHECK,GLOBAL,NOLTO
 ; RUN: opt -S -passes="print-stack-safety" -disable-output %t.combined.bc 2>&1 | FileCheck %s --check-prefixes=CHECK,GLOBAL,NOLTO
 
+; Do an end-to-test using the new LTO API
+; TODO: Hideous llvm-lto2 invocation, add a --default-symbol-resolution to llvm-lto2?
+; RUN: opt -module-summary %s -o %t.summ0.bc
+; RUN: opt -module-summary %S/Inputs/ipa.ll -o %t.summ1.bc
+
+; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t.lto -stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN:  -r %t.summ0.bc,Write1, \
+; RUN:  -r %t.summ0.bc,Write4, \
+; RUN:  -r %t.summ0.bc,Write4_2, \
+; RUN:  -r %t.summ0.bc,Write8, \
+; RUN:  -r %t.summ0.bc,WriteAndReturn8, \
+; RUN:  -r %t.summ0.bc,TestUpdateArg,px \
+; RUN:  -r %t.summ0.bc,ExternalCall, \
+; RUN:  -r %t.summ0.bc,PreemptableWrite1, \
+; RUN:  -r %t.summ0.bc,InterposableWrite1, \
+; RUN:  -r %t.summ0.bc,ReturnDependent, \
+; RUN:  -r %t.summ0.bc,Rec2, \
+; RUN:  -r %t.summ0.bc,RecursiveNoOffset, \
+; RUN:  -r %t.summ0.bc,RecursiveWithOffset, \
+; RUN:  -r %t.summ0.bc,f1,px \
+; RUN:  -r %t.summ0.bc,f2,px \
+; RUN:  -r %t.summ0.bc,f3,px \
+; RUN:  -r %t.summ0.bc,f4,px \
+; RUN:  -r %t.summ0.bc,f5,px \
+; RUN:  -r %t.summ0.bc,f6,px \
+; RUN:  -r %t.summ0.bc,PreemptableCall,px \
+; RUN:  -r %t.summ0.bc,InterposableCall,px \
+; RUN:  -r %t.summ0.bc,PrivateCall,px \
+; RUN:  -r %t.summ0.bc,f7,px \
+; RUN:  -r %t.summ0.bc,f8left,px \
+; RUN:  -r %t.summ0.bc,f8right,px \
+; RUN:  -r %t.summ0.bc,f8oobleft,px \
+; RUN:  -r %t.summ0.bc,f8oobright,px \
+; RUN:  -r %t.summ0.bc,TwoArguments,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOne,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOther,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBBoth,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveNoOffset,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,Write1,px \
+; RUN:  -r %t.summ1.bc,Write4,px \
+; RUN:  -r %t.summ1.bc,Write4_2,px \
+; RUN:  -r %t.summ1.bc,Write8,px \
+; RUN:  -r %t.summ1.bc,WriteAndReturn8,px \
+; RUN:  -r %t.summ1.bc,ExternalCall,px \
+; RUN:  -r %t.summ1.bc,PreemptableWrite1,px \
+; RUN:  -r %t.summ1.bc,InterposableWrite1,px \
+; RUN:  -r %t.summ1.bc,ReturnDependent,px \
+; RUN:  -r %t.summ1.bc,Rec0,px \
+; RUN:  -r %t.summ1.bc,Rec1,px \
+; RUN:  -r %t.summ1.bc,Rec2,px \
+; RUN:  -r %t.summ1.bc,RecursiveNoOffset,px \
+; RUN:  -r %t.summ1.bc,RecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,ReturnAlloca,px 2>&1 | FileCheck %s --check-prefixes=CHECK,GLOBAL,LTO
+
+; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t-newpm.lto -use-new-pm -stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN:  -r %t.summ0.bc,Write1, \
+; RUN:  -r %t.summ0.bc,Write4, \
+; RUN:  -r %t.summ0.bc,Write4_2, \
+; RUN:  -r %t.summ0.bc,Write8, \
+; RUN:  -r %t.summ0.bc,WriteAndReturn8, \
+; RUN:  -r %t.summ0.bc,TestUpdateArg,px \
+; RUN:  -r %t.summ0.bc,ExternalCall, \
+; RUN:  -r %t.summ0.bc,PreemptableWrite1, \
+; RUN:  -r %t.summ0.bc,InterposableWrite1, \
+; RUN:  -r %t.summ0.bc,ReturnDependent, \
+; RUN:  -r %t.summ0.bc,Rec2, \
+; RUN:  -r %t.summ0.bc,RecursiveNoOffset, \
+; RUN:  -r %t.summ0.bc,RecursiveWithOffset, \
+; RUN:  -r %t.summ0.bc,f1,px \
+; RUN:  -r %t.summ0.bc,f2,px \
+; RUN:  -r %t.summ0.bc,f3,px \
+; RUN:  -r %t.summ0.bc,f4,px \
+; RUN:  -r %t.summ0.bc,f5,px \
+; RUN:  -r %t.summ0.bc,f6,px \
+; RUN:  -r %t.summ0.bc,PreemptableCall,px \
+; RUN:  -r %t.summ0.bc,InterposableCall,px \
+; RUN:  -r %t.summ0.bc,PrivateCall,px \
+; RUN:  -r %t.summ0.bc,f7,px \
+; RUN:  -r %t.summ0.bc,f8left,px \
+; RUN:  -r %t.summ0.bc,f8right,px \
+; RUN:  -r %t.summ0.bc,f8oobleft,px \
+; RUN:  -r %t.summ0.bc,f8oobright,px \
+; RUN:  -r %t.summ0.bc,TwoArguments,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOne,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOther,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBBoth,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveNoOffset,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,Write1,px \
+; RUN:  -r %t.summ1.bc,Write4,px \
+; RUN:  -r %t.summ1.bc,Write4_2,px \
+; RUN:  -r %t.summ1.bc,Write8,px \
+; 

[PATCH] D81242: [StackSafety] Run ThinLTO

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:618
+ConstantRange Access = Found->sextOrTrunc(Use.Range.getBitWidth());
+if (Access.signedAddMayOverflow(C.Offset) !=
+ConstantRange::OverflowResult::NeverOverflows)

eugenis wrote:
> vitalybuka wrote:
> > eugenis wrote:
> > > Do we have a test for this overflow check?
> > yes
> > Example, in bit width 8
> > [-128,-127)+[-128,-127) = [0,1)
> > both non-wrapped and result is non-wrapped so we have no way to spot 
> > overflow
> Sure. I was asking if we have a testcase that covers the overflow check.
I've extracted this into function and added test to cover that.
It is possible to test this check, however we discussed clamping offsets to 
something more useful, e.g. 2^(ptrwidth/2). This way we will not be able to hit 
this check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81242



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

[PATCH] D81242: [StackSafety] Run ThinLTO

[Vitaly Buka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc1e47b47f884: [StackSafety] Run ThinLTO (authored by 
vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81242

Files:
  clang/test/Driver/memtag_lto.c
  llvm/include/llvm/Analysis/StackSafetyAnalysis.h
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll

Index: llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
===
--- llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
+++ llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
@@ -10,6 +10,111 @@
 ; RUN: opt -S -analyze -stack-safety %t.combined.bc | FileCheck %s --check-prefixes=CHECK,GLOBAL,NOLTO
 ; RUN: opt -S -passes="print-stack-safety" -disable-output %t.combined.bc 2>&1 | FileCheck %s --check-prefixes=CHECK,GLOBAL,NOLTO
 
+; Do an end-to-test using the new LTO API
+; TODO: Hideous llvm-lto2 invocation, add a --default-symbol-resolution to llvm-lto2?
+; RUN: opt -module-summary %s -o %t.summ0.bc
+; RUN: opt -module-summary %S/Inputs/ipa.ll -o %t.summ1.bc
+
+; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t.lto -stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN:  -r %t.summ0.bc,Write1, \
+; RUN:  -r %t.summ0.bc,Write4, \
+; RUN:  -r %t.summ0.bc,Write4_2, \
+; RUN:  -r %t.summ0.bc,Write8, \
+; RUN:  -r %t.summ0.bc,WriteAndReturn8, \
+; RUN:  -r %t.summ0.bc,TestUpdateArg,px \
+; RUN:  -r %t.summ0.bc,ExternalCall, \
+; RUN:  -r %t.summ0.bc,PreemptableWrite1, \
+; RUN:  -r %t.summ0.bc,InterposableWrite1, \
+; RUN:  -r %t.summ0.bc,ReturnDependent, \
+; RUN:  -r %t.summ0.bc,Rec2, \
+; RUN:  -r %t.summ0.bc,RecursiveNoOffset, \
+; RUN:  -r %t.summ0.bc,RecursiveWithOffset, \
+; RUN:  -r %t.summ0.bc,f1,px \
+; RUN:  -r %t.summ0.bc,f2,px \
+; RUN:  -r %t.summ0.bc,f3,px \
+; RUN:  -r %t.summ0.bc,f4,px \
+; RUN:  -r %t.summ0.bc,f5,px \
+; RUN:  -r %t.summ0.bc,f6,px \
+; RUN:  -r %t.summ0.bc,PreemptableCall,px \
+; RUN:  -r %t.summ0.bc,InterposableCall,px \
+; RUN:  -r %t.summ0.bc,PrivateCall,px \
+; RUN:  -r %t.summ0.bc,f7,px \
+; RUN:  -r %t.summ0.bc,f8left,px \
+; RUN:  -r %t.summ0.bc,f8right,px \
+; RUN:  -r %t.summ0.bc,f8oobleft,px \
+; RUN:  -r %t.summ0.bc,f8oobright,px \
+; RUN:  -r %t.summ0.bc,TwoArguments,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOne,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOther,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBBoth,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveNoOffset,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,Write1,px \
+; RUN:  -r %t.summ1.bc,Write4,px \
+; RUN:  -r %t.summ1.bc,Write4_2,px \
+; RUN:  -r %t.summ1.bc,Write8,px \
+; RUN:  -r %t.summ1.bc,WriteAndReturn8,px \
+; RUN:  -r %t.summ1.bc,ExternalCall,px \
+; RUN:  -r %t.summ1.bc,PreemptableWrite1,px \
+; RUN:  -r %t.summ1.bc,InterposableWrite1,px \
+; RUN:  -r %t.summ1.bc,ReturnDependent,px \
+; RUN:  -r %t.summ1.bc,Rec0,px \
+; RUN:  -r %t.summ1.bc,Rec1,px \
+; RUN:  -r %t.summ1.bc,Rec2,px \
+; RUN:  -r %t.summ1.bc,RecursiveNoOffset,px \
+; RUN:  -r %t.summ1.bc,RecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,ReturnAlloca,px 2>&1 | FileCheck %s --check-prefixes=CHECK,GLOBAL,LTO
+
+; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t-newpm.lto -use-new-pm -stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN:  -r %t.summ0.bc,Write1, \
+; RUN:  -r %t.summ0.bc,Write4, \
+; RUN:  -r %t.summ0.bc,Write4_2, \
+; RUN:  -r %t.summ0.bc,Write8, \
+; RUN:  -r %t.summ0.bc,WriteAndReturn8, \
+; RUN:  -r %t.summ0.bc,TestUpdateArg,px \
+; RUN:  -r %t.summ0.bc,ExternalCall, \
+; RUN:  -r %t.summ0.bc,PreemptableWrite1, \
+; RUN:  -r %t.summ0.bc,InterposableWrite1, \
+; RUN:  -r %t.summ0.bc,ReturnDependent, \
+; RUN:  -r %t.summ0.bc,Rec2, \
+; RUN:  -r %t.summ0.bc,RecursiveNoOffset, \
+; RUN:  -r %t.summ0.bc,RecursiveWithOffset, \
+; RUN:  -r %t.summ0.bc,f1,px \
+; RUN:  -r %t.summ0.bc,f2,px \
+; RUN:  -r %t.summ0.bc,f3,px \
+; RUN:  -r %t.summ0.bc,f4,px \
+; RUN:  -r %t.summ0.bc,f5,px \
+; RUN:  -r %t.summ0.bc,f6,px \
+; RUN:  -r %t.summ0.bc,PreemptableCall,px \
+; RUN:  -r %t.summ0.bc,InterposableCall,px \
+; RUN:  -r %t.summ0.bc,PrivateCall,px \
+; RUN:  -r %t.summ0.bc,f7,px \
+; RUN:  -r %t.summ0.bc,f8left,px \
+; RUN:  -r %t.summ0.bc,f8right,px \
+; RUN:  -r %t.summ0.bc,f8oobleft,px \
+; RUN:  -r %t.summ0.bc,f8oobright,px \
+; RUN:  -r %t.summ0.bc,TwoArguments,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOne,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBOther,px \
+; RUN:  -r %t.summ0.bc,TwoArgumentsOOBBoth,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveNoOffset,px \
+; RUN:  -r %t.summ0.bc,TestRecursiveWithOffset,px \
+; RUN:  -r %t.summ1.bc,Write1,px \
+; RUN:  -r %t.summ1.bc,Write4,px \
+; RUN:  -r %t.summ1.bc,Write4_2,px \
+; RUN:  -r %t.summ1

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > The called function is to be renamed.
> Any suggestions here about how to represent the functionality of `__sterm` 
> and `_GLOBAL__D_a` into one word? Cannot think of a good name...
> 
> Actually I am wondering do we need to rename the `Destruct` part? The 
> `__sterm` and `_GLOBAL__D_a` do destruct the object by calling sterm 
> finalizers and object destructors?
Being clear in the naming of the function helps to signal its purpose to future 
maintainers. The sterm finalizers are unlikely to be understood from `Destruct` 
(it implies plain destruction a bit too much). Maybe "cleanup"?



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
 llvm::Function *Fn,

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > This function is to be renamed.
>  I will try `GenerateCXXGlobalDestructFunc` based on the thoughts as I also 
> mentioned elsewhere that the `__sterm` and `_GLOBAL__D_` function do destruct 
> the object by calling sterm finalizers and object destructors.
> 
> Any suggestions or concerns about this renaming? Or do we really need to 
> rename this one?
I think "cleanup" works here too.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+return;
 

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Can this part be committed in a separate patch? It does not appear to have 
> > dependencies on other parts of this patch and has the appearance of being a 
> > possible change for other platforms.
> Sure. I will create a NFC patch for this part and try to commit it first. But 
> so far, I think I can keep this part in this patch just for review purpose to 
> make the patch look nicer?
Sure; you'd need the new patch to exist before rebasing on it anyway. We can 
leave the rebasing to the commit or later in the review.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Suggestion:
> > Create the finalization action associated with a variable.
> I don't get your point, can you elaborate on this a little bit?
This is my suggestion for the comment (to replace "Create a variable 
destruction function").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166



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

[PATCH] D81781: Add additional text format functions to Google style.

[Nicholas Seckar via Phabricator via cfe-commits]

nseckar created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nseckar updated this revision to Diff 270559.
nseckar added a comment.
nseckar added a reviewer: djasper.

Add reviewers


This adds a few more function names expecting a text proto argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81781

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1007,6 +1007,8 @@
   "PARSE_PARTIAL_TEXT_PROTO",
   "PARSE_TEST_PROTO",
   "PARSE_TEXT_PROTO",
+  "ParsePartialTestProto",
+  "ParseTestProto",
   "ParseTextOrDie",
   "ParseTextProtoOrDie",
   },


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1007,6 +1007,8 @@
   "PARSE_PARTIAL_TEXT_PROTO",
   "PARSE_TEST_PROTO",
   "PARSE_TEXT_PROTO",
+  "ParsePartialTestProto",
+  "ParseTestProto",
   "ParseTextOrDie",
   "ParseTextProtoOrDie",
   },
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81781: Add additional text format functions to Google style.

[Nicholas Seckar via Phabricator via cfe-commits]

nseckar updated this revision to Diff 270559.
nseckar added a comment.

Add reviewers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81781

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1007,6 +1007,8 @@
   "PARSE_PARTIAL_TEXT_PROTO",
   "PARSE_TEST_PROTO",
   "PARSE_TEXT_PROTO",
+  "ParsePartialTestProto",
+  "ParseTestProto",
   "ParseTextOrDie",
   "ParseTextProtoOrDie",
   },


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1007,6 +1007,8 @@
   "PARSE_PARTIAL_TEXT_PROTO",
   "PARSE_TEST_PROTO",
   "PARSE_TEXT_PROTO",
+  "ParsePartialTestProto",
+  "ParseTestProto",
   "ParseTextOrDie",
   "ParseTextProtoOrDie",
   },
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:31
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] 
[{ i32, void ()*, i8* } { i32 65535, void ()* 
@__sterm8000_clang_510e898aa8d263cac999dd03eeed5b51, i8* null }]
+

Okay, the restricted scope of this patch seems to landed in a place where how 
the `llvm.global_dtors` array will be handled is not indicated...

The backend should implement the semantics of the IR construct. When handling 
said array in the IR, it seems the method to handle the requisite semantics 
would be to generate an alias (with the appropriate linkage for the linker to 
pick it up) that is named using the `__sinit`/`__sterm` convention. Which is to 
say that at least some part of the naming should eventually move into the LLVM 
side and out of Clang.

Please update the description of this patch to indicate that:

  - The `llvm.global_ctors` and `llvm.global_dtors` functionality has not yet 
been implemented on AIX.
  - This patch temporarily side-steps the need to implement that functionality.
  - The apparent uses of that functionality in this patch (with respect to the 
name of the functions involved) are not representative of how the functionality 
will be used once implemented.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166



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

[clang] 6604295 - [WebAssembly] WebAssembly doesn't support "protected" visibility

[Dan Gohman via cfe-commits]

Author: Dan Gohman
Date: 2020-06-12T19:52:35-07:00
New Revision: 66042959590d6db9d2a12803a16476d4e3508f3f

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

LOG: [WebAssembly] WebAssembly doesn't support "protected" visibility

Implement the `hasProtectedVisibility()` hook to indicate that, like
Darwin, WebAssembly doesn't support "protected" visibility.

On ELF, "protected" visibility is intended to be an optimization, however
in practice it often [isn't], and ELF documentation generally ranges from
[not mentioning it at all] to [strongly discouraging its use].

[isn't]: https://www.airs.com/blog/archives/307
[not mentioning it at all]: https://gcc.gnu.org/wiki/Visibility
[strongly discouraging its use]: https://www.akkadia.org/drepper/dsohowto.pdf

While here, also mention the new Reactor support in the release notes.

Added: 


Modified: 
clang/lib/Basic/Targets/WebAssembly.h
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/lib/Basic/Targets/WebAssembly.h 
b/clang/lib/Basic/Targets/WebAssembly.h
index 030d79ea2ecc..e09e21d90802 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -132,7 +132,14 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : 
public TargetInfo {
   }
 
   bool hasExtIntType() const override { return true; }
+
+  bool hasProtectedVisibility() const override {
+// TODO: For now, continue to advertise "protected" support for
+// Emscripten targets.
+return getTriple().isOSEmscripten();
+  }
 };
+
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {
 public:

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index ea1fad1f6390..7d71d0c58a4c 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -144,8 +144,14 @@ Changes to the AVR Target
 Changes to the WebAssembly Target
 -
 
-During this release ...
-
+* Programs which don't have a "main" function, called "reactors" are now
+  properly supported, with a new `-mexec-model=reactor` flag. Programs which
+  previously used `-Wl,--no-entry` to avoid having a main function should
+  switch to this new flag, so that static initialization is properly
+  performed.
+
+* `__attribute__((visibility("protected")))` now evokes a warning, as
+  WebAssembly does not support "protected" visibility.
 
 Changes to the OCaml bindings
 -



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

[PATCH] D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility

[Dan Gohman via Phabricator via cfe-commits]

sunfish closed this revision.
sunfish added a comment.

Landed in 66042959590d6db9d2a12803a16476d4e3508f3f 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81688



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

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN:   FileCheck %s
 
 struct test {

Xiangling_L wrote:
> jasonliu wrote:
> > Looks like the non-inline comments are easier to get ignored and missed, so 
> > I will copy paste the non-inlined comment I previously had:
> > ```
> > -fregister_global_dtors_with_atexit does not seem to work properly in 
> > current implementation.
> > We should consider somehow disabling/report_fatal_error it instead of 
> > letting it generate invalid code on AIX.
> > ```
> Thanks for doing so!
> 
> The semantic of `global_dtors` here on AIX is `__sterm` function, which we 
> are never gonna register with `__atexit`. I am thinking we can disable it in 
> a follow-up driver patch with the handling of `-fno-use-cxa-atexit`.
The semantic of `global_dtors` is not limited to the `__sterm` functions 
associated with C++ cleanup actions. With respect to user-declared 
`__attribute__((__destructor__))` functions, the option could improve the 
interleaving of those cleanup actions with cleanup actions registered by 
user-declared `__attribute__((__constructor__))` functions.

This provides that rationale for separating the `__sterm` functions associated 
with the C++ cleanup actions from the other "destructor" functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166



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

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:277
+ llvm::FunctionType::get(CGM.VoidTy, false) &&
+ "atexit has wrong parameter type.");
+

s/atexit has wrong parameter type/Argument to atexit has a wrong type/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:302
+ llvm::FunctionType::get(CGM.VoidTy, false) &&
+ "unatexit has wrong parameter type.");
+

s/unatexit has wrong parameter type/Argument to unatexit has a wrong type/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:649
+FuncName = llvm::Twine("__sinit8000_clang_", GlobalUniqueModuleId)
+   .toNullTerminatedStringRef(FuncName);
+  else {

Replace the call to `toNullTerminatedStringRef` and the assignment with a call 
to `toVector`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:657
+   getTransformedFileName(getModule(), Storage))
+   .toNullTerminatedStringRef(FuncName);
   }

Replace the call to `toNullTerminatedStringRef` and the assignment with a call 
to `toVector`.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:664
+  if (getCXXABI().useSinitAndSterm())
+Fn->setLinkage(llvm::Function::ExternalLinkage);
 

`CreateGlobalInitOrDestructFunction` currently calls `Create` with 
`llvm::GlobalValue::InternalLinkage` and also calls 
`SetInternalFunctionAttributes`. Setting `ExternalLinkage` after-the-fact seems 
brittle.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:690
 
-void CodeGenModule::EmitCXXGlobalDtorFunc() {
-  if (CXXGlobalDtors.empty())
+void CodeGenModule::EmitCXXGlobalDestructFunc() {
+  if (CXXGlobalDtorsOrStermFinalizers.empty())

I think "cleanup" works here too.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:697
 
   // Create our global destructor function.
+  llvm::Function *Fn = nullptr;

s/Create our global destructor function/Create our global cleanup function/;



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:822
 
 // Emit the dtors, in reverse order from construction.
+for (unsigned i = 0, e = DtorsOrStermFinalizers.size(); i != e; ++i) {

s/dtors/cleanups/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:472
   /// Global destructor functions and arguments that need to run on 
termination.
+  /// Global destructor functions and arguments that need to run on 
termination;
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions

Near duplicate comment line.



Comment at: clang/lib/CodeGen/CodeGenModule.h:473
+  /// Global destructor functions and arguments that need to run on 
termination;
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions
+  /// instead that need to run on unloading a shared library.

s/finalizers/finalizer/;
s/it contains/it instead contains/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:474
+  /// When UseSinitAndSterm is set, it contains sterm finalizers functions
+  /// instead that need to run on unloading a shared library.
   std::vector<

s/instead that need to run/, which also run/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1059
+
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {

s/a destructor/an sterm finalizer/;
s/global destructor/global cleanup/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1060
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+CXXGlobalDtorsOrStermFinalizers.emplace_back(DtorFn.getFunctionType(),

s/Dtor/StermFinalizer/;



Comment at: clang/lib/CodeGen/CodeGenModule.h:1462
 
-  /// Emit the function that destroys C++ globals.
-  void EmitCXXGlobalDtorFunc();
+  /// Emit the function that destructs C++ globals.
+  void EmitCXXGlobalDestructFunc();

s/destructs/performs cleanup associated with/;



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4448
+  if (CGM.getCodeGenOpts().CXAAtExit)
+llvm::report_fatal_error("using __cxa_atexit unsupported on AIX.");
+  // Register above __dtor with atexit().

There should not be period at the end of a `report_fatal_error` message.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4452
+
+  // Emit __finalize function to unregister __dtor and call __dtor.
+  emitCXXStermFinalizer(D, dtorStub, addr);

s/and/and (as appropriate)/;



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4477
+  // registered by the atexit subroutine. If the referenced function is foun

[PATCH] D15528: Teach clang-tidy how to -Werror checks.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.
Herald added a subscriber: arphaman.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:362
+ << Plural << "\n";
+return WErrorCount;
+  }

Was there any specific reason for returning the error count instead of 
returning 1. It results in undefined behaviour on POSIX shells if a value is 
returned outside the range of 0<=n<=255. See 
https://bugs.llvm.org/show_bug.cgi?id=46305


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

https://reviews.llvm.org/D15528



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