date:20200816

[clang] 000ad1a - [clang] fix a compilation bug

2020-08-16 Thread Yonghong Song via cfe-commits


Author: Yonghong Song
Date: 2020-08-16T21:53:37-07:00
New Revision: 000ad1a976a537256b17788dcf8b50ca117007b8

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

LOG: [clang] fix a compilation bug

With gcc 6.3.0, I hit the following compilation bug:
  /home/yhs/work/llvm-project/clang/lib/Frontend/CompilerInvocation.cpp:
  In function ‘bool ParseCodeGenArgs(clang::CodeGenOptions&, 
llvm::opt::ArgList&,
  clang::InputKind, clang::DiagnosticsEngine&, const clang::TargetOptions&,
  const clang::FrontendOptions&)’:
  /home/yhs/work/llvm-project/clang/lib/Frontend/CompilerInvocation.cpp:780:12:
error: unused variable ‘A’ [-Werror=unused-variable]
 if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
  ^
  cc1plus: all warnings being treated as errors

The bug is introduced by Commit ae6523cd62a4 ("[DebugInfo] Add
-fuse-ctor-homing cc1 flag so we can turn on constructor homing only
if limited debug info is already on.")

Added: 


Modified: 
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 86504ed9f92b..3b69eef12b90 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -777,7 +777,7 @@ static bool ParseCodeGenArgs(CodeGenOptions , ArgList 
, InputKind IK,
   }
   // If -fuse-ctor-homing is set and limited debug info is already on, then use
   // constructor homing.
-  if (Arg *A = Args.getLastArg(OPT_fuse_ctor_homing))
+  if (Args.getLastArg(OPT_fuse_ctor_homing))
 if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
   Opts.setDebugInfo(codegenoptions::DebugInfoConstructor);
 



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

[clang] ae500e4 - Always keep unset fields in FPOptionsOverride zeroed.

2020-08-16 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-08-16T15:44:51-07:00
New Revision: ae500e4d0964adea69372d083416b0f13e9a87eb

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

LOG: Always keep unset fields in FPOptionsOverride zeroed.

There are three fields that the FPOptions default constructor sets to
non-zero values; those fields previously could have been zero or
non-zero depending on whether they'd been explicitly removed from the
FPOptionsOverride set. However, that doesn't seem to ever actually
happen, so this is NFC, except that it makes the AST file representation
of FPOptionsOverride make more sense.

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/test/PCH/determinism.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 31e8af4589b4..f1601aaa9d39 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -475,7 +475,7 @@ class FPOptions {
 /// The is implemented as a value of the new FPOptions plus a mask showing 
which
 /// fields are actually set in it.
 class FPOptionsOverride {
-  FPOptions Options;
+  FPOptions Options = FPOptions(0);
   FPOptions::storage_type OverrideMask = 0;
 
 public:

diff  --git a/clang/test/PCH/determinism.cpp b/clang/test/PCH/determinism.cpp
index 540088e2c91b..ea52971e3b26 100644
--- a/clang/test/PCH/determinism.cpp
+++ b/clang/test/PCH/determinism.cpp
@@ -6,6 +6,10 @@
 // RUN: cmp %t/a.pch %t/b.pch
 
 #pragma float_control(push)
+double fp_control_0(double x) {
+  return -x + x;
+}
+
 double fp_control_1(double x) {
 #pragma float_control(precise, on)
   return -x + x;



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

[clang] 948219d - Replace setter named 'getAsOpaqueInt' with a real getter.

2020-08-16 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-08-16T16:38:33-07:00
New Revision: 948219d1098736758123c43f995ec784db5d921e

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

LOG: Replace setter named 'getAsOpaqueInt' with a real getter.

Clean up a bunch of places where the opaque forms of FPOptions and
FPOptionsOverride were being used inappropriately.

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index f1601aaa9d39..4e277435bf8f 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -402,9 +402,6 @@ class FPOptions {
 setRoundingMode(static_cast(LangOptions::FPR_ToNearest));
 setFPExceptionMode(LangOptions::FPE_Ignore);
   }
-  // Used for serializing.
-  explicit FPOptions(unsigned I) { getFromOpaqueInt(I); }
-
   explicit FPOptions(const LangOptions ) {
 Value = 0;
 setFPContractMode(LO.getDefaultFPContractMode());
@@ -447,7 +444,11 @@ class FPOptions {
   static FPOptions defaultWithoutTrailingStorage(const LangOptions );
 
   storage_type getAsOpaqueInt() const { return Value; }
-  void getFromOpaqueInt(storage_type value) { Value = value; }
+  static FPOptions getFromOpaqueInt(storage_type Value) {
+FPOptions Opts;
+Opts.Value = Value;
+return Opts;
+  }
 
   // We can define most of the accessors automatically:
 #define OPTION(NAME, TYPE, WIDTH, PREVIOUS)
\
@@ -475,7 +476,7 @@ class FPOptions {
 /// The is implemented as a value of the new FPOptions plus a mask showing 
which
 /// fields are actually set in it.
 class FPOptionsOverride {
-  FPOptions Options = FPOptions(0);
+  FPOptions Options = FPOptions::getFromOpaqueInt(0);
   FPOptions::storage_type OverrideMask = 0;
 
 public:
@@ -493,14 +494,9 @@ class FPOptionsOverride {
   (static_cast(1) << FPOptions::StorageBitSize) - 1;
 
   FPOptionsOverride() {}
-  FPOptionsOverride(FPOptions::storage_type Value, FPOptions::storage_type 
Mask)
-  : Options(Value), OverrideMask(Mask) {}
   FPOptionsOverride(const LangOptions )
   : Options(LO), OverrideMask(OverrideMaskBits) {}
 
-  // Used for serializing.
-  explicit FPOptionsOverride(storage_type I) { getFromOpaqueInt(I); }
-
   bool requiresTrailingStorage() const { return OverrideMask != 0; }
 
   void setAllowFPContractWithinStatement() {
@@ -535,14 +531,17 @@ class FPOptionsOverride {
 << FPOptions::StorageBitSize) |
OverrideMask;
   }
-  void getFromOpaqueInt(storage_type I) {
-OverrideMask = I & OverrideMaskBits;
-Options.getFromOpaqueInt(I >> FPOptions::StorageBitSize);
+  static FPOptionsOverride getFromOpaqueInt(storage_type I) {
+FPOptionsOverride Opts;
+Opts.OverrideMask = I & OverrideMaskBits;
+Opts.Options = FPOptions::getFromOpaqueInt(I >> FPOptions::StorageBitSize);
+return Opts;
   }
 
   FPOptions applyOverrides(FPOptions Base) {
-FPOptions Result((Base.getAsOpaqueInt() & ~OverrideMask) |
- (Options.getAsOpaqueInt() & OverrideMask));
+FPOptions Result =
+FPOptions::getFromOpaqueInt((Base.getAsOpaqueInt() & ~OverrideMask) |
+ (Options.getAsOpaqueInt() & 
OverrideMask));
 return Result;
   }
 

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 558602b76e07..1d12551a8ad2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@ class Sema final {
   PragmaStack CodeSegStack;
 
   // This stack tracks the current state of Sema.CurFPFeatures.
-  PragmaStack FpPragmaStack;
+  PragmaStack FpPragmaStack;
   FPOptionsOverride CurFPFeatureOverrides() {
 FPOptionsOverride result;
 if (!FpPragmaStack.hasValue()) {
   result = FPOptionsOverride();
 } else {
-  result = FPOptionsOverride(FpPragmaStack.CurrentValue);
+  result = FpPragmaStack.CurrentValue;
 }
 return result;
   }
@@ -1405,12 +1405,12 @@ class Sema final {
   S.CurFPFeatures = OldFPFeaturesState;
   S.FpPragmaStack.CurrentValue = OldOverrides;
 }
-FPOptionsOverride::storage_type getOverrides() { return OldOverrides; }
+FPOptionsOverride getOverrides() { return OldOverrides; }
 
   private:
 Sema& S;
 FPOptions OldFPFeaturesState;
-FPOptionsOverride::storage_type

[clang] ae30670 - Use consistent code for setting FPFeatures from operator constructors.

2020-08-16 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-08-16T15:40:38-07:00
New Revision: ae3067055b33f6ab5657fbae5845cc743b91c299

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

LOG: Use consistent code for setting FPFeatures from operator constructors.

Added: 


Modified: 
clang/lib/AST/Expr.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index f5aecfb7fdae..e3e5e9dbdc62 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4487,8 +4487,8 @@ BinaryOperator::BinaryOperator(const ASTContext , 
Expr *lhs, Expr *rhs,
   SubExprs[LHS] = lhs;
   SubExprs[RHS] = rhs;
   BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
-  if (BinaryOperatorBits.HasFPFeatures)
-*getTrailingFPFeatures() = FPFeatures;
+  if (hasStoredFPFeatures())
+setStoredFPFeatures(FPFeatures);
   setDependence(computeDependence(this));
 }
 
@@ -4504,8 +4504,8 @@ BinaryOperator::BinaryOperator(const ASTContext , 
Expr *lhs, Expr *rhs,
   SubExprs[LHS] = lhs;
   SubExprs[RHS] = rhs;
   BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
-  if (BinaryOperatorBits.HasFPFeatures)
-*getTrailingFPFeatures() = FPFeatures;
+  if (hasStoredFPFeatures())
+setStoredFPFeatures(FPFeatures);
   setDependence(computeDependence(this));
 }
 



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

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

2020-08-16 Thread Nathan Ridge via Phabricator via cfe-commits

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



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1843
+  if (const auto *TC = D->getTypeConstraint()) {
+TRY_TO(TraverseStmt(TC->getImmediatelyDeclaredConstraint()));
 TRY_TO(TraverseConceptReference(*TC));

hokein wrote:
> nridge wrote:
> > hokein wrote:
> > > nridge wrote:
> > > > hokein wrote:
> > > > > Looks like we may visit some nodes in `ConceptReference` twice:
> > > > > -  getImmediatelyDeclaredConstraint returns a 
> > > > > `ConceptSpecializationExpr` (most cases?) which is a subclass of 
> > > > > `ConceptReference`;
> > > > > - `TraverseStmt(ConceptSpecializationExpr*)` will dispatch to 
> > > > > `TraverseConceptSpecializationExpr` which invokes 
> > > > > `TraverseConceptReference` (see Line 2719);
> > > > > 
> > > > > 
> > > > > It is sad that we don't have enough test coverage, could you write 
> > > > > some tests in `clang/unittests/Tooling/RecursiveASTVisitorTests/`?
> > > > It is true that there will be two calls to 
> > > > `TraverseConceptReference()`. However, they are called on two different 
> > > > `ConceptReference` objects:
> > > > 
> > > >   * the call in `TraverseConceptSpecializationExpr` will visit the base 
> > > > subobject of the `ConceptSpecializationExpr` (which inherits from 
> > > > `ConceptReference`)
> > > >   * the call in `TraverseTemplateTypeParmDecl` will visit the base 
> > > > subobject of the `TypeConstraint` (which also inherits from 
> > > > `ConceptReference`).
> > > > 
> > > > So, I think this is fine -- there are two distinct `ConceptReference` 
> > > > objects in the AST, and with this patch we visit both of them.
> > > I understand that they are two different `ConceptReference` objects, but 
> > > they have members (`FoundDecl`, `ArgsAsWritten`) that may refer to the 
> > > same AST nodes.
> > > 
> > > ```
> > > template 
> > > concept binary_concept = true;
> > > struct Foo {};
> > > 
> > > template T> // the template argument Foo will be 
> > > visited twice.
> > > void k2();
> > > ```
> > > 
> > > I'm not sure what's is the right approach here, I can see two options:
> > > 
> > > - traverse TC + immediately-declared-constraint expr, this seem to cause 
> > > some ast nodes visited twice (maybe not a big deal?)
> > > - just traverse immediately-declared-constraint expr, this seems not 
> > > breaking any tests, but the immediately-declared-constraint expr could be 
> > > nullptr (e.g. broken code, missing required template arguments); or the 
> > > immediately-declared-constraint expr could be a `CXXFoldExpr`, which will 
> > > make some members in `ConceptReference` not be visited;
> > > 
> > > @rsmith, do you have any idea about this?
> > > 
> > From clangd's point of view, it would be sufficient to visit the 
> > immediately-declared-constraint-expr without visiting any of its 
> > descendants. However, I'm not sure how to accomplish this using 
> > `RecursiveASTVisitor`. (I think I'd want to call 
> > `WalkUpFromXXX(TC->getImmediatelyDeclaredConstraint())`, where `XXX` is the 
> > dynamic type of the immediately-delcared-constraint, but I don't know how 
> > to dispatch to that dynamic type; `RecursiveASTVisitor` seems to be 
> > designed to do the dispatch on `Traverse` calls, not `WalkUpFrom` calls).
> Thinking more about this.
> 
> I'm aligned on the idea of only traversing the immediately declared 
> constraint since the TypeContraint is a wrapper of it. The regression is that 
> we'd not perform traversal if the immediately declared constraint is nullptr 
> (this just happens for broken code). 
> 
> Fix ideas:
> - build a recovery expression if we can't not build a immediately declared 
> constraint due to the missing required template arguments etc;
> - still traverse the TC if immediately declared constraint is nullptr;
> 
> 
Thanks, this suggestion makes sense to me. I went with the second approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84136

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

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

2020-08-16 Thread Nathan Ridge via Phabricator via cfe-commits

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

Update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84136

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
@@ -0,0 +1,46 @@
+//===- unittest/Tooling/RecursiveASTVisitorTests/Concept.cpp
+//===//
+//
+// 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 "TestVisitor.h"
+#include "clang/AST/ExprConcepts.h"
+
+using namespace clang;
+
+namespace {
+
+struct ConceptVisitor : ExpectedLocationVisitor {
+  bool VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) {
+++ConceptSpecializationExprsVisited;
+return true;
+  }
+  bool TraverseConceptReference(const ConceptReference ) {
+++ConceptReferencesTraversed;
+return true;
+  }
+
+  int ConceptSpecializationExprsVisited = 0;
+  int ConceptReferencesTraversed = 0;
+};
+
+TEST(RecursiveASTVisitor, ConstrainedParameter) {
+  ConceptVisitor Visitor;
+  EXPECT_TRUE(Visitor.runOver("template  concept Fooable = true;\n"
+  "template  void bar(T);",
+  ConceptVisitor::Lang_CXX2a));
+  // Check that we visit the "Fooable T" template parameter's TypeConstraint's
+  // ImmediatelyDeclaredConstraint, which is a ConceptSpecializationExpr.
+  EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited);
+  // There are two ConceptReference objects in the AST: the base subobject
+  // of the ConceptSpecializationExpr, and the base subobject of the
+  // TypeConstraint itself. To avoid traversing the concept and arguments
+  // multiple times, we only traverse one.
+  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -22,6 +22,7 @@
   RecursiveASTVisitorTests/Attr.cpp
   RecursiveASTVisitorTests/Callbacks.cpp
   RecursiveASTVisitorTests/Class.cpp
+  RecursiveASTVisitorTests/Concept.cpp
   RecursiveASTVisitorTests/ConstructExpr.cpp
   RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
   RecursiveASTVisitorTests/CXXMemberCall.cpp
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1777,8 +1777,16 @@
   // D is the "T" in something like "template class vector;"
   if (D->getTypeForDecl())
 TRY_TO(TraverseType(QualType(D->getTypeForDecl(), 0)));
-  if (const auto *TC = D->getTypeConstraint())
-TRY_TO(TraverseConceptReference(*TC));
+  if (const auto *TC = D->getTypeConstraint()) {
+Expr *IDC = TC->getImmediatelyDeclaredConstraint();
+TRY_TO(TraverseStmt(IDC));
+// Avoid traversing the ConceptReference in the TypeCosntraint
+// if we have an immediately-declared-constraint, otherwise
+// we'll end up visiting the concept and the arguments in
+// the TC twice.
+if (!IDC)
+  TRY_TO(TraverseConceptReference(*TC));
+  }
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
 TRY_TO(TraverseTypeLoc(D->getDefaultArgumentInfo()->getTypeLoc()));
 })
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -433,6 +433,28 @@
   )cpp";
   EXPECT_DECLS("ConceptSpecializationExpr",
{"template  concept Fooable = true;"});
+
+  // constrained-parameter
+  Code = R"cpp(
+template 
+concept Fooable = true;
+
+template <[[Fooable]] T>
+void bar(T t);
+  )cpp";
+  EXPECT_DECLS("ConceptSpecializationExpr",
+   {"template  concept Fooable = true;"});
+
+  // partial-concept-id
+  Code = R"cpp(
+template 
+concept Fooable = true;
+
+template <[[Fooable]] T>
+void bar(T t);
+  )cpp";
+  EXPECT_DECLS("ConceptSpecializationExpr",
+   {"template  concept Fooable = true;"});
 }
 
 TEST_F(TargetDeclTest, FunctionTemplate) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

2020-08-16 Thread Nathan Ridge via Phabricator via cfe-commits

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

Only visit the TypeConstraint if the immediately-declared-constraint is null


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84136

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
@@ -0,0 +1,45 @@
+//===- unittest/Tooling/RecursiveASTVisitorTests/Concept.cpp
+//===//
+//
+// 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 "TestVisitor.h"
+#include "clang/AST/ExprConcepts.h"
+
+using namespace clang;
+
+namespace {
+
+struct ConceptVisitor : ExpectedLocationVisitor {
+  bool VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) {
+++ConceptSpecializationExprsVisited;
+return true;
+  }
+  bool TraverseConceptReference(const ConceptReference ) {
+++ConceptReferencesTraversed;
+return true;
+  }
+
+  int ConceptSpecializationExprsVisited = 0;
+  int ConceptReferencesTraversed = 0;
+};
+
+TEST(RecursiveASTVisitor, ConstrainedParameter) {
+  ConceptVisitor Visitor;
+  EXPECT_TRUE(Visitor.runOver("template  concept Fooable = true;\n"
+  "template  void bar(T);",
+  ConceptVisitor::Lang_CXX2a));
+  // Check that we visit the "Fooable T" template parameter's TypeConstraint's
+  // ImmediatelyDeclaredConstraint, which is a ConceptSpecializationExpr.
+  EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited);
+  // There are two ConceptReference objects in the AST: the base subobject
+  // of the ConceptSpecializationExpr, and the base subobject of the
+  // TypeConstraint itself. Check that we traverse both.
+  EXPECT_EQ(2, Visitor.ConceptReferencesTraversed);
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -22,6 +22,7 @@
   RecursiveASTVisitorTests/Attr.cpp
   RecursiveASTVisitorTests/Callbacks.cpp
   RecursiveASTVisitorTests/Class.cpp
+  RecursiveASTVisitorTests/Concept.cpp
   RecursiveASTVisitorTests/ConstructExpr.cpp
   RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
   RecursiveASTVisitorTests/CXXMemberCall.cpp
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1777,8 +1777,16 @@
   // D is the "T" in something like "template class vector;"
   if (D->getTypeForDecl())
 TRY_TO(TraverseType(QualType(D->getTypeForDecl(), 0)));
-  if (const auto *TC = D->getTypeConstraint())
-TRY_TO(TraverseConceptReference(*TC));
+  if (const auto *TC = D->getTypeConstraint()) {
+Expr *IDC = TC->getImmediatelyDeclaredConstraint();
+TRY_TO(TraverseStmt(IDC));
+// Avoid traversing the ConceptReference in the TypeCosntraint
+// if we have an immediately-declared-constraint, otherwise
+// we'll end up visiting the concept and the arguments in
+// the TC twice.
+if (!IDC)
+  TRY_TO(TraverseConceptReference(*TC));
+  }
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
 TRY_TO(TraverseTypeLoc(D->getDefaultArgumentInfo()->getTypeLoc()));
 })
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -433,6 +433,28 @@
   )cpp";
   EXPECT_DECLS("ConceptSpecializationExpr",
{"template  concept Fooable = true;"});
+
+  // constrained-parameter
+  Code = R"cpp(
+template 
+concept Fooable = true;
+
+template <[[Fooable]] T>
+void bar(T t);
+  )cpp";
+  EXPECT_DECLS("ConceptSpecializationExpr",
+   {"template  concept Fooable = true;"});
+
+  // partial-concept-id
+  Code = R"cpp(
+template 
+concept Fooable = true;
+
+template <[[Fooable]] T>
+void bar(T t);
+  )cpp";
+  EXPECT_DECLS("ConceptSpecializationExpr",
+   {"template  concept Fooable = true;"});
 }
 
 TEST_F(TargetDeclTest, FunctionTemplate) {
___
cfe-commits mailing list

[clang] 9860e68 - Don't leave the FPOptions in a UnaryOperator uninitialized.

2020-08-16 Thread Richard Smith via cfe-commits


Author: Richard Smith
Date: 2020-08-16T15:16:12-07:00
New Revision: 9860e68450cd04ec2c53fe2dbcfab64a86c76673

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

LOG: Don't leave the FPOptions in a UnaryOperator uninitialized.

We don't appear to use these FPOptions for anything right now, but
they shouldn't be uninitialized because that makes our AST file output
nondeterministic.

Added: 
clang/test/PCH/determinism.cpp

Modified: 
clang/lib/AST/Expr.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 29f96674e174..f5aecfb7fdae 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4569,6 +4569,8 @@ UnaryOperator::UnaryOperator(const ASTContext , Expr 
*input, Opcode opc,
   UnaryOperatorBits.CanOverflow = CanOverflow;
   UnaryOperatorBits.Loc = l;
   UnaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
+  if (hasStoredFPFeatures())
+setStoredFPFeatures(FPFeatures);
   setDependence(computeDependence(this));
 }
 

diff  --git a/clang/test/PCH/determinism.cpp b/clang/test/PCH/determinism.cpp
new file mode 100644
index ..540088e2c91b
--- /dev/null
+++ b/clang/test/PCH/determinism.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: %clang_cc1 -x c++-header %s -emit-pch -o %t/a.pch
+// RUN: %clang_cc1 -x c++-header %s -emit-pch -o %t/b.pch
+// RUN: cmp %t/a.pch %t/b.pch
+
+#pragma float_control(push)
+double fp_control_1(double x) {
+#pragma float_control(precise, on)
+  return -x + x;
+}
+
+double fp_control_2(double x) {
+#pragma float_control(precise, off)
+  return -x + x;
+}
+#pragma float_control(pop)



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

[PATCH] D83536: [clangd] Index refs to main-file symbols as well

2020-08-16 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 285907.
nridge marked 4 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83536

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -624,7 +624,6 @@
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
   HaveRanges(Main.ranges("macro");
-  // Symbols *only* in the main file:
   // - (a, b) externally visible and should have refs.
   // - (c, FUNC) externally invisible and had no refs collected.
   auto MainSymbols =
@@ -633,6 +632,17 @@
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _)));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _;
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _;
+
+  // Run the collector again with CollectMainFileRefs = true.
+  InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem();
+  CollectorOpts.CollectMainFileRefs = true;
+  runSymbolCollector(Header.code(),
+ (Main.code() + SymbolsOnlyInMainCode.code()).str());
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "a").ID, _)));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "b").ID, _)));
+  EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "c").ID, _)));
+  // However, references to main-file macros are not collected.
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "FUNC").ID, _;
 }
 
 TEST_F(SymbolCollectorTest, MacroRefInHeader) {
@@ -818,8 +828,9 @@
 $Foo[[Foo]] fo;
   }
   )");
-  // The main file is normal .cpp file, we should collect the refs
-  // for externally visible symbols.
+  // We should collect refs to main-file symbols in all cases:
+
+  // 1. The main file is normal .cpp file.
   TestFileName = testPath("foo.cpp");
   runSymbolCollector("", Header.code());
   EXPECT_THAT(Refs,
@@ -828,7 +839,7 @@
Pair(findSymbol(Symbols, "Func").ID,
 HaveRanges(Header.ranges("Func");
 
-  // Run the .h file as main file, we should collect the refs.
+  // 2. Run the .h file as main file.
   TestFileName = testPath("foo.h");
   runSymbolCollector("", Header.code(),
  /*ExtraArgs=*/{"-xobjective-c++-header"});
@@ -839,8 +850,7 @@
Pair(findSymbol(Symbols, "Func").ID,
 HaveRanges(Header.ranges("Func");
 
-  // Run the .hh file as main file (without "-x c++-header"), we should collect
-  // the refs as well.
+  // 3. Run the .hh file as main file (without "-x c++-header").
   TestFileName = testPath("foo.hh");
   runSymbolCollector("", Header.code());
   EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -228,6 +228,63 @@
FileURI("unittest:///root/B.cc")}));
 }
 
+TEST_F(BackgroundIndexTest, MainFileRefs) {
+  MockFS FS;
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  void header_sym();
+  )cpp";
+  FS.Files[testPath("root/A.cc")] =
+  "#include \"A.h\"\nstatic void main_sym() { (void)header_sym; }";
+
+  // Check the behaviour with CollectMainFileRefs = false (the default).
+  {
+llvm::StringMap Storage;
+size_t CacheHits = 0;
+MemoryShardStorage MSS(Storage, CacheHits);
+OverlayCDB CDB(/*Base=*/nullptr);
+BackgroundIndex Idx(Context::empty(), FS, CDB,
+[&](llvm::StringRef) { return  });
+
+tooling::CompileCommand Cmd;
+Cmd.Filename = testPath("root/A.cc");
+Cmd.Directory = testPath("root");
+Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+ASSERT_TRUE(Idx.blockUntilIdleForTest());
+EXPECT_THAT(
+

[clang] ae7f088 - [InstCombine] Aggregate reconstruction simplification (PR47060)

2020-08-16 Thread Roman Lebedev via cfe-commits


Author: Roman Lebedev
Date: 2020-08-16T23:27:56+03:00
New Revision: ae7f08812e0995481eb345cecc5dd4529829ba44

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

LOG: [InstCombine] Aggregate reconstruction simplification (PR47060)

This pattern happens in clang C++ exception lowering code, on unwind branch.
We end up having a `landingpad` block after each `invoke`, where RAII
cleanup is performed, and the elements of an aggregate `{i8*, i32}`
holding exception info are `extractvalue`'d, and we then branch to common block
that takes extracted `i8*` and `i32` elements (via `phi` nodes),
form a new aggregate, and finally `resume`'s the exception.

The problem is that, if the cleanup block is effectively empty,
it shouldn't be there, there shouldn't be that `landingpad` and `resume`,
said `invoke` should be a  `call`.

Indeed, we do that simplification in e.g. SimplifyCFG 
`SimplifyCFGOpt::simplifyResume()`.
But the thing is, all this extra `extractvalue` + `phi` + `insertvalue` cruft,
while it is pointless, does not look like "empty cleanup block".
So the `SimplifyCFGOpt::simplifyResume()` fails, and the exception is has
higher cost than it could have on unwind branch :S

This doesn't happen *that* often, but it will basically happen once per C++
function with complex CFG that called more than one other function
that isn't known to be `nounwind`.

I think, this is a missing fold in InstCombine, so i've implemented it.

I think, the algorithm/implementation is rather self-explanatory:
1. Find a chain of `insertvalue`'s that fully tell us the initializer of the 
aggregate.
2. For each element, try to find from which aggregate it was extracted.
   If it was extracted from the aggregate with identical type,
   from identical element index, great.
3. If all elements were found to have been extracted from the same aggregate,
   then we can just use said original source aggregate directly,
   instead of re-creating it.
4. If we fail to find said aggregate when looking only in the current block,
   we need be PHI-aware - we might have different source aggregate when coming
   from each predecessor.

I'm not sure if this already handles everything, and there are some FIXME's,
i'll deal with all that later in followups.

I'd be fine with going with post-commit review here code-wise,
but just in case there are thoughts, i'm posting this.

On RawSpeed, for example, this has the following effect:
```
| statistic name| baseline | proposed | 
Δ |   % | abs(%) |
|---|-:|-:|--:|:|---:|
| instcombine.NumAggregateReconstructionsSimplified |0 | 1253 |  
1253 |   0.00% |  0.00% |
| simplifycfg.NumInvokes|  948 | 1355 |   
407 |  42.93% | 42.93% |
| instcount.NumInsertValueInst  | 4382 | 3210 | 
-1172 | -26.75% | 26.75% |
| simplifycfg.NumSinkCommonCode |  574 |  458 |  
-116 | -20.21% | 20.21% |
| simplifycfg.NumSinkCommonInstrs   | 1154 |  921 |  
-233 | -20.19% | 20.19% |
| instcount.NumExtractValueInst |29017 |26397 | 
-2620 |  -9.03% |  9.03% |
| instcombine.NumDeadInst   |   166618 |   174705 |  
8087 |   4.85% |  4.85% |
| instcount.NumPHIInst  |51526 |50678 |  
-848 |  -1.65% |  1.65% |
| instcount.NumLandingPadInst   |20865 |20609 |  
-256 |  -1.23% |  1.23% |
| instcount.NumInvokeInst   |34023 |33675 |  
-348 |  -1.02% |  1.02% |
| simplifycfg.NumSimpl  |   113634 |   114708 |  
1074 |   0.95% |  0.95% |
| instcombine.NumSunkInst   |15030 |14930 |  
-100 |  -0.67% |  0.67% |
| instcount.TotalBlocks |   219544 |   219024 |  
-520 |  -0.24% |  0.24% |
| instcombine.NumCombined   |   644562 |   645805 |  
1243 |   0.19% |  0.19% |
| instcount.TotalInsts  |  2139506 |  2135377 | 
-4129 |  -0.19% |  0.19% |
| instcount.NumBrInst   |   156988 |   156821 |  
-167 |  -0.11% |  0.11% |
| instcount.NumCallInst |  1206144 |  1207076 |   
932 |   0.08% |  0.08% |
| instcount.NumResumeInst   | 5193 | 5190 |
-3 |  -0.06% |  0.06% |
| asm-printer.EmittedInsts  |   948580 |   948299 |  
-281 |  -0.03% |  0.03% |
| instcount.TotalFuncs  |11509 |11507 |
-2 |  -0.02% |  0.02% |
| inline.NumDeleted |97595 |97597 | 
2 |   0.00% |  0.00% |
| inline.NumInlined

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

2020-08-16 Thread Roman Lebedev 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 rGae7f08812e09: [InstCombine] Aggregate reconstruction 
simplification (PR47060) (authored by lebedev.ri).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85787

Files:
  clang/test/CodeGenCXX/nrvo.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
  llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll

Index: llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
===
--- llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
+++ llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
@@ -17,21 +17,14 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:   left:
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
-; CHECK-NEXT:[[I2:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT]], 1
 ; CHECK-NEXT:call void @foo()
 ; CHECK-NEXT:br label [[END:%.*]]
 ; CHECK:   right:
-; CHECK-NEXT:[[I3:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 0
-; CHECK-NEXT:[[I4:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT]], 1
 ; CHECK-NEXT:call void @bar()
 ; CHECK-NEXT:br label [[END]]
 ; CHECK:   end:
-; CHECK-NEXT:[[I5:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I3]], [[RIGHT]] ]
-; CHECK-NEXT:[[I6:%.*]] = phi i32 [ [[I2]], [[LEFT]] ], [ [[I4]], [[RIGHT]] ]
+; CHECK-NEXT:[[I8:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:call void @baz()
-; CHECK-NEXT:[[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:[[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
 ; CHECK-NEXT:ret { i32, i32 } [[I8]]
 ;
 entry:
@@ -278,24 +271,17 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br i1 [[C0:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:   left:
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
-; CHECK-NEXT:[[I2:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT]], 1
 ; CHECK-NEXT:call void @foo()
 ; CHECK-NEXT:br label [[MIDDLE:%.*]]
 ; CHECK:   right:
-; CHECK-NEXT:[[I3:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 0
-; CHECK-NEXT:[[I4:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT]], 1
 ; CHECK-NEXT:call void @bar()
 ; CHECK-NEXT:br label [[MIDDLE]]
 ; CHECK:   middle:
-; CHECK-NEXT:[[I5:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I3]], [[RIGHT]] ], [ [[I5]], [[MIDDLE]] ]
-; CHECK-NEXT:[[I6:%.*]] = phi i32 [ [[I2]], [[LEFT]] ], [ [[I4]], [[RIGHT]] ], [ [[I6]], [[MIDDLE]] ]
+; CHECK-NEXT:[[I8:%.*]] = phi { i32, i32 } [ [[I8]], [[MIDDLE]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:call void @baz()
 ; CHECK-NEXT:[[C1:%.*]] = call i1 @geni1()
 ; CHECK-NEXT:br i1 [[C1]], label [[END:%.*]], label [[MIDDLE]]
 ; CHECK:   end:
-; CHECK-NEXT:[[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:[[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
 ; CHECK-NEXT:ret { i32, i32 } [[I8]]
 ;
 entry:
Index: llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
===
--- llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
+++ llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
@@ -13,11 +13,7 @@
 ; We should just return the source aggregate.
 define { i32, i32 } @test0({ i32, i32 } %srcagg) {
 ; CHECK-LABEL: @test0(
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[SRCAGG:%.*]], 0
-; CHECK-NEXT:[[I1:%.*]] = extractvalue { i32, i32 } [[SRCAGG]], 1
-; CHECK-NEXT:[[I2:%.*]] = insertvalue { i32, i32 } undef, i32 [[I0]], 0
-; CHECK-NEXT:[[I3:%.*]] = insertvalue { i32, i32 } [[I2]], i32 [[I1]], 1
-; CHECK-NEXT:ret { i32, i32 } [[I3]]
+; CHECK-NEXT:ret { i32, i32 } [[SRCAGG:%.*]]
 ;
   %i0 = extractvalue { i32, i32 } %srcagg, 0
   %i1 = extractvalue { i32, i32 } %srcagg, 1
@@ -29,11 +25,7 @@
 ; Arrays are still aggregates
 define [2 x i32] @test1([2 x i32] %srcagg) {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:[[I0:%.*]] = extractvalue [2 x i32] [[SRCAGG:%.*]], 0
-; CHECK-NEXT:[[I1:%.*]] = extractvalue [2 x i32] [[SRCAGG]], 1
-; CHECK-NEXT:[[I2:%.*]] = insertvalue [2 x i32] undef, i32 [[I0]], 0
-; CHECK-NEXT:[[I3:%.*]] = insertvalue [2 x i32] [[I2]], i32 [[I1]], 1
-; CHECK-NEXT:ret [2 x i32] [[I3]]
+; CHECK-NEXT:ret [2 x i32] [[SRCAGG:%.*]]
 ;
   %i0 = extractvalue [2 x i32]

[PATCH] D85934: Enable OpenMP offloading to VE and enable tests for offloading to VE

2020-08-16 Thread Joachim Protze via Phabricator via cfe-commits

protze.joachim added inline comments.



Comment at: openmp/libomptarget/test/lit.cfg:67
 append_dynamic_library_path('DYLD_LIBRARY_PATH', \
 config.omp_host_rtl_directory, ";")
 config.test_flags += " -Wl,-rpath," + config.library_dir

Unrelated to this patch, but why are different separators used for Darwin?



Comment at: openmp/libomptarget/test/lit.cfg:78-83
+if 've-unknown-linux-unknown' in config.libomptarget_system_targets and \
+config.omp_host_rtl_directory:
+config.test_flags = config.test_flags + " -Xopenmp-target \"-L" +\
+config.omp_host_rtl_directory + "\""
+append_dynamic_library_path('VE_LD_LIBRARY_PATH',
+config.omp_host_rtl_directory, ":")

I think, this should only be set for the specific device RUN-lines. I suggest 
to include the flags and export of the environment variable in the definition 
of `%libomptarget-compile-ve-unknown-linux-unknown`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85934

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

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

2020-08-16 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 285898.
lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.

Patch updated - addressed all nits other than the question about decoupling 
`enum` values from variable names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85787

Files:
  clang/test/CodeGenCXX/nrvo.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
  llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll

Index: llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
===
--- llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
+++ llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
@@ -17,21 +17,14 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:   left:
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
-; CHECK-NEXT:[[I2:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT]], 1
 ; CHECK-NEXT:call void @foo()
 ; CHECK-NEXT:br label [[END:%.*]]
 ; CHECK:   right:
-; CHECK-NEXT:[[I3:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 0
-; CHECK-NEXT:[[I4:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT]], 1
 ; CHECK-NEXT:call void @bar()
 ; CHECK-NEXT:br label [[END]]
 ; CHECK:   end:
-; CHECK-NEXT:[[I5:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I3]], [[RIGHT]] ]
-; CHECK-NEXT:[[I6:%.*]] = phi i32 [ [[I2]], [[LEFT]] ], [ [[I4]], [[RIGHT]] ]
+; CHECK-NEXT:[[I8:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:call void @baz()
-; CHECK-NEXT:[[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:[[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
 ; CHECK-NEXT:ret { i32, i32 } [[I8]]
 ;
 entry:
@@ -278,24 +271,17 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br i1 [[C0:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:   left:
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
-; CHECK-NEXT:[[I2:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT]], 1
 ; CHECK-NEXT:call void @foo()
 ; CHECK-NEXT:br label [[MIDDLE:%.*]]
 ; CHECK:   right:
-; CHECK-NEXT:[[I3:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 0
-; CHECK-NEXT:[[I4:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT]], 1
 ; CHECK-NEXT:call void @bar()
 ; CHECK-NEXT:br label [[MIDDLE]]
 ; CHECK:   middle:
-; CHECK-NEXT:[[I5:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I3]], [[RIGHT]] ], [ [[I5]], [[MIDDLE]] ]
-; CHECK-NEXT:[[I6:%.*]] = phi i32 [ [[I2]], [[LEFT]] ], [ [[I4]], [[RIGHT]] ], [ [[I6]], [[MIDDLE]] ]
+; CHECK-NEXT:[[I8:%.*]] = phi { i32, i32 } [ [[I8]], [[MIDDLE]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:call void @baz()
 ; CHECK-NEXT:[[C1:%.*]] = call i1 @geni1()
 ; CHECK-NEXT:br i1 [[C1]], label [[END:%.*]], label [[MIDDLE]]
 ; CHECK:   end:
-; CHECK-NEXT:[[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:[[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
 ; CHECK-NEXT:ret { i32, i32 } [[I8]]
 ;
 entry:
Index: llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
===
--- llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
+++ llvm/test/Transforms/InstCombine/aggregate-reconstruction.ll
@@ -13,11 +13,7 @@
 ; We should just return the source aggregate.
 define { i32, i32 } @test0({ i32, i32 } %srcagg) {
 ; CHECK-LABEL: @test0(
-; CHECK-NEXT:[[I0:%.*]] = extractvalue { i32, i32 } [[SRCAGG:%.*]], 0
-; CHECK-NEXT:[[I1:%.*]] = extractvalue { i32, i32 } [[SRCAGG]], 1
-; CHECK-NEXT:[[I2:%.*]] = insertvalue { i32, i32 } undef, i32 [[I0]], 0
-; CHECK-NEXT:[[I3:%.*]] = insertvalue { i32, i32 } [[I2]], i32 [[I1]], 1
-; CHECK-NEXT:ret { i32, i32 } [[I3]]
+; CHECK-NEXT:ret { i32, i32 } [[SRCAGG:%.*]]
 ;
   %i0 = extractvalue { i32, i32 } %srcagg, 0
   %i1 = extractvalue { i32, i32 } %srcagg, 1
@@ -29,11 +25,7 @@
 ; Arrays are still aggregates
 define [2 x i32] @test1([2 x i32] %srcagg) {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:[[I0:%.*]] = extractvalue [2 x i32] [[SRCAGG:%.*]], 0
-; CHECK-NEXT:[[I1:%.*]] = extractvalue [2 x i32] [[SRCAGG]], 1
-; CHECK-NEXT:[[I2:%.*]] = insertvalue [2 x i32] undef, i32 [[I0]], 0
-; CHECK-NEXT:[[I3:%.*]] = insertvalue [2 x i32] [[I2]], i32 [[I1]], 1
-; CHECK-NEXT:ret [2 x i32] [[I3]]
+; CHECK-NEXT:ret [2 x i32] [[SRCAGG:%.*]]
 ;
   %i0 = extractvalue [2 x i32] %srcagg, 0
   %i1 =

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

2020-08-16 Thread Johannes Doerfert 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 rG95a25e4c3203: [OpenMP][FIX] Do not use TBAA in type punning 
reduction GPU code PR46156 (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D86037?vs=285888=285895#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86037

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp

Index: clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx-unknown-unknown -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -fexceptions -fcxx-exceptions -aux-triple powerpc64le-unknown-unknown -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#include 
+
+// Verify we do not add tbaa metadata to type punned memory operations:
+
+// CHECK:  call i64 @__kmpc_shuffle_int64(
+// CHECK-NEXT: store i64 %{{.*}}, i64* %{{.*}}, align {{[0-9]+$}}
+
+// CHECK:  call i64 @__kmpc_shuffle_int64(
+// CHECK-NEXT: store i64 %{{.*}}, i64* %{{.*}}, align {{[0-9]+$}}
+
+template 
+void complex_reduction() {
+#pragma omp target teams distribute
+  for (int ib = 0; ib < 100; ib++) {
+std::complex partial_sum;
+const int istart = ib * 4;
+const int iend = (ib + 1) * 4;
+#pragma omp parallel for reduction(+ \
+   : partial_sum)
+for (int i = istart; i < iend; i++)
+  partial_sum += std::complex(i, i);
+  }
+}
+
+void test() {
+  complex_reduction();
+  complex_reduction();
+}
+#endif
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -2845,8 +2845,12 @@
   Address CastItem = CGF.CreateMemTemp(CastTy);
   Address ValCastItem = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   CastItem, Val->getType()->getPointerTo(CastItem.getAddressSpace()));
-  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy);
-  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc);
+  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy,
+LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
+  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc,
+  LValueBaseInfo(AlignmentSource::Type),
+  TBAAAccessInfo());
 }
 
 /// This function creates calls to one of two shuffle functions to copy
@@ -2933,9 +2937,14 @@
ThenBB, ExitBB);
   CGF.EmitBlock(ThenBB);
   llvm::Value *Res = createRuntimeShuffleFunction(
-  CGF, CGF.EmitLoadOfScalar(Ptr, /*Volatile=*/false, IntType, Loc),
+  CGF,

[clang] 95a25e4 - [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

2020-08-16 Thread Johannes Doerfert via cfe-commits


Author: Johannes Doerfert
Date: 2020-08-16T14:38:31-05:00
New Revision: 95a25e4c3203f35e9f57f9fac620b4a21bffd6e1

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

LOG: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

When we implement OpenMP GPU reductions we use type punning a lot during
the shuffle and reduce operations. This is not always compatible with
language rules on aliasing. So far we generated TBAA which later allowed
to remove some of the reduce code as accesses and initialization were
"known to not alias". With this patch we avoid TBAA in this step,
hopefully for all accesses that we need to.

Verified on the reproducer of PR46156 and QMCPack.

Reviewed By: ABataev

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

Added: 
clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp

Modified: 
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index 1b2608e9854a..d9ef6c2a1078 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -2845,8 +2845,12 @@ static llvm::Value *castValueToType(CodeGenFunction 
, llvm::Value *Val,
   Address CastItem = CGF.CreateMemTemp(CastTy);
   Address ValCastItem = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   CastItem, Val->getType()->getPointerTo(CastItem.getAddressSpace()));
-  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy);
-  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc);
+  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy,
+LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
+  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc,
+  LValueBaseInfo(AlignmentSource::Type),
+  TBAAAccessInfo());
 }
 
 /// This function creates calls to one of two shuffle functions to copy
@@ -2933,9 +2937,14 @@ static void shuffleAndStore(CodeGenFunction , 
Address SrcAddr,
ThenBB, ExitBB);
   CGF.EmitBlock(ThenBB);
   llvm::Value *Res = createRuntimeShuffleFunction(
-  CGF, CGF.EmitLoadOfScalar(Ptr, /*Volatile=*/false, IntType, Loc),
+  CGF,
+  CGF.EmitLoadOfScalar(Ptr, /*Volatile=*/false, IntType, Loc,
+   LValueBaseInfo(AlignmentSource::Type),
+   TBAAAccessInfo()),
   IntType, Offset, Loc);
-  CGF.EmitStoreOfScalar(Res, ElemPtr, /*Volatile=*/false, IntType);
+  CGF.EmitStoreOfScalar(Res, ElemPtr, /*Volatile=*/false, IntType,
+LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
   Address LocalPtr = Bld.CreateConstGEP(Ptr, 1);
   Address LocalElemPtr = Bld.CreateConstGEP(ElemPtr, 1);
   PhiSrc->addIncoming(LocalPtr.getPointer(), ThenBB);
@@ -2944,9 +2953,14 @@ static void shuffleAndStore(CodeGenFunction , 
Address SrcAddr,
   CGF.EmitBlock(ExitBB);
 } else {
   llvm::Value *Res = createRuntimeShuffleFunction(
-  CGF, CGF.EmitLoadOfScalar(Ptr, /*Volatile=*/false, IntType, Loc),
+  CGF,
+  CGF.EmitLoadOfScalar(Ptr, /*Volatile=*/false, IntType, Loc,
+   LValueBaseInfo(AlignmentSource::Type),
+   TBAAAccessInfo()),
   IntType, Offset, Loc);
-  CGF.EmitStoreOfScalar(Res, ElemPtr, /*Volatile=*/false, IntType);
+  CGF.EmitStoreOfScalar(Res, ElemPtr, /*Volatile=*/false, IntType,
+LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
   Ptr = Bld.CreateConstGEP(Ptr, 1);
   ElemPtr = Bld.CreateConstGEP(ElemPtr, 1);
 }
@@ -3100,12 +3114,14 @@ static void emitReductionListCopy(
 } else {
   switch (CGF.getEvaluationKind(Private->getType())) {
   case TEK_Scalar: {
-llvm::Value *Elem =
-CGF.EmitLoadOfScalar(SrcElementAddr, /*Volatile=*/false,
- Private->getType(), Private->getExprLoc());
+llvm::Value *Elem = CGF.EmitLoadOfScalar(
+SrcElementAddr, /*Volatile=*/false, Private->getType(),
+Private->getExprLoc(), LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
 // Store the source element value to the dest element address.
-CGF.EmitStoreOfScalar(Elem, DestElementAddr, /*Volatile=*/false,
-  Private->getType());
+CGF.EmitStoreOfScalar(
+Elem, DestElementAddr, /*Volatile=*/false, Private->getType(),
+

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

2020-08-16 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

@spatel thank you for taking a look!

In D85787#2220343 , @spatel wrote:

> This seems similar in spirit to the recursive element tracking that we do in 
> collectShuffleElements() in this file or foldIdentityShuffles() in 
> InstSimplify, but a bit more complicated (because of the phi translation?).

I actually haven't seen those, but i would imagine that yes, having to deal 
with PHI's makes things slightly more complicated.

> I've pointed out a few minor potential improvements, otherwise LGTM.

Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:730
+
+  static constexpr auto NotFound = None;
+

spatel wrote:
> IIUC, you're intentionally giving the placeholder objects the same names as 
> the enum values, but that confused me. Would it make sense to call this 
> "InvalidValue" or similar?
> IIUC, you're intentionally giving the placeholder objects the same names as 
> the enum values, but that confused me.

Yes. I first added `enum`, and then thought that it would be better to not
spell `None; // SourceAggegate::NotFound` / `nullptr; // 
SourceAggegate::FoundMismatch`
but literally just give them a name and spell them as close to the `enum`
values as possible, for obvious reasons of avoiding duplication/errors.

> Would it make sense to call this "InvalidValue" or similar?

Do we want to change both the `enum` values and the variable names,
or just variables to be different from the `enum` values?

`NotFound` seems like a good fit to me, because with it we literally mean
that we can't tell what scalar value that element of an aggregate has.

`InvalidValue` doesn't really have any connotation to me,
but it does look kinda sorta similar to `UndefValue`, which i want to avoid.

So i'm not sure here.

Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:855
+  // Okay, what have we found? Does that correlate with previous findings?
+  switch (Describe(SourceAggregateForElement)) {
+  case SourceAggegate::NotFound:

spatel wrote:
> Instead of switch, could we reduce to:
>   if (Describe(SourceAggForElt) != SourceAggregate::Found)
> return SourceAggForElt;
Hm, i guess, thanks!
But we still need the inner `switch`.

Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:907
+  // Don't bother if there are more than 64 predecessors.
+  if (UseBB->hasNPredecessorsOrMore(64 + 1))
+return nullptr;

spatel wrote:
> Best to give that "64" a name in case we want to experiment with other 
> settings.
Do you mean a `cl::opt`, or just a local `static const` variable?
I'll do the latter for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85787

___
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

2020-08-16 Thread Mark de Wever via Phabricator via cfe-commits

Mordante marked 14 inline comments as done.
Mordante added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > Hmm, I'm on the fence about specifying `201803` for these attributes. 
> > > Given that this is only the start of supporting the attribute, do we want 
> > > to claim it already matches the standard's behavior? Or do we just want 
> > > to return `1` to signify that we understand this attribute but we don't 
> > > yet fully support it in common cases (such as on labels in switch 
> > > statements, etc)?
> > > 
> > > As another question, should we consider adding a C2x spelling 
> > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality 
> > > to C?
> > I was also somewhat on the fence, for now I'll change it to specify `1`.
> > 
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> 
> There isn't one currently because there is no implementation experience with 
> the attributes in C. This is a way to get that implementation experience so 
> it's easier to propose the feature to WG14.
> I was also somewhat on the fence, for now I'll change it to specify `1`.

There seems to be an issue using the `1` so I used `2` instead. (Didn't want to 
look closely at the issue.)




Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Hmm, I'm on the fence about specifying `201803` for these attributes. 
> > > > Given that this is only the start of supporting the attribute, do we 
> > > > want to claim it already matches the standard's behavior? Or do we just 
> > > > want to return `1` to signify that we understand this attribute but we 
> > > > don't yet fully support it in common cases (such as on labels in switch 
> > > > statements, etc)?
> > > > 
> > > > As another question, should we consider adding a C2x spelling 
> > > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality 
> > > > to C?
> > > I was also somewhat on the fence, for now I'll change it to specify `1`.
> > > 
> > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > there's a proposal to add this to C2x?
> > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > there's a proposal to add this to C2x?
> > 
> > There isn't one currently because there is no implementation experience 
> > with the attributes in C. This is a way to get that implementation 
> > experience so it's easier to propose the feature to WG14.
> > I was also somewhat on the fence, for now I'll change it to specify `1`.
> 
> There seems to be an issue using the `1` so I used `2` instead. (Didn't want 
> to look closely at the issue.)
> 
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> 
> There isn't one currently because there is no implementation experience with 
> the attributes in C. This is a way to get that implementation experience so 
> it's easier to propose the feature to WG14.

Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I 
didn't implement it yet. I intend to look at it later. I first want the initial 
part done to see whether this is the right approach.



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It's not allowed to annotate a statement with both ``likely`` and
+``unlikely``.  It's not recommended to annotate both branches of an ``if``
+statement with an attribute.

aaron.ballman wrote:
> Mordante wrote:
> > Quuxplusone wrote:
> > > aaron.ballman wrote:
> > > > Why? I would expect this to be reasonable code:
> > > > ```
> > > > if (foo) [[likely]] {
> > > >   ...
> > > > } else if (bar) [[unlikely]] {
> > > >   ...
> > > > } else if (baz) [[likely]] {
> > > >   ...
> > > > } else {
> > > >   ...
> > > > }
> > > > ```
> > > > Especially because I would expect this to be reasonable code:
> > > > ```
> > > > switch (value) {
> > > > [[likely]] case 0: ... break;
> > > > [[unlikely]] case 1: ... break;
> > > > [[likely]] case 2: ... break;
> > > > [[unlikely]] default: ... break;
> > > > }
> > > > ```
> > > > As motivating examples, consider a code generator that knows whether a 
> > > > particular branch is likely or not and it writes out the attribute on 
> > > > all branches. Or, something like this:
> > > > ```
> > > > float rnd_value =

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

2020-08-16 Thread Jon Chesterfield via Phabricator via cfe-commits

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

Seems OK to me as a local fix. I'm not sure about encoding 'missing condition' 
as 0, but that's a preexisting choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85875

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

[PATCH] D85878: [OpenMP] Context selector extensions for return value overloading

2020-08-16 Thread Jon Chesterfield via Phabricator via cfe-commits

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

Link error seems reasonable to me. Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85878

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

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

2020-08-16 Thread Sanjay Patel via Phabricator via cfe-commits

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

This seems similar in spirit to the recursive element tracking that we do in 
collectShuffleElements() in this file or foldIdentityShuffles() in 
InstSimplify, but a bit more complicated (because of the phi translation?).
I've pointed out a few minor potential improvements, otherwise LGTM.




Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:730
+
+  static constexpr auto NotFound = None;
+

IIUC, you're intentionally giving the placeholder objects the same names as the 
enum values, but that confused me. Would it make sense to call this 
"InvalidValue" or similar?



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:778
+
+  static constexpr auto FoundMismatch = nullptr;
+

Similar comment about the name as above for NotFound (and might be better to 
declare it on the next line after above?).



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:780
+
+  enum class SourceAggegate {
+/// When analyzing the value that was inserted into an aggregate, we did

Spelling - "SourceAggregate"



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:855
+  // Okay, what have we found? Does that correlate with previous findings?
+  switch (Describe(SourceAggregateForElement)) {
+  case SourceAggegate::NotFound:

Instead of switch, could we reduce to:
  if (Describe(SourceAggForElt) != SourceAggregate::Found)
return SourceAggForElt;



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:907
+  // Don't bother if there are more than 64 predecessors.
+  if (UseBB->hasNPredecessorsOrMore(64 + 1))
+return nullptr;

Best to give that "64" a name in case we want to experiment with other settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85787

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

[PATCH] D85321: [OPENMP]Do not capture base pointer by reference if it is used as a base for array-like reduction.

2020-08-16 Thread Johannes Doerfert via Phabricator via cfe-commits

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

While we should be able to optimize this later in the pipeline soon, I guess we 
can do it early for now. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85321

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

[clang] fef2607 - [Sema] Use the proper cast for a fixed bool enum.

2020-08-16 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2020-08-16T18:40:08+02:00
New Revision: fef26071240711e8f7305715b5f22cfc7ad04bfe

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

LOG: [Sema] Use the proper cast for a fixed bool enum.

When casting an enumerate with a fixed bool type the casting should use
an IntegralToBoolean instead of an IntegralCast as is required per Core
Issue 2338.

Fixes PR47055: Incorrect codegen for enum with bool underlying type

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

Added: 
clang/test/CodeGen/enum-bool.cpp

Modified: 
clang/lib/Sema/SemaCast.cpp
clang/test/CXX/drs/dr23xx.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 93752766f4a2..726900c59f20 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1243,7 +1243,13 @@ static TryCastResult TryStaticCast(Sema , 
ExprResult ,
   return TC_Failed;
 }
 if (SrcType->isIntegralOrEnumerationType()) {
-  Kind = CK_IntegralCast;
+  // [expr.static.cast]p10 If the enumeration type has a fixed underlying
+  // type, the value is first converted to that type by integral conversion
+  const EnumType *Enum = DestType->getAs();
+  Kind = Enum->getDecl()->isFixed() &&
+ Enum->getDecl()->getIntegerType()->isBooleanType()
+ ? CK_IntegralToBoolean
+ : CK_IntegralCast;
   return TC_Success;
 } else if (SrcType->isRealFloatingType())   {
   Kind = CK_FloatingToIntegral;

diff  --git a/clang/test/CXX/drs/dr23xx.cpp b/clang/test/CXX/drs/dr23xx.cpp
index c265ebbe359c..a7ff2a5813ed 100644
--- a/clang/test/CXX/drs/dr23xx.cpp
+++ b/clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 
+#if __cplusplus >= 201103L
+namespace dr2338 { // dr2338: 12
+namespace B {
+enum E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace D
+} // namespace dr2338
+#endif
+
 namespace dr2346 { // dr2346: 11
   void test() {
 const int i2 = 0;

diff  --git a/clang/test/CodeGen/enum-bool.cpp 
b/clang/test/CodeGen/enum-bool.cpp
new file mode 100644
index ..220baa3fd8b6
--- /dev/null
+++ b/clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338



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

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

2020-08-16 Thread Mark de Wever via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfef260712407: [Sema] Use the proper cast for a fixed bool 
enum. (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CodeGen/enum-bool.cpp


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 
+#if __cplusplus >= 201103L
+namespace dr2338 { // dr2338: 12
+namespace B {
+enum E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace D
+} // namespace dr2338
+#endif
+
 namespace dr2346 { // dr2346: 11
   void test() {
 const int i2 = 0;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -1243,7 +1243,13 @@
   return TC_Failed;
 }
 if (SrcType->isIntegralOrEnumerationType()) {
-  Kind = CK_IntegralCast;
+  // [expr.static.cast]p10 If the enumeration type has a fixed underlying
+  // type, the value is first converted to that type by integral conversion
+  const EnumType *Enum = DestType->getAs();
+  Kind = Enum->getDecl()->isFixed() &&
+ Enum->getDecl()->getIntegerType()->isBooleanType()
+ ? CK_IntegralToBoolean
+ : CK_IntegralCast;
   return TC_Success;
 } else if (SrcType->isRealFloatingType())   {
   Kind = CK_FloatingToIntegral;


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp

[clang] 827ba67 - [Sema] Validate calls to GetExprRange.

2020-08-16 Thread Mark de Wever via cfe-commits


Author: Mark de Wever
Date: 2020-08-16T18:32:38+02:00
New Revision: 827ba67e383313b05e9b10c8215e501530d6c9e3

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

LOG: [Sema] Validate calls to GetExprRange.

When a conditional expression has a throw expression it called
GetExprRange with a void expression, which caused an assertion failure.

This approach was suggested by Richard Smith.

Fixes PR46484: Clang crash in clang/lib/Sema/SemaChecking.cpp:10028

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/conditional-expr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index bbd856e9262e..4efd62f58d2e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -10368,10 +10368,16 @@ static IntRange GetExprRange(ASTContext , const 
Expr *E, unsigned MaxWidth,
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 

diff  --git a/clang/test/SemaCXX/conditional-expr.cpp 
b/clang/test/SemaCXX/conditional-expr.cpp
index 8d0555ea5068..68e23a1f5727 100644
--- a/clang/test/SemaCXX/conditional-expr.cpp
+++ b/clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@ namespace lifetime_extension {
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484



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

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

2020-08-16 Thread Mark de Wever via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG827ba67e3833: [Sema] Validate calls to GetExprRange. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/conditional-expr.cpp


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,10 +10368,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,10 +10368,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

2020-08-16 Thread Alexey Bataev via Phabricator via cfe-commits

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86037

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

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

2020-08-16 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, ABataev.
Herald added subscribers: guansong, bollu, kosarev, yaxunl.
Herald added a project: clang.
jdoerfert requested review of this revision.
Herald added a subscriber: sstefan1.

When we implement OpenMP GPU reductions we use type punning a lot during
the shuffle and reduce operations. This is not always compatible with
language rules on aliasing. So far we generated TBAA which later allowed
to remove some of the reduce code as accesses and initialization were
"known to not alias". With this patch we avoid TBAA in this step,
hopefully for all accesses that we need to.

Verified on the reproducer of PR46156 and QMCPack.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86037

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp

Index: clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
===
--- /dev/null
+++ clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx-unknown-unknown -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -fexceptions -fcxx-exceptions -aux-triple powerpc64le-unknown-unknown -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#include 
+
+// Verify we do not add tbaa metadata to type punned memory operations:
+
+// CHECK:  call i64 @__kmpc_shuffle_int64(
+// CHECK-NEXT: store i64 %{{.*}}, i64* %{{.*}}, align {{[0-9]+$}}
+
+// CHECK:  call i64 @__kmpc_shuffle_int64(
+// CHECK-NEXT: store i64 %{{.*}}, i64* %{{.*}}, align {{[0-9]+$}}
+
+template
+void complex_reduction()
+{
+  #pragma omp target teams distribute
+  for (int ib = 0; ib < 100; ib++)
+  {
+std::complex partial_sum;
+const int istart = ib * 4;
+const int iend = (ib + 1) * 4;
+#pragma omp parallel for reduction(+: partial_sum)
+for (int i = istart; i < iend; i++)
+  partial_sum += std::complex(i, i);
+  }
+}
+
+void test()
+{
+  complex_reduction();
+  complex_reduction();
+}
+#endif
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -2845,8 +2845,12 @@
   Address CastItem = CGF.CreateMemTemp(CastTy);
   Address ValCastItem = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   CastItem, Val->getType()->getPointerTo(CastItem.getAddressSpace()));
-  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy);
-  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc);
+  CGF.EmitStoreOfScalar(Val, ValCastItem, /*Volatile=*/false, ValTy,
+LValueBaseInfo(AlignmentSource::Type),
+TBAAAccessInfo());
+  return CGF.EmitLoadOfScalar(CastItem, /*Volatile=*/false, CastTy, Loc,
+  LValueBaseInfo(AlignmentSource::Type),
+  TBAAAccessInfo());
 }
 
 /// This function creates calls to one of

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

2020-08-16 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

Mordante wrote:
> aaron.ballman wrote:
> > Hmm, I'm on the fence about specifying `201803` for these attributes. Given 
> > that this is only the start of supporting the attribute, do we want to 
> > claim it already matches the standard's behavior? Or do we just want to 
> > return `1` to signify that we understand this attribute but we don't yet 
> > fully support it in common cases (such as on labels in switch statements, 
> > etc)?
> > 
> > As another question, should we consider adding a C2x spelling 
> > `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality to 
> > C?
> I was also somewhat on the fence, for now I'll change it to specify `1`.
> 
> I'll have a look at the C2x changes. Just curious do you know whether there's 
> a proposal to add this to C2x?
> I'll have a look at the C2x changes. Just curious do you know whether there's 
> a proposal to add this to C2x?

There isn't one currently because there is no implementation experience with 
the attributes in C. This is a way to get that implementation experience so 
it's easier to propose the feature to WG14.



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It's not allowed to annotate a statement with both ``likely`` and
+``unlikely``.  It's not recommended to annotate both branches of an ``if``
+statement with an attribute.

Mordante wrote:
> Quuxplusone wrote:
> > aaron.ballman wrote:
> > > Why? I would expect this to be reasonable code:
> > > ```
> > > if (foo) [[likely]] {
> > >   ...
> > > } else if (bar) [[unlikely]] {
> > >   ...
> > > } else if (baz) [[likely]] {
> > >   ...
> > > } else {
> > >   ...
> > > }
> > > ```
> > > Especially because I would expect this to be reasonable code:
> > > ```
> > > switch (value) {
> > > [[likely]] case 0: ... break;
> > > [[unlikely]] case 1: ... break;
> > > [[likely]] case 2: ... break;
> > > [[unlikely]] default: ... break;
> > > }
> > > ```
> > > As motivating examples, consider a code generator that knows whether a 
> > > particular branch is likely or not and it writes out the attribute on all 
> > > branches. Or, something like this:
> > > ```
> > > float rnd_value = get_super_random_number_between_zero_and_one();
> > > if (rnd_value < .1) [[unlikely]] {
> > > } else if (rnd_value > .9) [[unlikely]] {
> > > } else [[likely]] {
> > > }
> > > ```
> > Right, annotating both/multiple branches of a control statement should be 
> > perfectly fine. Even `if (x) [[likely]] { } else [[likely]] { }` should be 
> > perfectly okay as far as the code generator is concerned (and we should 
> > have a test for that); it's silly, but there's no reason to warn against it 
> > in the compiler docs.
> > 
> > Aaron, notice that `if (x) [[likely]] { } else if (y) [[likely]] { }` is 
> > not actually annotating "both" branches of any single `if`-statement. There 
> > you're annotating the true branch of an if (without annotating the else 
> > branch), and then annotating the true branch of another if (which doesn't 
> > have an else branch).
> > 
> > Mordante, in these docs, please document the "interesting" behavior of the 
> > standard attribute on labels — annotating a label is different from 
> > annotating the labeled statement itself. In particular,
> > 
> > [[likely]] case 1: case 2: foo(); break;
> > case 3: [[likely]] case 4: foo(); break;
> > case 5: case 6: [[likely]] foo(); break;
> > 
> > indicates that the likely cases are 1, 4, 5, and 6. (1 and 4 because their 
> > labels are annotated; 5 and 6 because their control flow passes through an 
> > annotated statement. Case 3's control flow passes through an annotated 
> > label, but that doesn't matter to the standard attribute.)
> Aaron, the current `CodeGen` code for the `switch` statement allows all 
> branches to have a weight, for the `if` there are only two weights allowed. 
> As Arthur explained the chained `if`s are different statements, so this will 
> work properly.
> 
> Arthur, I agree we should add the documentation about the `switch`, but I'd 
> rather do that when the attributes are implemented for the `switch`. For now 
> I think it would make sense to add some documentation about the fact that the 
> attribute can be placed inside the `CompoundStmt`.
> Aaron, notice that if (x) [[likely]] { } else if (y) [[likely]] { } is not 
> actually annotating "both" branches of any single if-statement. There you're 
> annotating the true branch of an if (without annotating the else branch), and 
> then annotating the true branch of another if (which doesn't have an else 
> branch).

I agree, which is why I pointed it out -- I think users are going to be 
surprised by this behavior because it won't do what they expect it to do.

>  As Arthur

[clang] 44613bb - Create strict aligned code for OpenBSD/arm64.

2020-08-16 Thread Brad Smith via cfe-commits


Author: Brad Smith
Date: 2020-08-16T07:14:34-04:00
New Revision: 44613bbec88be9e86b8c52c4f40bb1b1ab48d84c

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

LOG: Create strict aligned code for OpenBSD/arm64.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/test/Driver/arm-alignment.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 43959f5abe43..ce7c5348a4d5 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -376,9 +376,11 @@ void aarch64::getAArch64TargetFeatures(const Driver ,
   D.Diag(diag::err_drv_invalid_sve_vector_bits);
 
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
-   options::OPT_munaligned_access))
+   options::OPT_munaligned_access)) {
 if (A->getOption().matches(options::OPT_mno_unaligned_access))
   Features.push_back("+strict-align");
+  } else if (Triple.isOSOpenBSD())
+Features.push_back("+strict-align");
 
   if (Args.hasArg(options::OPT_ffixed_x1))
 Features.push_back("+reserve-x1");

diff  --git a/clang/test/Driver/arm-alignment.c 
b/clang/test/Driver/arm-alignment.c
index e0b4946d0a4b..b2bc8a35dfc6 100644
--- a/clang/test/Driver/arm-alignment.c
+++ b/clang/test/Driver/arm-alignment.c
@@ -80,6 +80,9 @@
 // RUN: %clang -target aarch64-none-gnueabi -mkernel -mno-unaligned-access 
-### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-AARCH64 < %t %s
 
+// RUN: %clang -target aarch64-unknown-openbsd -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-AARCH64 < %t %s
+
 // CHECK-ALIGNED-ARM: "-target-feature" "+strict-align"
 // CHECK-ALIGNED-AARCH64: "-target-feature" "+strict-align"
 



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

[PATCH] D86034: [WIP] Attribute harden_misspeculation

2020-08-16 Thread Zola Bridges via Phabricator via cfe-commits

zbrid created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zbrid requested review of this revision.

Starting with if support first

- Parse attribute check for diagnosable issues with its usage


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86034

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/SemaCXX/attr-harden-misspeculation-unsupported-target.cpp
  clang/test/SemaCXX/attr-harden-misspeculation.cpp

Index: clang/test/SemaCXX/attr-harden-misspeculation.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-harden-misspeculation.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
+
+int b(int a, int b) {
+  if (a < b)
+  [[clang::harden_misspeculation]] { // expected-error {{'harden_misspeculation' attribute takes at least 1 argument}}
+a += 2;
+  }
+
+  if (a == b)
+  [[clang::harden_misspeculation(1)]] { // expected-error {{'harden_misspeculation' attribute requires an identifier}}
+a += 1;
+  }
+
+  if (a > b)
+[[clang::harden_misspeculation(a)]] // expected-error {{'harden_misspeculation' attribute is only allowed on compound statements (ie block statements)}}
+return a;
+
+  return a;
+}
Index: clang/test/SemaCXX/attr-harden-misspeculation-unsupported-target.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-harden-misspeculation-unsupported-target.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++11 -triple armv7 -fsyntax-only %s -verify
+
+int b(int a, int b) {
+  if (a < b)
+  [[clang::harden_misspeculation(a)]] { // expected-error {{'harden_misspeculation' attribute is not supported for this target}}
+return a;
+  }
+  return b;
+}
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -10,14 +10,16 @@
 //
 //===--===//
 
-#include "clang/AST/EvaluatedExprVisitor.h"
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/Triple.h"
 
 using namespace clang;
 using namespace sema;
@@ -51,6 +53,38 @@
   return ::new (S.Context) FallThroughAttr(S.Context, A);
 }
 
+static Attr *handleHardenMisspeculationAttr(Sema , Stmt *St,
+const ParsedAttr ,
+SourceRange Range) {
+  if (!isa(St)) {
+S.Diag(A.getRange().getBegin(),
+   diag::err_harden_misspeculation_attr_wrong_target)
+<< A << St->getBeginLoc();
+  }
+
+  // FIXME: Support non-x86_64 targets
+  if (!(S.Context.getTargetInfo().getTriple().getArch() ==
+llvm::Triple::ArchType::x86_64)) {
+S.Diag(A.getLoc(), diag::err_attribute_unsupported) << A;
+return nullptr;
+  }
+
+  if (A.getNumArgs() < 1) {
+S.Diag(A.getLoc(), diag::err_attribute_too_few_arguments) << A << 1;
+return nullptr;
+  }
+
+  for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
+if (!A.isArgIdent(I)) {
+  S.Diag(A.getLoc(), diag::err_attribute_argument_type)
+  << A << AANT_ArgumentIdentifier;
+  return nullptr;
+}
+  }
+
+  return ::new (S.Context) HardenMisspeculationAttr(S.Context, A);
+}
+
 static Attr *handleSuppressAttr(Sema , Stmt *St, const ParsedAttr ,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -374,6 +408,8 @@
 return handleOpenCLUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
 return handleSuppressAttr(S, St, A, Range);
+  case ParsedAttr::AT_HardenMisspeculation:
+return handleHardenMisspeculationAttr(S, St, A, Range);
   case ParsedAttr::AT_NoMerge:
 return handleNoMergeAttr(S, St, A, Range);
   default:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9155,6 +9155,10 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_harden_misspeculation_attr_wrong_target
+: Error<"%0 attribute is only allowed on compound statements (ie block "
+"statements)">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup,

[PATCH] D86033: This is mostly prototype code that should be deleted.

2020-08-16 Thread Zola Bridges via Phabricator via cfe-commits

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

Revert or squash this before uploading for review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86033

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/lib/Sema/SemaStmtAttr.cpp

Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/DelayedDiagnostic.h"
@@ -20,6 +21,7 @@
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Triple.h"
+#include 
 
 using namespace clang;
 using namespace sema;
@@ -53,6 +55,26 @@
   return ::new (S.Context) FallThroughAttr(S.Context, A);
 }
 
+static void recur(SmallVectorImpl , const Stmt& S) {
+  for (auto Child: S.children()) {
+if (isa(Child))
+  std::cout << "is a declrefexpr" << std::endl;
+else
+  std::cout << "is NOT a declrefexpr" << std::endl;
+recur(V, *Child);
+  }
+}
+namespace {
+class DeclRefExprVisitor : public StmtVisitor {
+public:
+  SmallVector ExprNodesForAttrArgs;
+  ExprResult VisitDeclRefExpr(DeclRefExpr *E) {
+std::cout << "Found a decl ref expr!" << std::endl;
+ExprNodesForAttrArgs.emplace_back();
+  }
+};
+} // namespace
+
 static Attr *handleHardenMisspeculationAttr(Sema , Stmt *St,
 const ParsedAttr ,
 SourceRange Range) {
@@ -81,6 +103,38 @@
   return nullptr;
 }
   }
+  // We need to find the DeclRefExpr nodes that are for each variable passed
+  // into the attribute since we have to create a GCCAsmNode that references
+  // each variable later.
+
+  DeclRefExprVisitor DREV;
+  DREV.Visit(St);
+  for (unsigned i = 0; i < A.getNumArgs(); i++) {
+IdentifierLoc *CurrArg = A.getArg(i).get();
+  }
+//CurrArg->dump();
+// Create GCC node...
+// TODO: Figure out appropriate source location.
+//SourceLocation AsmLoc = SourceLocation();
+//bool IsSimple = false;
+//bool IsVolatile = true;
+//unsigned NumOutputs = 1;
+//unsigned NumInputs = 0;
+//IdentifierInfo **Names = nullptr; // TODO: What should this be?
+//StringLiteral *Constraint = nullptr; //StringLiteral("+r");
+//StringLiteral **Constraints = 
+//StringLiteral **Clobbers= nullptr;
+//Expr **AsmExprs = 
+//StringLiteral * AsmString = nullptr; //StringLiteral("");
+//unsigned NumClobbers = 0;
+//unsigned NumLabels = 0;
+//SourceLocation RParenLoc = SourceLocation(); //TODO
+//
+//::new (S.Context)
+//  GCCAsmStmt(S.Context, AsmLoc, IsSimple, IsVolatile, NumOutputs,
+// NumInputs, Names, Constraints, AsmExprs, AsmString,
+// NumClobbers, Clobbers, NumLabels, RParenLoc);
+// } 
 
   return ::new (S.Context) HardenMisspeculationAttr(S.Context, A);
 }
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -24,6 +24,9 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
+#include 
+#include 
+#include 
 using namespace clang;
 using namespace sema;
 
@@ -245,11 +248,40 @@
  Expr *asmString, MultiExprArg clobbers,
  unsigned NumLabels,
  SourceLocation RParenLoc) {
+//  llvm::formatv("My asm statement args: {0}, {1}, {2}, {3}, {4}, {5}, {6}, "
+//"{7}, {8}, {9}, {10}, {11}, {12}",
+//AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names,
+//constraints, Exprs, asmString, clobbers->get, NumLabels);
+
+  std::cout << llvm::formatv("NumInputs: {0}\nNumOutputs: {1}\n", NumInputs, NumOutputs).str();
+  std::cout << llvm::formatv("IsSimple: {0}\nIsVolatile: {1}\n", IsSimple, IsVolatile).str();
+  asmString->dumpColor();
+  dyn_cast(asmString)->dumpPretty(Context);
+  std::cout << llvm::formatv("AsmString: \"{0}\"\n", asmString).str();
+  std::cout << llvm::formatv("NumLabels: {0}\n", NumLabels).str();
+  std::cout << llvm::formatv("NumExprs: {0}\n", Exprs.size()).str();
+  Exprs[0]->dumpColor();
   unsigned NumClobbers = clobbers.size();
+  std::cout << llvm::formatv("NumClobbers: {0}\n", NumClobbers).str();
+  std::cout << llvm::formatv("NumConstraints: {0}\n", constraints.size()).str();
   StringLiteral **Constraints =
 reinterpret_cast(constraints.data());
+  StringLiteral **CopyConstraints = Constraints;
+  int i = constraints.size();
+  while (i > 0) {
+std::cout <<

[clang] 000ad1a - [clang] fix a compilation bug

[clang] ae500e4 - Always keep unset fields in FPOptionsOverride zeroed.

[clang] 948219d - Replace setter named 'getAsOpaqueInt' with a real getter.

[clang] ae30670 - Use consistent code for setting FPFeatures from operator constructors.

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

[PATCH] D84136: [clang] Fix visitation of ConceptSpecializationExpr in constrained-parameter

[clang] 9860e68 - Don't leave the FPOptions in a UnaryOperator uninitialized.

[PATCH] D83536: [clangd] Index refs to main-file symbols as well

[clang] ae7f088 - [InstCombine] Aggregate reconstruction simplification (PR47060)

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

[PATCH] D85934: Enable OpenMP offloading to VE and enable tests for offloading to VE

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

[clang] 95a25e4 - [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

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

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

[PATCH] D85878: [OpenMP] Context selector extensions for return value overloading

[PATCH] D85787: [InstCombine] Aggregate reconstruction simplification (PR47060)

[PATCH] D85321: [OPENMP]Do not capture base pointer by reference if it is used as a base for array-like reduction.

[clang] fef2607 - [Sema] Use the proper cast for a fixed bool enum.

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[clang] 827ba67 - [Sema] Validate calls to GetExprRange.

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

[PATCH] D86037: [OpenMP][FIX] Do not use TBAA in type punning reduction GPU code PR46156

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

[clang] 44613bb - Create strict aligned code for OpenBSD/arm64.

[PATCH] D86034: [WIP] Attribute harden_misspeculation

[PATCH] D86033: This is mostly prototype code that should be deleted.

31 matches

Site Navigation

Mail list logo

Footer information