[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

2021-08-28 Thread Owen Pan via Phabricator via cfe-commits 
owenpan updated this revision to Diff 369308.
owenpan added a comment.

Reworked the logic for handling the backward compatibility.


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

https://reviews.llvm.org/D108882

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@
   PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
   CHECK_PARSE("PackConstructorInitializers: NextLine",
   PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  // For backward compatibility:
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: true",
+  PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
 
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   CHECK_PARSE("EmptyLineBeforeAccessModifier: Never",
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -668,17 +668,19 @@
 IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
OnCurrentLine);
 IO.mapOptional("AllowAllConstructorInitializersOnNextLine", OnNextLine);
-if (IsGoogleOrChromium &&
-Style.PackConstructorInitializers == FormatStyle::PCIS_NextLine) {
+if (!IsGoogleOrChromium) {
+  if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack &&
+  OnCurrentLine) {
+Style.PackConstructorInitializers = OnNextLine
+? FormatStyle::PCIS_NextLine
+: 
FormatStyle::PCIS_CurrentLine;
+  }
+} else if (Style.PackConstructorInitializers ==
+   FormatStyle::PCIS_NextLine) {
   if (!OnCurrentLine)
 Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   else if (!OnNextLine)
 Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
-} else if (Style.PackConstructorInitializers == FormatStyle::PCIS_BinPack 
&&
-   OnCurrentLine) {
-  Style.PackConstructorInitializers = OnNextLine
-  ? FormatStyle::PCIS_NextLine
-  : FormatStyle::PCIS_CurrentLine;
 }
 
 IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@
   PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
   CHECK_PARSE("PackConstructorInitializers: NextLine",
   PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  // For backward compatibility:
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: true",
+  PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
 
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;

[PATCH] D108886: Add RISC-V sifive-s51 cpu

2021-08-28 Thread Alexander Pivovarov via Phabricator via cfe-commits 
apivovarov created this revision.
apivovarov added reviewers: evandro, kito-cheng, khchen.
Herald added subscribers: vkmr, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
apivovarov requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

Add RISC-V sifive-s51 cpu.

sifive-51 spec: https://www.sifive.com/cores/s51
gcc: 
https://github.com/riscv/riscv-gcc/blob/c3911e6425f35e0722129cb30cc5ccaf3390cd75/gcc/config/riscv/riscv-cores.def#L42


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108886

Files:
  clang/test/Driver/riscv-cpus.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/lib/Target/RISCV/RISCV.td


Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -254,6 +254,11 @@
  FeatureStdExtA,
  FeatureStdExtC]>;
 
+def : ProcessorModel<"sifive-s51", RocketModel, [Feature64Bit,
+ FeatureStdExtM,
+ FeatureStdExtA,
+ FeatureStdExtC]>;
+
 def : ProcessorModel<"sifive-u54", RocketModel, [Feature64Bit,
  FeatureStdExtM,
  FeatureStdExtA,
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- llvm/include/llvm/Support/RISCVTargetParser.def
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -20,6 +20,7 @@
 PROC(SIFIVE_732, {"sifive-7-rv32"}, FK_NONE, {""})
 PROC(SIFIVE_764, {"sifive-7-rv64"}, FK_64BIT, {""})
 PROC(SIFIVE_E31, {"sifive-e31"}, FK_NONE, {"rv32imac"})
+PROC(SIFIVE_S51, {"sifive-s51"}, FK_64BIT, {"rv64imac"})
 PROC(SIFIVE_U54, {"sifive-u54"}, FK_64BIT, {"rv64gc"})
 PROC(SIFIVE_E76, {"sifive-e76"}, FK_NONE, {"rv32imafc"})
 PROC(SIFIVE_U74, {"sifive-u74"}, FK_64BIT, {"rv64gc"})
Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -45,6 +45,13 @@
 // RUN: %clang -target riscv64 -### -c %s 2>&1 -mtune=sifive-7-series | 
FileCheck -check-prefix=MTUNE-SIFIVE7-SERIES-64 %s
 // MTUNE-SIFIVE7-SERIES-64: "-tune-cpu" "sifive-7-rv64"
 
+// mcpu with mabi option
+// RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=sifive-s51 -mabi=lp64 | 
FileCheck -check-prefix=MCPU-ABI-SIFIVE-S51 %s
+// MCPU-SIFIVE-S51: "-nostdsysteminc" "-target-cpu" "sifive-s51"
+// MCPU-SIFIVE-S51: "-target-feature" "+m" "-target-feature" "+a"
+// MCPU-SIFIVE-S51: "-target-feature" "+c" "-target-feature" "+64bit"
+// MCPU-SIFIVE-S51: "-target-abi" "lp64"
+
 // mcpu with default march
 // RUN: %clang -target riscv64 -### -c %s 2>&1 -mcpu=sifive-u54 | FileCheck 
-check-prefix=MCPU-SIFIVE-U54 %s
 // MCPU-SIFIVE-U54: "-nostdsysteminc" "-target-cpu" "sifive-u54"


Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -254,6 +254,11 @@
  FeatureStdExtA,
  FeatureStdExtC]>;
 
+def : ProcessorModel<"sifive-s51", RocketModel, [Feature64Bit,
+ FeatureStdExtM,
+ FeatureStdExtA,
+ FeatureStdExtC]>;
+
 def : ProcessorModel<"sifive-u54", RocketModel, [Feature64Bit,
  FeatureStdExtM,
  FeatureStdExtA,
Index: llvm/include/llvm/Support/RISCVTargetParser.def
===
--- llvm/include/llvm/Support/RISCVTargetParser.def
+++ llvm/include/llvm/Support/RISCVTargetParser.def
@@ -20,6 +20,7 @@
 PROC(SIFIVE_732, {"sifive-7-rv32"}, FK_NONE, {""})
 PROC(SIFIVE_764, {"sifive-7-rv64"}, FK_64BIT, {""})
 PROC(SIFIVE_E31, {"sifive-e31"}, FK_NONE, {"rv32imac"})
+PROC(SIFIVE_S51, {"sifive-s51"}, FK_64BIT, {"rv64imac"})
 PROC(SIFIVE_U54, {"sifive-u54"}, FK_64BIT, {"rv64gc"})
 PROC(SIFIVE_E76, {"sifive-e76"}, FK_NONE, {"rv32imafc"})
 PROC(SIFIVE_U74, {"sifive-u74"}, FK_64BIT, {"rv64gc"})
Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -45,6 +45,13 @@
 // RUN: %clang -target riscv64 -### -c %s 2>&1 -mtune=sif

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

2021-08-28 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1975
+  bool AlignAttrCanDecreaseAlignment =
+  AlignIsRequired && (Ty->getAs() != nullptr || FieldPacked);
+

stevewan wrote:
> rjmccall wrote:
> > stevewan wrote:
> > > rjmccall wrote:
> > > > Okay, so first off, the comment and variable names here make this sound 
> > > > very general when in fact this computation is only relevant to the AIX 
> > > > alignment upgrade logic.  Also, please do not introduce new variables 
> > > > with broad scope named things like `Ty` that are actually referring to 
> > > > a very specific type and not, say, the unadulterated type of the field.
> > > > 
> > > > It seems to me that the right way to think about this is that, while 
> > > > the AIX power alignment upgrade considers whether field type alignment 
> > > > is "required", it uses a different condition than the standard rule 
> > > > when making that decision.  It's important to understand what that rule 
> > > > is exactly, and we can't see that from the current test cases.  In 
> > > > particular, attributes on fields that define structs inline actually 
> > > > end up applying to both the field and the struct, which confounds 
> > > > unrelated issues.  Consider:
> > > > 
> > > > ```
> > > > struct __attribute__((packed, aligned(2))) SS {
> > > >   double d;
> > > > };
> > > > 
> > > > struct S {
> > > >   // Neither the field nor the struct are packed, but the field type is.
> > > >   // Do we apply the alignment upgrade to S or not?
> > > >   struct SS s;
> > > > };
> > > > ```
> > > > 
> > > > Regardless, I don't think this check for a typedef works; it's 
> > > > bypassing quite a bit of recursive logic for computing whether type 
> > > > alignment is required.  For example, you could have an explicitly 
> > > > aligned typedef of an array type, and you'll lose that typedef here.
> > > Thanks for the review. 
> > > 
> > > > the comment and variable names here make this sound very general when 
> > > > in fact this computation is only relevant to the AIX alignment upgrade 
> > > > logic.
> > > Moved the variable declarations back to the AIX alignment upgrade logic. 
> > > >  It's important to understand what that rule is exactly, and we can't 
> > > > see that from the current test cases.
> > > Updated the test cases accordingly.
> > > > For example, you could have an explicitly aligned typedef of an array 
> > > > type, and you'll lose that typedef here.
> > > Updated the check to examine the immediate type rather than the base 
> > > element type, this should take care of the array-type considerations. 
> > > 
> > No, I'm not sure that's quite correct, either, because you might have an 
> > array of an explicitly aligned typedef.  You might need to do a recursive 
> > examination of the type the same way that getTypeInfo would, except I guess 
> > not recursing into records.
> > 
> > I assume you verified that the behavior in your new test cases matches the 
> > behavior of some existing AIX compiler?  For our general edification, what 
> > compiler(s) are you testing against?
> > 
> > I've found that extracting very complex conditions into a function (perhaps 
> > a lambda if it's inconvenient to have it separately-declared) can be useful.
> Added a lambda function that mimics getTypeInfo to do the recursive check. If 
> this looks about right in direction, I'm also going to memorize known types' 
> typedef-ness in a map for faster lookup.
> 
> Yes, the behaviour was verified, and matches the behaviour of XL 16.1 on AIX.
Somehow my requests didn't make it through.  Please name this lambda something 
AIX-specific, and please clarify in a comment that we can ignore enum alignment 
sources because the alignment upgrade only applies to floating-point types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

2021-08-28 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

Thanks, this looks a lot cleaner.  Minor requests only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

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

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

2021-08-28 Thread Steven Wan via Phabricator via cfe-commits 
stevewan updated this revision to Diff 369293.
stevewan added a comment.

Adapt to new TypeInfo design


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
  clang/test/Layout/aix-power-alignment-typedef.cpp
  clang/test/Layout/aix-type-align-and-pack-attr.cpp

Index: clang/test/Layout/aix-type-align-and-pack-attr.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-type-align-and-pack-attr.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+struct __attribute__((__aligned__(2))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test1{{.*}}s{{.*}} = global %"struct.test1::S" zeroinitializer, align 8
+} // namespace test1
+
+namespace test2 {
+struct __attribute__((__aligned__(2), packed)) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test2{{.*}}s{{.*}} = global %"struct.test2::S" zeroinitializer, align 2
+} // namespace test2
+
+namespace test3 {
+struct __attribute__((__aligned__(16))) S {
+  double d;
+};
+
+S s;
+
+// CHECK: @{{.*}}test3{{.*}}s{{.*}} = global %"struct.test3::S" zeroinitializer, align 16
+} // namespace test3
+
+namespace test4 {
+struct __attribute__((aligned(2))) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test4{{.*}}s{{.*}} = global %"struct.test4::S" zeroinitializer, align 8
+} // namespace test4
+
+namespace test5 {
+struct __attribute__((aligned(2), packed)) SS {
+  double d;
+};
+
+struct S {
+  struct SS ss;
+} s;
+
+// CHECK: @{{.*}}test5{{.*}}s{{.*}} = global %"struct.test5::S" zeroinitializer, align 2
+} // namespace test5
Index: clang/test/Layout/aix-power-alignment-typedef.cpp
===
--- clang/test/Layout/aix-power-alignment-typedef.cpp
+++ clang/test/Layout/aix-power-alignment-typedef.cpp
@@ -37,3 +37,39 @@
 // CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
 
 } // namespace test2
+
+namespace test3 {
+typedef double DblArr[] __attribute__((__aligned__(2)));
+
+union U {
+  DblArr da;
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:  0 | union test3::U
+// CHECK-NEXT: 0 |   test3::DblArr da
+// CHECK-NEXT: 0 |   char x
+// CHECK-NEXT:   | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test3
+
+namespace test4 {
+typedef double Dbl __attribute__((__aligned__(2)));
+
+union U {
+  Dbl DblArr[];
+  char x;
+};
+
+int x = sizeof(U);
+
+// CHECK:  0 | union test4::U
+// CHECK-NEXT: 0 |   test4::Dbl [] DblArr
+// CHECK-NEXT: 0 |   char x
+// CHECK-NEXT:   | [sizeof=2, dsize=2, align=2, preferredalign=2,
+// CHECK-NEXT:   |  nvsize=2, nvalign=2, preferrednvalign=2]
+
+} // namespace test4
Index: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
===
--- clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s | \
-// RUN:   FileCheck %s
-
-namespace test1 {
-struct __attribute__((__aligned__(2))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test1{{.*}}c{{.*}} = global %"struct.test1::C" zeroinitializer, align 8
-} // namespace test1
-
-namespace test2 {
-struct __attribute__((__aligned__(2), packed)) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global %"struct.test2::C" zeroinitializer, align 2
-} // namespace test2
-
-namespace test3 {
-struct __attribute__((__aligned__(16))) C {
-  double x;
-} c;
-
-// CHECK: @{{.*}}test3{{.*}}c{{.*}} = global %"struct.test3::C" zeroinitializer, align 16
-} // namespace test3
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1968,6 +1968,16 @@
 }
   }
 
+  // When used as part of a typedef, or together with a 'packed' attribute, the
+  // 'aligned' attribute can be used to decrease alignment. In that case, it
+  // overrides any computed alignment we have, and there is no need to upgrade
+  // the alignment.
+  auto alignedAttrCanDecreaseAlignment = [AlignRequirement, FieldPacked] {
+return AlignRequirement == AlignRequirementType::RequiredByTypedef ||
+   (AlignRequirement == AlignRequirementType::RequiredByRecord &&
+FieldPacked);
+  };
+
   // The AIX `power` ali

[PATCH] D108858: TypeInfo records more information about align requirement

2021-08-28 Thread Steven Wan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG73733ae526a5: TypeInfo records more information about align 
requirement (authored by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108858

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1839,7 +1839,7 @@
 
 // Pass over-aligned aggregates on Windows indirectly. This behavior was
 // added in MSVC 2015.
-if (IsWin32StructABI && TI.AlignIsRequired && TI.Align > 32)
+if (IsWin32StructABI && TI.isAlignRequired() && TI.Align > 32)
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 
 // Expand small (<= 128-bit) record types when we know that the stack layout
@@ -6992,7 +6992,7 @@
 TyAlignForABI = CharUnits::fromQuantity(4);
   }
 
-  TypeInfoChars TyInfo(TySize, TyAlignForABI, false);
+  TypeInfoChars TyInfo(TySize, TyAlignForABI, AlignRequirementKind::None);
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect, TyInfo,
   SlotSize, /*AllowHigherAlign*/ true);
 }
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -847,7 +847,7 @@
 // arguments was not supported and resulted in a compiler error. In 19.14
 // and later versions, such arguments are now passed indirectly.
 TypeInfo Info = getContext().getTypeInfo(RD->getTypeForDecl());
-if (Info.AlignIsRequired && Info.Align > 4)
+if (Info.isAlignRequired() && Info.Align > 4)
   return RAA_Indirect;
 
 // If C++ prohibits us from making a copy, construct the arguments directly
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -52,7 +52,7 @@
 
 static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) {
   auto TI = Ctx.getTypeInfo(Ty);
-  return TI.AlignIsRequired ? TI.Align : 0;
+  return TI.isAlignRequired() ? TI.Align : 0;
 }
 
 static uint32_t getTypeAlignIfRequired(QualType Ty, const ASTContext &Ctx) {
@@ -4676,7 +4676,7 @@
 llvm::DIType *fieldType;
 if (capture->isByRef()) {
   TypeInfo PtrInfo = C.getTypeInfo(C.VoidPtrTy);
-  auto Align = PtrInfo.AlignIsRequired ? PtrInfo.Align : 0;
+  auto Align = PtrInfo.isAlignRequired() ? PtrInfo.Align : 0;
   // FIXME: This recomputes the layout of the BlockByRefWrapper.
   uint64_t xoffset;
   fieldType =
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1538,7 +1538,7 @@
   TypeInfo FieldInfo = Context.getTypeInfo(D->getType());
   uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
-  bool AlignIsRequired = FieldInfo.AlignIsRequired;
+  bool AlignIsRequired = FieldInfo.isAlignRequired();
 
   // UnfilledBitsInLastUnit is the difference between the end of the
   // last allocated bitfield (i.e. the first bit offset available for
@@ -1889,7 +1889,7 @@
 
   bool FieldPacked = Packed || D->hasAttr();
 
-  bool AlignIsRequired = false;
+  AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   CharUnits FieldSize;
   CharUnits FieldAlign;
   // The amount of this class's dsize occupied by the field.
@@ -1904,7 +1904,7 @@
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
 IsIncompleteArrayType ? CharUnits::Zero() : TI.Width;
-AlignIsRequired = TI.AlignIsRequired;
+AlignRequirement = TI.AlignRequirement;
   };
 
   if (D->getType()->isIncompleteArrayType()) {
@@ -1978,7 +1978,8 @@
   // and zero-width bit-fields count as prior members; members of empty class
   // types marked `no_unique_address` are not considered to be prior members.
   CharUnits PreferredAlign = FieldAlign;
-  if (DefaultsToAIXPowerAlignment && !AlignIsRequired &&
+  if (DefaultsToAIXPowerAlignment &&
+  AlignRequirement == AlignRequirementKind::None &&
   (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
 auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
   if (BTy->getKind() == BuiltinType::Double ||
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1858

[clang] 73733ae - TypeInfo records more information about align requirement

2021-08-28 Thread Steven Wan via cfe-commits 

Author: Steven Wan
Date: 2021-08-28T19:47:48-04:00
New Revision: 73733ae526a5152e5427973ac12dc54f7dd243fb

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

LOG: TypeInfo records more information about align requirement

Extend the information preserved in `TypeInfo` by replacing the 
`AlignIsRequired` bool flag with a three-valued enum, the enum also indicates 
where the alignment attribute come from, which could be helpful in determining 
whether the attribute should overrule.

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index da372e854700b..5004cb6cb2671 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -164,24 +164,46 @@ namespace serialization {
 template  class AbstractTypeReader;
 } // namespace serialization
 
+enum class AlignRequirementKind {
+  /// The alignment was not explicit in code.
+  None,
+
+  /// The alignment comes from an alignment attribute on a typedef.
+  RequiredByTypedef,
+
+  /// The alignment comes from an alignment attribute on a record type.
+  RequiredByRecord,
+
+  /// The alignment comes from an alignment attribute on a enum type.
+  RequiredByEnum,
+};
+
 struct TypeInfo {
   uint64_t Width = 0;
   unsigned Align = 0;
-  bool AlignIsRequired : 1;
+  AlignRequirementKind AlignRequirement;
 
-  TypeInfo() : AlignIsRequired(false) {}
-  TypeInfo(uint64_t Width, unsigned Align, bool AlignIsRequired)
-  : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {}
+  TypeInfo() : AlignRequirement(AlignRequirementKind::None) {}
+  TypeInfo(uint64_t Width, unsigned Align,
+   AlignRequirementKind AlignRequirement)
+  : Width(Width), Align(Align), AlignRequirement(AlignRequirement) {}
+  bool isAlignRequired() {
+return AlignRequirement != AlignRequirementKind::None;
+  }
 };
 
 struct TypeInfoChars {
   CharUnits Width;
   CharUnits Align;
-  bool AlignIsRequired : 1;
+  AlignRequirementKind AlignRequirement;
 
-  TypeInfoChars() : AlignIsRequired(false) {}
-  TypeInfoChars(CharUnits Width, CharUnits Align, bool AlignIsRequired)
-  : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {}
+  TypeInfoChars() : AlignRequirement(AlignRequirementKind::None) {}
+  TypeInfoChars(CharUnits Width, CharUnits Align,
+AlignRequirementKind AlignRequirement)
+  : Width(Width), Align(Align), AlignRequirement(AlignRequirement) {}
+  bool isAlignRequired() {
+return AlignRequirement != AlignRequirementKind::None;
+  }
 };
 
 /// Holds long-lived AST nodes (such as types and decls) that can be

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 12b5d14f35c63..41dd9f7e3330b 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1858,7 +1858,7 @@ static getConstantArrayInfoInChars(const ASTContext 
&Context,
 Width = llvm::alignTo(Width, Align);
   return TypeInfoChars(CharUnits::fromQuantity(Width),
CharUnits::fromQuantity(Align),
-   EltInfo.AlignIsRequired);
+   EltInfo.AlignRequirement);
 }
 
 TypeInfoChars ASTContext::getTypeInfoInChars(const Type *T) const {
@@ -1866,8 +1866,7 @@ TypeInfoChars ASTContext::getTypeInfoInChars(const Type 
*T) const {
 return getConstantArrayInfoInChars(*this, CAT);
   TypeInfo Info = getTypeInfo(T);
   return TypeInfoChars(toCharUnitsFromBits(Info.Width),
-   toCharUnitsFromBits(Info.Align),
-   Info.AlignIsRequired);
+   toCharUnitsFromBits(Info.Align), Info.AlignRequirement);
 }
 
 TypeInfoChars ASTContext::getTypeInfoInChars(QualType T) const {
@@ -1875,7 +1874,7 @@ TypeInfoChars ASTContext::getTypeInfoInChars(QualType T) 
const {
 }
 
 bool ASTContext::isAlignmentRequired(const Type *T) const {
-  return getTypeInfo(T).AlignIsRequired;
+  return getTypeInfo(T).AlignRequirement != AlignRequirementKind::None;
 }
 
 bool ASTContext::isAlignmentRequired(QualType T) const {
@@ -1927,7 +1926,7 @@ TypeInfo ASTContext::getTypeInfo(const Type *T) const {
 TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
   uint64_t Width = 0;
   unsigned Align = 8;
-  bool AlignIsRequired = false;
+  AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   unsigned AS = 0;
   switch (T->getTypeClass()) {
 #define TYPE(Class, Base)
@@ -1961,7 +1960,7 @@ TypeIn

[PATCH] D108752: [clang-format] Group options that pack constructor initializers

2021-08-28 Thread Owen Pan via Phabricator via cfe-commits 
owenpan marked an inline comment as done.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:18472
+  FormatStyle::PCIS_Never);
+
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Should there be a test for the mapping?
> Ideally yes, but `CHECK_PARSE` can't check mappings of multiple to one, e.g.:
> ```
>   CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
>   "AllowAllConstructorInitializersOnNextLine: false",
>   PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
> ```
Added the tests in https://reviews.llvm.org/D108882.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108752

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

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

2021-08-28 Thread Owen Pan via Phabricator via cfe-commits 
owenpan created this revision.
owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks.
owenpan added a project: clang-format.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108882

Files:
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@
   PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
   CHECK_PARSE("PackConstructorInitializers: NextLine",
   PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  // For backward compatibility:
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: true",
+  PackConstructorInitializers, FormatStyle::PCIS_NextLine);
 
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   CHECK_PARSE("EmptyLineBeforeAccessModifier: Never",


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@
   PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
   CHECK_PARSE("PackConstructorInitializers: NextLine",
   PackConstructorInitializers, FormatStyle::PCIS_NextLine);
+  // For backward compatibility:
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
+  CHECK_PARSE("BasedOnStyle: Google\n"
+  "ConstructorInitializerAllOnOneLineOrOnePerLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_BinPack);
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: false",
+  PackConstructorInitializers, FormatStyle::PCIS_CurrentLine);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
+  CHECK_PARSE("ConstructorInitializerAllOnOneLineOrOnePerLine: true\n"
+  "AllowAllConstructorInitializersOnNextLine: true",
+  PackConstructorInitializers, FormatStyle::PCIS_NextLine);
 
   Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
   CHECK_PARSE("EmptyLineBeforeAccessModifier: Never",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108881: [clang][driver] Honor the last -flto= flag even if an earlier -flto is present

2021-08-28 Thread Usman Nadeem via Phabricator via cfe-commits 
mnadeem added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4495-4497
+Arg *A = Args.getLastArg(options::OPT_flto, options::OPT_flto_EQ);
+if (A && A->getOption().matches(options::OPT_flto))
   CmdArgs.push_back("-flto");

Another option would be to do the following, but i am not sure if there is any 
code that explicitly checks for/needs "=full":

```
if (D.getLTOMode() == LTOK_Thin)
CmdArgs.push_back("-flto=thin");
else
CmdArgs.push_back("-flto");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108881

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

[PATCH] D108881: [clang][driver] Honor the last -flto= flag even if an earlier -flto is present

2021-08-28 Thread Usman Nadeem via Phabricator via cfe-commits 
mnadeem created this revision.
mnadeem added a project: clang.
Herald added subscribers: ormris, steven_wu, hiraditya, inglorion.
mnadeem requested review of this revision.
Herald added a subscriber: cfe-commits.

After D102479  `-flto=` options the end of 
the command line were being ignored if an earlier `-flto` was present.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108881

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/lto.c


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,15 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 
| \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO %s
+//
+// FLTO-THIN: -flto=thin
+// FLTO-FULL: -flto=full
+// FLTO: "-flto"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4492,14 +4492,13 @@
 
 if (IsUsingLTO) {
   if (!IsDeviceOffloadAction) {
-if (Args.hasArg(options::OPT_flto))
+Arg *A = Args.getLastArg(options::OPT_flto, options::OPT_flto_EQ);
+if (A && A->getOption().matches(options::OPT_flto))
   CmdArgs.push_back("-flto");
-else {
-  if (D.getLTOMode() == LTOK_Thin)
-CmdArgs.push_back("-flto=thin");
-  else
-CmdArgs.push_back("-flto=full");
-}
+else if (D.getLTOMode() == LTOK_Thin)
+  CmdArgs.push_back("-flto=thin");
+else
+  CmdArgs.push_back("-flto=full");
 CmdArgs.push_back("-flto-unit");
   } else if (Triple.isAMDGPU()) {
 // Only AMDGPU supports device-side LTO
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -600,8 +600,9 @@
 
   StringRef LTOName("full");
 
-  const Arg *A = Args.getLastArg(OptEq);
-  if (A)
+  const Arg *A = Args.getLastArg(OptEq, OptPos);
+  // Use the OptEq only if it is after the -flto option.
+  if (A && A->getOption().matches(OptEq))
 LTOName = A->getValue();
 
   LTOMode = llvm::StringSwitch(LTOName)


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -85,3 +85,15 @@
 // FLTO-AUTO: -flto=full
 // FLTO-JOBSERVER: -flto=full
 //
+
+// Pass the last -flto argument
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto -flto=thin 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-THIN %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto=full 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO-FULL %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto 2>&1 | \
+// RUN: FileCheck --check-prefix=FLTO %s
+//
+// FLTO-THIN: -flto=thin
+// FLTO-FULL: -flto=full
+// FLTO: "-flto"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4492,14 +4492,13 @@
 
 if (IsUsingLTO) {
   if (!IsDeviceOffloadAction) {
-if (Args.hasArg(options::OPT_flto))
+Arg *A = Args.getLastArg(options::OPT_flto, options::OPT_flto_EQ);
+if (A && A->getOption().matches(options::OPT_flto))
   CmdArgs.push_back("-flto");
-else {
-  if (D.getLTOMode() == LTOK_Thin)
-CmdArgs.push_back("-flto=thin");
-  else
-CmdArgs.push_back("-flto=full");
-}
+else if (D.getLTOMode() == LTOK_Thin)
+  CmdArgs.push_back("-flto=thin");
+else
+  CmdArgs.push_back("-flto=full");
 CmdArgs.push_back("-flto-unit");
   } else if (Triple.isAMDGPU()) {
 // Only AMDGPU supports device-side LTO
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -600,8 +600,9 @@
 
   StringRef LTOName("full");
 
-  const Arg *A = Args.getLastArg(OptEq);
-  if (A)
+  const Arg *A = Args.getLastArg(OptEq, OptPos);
+  // Use the OptEq only if it is after the -flto option.
+  if (A && A->getOption().matches(OptEq))
 LTOName = A->getValue();
 
   LTOMode = llvm::StringSwitch(LTOName)
___
cfe-commits mailing list
cfe-commits@lists.

[PATCH] D108858: TypeInfo records more information about align requirement

2021-08-28 Thread John McCall via Phabricator via cfe-commits 
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108858

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

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

2021-08-28 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

Thanks for the updates! LGTM from my side.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:555
+  // Iterate over the types of the function parameters.
+  // If any of them are non-const reference paramteres, add it as a
+  // highlighting modifier to the corresponding expression

nit: still a typo here ("paramteres")



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:562
+// Is this parameter passed by non-const reference?
+// FIXME The condition !T->idDependentType() could be relaxed a bit,
+// e.g. std::vector& is dependent but we would want to highlight it

nit: typo (idDependentType)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108320

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

[PATCH] D108858: TypeInfo records more information about align requirement

2021-08-28 Thread Steven Wan via Phabricator via cfe-commits 
stevewan updated this revision to Diff 369258.
stevewan added a comment.

Add RequiredByEnum case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108858

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1839,7 +1839,7 @@
 
 // Pass over-aligned aggregates on Windows indirectly. This behavior was
 // added in MSVC 2015.
-if (IsWin32StructABI && TI.AlignIsRequired && TI.Align > 32)
+if (IsWin32StructABI && TI.isAlignRequired() && TI.Align > 32)
   return getIndirectResult(Ty, /*ByVal=*/false, State);
 
 // Expand small (<= 128-bit) record types when we know that the stack layout
@@ -6992,7 +6992,7 @@
 TyAlignForABI = CharUnits::fromQuantity(4);
   }
 
-  TypeInfoChars TyInfo(TySize, TyAlignForABI, false);
+  TypeInfoChars TyInfo(TySize, TyAlignForABI, AlignRequirementKind::None);
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect, TyInfo,
   SlotSize, /*AllowHigherAlign*/ true);
 }
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -847,7 +847,7 @@
 // arguments was not supported and resulted in a compiler error. In 19.14
 // and later versions, such arguments are now passed indirectly.
 TypeInfo Info = getContext().getTypeInfo(RD->getTypeForDecl());
-if (Info.AlignIsRequired && Info.Align > 4)
+if (Info.isAlignRequired() && Info.Align > 4)
   return RAA_Indirect;
 
 // If C++ prohibits us from making a copy, construct the arguments directly
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -52,7 +52,7 @@
 
 static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) {
   auto TI = Ctx.getTypeInfo(Ty);
-  return TI.AlignIsRequired ? TI.Align : 0;
+  return TI.isAlignRequired() ? TI.Align : 0;
 }
 
 static uint32_t getTypeAlignIfRequired(QualType Ty, const ASTContext &Ctx) {
@@ -4676,7 +4676,7 @@
 llvm::DIType *fieldType;
 if (capture->isByRef()) {
   TypeInfo PtrInfo = C.getTypeInfo(C.VoidPtrTy);
-  auto Align = PtrInfo.AlignIsRequired ? PtrInfo.Align : 0;
+  auto Align = PtrInfo.isAlignRequired() ? PtrInfo.Align : 0;
   // FIXME: This recomputes the layout of the BlockByRefWrapper.
   uint64_t xoffset;
   fieldType =
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1538,7 +1538,7 @@
   TypeInfo FieldInfo = Context.getTypeInfo(D->getType());
   uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
-  bool AlignIsRequired = FieldInfo.AlignIsRequired;
+  bool AlignIsRequired = FieldInfo.isAlignRequired();
 
   // UnfilledBitsInLastUnit is the difference between the end of the
   // last allocated bitfield (i.e. the first bit offset available for
@@ -1889,7 +1889,7 @@
 
   bool FieldPacked = Packed || D->hasAttr();
 
-  bool AlignIsRequired = false;
+  AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   CharUnits FieldSize;
   CharUnits FieldAlign;
   // The amount of this class's dsize occupied by the field.
@@ -1904,7 +1904,7 @@
 // aligned appropriately for their element type.
 EffectiveFieldSize = FieldSize =
 IsIncompleteArrayType ? CharUnits::Zero() : TI.Width;
-AlignIsRequired = TI.AlignIsRequired;
+AlignRequirement = TI.AlignRequirement;
   };
 
   if (D->getType()->isIncompleteArrayType()) {
@@ -1978,7 +1978,8 @@
   // and zero-width bit-fields count as prior members; members of empty class
   // types marked `no_unique_address` are not considered to be prior members.
   CharUnits PreferredAlign = FieldAlign;
-  if (DefaultsToAIXPowerAlignment && !AlignIsRequired &&
+  if (DefaultsToAIXPowerAlignment &&
+  AlignRequirement == AlignRequirementKind::None &&
   (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
 auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
   if (BTy->getKind() == BuiltinType::Double ||
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1858,7 +1858,7 @@
 Width = llvm::alignTo(Width, Align);
   return TypeInfoChars(CharUnits::

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

2021-08-28 Thread Marco Gartmann via Phabricator via cfe-commits 
mgartmann added a comment.

@aaron.ballman Thanks a lot for your review!

Can somebody help me merging this change? I do not have the rights to do so.
Thanks in advanve!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[PATCH] D108742: [WIP] Reclassify form-feed and vertical tab as vertical WS for the purposes of lexing.

2021-08-28 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

In D108742#2970302 , @rsmith wrote:

> In D108742#2970283 , @cor3ntin 
> wrote:
>
>>> Drive-by observation: under P2348 , Clang's 
>>> behavior of treating `\n\r` as a single new-line would be "non-standard" 
>>> (requiring special phase 1 mapping). Is that intentional? `\n\r` is used as 
>>> a new-line character on old Mac systems.
>>
>> Somewhat. `\n\r` is not described by Unicode so we could either mandate that 
>> all implementation support that or leave it as implementation-defined 
>> mapping. Correct me if I am wrong, but as the line number is itself 
>> implementation-defined, whether there are one or 2 line breaks would not 
>> materially affect the standard, either way.
>
> Yes, I suppose that's true. Though if we're nailing down exactly how new 
> lines are defined and asking every conforming implementation to support UTF-8 
> and such, maybe it's time to also define how the presumed line number is 
> determined? =)

I'm not sure that we want people to rely on the value on __LINE__, to be honest.

Anyway, I've been thinking a lot about that and I don't think this change is 
worth pursuing. I've realized all compilers treat VT and FF as non-line 
breaking horizontal whitespace and even if that's not consistent with Unicode 
it's also consistent with other programming languages.
I'll modify the paper to keep treating these codepoints as horizontal 
whitespaces as this seems the sane, non-disruptive course of action.

I will also try to address your feedback regarding acorn systems on the paper.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108742

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

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

2021-08-28 Thread Tom Praschan via Phabricator via cfe-commits 
tom-anders updated this revision to Diff 369249.
tom-anders marked an inline comment as done.
tom-anders added a comment.

- Use llvm::SmallVector, add more FIXME, remove old commented out test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108320

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

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -729,6 +729,47 @@
   }
 };
   )cpp",
+  // Modifier for variables passed as non-const references
+  R"cpp(
+void $Function_decl[[fun]](int, const int,
+   int*, const int*,
+   int&, const int&,
+   int*&, const int*&, const int* const &,
+   int**, int**&, int** const &,
+   int = 123) {
+  int $LocalVariable_decl[[val]];
+  int* $LocalVariable_decl[[ptr]];
+  const int* $LocalVariable_decl_readonly[[constPtr]];
+  int** $LocalVariable_decl[[array]];
+  $Function[[fun]]($LocalVariable[[val]], $LocalVariable[[val]], 
+   $LocalVariable[[ptr]], $LocalVariable_readonly[[constPtr]], 
+   $LocalVariable_usedAsMutableReference[[val]], $LocalVariable[[val]], 
+
+   $LocalVariable_usedAsMutableReference[[ptr]],
+   $LocalVariable_readonly_usedAsMutableReference[[constPtr]],
+   $LocalVariable_readonly[[constPtr]],
+
+   $LocalVariable[[array]], $LocalVariable_usedAsMutableReference[[array]], 
+   $LocalVariable[[array]]
+   );
+}
+struct $Class_decl[[S]] {
+  $Class_decl[[S]](int&) {
+$Class[[S]] $LocalVariable_decl[[s1]]($Field_usedAsMutableReference[[field]]);
+$Class[[S]] $LocalVariable_decl[[s2]]($LocalVariable[[s1]].$Field_usedAsMutableReference[[field]]);
+
+$Class[[S]] $LocalVariable_decl[[s3]]($StaticField_static_usedAsMutableReference[[staticField]]);
+$Class[[S]] $LocalVariable_decl[[s4]]($Class[[S]]::$StaticField_static_usedAsMutableReference[[staticField]]);
+  }
+  int $Field_decl[[field]];
+  static int $StaticField_decl_static[[staticField]];
+};
+template 
+void $Function_decl[[foo]]($TemplateParameter[[X]]& $Parameter_decl[[x]]) {
+  // We do not support dependent types, so this one should *not* get the modifier.
+  $Function[[foo]]($Parameter[[x]]); 
+}
+  )cpp",
   };
   for (const auto &TestCase : TestCases)
 // Mask off scope modifiers to keep the tests manageable.
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- clang-tools-extra/clangd/test/semantic-tokens.test
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  4097
+# CHECK-NEXT:  8193
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "1"
 # CHECK-NEXT:  }
@@ -49,7 +49,7 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  4097
+# CHECK-NEXT:  8193
 # CHECK-NEXT:],
 #Inserted at position 1
 # CHECK-NEXT:"deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  4097,
+# CHECK-NEXT:  8193,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  4,
 # CHECK-NEXT:  1,
 # CHECK-NEXT:  0,
-# CHECK-NEXT:  4097
+# CHECK-NEXT:  8193
 # CHECK-NEXT:],
 # CHECK-NEXT:"resultId": "3"
 # CHECK-NEXT:  }
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -91,6 +91,7 @@
 # CHECK-NEXT:"virtual",
 # CHECK-NEXT:"dependentName",
 # CHECK-NEXT:"defaultLibrary",
+# CHECK-NEXT:"usedAsMutableReference",
 # CHECK-NEXT:"functionScope",
 # CHECK-NEXT:"classScope",
 # CHECK-NEXT:"fileScope",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
=

[PATCH] D108320: Add semantic token modifier for non-const reference parameter

2021-08-28 Thread Tom Praschan via Phabricator via cfe-commits 
tom-anders marked 5 inline comments as done.
tom-anders added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:470
   std::vector Tokens;
+  std::map> ExtraModifiers;
   const HeuristicResolver *Resolver;

nridge wrote:
> Looking at existing usage of associative containers in clangd code, 
> `llvm::DenseMap` and `llvm::DenseSet` (which are hashtable-based) are more 
> commonly used than `std::map` and `std::set`, though it may require some 
> boilerplate to teach them to use new key types 
> ([example](https://searchfox.org/llvm/rev/2556f58148836f0af3ad2c73fe54bbdf49f0295a/clang-tools-extra/clangd/index/dex/Token.h#117))
> 
> We could also consider making the value just a vector instead of a set (maybe 
> even an `llvm::SmallVector` given its anticipated 
> usage), since even in the unlikely case that we get duplicates, 
> `addModifier()` will handle them correctly
Agreed, changed std::set to llvm::SmallVector. I can't really judge if it's 
worth using llvm::DenseMap here though.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:773
+  )cpp",
+/*struct $Class_decl[[S]] {
+  $Class_decl[[S]](int $Parameter_decl[[value]], const int& 
$Parameter_decl_readonly[[constRef]], 

nridge wrote:
> (test case needs uncommenting)
Actually it needed to be removed, has been replaced by the test above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108320

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

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

2021-08-28 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:23
+  plurals = {
+'IncludeCategory': 'IncludeCategories'
+  }

FederAndInk wrote:
> HazardyKnusperkeks wrote:
> > Could you not just check if there is a y at the end and replace it with 
> > ies, otherweise add an s?
> Well, I thought about it, but then what about: whish -> whishes, leaf -> 
> leaves, ... and irregulars? That's why I brought up the idea about using 
> python inflect. Do you think it's enough for now to replace y -> ies and put 
> an 's' to the others?
I'm okay with either way, in both cases there comes a time where someone must 
pay attention to add something here. We just have to look carefully in the 
review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108765

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