date:20200712

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D83648#2146472 , @pzheng wrote:

> Actually, this patch won't change --help because it just reduced some 
> duplication by extracting the common part (" the integrated assembler") of 
> the help message into the "help" of "OptOutFFlag". Sorry for the confusion.
>
> The real fix here is to restore "integrated_as" as a DriverOption. Without 
> this, when we have -fintegrated-as on a link line, it will be passed to the 
> linker and cause the linker to fail because it is not a valid linker flag.

The description is not correct. The actual difference is:

  static bool forwardToGCC(const Option ) {
// Don't forward inputs from the original command line.  They are added from
// InputInfoList.
return O.getKind() != Option::InputClass &&
   !O.hasFlag(options::DriverOption) && 
!O.hasFlag(options::LinkerInput);
  }

If `clang::driver::tools::gcc::Common` is selected, DriverOption modified 
options are not forwarded to gcc.

`clang -target x86_64-linux -fintegrated-as a.o '-###'` => no -fintegrated-as

`clang -target x86_64 -fintegrated-as a.o '-###'` => `"/usr/bin/gcc" 
"-fintegrated-as" "-m64" "-o" "a.out" "a.o"`

In this sense, all -f options not recognized by GCC should have a DriverOption 
tag I don't think we have properly tagged all options.

Comment at: clang/test/Driver/integrated-as.c:10

+// RUN: %clang -### -fintegrated-as %s 2>&1 | FileCheck %s -check-prefix 
FIAS-LINK
+

This test is incorrect. You need a specific target triple to select bare-metal 
like GCC driver.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83648

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

[clang] c943329 - Revert "Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression"

2020-07-12 Thread David Blaikie via cfe-commits


Author: David Blaikie
Date: 2020-07-12T20:29:19-07:00
New Revision: c94332919bd922032e979b3ae3ced5ca5bdf9650

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

LOG: Revert "Rename/refactor isIntegerConstantExpression to 
getIntegerConstantExpression"

Broke buildbots since I hadn't updated this patch in a while. Sorry for
the noise.

This reverts commit 49e5f603d40083dce9c05796e3cde3a185c3beba.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index a42c7bb5a9f2..66eafaaab715 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -510,15 +510,16 @@ class Expr : public ValueStmt {
   /// semantically correspond to a bool.
   bool isKnownToHaveBooleanValue(bool Semantic = true) const;
 
-  /// isIntegerConstantExpr - Return the value if this expression is a valid
-  /// integer constant expression.  If not a valid i-c-e, return None and fill
-  /// in Loc (if specified) with the location of the invalid expression.
+  /// isIntegerConstantExpr - Return true if this expression is a valid integer
+  /// constant expression, and, if so, return its value in Result.  If not a
+  /// valid i-c-e, return false and fill in Loc (if specified) with the 
location
+  /// of the invalid expression.
   ///
   /// Note: This does not perform the implicit conversions required by C++11
   /// [expr.const]p5.
-  Optional getIntegerConstantExpr(const ASTContext ,
-SourceLocation *Loc = nullptr,
-bool isEvaluated = true) const;
+  bool isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
+ SourceLocation *Loc = nullptr,
+ bool isEvaluated = true) const;
   bool isIntegerConstantExpr(const ASTContext ,
  SourceLocation *Loc = nullptr) const;
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 807028885652..2ba643f12a82 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9471,15 +9471,17 @@ QualType ASTContext::mergeTypes(QualType LHS, QualType 
RHS,
   const ConstantArrayType* CAT)
   -> std::pair {
 if (VAT) {
-  Optional TheInt;
+  llvm::APSInt TheInt;
   Expr *E = VAT->getSizeExpr();
-  if (E && (TheInt = E->getIntegerConstantExpr(*this)))
-return std::make_pair(true, *TheInt);
-  return std::make_pair(false, llvm::APSInt());
+  if (E && E->isIntegerConstantExpr(TheInt, *this))
+return std::make_pair(true, TheInt);
+  else
+return std::make_pair(false, TheInt);
+} else if (CAT) {
+return std::make_pair(true, CAT->getSize());
+} else {
+return std::make_pair(false, llvm::APInt());
 }
-if (CAT)
-  return std::make_pair(true, CAT->getSize());
-return std::make_pair(false, llvm::APInt());
   };
 
   bool HaveLSize, HaveRSize;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 011dc890496d..a4dc0ccad1e0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14883,22 +14883,16 @@ bool Expr::isIntegerConstantExpr(const ASTContext 
,
   return true;
 }
 
-Optional Expr::getIntegerConstantExpr(const ASTContext ,
-SourceLocation *Loc,
-bool isEvaluated) const {
+bool Expr::isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
+ SourceLocation *Loc, bool isEvaluated) const {
   assert(!isValueDependent() &&
  "Expression evaluator can't be called on a dependent expression.");
 
-  APSInt Value;
-
-  if (Ctx.getLangOpts().CPlusPlus11) {
-if (EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, , Loc))
-  return Value;
-return None;
-  }
+  if (Ctx.getLangOpts().CPlusPlus11)
+return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, , Loc);
 
   if (!isIntegerConstantExpr(Ctx, Loc))
-return None;
+return false;
 
   // The only possible side-effects here

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

2020-07-12 Thread Juneyoung Lee via Phabricator via cfe-commits

aqjune added a comment.

(renaming variables for readability)

  %a = select i1 %s, i1 undef, i1 %t
  %b = xor i1 %s, 1
  %c = and i1 %a, %b

This series of reasoning happened from a single SimplifyAndInst call:

  c = a & (s ^ 1)
= (a & s) ^ (a & 1); ExpandBinOp
= ((select s, undef, t) & s) ^ a
= (select s, (undef & s), (t & s)) ^ a ; ThreadBinOpOverSelect
= (select s, (undef & s), false) ^ a   ; since s = (x == y), t = (x < y)
= (select s, false, false) ^ a ; choose undef to be false
= a
= select s, undef, t

In general, distributing `a` into operands of xor (second line) isn't sound 
because it increases the number of uses of `a`. We don't want to totally 
disable the simplification, however.

If InstSimplify never increases the number of uses in the end, we have an 
alternative solution: tracking to which value undef is folded.
Whenever an undef value is chosen to be a concrete value, the decision should 
be remembered, so the copied undefs won't be folded into different values.
In case of InstSimplify, we can identify individual undefs by Use, since 
InstSimplify won't do any transformation inside.
This means SimplifyXXX needs to return two things: the simplified value & the 
undef cache. Since InstSimplify isn't designed to do transformation directly, 
other optimizations like InstCombine should perform the final change.

Does this solution make sense? Then, I can prepare a patch for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360



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

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

2020-07-12 Thread David Blaikie via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG49e5f603d400: Rename/refactor isIntegerConstantExpression to 
getIntegerConstantExpression (authored by dblaikie).
Herald added a subscriber: sstefan1.

Changed prior to commit:
  https://reviews.llvm.org/D76646?vs=252158=277317#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76646

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

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2476,8 +2476,8 @@
 return Context.getDependentVectorType(CurType, SizeExpr, AttrLoc,
VectorType::GenericVector);
 
-  llvm::APSInt VecSize(32);
-  if (!SizeExpr->isIntegerConstantExpr(VecSize, Context)) {
+  Optional VecSize = SizeExpr->getIntegerConstantExpr(Context);
+  if (!VecSize) {
 Diag(AttrLoc, diag::err_attribute_argument_type)
 << "vector_size" << AANT_ArgumentIntegerConstant
 << SizeExpr->getSourceRange();
@@ -2489,13 +2489,13 @@
VectorType::GenericVector);
 
   // vecSize is specified in bytes - convert to bits.
-  if (!VecSize.isIntN(61)) {
+  if (!VecSize->isIntN(61)) {
 // Bit size will overflow uint64.
 Diag(AttrLoc, diag::err_attribute_size_too_large)
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize.getZExtValue() * 8;
+  uint64_t VectorSizeBits = VecSize->getZExtValue() * 8;
   unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
@@ -2540,8 +2540,8 @@
   }
 
   if (!ArraySize->isTypeDependent() && !ArraySize->isValueDependent()) {
-llvm::APSInt vecSize(32);
-if (!ArraySize->isIntegerConstantExpr(vecSize, Context)) {
+Optional vecSize = ArraySize->getIntegerConstantExpr(Context);
+if (!vecSize) {
   Diag(AttrLoc, diag::err_attribute_argument_type)
 << "ext_vector_type" << AANT_ArgumentIntegerConstant
 << ArraySize->getSourceRange();
@@ -2555,7 +2555,7 @@
 }
 // Unlike gcc's vector_size attribute, the size is specified as the
 // number of elements, not the number of bytes.
-unsigned vectorSize = static_cast(vecSize.getZExtValue());
+unsigned vectorSize = static_cast(vecSize->getZExtValue());
 
 if (vectorSize == 0) {
   Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -6254,13 +6254,15 @@
const Expr *AddrSpace,
SourceLocation AttrLoc) {
   if (!AddrSpace->isValueDependent()) {
-llvm::APSInt addrSpace(32);
-if (!AddrSpace->isIntegerConstantExpr(addrSpace, S.Context)) {
+Optional OptAddrSpace =
+AddrSpace->getIntegerConstantExpr(S.Context);
+if (!OptAddrSpace) {
   S.Diag(AttrLoc, diag::err_attribute_argument_type)
   << "'address_space'" << AANT_ArgumentIntegerConstant
   << AddrSpace->getSourceRange();
   return false;
 }
+llvm::APSInt  = *OptAddrSpace;
 
 // Bounds checking.
 if (addrSpace.isSigned()) {
@@ -7712,9 +7714,9 @@
   }
   // The number of elements must be an ICE.
   Expr *numEltsExpr = static_cast(Attr.getArgAsExpr(0));
-  llvm::APSInt numEltsInt(32);
+  Optional numEltsInt;
   if (numEltsExpr->isTypeDependent() || numEltsExpr->isValueDependent() ||
-  !numEltsExpr->isIntegerConstantExpr(numEltsInt, S.Context)) {
+  !(numEltsInt = numEltsExpr->getIntegerConstantExpr(S.Context))) {
 S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
 << Attr << AANT_ArgumentIntegerConstant
 << numEltsExpr->getSourceRange();
@@ -7730,7 +7732,7 @@
 
   // The total size of the vector must be 64 or 128 bits.
   unsigned typeSize = static_cast(S.Context.getTypeSize(CurType));
-  unsigned numElts = static_cast(numEltsInt.getZExtValue());
+  unsigned numElts = static_cast(numEltsInt->getZExtValue());
   unsigned vecSize = typeSize * numElts;
   if (vecSize != 64 && vecSize != 128) {
 S.Diag(Attr.getLoc(), diag::err_attribute_bad_neon_vector_size) << CurType;
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -335,15 +335,15 @@
 
   if (NumArgs == 1) {
 Expr *E = A.getArgAsExpr(0);
-

[clang] 49e5f60 - Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

2020-07-12 Thread David Blaikie via cfe-commits


Author: David Blaikie
Date: 2020-07-12T19:43:24-07:00
New Revision: 49e5f603d40083dce9c05796e3cde3a185c3beba

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

LOG: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

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

Reviewers: aaron.ballman

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

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 66eafaaab715..a42c7bb5a9f2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -510,16 +510,15 @@ class Expr : public ValueStmt {
   /// semantically correspond to a bool.
   bool isKnownToHaveBooleanValue(bool Semantic = true) const;
 
-  /// isIntegerConstantExpr - Return true if this expression is a valid integer
-  /// constant expression, and, if so, return its value in Result.  If not a
-  /// valid i-c-e, return false and fill in Loc (if specified) with the 
location
-  /// of the invalid expression.
+  /// isIntegerConstantExpr - Return the value if this expression is a valid
+  /// integer constant expression.  If not a valid i-c-e, return None and fill
+  /// in Loc (if specified) with the location of the invalid expression.
   ///
   /// Note: This does not perform the implicit conversions required by C++11
   /// [expr.const]p5.
-  bool isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
- SourceLocation *Loc = nullptr,
- bool isEvaluated = true) const;
+  Optional getIntegerConstantExpr(const ASTContext ,
+SourceLocation *Loc = nullptr,
+bool isEvaluated = true) const;
   bool isIntegerConstantExpr(const ASTContext ,
  SourceLocation *Loc = nullptr) const;
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 2ba643f12a82..807028885652 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9471,17 +9471,15 @@ QualType ASTContext::mergeTypes(QualType LHS, QualType 
RHS,
   const ConstantArrayType* CAT)
   -> std::pair {
 if (VAT) {
-  llvm::APSInt TheInt;
+  Optional TheInt;
   Expr *E = VAT->getSizeExpr();
-  if (E && E->isIntegerConstantExpr(TheInt, *this))
-return std::make_pair(true, TheInt);
-  else
-return std::make_pair(false, TheInt);
-} else if (CAT) {
-return std::make_pair(true, CAT->getSize());
-} else {
-return std::make_pair(false, llvm::APInt());
+  if (E && (TheInt = E->getIntegerConstantExpr(*this)))
+return std::make_pair(true, *TheInt);
+  return std::make_pair(false, llvm::APSInt());
 }
+if (CAT)
+  return std::make_pair(true, CAT->getSize());
+return std::make_pair(false, llvm::APInt());
   };
 
   bool HaveLSize, HaveRSize;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index a4dc0ccad1e0..011dc890496d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14883,16 +14883,22 @@ bool Expr::isIntegerConstantExpr(const ASTContext 
,
   return true;
 }
 
-bool Expr::isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
- SourceLocation *Loc, bool isEvaluated) const {
+Optional Expr::getIntegerConstantExpr(const ASTContext ,
+SourceLocation *Loc,
+bool isEvaluated) const {
   assert(!isValueDependent() &&
  "Expression evaluator can't be called on a dependent expression.");
 
-  if (Ctx.getLangOpts().CPlusPlus11)
-return EvaluateCPlusPlus11IntegralConstantExpr(Ctx, this, , Loc);
+  APSInt Value;
+
+  if (Ctx.getLangOpts().CPlusPlus11) {
+if

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread Pengxuan Zheng via Phabricator via cfe-commits

pzheng updated this revision to Diff 277316.
pzheng added a comment.

Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83648

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/integrated-as.c


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -7,6 +7,10 @@
 
 // FIAS: cc1as
 
+// RUN: %clang -### -fintegrated-as %s 2>&1 | FileCheck %s -check-prefix 
FIAS-LINK
+
+// FIAS-LINK-NOT: -fintegrated-as
+
 // RUN: %clang -target none -### -fno-integrated-as -S %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix NOFIAS
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2887,7 +2887,7 @@
   MetaVarName<"">;
 def y : Joined<["-"], "y">;
 
-defm integrated_as : OptOutFFlag<"integrated-as", "Enable the integrated 
assembler", "Disable the integrated assembler">;
+defm integrated_as : OptOutFFlag<"integrated-as", "Enable", "Disable", " the 
integrated assembler", [DriverOption]>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,
   Flags<[CoreOption, DriverOption]>, Group,


Index: clang/test/Driver/integrated-as.c
===
--- clang/test/Driver/integrated-as.c
+++ clang/test/Driver/integrated-as.c
@@ -7,6 +7,10 @@
 
 // FIAS: cc1as
 
+// RUN: %clang -### -fintegrated-as %s 2>&1 | FileCheck %s -check-prefix FIAS-LINK
+
+// FIAS-LINK-NOT: -fintegrated-as
+
 // RUN: %clang -target none -### -fno-integrated-as -S %s 2>&1 \
 // RUN: | FileCheck %s -check-prefix NOFIAS
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2887,7 +2887,7 @@
   MetaVarName<"">;
 def y : Joined<["-"], "y">;
 
-defm integrated_as : OptOutFFlag<"integrated-as", "Enable the integrated assembler", "Disable the integrated assembler">;
+defm integrated_as : OptOutFFlag<"integrated-as", "Enable", "Disable", " the integrated assembler", [DriverOption]>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,
   Flags<[CoreOption, DriverOption]>, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83652: Merge some of the PCH object support with modular codegen

2020-07-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie created this revision.
dblaikie added reviewers: rnk, llunak, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I was trying to pick this up a bit when reviewing D48426 
 (& perhaps D69778 
) - in any case, looks like D48426 
 added a module level flag that might not be 
needed.

The D48426  implementation worked by setting a 
module level flag, then code generating contents from the PCH a special case in 
ASTContext::DeclMustBeEmitted would be used to delay emitting the definition of 
these functions if they came from a Module with this flag.

This strategy is similar to the one initially implemented for modular codegen 
that was removed in D29901  in favor of the 
modular decls list and a bit on each decl to specify whether it's homed to a 
module.

One major difference between PCH object support and modular code generation, 
other than the specific list of decls that are homed, is the compilation model: 
MSVC PCH modules are built into the object file for some other source file 
(when compiling that source file /Yc is specified to say "this compilation is 
where the PCH is homed"), whereas modular code generation invokes a separate 
compilation for the PCH alone. So the current modular code generation test of 
to decide if a decl should be emitted "is the module where this decl is 
serialized the current main file" has to be extended (as Lubos did in D69778 
) to also test the command line flag 
-building-pch-with-obj.

Otherwise the whole thing is basically streamlined down to the modular code 
generation path.

This even offers one extra material improvement compared to the existing 
divergent implementation: Homed functions are not emitted into object files 
that use the pch. Instead at -O0 they are not emitted into the IR at all, and 
at -O1 they are emitted using available_externally (existing functionality 
implemented for modular code generation). The pch-codegen test has been updated 
to reflect this new behavior.

[If possible: I'd love it if we could not have the extra MSVC-style way of 
accessing dllexport-pch-homing, and just do it the modular codegen way, but I 
understand that it might be a limitation of existing build systems. @hans / 
@thakis: Do either of you know if it'd be practical to move to something more 
similar to .pcm handling, where the pch itself is passed to the compilation, 
rather than homed as a side effect of compiling some other source file?]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83652

Files:
  clang/include/clang/AST/ExternalASTSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1119,7 +1119,6 @@
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Clang min.
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relocatable
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Timestamps
-  MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // PCHHasObjectFile
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Errors
   MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // SVN branch/tag
   unsigned MetadataAbbrevCode = Stream.EmitAbbrev(std::move(MetadataAbbrev));
@@ -1134,7 +1133,6 @@
 CLANG_VERSION_MINOR,
 !isysroot.empty(),
 IncludeTimestamps,
-false,
 ASTHasCompilerErrors};
 Stream.EmitRecordWithBlob(MetadataAbbrevCode, Record,
   getClangFullRepositoryVersion());
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2747,7 +2747,7 @@
 return VersionMismatch;
   }
 
-  bool hasErrors = Record[7];
+  bool hasErrors = Record[6];
   if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) {
 Diag(diag::err_pch_with_compiler_errors);
 return HadErrors;
@@ -2765,8 +2765,6 @@
 
   F.HasTimestamps = Record[5];
 
-  F.PCHHasObjectFile = Record[6];
-
   const std::string  = getClangFullRepositoryVersion();
   StringRef ASTBranch = Blob;
   if (StringRef(CurBranch) != ASTBranch && !DisableValidation) {
@@ -8590,11 +8588,6 @@
   return getSubmodule(ID);
 }
 
-bool

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D83648#2146472 , @pzheng wrote:

> Actually, this patch won't change --help because it just reduced some 
> duplication by extracting the common part (" the integrated assembler") of 
> the help message into the "help" of "OptOutFFlag". Sorry for the confusion.
>
> The real fix here is to restore "integrated_as" as a DriverOption. Without 
> this, when we have -fintegrated-as on a link line, it will be passed to the 
> linker and cause the linker to fail because it is not a valid linker flag.


Sounds like that could be tested with a driver test (using -### to see what the 
linker command would be and validating that it doesn't include -fintegrated-as? 
- see other similar tests in clang/test/Driver, I think)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83648



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

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread Pengxuan Zheng via Phabricator via cfe-commits

pzheng added a comment.

Actually, this patch won't change --help because it just reduced some 
duplication by extracting the common part (" the integrated assembler") of the 
help message into the "help" of "OptOutFFlag". Sorry for the confusion.

The real fix here is to restore "integrated_as" as a DriverOption. Without 
this, when we have -fintegrated-as on a link line, it will be passed to the 
linker and cause the linker to fail because it is not a valid linker flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83648



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

[clang] b4dbb37 - [X86] Rename X86_CPU_TYPE_COMPAT_ALIAS/X86_CPU_TYPE_COMPAT/X86_CPU_SUBTYPE_COMPAT macros. NFC

2020-07-12 Thread Craig Topper via cfe-commits


Author: Craig Topper
Date: 2020-07-12T17:00:24-07:00
New Revision: b4dbb37f32e554e4d6f118d9ddd87717721ea664

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

LOG: [X86] Rename 
X86_CPU_TYPE_COMPAT_ALIAS/X86_CPU_TYPE_COMPAT/X86_CPU_SUBTYPE_COMPAT macros. NFC

Remove _COMPAT. Drop the ARCHNAME. Remove the non-COMPAT versions
that are no longer needed.

We now only use these macros in places where we need compatibility
with libgcc/compiler-rt. So we don't need to call out _COMPAT
specifically.

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp
clang/lib/CodeGen/CGBuiltin.cpp
llvm/include/llvm/Support/X86TargetParser.def
llvm/include/llvm/Support/X86TargetParser.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index e280a7216645..543f232d2459 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -1062,9 +1062,9 @@ void X86TargetInfo::getCPUSpecificCPUDispatchFeatures(
 bool X86TargetInfo::validateCpuIs(StringRef FeatureStr) const {
   return llvm::StringSwitch(FeatureStr)
 #define X86_VENDOR(ENUM, STRING) .Case(STRING, true)
-#define X86_CPU_TYPE_COMPAT_ALIAS(ENUM, ALIAS) .Case(ALIAS, true)
-#define X86_CPU_TYPE_COMPAT(ARCHNAME, ENUM, STR) .Case(STR, true)
-#define X86_CPU_SUBTYPE_COMPAT(ARCHNAME, ENUM, STR) .Case(STR, true)
+#define X86_CPU_TYPE_ALIAS(ENUM, ALIAS) .Case(ALIAS, true)
+#define X86_CPU_TYPE(ENUM, STR) .Case(STR, true)
+#define X86_CPU_SUBTYPE(ENUM, STR) .Case(STR, true)
 #include "llvm/Support/X86TargetParser.def"
   .Default(false);
 }

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 1d81ede5dc31..35a93a7889f4 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -11655,11 +11655,11 @@ Value *CodeGenFunction::EmitX86CpuIs(StringRef 
CPUStr) {
   std::tie(Index, Value) = StringSwitch>(CPUStr)
 #define X86_VENDOR(ENUM, STRING)   
\
   .Case(STRING, {0u, static_cast(llvm::X86::ENUM)})
-#define X86_CPU_TYPE_COMPAT_ALIAS(ENUM, ALIAS) \
+#define X86_CPU_TYPE_ALIAS(ENUM, ALIAS)
\
   .Case(ALIAS, {1u, static_cast(llvm::X86::ENUM)})
-#define X86_CPU_TYPE_COMPAT(ARCHNAME, ENUM, STR)   
\
+#define X86_CPU_TYPE(ENUM, STR)
\
   .Case(STR, {1u, static_cast(llvm::X86::ENUM)})
-#define X86_CPU_SUBTYPE_COMPAT(ARCHNAME, ENUM, STR)
\
+#define X86_CPU_SUBTYPE(ENUM, STR) 
\
   .Case(STR, {2u, static_cast(llvm::X86::ENUM)})
 #include "llvm/Support/X86TargetParser.def"
.Default({0, 0});

diff  --git a/llvm/include/llvm/Support/X86TargetParser.def 
b/llvm/include/llvm/Support/X86TargetParser.def
index 9e9f0985d15e..697f8c70f962 100644
--- a/llvm/include/llvm/Support/X86TargetParser.def
+++ b/llvm/include/llvm/Support/X86TargetParser.def
@@ -20,80 +20,70 @@ X86_VENDOR(VENDOR_AMD,   "amd")
 #undef X86_VENDOR
 
 // This macro is used for cpu types present in compiler-rt/libgcc.
-#ifndef X86_CPU_TYPE_COMPAT
-#define X86_CPU_TYPE_COMPAT(ARCHNAME, ENUM, STR) X86_CPU_TYPE(ARCHNAME, ENUM)
-#endif
-
 #ifndef X86_CPU_TYPE
-#define X86_CPU_TYPE(ARCHNAME, ENUM)
+#define X86_CPU_TYPE(ENUM, STR)
 #endif
 
-#ifndef X86_CPU_TYPE_COMPAT_ALIAS
-#define X86_CPU_TYPE_COMPAT_ALIAS(ENUM, STR)
+#ifndef X86_CPU_TYPE_ALIAS
+#define X86_CPU_TYPE_ALIAS(ENUM, STR)
 #endif
 
-// The first part of this list must match what is implemented in libgcc and
-// compilert-rt. Clang uses this to know how to implement __builtin_cpu_is.
-X86_CPU_TYPE_COMPAT("bonnell",   INTEL_BONNELL,   "bonnell")
-X86_CPU_TYPE_COMPAT("core2", INTEL_CORE2, "core2")
-X86_CPU_TYPE_COMPAT("nehalem",   INTEL_COREI7,"corei7")
-X86_CPU_TYPE_COMPAT("amdfam10",  AMDFAM10H,   "amdfam10h")
-X86_CPU_TYPE_COMPAT("bdver1",AMDFAM15H,   "amdfam15h")
-X86_CPU_TYPE_COMPAT("silvermont",INTEL_SILVERMONT,"silvermont")
-X86_CPU_TYPE_COMPAT("knl",   INTEL_KNL,   "knl")
-X86_CPU_TYPE_COMPAT("btver1",AMD_BTVER1,  "btver1")
-X86_CPU_TYPE_COMPAT("btver2",AMD_BTVER2,  "btver2")
-X86_CPU_TYPE_COMPAT("znver1",AMDFAM17H,   "amdfam17h")
-X86_CPU_TYPE_COMPAT("knm",   INTEL_KNM,   "knm")
-X86_CPU_TYPE_COMPAT("goldmont",  INTEL_GOLDMONT,  "goldmont")
-X86_CPU_TYPE_COMPAT("goldmont-plus", INTEL_GOLDMONT_PLUS, "goldmont-plus")
-X86_CPU_TYPE_COMPAT("tremont",   INTEL_TREMONT,   "tremont")
+// This list must match what is implemented in libgcc and compilert-rt. Clang
+// uses this

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

Is there missing test coverage for this in some way? (how does this patch 
change the observable behavior of clang? I guess --help would change?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83648



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

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

2020-07-12 Thread Pengxuan Zheng via Phabricator via cfe-commits

pzheng created this revision.
pzheng added reviewers: MaskRay, dblaikie, echristo.
Herald added subscribers: cfe-commits, dang.
Herald added a project: clang.

DriverOption seems to be accidentally removed from integrated_as definition in
commit e5158b5 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83648

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2887,7 +2887,7 @@
   MetaVarName<"">;
 def y : Joined<["-"], "y">;
 
-defm integrated_as : OptOutFFlag<"integrated-as", "Enable the integrated 
assembler", "Disable the integrated assembler">;
+defm integrated_as : OptOutFFlag<"integrated-as", "Enable", "Disable", " the 
integrated assembler", [DriverOption]>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,
   Flags<[CoreOption, DriverOption]>, Group,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2887,7 +2887,7 @@
   MetaVarName<"">;
 def y : Joined<["-"], "y">;
 
-defm integrated_as : OptOutFFlag<"integrated-as", "Enable the integrated assembler", "Disable the integrated assembler">;
+defm integrated_as : OptOutFFlag<"integrated-as", "Enable", "Disable", " the integrated assembler", [DriverOption]>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,
   Flags<[CoreOption, DriverOption]>, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1111678 - [clang] Add -Wsuggest-override

2020-07-12 Thread David Blaikie via cfe-commits


Author: Logan Smith
Date: 2020-07-12T16:05:24-07:00
New Revision: 67895d47558989f9f3a593a82527b016c7e7

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

LOG: [clang] Add -Wsuggest-override

This patch adds `-Wsuggest-override`, which allows for more aggressive 
enforcement of modern C++ best practices, as well as better compatibility with 
gcc, which has had its own `-Wsuggest-override` since version 5.1.

Clang already has `-Winconsistent-missing-override`, which only warns in the 
case where there is at least one function already marked `override` in a class. 
This warning strengthens that warning by suggesting the `override` keyword 
regardless of whether it is already present anywhere.

The text between suggest-override and inconsistent-missing-override is now 
shared, using `TextSubstitution` for the entire diagnostic text.

Reviewed By: dblaikie

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

Added: 
clang/test/SemaCXX/warn-suggest-destructor-override
clang/test/SemaCXX/warn-suggest-override

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 6a50ceef4191..1e829be4028e 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -280,9 +280,12 @@ def CXX98CompatPedantic : 
DiagGroup<"c++98-compat-pedantic",
 
 def CXX11Narrowing : DiagGroup<"c++11-narrowing">;
 
-def CXX11WarnOverrideDestructor :
+def CXX11WarnInconsistentOverrideDestructor :
   DiagGroup<"inconsistent-missing-destructor-override">;
-def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">;
+def CXX11WarnInconsistentOverrideMethod :
+  DiagGroup<"inconsistent-missing-override">;
+def CXX11WarnSuggestOverrideDestructor : 
DiagGroup<"suggest-destructor-override">;
+def CXX11WarnSuggestOverride : DiagGroup<"suggest-override">;
 
 // Original name of this warning in Clang
 def : DiagGroup<"c++0x-narrowing", [CXX11Narrowing]>;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 24e942037ecf..71517edd6659 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2367,12 +2367,22 @@ def override_keyword_hides_virtual_member_function : 
Error<
   "%select{function|functions}1">;
 def err_function_marked_override_not_overriding : Error<
   "%0 marked 'override' but does not override any member functions">;
-def warn_destructor_marked_not_override_overriding : Warning <
-  "%0 overrides a destructor but is not marked 'override'">,
-  InGroup, DefaultIgnore;
-def warn_function_marked_not_override_overriding : Warning <
-  "%0 overrides a member function but is not marked 'override'">,
-  InGroup;
+def warn_destructor_marked_not_override_overriding : TextSubstitution <
+  "%0 overrides a destructor but is not marked 'override'">;
+def warn_function_marked_not_override_overriding : TextSubstitution <
+  "%0 overrides a member function but is not marked 'override'">;
+def warn_inconsistent_destructor_marked_not_override_overriding : Warning <
+  "%sub{warn_destructor_marked_not_override_overriding}0">,
+  InGroup, DefaultIgnore;
+def warn_inconsistent_function_marked_not_override_overriding : Warning <
+  "%sub{warn_function_marked_not_override_overriding}0">,
+  InGroup;
+def warn_suggest_destructor_marked_not_override_overriding : Warning <
+  "%sub{warn_destructor_marked_not_override_overriding}0">,
+  InGroup, DefaultIgnore;
+def warn_suggest_function_marked_not_override_overriding : Warning <
+  "%sub{warn_function_marked_not_override_overriding}0">,
+  InGroup, DefaultIgnore;
 def err_class_marked_final_used_as_base : Error<
   "base %0 is marked '%select{final|sealed}1'">;
 def warn_abstract_final_class : Warning<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e75ac185eb2c..6f7ad8076718 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6965,7 +6965,7 @@ class Sema final {
 
   /// DiagnoseAbsenceOfOverrideControl - Diagnose if 'override' keyword was
   /// not used in the declaration of an overriding method.
-  void DiagnoseAbsenceOfOverrideControl(NamedDecl *D);
+  void DiagnoseAbsenceOfOverrideControl(NamedDecl *D, bool Inconsistent);
 
   /// CheckForFunctionMarkedFinal - Checks whether a virtual member function
   /// overrides a virtual member function marked 'final', according to

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp

[PATCH] D82728: [clang] Add -Wsuggest-override

2020-07-12 Thread David Blaikie via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67895d47: [clang] Add -Wsuggest-override (authored by 
logan-5, committed by dblaikie).

Changed prior to commit:
  https://reviews.llvm.org/D82728?vs=276248=277308#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82728

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-suggest-destructor-override
  clang/test/SemaCXX/warn-suggest-override

Index: clang/test/SemaCXX/warn-suggest-override
===
--- /dev/null
+++ clang/test/SemaCXX/warn-suggest-override
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-override
+
+struct A {
+  ~A();
+  void run();
+};
+
+struct B : public A {
+  ~B();
+  void run();
+};
+
+struct C {
+  virtual void run(); // expected-note 2{{overridden virtual function is here}}
+  virtual ~C();
+};
+
+struct D : public C {
+  void run();
+  // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}}
+  ~D();
+};
+
+struct E : public C {
+  virtual void run();
+  // expected-warning@-1 {{'run()' overrides a member function but is not marked 'override'}}
+  virtual ~E();
+};
+
+struct F : public C {
+  void run() override;
+  ~F() override;
+};
+
+struct G : public C {
+  void run() final;
+  ~G() final;
+};
Index: clang/test/SemaCXX/warn-suggest-destructor-override
===
--- /dev/null
+++ clang/test/SemaCXX/warn-suggest-destructor-override
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Wsuggest-destructor-override
+
+struct A {
+  ~A();
+  virtual void run();
+};
+
+struct B : public A {
+  ~B();
+};
+
+struct C {
+  virtual void run();
+  virtual ~C();  // expected-note 2{{overridden virtual function is here}}
+};
+
+struct D : public C {
+  void run();
+  ~D();
+  // expected-warning@-1 {{'~D' overrides a destructor but is not marked 'override'}}
+};
+
+struct E : public C {
+  void run();
+  virtual ~E();
+  // expected-warning@-1 {{'~E' overrides a destructor but is not marked 'override'}}
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -3045,7 +3045,7 @@
   << MD->getDeclName();
 }
 
-void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) {
+void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D, bool Inconsistent) {
   if (D->isInvalidDecl() || D->hasAttr())
 return;
   CXXMethodDecl *MD = dyn_cast(D);
@@ -3061,12 +3061,22 @@
   return;
 
   if (MD->size_overridden_methods() > 0) {
-unsigned DiagID = isa(MD)
-  ? diag::warn_destructor_marked_not_override_overriding
-  : diag::warn_function_marked_not_override_overriding;
-Diag(MD->getLocation(), DiagID) << MD->getDeclName();
-const CXXMethodDecl *OMD = *MD->begin_overridden_methods();
-Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
+auto EmitDiag = [&](unsigned DiagInconsistent, unsigned DiagSuggest) {
+  unsigned DiagID =
+  Inconsistent && !Diags.isIgnored(DiagInconsistent, MD->getLocation())
+  ? DiagInconsistent
+  : DiagSuggest;
+  Diag(MD->getLocation(), DiagID) << MD->getDeclName();
+  const CXXMethodDecl *OMD = *MD->begin_overridden_methods();
+  Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
+};
+if (isa(MD))
+  EmitDiag(
+  diag::warn_inconsistent_destructor_marked_not_override_overriding,
+  diag::warn_suggest_destructor_marked_not_override_overriding);
+else
+  EmitDiag(diag::warn_inconsistent_function_marked_not_override_overriding,
+   diag::warn_suggest_function_marked_not_override_overriding);
   }
 }
 
@@ -6749,13 +6759,10 @@
 }
   }
 
-  if (HasMethodWithOverrideControl &&
-  HasOverridingMethodWithoutOverrideControl) {
-// At least one method has the 'override' control declared.
-// Diagnose all other overridden methods which do not have 'override'
-// specified on them.
+  if (HasOverridingMethodWithoutOverrideControl) {
+bool HasInconsistentOverrideControl = HasMethodWithOverrideControl;
 for (auto *M : Record->methods())
-  DiagnoseAbsenceOfOverrideControl(M);
+  DiagnoseAbsenceOfOverrideControl(M, HasInconsistentOverrideControl);
   }
 
   // Check the defaulted secondary comparisons after any other member functions.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

2020-07-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

Hm, I think this is not quite right. For example, given:

  template struct X {};
  template auto f(T a, decltype(a)) {
struct A {};
struct B {};
return X();
  }
  decltype(f(0, 0)) g() {}

... I think we won't use a substitution from the first parameter of `f` in 
the second  back to the first parameter of `f` in the first 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83647



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

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

2020-07-12 Thread Ulrich Weigand via Phabricator via cfe-commits

uweigand added a comment.

In D80833#2123508 , @aganea wrote:

> In D80833#2109172 , @uweigand wrote:
>
> > Hmm, with clang-cl it seems the driver is trying to use this:
> >  Target: s390x-pc-windows-msvc
> >  which of course doesn't exist.  Not sure what is supposed to be happening 
> > here, but it seems that it's falling back on s390x-linux since on s390x, 
> > Linux is currently the only supported OS.
>
>
> I'm seeing some of the tests are setting the target explicitly `%clang_cl 
> --target=x86_64-windows-msvc`. Would that work on your machine? Or should I 
> do `UNSUPPORTED: s390x` ?


Sorry, looks like I missed this.  I think using an explicit target should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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

[PATCH] D82728: [clang] Add -Wsuggest-override

2020-07-12 Thread Logan Smith via Phabricator via cfe-commits

logan-5 added a comment.

In D82728#2146067 , @dblaikie wrote:

> Looks good - thanks for the patch and all the details! Might be worth turning 
> on by default in the LLVM build (after all the cleanup)

Thanks a lot! I don't (think I) have commit access yada yada, so I'd really 
appreciate any help getting this committed.

I've already got some of the cleanup in the works (D83611 
 and D83616 
), and was planning on taking care of the rest 
throughout the coming weekish. I'd be happy to turn this warning on in the LLVM 
build after that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82728

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

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

2020-07-12 Thread Dimitry Andric via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG02cfa7530d9e: Bump the default target CPU for i386-freebsd 
to i686 (authored by dim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83645

Files:
  clang/lib/Driver/ToolChains/Arch/X86.cpp


Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -94,6 +94,7 @@
 
   switch (Triple.getOS()) {
   case llvm::Triple::FreeBSD:
+return "i686";
   case llvm::Triple::NetBSD:
   case llvm::Triple::OpenBSD:
 return "i486";


Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -94,6 +94,7 @@
 
   switch (Triple.getOS()) {
   case llvm::Triple::FreeBSD:
+return "i686";
   case llvm::Triple::NetBSD:
   case llvm::Triple::OpenBSD:
 return "i486";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 02cfa75 - Bump the default target CPU for i386-freebsd to i686

2020-07-12 Thread Dimitry Andric via cfe-commits


Author: Dimitry Andric
Date: 2020-07-12T23:45:22+02:00
New Revision: 02cfa7530d9e7cfd8ea940dab4173afb7938b831

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

LOG: Bump the default target CPU for i386-freebsd to i686

Summary:
Similar to what we have done downstream, some time ago:
https://svnweb.freebsd.org/changeset/base/353936

This followed some discussions on the freebsd-arch mailing lists, and
most people agreed that it was a better default, and also it worked
around several issues where clang generated libcalls to 64 bit atomic
primitives, instead of using cmpxchg8b.

Reviewers: emaste, brooks, rsmith

Reviewed By: emaste

Subscribers: arichardson, krytarowski, jfb, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/X86.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp 
b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index aa95c4189d1e..2cc44c09917f 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -94,6 +94,7 @@ const char *x86::getX86TargetCPU(const ArgList ,
 
   switch (Triple.getOS()) {
   case llvm::Triple::FreeBSD:
+return "i686";
   case llvm::Triple::NetBSD:
   case llvm::Triple::OpenBSD:
 return "i486";



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

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

2020-07-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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

Per the ABI rules, substitutable components are symbolic constructs, not
the associated mangling character strings, so references to function
parameters or template parameters of distinct s should not be
considered substitutable even if they're at the same depth / index.

See https://github.com/itanium-cxx-abi/cxx-abi/issues/106.

This change can be turned off by setting -fclang-abi-compat to 10 or
earlier.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83647

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/mangle-abi-tag.cpp
  clang/test/CodeGenCXX/mangle-local-substitutions.cpp

Index: clang/test/CodeGenCXX/mangle-local-substitutions.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-local-substitutions.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-linux-gnu -fclang-abi-compat=10 | FileCheck %s --check-prefix=CHECK-OLD
+
+namespace multiple_encodings {
+  template auto f(T a, decltype(a)) { struct A {}; return A(); }
+  template auto g(T b, decltype(b)) { struct B {}; return B(); }
+  using A = decltype(f(0, 0));
+  using B = decltype(g(0.0, 0.0));
+  template struct X {};
+  // Note that we do not use backreferences for the repeated T_ and DtfL1p_E
+  // here; they are symbolically different because they refer to parameters of
+  // different function template specializiations.
+  // CHECK-NEW: define {{.*}}@_ZN18multiple_encodings1hENS_1XIJZNS_1fIiEEDaT_DtfL1p_EE1AZNS_1gIdEEDaT_DtfL1p_EE1BEEE(
+  // CHECK-OLD: define {{.*}}@_ZN18multiple_encodings1hENS_1XIJZNS_1fIiEEDaT_DtfL1p_EE1AZNS_1gIdEEDaS2_S3_E1BEEE(
+  void h(X) {}
+}
+
+namespace multiple_lambdas {
+  template struct X { X(T f) { f(0); } template void operator()(U...); };
+  template void f(T&&) { X{[](auto &&, auto...){}}(); }
+  // Note that we use OT_ for both lambdas here, rather than using a
+  // substitution for the second OT_. These are references to distinct template
+  // parameters, so are not substitutable.
+  // CHECK-NEW: call {{.*}}_ZN16multiple_lambdas1XIZNS_1fIiEEvOT_EUlOT_DpT0_E_EclIJEEEvDpT_(
+  // CHECK-OLD: call {{.*}}_ZN16multiple_lambdas1XIZNS_1fIiEEvOT_EUlS3_DpT0_E_EclIJEEEvDpT_(
+  void g() { f(0); }
+}
Index: clang/test/CodeGenCXX/mangle-abi-tag.cpp
===
--- clang/test/CodeGenCXX/mangle-abi-tag.cpp
+++ clang/test/CodeGenCXX/mangle-abi-tag.cpp
@@ -225,7 +225,9 @@
 template void g(F);
 template auto h(A ...a)->decltype (g (0, g < a > (a) ...)) {
 }
-// CHECK-DAG: define {{.*}} @_ZN7pr304401hIJEEEDTcl1gLi0Espcl1gIL_ZZNS_1hEDpT_E1aEEfp_EEES2_(
+// FIXME: This mangling is wrong. We should never refer to a function parameter by .
+// Should be:   @_ZN7pr304401hIJEEEDTcl1gLi0Espcl1gIXfp_EEfp_EEEDpT_
+// CHECK-DAG: define {{.*}} @_ZN7pr304401hIJEEEDTcl1gLi0Espcl1gIL_ZZNS_1hEDpT_E1aEEfp_EEEDpT_(
 
 void pr30440_test () {
   h();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3406,6 +3406,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
   else if (Major <= 9)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver9);
+  else if (Major <= 10)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver10);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -377,7 +377,11 @@
   AbiTagState *AbiTags = nullptr;
   AbiTagState AbiTagsRoot;
 
-  llvm::DenseMap Substitutions;
+  struct Substitution {
+unsigned SeqID;
+unsigned EncodingScope;
+  };
+  llvm::DenseMap Substitutions;
   llvm::DenseMap ModuleSubstitutions;
 
   ASTContext () const { return Context.getASTContext(); }
@@ -404,13 +408,19 @@
   : Context(Outer.Context), Out(Out_), NullOut(false),
 Structor(Outer.Structor), StructorType(Outer.StructorType),
 SeqID(Outer.SeqID), FunctionTypeDepth(Outer.FunctionTypeDepth),
-AbiTagsRoot(AbiTags), Substitutions(Outer.Substitutions) {}
+AbiTagsRoot(AbiTags), Substitutions(Outer.Substitutions),
+ModuleSubstitutions(Outer.ModuleSubstitutions),
+InnermostScope(Outer.InnermostScope),
+EncodingScopes(Outer.EncodingScopes) {}
 
   CXXNameMangler(CXXNameMangler ,

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

2020-07-12 Thread Ed Maste via Phabricator via cfe-commits

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

Fine with me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83645



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

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

2020-07-12 Thread Dimitry Andric via Phabricator via cfe-commits

dim created this revision.
dim added reviewers: emaste, brooks, rsmith.
Herald added subscribers: jfb, krytarowski, arichardson.
Herald added a project: clang.

Similar to what we have done downstream, some time ago:
https://svnweb.freebsd.org/changeset/base/353936

This followed some discussions on the freebsd-arch mailing lists, and
most people agreed that it was a better default, and also it worked
around several issues where clang generated libcalls to 64 bit atomic
primitives, instead of using cmpxchg8b.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83645

Files:
  clang/lib/Driver/ToolChains/Arch/X86.cpp


Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -94,6 +94,7 @@
 
   switch (Triple.getOS()) {
   case llvm::Triple::FreeBSD:
+return "i686";
   case llvm::Triple::NetBSD:
   case llvm::Triple::OpenBSD:
 return "i486";


Index: clang/lib/Driver/ToolChains/Arch/X86.cpp
===
--- clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -94,6 +94,7 @@
 
   switch (Triple.getOS()) {
   case llvm::Triple::FreeBSD:
+return "i686";
   case llvm::Triple::NetBSD:
   case llvm::Triple::OpenBSD:
 return "i486";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83564: [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

2020-07-12 Thread MyDeveloperDay via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG65dc97b79eb1: [clang-format] PR46609 clang-format does not 
obey `PointerAlignment: Right` for… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83564

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5325,7 +5325,7 @@
   verifyFormat("template  S(Ts...) -> S;");
   verifyFormat(
   "template \n"
-  "array(T &&... t) -> array, sizeof...(T)>;");
+  "array(T &&...t) -> array, sizeof...(T)>;");
   verifyFormat("template  A() -> Afoo<3>())>;");
   verifyFormat("template  A() -> A>)>;");
   verifyFormat("template  A() -> Afoo<1>)>;");
@@ -8179,13 +8179,20 @@
 }
 
 TEST_F(FormatTest, UnderstandsEllipsis) {
+  FormatStyle Style = getLLVMStyle();
   verifyFormat("int printf(const char *fmt, ...);");
   verifyFormat("template  void Foo(Ts... ts) { Foo(ts...); }");
-  verifyFormat("template  void Foo(Ts *... ts) {}");
+  verifyFormat("template  void Foo(Ts *...ts) {}");
+
+  verifyFormat("template  a;", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("template  void Foo(Ts*... ts) {}", Style);
+
+  verifyFormat("template  a;", Style);
 
-  FormatStyle PointersLeft = getLLVMStyle();
-  PointersLeft.PointerAlignment = FormatStyle::PAS_Left;
-  verifyFormat("template  void Foo(Ts*... ts) {}", PointersLeft);
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("template  a;", Style);
 }
 
 TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2844,6 +2844,11 @@
 Left.Previous &&
 !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
 tok::l_square));
+  // Ensure right pointer alignement with ellipsis e.g. int *...P
+  if (Left.is(tok::ellipsis) && Left.Previous &&
+  Left.Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+return Style.PointerAlignment != FormatStyle::PAS_Right;
+
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
   if (Left.is(tok::star) && Right.isOneOf(tok::star, tok::amp, tok::ampamp))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5325,7 +5325,7 @@
   verifyFormat("template  S(Ts...) -> S;");
   verifyFormat(
   "template \n"
-  "array(T &&... t) -> array, sizeof...(T)>;");
+  "array(T &&...t) -> array, sizeof...(T)>;");
   verifyFormat("template  A() -> Afoo<3>())>;");
   verifyFormat("template  A() -> A>)>;");
   verifyFormat("template  A() -> Afoo<1>)>;");
@@ -8179,13 +8179,20 @@
 }
 
 TEST_F(FormatTest, UnderstandsEllipsis) {
+  FormatStyle Style = getLLVMStyle();
   verifyFormat("int printf(const char *fmt, ...);");
   verifyFormat("template  void Foo(Ts... ts) { Foo(ts...); }");
-  verifyFormat("template  void Foo(Ts *... ts) {}");
+  verifyFormat("template  void Foo(Ts *...ts) {}");
+
+  verifyFormat("template  a;", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("template  void Foo(Ts*... ts) {}", Style);
+
+  verifyFormat("template  a;", Style);
 
-  FormatStyle PointersLeft = getLLVMStyle();
-  PointersLeft.PointerAlignment = FormatStyle::PAS_Left;
-  verifyFormat("template  void Foo(Ts*... ts) {}", PointersLeft);
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("template  a;", Style);
 }
 
 TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2844,6 +2844,11 @@
 Left.Previous &&
 !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
 tok::l_square));
+  // Ensure right pointer alignement with ellipsis e.g. int *...P
+  if (Left.is(tok::ellipsis) && Left.Previous &&
+  Left.Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+return Style.PointerAlignment != FormatStyle::PAS_Right;
+
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
   if (Left.is(tok::star) && Right.isOneOf(tok::star, tok::amp, tok::ampamp))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 65dc97b - [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

2020-07-12 Thread via cfe-commits


Author: mydeveloperday
Date: 2020-07-12T18:44:26+01:00
New Revision: 65dc97b79eb1979c54e7e17c411ea5f58f8dcc9c

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

LOG: [clang-format] PR46609 clang-format does not obey `PointerAlignment: 
Right` for ellipsis in declarator for pack

Summary:
https://bugs.llvm.org/show_bug.cgi?id=46609

Ensure `*...` obey they left/middle/right rules of Pointer alignment

Reviewed By: curdeius

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index a74015d3b4dc..7f8e35126512 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2844,6 +2844,11 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine ,
 Left.Previous &&
 !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
 tok::l_square));
+  // Ensure right pointer alignement with ellipsis e.g. int *...P
+  if (Left.is(tok::ellipsis) && Left.Previous &&
+  Left.Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
+return Style.PointerAlignment != FormatStyle::PAS_Right;
+
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
   if (Left.is(tok::star) && Right.isOneOf(tok::star, tok::amp, tok::ampamp))

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index ff9a64e81d5b..6ac3ffbffd1c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -5325,7 +5325,7 @@ TEST_F(FormatTest, DeductionGuides) {
   verifyFormat("template  S(Ts...) -> S;");
   verifyFormat(
   "template \n"
-  "array(T &&... t) -> array, sizeof...(T)>;");
+  "array(T &&...t) -> array, sizeof...(T)>;");
   verifyFormat("template  A() -> Afoo<3>())>;");
   verifyFormat("template  A() -> A>)>;");
   verifyFormat("template  A() -> Afoo<1>)>;");
@@ -8179,13 +8179,20 @@ TEST_F(FormatTest, AttributePenaltyBreaking) {
 }
 
 TEST_F(FormatTest, UnderstandsEllipsis) {
+  FormatStyle Style = getLLVMStyle();
   verifyFormat("int printf(const char *fmt, ...);");
   verifyFormat("template  void Foo(Ts... ts) { Foo(ts...); }");
-  verifyFormat("template  void Foo(Ts *... ts) {}");
+  verifyFormat("template  void Foo(Ts *...ts) {}");
+
+  verifyFormat("template  a;", Style);
+
+  Style.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("template  void Foo(Ts*... ts) {}", Style);
+
+  verifyFormat("template  a;", Style);
 
-  FormatStyle PointersLeft = getLLVMStyle();
-  PointersLeft.PointerAlignment = FormatStyle::PAS_Left;
-  verifyFormat("template  void Foo(Ts*... ts) {}", PointersLeft);
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("template  a;", Style);
 }
 
 TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {



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

[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

2020-07-12 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In D82087#2146170 , @sameerds wrote:

> >> https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_kernel_language.md#warp-vote-and-ballot-functions
> > 
> > I think if the language interface insists on fixing the wave size, then I 
> > think the correct solution is to implement this in the header based on a 
> > wave size macro (which we're desperately missing). The library 
> > implementation should be responsible for inserting the extension to 64-bit 
> > for wave32

Agree that FE should have a predefined macro for wave front size. Since it is 
per amdgpu target and not per language, it should be named as 
__amdgpu_wavefront_size__ or something similar, then it could be used by all 
languages.

Then we need to initialize warpSize in HIP header by this macro 
https://github.com/ROCm-Developer-Tools/HIP/blob/386a0e0123d67b95b4c0ebb3ebcf1d1615758146/include/hip/hcc_detail/device_functions.h#L300

I tends to think we should define __ballot in HIP header conditionally by the 
wavefront size so that the return type is correct for both wave32 and wave64 
mode. We should assume normal HIP compilation always have -mcpu specified so 
that wavefront size is known at compile time. If -mcpu is not specified 
probably we should not define warpSize or __ballot.

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

https://reviews.llvm.org/D82087

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

[PATCH] D83564: [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

2020-07-12 Thread Marek Kurdej via Phabricator via cfe-commits

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

LGTM! Thanks for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83564



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

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

2020-07-12 Thread David Zarzycki via Phabricator via cfe-commits

davezarzycki added a comment.

Hello. I have an auto-bisecting multi-stage bot that is failing on two after 
this change. Can we please revert this or commit a quick fix?

  FAIL: Clang :: CXX/class/class.compare/class.spaceship/p1.cpp (6232 of 64222)
   TEST 'Clang :: 
CXX/class/class.compare/class.spaceship/p1.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 1';   /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem 
/tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify 
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp 
-fcxx-exceptions
  --
  Exit Code: 134
  
  Command Output (stderr):
  --
  clang: /home/dave/s/lp/clang/lib/Basic/SourceManager.cpp:917: clang::FileID 
clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && 
"Invalid SLocOffset or bad function choice"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: /tmp/_update_lc/t/bin/clang -cc1 -internal-isystem 
/tmp/_update_lc/t/lib/clang/11.0.0/include -nostdsysteminc -std=c++2a -verify 
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp 
-fcxx-exceptions
  1.  
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:127:38:
 current parser token ','
  2.  
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:39:1: 
parsing namespace 'Deletedness'
  3.  
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:123:12:
 parsing function body 'Deletedness::g'
  4.  
/home/dave/s/lp/clang/test/CXX/class/class.compare/class.spaceship/p1.cpp:123:12:
 in compound statement ('{}')
   #0 0x0359273f llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(/tmp/_update_lc/t/bin/clang+0x359273f)
   #1 0x03590912 llvm::sys::RunSignalHandlers() 
(/tmp/_update_lc/t/bin/clang+0x3590912)
   #2 0x03592bb5 SignalHandler(int) 
(/tmp/_update_lc/t/bin/clang+0x3592bb5)
   #3 0x77fa6a90 __restore_rt (/lib64/libpthread.so.0+0x14a90)
   #4 0x77b3da25 raise (/lib64/libc.so.6+0x3ca25)
   #5 0x77b26895 abort (/lib64/libc.so.6+0x25895)
   #6 0x77b26769 _nl_load_domain.cold (/lib64/libc.so.6+0x25769)
   #7 0x77b35e86 (/lib64/libc.so.6+0x34e86)
   #8 0x0375636c clang::SourceManager::getFileIDLoaded(unsigned int) 
const (/tmp/_update_lc/t/bin/clang+0x375636c)
   #9 0x03ee0bbb 
clang::VerifyDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level,
 clang::Diagnostic const&) (/tmp/_update_lc/t/bin/clang+0x3ee0bbb)
  #10 0x037501ab 
clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const 
(/tmp/_update_lc/t/bin/clang+0x37501ab)
  #11 0x03749fca clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) 
(/tmp/_update_lc/t/bin/clang+0x3749fca)
  #12 0x04df0c60 clang::Sema::EmitCurrentDiagnostic(unsigned int) 
(/tmp/_update_lc/t/bin/clang+0x4df0c60)
  #13 0x05092783 (anonymous 
namespace)::DefaultedComparisonAnalyzer::visitBinaryOperator(clang::OverloadedOperatorKind,
 llvm::ArrayRef, (anonymous 
namespace)::DefaultedComparisonSubobject, clang::OverloadCandidateSet*) 
(/tmp/_update_lc/t/bin/clang+0x5092783)
  #14 0x05091dba (anonymous 
namespace)::DefaultedComparisonAnalyzer::visitExpandedSubobject(clang::QualType,
 (anonymous namespace)::DefaultedComparisonSubobject) 
(/tmp/_update_lc/t/bin/clang+0x5091dba)
  #15 0x05091b86 (anonymous 
namespace)::DefaultedComparisonVisitor<(anonymous 
namespace)::DefaultedComparisonAnalyzer, (anonymous 
namespace)::DefaultedComparisonInfo, (anonymous 
namespace)::DefaultedComparisonInfo, (anonymous 
namespace)::DefaultedComparisonSubobject>::visitSubobjects((anonymous 
namespace)::DefaultedComparisonInfo&, clang::CXXRecordDecl*, clang::Qualifiers) 
(/tmp/_update_lc/t/bin/clang+0x5091b86)
  #16 0x05058c8c (anonymous 
namespace)::DefaultedComparisonAnalyzer::visit() 
(/tmp/_update_lc/t/bin/clang+0x5058c8c)
  #17 0x0505ab22 
clang::Sema::DiagnoseDeletedDefaultedFunction(clang::FunctionDecl*) 
(/tmp/_update_lc/t/bin/clang+0x505ab22)
  #18 0x053e60ed 
clang::Sema::CreateOverloadedBinOp(clang::SourceLocation, 
clang::BinaryOperatorKind, clang::UnresolvedSetImpl const&, clang::Expr*, 
clang::Expr*, bool, bool, clang::FunctionDecl*) 
(/tmp/_update_lc/t/bin/clang+0x53e60ed)
  #19 0x0514270a BuildOverloadedBinOp(clang::Sema&, clang::Scope*, 
clang::SourceLocation, clang::BinaryOperatorKind, clang::Expr*, clang::Expr*) 
(/tmp/_update_lc/t/bin/clang+0x514270a)
  #20 0x050fbf49 clang::Sema::ActOnBinOp(clang::Scope*, 
clang::SourceLocation, clang::tok::TokenKind, clang::Expr*, clang::Expr*) 
(/tmp/_update_lc/t/bin/clang+0x50fbf49)
  #21 0x04d52ccc 
clang::Parser::ParseRHSOfBinaryExpression(clang::ActionResult, clang::prec::Level)

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

2020-07-12 Thread Ten Tzen via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66f1dcd872db: [Windows SEH] Fix the frame-ptr of a 
nested-filter within a _finally (authored by tentzen).

Changed prior to commit:
  https://reviews.llvm.org/D77982?vs=257227=277279#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77982

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/windows-seh-filter-inFinally.c

Index: clang/test/CodeGen/windows-seh-filter-inFinally.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-filter-inFinally.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-windows -fms-extensions -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[dst:[0-9-]+]] = call i8* @llvm.eh.recoverfp(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %frame_pointer)
+// CHECK-NEXT: %[[dst1:[0-9-]+]] = call i8* @llvm.localrecover(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %[[dst]], i32 0)
+// CHECK-NEXT: %[[dst2:[0-9-]+]] = bitcast i8* %[[dst1]] to i8**
+// CHECK-NEXT: = load i8*, i8** %[[dst2]], align 8
+
+int
+main(int argc, char *argv[])
+{
+int Counter = 0;
+//
+// Try/except within the finally clause of a try/finally.
+//
+__try {
+  Counter -= 1;
+}
+__finally {
+  __try {
+Counter += 2;
+// RtlRaiseStatus(STATUS_INTEGER_OVERFLOW);
+  } __except(Counter) {
+__try {
+  Counter += 3;
+}
+__finally {
+  if (abnormal_termination() == 1) {
+Counter += 5;
+  }
+}
+  }
+}
+// expect Counter == 9
+return 1;
+}
+
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -264,6 +264,9 @@
   CodeGenModule   // Per-module state.
   const TargetInfo 
 
+  // For EH/SEH outlined funclets, this field points to parent's CGF
+  CodeGenFunction *ParentCGF = nullptr;
+
   typedef std::pair ComplexPairTy;
   LoopInfoStack LoopStack;
   CGBuilderTy Builder;
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1815,6 +1815,48 @@
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
 ParentFP = Builder.CreateCall(RecoverFPIntrin, {ParentI8Fn, EntryFP});
+
+// if the parent is a _finally, the passed-in ParentFP is the FP
+// of parent _finally, not Establisher's FP (FP of outermost function).
+// Establkisher FP is 2nd paramenter passed into parent _finally.
+// Fortunately, it's always saved in parent's frame. The following
+// code retrieves it, and escapes it so that spill instruction won't be
+// optimized away.
+if (ParentCGF.ParentCGF != nullptr) {
+  // Locate and escape Parent's frame_pointer.addr alloca
+  // Depending on target, should be 1st/2nd one in LocalDeclMap.
+  // Let's just scan for ImplicitParamDecl with VoidPtrTy.
+  llvm::AllocaInst *FramePtrAddrAlloca = nullptr;
+  for (auto  : ParentCGF.LocalDeclMap) {
+const VarDecl *D = cast(I.first);
+if (isa(D) &&
+D->getType() == getContext().VoidPtrTy) {
+  assert(D->getName().startswith("frame_pointer"));
+  FramePtrAddrAlloca = cast(I.second.getPointer());
+  break;
+}
+  }
+  assert(FramePtrAddrAlloca);
+  auto InsertPair = ParentCGF.EscapedLocals.insert(
+  std::make_pair(FramePtrAddrAlloca, ParentCGF.EscapedLocals.size()));
+  int FrameEscapeIdx = InsertPair.first->second;
+
+  // an example of a filter's prolog::
+  // %0 = call i8* @llvm.eh.recoverfp(bitcast(@"?fin$0@0@main@@"),..)
+  // %1 = call i8* @llvm.localrecover(bitcast(@"?fin$0@0@main@@"),..)
+  // %2 = bitcast i8* %1 to i8**
+  // %3 = load i8*, i8* *%2, align 8
+  //   ==> %3 is the frame-pointer of outermost host function
+  llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::localrecover);
+  llvm::Constant *ParentI8Fn =
+  llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
+  ParentFP = Builder.CreateCall(
+  FrameRecoverFn, {ParentI8Fn, ParentFP,
+   llvm::ConstantInt::get(Int32Ty, FrameEscapeIdx)});
+  ParentFP = Builder.CreateBitCast(ParentFP, CGM.VoidPtrPtrTy);
+  ParentFP = Builder.CreateLoad(Address(ParentFP, getPointerAlign()));
+}
   }
 
   // Create llvm.localrecover calls for all captures.
@@ -2013,6 +2055,7 @@
 
 void CodeGenFunction::EnterSEHTryStmt(const SEHTryStmt ) {

[clang] 66f1dcd - [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

2020-07-12 Thread Ten Tzen via cfe-commits


Author: Ten Tzen
Date: 2020-07-12T01:37:56-07:00
New Revision: 66f1dcd872dba189ee054fb016f4bff535fb5afc

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

LOG: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

This change fixed a SEH bug (exposed by test58 & test61 in MSVC test xcpt4u.c);
when an Except-filter is located inside a finally, the frame-pointer generated 
today
via intrinsic @llvm.eh.recoverfp is the frame-pointer of the immediate
parent _finally, not the frame-ptr of outermost host function.

The fix is to retrieve the Establisher's frame-pointer that was previously 
saved in
parent's frame.
The prolog of a filter inside a _finally should be like code below:

%0 = call i8* @llvm.eh.recoverfp(i8* bitcast (@"?fin$0@0@main@@"), 
i8*%frame_pointer)
%1 = call i8* @llvm.localrecover(i8* bitcast (@"?fin$0@0@main@@"), i8*%0, i32 0)
%2 = bitcast i8* %1 to i8**
%3 = load i8*, i8** %2, align 8

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

Added: 
clang/test/CodeGen/windows-seh-filter-inFinally.c

Modified: 
clang/lib/CodeGen/CGException.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGException.cpp 
b/clang/lib/CodeGen/CGException.cpp
index 2494f38b3159..bdf70252b5ad 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -1815,6 +1815,48 @@ void CodeGenFunction::EmitCapturedLocals(CodeGenFunction 
,
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
 ParentFP = Builder.CreateCall(RecoverFPIntrin, {ParentI8Fn, EntryFP});
+
+// if the parent is a _finally, the passed-in ParentFP is the FP
+// of parent _finally, not Establisher's FP (FP of outermost function).
+// Establkisher FP is 2nd paramenter passed into parent _finally.
+// Fortunately, it's always saved in parent's frame. The following
+// code retrieves it, and escapes it so that spill instruction won't be
+// optimized away.
+if (ParentCGF.ParentCGF != nullptr) {
+  // Locate and escape Parent's frame_pointer.addr alloca
+  // Depending on target, should be 1st/2nd one in LocalDeclMap.
+  // Let's just scan for ImplicitParamDecl with VoidPtrTy.
+  llvm::AllocaInst *FramePtrAddrAlloca = nullptr;
+  for (auto  : ParentCGF.LocalDeclMap) {
+const VarDecl *D = cast(I.first);
+if (isa(D) &&
+D->getType() == getContext().VoidPtrTy) {
+  assert(D->getName().startswith("frame_pointer"));
+  FramePtrAddrAlloca = cast(I.second.getPointer());
+  break;
+}
+  }
+  assert(FramePtrAddrAlloca);
+  auto InsertPair = ParentCGF.EscapedLocals.insert(
+  std::make_pair(FramePtrAddrAlloca, ParentCGF.EscapedLocals.size()));
+  int FrameEscapeIdx = InsertPair.first->second;
+
+  // an example of a filter's prolog::
+  // %0 = call i8* @llvm.eh.recoverfp(bitcast(@"?fin$0@0@main@@"),..)
+  // %1 = call i8* @llvm.localrecover(bitcast(@"?fin$0@0@main@@"),..)
+  // %2 = bitcast i8* %1 to i8**
+  // %3 = load i8*, i8* *%2, align 8
+  //   ==> %3 is the frame-pointer of outermost host function
+  llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::localrecover);
+  llvm::Constant *ParentI8Fn =
+  llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
+  ParentFP = Builder.CreateCall(
+  FrameRecoverFn, {ParentI8Fn, ParentFP,
+   llvm::ConstantInt::get(Int32Ty, FrameEscapeIdx)});
+  ParentFP = Builder.CreateBitCast(ParentFP, CGM.VoidPtrPtrTy);
+  ParentFP = Builder.CreateLoad(Address(ParentFP, getPointerAlign()));
+}
   }
 
   // Create llvm.localrecover calls for all captures.
@@ -2013,6 +2055,7 @@ void CodeGenFunction::pushSEHCleanup(CleanupKind Kind,
 
 void CodeGenFunction::EnterSEHTryStmt(const SEHTryStmt ) {
   CodeGenFunction HelperCGF(CGM, /*suppressNewContext=*/true);
+  HelperCGF.ParentCGF = this;
   if (const SEHFinallyStmt *Finally = S.getFinallyHandler()) {
 // Outline the finally block.
 llvm::Function *FinallyFunc =

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index b1841d646643..1fc2ed76ca9e 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -264,6 +264,9 @@ class CodeGenFunction : public CodeGenTypeCache {
   CodeGenModule   // Per-module state.
   const TargetInfo 
 
+  // For EH/SEH outlined funclets, this field points to parent's CGF
+  CodeGenFunction *ParentCGF = nullptr;
+
   typedef std::pair ComplexPairTy;
   LoopInfoStack LoopStack;
   CGBuilderTy Builder;

diff  --git

[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

2020-07-12 Thread Sameer Sahasrabuddhe via Phabricator via cfe-commits

sameerds added a comment.

>> https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_kernel_language.md#warp-vote-and-ballot-functions
> 
> I think if the language interface insists on fixing the wave size, then I 
> think the correct solution is to implement this in the header based on a wave 
> size macro (which we're desperately missing). The library implementation 
> should be responsible for inserting the extension to 64-bit for wave32

Not sure if the frontend should try to infer warpsize and the mask size, or 
even whether it can in all cases. But this can result in wrong behaviour when 
the program passes 32-bit mask but then gets compiled for a 64-bit mask. It's 
easy to say that the programmer must not assume a warp-size, but it would be 
useful if the language can

In D82087#2144712 , @arsenm wrote:

> In D82087#2140778 , @sameerds wrote:
>
> > The documentation for HIP __ballot seems to indicate that the user does not 
> > have to explicitly specify the warp size. How is that achieved with these 
> > new builtins? Can this  be captured in a lit test here?
>
>
> This seems like a defect in the design to me
>
> > https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_kernel_language.md#warp-vote-and-ballot-functions


I tend to agree. The HIP vote/ballot builtins are also missing a mask 
parameter, whose type needs to match the wavesize.


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

https://reviews.llvm.org/D82087



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

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[clang] c943329 - Revert "Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression"

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

[clang] 49e5f60 - Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[clang] b4dbb37 - [X86] Rename X86_CPU_TYPE_COMPAT_ALIAS/X86_CPU_TYPE_COMPAT/X86_CPU_SUBTYPE_COMPAT macros. NFC

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

[clang] 1111678 - [clang] Add -Wsuggest-override

[PATCH] D82728: [clang] Add -Wsuggest-override

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

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

[PATCH] D82728: [clang] Add -Wsuggest-override

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

[clang] 02cfa75 - Bump the default target CPU for i386-freebsd to i686

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

[PATCH] D83645: Bump the default target CPU for i386-freebsd to i686

[PATCH] D83564: [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

[clang] 65dc97b - [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

[PATCH] D83564: [clang-format] PR46609 clang-format does not obey `PointerAlignment: Right` for ellipsis in declarator for pack

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[clang] 66f1dcd - [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

30 matches

Site Navigation

Mail list logo

Footer information