[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

[MyDeveloperDay via Phabricator via cfe-commits]

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

It seems a little convoluted, but OK


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

https://reviews.llvm.org/D108882

___
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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



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

HazardyKnusperkeks wrote:
> 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.
it would be nice if in the event of a missing plural it complained.


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

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

[Steven Wan via Phabricator via cfe-commits]

stevewan updated this revision to Diff 369319.
stevewan added a comment.

Rename lambda and add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  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,19 @@
 }
   }
 
+  // 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 alignedAttrCanDecreaseAIXAlignment = [AlignRequirement, FieldPacked] {
+// Enum alignment sources can be safely ignored here, because this only
+// helps decide whether we need the AIX alignment upgrade, which only
+// applies to floating-point types.
+return AlignRequirement == AlignRequir

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

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson closed this revision.
tejohnson added a comment.

This was committed awhile back (d3f1f588f902a968f102d6cdaf052674efc257aa 
), 
manually closing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86958

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd created this revision.
compnerd added a reviewer: aaron.ballman.
Herald added a subscriber: mgorny.
compnerd requested review of this revision.
Herald added a project: clang-tools-extra.

This introduces a new check, readability-containter-data-pointer.  This
check is meant to catch the cases where the user may be trying to
materialize the data pointer by taking the address of the 0-th member of
a container.  With C++11 or newer, the `data` member should be used for
this.  This provides the following benefits:

- `.data()` is easier to read than `&[0]`
- it avoids an unnecessary re-materialization of the pointer
  - this doesn't matter in the case of optimized code, but in the case of 
unoptimized code, this will be visible
- it avoids a potential invalid memory de-reference caused by the indexing when 
the container is empty (in debug mode, clang will normally optimize away the 
re-materialization in optimized builds).

The small potential behavioural change raises the question of where the
check should belong.  A reasoning of defense in depth applies here, and
this does an unchecked conversion, with the assumption that users can
use the static analyzer to catch cases where we can statically identify
an invalid memory de-reference.  For the cases where the static analysis
is unable to prove the size of the container, UBSan can be used to track
the invalid access.

Special thanks to Aaron Ballman for the discussion on whether this
check would be useful and where to place it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_t);
+  const T &operator[](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f(&b[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector &v) {
+  f(&v[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f(&b[0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]: warning: 'data' s

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 369331.
compnerd added a comment.

Reflow the text using clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -fno-delayed-template-parsing
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_t);
+  const T &operator[](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f(&b[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector &v) {
+  f(&v[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f(&b[0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+}
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ConstReturnTypeCheck.h"
+#include "ContainerDataPointerCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ConvertMemberFunctionsToStatic.h"
 #include "DeleteNullPointerCheck.h"
@@ -62,6 +63,8 @@
 "readability-braces-around-statements");
 CheckFactories.registerCheck(
 "readability-const-return-type");
+CheckFactories.registerCheck(
+"readability-container-data-pointer");
 CheckFactories.registerCheck(
 "readability-container-size-empty");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -0,0 +1,45 @@
+//===--- ContainerDataPoin

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

[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/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] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

[Alexandre Rames via Phabricator via cfe-commits]

arames added inline comments.



Comment at: clang/include/clang/Serialization/ModuleFileExtension.h:89
+  using ExtensionHashBuilder =
+  llvm::HashBuilderImpl;
+  virtual void hashExtension(ExtensionHashBuilder &HBuilder) const;

This obviously needs to be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102943

___
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

[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'
+  }

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > 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.
> it would be nice if in the event of a missing plural it complained.
But that would mean we have to add each plural here. So every new list, which 
is not of strings, would most likely also needed to be added here. I think that 
is too much, but then again there are not that much lists, so maybe it's worth 
the work.


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

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D108882#2971192 , @MyDeveloperDay 
wrote:

> It seems a little convoluted, but OK

Will add comments to explain the logic.


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

https://reviews.llvm.org/D108882

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

[clang] 4b1fde8 - [clang-format] Add PackConstructorInitializers backward compat test

[via cfe-commits]

Author: owenca
Date: 2021-08-29T13:47:11-07:00
New Revision: 4b1fde8a2b681dad2ce0c082a5d6422caa06b0bc

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

LOG: [clang-format] Add PackConstructorInitializers backward compat test

Add backward compatibility tests for mapping the deprecated
ConstructorInitializerAllOnOneLineOrOnePerLine and
AllowAllConstructorInitializersOnNextLine to
PackConstructorInitializers.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 78a92aa542000..ecc6d3f3a47e4 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -658,7 +658,14 @@ template <> struct MappingTraits {
 
 IO.mapOptional("PackConstructorInitializers",
Style.PackConstructorInitializers);
-// For backward compatibility.
+// For backward compatibility:
+// The default value of ConstructorInitializerAllOnOneLineOrOnePerLine was
+// false unless BasedOnStyle was Google or Chromium whereas that of
+// AllowAllConstructorInitializersOnNextLine was always true, so the
+// equivalent default value of PackConstructorInitializers is PCIS_NextLine
+// for Google/Chromium or PCIS_BinPack otherwise. If the deprecated options
+// had a non-default value while PackConstructorInitializers has a default
+// value, set the latter to an equivalent non-default value if needed.
 StringRef BasedOn;
 IO.mapOptional("BasedOnStyle", BasedOn);
 const bool IsGoogleOrChromium = BasedOn.equals_insensitive("google") ||
@@ -668,17 +675,19 @@ template <> struct MappingTraits {
 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);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 4ca3a22a76c8e..45ed0f9de7d36 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -18467,6 +18467,22 @@ TEST_F(FormatTest, ParsesConfiguration) {
   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"

[PATCH] D108882: Add backward compatibility tests for PackConstructorInitializers

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b1fde8a2b68: [clang-format] Add PackConstructorInitializers 
backward compat test (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D108882?vs=369308&id=369339#toc

Repository:
  rG LLVM Github Monorepo

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
@@ -658,7 +658,14 @@
 
 IO.mapOptional("PackConstructorInitializers",
Style.PackConstructorInitializers);
-// For backward compatibility.
+// For backward compatibility:
+// The default value of ConstructorInitializerAllOnOneLineOrOnePerLine was
+// false unless BasedOnStyle was Google or Chromium whereas that of
+// AllowAllConstructorInitializersOnNextLine was always true, so the
+// equivalent default value of PackConstructorInitializers is PCIS_NextLine
+// for Google/Chromium or PCIS_BinPack otherwise. If the deprecated options
+// had a non-default value while PackConstructorInitializers has a default
+// value, set the latter to an equivalent non-default value if needed.
 StringRef BasedOn;
 IO.mapOptional("BasedOnStyle", BasedOn);
 const bool IsGoogleOrChromium = BasedOn.equals_insensitive("google") ||
@@ -668,17 +675,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 compatib

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Thank you for implementing 26817 ! 
But shouldn't this check belong to `modernize` module?

Please add documentation and mention new check in Release Notes.




Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18
+namespace tidy {
+namespace readability {
+ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:117
+  << Hint;
+}
+} // namespace readability

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:17
+namespace readability {
+/// Checks whether a call to `operator[]` and `&` can be replaced with a call 
to
+/// `data()`.

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:40
+  }
+};
+} // namespace readability

Please separate with empty line.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:45
+
+#endif

Please add inclusion guard in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 369340.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_t);
+  const T &operator[](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f(&b[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector &v) {
+  f(&v[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f(&b[0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+}
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "AvoidConstParamsInDecls.h"
 #include "BracesAroundStatementsCheck.h"
 #include "ConstReturnTypeCheck.h"
+#include "ContainerDataPointerCheck.h"
 #include "ContainerSizeEmptyCheck.h"
 #include "ConvertMemberFunctionsToStatic.h"
 #include "DeleteNullPointerCheck.h"
@@ -62,6 +63,8 @@
 "readability-braces-around-statements");
 CheckFactories.registerCheck(
 "readability-const-return-type");
+CheckFactories.registerCheck(
+"readability-container-data-pointer");
 CheckFactories.registerCheck(
 "readability-container-size-empty");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
@@ -0,0 +1,45 @@
+//===--- ContainerDataPointerCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// Part of the LLVM Projec

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

In D108893#2971410 , @Eugene.Zelenko 
wrote:

> Thank you for implementing 26817 
> ! But shouldn't this check 
> belong to `modernize` module?

Oh, I was unaware of the PR, I'll tag that in the commit message, thanks!

Hmm, I'm somewhat on the fence, but I wouldn't be against the reorganization to 
`modernize`, only I would prefer that we get that settled before I do the 
actual rename/move.

> Please add documentation and mention new check in Release Notes.

Ah, good idea, I'll add that as well.




Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18
+namespace tidy {
+namespace readability {
+ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,

Eugene.Zelenko wrote:
> Please separate with empty line.
Hmm, this is what clang-format does, which should be unambiguously correct.  Is 
there something that needs to be changed in clang-format or .clang-format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18
+namespace tidy {
+namespace readability {
+ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,

compnerd wrote:
> Eugene.Zelenko wrote:
> > Please separate with empty line.
> Hmm, this is what clang-format does, which should be unambiguously correct.  
> Is there something that needs to be changed in clang-format or .clang-format?
Hard to tell about current state of clang-format, but you could take a look on 
existing checks code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:18
+namespace tidy {
+namespace readability {
+ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,

Eugene.Zelenko wrote:
> compnerd wrote:
> > Eugene.Zelenko wrote:
> > > Please separate with empty line.
> > Hmm, this is what clang-format does, which should be unambiguously correct. 
> >  Is there something that needs to be changed in clang-format or 
> > .clang-format?
> Hard to tell about current state of clang-format, but you could take a look 
> on existing checks code.
I would prefer that we fix the clang-format configuration rather than manually 
change the formatting here.  This is introducing a new set of files, so it 
isn't resulting in inconsistency in a single file.  I don't mind reformatting 
the code with clang-format, but generally, that is considered the definitive 
arbiter for formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Hmm, one case that doesn't currently get handled properly is the following test 
case:

  c++
  template  void f(const T *);
  void g(const std::vector **v) {
f(&(**v)[0]);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 369343.
compnerd edited the summary of this revision.
compnerd added a comment.

Add some documentation and release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_t);
+  const T &operator[](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f(&b[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector &v) {
+  f(&v[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f(&b[0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-data-pointer
+
+readability-data-pointer
+
+
+Finds instances of the data pointer being materialized by taking the address of
+the element at index 0 of a container. In particular, ``std::vector`` and
+``std::string`` provide a ``data()`` accessor to retrieve the data pointer which
+should be preferred.
+
+This also ensures that in the case that the container is empty, the data pointer
+access does not perform an errant memory access.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -82,6 +82,10 @@
 
   Reports identifiers whose names are too short. Currently checks local variab

[clang] 83e82ff - [X86] Support __SSC_MARK(const int id)

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2021-08-30T08:51:20+08:00
New Revision: 83e82ff767530158fd5590ffea617f50a07534b5

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

LOG: [X86] Support __SSC_MARK(const int id)

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

Added: 
clang/test/CodeGen/X86/x86-ssc-mark.c

Modified: 
clang/lib/Headers/x86gprintrin.h

Removed: 




diff  --git a/clang/lib/Headers/x86gprintrin.h 
b/clang/lib/Headers/x86gprintrin.h
index 1fc6cab4b28f..327ccb724be8 100644
--- a/clang/lib/Headers/x86gprintrin.h
+++ b/clang/lib/Headers/x86gprintrin.h
@@ -20,4 +20,9 @@
 #include 
 #endif
 
+#define __SSC_MARK(Tag)
\
+  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
+   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
+   : "%eax");
+
 #endif /* __X86GPRINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/x86-ssc-mark.c 
b/clang/test/CodeGen/X86/x86-ssc-mark.c
new file mode 100644
index ..cbadc11b6347
--- /dev/null
+++ b/clang/test/CodeGen/X86/x86-ssc-mark.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple=x86_64-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=i386-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
+
+#include 
+
+// The ebx may be use for base pointer, we need to restore it in time.
+void ssc_mark() {
+// CHECK-LABEL: ssc_mark
+// CHECK: #APP
+// CHECK: movl%ebx, %eax
+// CHECK: movl$0, %ebx
+// CHECK: .byte   100
+// CHECK: .byte   103
+// CHECK: .byte   144
+// CHECK: movl%eax, %ebx
+// CHECK: #NO_APP
+
+  __SSC_MARK(0x0);
+}



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

[PATCH] D108682: [X86] Support __SSC_MARK(const int id) in x86gprintrin.h

[Xiang Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83e82ff76753: [X86] Support __SSC_MARK(const int id) 
(authored by xiangzhangllvm).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108682

Files:
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/X86/x86-ssc-mark.c


Index: clang/test/CodeGen/X86/x86-ssc-mark.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/x86-ssc-mark.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple=x86_64-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=i386-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
+
+#include 
+
+// The ebx may be use for base pointer, we need to restore it in time.
+void ssc_mark() {
+// CHECK-LABEL: ssc_mark
+// CHECK: #APP
+// CHECK: movl%ebx, %eax
+// CHECK: movl$0, %ebx
+// CHECK: .byte   100
+// CHECK: .byte   103
+// CHECK: .byte   144
+// CHECK: movl%eax, %ebx
+// CHECK: #NO_APP
+
+  __SSC_MARK(0x0);
+}
Index: clang/lib/Headers/x86gprintrin.h
===
--- clang/lib/Headers/x86gprintrin.h
+++ clang/lib/Headers/x86gprintrin.h
@@ -20,4 +20,9 @@
 #include 
 #endif
 
+#define __SSC_MARK(Tag)
\
+  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
+   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
+   : "%eax");
+
 #endif /* __X86GPRINTRIN_H */


Index: clang/test/CodeGen/X86/x86-ssc-mark.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/x86-ssc-mark.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple=x86_64-unknow-unknow -S -ffreestanding -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=i386-unknow-unknow -S -ffreestanding -o - | FileCheck %s
+
+#include 
+
+// The ebx may be use for base pointer, we need to restore it in time.
+void ssc_mark() {
+// CHECK-LABEL: ssc_mark
+// CHECK: #APP
+// CHECK: movl%ebx, %eax
+// CHECK: movl$0, %ebx
+// CHECK: .byte   100
+// CHECK: .byte   103
+// CHECK: .byte   144
+// CHECK: movl%eax, %ebx
+// CHECK: #NO_APP
+
+  __SSC_MARK(0x0);
+}
Index: clang/lib/Headers/x86gprintrin.h
===
--- clang/lib/Headers/x86gprintrin.h
+++ clang/lib/Headers/x86gprintrin.h
@@ -20,4 +20,9 @@
 #include 
 #endif
 
+#define __SSC_MARK(Tag)\
+  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " \
+   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   \
+   : "%eax");
+
 #endif /* __X86GPRINTRIN_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fd88fac - Revert "[X86] Support __SSC_MARK(const int id)"

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2021-08-30T09:18:27+08:00
New Revision: fd88fac6ca3967e85906dfd059f512c0cee3fdaf

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

LOG: Revert "[X86] Support __SSC_MARK(const int id)"

This reverts commit 83e82ff767530158fd5590ffea617f50a07534b5.

Added: 


Modified: 
clang/lib/Headers/x86gprintrin.h

Removed: 
clang/test/CodeGen/X86/x86-ssc-mark.c



diff  --git a/clang/lib/Headers/x86gprintrin.h 
b/clang/lib/Headers/x86gprintrin.h
index 327ccb724be80..1fc6cab4b28fc 100644
--- a/clang/lib/Headers/x86gprintrin.h
+++ b/clang/lib/Headers/x86gprintrin.h
@@ -20,9 +20,4 @@
 #include 
 #endif
 
-#define __SSC_MARK(Tag)
\
-  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
-   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
-   : "%eax");
-
 #endif /* __X86GPRINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/x86-ssc-mark.c 
b/clang/test/CodeGen/X86/x86-ssc-mark.c
deleted file mode 100644
index cbadc11b6347a..0
--- a/clang/test/CodeGen/X86/x86-ssc-mark.c
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 %s -triple=x86_64-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple=i386-unknow-unknow -S -ffreestanding -o - | 
FileCheck %s
-
-#include 
-
-// The ebx may be use for base pointer, we need to restore it in time.
-void ssc_mark() {
-// CHECK-LABEL: ssc_mark
-// CHECK: #APP
-// CHECK: movl%ebx, %eax
-// CHECK: movl$0, %ebx
-// CHECK: .byte   100
-// CHECK: .byte   103
-// CHECK: .byte   144
-// CHECK: movl%eax, %ebx
-// CHECK: #NO_APP
-
-  __SSC_MARK(0x0);
-}



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

[clang] 78fbde5 - [X86] Support __SSC_MARK(const int id)

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2021-08-30T09:21:22+08:00
New Revision: 78fbde57794e50f5629979f5d69592caf64067e3

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

LOG: [X86] Support __SSC_MARK(const int id)

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

Added: 
clang/test/CodeGen/X86/x86-ssc-mark.c

Modified: 
clang/lib/Headers/x86gprintrin.h

Removed: 




diff  --git a/clang/lib/Headers/x86gprintrin.h 
b/clang/lib/Headers/x86gprintrin.h
index 1fc6cab4b28fc..327ccb724be80 100644
--- a/clang/lib/Headers/x86gprintrin.h
+++ b/clang/lib/Headers/x86gprintrin.h
@@ -20,4 +20,9 @@
 #include 
 #endif
 
+#define __SSC_MARK(Tag)
\
+  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
+   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
+   : "%eax");
+
 #endif /* __X86GPRINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/x86-ssc-mark.c 
b/clang/test/CodeGen/X86/x86-ssc-mark.c
new file mode 100644
index 0..ce036aaadf410
--- /dev/null
+++ b/clang/test/CodeGen/X86/x86-ssc-mark.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=i386-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
+
+#include 
+
+// The ebx may be use for base pointer, we need to restore it in time.
+void ssc_mark() {
+// CHECK-LABEL: ssc_mark
+// CHECK: #APP
+// CHECK: movl%ebx, %eax
+// CHECK: movl$0, %ebx
+// CHECK: .byte   100
+// CHECK: .byte   103
+// CHECK: .byte   144
+// CHECK: movl%eax, %ebx
+// CHECK: #NO_APP
+
+  __SSC_MARK(0x0);
+}



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

[clang] 71b170c - [AIX] "aligned" attribute does not decrease alignment

[Steven Wan via cfe-commits]

Author: Steven Wan
Date: 2021-08-29T21:33:05-04:00
New Revision: 71b170ccf36ee02e6a4c472bc1d3e89bbaf0e2b4

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

LOG: [AIX] "aligned" attribute does not decrease alignment

The "aligned" attribute can only increase the alignment of a struct, or struct 
member, unless it's used together with the "packed" attribute, or used as a 
part of a typedef, in which case, the "aligned" attribute can both increase and 
decrease alignment.

That said, we expect:
1. "aligned" attribute alone: does not interfere with the alignment upgrade 
instrumented by the AIX "power" alignment rule,
2. "aligned" attribute + typedef: overrides any computed alignment,
3. "aligned" attribute + "packed" attribute: overrides any computed alignment.
The old implementation achieved 2 and 3, but didn't get 1 right, in that any 
field marked attribute "aligned" would not go through the alignment upgrade.

Reviewed By: rjmccall

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

Added: 
clang/test/Layout/aix-type-align-and-pack-attr.cpp

Modified: 
clang/lib/AST/RecordLayoutBuilder.cpp
clang/test/Layout/aix-power-alignment-typedef.cpp

Removed: 
clang/test/Layout/aix-alignof-align-and-pack-attr.cpp



diff  --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 30e88da9ac4f5..5c86b06da835e 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1968,6 +1968,19 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
 }
   }
 
+  // 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 alignedAttrCanDecreaseAIXAlignment = [AlignRequirement, FieldPacked] {
+// Enum alignment sources can be safely ignored here, because this only
+// helps decide whether we need the AIX alignment upgrade, which only
+// applies to floating-point types.
+return AlignRequirement == AlignRequirementKind::RequiredByTypedef ||
+   (AlignRequirement == AlignRequirementKind::RequiredByRecord &&
+FieldPacked);
+  };
+
   // The AIX `power` alignment rules apply the natural alignment of the
   // "first member" if it is of a floating-point data type (or is an aggregate
   // whose recursively "first" member or element is such a type). The alignment
@@ -1978,8 +1991,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   // 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 &&
-  AlignRequirement == AlignRequirementKind::None &&
+  if (DefaultsToAIXPowerAlignment && !alignedAttrCanDecreaseAIXAlignment() &&
   (FoundFirstNonOverlappingEmptyFieldForAIX || IsNaturalAlign)) {
 auto performBuiltinTypeAlignmentUpgrade = [&](const BuiltinType *BTy) {
   if (BTy->getKind() == BuiltinType::Double ||
@@ -1990,12 +2002,13 @@ void ItaniumRecordLayoutBuilder::LayoutField(const 
FieldDecl *D,
   }
 };
 
-const Type *Ty = D->getType()->getBaseElementTypeUnsafe();
-if (const ComplexType *CTy = Ty->getAs()) {
-  
performBuiltinTypeAlignmentUpgrade(CTy->getElementType()->castAs());
-} else if (const BuiltinType *BTy = Ty->getAs()) {
+const Type *BaseTy = D->getType()->getBaseElementTypeUnsafe();
+if (const ComplexType *CTy = BaseTy->getAs()) {
+  performBuiltinTypeAlignmentUpgrade(
+  CTy->getElementType()->castAs());
+} else if (const BuiltinType *BTy = BaseTy->getAs()) {
   performBuiltinTypeAlignmentUpgrade(BTy);
-} else if (const RecordType *RT = Ty->getAs()) {
+} else if (const RecordType *RT = BaseTy->getAs()) {
   const RecordDecl *RD = RT->getDecl();
   assert(RD && "Expected non-null RecordDecl.");
   const ASTRecordLayout &FieldRecord = Context.getASTRecordLayout(RD);

diff  --git a/clang/test/Layout/aix-alignof-align-and-pack-attr.cpp 
b/clang/test/Layout/aix-alignof-align-and-pack-attr.cpp
deleted file mode 100644
index 51f3c5a2adc11..0
--- a/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.t

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

[Steven Wan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG71b170ccf36e: [AIX] "aligned" attribute does not 
decrease alignment (authored by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107394

Files:
  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,19 @@
 }
   }
 
+  // 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 alignedAttrCanDecreaseAIXAlignment = [AlignRequirement, FieldPacked] {
+// Enum alignment sources can be safely ignored here, because this only
+// helps decide whether we need the AIX alignment upgrade, which only
+   

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

errno handling for math library functions is a mess.  Currently, we don't model 
it properly; we just mark the calls "readnone" and hope for the best.  If you 
don't want to fix that, just check for readnone for now.

I don't think we want to be querying function attributes or options here; afn 
plus enabling MASS should be enough.  The function attributes are the old 
mechanism; we just haven't completely migrated some parts of SelectionDAG yet.




Comment at: llvm/include/llvm/Analysis/ScalarFuncs.def:19
+TLI_DEFINE_SCALAR_MASS_FUNC("acosf", "__xl_acosf")
+TLI_DEFINE_SCALAR_MASS_FUNC("__acosf_finite", "__xl_acosf")
+TLI_DEFINE_SCALAR_MASS_FUNC("acos", "__xl_acos")

Do "__acosf_finite" etc. actually exist on AIX?  I thought they only existed on 
glibc, and the glibc functions are all deprecated.

I think I'd prefer to track this information in TargetLibraryInfo, like we do 
for the vector functions, so we can more easily generalize this mechanism in 
the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101759

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

[clang] 4c29dc1 - Revert "[X86] Support __SSC_MARK(const int id)"

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2021-08-30T09:50:26+08:00
New Revision: 4c29dc18cf23d3a644158b567b8c3d471358e2bd

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

LOG: Revert "[X86] Support __SSC_MARK(const int id)"

This reverts commit 78fbde57794e50f5629979f5d69592caf64067e3.

Added: 


Modified: 
clang/lib/Headers/x86gprintrin.h

Removed: 
clang/test/CodeGen/X86/x86-ssc-mark.c



diff  --git a/clang/lib/Headers/x86gprintrin.h 
b/clang/lib/Headers/x86gprintrin.h
index 327ccb724be80..1fc6cab4b28fc 100644
--- a/clang/lib/Headers/x86gprintrin.h
+++ b/clang/lib/Headers/x86gprintrin.h
@@ -20,9 +20,4 @@
 #include 
 #endif
 
-#define __SSC_MARK(Tag)
\
-  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
-   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
-   : "%eax");
-
 #endif /* __X86GPRINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/x86-ssc-mark.c 
b/clang/test/CodeGen/X86/x86-ssc-mark.c
deleted file mode 100644
index ce036aaadf410..0
--- a/clang/test/CodeGen/X86/x86-ssc-mark.c
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
-// RUN: %clang_cc1 %s -triple=i386-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
-
-#include 
-
-// The ebx may be use for base pointer, we need to restore it in time.
-void ssc_mark() {
-// CHECK-LABEL: ssc_mark
-// CHECK: #APP
-// CHECK: movl%ebx, %eax
-// CHECK: movl$0, %ebx
-// CHECK: .byte   100
-// CHECK: .byte   103
-// CHECK: .byte   144
-// CHECK: movl%eax, %ebx
-// CHECK: #NO_APP
-
-  __SSC_MARK(0x0);
-}



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

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
+
+  Finds cases where code could use ``data`` rather than the address of an 
element.
+

It'll be good idea to mention containers and that `data` is method.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst:6
+
+Finds instances of the data pointer being materialized by taking the address of
+the element at index 0 of a container. In particular, ``std::vector`` and

Please make first statement same as in Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 369364.
kito-cheng added a comment.

Changes:

- Remove unused argument MArch for getExtensionVersion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-abi.c
  clang/test/Driver/riscv-arch.c
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/attribute-with-insts.s
  llvm/test/MC/RISCV/invalid-attribute.s

Index: llvm/test/MC/RISCV/invalid-attribute.s
===
--- llvm/test/MC/RISCV/invalid-attribute.s
+++ llvm/test/MC/RISCV/invalid-attribute.s
@@ -7,10 +7,10 @@
 # RUN: not llvm-mc %s -triple=riscv64 -filetype=asm 2>&1 | FileCheck %s
 
 .attribute arch, "foo"
-# CHECK: [[@LINE-1]]:18: error: bad arch string foo
+# CHECK: [[@LINE-1]]:18: error: invalid arch name 'foo', string must begin with rv32{i,e,g} or rv64{i,g}
 
 .attribute arch, "rv32i2p0_y2p0"
-# CHECK: [[@LINE-1]]:18: error: bad arch string y2p0
+# CHECK: [[@LINE-1]]:18: error: invalid arch name 'rv32i2p0_y2p0', invalid standard user-level extension 'y'
 
 .attribute stack_align, "16"
 # CHECK: [[@LINE-1]]:25: error: expected numeric constant
Index: llvm/test/MC/RISCV/attribute-with-insts.s
===
--- llvm/test/MC/RISCV/attribute-with-insts.s
+++ llvm/test/MC/RISCV/attribute-with-insts.s
@@ -10,7 +10,7 @@
 # RUN:   | llvm-objdump --triple=riscv64 -d -M no-aliases - \
 # RUN:   | FileCheck -check-prefix=CHECK-INST %s
 
-.attribute arch, "rv64i2p0_m2p0_a2p0_d2p0_c2p0"
+.attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
 # CHECK-INST: lr.w t0, (t1)
 lr.w t0, (t1)
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -9,9 +9,6 @@
 .attribute arch, "rv32i2"
 # CHECK: attribute  5, "rv32i2p0"
 
-.attribute arch, "rv32i2p"
-# CHECK: attribute  5, "rv32i2p0"
-
 .attribute arch, "rv32i2p0"
 # CHECK: attribute  5, "rv32i2p0"
 
@@ -33,14 +30,14 @@
 .attribute arch, "rv32ima2p0_fdc"
 # CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
-.attribute arch, "rv32ima2p_fdc"
+.attribute arch, "rv32ima2p0_fdc"
 # CHECK: attribute  5, "rv32i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
 
 .attribute arch, "rv32ib"
 # CHECK: attribute  5, "rv32i2p0_b0p93_zba0p93_zbb0p93_zbc0p93_zbe0p93_zbf0p93_zbm0p93_zbp0p93_zbr0p93_zbs0p93_zbt0p93"
 
 .attribute arch, "rv32iv"
-# CHECK: attribute  5, "rv32i2p0_v0p10"
+# CHECK: attribute  5, "rv32i2p0_v0p10_zvlsseg0p10"
 
 .attribute arch, "rv32izba"
 # CHECK: attribute  5, "rv32i2p0_zba0p93"
Index: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
===
--- llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
+++ llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
@@ -11,9 +11,11 @@
 //===--===//
 
 #include "RISCVTargetStreamer.h"
+#include "RISCVBaseInfo.h"
 #include "RISCVMCTargetDesc.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/RISCVAttributes.h"
+#include "llvm/Support/RISCVISAInfo.h"
 
 using namespace llvm;
 
@@ -43,57 +45,13 @@
   else
 emitAttribute(RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_16);
 
-  std::string Arch = "rv32";
-  if (STI.hasFeature(RISCV::Feature64Bit))
-Arch = "rv64";
-  if (STI.hasFeature(RISCV::FeatureRV32E))
-Arch += "e1p9";
-  else
-Arch += "i2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtM))
-Arch += "_m2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtA))
-Arch += "_a2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtF))
-Arch += "_f2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtD))
-Arch += "_d2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtC))
-Arch += "_c2p0";
-  if (STI.hasFeature(RISCV::FeatureStdExtB))
-Arch += "_b0p93";
-  if (STI.hasFeature(RISCV::FeatureStdExtV))
-Arch += "_v0p10";
-  if (STI.hasFeature(RISCV::FeatureExtZfh))
-Arch += "_zfh0p1";
-  if (STI.hasFeature(RISCV::FeatureExtZba))
-Arch += "_zba0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbb))
-Arch += "_zbb0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbc))
-Arch += "_zbc0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbe))
-Arch += "_zbe0p93";
-  if (STI.hasFeature(RISCV::FeatureExtZbf))
-Arch +

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng marked an inline comment as done.
kito-cheng added inline comments.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:268
+// omitted from the version string. E.g., rv32i2p0, rv32i2, rv32i2p1.
+static Error getExtensionVersion(StringRef MArch, StringRef Ext, StringRef In,
+ unsigned &Major, unsigned &Minor,

eopXD wrote:
> For naming consistency with `parseArchString`, maybe rename `MArch` to `Arch`?
It turns out the argument isn't used anywhere...so I remove that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168

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

[clang] 80f7ce8 - [X86] Support __SSC_MARK(const int id)

[Xiang1 Zhang via cfe-commits]

Author: Xiang1 Zhang
Date: 2021-08-30T09:55:35+08:00
New Revision: 80f7ce89938874fb6b40ca5bafa1f5c4eca4746d

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

LOG: [X86] Support __SSC_MARK(const int id)

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

Added: 
clang/test/CodeGen/X86/x86-ssc-mark.c

Modified: 
clang/lib/Headers/x86gprintrin.h

Removed: 




diff  --git a/clang/lib/Headers/x86gprintrin.h 
b/clang/lib/Headers/x86gprintrin.h
index 1fc6cab4b28fc..327ccb724be80 100644
--- a/clang/lib/Headers/x86gprintrin.h
+++ b/clang/lib/Headers/x86gprintrin.h
@@ -20,4 +20,9 @@
 #include 
 #endif
 
+#define __SSC_MARK(Tag)
\
+  __asm__ __volatile__("movl %%ebx, %%eax; movl %0, %%ebx; .byte 0x64, 0x67, " 
\
+   "0x90; movl %%eax, %%ebx;" ::"i"(Tag)   
\
+   : "%eax");
+
 #endif /* __X86GPRINTRIN_H */

diff  --git a/clang/test/CodeGen/X86/x86-ssc-mark.c 
b/clang/test/CodeGen/X86/x86-ssc-mark.c
new file mode 100644
index 0..a2a57a13d7d44
--- /dev/null
+++ b/clang/test/CodeGen/X86/x86-ssc-mark.c
@@ -0,0 +1,20 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
+// RUN: %clang_cc1 %s -triple=i386-unknown-unknown -S -ffreestanding -o - | 
FileCheck %s
+
+#include 
+
+// The ebx may be use for base pointer, we need to restore it in time.
+void ssc_mark() {
+// CHECK-LABEL: ssc_mark
+// CHECK: #APP
+// CHECK: movl%ebx, %eax
+// CHECK: movl$0, %ebx
+// CHECK: .byte   100
+// CHECK: .byte   103
+// CHECK: .byte   144
+// CHECK: movl%eax, %ebx
+// CHECK: #NO_APP
+
+  __SSC_MARK(0x0);
+}



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

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

[Alexander Pivovarov via Phabricator via cfe-commits]

apivovarov updated this revision to Diff 369369.
apivovarov added a comment.

fix typo in MCPU-ABI-SIFIVE-S51


Repository:
  rG LLVM Github Monorepo

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

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-ABI-SIFIVE-S51: "-nostdsysteminc" "-target-cpu" "sifive-s51"
+// MCPU-ABI-SIFIVE-S51: "-target-feature" "+m" "-target-feature" "+a"
+// MCPU-ABI-SIFIVE-S51: "-target-feature" "+c" "-target-feature" "+64bit"
+// MCPU-ABI-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=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-ABI-SIFIVE-S51: "-nostdsysteminc" "-target-cpu" "sifive-s51"
+// MCPU-ABI-SIFIVE-S51: "-target-feature" "+m" "-target-feature" "+a"
+// MCPU-ABI-SIFIVE-S51: "-target-feature" "+c" "-target-feature" "+64bit"
+// MCPU-ABI-SI

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

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

That code has changed quite a bit since I've worked on it.

The only problem I could see is that passing `-flto=thin -flto` and choosing 
thin LTO kinda makes sense if you interpret `-flto` as just "use LTO". The 
`-flto=thin` is more specific, so it could make sense to pick that. But that's 
just something I wonder while looking at it. Not sure if we process other 
options like this by just choosing the last one.


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

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D108881#2971651 , @tbaeder wrote:

> That code has changed quite a bit since I've worked on it.
>
> The only problem I could see is that passing `-flto=thin -flto` and choosing 
> thin LTO kinda makes sense if you interpret `-flto` as just "use LTO". The 
> `-flto=thin` is more specific, so it could make sense to pick that. But 
> that's just something I wonder while looking at it. Not sure if we process 
> other options like this by just choosing the last one.

The default of -flto is documented as -flto=full, so I think the change here 
makes sense. In general the last option wins.




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");

mnadeem wrote:
> 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");
> ```
Yes, that should be fine since the default is full. But better yet, make the 
else pass the explicit -flto=full.



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

Can we consolidate the option handling into parseLTOMode, which does the rest 
of the -flto option handling and sets the LTOMode returned by the below call to 
getLTOMode()? The default of -flto is "full" so it can be treated there as 
such. Actually, looks like it should already be doing this?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4498
   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");

Looks like the result of getLTOMode() is already saved in the LTOMode local 
variable.



Comment at: clang/test/Driver/lto.c:97
+//
+// FLTO-THIN: -flto=thin
+// FLTO-FULL: -flto=full

Probably want to check that we don't have additional -flto options being passed 
too, here and on the below lines.


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] D108905: [ItaniumCXXABI] Set nounwind on __cxa_begin_catch/__cxa_end_catch nounwind

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: ChuanqiXu, rjmccall.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

See https://lists.llvm.org/pipermail/cfe-dev/2021-August/068740.html

A catch clause calls __cxa_begin_catch/__cxa_end_catch.
This allows a function with all non-nounwind callees surrounded by catch
(...) can infer the nounwind attribute, e.g.
`void test() { try { can_throw(); } catch (...) {}}`

This can avoid unneeded call site records whose action record offset is 0
(cleanup). In addition, a `__clang_call_terminate` define may be avoided.

Note: libcxxrt `__cxa_end_catch` doesn't call external functions.  
libsupc++/libc++abi
`__cxa_end_catch` calls `_Unwind_DeleteException` for a foreign exception.
We can except `_Unwind_DeleteException` does not throw (the behavior would be 
unclear
if it throws).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108905

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/stack-reuse-exceptions.cpp


Index: clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
+++ clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
@@ -86,8 +86,8 @@
 //
 // CHECK: [[CATCH]]:
 // CHECK-NOT: call void @llvm.lifetime
-// CHECK: invoke void
-// CHECK-NEXT: to label %[[TRY_CONT]] unwind label %[[OUTER_LPAD:.+]]
+// CHECK: call void @__cxa_end_catch
+// CHECK-NEXT: br label %[[TRY_CONT]]
 //
 // CHECK: [[TRY_CONT]]:
 // CHECK: %[[T_OUTER:[^ ]+]] = bitcast %struct.Large* %{{[^ ]+}} to i8*
@@ -100,15 +100,7 @@
 // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* nonnull %[[CLEAN]])
 // CHECK: ret void
 //
-// CHECK: [[OUTER_LPAD]]:
-// CHECK-NOT: call void @llvm.lifetime
-// CHECK: br label %[[EHCLEANUP:.+]]
-//
-// CHECK: [[OUTER_LPAD2]]:
-// CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* nonnull 
%[[T_OUTER]])
-// CHECK: br label %[[EHCLEANUP]]
-//
-// CHECK: [[EHCLEANUP]]:
+// CHECK: lpad[[#]]:
 // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* nonnull %[[CLEAN]])
 
   NontrivialDtor clean;
Index: clang/test/CodeGenCXX/exceptions.cpp
===
--- clang/test/CodeGenCXX/exceptions.cpp
+++ clang/test/CodeGenCXX/exceptions.cpp
@@ -326,9 +326,12 @@
   }
 }
 
+// CHECK11-LABEL:declare{{.*}} i8* @__cxa_begin_catch(i8*) #[[#NOUNWIND:]]
+
 // PR9303: invalid assert on this
 namespace test6 {
   bool cond();
+  // CHECK11-LABEL:define{{.*}} void @_ZN5test64testEv() #[[#TEST6:]]
   void test() {
 try {
 lbl:
@@ -634,3 +637,6 @@
 }
 
 // CHECK98: attributes [[NI_NR_NUW]] = { noinline noreturn nounwind }
+
+// CHECK11: attributes #[[#NOUNWIND]] = { nounwind }
+// CHECK11: attributes #[[#TEST6]] = {{.*}} nounwind
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4393,7 +4393,12 @@
   llvm::FunctionType *FTy = llvm::FunctionType::get(
   CGM.Int8PtrTy, CGM.Int8PtrTy, /*isVarArg=*/false);
 
-  return CGM.CreateRuntimeFunction(FTy, "__cxa_begin_catch");
+  // Set nounwind so that a function with all non-nounwind callees surrounded 
by
+  // catch (...) can infer nounwind, which can avoid unneded call site records.
+  llvm::FunctionCallee Ret =
+  CGM.CreateRuntimeFunction(FTy, "__cxa_begin_catch");
+  cast(Ret.getCallee())->addFnAttr(llvm::Attribute::NoUnwind);
+  return Ret;
 }
 
 static llvm::FunctionCallee getEndCatchFn(CodeGenModule &CGM) {
@@ -4401,7 +4406,9 @@
   llvm::FunctionType *FTy =
   llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
 
-  return CGM.CreateRuntimeFunction(FTy, "__cxa_end_catch");
+  llvm::FunctionCallee Ret = CGM.CreateRuntimeFunction(FTy, "__cxa_end_catch");
+  cast(Ret.getCallee())->addFnAttr(llvm::Attribute::NoUnwind);
+  return Ret;
 }
 
 static llvm::FunctionCallee getGetExceptionPtrFn(CodeGenModule &CGM) {


Index: clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
+++ clang/test/CodeGenCXX/stack-reuse-exceptions.cpp
@@ -86,8 +86,8 @@
 //
 // CHECK: [[CATCH]]:
 // CHECK-NOT: call void @llvm.lifetime
-// CHECK: invoke void
-// CHECK-NEXT: to label %[[TRY_CONT]] unwind label %[[OUTER_LPAD:.+]]
+// CHECK: call void @__cxa_end_catch
+// CHECK-NEXT: br label %[[TRY_CONT]]
 //
 // CHECK: [[TRY_CONT]]:
 // CHECK: %[[T_OUTER:[^ ]+]] = bitcast %struct.Large* %{{[^ ]+}} to i8*
@@ -100,15 +100,7 @@
 // CHECK: call void @llvm.lifetime.end.p0i8({{[^,]+}}, i8* nonnull %[[CLEAN]])
 // CHECK: ret void
 //
-// CHECK: [[OUTER_LPAD]]:
-// CHECK-NOT: call void @llvm.lifetime
-// CHECK: br label %[[EHCLEANUP:.+]]
-//
-// CHECK: [[OUTER_LPAD2]]:
-

[PATCH] D108905: [ItaniumCXXABI] Set nounwind on __cxa_begin_catch/__cxa_end_catch nounwind

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

Thanks for looking into this!
I am not familiar with exception handling. But from the comments, I worried 
that if it is a problem if there are exceptions whose destructor could throw 
(although it is not usual nor good).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108905

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