[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 289085.
tambre added a comment.

Remove define to change driver behaviour, use 
ToolChain::getCompilerRTArgString() instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,7 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList &Args,
   ArgStringList &CmdArgs) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+  CmdArgs.push_back(getCompilerRTArgString(Args, "builtins"));
 }
 
 void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,7 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList &Args,
   ArgStringList &CmdArgs) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+  CmdArgs.push_back(getCompilerRTArgString(Args, "builtins"));
 }
 
 void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Just for the record, the additional analysis has a measurable compile-time 
impact (0.3% at O0): 
https://llvm-compile-time-tracker.com/compare.php?from=e7f53044e7263cdbbb4fed9abf086b88ba486bba&to=cff6dda604cb0548bef5e5951dd1e74536110588&stat=instructions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85778

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

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D86877/new/

https://reviews.llvm.org/D86877

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

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

In D86877#2248064 , @phosek wrote:

> This was discussed when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` was introduced 
> and there was a pushback against changing the driver behavior depending on 
> the value of that option, so if we're going to reverse that decision for 
> BareMetal, I think that deserves a broader discussion.

Thanks for the background. I'd rather not go for a compile-time behaviour 
change in that case and have gone with using `getCompilerRTArgString()`. I'll 
do an internal toolchain build and do some testing before landing this.




Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:163
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+#endif

phosek wrote:
> Is there a reason why BareMetal doesn't just use 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChain.cpp#L462
>  like all other drivers, and instead re-implements the runtime library lookup 
> logic?
Was unaware of that function before, thanks!
I can't see a good reason not to use it, so I've switched it over.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 289099.
martong added a comment.

- Support empty ranges


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Index: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s --allow-empty
+
+// We test here the implementation of our summary API with Optional types. In
+// this TU we do not provide declaration for any of the functions that have
+// summaries. The implementation should be able to handle the nonexistent
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
+
+// CHECK-NOT: Loaded summary for:
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -138,18 +138,24 @@
   /// Given a range, should the argument stay inside or outside this range?
   enum RangeKind { OutOfRange, WithinRange };
 
-  /// Encapsulates a single range on a single symbol within a branch.
+  /// Encapsulates a range on a single symbol.
   class RangeConstraint : public ValueConstraint {
-RangeKind Kind;  // Kind of range definition.
-IntRangeVector Args; // Polymorphic arguments.
+RangeKind Kind;
+// A range is formed as a set of intervals (sub-ranges).
+// E.g. {['A', 'Z'], ['a', 'z']}
+//
+// The default constructed RangeConstraint has an empty range set, applying
+// such constraint does not involve any assumptions, thus the State remains
+// unchanged. This is meaningful, if the range is dependent on a looked up
+// type (e.g. [0, Socklen_tMax]). If the type is not found, then the range
+// is default initialized to be empty.
+IntRangeVector Ranges;
 
   public:
-RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
-: ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
+RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
+: ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
 
-const IntRangeVector &getRanges() const {
-  return Args;
-}
+const IntRangeVector &getRanges() const { return Ranges; }
 
   private:
 ProgramStateRef applyAsOutOfRange(ProgramStateRef State,
@@ -314,7 +320,8 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,17 +331,37 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!Arg) {
+  Invalid = true;
+  return;
+} else {
+  assertArgTypeSuitableForSignature(*Arg);
+  this->ArgTys.push_back(*Arg);
+}
+  }
+  if (!RetTy) {
+Invalid = true;
+retu

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Balázs Kéri via Phabricator via cfe-commits]

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

Looks good now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

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

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Note that we don't deal with wrapping here.

Wrapping? Please elaborate.




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:226
+  // No unsigned symbolic value can be less then a negative constant.
+  if (const auto SymbolicRoot = RootNonLoc.getAs())
+if (SymbolicRoot->getSymbol()->getType()->isUnsignedIntegerType() &&

I really feel that this check would have a better place in the implementation 
of `eval`. This seems really counter-intuitive to do this stuff at the 
Checker's level. Is there any reason why we can't do this in `eval`?

`evalBinOpNN` could return with Unknown, and the state should remain unchanged. 
Am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86874

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

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning

Hmm, this seems to be quite redundant with the `size_t` tests. Why is it not 
enough to have test for one unsigned type?
Are you trying to check for overflow errors? Then I'd expect to have indexes 
around UINT_MAX and so on. 

Same comment applies to the tests with the signed types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

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

Szelethus added a comment.

I'd prefer to have a couple more green lights. In particular, another look on 
the constraint manager and Objective C stuff would be great.




Comment at: clang/docs/ReleaseNotes.rst:487
+- While still in alpha, ``alpha.unix.PthreadLock``, the iterator and container
+  modeling infrastructure, ``alpha.unix.Stream``, and taint analysis were
+  improved greatly. An ongoing, currently off-by-default improvement was made 
on

steakhal wrote:
> Unfortunately, my taint related patched not upstreamed yet. So you should not 
> mention that IMO.
I definitely saw some patches that did land.


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

https://reviews.llvm.org/D86533

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko accepted this revision.
vsavchenko added a comment.

In D86533#2248856 , @Szelethus wrote:

> I'd prefer to have a couple more green lights. In particular, another look on 
> the constraint manager and Objective C stuff would be great.

Those parts are good IMO.


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

https://reviews.llvm.org/D86533

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

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:52
+  ///   Offset: SymIntExpr{conj{n, int}, +, 12, long long}
+  class RawOffsetCalculator final
+  : public MemRegionVisitor {

Since you are already deep in refactoring. This calculation together with the 
enclosing class seems to be useful for other Checkers too. Would make sense to 
put this to `CheckerHelpers`?
E.g. the PlacementNewChecker could have benefited from this class if it had 
been available for use during the implementation of the checker.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:298
+  //
+  // The algorithm we are using here for bounds checking is to see if the
+  // memory access is within the extent of the base region.  Since we

What is the exact difference in between this algorithm and `assumeInBound`? 
What are the benefits (and disadvantages)  of using each of them?

And oh wait, there's more: `assumeInclusiveRange`. Is that also related? 
(StdLibraryFunctionsChecker uses that for range assumptions.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86873

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

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

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

steakhal added inline comments.



Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning

martong wrote:
> Hmm, this seems to be quite redundant with the `size_t` tests. Why is it not 
> enough to have test for one unsigned type?
> Are you trying to check for overflow errors? Then I'd expect to have indexes 
> around UINT_MAX and so on. 
> 
> Same comment applies to the tests with the signed types.
In the current implementation - and in any implementation of the checker logic 
will have to deal with //integral-promotion// during the //simplification// of 
the //array indexer expression// and the given //extent//.
All of these can have different signess and bitwidth which makes the 
implementation quite tricky.

In fact, this resulted in the bug, which this patch-stack aims to fix.
I'm gonna highlight the related parts in the refactoring patch if you think it 
helps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

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

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

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

Szelethus added a comment.

Before we dive into this too much, if you can point to discussion that explains 
why we have a 2 versions of the same checker, that would be nice. Why did you 
chose to work on this one, and not the other? I recall us talking about this in 
a meeting, but its always great to have it set in stone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86873

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

[PATCH] D86699: [SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 289103.
eduucaldas marked 2 inline comments as done.
eduucaldas added a comment.

Add further tests and extract `IgnoreImplicitConstructorSingleStep`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86699

Files:
  clang/include/clang/AST/IgnoreExpr.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/IgnoreExpr.cpp
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -1745,19 +1745,15 @@
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
   {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-| `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
@@ -3821,26 +3817,73 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Equal) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = 1]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  `-IntegerLiteralExpression
+`-'1' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, InitDeclarator_Brace) {
   if (!GetParam().isCXX11OrLater()) {
 return;
   }
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-int a {};
+struct S { S(int);};
+void test(){
+  [[S s{1}]];
+}
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'int'
-  |-SimpleDeclarator Declarator
-  | |-'a'
-  | `-UnknownExpression
-  |   `-UnknownExpression
-  | |-'{'
-  | `-'}'
-  `-';'
-)txt"));
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'{'
+  |-IntegerLiteralExpression
+  | `-'1' LiteralToken
+  `-'}'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, InitDeclarator_EqualBrace) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct S { S(int);};
+void test() {
+  [[S s = {1}]];
+}
+)cpp",
+  {R"txt(
+SimpleDeclaration
+|-'S'
+`-SimpleDeclarator Declarator
+  |-'s'
+  |-'='
+  |-'{'
+  |-IntegerLiteralExpression
+  | `-'1' LiteralToken
+  `-'}'
+)txt"}));
 }
 
 TEST_P(SyntaxTreeTest, InitDeclarator_Paren) {
@@ -3858,12 +3901,60 @@
 SimpleDeclaration
 |-'S'
 |-SimpleDeclarator Declarator
-| `-UnknownExpression
-|   |-'s'
-|   |-'('
-|   |-IntegerLiteralExpression
-|   | `-'1' LiteralToken
-|   `-')'
+| |-'s'
+| |-'('
+| |-IntegerLiteralExpression
+| | `-'1' LiteralToken
+| `-')'
+`-';'
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+void TakeX(const X&);
+void test() {
+  [[TakeX(1)]];
+}
+)cpp",
+  {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'TakeX'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest, ImplicitConversion_Return) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+  R"cpp(
+struct X {
+  X(int);
+};
+X CreateX(){
+  [[return 1;]]
+}
+)cpp",
+  {R"txt(
+ReturnStatement Statement
+|-'return' IntroducerKeyword
+|-IntegerLiteralExpression ReturnValue
+| `-'1' LiteralToken
 `-';'
 )txt"}));
 }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
@@ -44,8 +45,13 @@
 
 using namespace clang;
 
+static Expr *IgnoreImplicit(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreImplicitSingleStep,
+ IgnoreImplicitConstructorSingleStep);
+}
+
 LLVM_ATTRIBUTE_UNUSED
-static bool isImplicitExpr(Expr *E) { return E->IgnoreImplicit() != E; }
+static bool isImplicitExpr(Expr *E) { return IgnoreImplicit(E) != E; }
 
 namespace {
 /// Get start location

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:52
+  ///   Offset: SymIntExpr{conj{n, int}, +, 12, long long}
+  class RawOffsetCalculator final
+  : public MemRegionVisitor {

martong wrote:
> Since you are already deep in refactoring. This calculation together with the 
> enclosing class seems to be useful for other Checkers too. Would make sense 
> to put this to `CheckerHelpers`?
> E.g. the PlacementNewChecker could have benefited from this class if it had 
> been available for use during the implementation of the checker.
You are probably right.
However, I'm not qualified enough to make such a decision.

By the same token, I don't really understand why would anyone do such 
//unfolding// by hand?
Could someone clarify the use-cases for `RegionRawOffset` and 
`RegionRawOffsetV2`? @NoQ @vsavchenko @Szelethus?
Eg. we don't use such facilities in the `evalBindOpL#` functions, but it would 
probably come handy.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:171-172
 
-  SVal extentBegin = computeExtentBegin(svalBuilder, rawOffset.getRegion());
+  llvm::APSInt RHSConstant =
+  APSIntType(Index.getValue()).convert(E->getRHS());
 

Here we 'c-style' integral cast the right-hand side value to match the signess 
and bitwidth of `Index`. @martong 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86873

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

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

[Gabor Marton via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning

steakhal wrote:
> martong wrote:
> > Hmm, this seems to be quite redundant with the `size_t` tests. Why is it 
> > not enough to have test for one unsigned type?
> > Are you trying to check for overflow errors? Then I'd expect to have 
> > indexes around UINT_MAX and so on. 
> > 
> > Same comment applies to the tests with the signed types.
> In the current implementation - and in any implementation of the checker 
> logic will have to deal with //integral-promotion// during the 
> //simplification// of the //array indexer expression// and the given 
> //extent//.
> All of these can have different signess and bitwidth which makes the 
> implementation quite tricky.
> 
> In fact, this resulted in the bug, which this patch-stack aims to fix.
> I'm gonna highlight the related parts in the refactoring patch if you think 
> it helps.
Okay, makes sense. 

It's just very painful to have code repetitions, even in the test code. In 
gtest unittests we can have tests with type parameters to avoid this kind of 
repetition. But I guess, there is no way to get rid of this repetition in lit 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

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

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

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

steakhal added a comment.

I tried to run the benchmark on the small set of projects but it would take an 
enormous time to analyze all such projects 20 times each...
Dedicating a box for this is unfeasible for me.
So I stuck with analyzing only `tmux` with 20 iterations.
The results are not convincing enough IMO.
F12828077: tmux-benchmark.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86295

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

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D86621#2247786 , @efriedma wrote:

> 



> For compiler-rt, the XFAILs should probably reflect whatever config the bot 
> is running.  (Alternatively, you could use UNSUPPORTED, but that doesn't seem 
> warranted here.)

That's been my guess, too (with the goal of getting the bot green eventually in 
mind).  Once the underlying `clang` bug is fixed, the tests will pass with both 
compilers, so I agree that `UNSUPPORTED` isn't warranted here.




Comment at: clang/lib/Basic/Targets/Sparc.cpp:224
+Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
+  }
 }

efriedma wrote:
> This probably should be refactored so the target-independent code generates 
> it based on MaxAtomicInlineWidth, instead of duplicating it for each target.  
> But I guess you don't need to do that here.
> 
> From the other code, the `getCPUGeneration(CPU) == CG_V9` check should only 
> guard the definition of __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> This probably should be refactored so the target-independent code generates 
> it based on MaxAtomicInlineWidth, instead of duplicating it for each target.  
> But I guess you don't need to do that here.

Good: one issue at a time ;-)

> From the other code, the `getCPUGeneration(CPU) == CG_V9` check should only 
> guard the definition of __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?

I don't think so: at least `gcc` defines none of the four with `-m32 -mcpu=v8` 
and all with `-m32 -mcpu=v9`.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:350
   return A->getValue();
+if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris())
+  return "v9";

efriedma wrote:
> Do we want to make sparc and sparcel behave differently here?
The only thing about little-endian SPARC is that UltraSPARC CPUs can be run in 
little-endian  mode, presumably to ease a Windows NT port that never 
materialized.

I could barely find any info on sparcel: the triplet isn't in `config.guess`, 
there's very little else about such CPUs and certainly no specification or even 
an ABI. That's why I tend to leave sparcel-specific code alone unless someone 
who knows these beasts tells me otherwise.

What I can say for certain that Solaris never ran on little-endian SPARC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa787a4ed16d6: [analyzer][StdLibraryFunctionsChecker] Use 
Optionals throughout the summary API (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D86531?vs=289099&id=289108#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Index: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s --allow-empty
+
+// We test here the implementation of our summary API with Optional types. In
+// this TU we do not provide declaration for any of the functions that have
+// summaries. The implementation should be able to handle the nonexistent
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
+
+// CHECK-NOT: Loaded summary for:
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -138,16 +138,24 @@
   /// Given a range, should the argument stay inside or outside this range?
   enum RangeKind { OutOfRange, WithinRange };
 
-  /// Encapsulates a single range on a single symbol within a branch.
+  /// Encapsulates a range on a single symbol.
   class RangeConstraint : public ValueConstraint {
-RangeKind Kind;  // Kind of range definition.
-IntRangeVector Args; // Polymorphic arguments.
+RangeKind Kind;
+// A range is formed as a set of intervals (sub-ranges).
+// E.g. {['A', 'Z'], ['a', 'z']}
+//
+// The default constructed RangeConstraint has an empty range set, applying
+// such constraint does not involve any assumptions, thus the State remains
+// unchanged. This is meaningful, if the range is dependent on a looked up
+// type (e.g. [0, Socklen_tMax]). If the type is not found, then the range
+// is default initialized to be empty.
+IntRangeVector Ranges;
 
   public:
-RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
-: ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
+RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
+: ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
 
-const IntRangeVector &getRanges() const { return Args; }
+const IntRangeVector &getRanges() const { return Ranges; }
 
   private:
 ProgramStateRef applyAsOutOfRange(ProgramStateRef State,
@@ -313,7 +321,8 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -323,17 +332,37 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!

[clang] a787a4e - [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the summary API

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-09-01T11:36:20+02:00
New Revision: a787a4ed16d6867f56d81159a8fcf2b711d18a8a

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

LOG: [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the 
summary API

By using optionals, we no longer have to check the validity of types that we
get from a lookup. This way, the definition of the summaries have a declarative
form, there are no superflous conditions in the source code.

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

Added: 
clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Modified: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 030f995739e3..c65d58e49d78 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -138,16 +138,24 @@ class StdLibraryFunctionsChecker
   /// Given a range, should the argument stay inside or outside this range?
   enum RangeKind { OutOfRange, WithinRange };
 
-  /// Encapsulates a single range on a single symbol within a branch.
+  /// Encapsulates a range on a single symbol.
   class RangeConstraint : public ValueConstraint {
-RangeKind Kind;  // Kind of range definition.
-IntRangeVector Args; // Polymorphic arguments.
+RangeKind Kind;
+// A range is formed as a set of intervals (sub-ranges).
+// E.g. {['A', 'Z'], ['a', 'z']}
+//
+// The default constructed RangeConstraint has an empty range set, applying
+// such constraint does not involve any assumptions, thus the State remains
+// unchanged. This is meaningful, if the range is dependent on a looked up
+// type (e.g. [0, Socklen_tMax]). If the type is not found, then the range
+// is default initialized to be empty.
+IntRangeVector Ranges;
 
   public:
-RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Args)
-: ValueConstraint(ArgN), Kind(Kind), Args(Args) {}
+RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
+: ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
 
-const IntRangeVector &getRanges() const { return Args; }
+const IntRangeVector &getRanges() const { return Ranges; }
 
   private:
 ProgramStateRef applyAsOutOfRange(ProgramStateRef State,
@@ -313,7 +321,8 @@ class StdLibraryFunctionsChecker
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -323,17 +332,37 @@ class StdLibraryFunctionsChecker
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!Arg) {
+  Invalid = true;
+  return;
+} else {
+  assertArgTypeSuitableForSignature(*Arg);
+  this->ArgTys.push_back(*Arg);
+}
+  }
+  if (!RetTy) {
+Invalid = true;
+return;
+  } else {
+assertRetTypeSuitableForSignature(*RetTy);
+this->RetTy = *RetTy;
   }
 }
 
+bool isInvalid() const { return Invalid; }
 bool matches(const FunctionDecl *FD) const;
 
   private:
@@ -387,6 +416,9 @@ class StdLibraryFunctionsChecker
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
+// FIXME Probably the Signature should not be part of the Summary,
+// We can remove once all overload of addToFunctionSummaryMap requires the
+ 

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D86621#2247791 , @jrtc27 wrote:

> GCC on Linux defines `__sparc_v9__` even with `-m32`. I don't know what 
> Solaris does but please don't break other operating systems just because 
> Solaris has broken headers that conflate the CPU and the ABI.

I'd have sworn that I checked gcc on Linux/sparc64: must have been dreaming.

I quoted the relevant snippet of the Studio `cc(1)` man page which shows that 
Solaris compilers predefine `__sparcv8` for `-m32` and `__sparcv9` for `-m64`.  
I'd argue that the creators of the CPU, the OS, and the initial compiler can 
decide to do so as they please (after all, AFAICT that's not part of any ABI 
and I couldn't find an API document that mentioned those), nothing broken or 
conflated here.

I've now looked at the GCC sources and found:

- FreeBSD/sparc64 is 64-bit-only, no 32-bit support in sight
- while the NetBSD/sparc64 port still includes bi-arch support, it is disabled 
because the don't ship a 32-bit environment and even if they did, they only 
define `__sparc_v9__` etc. for `-m64`.
- OpenBSD/sparc64 again is 64-bit only

I'll see how best to reconcile those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

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

steakhal added inline comments.



Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning

martong wrote:
> steakhal wrote:
> > martong wrote:
> > > Hmm, this seems to be quite redundant with the `size_t` tests. Why is it 
> > > not enough to have test for one unsigned type?
> > > Are you trying to check for overflow errors? Then I'd expect to have 
> > > indexes around UINT_MAX and so on. 
> > > 
> > > Same comment applies to the tests with the signed types.
> > In the current implementation - and in any implementation of the checker 
> > logic will have to deal with //integral-promotion// during the 
> > //simplification// of the //array indexer expression// and the given 
> > //extent//.
> > All of these can have different signess and bitwidth which makes the 
> > implementation quite tricky.
> > 
> > In fact, this resulted in the bug, which this patch-stack aims to fix.
> > I'm gonna highlight the related parts in the refactoring patch if you think 
> > it helps.
> Okay, makes sense. 
> 
> It's just very painful to have code repetitions, even in the test code. In 
> gtest unittests we can have tests with type parameters to avoid this kind of 
> repetition. But I guess, there is no way to get rid of this repetition in lit 
> tests.
You can imagine duplicating all of this several times, since the constant in 
the subscript expression could also have different types, such as: `unsigned`, 
`long`, `unsigned long`, etc. Potentially uncovering bugs.
However, I was reluctant to add those as well :|

I could introduce a macro, to expand these - but IMO macros in tests ehh. 
probably reduces readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

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

[PATCH] D86921: [FPEnv] Partially implement #pragma STDC FENV_ROUND

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall, kpn, andrew.w.kaylor, steven_wu, 
mibintc, shafik.
Herald added a subscriber: dexonsmith.
Herald added a project: clang.
sepavloff requested review of this revision.

This change implements pragma STDC FENV_ROUND, which is introduced by
the extension to standard (TS 18661-1). The pragma is implemented only
in frontend, it sets apprpriate state of FPOptions stored in Sema. Use
of these bits in constant evaluation adn/or code generator is not in the
scope of this change.

Parser issues warning on unsuppored pragma when it encounteres pragma
STDC FENV_ROUND, however it makes syntax checks and updates Sema state
as if the pragma were supported.

Primary purpose of the partial implementation is to facilitate
development of non-default floating poin environment. Previously a
developer cannot set non-default rounding mode in sources, this mades
preparing tests for say constant evaluation  substantially complicated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86921

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/Parser/pragma-fenv_round.c

Index: clang/test/Parser/pragma-fenv_round.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_round.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -Wignored-pragmas -verify %s
+
+#pragma STDC FENV_ROUND ON   // expected-warning {{invalid or unsupported rounding mode}}
+
+float func_01(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ROUND FE_DOWNWARD // expected-error{{'#pragma STDC FENV_ROUND' can only appear at file scope or at the start of a compound statement}}
+  // expected-warning@-1{{pragma STDC FENV_ROUND is not supported}}
+  return x + y;
+}
Index: clang/test/AST/ast-dump-fpfeatures.cpp
===
--- clang/test/AST/ast-dump-fpfeatures.cpp
+++ clang/test/AST/ast-dump-fpfeatures.cpp
@@ -34,4 +34,69 @@
 // CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT:   ReturnStmt
-// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
\ No newline at end of file
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
+
+
+
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
+
+float func_10(float x, float y) {
+  return x + y;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} func_10 'float (float, float)'
+// CHECK: BinaryOperator {{.*}} 'float' '+' RoundingMode=3
+
+float func_11(float x, float y) {
+  if (x < 0) {
+#pragma STDC FENV_ROUND FE_UPWARD
+return x + y;
+  }
+  return x - y;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} func_11 'float (float, float)'
+// CHECK: BinaryOperator {{.*}} 'float' '+' RoundingMode=2
+// CHECK: BinaryOperator {{.*}} 'float' '-' RoundingMode=3
+
+
+#pragma STDC FENV_ROUND FE_DYNAMIC
+
+float func_12(float x, float y) {
+  return x + y;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} func_12 'float (float, float)'
+// CHECK: BinaryOperator {{.*}} 'float' '+' RoundingMode=1
+
+#pragma STDC FENV_ROUND FE_TONEAREST
+
+float func_13(float x, float y) {
+  return x + y;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} func_13 'float (float, float)'
+// CHECK: BinaryOperator {{.*}} 'float' '+' RoundingMode=1
+
+
+template 
+T func_14(T x, T y) {
+#pragma STDC FENV_ROUND FE_TOWARDZERO
+  return x + y;
+}
+
+float func_15(float x, float y) {
+#pragma STDC FPENV_ROUND FE_DOWNWARD
+  return func_14(x, y);
+}
+
+// CHECK-LABEL: FunctionTemplateDecl {{.*}} func_14
+// CHECK: FunctionDecl {{.*}} func_14 'T (T, T)'
+// CHECK:   CompoundStmt
+// CHECK-NEXT:ReturnStmt
+// CHECK-NEXT:  BinaryOperator {{.*}} '+' RoundingMode=0
+// CHECK: FunctionDecl {{.*}} func_14 'float (float, float)'
+// CHECK:   CompoundStmt
+// CHECK-NEXT:ReturnStmt
+// CHECK-NEXT:  BinaryOperator {{.*}} 'float' '+' RoundingMode=0
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -979,6 +979,11 @@
 }
 
 void Sema::setRoundingMode(SourceLocation Loc, llvm::RoundingMode FPR) {
+  // C2x: 7.6.2p3  If the FE_DYNAMIC mode is specified and FENV_ACCESS is "off",
+  // the translator may assume that the default rounding mode is in effect.
+  if (FPR == llvm::RoundingMode::Dynamic && !CurFPFeatures.getAllowFEnvAccess())
+FPR = llvm::RoundingMode::NearestTiesToEven;
+
   FPOptionsOverride NewFPFeatures = CurFPFeatureOverrides();
   NewFPFeatures.setRoundingModeOverride(FPR);
   F

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 289111.
awarzynski marked an inline comment as done.
awarzynski added a comment.

Update README.md with instructions for building `flang-new`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/unittests/Driver/SanitizerArgsTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/CMakeLists.txt
  flang/README.md
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/FrontendTool/Utils.h
  flang/lib/CMakeLists.txt
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/CMakeLists.txt
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/CMakeLists.txt
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/emit-obj.f90
  flang/test/lit.cfg.py
  flang/test/lit.site.cfg.py.in
  flang/tools/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/CMakeLists.txt
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/CompilerInstanceTest.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -610,9 +610,12 @@
   continue;
 
 unsigned Flags = getInfo(Id).Flags;
-if (FlagsToInclude && !(Flags & FlagsToInclude))
+
+unsigned ExplicitelyIncluded = Flags & FlagsToInclude;
+if (FlagsToInclude && !(ExplicitelyIncluded))
   continue;
-if (Flags & FlagsToExclude)
+
+if (!(ExplicitelyIncluded) && (Flags & FlagsToExclude))
   continue;
 
 // If an alias doesn't have a help text, show a help text for the aliased
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -225,7 +225,8 @@
   /// \param Usage - USAGE: Usage
   /// \param Title - OVERVIEW: Title
   /// \param FlagsToInclude - If non-zero, only include options with any
-  /// of these flags set.
+  /// of these flags set. Takes precedence over
+  /// FlagsToExclude.
   /// \param FlagsToExclude - Exclude options with any of these flags set.
   /// \param ShowAllAliases - If true, display all options including aliases
   /// that don't have help texts. By default, we display
Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -0,0 +1,52 @@
+//===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+using namespace llvm;
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  // 1. Set-up a basic DiagnosticConsumer
+  std::string diagnosticOutput;
+  llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  auto diagPrinter = std::make_unique(
+  diagnosticsOS, new clang::DiagnosticOptions());
+
+  // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
+  CompilerInstance compInst;
+
+  // 3. Set-up DiagnosticOptions
+  auto diagOpts = new clang::DiagnosticOptions();
+  // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
+  // ensures that a chained d

[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning

steakhal wrote:
> martong wrote:
> > steakhal wrote:
> > > martong wrote:
> > > > Hmm, this seems to be quite redundant with the `size_t` tests. Why is 
> > > > it not enough to have test for one unsigned type?
> > > > Are you trying to check for overflow errors? Then I'd expect to have 
> > > > indexes around UINT_MAX and so on. 
> > > > 
> > > > Same comment applies to the tests with the signed types.
> > > In the current implementation - and in any implementation of the checker 
> > > logic will have to deal with //integral-promotion// during the 
> > > //simplification// of the //array indexer expression// and the given 
> > > //extent//.
> > > All of these can have different signess and bitwidth which makes the 
> > > implementation quite tricky.
> > > 
> > > In fact, this resulted in the bug, which this patch-stack aims to fix.
> > > I'm gonna highlight the related parts in the refactoring patch if you 
> > > think it helps.
> > Okay, makes sense. 
> > 
> > It's just very painful to have code repetitions, even in the test code. In 
> > gtest unittests we can have tests with type parameters to avoid this kind 
> > of repetition. But I guess, there is no way to get rid of this repetition 
> > in lit tests.
> You can imagine duplicating all of this several times, since the constant in 
> the subscript expression could also have different types, such as: 
> `unsigned`, `long`, `unsigned long`, etc. Potentially uncovering bugs.
> However, I was reluctant to add those as well :|
> 
> I could introduce a macro, to expand these - but IMO macros in tests ehh. 
> probably reduces readability.
Macros, ehh. 

Could it work if we instantiated a test function template with the types as 
parameters? AFAIK, we analyze all instantiations as top level. The problem 
seems to be with the `-verify` and matching the `expected-warning` for each 
instantiation...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86870

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

[PATCH] D86721: [clang-format] Parse double-square attributes as pointer qualifiers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289118.
arichardson added a comment.

add test case with argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86721

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,8 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8139,17 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified 
"
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), 
LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous) {
+T = T->MatchingParen->Previous;
+continue;
+  }
 } else if (T->canBePointerOrReferenceQualifier()) {
   T = T->Previous;
   continue;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,8 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8139,17 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified "
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->

[PATCH] D86926: FormatTest: Provide real line number in failure messages

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

Currently a test failure always reports a line number inside verifyFormat()
which is not very helpful to see which test failed. With this change we now
emit the line number where the verify function was called. When using an
IDE such as CLion, the output now includes a clickable link that points to
the call site.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86926

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20,6 +20,7 @@
 
 using clang::tooling::ReplacementTest;
 using clang::tooling::toReplacements;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -65,8 +66,10 @@
 return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
@@ -79,24 +82,23 @@
 }
   }
 
-  void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Code, test::messUp(Code), Style);
   }
 
-  void verifyIncompleteFormat(llvm::StringRef Code,
-  const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef 
Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(),
   format(test::messUp(Code), Style, SC_ExpectIncomplete));
   }
 
-  void verifyGoogleFormat(llvm::StringRef Code) {
-verifyFormat(Code, getGoogleStyle());
-  }
-
-  void verifyIndependentOfContext(llvm::StringRef text) {
-verifyFormat(text);
-verifyFormat(llvm::Twine("void f() { " + text + " }").str());
+  void _verifyIndependentOfContext(const char *File, int Line,
+   llvm::StringRef Text,
+   const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Text, Style);
+_verifyFormat(File, Line, llvm::Twine("void f() { " + Text + " }").str());
   }
 
   /// \brief Verify that clang-format does not crash on the given input.
@@ -108,6 +110,13 @@
   int ReplacementCount;
 };
 
+#define verifyIndependentOfContext(...)
\
+  _verifyIndependentOfContext(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyIncompleteFormat(...)
\
+  _verifyIncompleteFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyGoogleFormat(Code) verifyFormat(Code, getGoogleStyle())
+
 TEST_F(FormatTest, MessUp) {
   EXPECT_EQ("1 2 3", test::messUp("1 2 3"));
   EXPECT_EQ("1 2 3\n", test::messUp("1\n2\n3\n"));


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20,6 +20,7 @@
 
 using clang::tooling::ReplacementTest;
 using clang::tooling::toReplacements;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -65,8 +66,10 @@
 return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
@@ -79,24 +82,23 @@
 }
   }
 
-  void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyForma

[PATCH] D86775: [clang-format] Parse __ptr32/__ptr64 as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289120.
arichardson added a comment.

rebase and add one more testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86775

Files:
  clang/lib/Format/FormatToken.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8028,6 +8028,8 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
@@ -8070,6 +8072,8 @@
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
+  verifyIndependentOfContext("MACRO(A *__ptr32 a);");
+  verifyIndependentOfContext("MACRO(A *__ptr64 a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8141,6 +8145,8 @@
   verifyFormat("x = (foo *_Nonnull)*v;");
   verifyFormat("x = (foo *[[clang::attr]])*v;");
   verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
+  verifyFormat("x = (foo *__ptr32)*v;");
+  verifyFormat("x = (foo *__ptr64)*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
@@ -8149,7 +8155,7 @@
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
   StringRef AllQualifiers =
   "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified 
"
-  "_Nonnull [[clang::attr]]";
+  "_Nonnull [[clang::attr]] __ptr32 __ptr64";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), 
LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -442,7 +442,7 @@
   bool canBePointerOrReferenceQualifier() const {
 return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
-   tok::kw__Null_unspecified);
+   tok::kw__Null_unspecified, tok::kw___ptr32, 
tok::kw___ptr64);
   }
 
   /// Determine whether the token is a simple-type-specifier.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8028,6 +8028,8 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
@@ -8070,6 +8072,8 @@
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
   verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
+  verifyIndependentOfContext("MACRO(A *__ptr32 a);");
+  verifyIndependentOfContext("MACRO(A *__ptr64 a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8141,6 +8145,8 @@
   verifyFormat("x = (foo *_Nonnull)*v;");
   verifyFormat("x = (foo *[[clang::attr]])*v;");
   verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
+  verifyFormat("x = (foo *__ptr32)*v;");
+  verifyFormat("x = (foo *__ptr64)*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
@@ -8149,7 +8155,7 @@
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
   StringRef AllQualifiers =
   "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified "
-  "_Nonnull [[clang::attr]]";
+  "_Nonnull [[clang::attr]] __ptr32 __ptr64";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -442,7 +442,7 @@
   bool canBePointerOrReferenceQualifier() const {
 return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
-   tok::kw__Null_unspecified);
+   t

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289122.
arichardson added a comment.

fix formatting and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8030,7 +8030,20 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  FormatStyle CustomQualifier = getLLVMStyle();
+  // Add indentifers that should not be parsed as a qualifier by default.
+  CustomQualifier.AttributeMacros.push_back("__my_qualifier");
+  CustomQualifier.AttributeMacros.push_back("_My_qualifier");
+  CustomQualifier.AttributeMacros.push_back("my_other_qualifier");
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8074,10 +8087,23 @@
   verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO(A *__ptr32 a);");
   verifyIndependentOfContext("MACRO(A *__ptr64 a);");
+  verifyIndependentOfContext("MACRO(A *__capability);");
+  verifyIndependentOfContext("MACRO(A &__capability);");
+  verifyFormat("MACRO(A *__my_qualifier);");   // type declaration
+  verifyFormat("void f() { MACRO(A * __my_qualifier); }"); // multiplication
+  // If we add __my_qualifier to AttributeMacros it should always be parsed as
+  // a type declaration:
+  verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
+  verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
+  verifyFormat("MACRO(A &B);");
+  verifyFormat("MACRO(A *B);");
+  verifyFormat("void f() { MACRO(A * B); }");
+  verifyFormat("void f() { MACRO(A & B); }");
 
   verifyFormat("DatumHandle const *operator->() const { return input_; }");
   verifyFormat("return options != nullptr && operator==(*options);");
@@ -8127,10 +8153,47 @@
   verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
-  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   verifyFormat("__attribute__((nodebug)) void\n"
"foo() {}\n",
AfterType);
+  verifyFormat("__unused void\n"
+   "foo() {}",
+   AfterType);
+
+  FormatStyle CustomAttrs = getLLVMStyle();
+  CustomAttrs.AttributeMacros.push_back("__unused");
+  CustomAttrs.AttributeMacros.push_back("__attr1");
+  CustomAttrs.AttributeMacros.push_back("__attr2");
+  CustomAttrs.AttributeMacros.push_back("no_underscore_attr");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  // Check that it is parsed as a multiplication without AttributeMacros and
+  // as a pointer qualifier when we add __attr1/__attr2 to AttributeMacros.
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+
+  // Check that these are not parsed as function declarations:
+  CustomAttrs.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  CustomAttrs.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("SomeType s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__unused s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__unused s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType s __unused(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s __unused{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__capability s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__capabilit

[PATCH] D86820: [X86] Add a /tune: option for clang-cl

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Okay, adding it to clang-cl seems fine to me. But I think it could be a simple 
alias?




Comment at: clang/include/clang/Driver/Options.td:4693
   HelpText<"Set architecture for code generation">;
+def _SLASH_tune : CLCompileJoined<"tune:">,
+  HelpText<"Set CPU for optimization without affecting instruction set">;

Any reason this can't just be an alias?


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

https://reviews.llvm.org/D86820

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

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 289128.
ro added a comment.

Define `__sparcv8` only on 32-bit Solaris/SPARC.
Update testcase.
Remove `XFAIL`s that only apply to `clang`/2-stage builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

Files:
  clang/lib/Basic/Targets/Sparc.cpp
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
  compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp

Index: compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Float/cast-overflow.cpp
@@ -11,9 +11,6 @@
 // FIXME: not %run %t 8 2>&1 | FileCheck %s --check-prefix=CHECK-8
 // RUN: not %run %t 9 2>&1 | FileCheck %s --check-prefix=CHECK-9
 
-// Bug 42535
-// XFAIL: sparc-target-arch
-
 // This test assumes float and double are IEEE-754 single- and double-precision.
 
 #if defined(__APPLE__)
Index: compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
===
--- compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
+++ compiler-rt/test/profile/Posix/instrprof-gcov-parallel.test
@@ -10,9 +10,6 @@
 RUN: llvm-cov gcov instrprof-gcov-parallel.target.gcda
 RUN: FileCheck --input-file instrprof-gcov-parallel.target.c.gcov %s
 
-# Bug 42535
-# XFAIL: sparc-target-arch
-
 # Test if the .gcda file is correctly created from one of child processes
 # and counters of all processes are recorded correctly.
 # 707 = CHILDREN * COUNT
Index: clang/test/Preprocessor/predefined-arch-macros.c
===
--- clang/test/Preprocessor/predefined-arch-macros.c
+++ clang/test/Preprocessor/predefined-arch-macros.c
@@ -3233,9 +3233,26 @@
 // RUN: -target sparc-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
 // CHECK_SPARC-V9-NOT: #define __sparcv8 1
+// CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
 // CHECK_SPARC-V9: #define __sparc_v9__ 1
 // CHECK_SPARC-V9: #define __sparcv9 1
-// CHECK_SPARC-V9-NOT: #define __sparcv8 1
+// CHECK_SPARC-V9: #define __sparcv9__ 1
+
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN: -target sparc-sun-solaris \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC_SOLARIS_GCC_ATOMICS
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
+
+// RUN: %clang -mcpu=v8 -E -dM %s -o - 2>&1 \
+// RUN: -target sparc-sun-solaris \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// CHECK_SPARC_SOLARIS_GCC_ATOMICS-V8-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
 
 // RUN: %clang -E -dM %s -o - 2>&1 \
 // RUN: -target sparcel-unknown-linux \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -347,6 +347,8 @@
   case llvm::Triple::sparcv9:
 if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
   return A->getValue();
+if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris())
+  return "v9";
 return "";
 
   case llvm::Triple::x86:
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -166,10 +166,13 @@
   PtrDiffType = SignedLong;
   break;
 }
-// Up to 32 bits are lock-free atomic, but we're willing to do atomic ops
-// on up to 64 bits.
+// Up to 32 bits (V8) or 64 bits (V9) are lock-free atomic, but we're
+// willing to do atomic ops on up to 64 bits.
 MaxAtomicPromoteWidth = 64;
-MaxAtomicInlineWidth = 32;
+if (getCPUGeneration(CPU) == CG_V9)
+  MaxAtomicInlineWidth = 64;
+else
+  MaxAtomicInlineWidth = 32;
   }
 
   void getTargetDefines(const LangOptions &Opts,
Index: clang/lib/Basic/Targets/Sparc.cpp
===
--- clang/lib/Basic/Targets/Sparc.cpp
+++ clang/lib/Basic/Targets/Sparc.cpp
@@ -147,19 +147,20 @@
 void SparcV8TargetInfo::getTargetDef

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:327
+using ObjCForLctxPair =
+std::pair;
+

martong wrote:
> Why it is not enough to simply have ObjCForCollectionStmt* as a key?
`Environment` is the data structure we use to bind **values** to **statements** 
(which would ideally be expressions only). The problem is that a statement on 
its own isn't enough to identify where we are in the analysis:

```lang=c++
void traverse(Graph &G) {
 // SubG may need to be modeled in different stackframes, but has the same Stmt*
 for (Graph &SubG : G->successors()) {
traverse(SubG);
  }
}
```

`LocationContext` can be thought of as a stack function calls (or anything with 
a distinct stack frame). Indeed, if two `(Stmt *, LocationContext *)` pairs are 
equal, we're rebinding the same expression (another loop iteration, goto), if 
only the statements are the same, we're not in the same function call (//stack 
frame//).

Of course `ObjCForCollectionStmt` can not have a value, that is the whole point 
of the patch, but we still need a `LocationContext` to correctly identify 
precisely which loop we're talking about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86736

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

[PATCH] D86930: [clang-format] Handle typename macros inside cast expressions

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

Before: x = (STACK_OF(uint64_t)) & a;
After:  x = (STACK_OF(uint64_t))&a;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86930

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16550,6 +16550,8 @@
   Macros.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("STACK_OF(int)* a;", Macros);
   verifyFormat("STACK_OF(int*)* a;", Macros);
+  verifyFormat("x = (STACK_OF(uint64_t))*a;", Macros);
+  verifyFormat("x = (STACK_OF(uint64_t))&a;", Macros);
 }
 
 TEST_F(FormatTest, AmbersandInLamda) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -244,6 +244,8 @@
   Contexts.back().IsExpression = false;
 } else if (Left->Previous && Left->Previous->is(tok::kw___attribute)) {
   Left->setType(TT_AttributeParen);
+} else if (Left->Previous && Left->Previous->is(TT_TypenameMacro)) {
+  Left->setType(TT_TypenameMacroParen);
 } else if (Left->Previous && Left->Previous->is(TT_ForEachMacro)) {
   // The first argument to a foreach macro is a declaration.
   Contexts.back().IsForEachMacro = true;
@@ -335,6 +337,8 @@
 
 if (Left->is(TT_AttributeParen))
   CurrentToken->setType(TT_AttributeParen);
+if (Left->is(TT_TypenameMacroParen))
+  CurrentToken->setType(TT_TypenameMacroParen);
 if (Left->Previous && Left->Previous->is(TT_JavaAnnotation))
   CurrentToken->setType(TT_JavaAnnotation);
 if (Left->Previous && Left->Previous->is(TT_LeadingJavaAnnotation))
@@ -1855,9 +1859,11 @@
   }
   return T && T->is(TT_PointerOrReference);
 };
-bool ParensAreType = !Tok.Previous || Tok.Previous->is(TT_TemplateCloser) 
||
- Tok.Previous->isSimpleTypeSpecifier() ||
- IsQualifiedPointerOrReference(Tok.Previous);
+bool ParensAreType =
+!Tok.Previous ||
+Tok.Previous->isOneOf(TT_TemplateCloser, TT_TypenameMacroParen) ||
+Tok.Previous->isSimpleTypeSpecifier() ||
+IsQualifiedPointerOrReference(Tok.Previous);
 bool ParensCouldEndDecl =
 Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
 if (ParensAreType && !ParensCouldEndDecl)
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -102,6 +102,7 @@
   TYPE(TrailingReturnArrow)
\
   TYPE(TrailingUnaryOperator)  
\
   TYPE(TypenameMacro)  
\
+  TYPE(TypenameMacroParen) 
\
   TYPE(UnaryOperator)  
\
   TYPE(UntouchableMacroFunc)   
\
   TYPE(CSharpStringLiteral)
\


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16550,6 +16550,8 @@
   Macros.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("STACK_OF(int)* a;", Macros);
   verifyFormat("STACK_OF(int*)* a;", Macros);
+  verifyFormat("x = (STACK_OF(uint64_t))*a;", Macros);
+  verifyFormat("x = (STACK_OF(uint64_t))&a;", Macros);
 }
 
 TEST_F(FormatTest, AmbersandInLamda) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -244,6 +244,8 @@
   Contexts.back().IsExpression = false;
 } else if (Left->Previous && Left->Previous->is(tok::kw___attribute)) {
   Left->setType(TT_AttributeParen);
+} else if (Left->Previous && Left->Previous->is(TT_TypenameMacro)) {
+  Left->setType(TT_TypenameMacroParen);
 } else if (Left->Previous && Left->Previous->is(TT_ForEachMacro)) {
   // The first argument to a foreach macro is a declaration.
   Contexts.back().IsForEachMacro = true;
@@ -335,6 +337,8 @@
 
 if (Left->is(TT_AttributeParen))
   CurrentToken->setType(TT_AttributeParen);
+if (Left->is(TT_TypenameMacroParen))
+  CurrentToken->setTyp

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Actually, `__sparcv8` is only for V8; if you have 32-bit V9 on Solaris it 
defines `__sparcv8plus` _instead_:

  jrtc27@gcc-solaris11:~$ /opt/solarisstudio12.4/bin/cc -E - -xarch=v9 -m32 
-xdumpmacros &1 | grep sparc
  #define __sparcv8plus 1
  #define __sparc 1
  #define sparc 1

In fact, -xarch=v9 + -m32 is a bit weird because -xarch=v9 implies -m64 so the 
argument order matters, and the modern way to do it is (if you read the man 
page, -xarch=sparc means V9 and -xarch=v9 is an alias for -m64 -xarch=sparc...):

  jrtc27@gcc-solaris11:~$ /opt/solarisstudio12.4/bin/cc -E - -m32 -xarch=sparc 
-xdumpmacros &1 | grep sparc
  #define __sparcv8plus 1
  #define __sparc 1
  #define sparc 1

(gcc211 on the GCC compile farm; any open-source developer can register, it's 
not specific to GCC developers despite the name)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2244839 , @leonardchan wrote:

>> The IR differences were caused by the new pass manager which is on by 
>> default for the Fuchsia builder. I've re-landed the patch with a fix for 
>> `CodeGen/attr-arm-sve-vector-bits-call.c` to use the legacy pm with 
>> `-fno-experimental-new-pass-manager`.
>
> Thanks for the update! We do have the new PM on by default, but I'm surprised 
> that this wouldn't appear on `clang-x86_64-debian-new-pass-manager-fast` 
> which also tests the new PM.

No problem, I checked and it did fail for that builder [1] but for some reason 
I didn't receive an email.

[1] 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/14050


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85743

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

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D86621#2249240 , @jrtc27 wrote:

> Actually, `__sparcv8` is only for V8; if you have 32-bit V9 on Solaris it 
> defines `__sparcv8plus` _instead_:
>
>   jrtc27@gcc-solaris11:~$ /opt/solarisstudio12.4/bin/cc -E - -xarch=v9 -m32 
> -xdumpmacros &1 | grep sparc
>   #define __sparcv8plus 1
>   #define __sparc 1
>   #define sparc 1

That's only true up to Studio 12.4: 12.5 and 12.6 define both `__sparcv8` and 
`__sparcv8plus`, and strangely even the 12.4 and 12.3 `cc(1)` man pages only 
documents `__sparcv8` while 12.2 only has `__sparc`.  We could also define 
`__sparcv8plus`, but `gcc` doesn't do that, so it would only help people 
compiling with `cc`.

> In fact, -xarch=v9 + -m32 is a bit weird because -xarch=v9 implies -m64 so 
> the argument order matters, and the modern way to do it is (if you read the 
> man page, -xarch=sparc means V9 and -xarch=v9 is an alias for -m64 
> -xarch=sparc...):

Right; 12.5 and 12.6 `cc` even warn

  cc: Warning: -xarch=v9 is deprecated, use -m64 -xarch=sparc instead



>   jrtc27@gcc-solaris11:~$ /opt/solarisstudio12.4/bin/cc -E - -m32 
> -xarch=sparc -xdumpmacros &1 | grep sparc
>   #define __sparcv8plus 1
>   #define __sparc 1
>   #define sparc 1
>
> (gcc211 on the GCC compile farm; any open-source developer can register, it's 
> not specific to GCC developers despite the name)

I do have a cfarm account already.  However, there's no need for that: I do 
have every version of Studio back to 11 installed locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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

[clang] 2114f71 - [OpenMP] Fix infinite loop in Sema::isOpenMPGlobalCapturedDecl()

[Alexey Bataev via cfe-commits]

Author: Yang Fan
Date: 2020-09-01T08:45:38-04:00
New Revision: 2114f71aaa8dc2e75fe9cd79aa4d72d164e9b95d

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

LOG: [OpenMP] Fix infinite loop in Sema::isOpenMPGlobalCapturedDecl()

Function Sema::isOpenMPGlobalCapturedDecl() has a parameter `unsigned Level`,
but use `Level >= 0` as the condition of `while`, thus cause an infinite loop.
Fix by changing the loop condition to `Level > 0`.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 7b62c841b48a2..352f52d2f6260 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2430,7 +2430,7 @@ bool Sema::isOpenMPGlobalCapturedDecl(ValueDecl *D, 
unsigned Level,
 DSAStackTy::DSAVarData DVar = DSAStack->getImplicitDSA(D, Level);
 if (DVar.CKind != OMPC_shared)
   return true;
-  } while (Level >= 0);
+  } while (Level > 0);
 }
   }
   return true;



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

[PATCH] D86858: [OpenMP] Fix infinite loop in Sema::isOpenMPGlobalCapturedDecl()

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2114f71aaa8d: [OpenMP] Fix infinite loop in 
Sema::isOpenMPGlobalCapturedDecl() (authored by nullptr.cpp, committed by 
ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86858

Files:
  clang/lib/Sema/SemaOpenMP.cpp


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2430,7 +2430,7 @@
 DSAStackTy::DSAVarData DVar = DSAStack->getImplicitDSA(D, Level);
 if (DVar.CKind != OMPC_shared)
   return true;
-  } while (Level >= 0);
+  } while (Level > 0);
 }
   }
   return true;


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2430,7 +2430,7 @@
 DSAStackTy::DSAVarData DVar = DSAStack->getImplicitDSA(D, Level);
 if (DVar.CKind != OMPC_shared)
   return true;
-  } while (Level >= 0);
+  } while (Level > 0);
 }
   }
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 289148.
abhina.sreeskantharajan marked an inline comment as done.
abhina.sreeskantharajan added a comment.

Thanks Hubert, I removed the lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/ZOS.cpp
  clang/lib/Driver/ToolChains/ZOS.h

Index: clang/lib/Driver/ToolChains/ZOS.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/ZOS.h
@@ -0,0 +1,36 @@
+//===--- ZOS.h - z/OS ToolChain Implementations -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY ZOS : public ToolChain {
+public:
+  ZOS(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args);
+  ~ZOS() override;
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }
+  bool isPICDefaultForced() const override { return false; }
+
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
Index: clang/lib/Driver/ToolChains/ZOS.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/ZOS.cpp
@@ -0,0 +1,21 @@
+//===--- ZOS.cpp - z/OS ToolChain Implementations ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ZOS.h"
+#include "CommonArgs.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Option/ArgList.h"
+
+using namespace clang::driver::toolchains;
+using namespace llvm::opt;
+
+ZOS::ZOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+: ToolChain(D, Triple, Args) {}
+
+ZOS::~ZOS() {}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -46,6 +46,7 @@
 #include "ToolChains/VEToolchain.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
+#include "ToolChains/ZOS.h"
 #include "clang/Basic/TargetID.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
@@ -5072,6 +5073,9 @@
 case llvm::Triple::Hurd:
   TC = std::make_unique(*this, Target, Args);
   break;
+case llvm::Triple::ZOS:
+  TC = std::make_unique(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -73,6 +73,7 @@
   ToolChains/XCore.cpp
   ToolChains/PPCLinux.cpp
   ToolChains/InterfaceStubs.cpp
+  ToolChains/ZOS.cpp
   Types.cpp
   XRayArgs.cpp
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
hokein requested review of this revision.

Introduce a new diagnostic, and respect the bracket-depth (256) by default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{fold expression expansion level 3 exceeded maximum of 2}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is 
not an integral constant expression}} \
+ expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13192,6 +13192,13 @@
 Callee, E->getBeginLoc(), LHS.get(), E->getOperator(),
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_expansion_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+return ExprError();
+  }
 
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5089,6 +5089,8 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{fold expression expansion level 3 exceeded maximum of 2}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is not an integral constant expression}} \
+ expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13192,6 +13192,13 @@
 Callee, E->getBeginLoc(), LHS.get(), E->getOperator(),
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_expansion_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+return ExprError();
+  }
 
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5089,6 +5089,8 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_expansion_exceeded: Error<
+  "fold expression expansion level %0 exceeded maximum of %1">;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

Confirming LGTM. @abhina.sreeskantharajan, it seems that you have a good number 
of commits to the project (I see at least three). If you do not yet have commit 
access, it may be appropriate to request it now so that you can push this 
change directly when you believe you have the feedback that you're looking for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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

[PATCH] D85619: [clang][OpenMP][OMPBuilder] Use OMPBuilder to CG `omp single`

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added a comment.

In D85619#2248195 , @kiranchandramohan 
wrote:

> What is the plan for this patch?

Waiting on you to review it ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85619

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

[clang] 5a3ede5 - [NFC] Test commit, whitespace changes

[Abhina Sreeskantharajan via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2020-09-01T09:55:01-04:00
New Revision: 5a3ede58e2c1df53cf1f74d2b8c4916c4a974b0b

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

LOG: [NFC] Test commit, whitespace changes

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

Added: 


Modified: 
clang/test/Preprocessor/init-zos.c

Removed: 




diff  --git a/clang/test/Preprocessor/init-zos.c 
b/clang/test/Preprocessor/init-zos.c
index 50c4ed9e539e..330eb44852dd 100644
--- a/clang/test/Preprocessor/init-zos.c
+++ b/clang/test/Preprocessor/init-zos.c
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
S390X-ZOS %s
 // RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding 
-triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
 
-// S390X-ZOS-GNUXX:#define _EXT 1
-// S390X-ZOS:#define _LONG_LONG 1
-// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
-// S390X-ZOS:#define _OPEN_DEFAULT 1
-// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
-// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
-// S390X-ZOS:#define __370__ 1
-// S390X-ZOS:#define __64BIT__ 1
-// S390X-ZOS:#define __BFP__ 1
-// S390X-ZOS:#define __BOOL__ 1
-// S390X-ZOS-GNUXX:#define __DLL__ 1
-// S390X-ZOS:#define __LONGNAME__ 1
-// S390X-ZOS:#define __MVS__ 1
-// S390X-ZOS:#define __THW_370__ 1
-// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
-// S390X-ZOS:#define __TOS_390__ 1
-// S390X-ZOS:#define __TOS_MVS__ 1
-// S390X-ZOS:#define __XPLINK__ 1
-// S390X-ZOS-GNUXX:#define __wchar_t 1
+// S390X-ZOS-GNUXX: #define _EXT 1
+// S390X-ZOS:   #define _LONG_LONG 1
+// S390X-ZOS-GNUXX: #define _MI_BUILTIN 1
+// S390X-ZOS:   #define _OPEN_DEFAULT 1
+// S390X-ZOS:   #define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX: #define _XOPEN_SOURCE 600
+// S390X-ZOS:   #define __370__ 1
+// S390X-ZOS:   #define __64BIT__ 1
+// S390X-ZOS:   #define __BFP__ 1
+// S390X-ZOS:   #define __BOOL__ 1
+// S390X-ZOS-GNUXX: #define __DLL__ 1
+// S390X-ZOS:   #define __LONGNAME__ 1
+// S390X-ZOS:   #define __MVS__ 1
+// S390X-ZOS:   #define __THW_370__ 1
+// S390X-ZOS:   #define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:   #define __TOS_390__ 1
+// S390X-ZOS:   #define __TOS_MVS__ 1
+// S390X-ZOS:   #define __XPLINK__ 1
+// S390X-ZOS-GNUXX: #define __wchar_t 1



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

[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

[Fady Ghanim via Phabricator via cfe-commits]

fghanim added a comment.

In D79677#2248663 , @lebedev.ri wrote:

> Tests missing

I am not sure what to test here that isn't tested elsewhere in the series. This 
patch is the last in a series, and it represents the "usage" of the 
functionality added by the other patches. Each of the other patches has its 
test as a part of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79677

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

[PATCH] D86926: FormatTest: Provide real line number in failure messages

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:69
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,

Nit: I'm unsure of the convention for using _ at the front of functions, I'm 
not a major fan others might have other opinions



Comment at: clang/unittests/Format/FormatTest.cpp:72
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))

could you add an example as to what the output would look like? 

I think we could pass the File and line number to the EXPECT_EQ in the `<<` 
message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86926

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm wondering if this could be used to help the CUDA usages of clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

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

[PATCH] D86926: FormatTest: Provide real line number in failure messages

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:69
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,

MyDeveloperDay wrote:
> Nit: I'm unsure of the convention for using _ at the front of functions, I'm 
> not a major fan others might have other opinions
I just picked something that's different and makes it easy to change the calls 
without reformatting everything. Also happy to use `verifyImpl` or something 
like that?



Comment at: clang/unittests/Format/FormatTest.cpp:72
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))

MyDeveloperDay wrote:
> could you add an example as to what the output would look like? 
> 
> I think we could pass the File and line number to the EXPECT_EQ in the `<<` 
> message
This is what it looks like now:
```
/Users/alex/cheri/upstream-llvm-project/clang/unittests/Format/FormatTest.cpp:81:
 Failure
  Expected: Expected.str()
  Which is: "void f() { MACRO(LIST(uint64_t) *a); }"
To be equal to: format(test::messUp(Code), ObjCStyle)
  Which is: "void f() { MACRO(LIST(uint64_t) * a); }"
Google Test trace:
/Users/alex/cheri/upstream-llvm-project/clang/unittests/Format/FormatTest.cpp:7859:
 void f() { MACRO(LIST(uint64_t) *a); }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86926

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D86782#2249399 , @MyDeveloperDay 
wrote:

> I'm wondering if this could be used to help the CUDA usages of clang-format

Do you have any examples that are not formatted sensibly? I was also 
considering adding another list for attribute macros with arguments (e.g. `foo 
*addrspace(1)`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Jessica Clarke via Phabricator via cfe-commits]

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

The documentation currently shows `__capability` being included, but from 
looking at this patch does the configuration file not append (which I think 
makes sense, at least for `__capability`) rather than replace?




Comment at: clang/docs/ClangFormatStyleOptions.rst:776
+
+TypeQualifiers: ['__capability', '__output', '__ununsed']
+





Comment at: clang/include/clang/Format/Format.h:599
+  /// \code{.yaml}
+  ///   TypeQualifiers: ['__capability', '__output', '__ununsed']
+  /// \endcode




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

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

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 289161.
tambre added a comment.

Rework patch to simply cleanup runtime handling in the bare-metal toolchain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/arm-compiler-rt.c
  clang/test/Driver/baremetal.cpp

Index: clang/test/Driver/baremetal.cpp
===
--- clang/test/Driver/baremetal.cpp
+++ clang/test/Driver/baremetal.cpp
@@ -2,6 +2,7 @@
 // RUN: -target armv6m-none-eabi \
 // RUN: -T semihosted.lds \
 // RUN: -L some/directory/user/asked/for \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
 // CHECK-V6M-C: "[[PREFIX_DIR:.*]]{{[/\\]+}}{{[^/^\\]+}}{{[/\\]+}}clang{{.*}}" "-cc1" "-triple" "thumbv6m-none-unknown-eabi"
@@ -13,12 +14,14 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-C-SAME: "-lc" "-lm"
+// CHECK-V6M-C-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
 // RUN: -nostdlibinc -nobuiltininc \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -30,38 +33,47 @@
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
+// CHECK-V6M-DEFAULTCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-DEFAULTCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-DEFAULTCXX-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -stdlib=libc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
 // CHECK-V6M-LIBCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}{{[^v].*}}"
-// CHECK-V6M-LIBCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-V6M-LIBCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-LIBCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-LIBCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-LIBCXX-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -stdlib=libstdc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
 // CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECK-V6M-LIBSTDCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
+// CHECK-V6M-LIBSTDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBSTDCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
 // CHECK-V6M-LIBSTDCXX: "{{[^"]*}}ld{{(\.(lld|

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto &ASTC = getASTContext();

It might simplify the patch to move this condition out of the tree and just 
early return for the other case. That is:

```
if (isUnboundedArray) {
  if (!(index.isUnsigned() || !index.isNegative()))
return;

  ...
  return;
}

if (index.isUnsigned() ...
```



Comment at: clang/lib/Sema/SemaChecking.cpp:13989
+  MaxElems <<= AddrBits;
+  MaxElems /= ElemBytes;
+

The size calculations here could probably be simplified by doing something like 
this:

* If getActiveBits of the index is greater than AddrBits, it's indexing outside
* Construct an AddrBits-wide APInt containing the index value
* Use umul_ovf with getTypeSizeInChars(ElementType); if that overflows, it's 
indexing outside



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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

[clang] 3e1e5f5 - [SystemZ][z/OS] Adding initial toolchain for z/OS

[Abhina Sreeskantharajan via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2020-09-01T10:15:01-04:00
New Revision: 3e1e5f54492d5bdebd40388247254e310cf62c3d

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

LOG: [SystemZ][z/OS] Adding initial toolchain for z/OS

This patch adds the initial toolchain for z/OS that will set some defaults. In 
subsequent patches, we plan to add support to use the system linker and 
assembler.

Reviewed By: hubert.reinterpretcast

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

Added: 
clang/lib/Driver/ToolChains/ZOS.cpp
clang/lib/Driver/ToolChains/ZOS.h

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

Removed: 




diff  --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 9463ca5c109d..aeffcf0bb43a 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -73,6 +73,7 @@ add_clang_library(clangDriver
   ToolChains/XCore.cpp
   ToolChains/PPCLinux.cpp
   ToolChains/InterfaceStubs.cpp
+  ToolChains/ZOS.cpp
   Types.cpp
   XRayArgs.cpp
 

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 93b6d8464900..4ac813718eac 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -46,6 +46,7 @@
 #include "ToolChains/VEToolchain.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
+#include "ToolChains/ZOS.h"
 #include "clang/Basic/TargetID.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
@@ -5072,6 +5073,9 @@ const ToolChain &Driver::getToolChain(const ArgList &Args,
 case llvm::Triple::Hurd:
   TC = std::make_unique(*this, Target, Args);
   break;
+case llvm::Triple::ZOS:
+  TC = std::make_unique(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.

diff  --git a/clang/lib/Driver/ToolChains/ZOS.cpp 
b/clang/lib/Driver/ToolChains/ZOS.cpp
new file mode 100644
index ..333b5dc2d4a3
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/ZOS.cpp
@@ -0,0 +1,21 @@
+//===--- ZOS.cpp - z/OS ToolChain Implementations ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ZOS.h"
+#include "CommonArgs.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Option/ArgList.h"
+
+using namespace clang::driver::toolchains;
+using namespace llvm::opt;
+
+ZOS::ZOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+: ToolChain(D, Triple, Args) {}
+
+ZOS::~ZOS() {}

diff  --git a/clang/lib/Driver/ToolChains/ZOS.h 
b/clang/lib/Driver/ToolChains/ZOS.h
new file mode 100644
index ..3a90f4a12428
--- /dev/null
+++ b/clang/lib/Driver/ToolChains/ZOS.h
@@ -0,0 +1,36 @@
+//===--- ZOS.h - z/OS ToolChain Implementations -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY ZOS : public ToolChain {
+public:
+  ZOS(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args);
+  ~ZOS() override;
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }
+  bool isPICDefaultForced() const override { return false; }
+
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H



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

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

[Abhina Sree via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e1e5f54492d: [SystemZ][z/OS] Adding initial toolchain for 
z/OS (authored by abhina.sreeskantharajan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/ZOS.cpp
  clang/lib/Driver/ToolChains/ZOS.h

Index: clang/lib/Driver/ToolChains/ZOS.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/ZOS.h
@@ -0,0 +1,36 @@
+//===--- ZOS.h - z/OS ToolChain Implementations -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY ZOS : public ToolChain {
+public:
+  ZOS(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args);
+  ~ZOS() override;
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }
+  bool isPICDefaultForced() const override { return false; }
+
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_ZOS_H
Index: clang/lib/Driver/ToolChains/ZOS.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/ZOS.cpp
@@ -0,0 +1,21 @@
+//===--- ZOS.cpp - z/OS ToolChain Implementations ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ZOS.h"
+#include "CommonArgs.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Option/ArgList.h"
+
+using namespace clang::driver::toolchains;
+using namespace llvm::opt;
+
+ZOS::ZOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+: ToolChain(D, Triple, Args) {}
+
+ZOS::~ZOS() {}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -46,6 +46,7 @@
 #include "ToolChains/VEToolchain.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
+#include "ToolChains/ZOS.h"
 #include "clang/Basic/TargetID.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
@@ -5072,6 +5073,9 @@
 case llvm::Triple::Hurd:
   TC = std::make_unique(*this, Target, Args);
   break;
+case llvm::Triple::ZOS:
+  TC = std::make_unique(*this, Target, Args);
+  break;
 default:
   // Of these targets, Hexagon is the only one that might have
   // an OS of Linux, in which case it got handled above already.
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -73,6 +73,7 @@
   ToolChains/XCore.cpp
   ToolChains/PPCLinux.cpp
   ToolChains/InterfaceStubs.cpp
+  ToolChains/ZOS.cpp
   Types.cpp
   XRayArgs.cpp
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

@phosek Please review again.
I've overhauled the patch as I realized that per-target runtime directories 
don't make sense for the bare-metal target, since the runtime is only 
distinguished by the //specific// architecture and nothing else.
As a result I've simply changed this into a cleanup doing the same thing as 
D59425 , which seems to have languished.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Chris Hamilton via Phabricator via cfe-commits]

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

I will tinker with the math to simplify as you suggest.  Working with APInt and 
APSInt seems to promulgate sensitive and brittle code, which makes trying 
alternative expressions more tedious than I'd like (which is why I bailed on an 
earlier attempt to simplify this).  However, that same observation about 
brittle code supports the goal that simpler math would be safer, as there would 
presumably be fewer opportunities for AP/APSInt to misbehave as they interact.




Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-// It is possible that the type of the base expression after
-// IgnoreParenCasts is incomplete, even though the type of the base
-// expression before IgnoreParenCasts is complete (see PR39746 for an
-// example). In this case we have no information about whether the array
-// access exceeds the array bounds. However we can still diagnose an array
-// access which precedes the array bounds.
-if (BaseType->isIncompleteType())
-  return;
+if (isUnboundedArray) {
+  const auto &ASTC = getASTContext();

ebevhan wrote:
> It might simplify the patch to move this condition out of the tree and just 
> early return for the other case. That is:
> 
> ```
> if (isUnboundedArray) {
>   if (!(index.isUnsigned() || !index.isNegative()))
> return;
> 
>   ...
>   return;
> }
> 
> if (index.isUnsigned() ...
> ```
There's a bit more code (starting at line 14094 in this patch set) that applies 
in all cases, so an early return here would prevent the "Array declared here" 
note from being generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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

[PATCH] D86795: [PowerPC] Implement builtins for xvcvspbf16 and xvcvbf16spn

[Baptiste Saleil via Phabricator via cfe-commits]

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

LGTM, thanks for adding that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86795

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

[clang] c831a14 - [SystemZ][z/OS] Fix build break in z/OS toolchain

[Abhina Sreeskantharajan via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2020-09-01T10:40:10-04:00
New Revision: c831a14aa16a74fa94c94a351a4bc7812a8bd166

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

LOG: [SystemZ][z/OS] Fix build break in z/OS toolchain

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/ZOS.cpp 
b/clang/lib/Driver/ToolChains/ZOS.cpp
index 333b5dc2d4a3..d57686b8930a 100644
--- a/clang/lib/Driver/ToolChains/ZOS.cpp
+++ b/clang/lib/Driver/ToolChains/ZOS.cpp
@@ -12,8 +12,10 @@
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
 
+using namespace clang::driver;
 using namespace clang::driver::toolchains;
 using namespace llvm::opt;
+using namespace clang;
 
 ZOS::ZOS(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
 : ToolChain(D, Triple, Args) {}



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

[PATCH] D86861: [clangd] Handle templates more consistently in type hierarchy

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks, the current behavior looks nicer than before.

IIUC, you're mostly fixing the issues mentioned in 
https://github.com/clangd/vscode-clangd/pull/44#issuecomment-669092168. If it 
just contains two fixes (this, and https://reviews.llvm.org/D86424), I think it 
might make sense to cherry-pick in release 11.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86861

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

[PATCH] D85924: [clang][feature] Add cxx_abi_relative_vtable feature

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

This LGTM, but I agree someone should sign off on whether `__has_feature` is 
the right approach here. @aaron.ballman ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85924

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D86782#2249408 , @jrtc27 wrote:

> The documentation currently shows `__capability` being included, but from 
> looking at this patch does the configuration file not append (which I think 
> makes sense, at least for `__capability`) rather than replace?

I just wrote a test to check what clang-format does and it appears it's 
neither: D86941 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

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

[PATCH] D86941: [clang-format] Add a test showing the current config file list parsing

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius, jrtc27.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

Currently clang-format starts overriding the default values at index 0
(keeping the existing values) instead of appending or replacing all values.
This patch simply checks the current (IMO surprising) behaviour and does
not attempt to change it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86941

Files:
  clang/test/Format/dump-config-list-override.cpp


Index: clang/test/Format/dump-config-list-override.cpp
===
--- /dev/null
+++ clang/test/Format/dump-config-list-override.cpp
@@ -0,0 +1,24 @@
+/// Check that the ForEachMacros, etc. config entries replace default values 
instead of appending
+/// FIXME: clang-format currently start overriding at index 0 (keeping the 
remaining
+///  values) instead of either appending or completely replacing the values
+/// This behaviour is highly confusing. For now this test documents the 
current state
+// RUN: clang-format -style="{BasedOnStyle: LLVM}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,DEFAULT
+// RUN: clang-format -style="{BasedOnStyle: LLVM, ForEachMacros: 
['OVERRIDE_FOREACH']}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,OVERRIDE,FIXME-SHOULD-NOT-BE
+// RUN: clang-format -style="{BasedOnStyle: LLVM, ForEachMacros: ['M1', 'M2', 
'M3', 'M4']}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,MORE-ENTRIES-THAN-DEFAULT
+
+
+// CHECK-LABEL:   ForEachMacros:
+// DEFAULT-NEXT:  {{^  }}- foreach
+// DEFAULT-NEXT:  {{^  }}- Q_FOREACH
+// DEFAULT-NEXT:  {{^  }}- BOOST_FOREACH
+// OVERRIDE-NEXT: {{^  }}- OVERRIDE_FOREACH
+// FIXME-SHOULD-NOT-BE-NEXT:  {{^  }}- Q_FOREACH
+// FIXME-SHOULD-NOT-BE-NEXT:  {{^  }}- BOOST_FOREACH
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M1
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M2
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M3
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M4
+// CHECK-NEXT:{{^[F-Z]}}


Index: clang/test/Format/dump-config-list-override.cpp
===
--- /dev/null
+++ clang/test/Format/dump-config-list-override.cpp
@@ -0,0 +1,24 @@
+/// Check that the ForEachMacros, etc. config entries replace default values instead of appending
+/// FIXME: clang-format currently start overriding at index 0 (keeping the remaining
+///  values) instead of either appending or completely replacing the values
+/// This behaviour is highly confusing. For now this test documents the current state
+// RUN: clang-format -style="{BasedOnStyle: LLVM}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,DEFAULT
+// RUN: clang-format -style="{BasedOnStyle: LLVM, ForEachMacros: ['OVERRIDE_FOREACH']}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,OVERRIDE,FIXME-SHOULD-NOT-BE
+// RUN: clang-format -style="{BasedOnStyle: LLVM, ForEachMacros: ['M1', 'M2', 'M3', 'M4']}" -dump-config %s | \
+// RUN:   FileCheck %s --check-prefixes=CHECK,MORE-ENTRIES-THAN-DEFAULT
+
+
+// CHECK-LABEL:   ForEachMacros:
+// DEFAULT-NEXT:  {{^  }}- foreach
+// DEFAULT-NEXT:  {{^  }}- Q_FOREACH
+// DEFAULT-NEXT:  {{^  }}- BOOST_FOREACH
+// OVERRIDE-NEXT: {{^  }}- OVERRIDE_FOREACH
+// FIXME-SHOULD-NOT-BE-NEXT:  {{^  }}- Q_FOREACH
+// FIXME-SHOULD-NOT-BE-NEXT:  {{^  }}- BOOST_FOREACH
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M1
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M2
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M3
+// MORE-ENTRIES-THAN-DEFAULT-NEXT: {{^  }}- M4
+// CHECK-NEXT:{{^[F-Z]}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:453
+  equal or known to be non-equal.
+
+- Added :ref:`on-demand parsing ` capability to Cross 
Translation

I've added the patch "Reasoning about comparison expressions in 
RangeConstraintManager". You can mention that as well.


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

https://reviews.llvm.org/D86533

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289167.
arichardson added a comment.

- fix name of key in config file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8030,7 +8030,20 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  FormatStyle CustomQualifier = getLLVMStyle();
+  // Add indentifers that should not be parsed as a qualifier by default.
+  CustomQualifier.AttributeMacros.push_back("__my_qualifier");
+  CustomQualifier.AttributeMacros.push_back("_My_qualifier");
+  CustomQualifier.AttributeMacros.push_back("my_other_qualifier");
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8074,10 +8087,23 @@
   verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO(A *__ptr32 a);");
   verifyIndependentOfContext("MACRO(A *__ptr64 a);");
+  verifyIndependentOfContext("MACRO(A *__capability);");
+  verifyIndependentOfContext("MACRO(A &__capability);");
+  verifyFormat("MACRO(A *__my_qualifier);");   // type declaration
+  verifyFormat("void f() { MACRO(A * __my_qualifier); }"); // multiplication
+  // If we add __my_qualifier to AttributeMacros it should always be parsed as
+  // a type declaration:
+  verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
+  verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
+  verifyFormat("MACRO(A &B);");
+  verifyFormat("MACRO(A *B);");
+  verifyFormat("void f() { MACRO(A * B); }");
+  verifyFormat("void f() { MACRO(A & B); }");
 
   verifyFormat("DatumHandle const *operator->() const { return input_; }");
   verifyFormat("return options != nullptr && operator==(*options);");
@@ -8127,10 +8153,47 @@
   verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
-  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   verifyFormat("__attribute__((nodebug)) void\n"
"foo() {}\n",
AfterType);
+  verifyFormat("__unused void\n"
+   "foo() {}",
+   AfterType);
+
+  FormatStyle CustomAttrs = getLLVMStyle();
+  CustomAttrs.AttributeMacros.push_back("__unused");
+  CustomAttrs.AttributeMacros.push_back("__attr1");
+  CustomAttrs.AttributeMacros.push_back("__attr2");
+  CustomAttrs.AttributeMacros.push_back("no_underscore_attr");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  // Check that it is parsed as a multiplication without AttributeMacros and
+  // as a pointer qualifier when we add __attr1/__attr2 to AttributeMacros.
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+
+  // Check that these are not parsed as function declarations:
+  CustomAttrs.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  CustomAttrs.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("SomeType s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__unused s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__unused s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType s __unused(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s __unused{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__capability s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__ca

[PATCH] D86926: FormatTest: Provide real line number in failure messages

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23
 using clang::tooling::toReplacements;
+using testing::internal::ScopedTrace;
 

This is an internal class in the current gtest.h, but upstream has now included 
it in the public API 
(https://github.com/google/googletest/commit/9c82e7745c257f38d7dd7ff8a9759ea58b6a4e89)
 and documents its use here 
https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#adding-traces-to-assertions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86926

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

[PATCH] D86782: [clang-format] Allow configuring list of macros that map to attributes

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289180.
arichardson added a comment.

- Add test for config parsing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86782

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8030,7 +8030,20 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  FormatStyle CustomQualifier = getLLVMStyle();
+  // Add indentifers that should not be parsed as a qualifier by default.
+  CustomQualifier.AttributeMacros.push_back("__my_qualifier");
+  CustomQualifier.AttributeMacros.push_back("_My_qualifier");
+  CustomQualifier.AttributeMacros.push_back("my_other_qualifier");
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
+  verifyFormat("vector parse_as_multiply;");
+  verifyFormat("vector v;", CustomQualifier);
   verifyFormat("vector v;");
+  verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("foo();");
   verifyFormat("foo();");
@@ -8074,10 +8087,23 @@
   verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO(A *__ptr32 a);");
   verifyIndependentOfContext("MACRO(A *__ptr64 a);");
+  verifyIndependentOfContext("MACRO(A *__capability);");
+  verifyIndependentOfContext("MACRO(A &__capability);");
+  verifyFormat("MACRO(A *__my_qualifier);");   // type declaration
+  verifyFormat("void f() { MACRO(A * __my_qualifier); }"); // multiplication
+  // If we add __my_qualifier to AttributeMacros it should always be parsed as
+  // a type declaration:
+  verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
+  verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
+  verifyFormat("MACRO(A &B);");
+  verifyFormat("MACRO(A *B);");
+  verifyFormat("void f() { MACRO(A * B); }");
+  verifyFormat("void f() { MACRO(A & B); }");
 
   verifyFormat("DatumHandle const *operator->() const { return input_; }");
   verifyFormat("return options != nullptr && operator==(*options);");
@@ -8127,10 +8153,47 @@
   verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
-  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   verifyFormat("__attribute__((nodebug)) void\n"
"foo() {}\n",
AfterType);
+  verifyFormat("__unused void\n"
+   "foo() {}",
+   AfterType);
+
+  FormatStyle CustomAttrs = getLLVMStyle();
+  CustomAttrs.AttributeMacros.push_back("__unused");
+  CustomAttrs.AttributeMacros.push_back("__attr1");
+  CustomAttrs.AttributeMacros.push_back("__attr2");
+  CustomAttrs.AttributeMacros.push_back("no_underscore_attr");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  // Check that it is parsed as a multiplication without AttributeMacros and
+  // as a pointer qualifier when we add __attr1/__attr2 to AttributeMacros.
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;");
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+  verifyFormat("vector v;", CustomAttrs);
+
+  // Check that these are not parsed as function declarations:
+  CustomAttrs.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+  CustomAttrs.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("SomeType s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__unused s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__unused s{InitValue};", CustomAttrs);
+  verifyFormat("SomeType s __unused(InitValue);", CustomAttrs);
+  verifyFormat("SomeType s __unused{InitValue};", CustomAttrs);
+  verifyFormat("SomeType *__capability s(InitValue);", CustomAttrs);
+  verifyFormat("SomeType *__capab

[PATCH] D84049: Disable use of _ExtInt with '__atomic' builtins

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: libcxx/test/libcxx/atomics/ext-int.verify.cpp:1
+// REQUIRES: clang-11
+

This isn't great, since it won't run on clang-12, etc. I'll change it to:

```
// UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
// UNSUPPORTED: apple-clang-9, apple-clang-10, apple-clang-11
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84049

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

[PATCH] D86926: FormatTest: Provide real line number in failure messages

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289181.
arichardson added a comment.

- fix missing argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86926

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20,6 +20,7 @@
 
 using clang::tooling::ReplacementTest;
 using clang::tooling::toReplacements;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -65,8 +66,10 @@
 return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
@@ -79,24 +82,24 @@
 }
   }
 
-  void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Code, test::messUp(Code), Style);
   }
 
-  void verifyIncompleteFormat(llvm::StringRef Code,
-  const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef 
Code,
+   const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Code.str(),
   format(test::messUp(Code), Style, SC_ExpectIncomplete));
   }
 
-  void verifyGoogleFormat(llvm::StringRef Code) {
-verifyFormat(Code, getGoogleStyle());
-  }
-
-  void verifyIndependentOfContext(llvm::StringRef text) {
-verifyFormat(text);
-verifyFormat(llvm::Twine("void f() { " + text + " }").str());
+  void _verifyIndependentOfContext(const char *File, int Line,
+   llvm::StringRef Text,
+   const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Text, Style);
+_verifyFormat(File, Line, llvm::Twine("void f() { " + Text + " }").str(),
+  Style);
   }
 
   /// \brief Verify that clang-format does not crash on the given input.
@@ -108,6 +111,13 @@
   int ReplacementCount;
 };
 
+#define verifyIndependentOfContext(...)
\
+  _verifyIndependentOfContext(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyIncompleteFormat(...)
\
+  _verifyIncompleteFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__)
+#define verifyGoogleFormat(Code) verifyFormat(Code, getGoogleStyle())
+
 TEST_F(FormatTest, MessUp) {
   EXPECT_EQ("1 2 3", test::messUp("1 2 3"));
   EXPECT_EQ("1 2 3\n", test::messUp("1\n2\n3\n"));


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20,6 +20,7 @@
 
 using clang::tooling::ReplacementTest;
 using clang::tooling::toReplacements;
+using testing::internal::ScopedTrace;
 
 namespace clang {
 namespace format {
@@ -65,8 +66,10 @@
 return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
-  void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
 EXPECT_EQ(Expected.str(), format(Expected, Style))
 << "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
@@ -79,24 +82,24 @@
 }
   }
 
-  void verifyFormat(llvm::StringRef Code,
-const FormatStyle &Style = getLLVMStyle()) {
-verifyFormat(Code, test::messUp(Code), Style);
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Code, test::messUp(Code), Style);
   }
 
-  void verifyIncompleteFormat(llvm::StringRef Code,
-  const FormatStyle &Style =

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483
   bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion;
 
   if (CR.index() + 1 == Regions.size() ||

zequanwu wrote:
> vsk wrote:
> > zequanwu wrote:
> > > vsk wrote:
> > > > Why is this deletion necessary? Do we need to introduce 0-length 
> > > > regions which alter the count? An example would help.
> > > Because a single empty line will be a 0 length region. I don't know why 
> > > is this condition necessary before. Does zero-length region exists before 
> > > this change?
> > > 
> > > example:
> > > ```
> > > int main() {
> > > 
> > >   return 0;
> > > }
> > > ```
> > > Before, llvm-cov gives the following.
> > > ```
> > > Counter in file 0 1:12 -> 4:2, #0
> > > Counter in file 0 2:1 -> 2:1, 0
> > > Emitting segments for file: /tmp/a.c
> > > Combined regions:
> > >   1:12 -> 4:2 (count=1)
> > >   2:1 -> 2:1 (count=0)
> > > Segment at 1:12 (count = 1), RegionEntry
> > > Segment at 2:1 (count = 0), RegionEntry, Skipped
> > > Segment at 4:2 (count = 0), Skipped
> > > 1|  1|int main() {
> > > 2|   |
> > > 3|   |return 0;
> > > 4|   |}
> > > ```
> > > After:
> > > ```
> > > Counter in file 0 1:12 -> 4:2, #0
> > > Counter in file 0 2:1 -> 2:1, 0
> > > Emitting segments for file: /tmp/a.c
> > > Combined regions:
> > >   1:12 -> 4:2 (count=1)
> > >   2:1 -> 2:1 (count=0)
> > > Segment at 1:12 (count = 1), RegionEntry
> > > Segment at 2:1 (count = 0), RegionEntry, Skipped
> > > Segment at 2:1 (count = 1)
> > > Segment at 4:2 (count = 0), Skipped
> > > 1|  1|int main() {
> > > 2|   |
> > > 3|  1|return 0;
> > > 4|  1|}
> > > ```
> > It looks like we do occasionally see 0-length regions, possibly due to bugs 
> > in the frontend 
> > (http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp.html#L485).
> > 
> > I don't expect the reporting tools to be able to handle duplicate segments 
> > in a robust way. Having duplicate segments was the source of "messed up" 
> > coverage bugs in the past, due to the contradiction inherent in having two 
> > different segments begin at the same source location.
> > 
> > Do you see some other way to represent empty lines? Perhaps, if we emit a 
> > skipped region for an empty line, we can emit a follow-up segment that 
> > restores the previously-active region starting on the next line? So in this 
> > case:
> > 
> > Segment at 1:12 (count = 1), RegionEntry
> > Segment at 2:1 (count = 0), RegionEntry, Skipped
> > Segment at 3:1 (count = 1)
> > Segment at 4:2 (count = 0), Skipped
> I think we should have the following, because the wrapped segment's count 
> will be used in next line (e.g. line 3). 
> ```
> Segment at 1:12 (count = 1), RegionEntry
> Segment at 2:1 (count = 0), RegionEntry, Skipped
> Segment at 2:1 (count = 1)
> Segment at 4:2 (count = 0), Skipped
> ```
I think one issue with that output is that it contains two segments that start 
at the same location (2:1). Historically, this sort of duplication was a source 
of bugs/inconsistencies  (e.g two entry regions beginning at the same location 
with different counts), and I’m concerned that re-allowing multiple segments 
with the same start location could lead to regressions down the road.

OTOH, your change is consistent with how non-zero length segments are handled, 
and it could be fragile to look for an alternative start location (like 3:1) 
that restores the count from before the skipped region.

I’d be curious to know your thoughts on how to prevent regressions related to 
segments which share the same start location. Maybe they could only be allowed 
in this limited case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84988

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

[PATCH] D86950: [clang-format] Check that */& after typename macros are pointers/references

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

Depends on D86930  (Fix the actual parsing of 
typename macros)
Depends on D86926  (Style argument added to 
verifyIndependentOfContext())


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86950

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8041,6 +8041,14 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  FormatStyle TypeMacros = getLLVMStyle();
+  TypeMacros.TypenameMacros = {"LIST"};
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros); // multiplication
+
   FormatStyle CustomQualifier = getLLVMStyle();
   // Add indentifers that should not be parsed as a qualifier by default.
   CustomQualifier.AttributeMacros.push_back("__my_qualifier");
@@ -8105,6 +8113,9 @@
   // a type declaration:
   verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
   verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+  // Also check that TypenameMacros prevents parsing it as multiplication:
+  verifyIndependentOfContext("MACRO(LIST(uint64_t) * a);"); // multiplication
+  verifyIndependentOfContext("MACRO(LIST(uint64_t) *a);", TypeMacros); // type
 
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
@@ -16552,12 +16563,15 @@
   verifyFormat("STACK_OF(LIST(int)) a, b;", Macros);
   verifyFormat("for (LIST(int) *a = NULL; a;) {\n}", Macros);
   verifyFormat("STACK_OF(int) f(LIST(int) *arg);", Macros);
+  verifyFormat("vector x;", Macros);
+  verifyFormat("vector f(LIST(uint64_t) *arg);", 
Macros);
 
   Macros.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("STACK_OF(int)* a;", Macros);
   verifyFormat("STACK_OF(int*)* a;", Macros);
   verifyFormat("x = (STACK_OF(uint64_t))*a;", Macros);
   verifyFormat("x = (STACK_OF(uint64_t))&a;", Macros);
+  verifyFormat("vector x;", Macros);
 }
 
 TEST_F(FormatTest, AmbersandInLamda) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8041,6 +8041,14 @@
   verifyFormat("vector v;");
   verifyFormat("vector v;");
   verifyFormat("vector v;");
+  FormatStyle TypeMacros = getLLVMStyle();
+  TypeMacros.TypenameMacros = {"LIST"};
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros);
+  verifyFormat("vector v;", TypeMacros); // multiplication
+
   FormatStyle CustomQualifier = getLLVMStyle();
   // Add indentifers that should not be parsed as a qualifier by default.
   CustomQualifier.AttributeMacros.push_back("__my_qualifier");
@@ -8105,6 +8113,9 @@
   // a type declaration:
   verifyFormat("MACRO(A *__my_qualifier);", CustomQualifier);
   verifyFormat("void f() { MACRO(A *__my_qualifier); }", CustomQualifier);
+  // Also check that TypenameMacros prevents parsing it as multiplication:
+  verifyIndependentOfContext("MACRO(LIST(uint64_t) * a);"); // multiplication
+  verifyIndependentOfContext("MACRO(LIST(uint64_t) *a);", TypeMacros); // type
 
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
@@ -16552,12 +16563,15 @@
   verifyFormat("STACK_OF(LIST(int)) a, b;", Macros);
   verifyFormat("for (LIST(int) *a = NULL; a;) {\n}", Macros);
   verifyFormat("STACK_OF(int) f(LIST(int) *arg);", Macros);
+  verifyFormat("vector x;", Macros);
+  verifyFormat("vector f(LIST(uint64_t) *arg);", Macros);
 
   Macros.PointerAlignment = FormatStyle::PAS_Left;
   verifyFormat("STACK_OF(int)* a;", Macros);
   verifyFormat("STACK_OF(int*)* a;", Macros);
   verifyFormat("x = (STACK_OF(uint64_t))*a;", Macros);
   verifyFormat("x = (STACK_OF(uint64_t))&a;", Macros);
+  verifyFormat("vector x;", Macros);
 }
 
 TEST_F(FormatTest, AmbersandInLamda) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84049: Disable use of _ExtInt with '__atomic' builtins

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

jtmott-intel added inline comments.



Comment at: libcxx/test/libcxx/atomics/ext-int.verify.cpp:1
+// REQUIRES: clang-11
+

ldionne wrote:
> This isn't great, since it won't run on clang-12, etc. I'll change it to:
> 
> ```
> // UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, clang-10
> // UNSUPPORTED: apple-clang-9, apple-clang-10, apple-clang-11
> ```
> 
Thanks! That seems better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84049

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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

akhuang closed this revision.
akhuang added a comment.

ah sorry, this was relanded in b1009ee84fc0242bcebd07889306bf39d9b7170f 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

[Milos Lalovic via Phabricator via cfe-commits]

lalovic added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 289190.
ASDenysPetrov added a comment.

Added //timed// functions support and tests for them.


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

https://reviews.llvm.org/D85984

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp

Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1, @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+namespace chrono {
+using duration = int;
+using time_point = int;
+} // namespace chrono
+
+struct mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+struct timed_mutex {
+  void lock();
+  bool try_lock();
+  bool try_lock_for(std::chrono::duration);
+  bool try_lock_until(std::chrono::time_point);
+  void unlock();
+};
+
+struct recursive_mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+struct recursive_timed_mutex {
+  void lock();
+  bool try_lock();
+  bool try_lock_for(std::chrono::duration);
+  bool try_lock_until(std::chrono::time_point);
+  void unlock();
+};
+
+struct shared_mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+  void lock_shared();
+  bool try_lock_shared();
+  void unlock_shared();
+};
+
+struct shared_timed_mutex {
+  void lock();
+  bool try_lock();
+  bool try_lock_for(std::chrono::duration);
+  bool try_lock_until(std::chrono::time_point);
+  void unlock();
+  void lock_shared();
+  bool try_lock_shared();
+  bool try_lock_shared_for(std::chrono::duration);
+  bool try_lock_shared_until(std::chrono::time_point);
+  void unlock_shared();
+};
+
+template 
+struct lock_guard {
+  T &t;
+  lock_guard(T &m) : t(m) {
+t.lock();
+  }
+  ~lock_guard() {
+t.unlock();
+  }
+};
+
+template 
+struct shared_lock {
+  T &t;
+  shared_lock(T &m) : t(m) {
+t.lock_shared();
+  }
+  ~shared_lock() {
+t.unlock_shared();
+  }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+  m1.lock(); // no-warning
+}
+
+void m_ok2() {
+  m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+  m1.unlock();   // no-warning
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+  if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+  m1.unlock();// no-warning
+}
+
+void m_ok10() {
+  std::lock_guard gl(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+  m1.lock(); // no-warning
+  m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+  while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+  while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad7() {
+  if (m1.try_lock()) // no-warning
+m1.lock();   // expected-warning{{This lock has already been acquired}}
+}
+
+std::timed_mutex tm1;
+std::timed_mutex tm2;
+
+// timed_mutex ok
+
+void tm_ok1() {
+  tm1.lock(); // no-warning
+}
+
+void tm_ok2() {
+  tm1.unlock(); // no-warning
+}
+
+void tm_ok3() {
+  tm1.lock();   // no-warning
+  tm1.unlock(); // no-warning
+}
+
+void tm_ok4() {
+  tm1.lock();   // no-warning
+  tm1.unlock(); // no-warning
+  tm1.lock();   // no-warning
+  tm1.unlock(); // no-warning
+}
+
+void tm_ok5() {
+  tm1.lock();   // no-warning
+  tm1.unlock(); // no-warning
+  tm2.lock();   // no-warning
+  tm2.unlock(); // no-warning
+}
+
+void tm_ok6(void) {
+  tm1.lock();   // no-warning
+  tm2.lock();   // no-warning
+  tm2.unlock(); // no-warning
+  tm1.unlock(); // no-warning
+}
+
+void tm_ok7(void) {
+  if (tm1.try_lock()) // no-warning
+tm1.unlock(); // no-warning
+}
+
+void tm_ok8(v

[PATCH] D84049: Disable use of _ExtInt with '__atomic' builtins

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: libcxx/test/libcxx/atomics/ext-int.verify.cpp:1
+// REQUIRES: clang-11
+

jtmott-intel wrote:
> ldionne wrote:
> > This isn't great, since it won't run on clang-12, etc. I'll change it to:
> > 
> > ```
> > // UNSUPPORTED: clang-4, clang-5, clang-6, clang-7, clang-8, clang-9, 
> > clang-10
> > // UNSUPPORTED: apple-clang-9, apple-clang-10, apple-clang-11
> > ```
> > 
> Thanks! That seems better.
8df143c2cc0a3b49440cafe8a80b0f72cee6f166


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84049

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

[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e90d8d4fed8: [builtins] Unify the softfloat division 
implementation (authored by atrosinenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85031

Files:
  compiler-rt/lib/builtins/divdf3.c
  compiler-rt/lib/builtins/divsf3.c
  compiler-rt/lib/builtins/divtf3.c
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/lib/builtins/fp_lib.h
  compiler-rt/lib/builtins/int_util.h
  compiler-rt/test/builtins/Unit/divdf3_test.c

Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -92,5 +92,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
+  return 1;
+if (test__divdf3(-0x1.78abb261d47c8p+794, 0x1.fb01d537cc5aep+266, UINT64_C(0xe0e7c6148ffc23e3)))
+  return 1;
+if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/lib/builtins/int_util.h
===
--- compiler-rt/lib/builtins/int_util.h
+++ compiler-rt/lib/builtins/int_util.h
@@ -28,4 +28,20 @@
 #define COMPILE_TIME_ASSERT2(expr, cnt)\
   typedef char ct_assert_##cnt[(expr) ? 1 : -1] UNUSED
 
+// Force unrolling the code specified to be repeated N times.
+#define REPEAT_0_TIMES(code_to_repeat) /* do nothing */
+#define REPEAT_1_TIMES(code_to_repeat) code_to_repeat
+#define REPEAT_2_TIMES(code_to_repeat) \
+  REPEAT_1_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_3_TIMES(code_to_repeat) \
+  REPEAT_2_TIMES(code_to_repeat)   \
+  code_to_repeat
+#define REPEAT_4_TIMES(code_to_repeat) \
+  REPEAT_3_TIMES(code_to_repeat)   \
+  code_to_repeat
+
+#define REPEAT_N_TIMES_(N, code_to_repeat) REPEAT_##N##_TIMES(code_to_repeat)
+#define REPEAT_N_TIMES(N, code_to_repeat) REPEAT_N_TIMES_(N, code_to_repeat)
+
 #endif // INT_UTIL_H
Index: compiler-rt/lib/builtins/fp_lib.h
===
--- compiler-rt/lib/builtins/fp_lib.h
+++ compiler-rt/lib/builtins/fp_lib.h
@@ -40,9 +40,12 @@
 
 #if defined SINGLE_PRECISION
 
+typedef uint16_t half_rep_t;
 typedef uint32_t rep_t;
+typedef uint64_t twice_rep_t;
 typedef int32_t srep_t;
 typedef float fp_t;
+#define HALF_REP_C UINT16_C
 #define REP_C UINT32_C
 #define significandBits 23
 
@@ -58,9 +61,11 @@
 
 #elif defined DOUBLE_PRECISION
 
+typedef uint32_t half_rep_t;
 typedef uint64_t rep_t;
 typedef int64_t srep_t;
 typedef double fp_t;
+#define HALF_REP_C UINT32_C
 #define REP_C UINT64_C
 #define significandBits 52
 
@@ -102,9 +107,11 @@
 #elif defined QUAD_PRECISION
 #if __LDBL_MANT_DIG__ == 113 && defined(__SIZEOF_INT128__)
 #define CRT_LDBL_128BIT
+typedef uint64_t half_rep_t;
 typedef __uint128_t rep_t;
 typedef __int128_t srep_t;
 typedef long double fp_t;
+#define HALF_REP_C UINT64_C
 #define REP_C (__uint128_t)
 // Note: Since there is no explicit way to tell compiler the constant is a
 // 128-bit integer, we let the constant be casted to 128-bit integer
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- /dev/null
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -0,0 +1,414 @@
+//===-- fp_div_impl.inc - Floating point division -*- C -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements soft-float division with the IEEE-754 default
+// rounding (to nearest, ties to even).
+//
+//===--===//
+
+#include "fp_lib.h"
+
+// The __divXf3__ function implements Newton-Raphson floating point division.
+// It uses 3 iterations for float32, 4 for float64 and 5 for float128,
+// respectively. Due to number of significant bits being roughly doubled
+// every iteration, the two modes are supported: N full-width iterations (as
+// it is done for float32 by default) and (N-

[PATCH] D62574: Add support for target-configurable address spaces.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D62574#2242471 , @Anastasia wrote:

> The only thing is that it would be good to test the new target setting logic 
> somehow... do you have any ideas in mind? We could think of creating a dummy 
> target for that or adding a dummy setting in existing targets - maybe SPIR 
> could be a candidate. We have done something similar in the past if you look 
> at `FakeAddrSpaceMap` in `LangOpts`.

Perhaps we could add a configuration to AMDGPU? That has address spaces.

I'm not a big fan of adding an option just for testing.




Comment at: clang/include/clang/AST/ASTContext.h:2612
+
+  /// Returns true if an explicit cast from address space A to B is legal.
+  /// Explicit conversion between address spaces is permitted if the address

Anastasia wrote:
> Here we should say that this is an extension or enhancement of embedded C 
> rules that clang implements.
> 
> Technically for OpenCL we could refactor to use this functionality as we 
> don't support such explicit casts on disjoint address spaces. But then this 
> would not be necessarily a target setting.
I'm still a bit on the fence about what Embedded-C really stipulates. I don't 
think it's against the spec to simply disallow disjoint conversion altogether, 
but it's only necessary to keep Clang's current implementation working.



Comment at: clang/lib/AST/ASTContext.cpp:10959
+  // Otherwise, ask the target.
+  return Target->isAddressSpaceSupersetOf(A, B);
+}

Anastasia wrote:
> I guess we should add a similar check here as below?
> 
>  
> ```
> if (isTargetAddressSpace(From) || isTargetAddressSpace(To) ||
>   From == LangAS::Default || To == LangAS::Default)
> ```
Is it not useful for targets to be able to express relations of LangASes and 
target ASes?

The method below must be guarded because otherwise all casts between LangASes 
would be legal.



Comment at: clang/lib/AST/ASTContext.cpp:10963
+bool
+ASTContext::isExplicitAddrSpaceConversionLegal(LangAS From, LangAS To) const {
+  // If From and To overlap, the cast is legal.

Anastasia wrote:
> Btw I assume that explicit cast can't reject what is not rejected by implicit 
> cast?
> 
> I am not sure if we need to enforce or document this somehow considering that 
> we provide full configurability now?
It shouldn't do that, no. I don't think there's any way to guarantee this, 
though.

I could add something to the target methods about it.



Comment at: clang/lib/Sema/SemaCast.cpp:2423
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
+  if (!Self.Context.isExplicitAddrSpaceConversionLegal(
+SrcPointeeType.getQualifiers(), DestPointeeType.getQualifiers())) {

Anastasia wrote:
> Btw just to point our that overlapping used to be a commutative operation so 
> you could swap arguments and still get the same answer but for 
> `isExplicitAddrSpaceConversionLegal` is not the same I assume?
Correct, isExplicitAddrSpaceConversionLegal doesn't have to be commutative.



Comment at: clang/lib/Sema/SemaOverload.cpp:3235
   //  - in non-top levels it is not a valid conversion.
+  // FIXME: This should probably be using isExplicitAddrSpaceConversionLegal,
+  // but we don't know if this is an implicit or explicit conversion.

Anastasia wrote:
> Sorry if this has been discussed previously, do you refer to the first or the 
> second case and is there any failing test case?
It refers to the first case of "valid to convert to addr space that is a 
superset in all cases". Technically, it could be permitted even if the addr 
space is not a superset, if this is an explicit cast. But we don't know that. 
We only know if it's a c-style cast, because those are always 'explicit'.

I don't have a test case, unfortunately. I just made this observation as I was 
redoing all of the overlap/superspace checks. It might not even be a problem.



Comment at: clang/lib/Sema/SemaOverload.cpp:5289
 
+  // FIXME: hasAddressSpace is wrong; this check will be skipped if FromType is
+  // not qualified with an address space, but if there's no implicit conversion

Anastasia wrote:
> Do you have a failing test case, if so feel free to create a bug?
Unsure how I'd make one. I suspect this can't be triggered in OpenCL++, because 
you can't really have LangAS::Default on FromType there, can you? It would 
always be some AS.

Doing it in another way would require a target that has configurable ASes, 
which doesn't exist yet. Also, it would require methods qualified with target 
ASes, and that doesn't work yet either.



Comment at: clang/test/CodeGenCXX/address-space-cast.cpp:41
+  // CHECK: %[[cast:.*]] = bitcast i32* %{{.*}} to i8*
+  // CHECK-NEXT: %[[cast2:.*]] = addrspacecast i8* %[[

[PATCH] D86861: [clangd] Handle templates more consistently in type hierarchy

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D86861#2249490 , @hokein wrote:

> IIUC, you're mostly fixing the issues mentioned in 
> https://github.com/clangd/vscode-clangd/pull/44#issuecomment-669092168. If it 
> just contains two fixes (this, and https://reviews.llvm.org/D86424), I think 
> it might make sense to cherry-pick in release 11.

There is also https://github.com/clangd/clangd/issues/510, and there may be 
other issues I haven't identified yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86861

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

[PATCH] D85676: [NewPM] collapsing nested pass mangers of the same type

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D85676#2245787 , @asbirlea wrote:

> Seems like a good idea. I'm curious if you saw any impact from this change.

Yeah, I'm curious too but I didn't measure it yet. What's the recommended ways 
to measure it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85676

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

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I don't have anymore immediate concerns, but I will need more time to comb 
through the rest of the patch in more details... hopefully I can do that in the 
following days.




Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:327
+using ObjCForLctxPair =
+std::pair;
+

Szelethus wrote:
> martong wrote:
> > Why it is not enough to simply have ObjCForCollectionStmt* as a key?
> `Environment` is the data structure we use to bind **values** to 
> **statements** (which would ideally be expressions only). The problem is that 
> a statement on its own isn't enough to identify where we are in the analysis:
> 
> ```lang=c++
> void traverse(Graph &G) {
>  // SubG may need to be modeled in different stackframes, but has the same 
> Stmt*
>  for (Graph &SubG : G->successors()) {
> traverse(SubG);
>   }
> }
> ```
> 
> `LocationContext` can be thought of as a stack function calls (or anything 
> with a distinct stack frame). Indeed, if two `(Stmt *, LocationContext *)` 
> pairs are equal, we're rebinding the same expression (another loop iteration, 
> goto), if only the statements are the same, we're not in the same function 
> call (//stack frame//).
> 
> Of course `ObjCForCollectionStmt` can not have a value, that is the whole 
> point of the patch, but we still need a `LocationContext` to correctly 
> identify precisely which loop we're talking about.
Yeah, I did not think about stack frames, it makes perfect sense what you 
write, thanks for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86736

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

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: hans.
Herald added subscribers: dexonsmith, inglorion.
Herald added a reviewer: MaskRay.
Herald added projects: clang, LLVM.
tejohnson requested review of this revision.

This adds documentation for the options added / changed by D71913 
, which
enabled aggressive WPD under LTO. The lld release notes already
mentioned it, but I expanded the note.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86958

Files:
  clang/docs/ReleaseNotes.rst
  lld/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -267,6 +267,12 @@
   passed to the callee. The feature improves the debugging user experience when
   debugging optimized code.
 
+Changes to the Gold Plugin
+--
+
+* ``whole-program-visibility`` is added to specify that classes have hidden 
LTO visibility in LTO and ThinLTO links of source files compiled with 
-fwhole-program-vtables. See `LTOVisibility 
`_ for details.
+  (`D71913 `_)
+
 Changes to the LLVM tools
 -
 
Index: lld/docs/ReleaseNotes.rst
===
--- lld/docs/ReleaseNotes.rst
+++ lld/docs/ReleaseNotes.rst
@@ -26,7 +26,7 @@
 
 * ``--lto-emit-asm`` is added to emit assembly output for debugging purposes.
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program 
devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have 
hidden LTO visibility in LTO and ThinLTO links of source files compiled with 
-fwhole-program-vtables. See `LTOVisibility 
`_ for details.
   (`D71913 `_)
 * ``--print-archive-stats=`` is added to print the number of members and the 
number of fetched members for each archive.
   The feature is similar to GNU gold's ``--print-symbol-counts=``.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -296,6 +296,8 @@
   ``char8_t`` as the character type of ``u8`` literals. This restores the
   Clang 8 behavior that regressed in Clang 9 and 10.
 - -print-targets has been added to print the registered targets.
+- ``-fwhole-program-vtables`` (along with ``-flto*``) now prepares all classes 
for possible whole program visibility if specified during the LTO link.
+  (`D71913 `_)
 
 New Pragmas in Clang
 


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -267,6 +267,12 @@
   passed to the callee. The feature improves the debugging user experience when
   debugging optimized code.
 
+Changes to the Gold Plugin
+--
+
+* ``whole-program-visibility`` is added to specify that classes have hidden LTO visibility in LTO and ThinLTO links of source files compiled with -fwhole-program-vtables. See `LTOVisibility `_ for details.
+  (`D71913 `_)
+
 Changes to the LLVM tools
 -
 
Index: lld/docs/ReleaseNotes.rst
===
--- lld/docs/ReleaseNotes.rst
+++ lld/docs/ReleaseNotes.rst
@@ -26,7 +26,7 @@
 
 * ``--lto-emit-asm`` is added to emit assembly output for debugging purposes.
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have hidden LTO visibility in LTO and ThinLTO links of source files compiled with -fwhole-program-vtables. See `LTOVisibility `_ for details.
   (`D71913 `_)
 * ``--print-archive-stats=`` is added to print the number of members and the number of fetched members for each archive.
   The feature is similar to GNU gold's ``--print-symbol-counts=``.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -296,6 +296,8 @@
   ``char8_t`` as the character type of ``u8`` literals. This restores the
   Clang 8 behavior that regressed in Clang 9 and 10.
 - -print-targets has been added to print the registered targets.
+- ``-fwhole-program-vtables`` (along with ``-flto*``) now prepares all classes for possible whole program v

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: lld/docs/ReleaseNotes.rst:29
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program 
devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have 
hidden LTO visibility in LTO and ThinLTO links of source files compiled with 
-fwhole-program-vtables. See `LTOVisibility 
`_ for details.
   (`D71913 `_)

-fwhole-program-vtables -> add backquotes



Comment at: llvm/docs/ReleaseNotes.rst:273
+
+* ``whole-program-visibility`` is added to specify that classes have hidden 
LTO visibility in LTO and ThinLTO links of source files compiled with 
-fwhole-program-vtables. See `LTOVisibility 
`_ for details.
+  (`D71913 `_)

``-plugin-opt=whole-program-visibility``


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86958

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

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

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

Szelethus added a comment.

In D86736#2249920 , @martong wrote:

> I don't have anymore immediate concerns, but I will need more time to comb 
> through the rest of the patch in more details... hopefully I can do that in 
> the following days.

Thank you so much! I fear my project might take a turn, so this definitely 
isn't a high priority project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86736

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

[PATCH] D86959: [clang-format] Fix formatting of _Atomic() qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
arichardson requested review of this revision.

Before: _Atomic(uint64_t) * a;
After: _Atomic(uint64_t) *a;

This treats _Atomic the same as the the TypenameMacros and decltype. It
also allows some cleanup by removing checks whether the token before a
paren is kw_decltype and instead checking for TT_TypeDeclarationParen.
While touching this code also extend the decltype test cases to also check
for typeof() and _Atomic(T).

Depends on D86950 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86959

Files:
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -168,6 +168,8 @@
   verifyFormat("vector<::Type> v;");
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
+  verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
   verifyFormat("some::string getName();");
@@ -7904,7 +7906,10 @@
   verifyFormat("auto PointerBinding = [](const char *S) {};");
   verifyFormat("typedef typeof(int(int, int)) *MyFunc;");
   verifyFormat("[](const decltype(*a) &value) {}");
+  verifyFormat("[](const typeof(*a) &value) {}");
+  verifyFormat("[](const _Atomic(*a) &value) {}");
   verifyFormat("decltype(a * b) F();");
+  verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
   verifyFormat("Constructor() : member([](A *a, B *b) {}) {}");
   verifyIndependentOfContext("typedef void (*f)(int *a);");
@@ -7970,6 +7975,8 @@
   verifyFormat("delete *x;", Left);
   verifyFormat("typedef typeof(int(int, int))* MyFuncPtr;", Left);
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
+  verifyFormat("[](const typeof(*a)* ptr) {}", Left);
+  verifyFormat("[](const _Atomic(*a)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8066,6 +8073,8 @@
   verifyFormat("foo();");
   verifyFormat("foo();");
   verifyFormat("decltype(*::std::declval()) void F();");
+  verifyFormat("typeof(*::std::declval()) void F();");
+  verifyFormat("_Atomic(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8089,6 +8098,9 @@
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
+  verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
+  verifyIndependentOfContext("MACRO(decltype(A) *a);");
+  verifyIndependentOfContext("MACRO(typeof(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8639,6 +8651,10 @@
"LooongFunctionDefinition() {}");
   verifyFormat("decltype(LngName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("typeof(LoongName)\n"
+   "LooongFunctionDefinition() {}");
+  verifyFormat("_Atomic(LongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -8988,6 +9004,8 @@
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("auto i = decltype(x){};");
+  verifyFormat("auto i = typeof(x){};");
+  verifyFormat("auto i = _Atomic(x){};");
   verifyFormat("std::vector v = {1, 0 /* comment */};");
   verifyFormat("Node n{1, Node{1000}, //\n"
"   2};");
@@ -11580,6 +11598,8 @@
   verifyFormat("auto i = std::make_unique(5);", NoSpace);
   verifyFormat("size_t x = sizeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
+  verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
+  verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
   verifyFormat("int f(T x) 

[PATCH] D86960: [clang-format] Parse __underlying_type(T) as a type

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
arichardson requested review of this revision.

Before: MACRO(__underlying_type(A) * a);
After:  MACRO(__underlying_type(A) *a);

Depends on D86959 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86960

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -169,6 +169,7 @@
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
   verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = __underlying_type(bar())::value;");
   verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
@@ -7907,6 +7908,7 @@
   verifyFormat("typedef typeof(int(int, int)) *MyFunc;");
   verifyFormat("[](const decltype(*a) &value) {}");
   verifyFormat("[](const typeof(*a) &value) {}");
+  verifyFormat("[](const __underlying_type(*a) &value) {}");
   verifyFormat("[](const _Atomic(*a) &value) {}");
   verifyFormat("decltype(a * b) F();");
   verifyFormat("typeof(a * b) F();");
@@ -7977,6 +7979,7 @@
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
   verifyFormat("[](const typeof(*a)* ptr) {}", Left);
   verifyFormat("[](const _Atomic(*a)* ptr) {}", Left);
+  verifyFormat("[](const __underlying_type(*a)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8075,6 +8078,7 @@
   verifyFormat("decltype(*::std::declval()) void F();");
   verifyFormat("typeof(*::std::declval()) void F();");
   verifyFormat("_Atomic(*::std::declval()) void F();");
+  verifyFormat("__underlying_type(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8101,6 +8105,7 @@
   verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
   verifyIndependentOfContext("MACRO(decltype(A) *a);");
   verifyIndependentOfContext("MACRO(typeof(A) *a);");
+  verifyIndependentOfContext("MACRO(__underlying_type(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8655,6 +8660,8 @@
"LooongFunctionDefinition() {}");
   verifyFormat("_Atomic(LongName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("__underlying_type(LooongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -11600,6 +11607,7 @@
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
   verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", NoSpace);
   verifyFormat("int f(T x) noexcept(x.create());", NoSpace);
   verifyFormat("alignas(128) char a[128];", NoSpace);
   verifyFormat("size_t x = alignof(MyType);", NoSpace);
@@ -11650,6 +11658,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", Space);
   verifyFormat("auto f (int x) -> typeof (x);", Space);
   verifyFormat("auto f (int x) -> _Atomic (x);", Space);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", Space);
   verifyFormat("int f (T x) noexcept (x.create ());", Space);
   verifyFormat("alignas (128) char a[128];", Space);
   verifyFormat("size_t x = alignof (MyType);", Space);
@@ -11704,6 +11713,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
   verifyFormat("auto f (int x) -> typeof (x);", SomeSpace);
   verifyFormat("auto f (int x) -> _Atomic (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", SomeSpace);
   verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
   verifyFormat("alignas (128) char a[128];", SomeSpace);
   verifyFormat("size_t x = alignof (MyType);", SomeSpace);
@@ -14960,6 +14970,7 @@
"  SomeFunction([](decltype(x), A *a) {});\n"
"  SomeFunction([](typeof(x), A *a) {});\n"
"  SomeFunction([](_A

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86559#2243575 , @staffantj wrote:

> As one of the SG14 industry members driving this, I'm firmly in the camp that 
> this is what we're expecting. In the first case the 1/2 case are "neutral". 
> This is a very explicit, and local, marker. Anything else makes it so vague 
> as to be unusable for fine tuned code.

Thank you for chiming in!

> I should also make the point that we are not talking about a feature that is 
> expected, or indeed should be, used by anyone other than someone with an 
> exceedingly good understanding of what is going on.

That doesn't mean we should design something that's really hard to use for 
average folks too.

> This is not a "teach everyone about it, it's safe" feature. It's there to 
> produce a very fine-grained control in those cases where it really matters, 
> and where profiling-guided optimizations would produce exactly the wrong 
> result. Using this feature should be an automatic "is this needed" question 
> in a code review. It is needed sometimes, just rarely.

+1 but I would point out that when PGO is enabled, this attribute is ignored, 
so it's an either/or feature. Either you get tuned optimizations, or you get to 
guess at them, but the current design doesn't let you mix and match. Do see 
that as being a major issue with the design?

In D86559#2246127 , @Mordante wrote:

> In the example above if `x == 0` there will be a jump to `case 0` which then 
> falls through to `case 1` and `case 2` so `case 0` doesn't jump to `case 2` 
> and thus doesn't "execute" the label.

My point was that the standard doesn't say the jump has to be executed, just 
that it has to exist. I think we both agree with how we'd like to interpret 
this bit, but if we're going to write a paper trying to improve the wording in 
the standard, I think this is a minor thing we could perhaps clean up.

> I had thought about RAII before and I think there it's also not a real issue. 
> Your example does the same as:
>
>   if (a)
>  [[likely]] SomeRAIIObj{*a};
>
>
> Here's  no declaration and the attribute is allowed. If the RAII object is 
> used in a declaration I expect it usually will be inside a compound statement 
> to create a scope where the object is alive. In that case the attribute is 
> placed on the compound statement. I don't expect people to really write code 
> like this, but it may happen when using macros.

This is a good point. I also agree that more RAII uses are going to use a 
compound statement than not. I think we're in agreement that this isn't a case 
we need to do anything special for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Staffan Tjernstrom via Phabricator via cfe-commits]

staffantj added a comment.

In D86559#2250034 , @aaron.ballman 
wrote:

> In D86559#2243575 , @staffantj wrote:
>
>> As one of the SG14 industry members driving this, I'm firmly in the camp 
>> that this is what we're expecting. In the first case the 1/2 case are 
>> "neutral". This is a very explicit, and local, marker. Anything else makes 
>> it so vague as to be unusable for fine tuned code.
>
> Thank you for chiming in!
>
>> I should also make the point that we are not talking about a feature that is 
>> expected, or indeed should be, used by anyone other than someone with an 
>> exceedingly good understanding of what is going on.
>
> That doesn't mean we should design something that's really hard to use for 
> average folks too.
>
>> This is not a "teach everyone about it, it's safe" feature. It's there to 
>> produce a very fine-grained control in those cases where it really matters, 
>> and where profiling-guided optimizations would produce exactly the wrong 
>> result. Using this feature should be an automatic "is this needed" question 
>> in a code review. It is needed sometimes, just rarely.
>
> +1 but I would point out that when PGO is enabled, this attribute is ignored, 
> so it's an either/or feature. Either you get tuned optimizations, or you get 
> to guess at them, but the current design doesn't let you mix and match. Do 
> see that as being a major issue with the design?

We did have discussions in SG14 about the possible need for "inverse PGO" 
optimization passes, but we didn't have the compiler dev expertise to know if 
we were on a good track or not. From the 8 years I've spent working on this 
kind of code, I've never mixed the two, since the interactions are just too 
unpredictable. What I will frequently do however, is compare benchmarks between 
PGO generated code, and our hand-specified optimizations. Sometimes the PGO 
detects interesting program flow that a mere human brain could not have 
envisioned, which we can then take advantage of (or de-prioritize) as needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86514: Correctly parse LateParsedTemplates in case of multiple dependent modules

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

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

Thanks!




Comment at: clang/lib/Serialization/ASTReader.cpp:8395
+for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
+ /* In loop */) {
+  FunctionDecl *FD =

Phabricator claims there's a tab here and below. Please double-check you're 
indenting with spaces before commit.


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

https://reviews.llvm.org/D86514

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.



Comment at: clang/test/Modules/explicit-module-maps.cpp:4
+
+// RUN: %clang_cc1 
-fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s
+// expected-no-diagnostics

Minor suggestion here given the content of the file is pretty simple, how about 
something along these lines:

```
// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86559#2250106 , @staffantj wrote:

> In D86559#2250034 , @aaron.ballman 
> wrote:
>
>> In D86559#2243575 , @staffantj 
>> wrote:
>>
>>> As one of the SG14 industry members driving this, I'm firmly in the camp 
>>> that this is what we're expecting. In the first case the 1/2 case are 
>>> "neutral". This is a very explicit, and local, marker. Anything else makes 
>>> it so vague as to be unusable for fine tuned code.
>>
>> Thank you for chiming in!
>>
>>> I should also make the point that we are not talking about a feature that 
>>> is expected, or indeed should be, used by anyone other than someone with an 
>>> exceedingly good understanding of what is going on.
>>
>> That doesn't mean we should design something that's really hard to use for 
>> average folks too.
>>
>>> This is not a "teach everyone about it, it's safe" feature. It's there to 
>>> produce a very fine-grained control in those cases where it really matters, 
>>> and where profiling-guided optimizations would produce exactly the wrong 
>>> result. Using this feature should be an automatic "is this needed" question 
>>> in a code review. It is needed sometimes, just rarely.
>>
>> +1 but I would point out that when PGO is enabled, this attribute is 
>> ignored, so it's an either/or feature. Either you get tuned optimizations, 
>> or you get to guess at them, but the current design doesn't let you mix and 
>> match. Do see that as being a major issue with the design?
>
> We did have discussions in SG14 about the possible need for "inverse PGO" 
> optimization passes, but we didn't have the compiler dev expertise to know if 
> we were on a good track or not. From the 8 years I've spent working on this 
> kind of code, I've never mixed the two, since the interactions are just too 
> unpredictable. What I will frequently do however, is compare benchmarks 
> between PGO generated code, and our hand-specified optimizations. Sometimes 
> the PGO detects interesting program flow that a mere human brain could not 
> have envisioned, which we can then take advantage of (or de-prioritize) as 
> needed.

Great, thank you for your perspective!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1693
+The attributes are used to aid the compiler to determine which branch is
+likely or unlikely to be taken. This is done by marking the first statement in
+the branch with one of the two attributes.

How about: `This is done by marking the branch substatement with one of the two 
attributes.`



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It isn't allowed to annotate a single statement with both ``likely`` and
+``unlikely``. It's allowed to create a contradictions by marking
+the first statement in the ``true`` and ``false`` branch with the same

This sentence and the next one seem to say opposite things. I think this should 
read:

`Annotating the 'true' and 'false' branch of an 'if' statement with the same 
likelihood attribute will result in a diagnostic and the attributes are ignored 
on both branches.`



Comment at: clang/include/clang/Basic/AttrDocs.td:1705
+
+In Clang the attributes will be ignored if they're not placed on the first
+statement in a branch. The C++ Standard recommends to honor them on every

How about: `In Clang, the attributes will be ignored if they're not placed on 
the substatement of a selection statement.`



Comment at: clang/include/clang/Basic/AttrDocs.td:1726
+foo(b);
+   // Whether or not the next statement is in the path of execution depends
+// on the declaration of foo():

The formatting on this comment looks off (maybe different indentation, or tabs 
vs spaces).



Comment at: clang/include/clang/Basic/AttrDocs.td:1736
+
+At the moment the attribute only has effect when used in an ``if`` statement.
+

`if` or `else` statement.



Comment at: clang/lib/AST/JSONNodeDumper.cpp:1452
   attributeOnlyIfTrue("isConstexpr", IS->isConstexpr());
+  dumpLikelihood(JOS, "thenLikelihood", IS->getThenLikelihood());
 }

I don't think this change should be necessary because the attribute should be 
written out for the AST node already. It's worth verifying though.



Comment at: clang/lib/AST/TextNodeDumper.cpp:927
 OS << " has_else";
+  dumpLikelihood(OS, Node->getThenLikelihood());
 }

Similar here.



Comment at: clang/lib/Sema/SemaStmt.cpp:618
+
+  // The function returs the likelihood of Then so invert the likelihood of
+  // Else.

returs -> returns


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

https://reviews.llvm.org/D85091

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

[PATCH] D86721: [clang-format] Parse double-square attributes as pointer qualifiers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman 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/D86721/new/

https://reviews.llvm.org/D86721

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

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

bumping for review. Are there additional reviewers we could add to share the 
burden?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86508

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

[PATCH] D86964: [ASTMatchers] Avoid recursion in ancestor matching to save stack space.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
sammccall requested review of this revision.

A recent change increased the stack size of memoizedMatchesAncestorOfRecursively
leading to stack overflows on real code involving large fold expressions.
It's not totally unreasonable to choke on very deep ASTs, but as common
infrastructure it's be nice if ASTMatchFinder is more robust.
(It already uses data recursion for the regular "downward" traversal.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86964

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp

Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -544,8 +544,9 @@
 // don't invalidate any iterators.
 if (ResultCache.size() > MaxMemoizationEntries)
   ResultCache.clear();
-return memoizedMatchesAncestorOfRecursively(Node, Ctx, Matcher, Builder,
-MatchMode);
+if (MatchMode == AncestorMatchMode::AMM_ParentOnly)
+  return matchesParentOf(Node, Matcher, Builder);
+return matchesAnyAncestorOf(Node, Ctx, Matcher, Builder);
   }
 
   // Matches all registered matchers on the given node and calls the
@@ -699,9 +700,23 @@
   void matchDispatch(const void *) { /* Do nothing. */ }
   /// @}
 
+  // Returns whether a direct parent of \p Node matches \p Matcher.
+  // Unlike matchesAnyAncestorOf there's no memoization: it doesn't save much.
+  bool matchesParentOf(const DynTypedNode &Node, const DynTypedMatcher &Matcher,
+   BoundNodesTreeBuilder *Builder) {
+for (const auto &Parent : ActiveASTContext->getParents(Node)) {
+  BoundNodesTreeBuilder BuilderCopy = *Builder;
+  if (Matcher.matches(Parent, this, &BuilderCopy)) {
+*Builder = std::move(BuilderCopy);
+return true;
+  }
+}
+return false;
+  }
+
   // Returns whether an ancestor of \p Node matches \p Matcher.
   //
-  // The order of matching ((which can lead to different nodes being bound in
+  // The order of matching (which can lead to different nodes being bound in
   // case there are multiple matches) is breadth first search.
   //
   // To allow memoization in the very common case of having deeply nested
@@ -712,51 +727,62 @@
   // Once there are multiple parents, the breadth first search order does not
   // allow simple memoization on the ancestors. Thus, we only memoize as long
   // as there is a single parent.
-  bool memoizedMatchesAncestorOfRecursively(const DynTypedNode &Node,
-ASTContext &Ctx,
-const DynTypedMatcher &Matcher,
-BoundNodesTreeBuilder *Builder,
-AncestorMatchMode MatchMode) {
-// For AST-nodes that don't have an identity, we can't memoize.
-// When doing a single-level match, we don't need to memoize because
-// ParentMap (in ASTContext) already memoizes the result.
-if (!Builder->isComparable() ||
-MatchMode == AncestorMatchMode::AMM_ParentOnly)
-  return matchesAncestorOfRecursively(Node, Ctx, Matcher, Builder,
-  MatchMode);
-
-MatchKey Key;
-Key.MatcherID = Matcher.getID();
-Key.Node = Node;
-Key.BoundNodes = *Builder;
-Key.Traversal = Ctx.getParentMapContext().getTraversalKind();
-Key.Type = MatchType::Ancestors;
-
-// Note that we cannot use insert and reuse the iterator, as recursive
-// calls to match might invalidate the result cache iterators.
-MemoizationMap::iterator I = ResultCache.find(Key);
-if (I != ResultCache.end()) {
-  *Builder = I->second.Nodes;
-  return I->second.ResultOfMatch;
-}
+  //
+  // We avoid a recursive implementation to prevent excessive stack use on
+  // very deep ASTs (similarly to RecursiveASTVisitor's data recursion).
+  bool matchesAnyAncestorOf(DynTypedNode Node, ASTContext &Ctx,
+const DynTypedMatcher &Matcher,
+BoundNodesTreeBuilder *Builder) {
 
-MemoizedMatchResult Result;
-Result.Nodes = *Builder;
-Result.ResultOfMatch = matchesAncestorOfRecursively(
-Node, Ctx, Matcher, &Result.Nodes, MatchMode);
+// Memoization keys that must be updated with the result.
+std::vector Keys;
+// When returning, update the memoization cache.
+auto Finish = [&](bool Result) {
+  for (const auto &Key : Keys) {
+MemoizedMatchResult &CachedResult = ResultCache[Key];
+CachedResult.ResultOfMatch = Result;
+CachedResult.Nodes = *Builder;
+  }
+  return Result;
+};
+
+// Loop while there's a single parent and we want to attempt memoization

[PATCH] D86713: [clang-format] Parse nullability attributes as a pointer qualifier

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D86713#2242381 , @arichardson wrote:

> In D86713#2242354 , @JakeMerdichAMD 
> wrote:
>
>> In D86713#2242300 , @arichardson 
>> wrote:
>>
>>> In D86713#2242253 , 
>>> @JakeMerdichAMD wrote:
>>>
 LGTM, again assuming tests pass locally (patch did not resolve).

 Out of curiosity, is _Atomic on your radar? I found some code in clang 
 proper that handled restrict and _Atomic together. C/C++ have way too many 
 qualifiers...
>>>
>>> I have not looked at _Atomic yet and it's probably low priority for me 
>>> unless it's a trivial change.
>>> My main motivation with these changes is to format a `__capability` pointer 
>>> qualifier correctly (an extension that we add for our out-of-tree CHERI 
>>> C/C++ dialect).
>>
>> I don't know of anyone who uses it yet, so just adding it for posterity, 
>> definitely not a blocker. Were you planning on handling `__capability` 
>> directly or in a user-configurable option? I can imagine other ad-hoc 
>> pointer qualifiers specific to static analysis tools or as properly-ifdef'd 
>> aliases of `__attribute__(...)`, so an option might be useful.
>
> I'm not sure yet what the best solution is. I was thinking of either
>
>   a) option to treat double-underscore prefixed strings after `*/&` as 
> qualifiers (probably on by default, but not sure about that).
>   b) adding a config option with a list of strings that should be treated as 
> qualifiers
>   c) Adding a new __capability keyword to clang-format since it already 
> includes things such as Qt-specific keywords.
>   d) Option b) but with __capability included in the default list of 
> qualifiers.

Implemented option d) in D86782  and fixed 
parsing of _Atomic in D86959 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86713

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

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 289236.
andrewjcg added a comment.

simplify test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86802

Files:
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Modules/explicit-module-maps.cpp


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84345: [AMDGPU] Set the default globals address space to 1

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 289235.
arichardson added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84345

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGVTT.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp

Index: llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
===
--- llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
+++ llvm/unittests/Bitcode/DataLayoutUpgradeTest.cpp
@@ -27,6 +27,10 @@
  "-f80:32-n8:16:32-S32");
   EXPECT_EQ(DL3, "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128"
  "-n32:64-S128");
+
+  // Check that AMDGPU targets add -G1 if it's not present.
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32", "r600"), "e-p:32:32-G1");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64", "amdgcn"), "e-p:64:64-G1");
 }
 
 TEST(DataLayoutUpgradeTest, NoDataLayoutUpgrade) {
@@ -46,6 +50,13 @@
   EXPECT_EQ(DL2, "e-p:32:32");
   EXPECT_EQ(DL3, "e-m:e-i64:64-n32:64");
   EXPECT_EQ(DL4, "e-m:o-i64:64-i128:128-n32:64-S128");
+
+  // Check that AMDGPU targets don't add -G1 if there is already a -G flag.
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:32:32-G2", "r600"), "e-p:32:32-G2");
+  EXPECT_EQ(UpgradeDataLayoutString("G2", "r600"), "G2");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G2", "amdgcn"), "e-p:64:64-G2");
+  EXPECT_EQ(UpgradeDataLayoutString("G2-e-p:64:64", "amdgcn"), "G2-e-p:64:64");
+  EXPECT_EQ(UpgradeDataLayoutString("e-p:64:64-G0", "amdgcn"), "e-p:64:64-G0");
 }
 
 TEST(DataLayoutUpgradeTest, EmptyDataLayout) {
@@ -54,6 +65,10 @@
   "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128", "");
   EXPECT_EQ(DL1, "");
   EXPECT_EQ(DL2, "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128");
+
+  // Check that AMDGPU targets add G1 if it's not present.
+  EXPECT_EQ(UpgradeDataLayoutString("", "r600"), "G1");
+  EXPECT_EQ(UpgradeDataLayoutString("", "amdgcn"), "G1");
 }
 
 } // end namespace
Index: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -344,15 +344,15 @@
 static StringRef computeDataLayout(const Triple &TT) {
   if (TT.getArch() == Triple::r600) {
 // 32-bit pointers.
-  return "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
- "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5";
+return "e-p:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
+   "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1";
   }
 
   // 32-bit private, local, and region pointers. 64-bit global, constant and
   // flat, non-integral buffer fat pointers.
-return "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
+  return "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
  "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
- "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5"
+ "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1"
  "-ni:7";
 }
 
Index: llvm/lib/IR/AutoUpgrade.cpp
===
--- llvm/lib/IR/AutoUpgrade.cpp
+++ llvm/lib/IR/AutoUpgrade.cpp
@@ -4398,11 +4398,17 @@
 }
 
 std::string llvm::UpgradeDataLayoutString(StringRef DL, StringRef TT) {
-  StringRef AddrSpaces = "-p270:32:32-p271:32:32-p272:64:64";
+  Triple T(TT);
+  // For AMDGPU we uprgrade older DataLayouts to include the default globals
+  // address space of 1.
+  if (T.isAMDGPU() && !DL.contains("-G") && !DL.startswith("G")) {
+return DL.empty() ? std::string("G1") : (DL + "-G1").str();
+  }
 
+  std::string AddrSpaces = "-p270:32:32-p271:32:32-p272:64:64";
   // If X86, and the datalayout matches the expected format, add pointer size
   // address spaces to the datalayout.
-  if (!Triple(TT).isX86() || DL.contains(AddrSpaces))
+  if (!T.isX86() || DL.contains(AddrSpaces))
 return std::string(DL);
 
   SmallVector Groups;
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -214,7 +214,7 @@
 GV->setAlignment(Align(8));
 Ident = GV;
   }
-  return Ident;
+  return Builder.CreatePointerCast(Ident, IdentPtr);
 }
 
 Constant *OpenMPIRBuilder::getOrCreateSrcLocStr(StringRef LocStr) {
Index: clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
=

[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added reviewers: rsmith, v.g.vassilev.
bruno added inline comments.



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:870
+  // definition (which is required for `setPure`).
+  const bool pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());

`pure` -> `Pure`



Comment at: clang/test/Modules/set-pure-crash.cpp:2
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+

To speed this up a little, instead of `-O0 -emit-llvm` just use `-fsyntax-only`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86853

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

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 289238.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86958

Files:
  clang/docs/ReleaseNotes.rst
  lld/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -267,6 +267,12 @@
   passed to the callee. The feature improves the debugging user experience when
   debugging optimized code.
 
+Changes to the Gold Plugin
+--
+
+* ``--plugin-opt=whole-program-visibility`` is added to specify that classes 
have hidden LTO visibility in LTO and ThinLTO links of source files compiled 
with ``-fwhole-program-vtables``. See `LTOVisibility 
`_ for details.
+  (`D71913 `_)
+
 Changes to the LLVM tools
 -
 
Index: lld/docs/ReleaseNotes.rst
===
--- lld/docs/ReleaseNotes.rst
+++ lld/docs/ReleaseNotes.rst
@@ -26,7 +26,7 @@
 
 * ``--lto-emit-asm`` is added to emit assembly output for debugging purposes.
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program 
devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have 
hidden LTO visibility in LTO and ThinLTO links of source files compiled with 
``-fwhole-program-vtables``. See `LTOVisibility 
`_ for details.
   (`D71913 `_)
 * ``--print-archive-stats=`` is added to print the number of members and the 
number of fetched members for each archive.
   The feature is similar to GNU gold's ``--print-symbol-counts=``.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -296,6 +296,8 @@
   ``char8_t`` as the character type of ``u8`` literals. This restores the
   Clang 8 behavior that regressed in Clang 9 and 10.
 - -print-targets has been added to print the registered targets.
+- ``-fwhole-program-vtables`` (along with ``-flto*``) now prepares all classes 
for possible whole program visibility if specified during the LTO link.
+  (`D71913 `_)
 
 New Pragmas in Clang
 


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -267,6 +267,12 @@
   passed to the callee. The feature improves the debugging user experience when
   debugging optimized code.
 
+Changes to the Gold Plugin
+--
+
+* ``--plugin-opt=whole-program-visibility`` is added to specify that classes have hidden LTO visibility in LTO and ThinLTO links of source files compiled with ``-fwhole-program-vtables``. See `LTOVisibility `_ for details.
+  (`D71913 `_)
+
 Changes to the LLVM tools
 -
 
Index: lld/docs/ReleaseNotes.rst
===
--- lld/docs/ReleaseNotes.rst
+++ lld/docs/ReleaseNotes.rst
@@ -26,7 +26,7 @@
 
 * ``--lto-emit-asm`` is added to emit assembly output for debugging purposes.
   (`D77231 `_)
-* ``--lto-whole-program-visibility`` is added to support LTO whole-program devirtualization.
+* ``--lto-whole-program-visibility`` is added to specify that classes have hidden LTO visibility in LTO and ThinLTO links of source files compiled with ``-fwhole-program-vtables``. See `LTOVisibility `_ for details.
   (`D71913 `_)
 * ``--print-archive-stats=`` is added to print the number of members and the number of fetched members for each archive.
   The feature is similar to GNU gold's ``--print-symbol-counts=``.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -296,6 +296,8 @@
   ``char8_t`` as the character type of ``u8`` literals. This restores the
   Clang 8 behavior that regressed in Clang 9 and 10.
 - -print-targets has been added to print the registered targets.
+- ``-fwhole-program-vtables`` (along with ``-flto*``) now prepares all classes for possible whole program visibility if specified during the LTO link.
+  (`D71913 `_)
 
 New Pragmas in Clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt

[PATCH] D86965: Do not emit "-tune-cpu generic" for PS4 platform

[Douglas Yung via Phabricator via cfe-commits]

dyung created this revision.
dyung added a reviewer: probinson.
dyung added a project: clang.
dyung requested review of this revision.

For the PS4, do not emit "-tune-cpu generic" since the platform only has 1 
known CPU and we do not want to prevent optimizations by tuning for a generic 
rather than the specific processor it contains.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86965

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ps4-cpu-defaults.cpp


Index: clang/test/Driver/ps4-cpu-defaults.cpp
===
--- clang/test/Driver/ps4-cpu-defaults.cpp
+++ clang/test/Driver/ps4-cpu-defaults.cpp
@@ -1,6 +1,7 @@
 // Check that on the PS4 we default to:
-// -target-cpu btver2 and no exceptions
+// -target-cpu btver2, no exceptions and not -tune-cpu generic
 
 // RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
 // CHECK: "-target-cpu" "btver2"
 // CHECK-NOT: exceptions
+// CHECK-NOT: "-tune-cpu" "generic"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2075,9 +2075,10 @@
 
   // Handle -mtune.
 
-  // Default to "generic" unless -march is present.
+  // Default to "generic" unless -march is present or targetting the PS4
   std::string TuneCPU;
-  if (!Args.hasArg(clang::driver::options::OPT_march_EQ))
+  if (!Args.hasArg(clang::driver::options::OPT_march_EQ) &&
+  !getToolChain().getTriple().isPS4CPU())
 TuneCPU = "generic";
 
   // Override based on -mtune.


Index: clang/test/Driver/ps4-cpu-defaults.cpp
===
--- clang/test/Driver/ps4-cpu-defaults.cpp
+++ clang/test/Driver/ps4-cpu-defaults.cpp
@@ -1,6 +1,7 @@
 // Check that on the PS4 we default to:
-// -target-cpu btver2 and no exceptions
+// -target-cpu btver2, no exceptions and not -tune-cpu generic
 
 // RUN: %clang -target x86_64-scei-ps4 -c %s -### 2>&1 | FileCheck %s
 // CHECK: "-target-cpu" "btver2"
 // CHECK-NOT: exceptions
+// CHECK-NOT: "-tune-cpu" "generic"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2075,9 +2075,10 @@
 
   // Handle -mtune.
 
-  // Default to "generic" unless -march is present.
+  // Default to "generic" unless -march is present or targetting the PS4
   std::string TuneCPU;
-  if (!Args.hasArg(clang::driver::options::OPT_march_EQ))
+  if (!Args.hasArg(clang::driver::options::OPT_march_EQ) &&
+  !getToolChain().getTriple().isPS4CPU())
 TuneCPU = "generic";
 
   // Override based on -mtune.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85032: [builtins] Make divXf3 handle denormal results

[Anatoly Trosinenko via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG93eed63d2f31: [builtins] Make __div[sdt]f3 handle denormal 
results (authored by atrosinenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85032

Files:
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/test/builtins/Unit/divdf3_test.c
  compiler-rt/test/builtins/Unit/divsf3_test.c
  compiler-rt/test/builtins/Unit/divtf3_test.c

Index: compiler-rt/test/builtins/Unit/divtf3_test.c
===
--- compiler-rt/test/builtins/Unit/divtf3_test.c
+++ compiler-rt/test/builtins/Unit/divtf3_test.c
@@ -146,6 +146,13 @@
  UINT64_C(0xfffe)))
 return 1;
 
+// smallest normal value divided by 2.0
+if (test__divtf3(0x1.0p-16382L, 2.L, UINT64_C(0x8000), UINT64_C(0x0)))
+  return 1;
+// smallest subnormal result
+if (test__divtf3(0x1.0p-1022L, 0x1p+52L, UINT64_C(0x0), UINT64_C(0x1)))
+  return 1;
+
 // any / any
 if (test__divtf3(0x1.a23b45362464523375893ab4cdefp+5L,
  0x1.eedcbaba3a94546558237654321fp-1L,
Index: compiler-rt/test/builtins/Unit/divsf3_test.c
===
--- compiler-rt/test/builtins/Unit/divsf3_test.c
+++ compiler-rt/test/builtins/Unit/divsf3_test.c
@@ -92,5 +92,20 @@
 if (test__divsf3(0x1.0p+0F, 0x1.0001p+0F, UINT32_C(0x3f7fff00)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divsf3(0x1.0p-126F, 2.0F, UINT32_C(0x0040)))
+  return 1;
+// smallest subnormal result
+if (test__divsf3(0x1.0p-126F, 0x1p+23F, UINT32_C(0x0001)))
+  return 1;
+
+// some misc test cases obtained by fuzzing against h/w implementation
+if (test__divsf3(-0x1.3e75e6p-108F, -0x1.cf372p+38F, UINT32_C(0x0006)))
+  return 1;
+if (test__divsf3(0x1.e77c54p+81F, -0x1.e77c52p-47F, UINT32_C(0xff80)))
+  return 1;
+if (test__divsf3(0x1.fep-126F, 2.F, UINT32_C(0x0080)))
+  return 1;
+
 return 0;
 }
Index: compiler-rt/test/builtins/Unit/divdf3_test.c
===
--- compiler-rt/test/builtins/Unit/divdf3_test.c
+++ compiler-rt/test/builtins/Unit/divdf3_test.c
@@ -92,6 +92,13 @@
 if (test__divdf3(0x1.0p+0, 0x1.0001p+0, UINT64_C(0x3fefffe0)))
   return 1;
 
+// smallest normal value divided by 2.0
+if (test__divdf3(0x1.0p-1022, 2., UINT64_C(0x0008)))
+  return 1;
+// smallest subnormal result
+if (test__divdf3(0x1.0p-1022, 0x1.0p+52, UINT64_C(0x0001)))
+  return 1;
+
 // some misc test cases obtained by fuzzing against h/w implementation
 if (test__divdf3(0x1.fdc239dd64735p-658, -0x1.fff9364c0843fp-948, UINT64_C(0xd20fdc8fc0ceffb1)))
   return 1;
@@ -99,6 +106,12 @@
   return 1;
 if (test__divdf3(-0x1.da7dfe6048b8bp-875, 0x1.ffc7ea3ff60a4p-610, UINT64_C(0xaf5dab1fe0269e2a)))
   return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.9p+5, UINT64_C(0x51eb851eb852)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+41, UINT64_C(0x07ff)))
+  return 1;
+if (test__divdf3(0x1.0p-1022, 0x1.0028p+52, UINT64_C(0x1)))
+  return 1;
 
 return 0;
 }
Index: compiler-rt/lib/builtins/fp_div_impl.inc
===
--- compiler-rt/lib/builtins/fp_div_impl.inc
+++ compiler-rt/lib/builtins/fp_div_impl.inc
@@ -348,12 +348,17 @@
 // effectively doubling its value as well as its error estimation.
 residualLo = (aSignificand << (significandBits + 1)) - quotient_UQ1 * bSignificand;
 writtenExponent -= 1;
+aSignificand <<= 1;
   } else {
 // Highest bit is 1 (the UQ1.(SB+1) value is in [1, 2)), convert it
 // to UQ1.SB by right shifting by 1. Least significant bit is omitted.
 quotient_UQ1 >>= 1;
 residualLo = (aSignificand << significandBits) - quotient_UQ1 * bSignificand;
   }
+  // NB: residualLo is calculated above for the normal result case.
+  // It is re-computed on denormal path that is expected to be not so
+  // performance-sensitive.
+
   // Now, q cannot be greater than a/b and can differ by at most 8*P * 2^-W + 2^-SB
   // Each NextAfter() increments the floating point value by at least 2^-SB
   // (more, if exponent was incremented).
@@ -381,19 +386,25 @@
   // Now, quotient_UQ1_SB <= the correctly-rounded result
   // and may need taking NextAfter() up to 3 times (see error estimates above)
   // r = a - b * q
+  rep_t absResult;
+  if (writtenExponent > 0) {
+// Clear the implicit bit
+absResult = quotient_UQ1 & significandMask;
+// Insert the exponent
+absResult |= (rep_t)writtenExponent << significandBits;
+