[PATCH] D120244: [clang][sema] Enable first-class bool support for C2x

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

tbaeder updated this revision to Diff 413279.
tbaeder marked 3 inline comments as done.
tbaeder added a comment.

Thanks for the links to the papers. What's the best way of updating 
https://clang.llvm.org/c_status.html#c2x with the new papers?

I added the changes to `stdbool.h`, but I haven't been able to come up with a 
good way of testing them...


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

https://reviews.llvm.org/D120244

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Headers/stdbool.h
  clang/test/Sema/c2x-bool.c


Index: clang/test/Sema/c2x-bool.c
===
--- /dev/null
+++ clang/test/Sema/c2x-bool.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+_Static_assert(_Generic(true, _Bool : 1, default: 0));
+_Static_assert(_Generic(false, _Bool : 1, default: 0));
+
+_Static_assert(_Generic(true, bool : 1, default: 0));
+_Static_assert(_Generic(false, bool : 1, default: 0));
+
+_Static_assert(_Generic(true, bool : true, default: false));
+_Static_assert(_Generic(false, bool : true, default: false));
+
+_Static_assert(true == (bool)+1);
+_Static_assert(false == (bool)+0);
+
+_Static_assert(_Generic(+true, bool : 0, unsigned int : 0, signed int : 1, 
default : 0));
+
+struct S {
+  bool b : 1;
+} s;
+_Static_assert(_Generic(+s.b, bool : 0, unsigned int : 0, signed int : 1, 
default : 0));
+
+static void *f = false; // expected-warning {{to null from a constant boolean 
expression}}
+static int one = true;
+static int zero = false;
+
+static void do_work() {
+  char *str = "Foo";
+  str[false] = 'f';
+  str[true] = 'f';
+
+  char c1[true];
+  char c2[false];
+}
+
+#if true != 1
+#error true should be 1 in the preprocessor
+#endif
+
+#if false != 0
+#error false should be 0 in the preprocessor
+#endif
Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -10,6 +10,8 @@
 #ifndef __STDBOOL_H
 #define __STDBOOL_H
 
+/* true, false and bool are first-class keywords in C2x */
+#if __STDC_VERSION__ < 202000L
 /* Don't define bool, true, and false in C++, except as a GNU extension. */
 #ifndef __cplusplus
 #define bool _Bool
@@ -20,9 +22,10 @@
 #define _Bool bool
 #if __cplusplus < 201103L
 /* For C++98, define bool, false, true as a GNU extension. */
-#define bool  bool
+#define bool bool
 #define false false
-#define true  true
+#define true true
+#endif
 #endif
 #endif
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3249,8 +3249,8 @@
 
   Opts.RenderScript = IK.getLanguage() == Language::RenderScript;
 
-  // OpenCL and C++ both have bool, true, false keywords.
-  Opts.Bool = Opts.OpenCL || Opts.CPlusPlus;
+  // OpenCL, C++ and C2x have bool, true, false keywords.
+  Opts.Bool = Opts.OpenCL || Opts.CPlusPlus || Opts.C2x;
 
   // OpenCL has half keyword
   Opts.Half = Opts.OpenCL;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -111,6 +111,7 @@
 
 - Implemented `WG14 N2674 The noreturn attribute 
`_.
 - Implemented `WG14 N2418 Adding the u8 character prefix 
`_.
+- Implemented `WG14 N2935 Make false and true first-class language features 
`_.
 
 C++ Language Changes in Clang
 -


Index: clang/test/Sema/c2x-bool.c
===
--- /dev/null
+++ clang/test/Sema/c2x-bool.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify %s
+
+_Static_assert(_Generic(true, _Bool : 1, default: 0));
+_Static_assert(_Generic(false, _Bool : 1, default: 0));
+
+_Static_assert(_Generic(true, bool : 1, default: 0));
+_Static_assert(_Generic(false, bool : 1, default: 0));
+
+_Static_assert(_Generic(true, bool : true, default: false));
+_Static_assert(_Generic(false, bool : true, default: false));
+
+_Static_assert(true == (bool)+1);
+_Static_assert(false == (bool)+0);
+
+_Static_assert(_Generic(+true, bool : 0, unsigned int : 0, signed int : 1, default : 0));
+
+struct S {
+  bool b : 1;
+} s;
+_Static_assert(_Generic(+s.b, bool : 0, unsigned int : 0, signed int : 1, default : 0));
+
+static void *f = false; // expected-warning {{to null from a constant boolean expression}}
+static int one = true;
+static int zero = false;
+
+static void do_work() {
+  char *str = "Foo";
+  str[false] = 'f';
+  str[true] = 'f';
+
+  char c1[true];
+  char c2[false];
+}
+
+#if true != 1
+#error true should be 1 in the preprocessor
+#end

[PATCH] D120244: [clang][sema] Enable first-class bool support for C2x

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

tbaeder added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:113
 - Implemented `WG14 N2674 The noreturn attribute 
`_.
 - Implemented `WG14 N2418 Adding the u8 character prefix 
`_.
+- Implemented `WG14 N2935 Make false and true first-class language features 
`_.

This line is from https://reviews.llvm.org/D119221 and probably why the patch 
application fails. Not relevant for review I think but I'll fix it up next time.


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

https://reviews.llvm.org/D120244

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan.
MyDeveloperDay added projects: clang, clang-format.
Herald added a project: All.
MyDeveloperDay requested review of this revision.

I have lost count of the number of times this has been reported, but it 
fundamentally comes down to the fact that the "AlignArrayLeft/Right" function 
is fundamentally broken for non-square arrays.

As a result, a point can end up running off the end of the structure, I've 
spent the last 2 weekends trying to rewrite this algorithm but I've struggled 
to get it aligned correctly.

This is an interim fix, that ignores all array that are non-square and leaves 
them alone. I think this can allow us to close out most of the crashes (if not 
all).

I think this can help reduce the number of bugs coming in that are duplicates.

https://github.com/llvm/llvm-project/issues/53748
https://github.com/llvm/llvm-project/issues/51767
https://github.com/llvm/llvm-project/issues/51277


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121069

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,40 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -196,8 +196,21 @@
 
   struct CellDescriptions {
 SmallVector Cells;
-unsigned CellCount = 0;
+SmallVector CellCounts;
 unsigned InitialSpaces = 0;
+
+// Determine if the array is "Square" i.e. every row has the same number
+// of columns as the first row
+bool IsSquare() {
+  if (CellCounts.empty())
+return false;
+
+  for (auto NumberOfColumns : CellCounts) {
+if (NumberOfColumns != CellCounts[0])
+  return false;
+  }
+  return true;
+}
   };
 
   /// Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1032,11 +1032,13 @@
 
 void WhitespaceManager::alignArrayInitializersRightJustified(
 CellDescriptions &&CellDescs) {
+  if (!CellDescs.IsSquare())
+return;
+  
   auto &Cells = CellDescs.Cells;
-
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
-  for (auto i = 0U; i < CellDescs.CellCount; ++i, ++CellIter) {
+  for (auto i = 0U; i < CellDescs.CellCounts[0]; ++i, ++CellIter) {
 unsigned NetWidth = 0U;
 if (isSplitCell(*CellIter))
   NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
@@ -1060,14 +1062,14 @@
 getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth =
 getMaximumNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces,
-   CellDescs.CellCount);
+   CellDescs.CellCounts[0]);
 if (ThisNetWidth < MaxNetWidth)
   Changes[CellIter->Index].Spaces = (MaxNetWidth - ThisNetWidth);
 auto RowCount = 1U;
 auto Offset = std::distance(Cells.begin(), CellIter);
 for (const auto *Next = CellIter->NextColumnElement; Next != nullptr;
  Next = Next->NextColumnElement) {
-  auto *Start =

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 413286.
MyDeveloperDay added a comment.

Update to always use the first row number of columns or unit tests crash


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

https://reviews.llvm.org/D121069

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,40 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -196,8 +196,21 @@
 
   struct CellDescriptions {
 SmallVector Cells;
-unsigned CellCount = 0;
+SmallVector CellCounts;
 unsigned InitialSpaces = 0;
+
+// Determine if the array is "Square" i.e. every row has the same number
+// of columns as the first row
+bool IsSquare() {
+  if (CellCounts.empty())
+return false;
+
+  for (auto NumberOfColumns : CellCounts) {
+if (NumberOfColumns != CellCounts[0])
+  return false;
+  }
+  return true;
+}
   };
 
   /// Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1032,11 +1032,13 @@
 
 void WhitespaceManager::alignArrayInitializersRightJustified(
 CellDescriptions &&CellDescs) {
+  if (!CellDescs.IsSquare())
+return;
+  
   auto &Cells = CellDescs.Cells;
-
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
-  for (auto i = 0U; i < CellDescs.CellCount; ++i, ++CellIter) {
+  for (auto i = 0U; i < CellDescs.CellCounts[0]; ++i, ++CellIter) {
 unsigned NetWidth = 0U;
 if (isSplitCell(*CellIter))
   NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
@@ -1060,14 +1062,14 @@
 getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth =
 getMaximumNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces,
-   CellDescs.CellCount);
+   CellDescs.CellCounts[0]);
 if (ThisNetWidth < MaxNetWidth)
   Changes[CellIter->Index].Spaces = (MaxNetWidth - ThisNetWidth);
 auto RowCount = 1U;
 auto Offset = std::distance(Cells.begin(), CellIter);
 for (const auto *Next = CellIter->NextColumnElement; Next != nullptr;
  Next = Next->NextColumnElement) {
-  auto *Start = (Cells.begin() + RowCount * CellDescs.CellCount);
+  auto *Start = (Cells.begin() + RowCount * CellDescs.CellCounts[RowCount]);
   auto *End = Start + Offset;
   ThisNetWidth = getNetWidth(Start, End, CellDescs.InitialSpaces);
   if (ThisNetWidth < MaxNetWidth)
@@ -1100,8 +1102,10 @@
 
 void WhitespaceManager::alignArrayInitializersLeftJustified(
 CellDescriptions &&CellDescs) {
+  if (!CellDescs.IsSquare()) 
+return;
+  
   auto &Cells = CellDescs.Cells;
-
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
   // The first cell needs to be against the left brace.
@@ -1110,9 +1114,9 @@
   else
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
-  for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
+  for (auto i = 1U; i 

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 413288.
MyDeveloperDay added a comment.

Add more producers


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

https://reviews.llvm.org/D121069

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,66 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -196,8 +196,21 @@
 
   struct CellDescriptions {
 SmallVector Cells;
-unsigned CellCount = 0;
+SmallVector CellCounts;
 unsigned InitialSpaces = 0;
+
+// Determine if the array is "Square" i.e. every row has the same number
+// of columns as the first row
+bool IsSquare() {
+  if (CellCounts.empty())
+return false;
+
+  for (auto NumberOfColumns : CellCounts) {
+if (NumberOfColumns != CellCounts[0])
+  return false;
+  }
+  return true;
+}
   };
 
   /// Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1032,11 +1032,13 @@
 
 void WhitespaceManager::alignArrayInitializersRightJustified(
 CellDescriptions &&CellDescs) {
+  if (!CellDescs.IsSquare())
+return;
+  
   auto &Cells = CellDescs.Cells;
-
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
-  for (auto i = 0U; i < CellDescs.CellCount; ++i, ++CellIter) {
+  for (auto i = 0U; i < CellDescs.CellCounts[0]; ++i, ++CellIter) {
 unsigned NetWidth = 0U;
 if (isSplitCell(*CellIter))
   NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
@@ -1060,14 +1062,14 @@
 getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth =
 getMaximumNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces,
-   CellDescs.CellCount);
+   CellDescs.CellCounts[0]);
 if (ThisNetWidth < MaxNetWidth)
   Changes[CellIter->Index].Spaces = (MaxNetWidth - ThisNetWidth);
 auto RowCount = 1U;
 auto Offset = std::distance(Cells.begin(), CellIter);
 for (const auto *Next = CellIter->NextColumnElement; Next != nullptr;
  Next = Next->NextColumnElement) {
-  auto *Start = (Cells.begin() + RowCount * CellDescs.CellCount);
+  auto *Start = (Cells.begin() + RowCount * CellDescs.CellCounts[RowCount]);
   auto *End = Start + Offset;
   ThisNetWidth = getNetWidth(Start, End, CellDescs.InitialSpaces);

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw added a comment.

About chained assignments, the current program does not attempt to align them 
in a consistent way.  And this revision doesn't change it.  Both before and 
after this revision, the output depend on the order of the statements.

  Foo = Bar = 5;
  Int Baz   = 5;
  
  Int Baz = 5;
  Foo = Bar = 5;

Is the title making you think it is about chained assignments?  I took the term 
"compound assignment" from Section 6.5.16.2 of C17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D117836: Fix obvious typo

[Phoebe Wang via Phabricator via cfe-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117836

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

[PATCH] D121049: [Clang][VE] Add vector load intrinsics

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 updated this revision to Diff 413295.
kaz7 added a comment.

Add intrinsics for not only VLD instructions but also VLD2D instructions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121049

Files:
  clang/include/clang/Basic/BuiltinsVE.def
  clang/include/clang/Basic/BuiltinsVEVL.gen.def
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/module.modulemap
  clang/lib/Basic/Targets/VE.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/velintrin.h
  clang/lib/Headers/velintrin_gen.h
  clang/test/CodeGen/VE/ve-velintrin.c

Index: clang/test/CodeGen/VE/ve-velintrin.c
===
--- /dev/null
+++ clang/test/CodeGen/VE/ve-velintrin.c
@@ -0,0 +1,232 @@
+// REQUIRES: ve-registered-target
+
+// RUN: %clang_cc1 -S -emit-llvm -triple ve-unknown-linux-gnu \
+// RUN:   -ffreestanding %s -o - | FileCheck %s
+
+#include 
+
+__vr vr1;
+
+void __attribute__((noinline))
+test_vld_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vld_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vld.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vld_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vld_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vld_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vld.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vld_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldnc_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldnc_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldnc.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldnc_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldnc_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldnc_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldnc.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldnc_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldu_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldu_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldu.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldu_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldu_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldu_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldu.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldu_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldunc_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldunc_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldunc.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldunc_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldunc_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldunc_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldunc.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldunc_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldlsx_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlsx_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlsx.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldlsx_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldlsx_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlsx_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlsx.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldlsx_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldlsxnc_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlsxnc_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlsxnc.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldlsxnc_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldlsxnc_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlsxnc_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlsxnc.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldlsxnc_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldlzx_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlzx_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlzx.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldlzx_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldlzx_vssvl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlzx_vssvl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlzx.vssvl(i64 %{{.*}}, i8* %{{.*}}, <256 x double> %{{.*}}, i32 256)
+  vr1 = _vel_vldlzx_vssvl(idx, p, vr1, 256);
+}
+
+void __attribute__((noinline))
+test_vldlzxnc_vssl(char* p, long idx) {
+  // CHECK-LABEL: @test_vldlzxnc_vssl
+  // CHECK: call <256 x double> @llvm.ve.vl.vldlzxnc.vssl(i64 %{{.*}}, i8* %{{.*}}, i32 256)
+  vr1 = _vel_vldlzxnc_vssl(idx, p, 256);
+}
+
+void __attribute__((noinline))
+test_vldlzxnc_vssvl(char* p, long idx) {
+  //

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.
Herald added a project: All.

At the beginning, this implementation extends `vector_type` attribute which is 
GCC's attribute.  So, this may cause future conflicts with GCC when they extend 
it.  But, now this patch uses it's own `ext_vector_type` attribute.  So, 
basically this modification is safe against to the C/C++ future extension and 
the GCC future extension, in my honest opinion.

Is it OK to accept this patch?  Or is there anything need to consider.  I 
understand that this is a language extension, so it not easy to say OK...  But, 
this patch spent 1 year and a half almost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88905

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.h:204
+// of columns as the first row
+bool IsSquare() {
+  if (CellCounts.empty())

Don't really like the name...
Some suggestions:
- HasEqualRows
- HasEqualRowLengths
- HasSameRowLengths
- IsRectangular



Comment at: clang/unittests/Format/FormatTest.cpp:25309
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"

Not sure I understand the comment here, this formatting looks ok, no?



Comment at: clang/unittests/Format/FormatTest.cpp:25340
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"

This one is "square", isn't it?
I think you wanted to omit 3 in the first row 


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

https://reviews.llvm.org/D121069

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

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:166
+/// child is a sink node.
+static bool unconditionallyLeadsHere(const ExplodedNode *N) {
+  size_t NonSinkNodeCount = llvm::count_if(

xazax.hun wrote:
> xazax.hun wrote:
> > Consider the following code snippet:
> > ```
> > void f(int *p, bool b)
> > {
> >   if (b) {
> > *p = 4;
> >   }
> >   if (p) {
> >...
> >   }
> > }
> > ```
> > 
> > I suspect that we would get a warning for the code above. I think warning 
> > on the code above might be reasonable (the values of `b` and `p` might be 
> > correlated but in some cases the analyzer has no way to know this, probably 
> > some assertions could make the code clearer in that case).
> > 
> > My problem is with the wording of the error message.
> > The warning `Pointer is unconditionally non-null here` on the null check is 
> > not true for the code above.
> Also, if the check would warn for the code snippet above, the note "suggest 
> moving the condition here" would also be incorrect.
What if we demand that the the `CFGBlock` of the dereference must dominate the 
`CFGBlock` of the condition point?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120992

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

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:16800
+   Alignment));
+}
+

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Can you test it with `AlignConsecutiveDeclarations`?
> Do you mean like the formatting the same code with 
> `AlignConsecutiveAssignments.Enabled` being false and 
> `AlignConsecutiveDeclarations.Enabled` being true?  By the way, I just 
> realized that things like `int a += 5;` are not valid code.  Should I remove 
> the `int ` instead?
No I meant both enabled, with the `AlignCompound` one time with and without 
`PadOperators`. To see if the options work nicely together.

But yeah, as you stated this isn't valid code, so it doen't really matter how 
it is formatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

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

steakhal added a comment.

Land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

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

[PATCH] D121072: [X86][Experiment] Mangle global variable for inline assembly

[Phoebe Wang via Phabricator via cfe-commits]

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

This is an experiment inspired by @efriedma 's suggestion on D120887 
.
It seems a more concise way to solve Xiang's problem.

There're some lit failures which I didn't care for now. I can fix them
if we think this is a good approch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121072

Files:
  clang/lib/CodeGen/CGStmt.cpp


Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -31,6 +31,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Mangler.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -2544,6 +2545,19 @@
 Constraints += InputConstraint;
   }
 
+  // Replace global value to its name.
+  if (isa(&S)) {
+for (int I = 0, E = Args.size(); I != E; ++I) {
+  if (auto *GV = dyn_cast(Args[I])) {
+llvm::Mangler M;
+SmallString<256> MangleName;
+M.getNameWithPrefix(MangleName, GV, /*CannotUsePrivateLabel=*/true);
+AsmString.replace(AsmString.find("$" + std::to_string(I)), 2,
+  MangleName.c_str());
+  }
+}
+  }
+
   // Append the "input" part of inout constraints.
   for (unsigned i = 0, e = InOutArgs.size(); i != e; i++) {
 ArgTypes.push_back(InOutArgTypes[i]);


Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -31,6 +31,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Mangler.h"
 #include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
@@ -2544,6 +2545,19 @@
 Constraints += InputConstraint;
   }
 
+  // Replace global value to its name.
+  if (isa(&S)) {
+for (int I = 0, E = Args.size(); I != E; ++I) {
+  if (auto *GV = dyn_cast(Args[I])) {
+llvm::Mangler M;
+SmallString<256> MangleName;
+M.getNameWithPrefix(MangleName, GV, /*CannotUsePrivateLabel=*/true);
+AsmString.replace(AsmString.find("$" + std::to_string(I)), 2,
+  MangleName.c_str());
+  }
+}
+  }
+
   // Append the "input" part of inout constraints.
   for (unsigned i = 0, e = InOutArgs.size(); i != e; i++) {
 ArgTypes.push_back(InOutArgTypes[i]);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

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

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

I would probably query the opcode only once and reuse it, but is also fine.

Btw whats your intention making this change? Do you plan greater 
refactorings/cleanups?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121045

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

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM (though I'd also be fine if we only wanted to call `getOpcode()` once).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121045

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 413298.
MyDeveloperDay added a comment.

Fix review comments and clang-format


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

https://reviews.llvm.org/D121069

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,104 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // The AlignArray code is incorrect for non square Arrays and can cause
+  // crashes, these tests assert that the array is not changed but will
+  // also act as regression tests for when it is properly fixed
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -196,8 +196,21 @@
 
   struct CellDescriptions {
 SmallVector Cells;
-unsigned CellCount = 0;
+SmallVector CellCounts;
 unsigned InitialSpaces = 0;
+
+// Determine if the array is "Square" i.e. every row has the same number
+// of columns as the first row
+bool HasEqualRowLengths() const {
+  if (CellCounts.empty())
+return false;
+
+  for (auto NumberOfColumns : CellCounts) {
+if (NumberOfColumns != CellCounts[0])
+  return false;
+  }
+  return true;
+}
   };
 
   /// Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1032,11 +1032,13 @@
 
 void WhitespaceM

[PATCH] D121072: [X86][Experiment] Mangle global variable for inline assembly

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2555-2556
+M.getNameWithPrefix(MangleName, GV, /*CannotUsePrivateLabel=*/true);
+AsmString.replace(AsmString.find("$" + std::to_string(I)), 2,
+  MangleName.c_str());
+  }

I'll use regex to match precisely and replace all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121072

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

There should also be a warning or at least note on the option description about 
the current limitations.




Comment at: clang/lib/Format/WhitespaceManager.h:202-203
+
+// Determine if the array is "Square" i.e. every row has the same number
+// of columns as the first row
+bool HasEqualRowLengths() const {

This should also be adapted.
And a full stop at the end.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 413299.
MyDeveloperDay marked 4 inline comments as done.
MyDeveloperDay added a comment.

Lets get the comment aligned with the code


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

https://reviews.llvm.org/D121069

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,104 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // The AlignArray code is incorrect for non square Arrays and can cause
+  // crashes, these tests assert that the array is not changed but will
+  // also act as regression tests for when it is properly fixed
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -196,8 +196,21 @@
 
   struct CellDescriptions {
 SmallVector Cells;
-unsigned CellCount = 0;
+SmallVector CellCounts;
 unsigned InitialSpaces = 0;
+
+// Determine if every row in the the array
+// as the same number of Columns.
+bool HasEqualRowLengths() const {
+  if (CellCounts.empty())
+return false;
+
+  for (auto NumberOfColumns : CellCounts) {
+if (NumberOfColumns != CellCounts[0])
+  return false;
+  }
+  return true;
+}
   };
 
   /// Calculate \c IsTrailingComment, \c TokenLength for the last tokens
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1032,11 +10

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D121045#3362373 , @steakhal wrote:

> I would probably query the opcode only once and reuse it, but is also fine.
>
> Btw whats your intention making this change? Do you plan greater 
> refactorings/cleanups?

Yeah, I think query the opcode once is a nice idea tho, let me do this in this 
patch only. And yeah I'm thinking of refactoring/cleanups where it needed and 
would love to make some big refactors. But I found this when I was looking into 
this issue (https://github.com/llvm/llvm-project/issues/53564 and needs to look 
at the deadstorechecker file.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121045

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

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 413300.
phyBrackets added a comment.

query the opcode only once and reuse it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121045

Files:
  clang/docs/DataFlowAnalysisIntro.md
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -107,11 +107,8 @@
   dyn_cast(Ex->IgnoreParenCasts());
 if (!BO)
   break;
-if (BO->getOpcode() == BO_Assign) {
-  Ex = BO->getRHS();
-  continue;
-}
-if (BO->getOpcode() == BO_Comma) {
+BinaryOperatorKind BO_AssignOrComma = BO->getOpcode();
+if (BO_AssignOrComma == BO_Assign || BO_AssignOrComma == BO_Comma) {
   Ex = BO->getRHS();
   continue;
 }
Index: clang/docs/DataFlowAnalysisIntro.md
===
--- clang/docs/DataFlowAnalysisIntro.md
+++ clang/docs/DataFlowAnalysisIntro.md
@@ -287,7 +287,7 @@
 
 (Note that there are other ways to write this equation that produce higher
 precision analysis results. The trick is to keep exploring the execution paths
-separately and delay joining until later. Hoowever, we won't discuss those
+separately and delay joining until later. However, we won't discuss those
 variations here.)
 
 To make a conclusion about all paths through the program, we repeat this


Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -107,11 +107,8 @@
   dyn_cast(Ex->IgnoreParenCasts());
 if (!BO)
   break;
-if (BO->getOpcode() == BO_Assign) {
-  Ex = BO->getRHS();
-  continue;
-}
-if (BO->getOpcode() == BO_Comma) {
+BinaryOperatorKind BO_AssignOrComma = BO->getOpcode();
+if (BO_AssignOrComma == BO_Assign || BO_AssignOrComma == BO_Comma) {
   Ex = BO->getRHS();
   continue;
 }
Index: clang/docs/DataFlowAnalysisIntro.md
===
--- clang/docs/DataFlowAnalysisIntro.md
+++ clang/docs/DataFlowAnalysisIntro.md
@@ -287,7 +287,7 @@
 
 (Note that there are other ways to write this equation that produce higher
 precision analysis results. The trick is to keep exploring the execution paths
-separately and delay joining until later. Hoowever, we won't discuss those
+separately and delay joining until later. However, we won't discuss those
 variations here.)
 
 To make a conclusion about all paths through the program, we repeat this
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

Sorry but Idk why that DataFlowAnalysisIntro typo change also commited , I only 
add the deadstorechecker file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121045

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

NOTE: I've tried to collate all the reported crashing examples and run this fix 
through them (both Left and Right) all pass except this one below



  void foo()
  {
  auto thing = test{
  {
 {something}, //A
  }
};
  }

This is not related to it being non-square because it's not.




Comment at: clang/unittests/Format/FormatTest.cpp:25309
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"

curdeius wrote:
> Not sure I understand the comment here, this formatting looks ok, no?
The test is testing that it's not changing it.. with the old code it would (but 
incorrectly or crash)


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Marek Kurdej via Phabricator via cfe-commits]

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

Nits only.
+1 for adding a note in the option documentation.




Comment at: clang/lib/Format/WhitespaceManager.h:203
+// Determine if every row in the the array
+// as the same number of Columns.
+bool HasEqualRowLengths() const {





Comment at: clang/unittests/Format/FormatTest.cpp:25359
+
+  // TODO don't adjust this non square array
+  verifyFormat("struct test demo[] = {\n"

Nit: should be updated as above I guess.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 413301.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

There were no checks for if the cell would run off the end of the CellDescs 
structure, this is part of the reason for the crashes that the PrevIter->Index 
becomes invalid and is then used to index into the Changes.

The code just assumed it knew how many rows and columns there were, but in some 
circumstances it gets it wrong and just keeps going. This MaxRows check should 
at least prevent that by ensuring we just don't keep going off the end of the 
rows. (this was the final crash)

Added a warning to the documentation.

F22348813: image.png 


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

https://reviews.llvm.org/D121069

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25298,6 +25298,138 @@
   verifyFormat("#define A x%:%:y");
 }
 
+TEST_F(FormatTest, AlignArrayOfStructuresLeftAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Left;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // The AlignArray code is incorrect for non square Arrays and can cause
+  // crashes, these tests assert that the array is not changed but will
+  // also act as regression tests for when it is properly fixed
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b}\n"
+   "};\n",
+   Style);
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+   "{a, b},\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  auto thing = test{\n"
+   "  {\n"
+   "   {13}, {something}, // A\n"
+   "  }\n"
+   "  };\n"
+   "}",
+   "void foo() {\n"
+   "  auto thing = test{\n"
+   "  {\n"
+   "   {13},\n"
+   "   {something}, // A\n"
+   "  }\n"
+   "  };\n"
+   "}",
+   Style);
+}
+
+TEST_F(FormatTest, AlignArrayOfStructuresRightAlignmentNonSquare) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = FormatStyle::AIAS_Right;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+
+  // The AlignArray code is incorrect for non square Arrays and can cause
+  // crashes, these tests assert that the array is not changed but will
+  // also act as regression tests for when it is properly fixed
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3, 4, 5},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{1, 2, 3},\n"
+   "{3, 4, 5},\n"
+   "{6, 7, 8, 9, 10, 11, 12}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("S{\n"
+   "{},\n"
+   "{},\n"
+ 

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1118
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
+  for (auto i = 1U; i < CellDescs.CellCounts[0]; i++, ++CellIter) {

I don't understand in Left alignment why we ignore the first cell, but in right 
alignment, we don't!



Comment at: clang/lib/Format/WhitespaceManager.h:321
+  }
   auto Start = (CellStart + RowCount * CellCount);
   auto End = Start + Offset;

Ultimately these calculations are incorrect unless every CellCount for every 
row is the same, and for non-square they are not, this always had me confused 
as to why it starts at 1? 


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Fred Grim via Phabricator via cfe-commits]

feg208 accepted this revision.
feg208 added a comment.

I have been watching the issues pile up around this but work has prevented me 
from focusing on them. This seems like a reasonable compromise but the note of 
disabling this feature is also reasonable. Given where it had to be placed in 
the formatting flow I suspect it will be challenging to resolve most of the 
troublesome corner cases. I did investigate one bug I picked up but ran into 
the problems you described earlier and was going down the same compromise path.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@feg208 I could do with some clarity on the algorithm, am I correct in thinking 
it requires that the first row, contain at least the maximum number of columns 
for the rest of the array. I sort of noticed it was fine if the first row was 
larger

  {
  {1,2,3 }
  {4,5}
  {6}
  }




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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D121069#3362448 , @feg208 wrote:

>> This seems like a reasonable compromise

I kind of feel this might be the best initial option, I don't want to disable 
it completely because it looks like people are trying to use it, (and it seems 
to work for the square array case).

At present, I want to stop the "crash" haemorrhaging.


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

https://reviews.llvm.org/D121069

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

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:166
+/// child is a sink node.
+static bool unconditionallyLeadsHere(const ExplodedNode *N) {
+  size_t NonSinkNodeCount = llvm::count_if(

Szelethus wrote:
> xazax.hun wrote:
> > xazax.hun wrote:
> > > Consider the following code snippet:
> > > ```
> > > void f(int *p, bool b)
> > > {
> > >   if (b) {
> > > *p = 4;
> > >   }
> > >   if (p) {
> > >...
> > >   }
> > > }
> > > ```
> > > 
> > > I suspect that we would get a warning for the code above. I think warning 
> > > on the code above might be reasonable (the values of `b` and `p` might be 
> > > correlated but in some cases the analyzer has no way to know this, 
> > > probably some assertions could make the code clearer in that case).
> > > 
> > > My problem is with the wording of the error message.
> > > The warning `Pointer is unconditionally non-null here` on the null check 
> > > is not true for the code above.
> > Also, if the check would warn for the code snippet above, the note "suggest 
> > moving the condition here" would also be incorrect.
> What if we demand that the the `CFGBlock` of the dereference must dominate 
> the `CFGBlock` of the condition point?
I think it makes sense to warn both when the dereference dominates the null 
check, and when the null check post-dominates the dereference. We just want to 
give different error messages in those cases. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120992

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

[PATCH] D120984: [clang][dataflow] Extend flow conditions from block terminators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:154
+
+  // `X v (X ^ Y ^ ...)` is logically equivalent to `X`. The common conditions
+  // have already been added to the result so we don't have to do anything here

The current comment gets at the big picture, but focusing on the actual 
disjunction that is being guarded (that is, just the remaining, unshared 
clauses) may be better. So, maybe instead point out that "True v (X ^ ...)"  is 
equivalent to "True", since if either val is nullptr it represents "true"?



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:112
+
+// The condition must be inversed in one of the successors.
+if (BlockSuccIdx == 1)

Can we be more specific? I'd think we need to invert for specifically the 
successor corresponding to "else" or what not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120984

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.h:318
  Next = Next->NextColumnElement) {
+  if (RowCount > MaxRowCount) {
+break;

You can elide braces. Personally I use RemoveBraces locally.


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

https://reviews.llvm.org/D121069

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

[PATCH] D120989: Support debug info for alias variable

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Broad question about debug info for aliases: How's this proposed solution 
compare/contrast to GCC's behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

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

[PATCH] D120989: Support debug info for alias variable

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D120989#3362490 , @dblaikie wrote:

> Broad question about debug info for aliases: How's this proposed solution 
> compare/contrast to GCC's behavior?

Aaand, I see that was described in thhe patch description. Sorry for not 
reading...

Right, paging in all the context - this review hinges on whether gdb/lldb can 
handle/be made to handle this sort of debug info correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

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

[PATCH] D121061: [OpenMPIRBuilder] Allocate temporary at the correct block in a nested parallel

[William Moses via Phabricator via cfe-commits]

wsmoses updated this revision to Diff 413309.
wsmoses added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix clang test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121061

Files:
  clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
  clang/test/OpenMP/irbuilder_nested_parallel_for.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Transforms/Utils/CodeExtractor.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/HotColdSplitting.cpp
  llvm/lib/Transforms/IPO/IROutliner.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  mlir/test/Target/LLVMIR/openmp-nested.mlir

Index: mlir/test/Target/LLVMIR/openmp-nested.mlir
===
--- /dev/null
+++ mlir/test/Target/LLVMIR/openmp-nested.mlir
@@ -0,0 +1,41 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+ 
+module {
+  llvm.func @printf(!llvm.ptr, ...) -> i32
+  llvm.mlir.global internal constant @str0("WG size of kernel = %d X %d\0A\00")
+
+  llvm.func @main(%arg0: i32, %arg1: !llvm.ptr>) -> i32 {
+omp.parallel   {
+  %0 = llvm.mlir.constant(1 : index) : i64
+  %1 = llvm.mlir.constant(10 : index) : i64
+  %2 = llvm.mlir.constant(0 : index) : i64
+  %4 = llvm.mlir.constant(0 : i32) : i32
+  %12 = llvm.alloca %0 x i64 : (i64) -> !llvm.ptr
+  omp.wsloop (%arg2) : i64 = (%2) to (%1) step (%0)  {
+omp.parallel   {
+  omp.wsloop (%arg3) : i64 = (%2) to (%0) step (%0)  {
+llvm.store %2, %12 : !llvm.ptr
+omp.yield
+  }
+  omp.terminator
+}
+%19 = llvm.load %12 : !llvm.ptr
+%20 = llvm.trunc %19 : i64 to i32
+%5 = llvm.mlir.addressof @str0 : !llvm.ptr>
+%6 = llvm.getelementptr %5[%4, %4] : (!llvm.ptr>, i32, i32) -> !llvm.ptr
+%21 = llvm.call @printf(%6, %20, %20) : (!llvm.ptr, i32, i32) -> i32
+omp.yield
+  }
+  omp.terminator
+}
+%a4 = llvm.mlir.constant(0 : i32) : i32
+llvm.return %a4 : i32
+  }
+
+}
+
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @[[inner1:.+]] to void (i32*, i32*, ...)*))
+
+// CHECK: define internal void @[[inner1]]
+// CHECK: %[[structArg:.+]] = alloca { i64* }
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @3, i32 1, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, { i64* }*)* @[[inner2:.+]] to void (i32*, i32*, ...)*), { i64* }* %[[structArg]])
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -246,9 +246,10 @@
  bool AggregateArgs, BlockFrequencyInfo *BFI,
  BranchProbabilityInfo *BPI, AssumptionCache *AC,
  bool AllowVarArgs, bool AllowAlloca,
- std::string Suffix)
+ BasicBlock *AllocationBlock, std::string Suffix)
 : DT(DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-  BPI(BPI), AC(AC), AllowVarArgs(AllowVarArgs),
+  BPI(BPI), AC(AC), AllocationBlock(AllocationBlock),
+  AllowVarArgs(AllowVarArgs),
   Blocks(buildExtractionBlockSet(BBs, DT, AllowVarArgs, AllowAlloca)),
   Suffix(Suffix) {}
 
@@ -257,7 +258,7 @@
  BranchProbabilityInfo *BPI, AssumptionCache *AC,
  std::string Suffix)
 : DT(&DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-  BPI(BPI), AC(AC), AllowVarArgs(false),
+  BPI(BPI), AC(AC), AllocationBlock(nullptr), AllowVarArgs(false),
   Blocks(buildExtractionBlockSet(L.getBlocks(), &DT,
  /* AllowVarArgs */ false,
  /* AllowAlloca */ false)),
@@ -1188,9 +1189,10 @@
 
 // Allocate a struct at the beginning of this function
 StructArgTy = StructType::get(newFunction->getContext(), ArgTypes);
-Struct = new AllocaInst(StructArgTy, DL.getAllocaAddrSpace(), nullptr,
-"structArg",
-&codeReplacer->getParent()->front().front());
+Struct = new AllocaInst(
+StructArgTy, DL.getAllocaAddrSpace(), nullptr, "structArg",
+AllocationBlock ? &AllocationBlock->front()
+: &codeReplacer->getParent()->front().front());
 params.push_back(Struct);
 
 // Store aggregated inputs in the struct.
Index: llvm/lib/Transforms/IPO/IROutliner.cpp
===
--- llvm/lib/Transforms/IPO/IROu

[PATCH] D121077: Simplify OpenMP Lambda use

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added a reviewer: ABataev.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

- Use default ref capture for non-escaping lambdas (this makes maintenance 
easier by allowing new uses, removing uses, having conditional uses (such as in 
assertions) not require updates to an explicit capture list)
- Simplify addPrivate API not to take a lambda, since it calls it 
unconditionally/immediately anyway - most callers are simply passing in a named 
value or short expression anyway and the lambda syntax just adds noise/overhead


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121077

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h

Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1068,15 +1068,14 @@
 /// Enter a new OpenMP private scope.
 explicit OMPPrivateScope(CodeGenFunction &CGF) : RunCleanupsScope(CGF) {}
 
-/// Registers \p LocalVD variable as a private and apply \p PrivateGen
-/// function for it to generate corresponding private variable. \p
-/// PrivateGen returns an address of the generated private variable.
+/// Registers \p LocalVD variable as a private with \p Addr as the address
+/// of the corresponding private variable. \p
+/// PrivateGen is the address of the generated private variable.
 /// \return true if the variable is registered as private, false if it has
 /// been privatized already.
-bool addPrivate(const VarDecl *LocalVD,
-const llvm::function_ref PrivateGen) {
+bool addPrivate(const VarDecl *LocalVD, Address Addr) {
   assert(PerformCleanup && "adding private to dead scope");
-  return MappedVars.setVarAddr(CGF, LocalVD, PrivateGen());
+  return MappedVars.setVarAddr(CGF, LocalVD, Addr);
 }
 
 /// Privatizes local variables previously registered as private.
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -95,9 +95,7 @@
 isCapturedVar(CGF, VD) || (CGF.CapturedStmtInfo &&
InlinedShareds.isGlobalVarCaptured(VD)),
 VD->getType().getNonReferenceType(), VK_LValue, C.getLocation());
-InlinedShareds.addPrivate(VD, [&CGF, &DRE]() -> Address {
-  return CGF.EmitLValue(&DRE).getAddress(CGF);
-});
+InlinedShareds.addPrivate(VD, CGF.EmitLValue(&DRE).getAddress(CGF));
   }
 }
 (void)InlinedShareds.Privatize();
@@ -273,9 +271,7 @@
InlinedShareds.isGlobalVarCaptured(VD)),
   VD->getType().getNonReferenceType(), VK_LValue,
   C.getLocation());
-  InlinedShareds.addPrivate(VD, [&CGF, &DRE]() -> Address {
-return CGF.EmitLValue(&DRE).getAddress(CGF);
-  });
+  InlinedShareds.addPrivate(VD, CGF.EmitLValue(&DRE).getAddress(CGF));
 }
   }
   CS = dyn_cast(CS->getCapturedStmt());
@@ -630,9 +626,8 @@
   CodeGenFunction::OMPPrivateScope LocalScope(*this);
   for (const auto &LocalAddrPair : LocalAddrs) {
 if (LocalAddrPair.second.first) {
-  LocalScope.addPrivate(LocalAddrPair.second.first, [&LocalAddrPair]() {
-return LocalAddrPair.second.second;
-  });
+  LocalScope.addPrivate(LocalAddrPair.second.first,
+LocalAddrPair.second.second);
 }
   }
   (void)LocalScope.Privatize();
@@ -779,8 +774,8 @@
 // destination and source variables to corresponding array
 // elements.
 CodeGenFunction::OMPPrivateScope Remap(*this);
-Remap.addPrivate(DestVD, [DestElement]() { return DestElement; });
-Remap.addPrivate(SrcVD, [SrcElement]() { return SrcElement; });
+Remap.addPrivate(DestVD, DestElement);
+Remap.addPrivate(SrcVD, SrcElement);
 (void)Remap.Privatize();
 EmitIgnoredExpr(Copy);
   });
@@ -788,8 +783,8 @@
   } else {
 // Remap pseudo source variable to private copy.
 CodeGenFunction::OMPPrivateScope Remap(*this);
-Remap.addPrivate(SrcVD, [SrcAddr]() { return SrcAddr; });
-Remap.addPrivate(DestVD, [DestAddr]() { return DestAddr; });
+Remap.addPrivate(SrcVD, SrcAddr);
+Remap.addPrivate(DestVD, DestAddr);
 (void)Remap.Privatize();
 // Emit copying of the whole variable.
 EmitIgnoredExpr(Copy);
@@ -878,68 +873,56 @@
   // Emit VarDecl with copy init for arrays.
   // Get the address of the

Re: [clang] 1d1b089 - Fix more unused lambda capture warnings, NFC

[David Blaikie via cfe-commits]

On Mon, Feb 28, 2022 at 10:12 AM Reid Kleckner  wrote:

> I agree, but clearly this person chose a particular style, and I wasn't
> trying to revise their code style, just to fix some the Bazel build, which
> uses -Werror.
>

Oh, wow, yeah, this file/corner of the codebase is deep into lambdas, wow.
Sent https://reviews.llvm.org/D121077 for the broader change (both the
capture lists and at least one function that seemed to not really benefit
from taking a lambda - maybe other functions could have similar cleanup)


> The way that `assert` can affect a lambda's capture list seems like a
> particularly good argument for having a project wide style recommendation
> to never use explicit capture lists.
>

Yeah, not sure it's worth words in the style guide unless it's especially
contentious (the presence of code like this might argue that it is - but
hopefully even in this case only pointing it out will be enough) but maybe.

- Dave


>
> On Mon, Feb 28, 2022 at 9:45 AM David Blaikie  wrote:
>
>> FWIW, I think it's probably simpler/more maintainable to default capture
>> by reference ("[&]") if a lambda doesn't escape its scope (if it's never
>> type erased/put in a std::function or equivalent). Avoids assert/non-assert
>> unused issues, having to maintain/update the list when the code changes and
>> makes a capture live/dead/etc.
>>
>> On Wed, Feb 23, 2022 at 2:09 PM Reid Kleckner via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>> Author: Reid Kleckner
>>> Date: 2022-02-23T14:07:04-08:00
>>> New Revision: 1d1b089c5d503e2fc8697887411730105f66c774
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/1d1b089c5d503e2fc8697887411730105f66c774.diff
>>>
>>> LOG: Fix more unused lambda capture warnings, NFC
>>>
>>> Added:
>>>
>>>
>>> Modified:
>>> clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> 
>>> diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> index b4033da890c4..3f4a78ddbf3c 100644
>>> --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
>>> @@ -10358,8 +10358,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>>llvm::Value *MapTypesArray = nullptr;
>>>llvm::Value *MapNamesArray = nullptr;
>>>// Generate code for the host fallback function.
>>> -  auto &&FallbackGen = [this, OutlinedFn, &D, &CapturedVars,
>>> RequiresOuterTask,
>>> -&CS, OffloadingMandatory](CodeGenFunction &CGF)
>>> {
>>> +  auto &&FallbackGen = [this, &D, OutlinedFn, &CapturedVars,
>>> RequiresOuterTask, &CS,
>>> +OffloadingMandatory](CodeGenFunction &CGF) {
>>>  if (OffloadingMandatory) {
>>>CGF.Builder.CreateUnreachable();
>>>  } else {
>>> @@ -10371,9 +10371,8 @@ void CGOpenMPRuntime::emitTargetCall(
>>>  }
>>>};
>>>// Fill up the pointer arrays and transfer execution to the device.
>>> -  auto &&ThenGen = [this, Device, OutlinedFn, OutlinedFnID, &D,
>>> &InputInfo,
>>> -&MapTypesArray, &MapNamesArray, &CS,
>>> RequiresOuterTask,
>>> -&CapturedVars, SizeEmitter,
>>> +  auto &&ThenGen = [this, Device, OutlinedFnID, &D, &InputInfo,
>>> +&MapTypesArray, &MapNamesArray, SizeEmitter,
>>>  FallbackGen](CodeGenFunction &CGF, PrePostActionTy
>>> &) {
>>>  if (Device.getInt() == OMPC_DEVICE_ancestor) {
>>>// Reverse offloading is not supported, so just execute on the
>>> host.
>>> @@ -10392,6 +10391,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>>
>>>  // From this point on, we need to have an ID of the target region
>>> defined.
>>>  assert(OutlinedFnID && "Invalid outlined function ID!");
>>> +(void)OutlinedFnID;
>>>
>>>  // Emit device ID if any.
>>>  llvm::Value *DeviceID;
>>> @@ -10529,8 +10529,7 @@ void CGOpenMPRuntime::emitTargetCall(
>>>};
>>>
>>>// Notify that the host version must be executed.
>>> -  auto &&ElseGen = [this, &D, OutlinedFn, &CS, &CapturedVars,
>>> RequiresOuterTask,
>>> -FallbackGen](CodeGenFunction &CGF, PrePostActionTy
>>> &) {
>>> +  auto &&ElseGen = [FallbackGen](CodeGenFunction &CGF, PrePostActionTy
>>> &) {
>>>  FallbackGen(CGF);
>>>};
>>>
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

[Danny Mösch via Phabricator via cfe-commits]

SimplyDanny created this revision.
SimplyDanny added a reviewer: tonic.
Herald added subscribers: libcxx-commits, arphaman.
Herald added a reviewer: sscalpone.
Herald added projects: libunwind, Flang, All.
Herald added a reviewer: libunwind.
SimplyDanny requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, Sanitizers, 
jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, libc++, LLVM, clang-tools-extra.
Herald added a reviewer: libc++.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121078

Files:
  clang-tools-extra/README.txt
  clang/README.txt
  clang/www/analyzer/menu.html.incl
  clang/www/demo/index.cgi
  clang/www/menu.html.incl
  compiler-rt/www/menu.html.incl
  flang/docs/GettingInvolved.md
  libcxx/docs/index.rst
  libunwind/docs/index.rst
  lldb/docs/index.rst
  llvm/docs/Contributing.rst
  llvm/docs/ExtendingLLVM.rst
  llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl10.rst

Index: llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl10.rst
===
--- llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl10.rst
+++ llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl10.rst
@@ -87,8 +87,8 @@
 Have fun - try doing something crazy and unusual. Building a language
 like everyone else always has, is much less fun than trying something a
 little crazy or off the wall and seeing how it turns out. If you get
-stuck or want to talk about it, feel free to email the `llvm-dev mailing
-list `_: it has lots
+stuck or want to talk about it, feel free to open a thread in the `LLVM
+forum `_: it has lots
 of people who are interested in languages and are often willing to help
 out.
 
@@ -166,9 +166,8 @@
 IR does not itself guarantee safety. The LLVM IR allows unsafe pointer
 casts, use after free bugs, buffer over-runs, and a variety of other
 problems. Safety needs to be implemented as a layer on top of LLVM and,
-conveniently, several groups have investigated this. Ask on the `llvm-dev
-mailing list `_ if
-you are interested in more details.
+conveniently, several groups have investigated this. Ask in the `LLVM
+forum `_ if you are interested in more details.
 
 Language-Specific Optimizations
 ---
Index: llvm/docs/ExtendingLLVM.rst
===
--- llvm/docs/ExtendingLLVM.rst
+++ llvm/docs/ExtendingLLVM.rst
@@ -14,8 +14,8 @@
 When you come to this realization, stop and think. Do you really need to extend
 LLVM? Is it a new fundamental capability that LLVM does not support at its
 current incarnation or can it be synthesized from already pre-existing LLVM
-elements? If you are not sure, ask on the `LLVM-dev
-`_ list. The reason is that
+elements? If you are not sure, ask in the `LLVM forum
+`_ list. The reason is that
 extending LLVM will get involved as you need to update all the different passes
 that you intend to use with your extension, and there are ``many`` LLVM analyses
 and transformations, so it may be quite a bit of work.
Index: llvm/docs/Contributing.rst
===
--- llvm/docs/Contributing.rst
+++ llvm/docs/Contributing.rst
@@ -147,7 +147,7 @@
 
   .. __: http://www.aosabook.org/en/llvm.html
 
-.. _Developer's List (llvm-dev): http://lists.llvm.org/mailman/listinfo/llvm-dev
+.. _Forum: https://discourse.llvm.org
 .. _irc.oftc.net: irc://irc.oftc.net/llvm
 .. _beginner: https://github.com/llvm/llvm-project/issues?q=is%3Aopen+is%3Aissue+label%3Abeginner
 .. _bug tracker: https://github.com/llvm/llvm-project/issues
@@ -155,4 +155,3 @@
 .. _git-clang-format: https://reviews.llvm.org/source/llvm-github/browse/main/clang/tools/clang-format/git-clang-format
 .. _LLVM's Phabricator: https://reviews.llvm.org/
 .. _LLVM's Open Projects page: https://llvm.org/OpenProjects.html#what
-.. _LLVM Developer's mailing list: http://lists.llvm.org/mailman/listinfo/llvm-dev
Index: lldb/docs/index.rst
===
--- lldb/docs/index.rst
+++ lldb/docs/index.rst
@@ -102,10 +102,10 @@
 
 See the :doc:`LLDB Build Page ` for build instructions.
 
-Discussions about LLDB should go to the `lldb-dev
-`__ mailing list. Commit
+Discussions about LLDB should go to the `LLDB forum
+`__. Commit
 messages are automatically sent to the `lldb-commits
-`__ mailing list , and
+`__ mailing list, and
 this is also the preferred mailing list for patch submissions.
 
 See the :doc:`Projects page

[PATCH] D120187: [clang-tidy] Allow newline characters as separators for checks in Clang-Tidy configurations

[Danny Mösch via Phabricator via cfe-commits]

SimplyDanny added a comment.
Herald added a project: All.

What else can be done to get this change approved, @njames93?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120187

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Fred Grim via Phabricator via cfe-commits]

feg208 added a comment.

In D121069#3362450 , @MyDeveloperDay 
wrote:

> @feg208 I could do with some clarity on the algorithm, am I correct in 
> thinking it requires that the first row, to contain at least the maximum 
> number of columns for the rest of the array. I sort of noticed it was fine if 
> the first row was larger
>
>   {
>   {1,2,3 }
>   {4,5}
>   {6}
>   }

Honestly it really is best with the square option. The assumption is an array 
of simple initializers though default args to constructors complicate that in 
the case of an array of single constructors. I think that you are seeing it be 
okay with the maximum as the first row is just an accident of the 
implementation. The direction I was heading in in the bug I picked up was to 
start restricting the set of things it would consider as array of initializers. 
Does that answer your question?


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

https://reviews.llvm.org/D121069

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

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 413313.
sstwcw added a comment.

remove `int` in tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestJS.cpp

Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2699,7 +2699,7 @@
 
 TEST_F(FormatTestJS, AlignConsecutiveDeclarations) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("letletVariable = 5;\n"
"double constVariable = 10;",
Style);
@@ -2736,7 +2736,7 @@
 TEST_F(FormatTestJS, AlignConsecutiveAssignments) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
 
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("let letVariable  = 5;\n"
"double constVariable = 10;",
Style);
@@ -2772,8 +2772,8 @@
 
 TEST_F(FormatTestJS, AlignConsecutiveAssignmentsAndDeclarations) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_JavaScript);
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+  Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("letletVariable   = 5;\n"
"double constVariable = 10;",
Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2076,7 +2076,7 @@
   " res2 = [](int &a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int *c;\n"
"const unsigned int *d;\n"
"Const unsigned int &e;\n"
@@ -2117,7 +2117,7 @@
   "res2 = [](int& a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int* c;\n"
"const unsigned int* d;\n"
"Const unsigned int& e;\n"
@@ -2138,7 +2138,7 @@
   verifyFormat("for (int a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo *c : {1, 2, 3})", Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int *c;\n"
"const unsigned int *d;\n"
"Const unsigned int& e;\n"
@@ -2174,7 +2174,7 @@
   " res2 = [](int & a) { return 0; };",
   Style);
 
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int*  c;\n"
"const unsigned int*  d;\n"
"Const unsigned int & e;\n"
@@ -14425,8 +14425,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveAssignments.Enabled = true;
+  Tab.AlignConsecutiveDeclarations.Enabled = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -14664,8 +14664,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveAssignments.Enabled = true;
+  Tab.AlignConsecutiveDeclarations.Enabled = true;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -15835,9 +15835,8 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
-  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
 
   verifyFormat("#define a 3\n"
   

[PATCH] D121061: [OpenMPIRBuilder] Allocate temporary at the correct block in a nested parallel

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG, two nits.




Comment at: llvm/include/llvm/Transforms/Utils/CodeExtractor.h:127
 /// extraction of blocks containing alloca instructions would be possible,
 /// however code extractor won't validate whether extraction is legal.
 CodeExtractor(ArrayRef BBs, DominatorTree *DT = nullptr,

Nit: Mention the AllocationBlock here too.



Comment at: llvm/lib/Transforms/Utils/CodeExtractor.cpp:1194
+StructArgTy, DL.getAllocaAddrSpace(), nullptr, "structArg",
+AllocationBlock ? &AllocationBlock->front()
+: &codeReplacer->getParent()->front().front());




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121061

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1073
+  auto *Start =
+  (Cells.begin() + RowCount * CellDescs.CellCounts[RowCount]);
   auto *End = Start + Offset;

Can we use `CellCounts[0]` instead? The outer parentheses are superfluous. Same 
in `alignArrayInitializersLeftJustified` below.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1118
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
+  for (auto i = 1U; i < CellDescs.CellCounts[0]; i++, ++CellIter) {

MyDeveloperDay wrote:
> I don't understand in Left alignment why we ignore the first cell, but in 
> right alignment, we don't!
Because the first cell is already left-aligned by default?



Comment at: clang/lib/Format/WhitespaceManager.h:204
+// has the same number of columns.
+bool HasEqualRowLengths() const {
+  if (CellCounts.empty())

My top choice by far is `isRectangular`.



Comment at: clang/lib/Format/WhitespaceManager.h:211
+  return false;
+  }
+  return true;

We can omit the braces here.



Comment at: clang/lib/Format/WhitespaceManager.h:318
  Next = Next->NextColumnElement) {
+  if (RowCount > MaxRowCount) {
+break;

curdeius wrote:
> You can elide braces. Personally I use RemoveBraces locally.
Why do we need this check? Is it because `CellStop` might not be null 
terminated? Or is it because `CellCount` may be zero? If the latter, we can 
check `CellCount` before the loop instead of this check in the loop.



Comment at: clang/unittests/Format/FormatTest.cpp:25316
+   "{6, 7, 8}\n"
+   "};\n",
+   Style);

Please remove `\n` here and in other places below when it's the last line of a 
test case.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D121069#3362424 , @MyDeveloperDay 
wrote:

> NOTE: I've tried to collate all the reported crashing examples and run this 
> fix through them (both Left and Right) all pass except this one below
>
>
>
>   void foo()
>   {
>   auto thing = test{
>   {
>  {13},
>  {something}, //A
>   }
> };
>   }
>
> This is not related to it being non-square because it's not.

I think had a simple fix for this. I will revisit it after you land this patch.




Comment at: clang/lib/Format/WhitespaceManager.h:202-203
+
+// Determine if every row in the the array
+// has the same number of columns.
+bool HasEqualRowLengths() const {

Do they fit in one line?


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.h:318
  Next = Next->NextColumnElement) {
+  if (RowCount > MaxRowCount) {
+break;

owenpan wrote:
> curdeius wrote:
> > You can elide braces. Personally I use RemoveBraces locally.
> Why do we need this check? Is it because `CellStop` might not be null 
> terminated? Or is it because `CellCount` may be zero? If the latter, we can 
> check `CellCount` before the loop instead of this check in the loop.
Ok, you already explained why you added the check, but I still wonder what 
causes it.


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

https://reviews.llvm.org/D121069

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

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.h:321
+  }
   auto Start = (CellStart + RowCount * CellCount);
   auto End = Start + Offset;

MyDeveloperDay wrote:
> Ultimately these calculations are incorrect unless every CellCount for every 
> row is the same, and for non-square they are not, this always had me confused 
> as to why it starts at 1? 
> Ultimately these calculations are incorrect unless every CellCount for every 
> row is the same, and for non-square they are not, this always had me confused 
> as to why it starts at 1? 

Because the loop starts at `CellStop->NextColumnElement`, the 2nd row of the 
column?


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

https://reviews.llvm.org/D121069

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

[clang] 87ec6f4 - [OpenMPIRBuilder] Allocate temporary at the correct block in a nested parallel

[William S. Moses via cfe-commits]

Author: William S. Moses
Date: 2022-03-06T18:34:25-05:00
New Revision: 87ec6f41bba6d72a3408e71cf19ae56feff523bc

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

LOG: [OpenMPIRBuilder] Allocate temporary at the correct block in a nested 
parallel

The OpenMPIRBuilder has a bug. Specifically, suppose you have two nested openmp 
parallel regions (writing with MLIR for ease)

```
omp.parallel {
  %a = ...
  omp.parallel {
use(%a)
  }
}
```

As OpenMP only permits pointer-like inputs, the builder will wrap all of the 
inputs into a stack allocation, and then pass this
allocation to the inner parallel. For example, we would want to get something 
like the following:

```
omp.parallel {
  %a = ...
  %tmp = alloc
  store %tmp[] = %a
  kmpc_fork(outlined, %tmp)
}
```

However, in practice, this is not what currently occurs in the context of 
nested parallel regions. Specifically to the OpenMPIRBuilder,
the entirety of the function (at the LLVM level) is currently inlined with 
blocks marking the corresponding start and end of each
region.

```
entry:
  ...

parallel1:
  %a = ...
  ...

parallel2:
  use(%a)
  ...

endparallel2:
  ...

endparallel1:
  ...
```

When the allocation is inserted, it presently inserted into the parent of the 
entire function (e.g. entry) rather than the parent
allocation scope to the function being outlined. If we were outlining 
parallel2, the corresponding alloca location would be parallel1.

This causes a variety of bugs, including 
https://github.com/llvm/llvm-project/issues/54165 as one example.

This PR allows the stack allocation to be created at the correct allocation 
block, and thus remedies such issues.

Reviewed By: jdoerfert

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

Added: 
mlir/test/Target/LLVMIR/openmp-nested.mlir

Modified: 
clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
clang/test/OpenMP/irbuilder_nested_parallel_for.c
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/include/llvm/Transforms/Utils/CodeExtractor.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/Transforms/IPO/HotColdSplitting.cpp
llvm/lib/Transforms/IPO/IROutliner.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp

Removed: 




diff  --git a/clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c 
b/clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
index 56aa41b0db466..2c1139d7ef7df 100644
--- a/clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
+++ b/clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
@@ -33,8 +33,7 @@ void nested_parallel_0(void) {
 
 // ALL-LABEL: @_Z17nested_parallel_1Pfid(
 // ALL-NEXT:  entry:
-// ALL-NEXT:[[STRUCTARG14:%.*]] = alloca { { i32*, double*, float** }*, 
i32*, double*, float** }, align 8
-// ALL-NEXT:[[STRUCTARG:%.*]] = alloca { i32*, double*, float** }, align 8
+// ALL-NEXT:[[STRUCTARG14:%.*]] = alloca { i32*, double*, float** }, align 
8
 // ALL-NEXT:[[R_ADDR:%.*]] = alloca float*, align 8
 // ALL-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
 // ALL-NEXT:[[B_ADDR:%.*]] = alloca double, align 8
@@ -44,15 +43,13 @@ void nested_parallel_0(void) {
 // ALL-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 
@__kmpc_global_thread_num(%struct.ident_t* @[[GLOB1]])
 // ALL-NEXT:br label [[OMP_PARALLEL:%.*]]
 // ALL:   omp_parallel:
-// ALL-NEXT:[[GEP_STRUCTARG:%.*]] = getelementptr { { i32*, double*, 
float** }*, i32*, double*, float** }, { { i32*, double*, float** }*, i32*, 
double*, float** }* [[STRUCTARG14]], i32 0, i32 0
-// ALL-NEXT:store { i32*, double*, float** }* [[STRUCTARG]], { i32*, 
double*, float** }** [[GEP_STRUCTARG]], align 8
-// ALL-NEXT:[[GEP_A_ADDR15:%.*]] = getelementptr { { i32*, double*, 
float** }*, i32*, double*, float** }, { { i32*, double*, float** }*, i32*, 
double*, float** }* [[STRUCTARG14]], i32 0, i32 1
+// ALL-NEXT:[[GEP_A_ADDR15:%.*]] = getelementptr { i32*, double*, float** 
}, { i32*, double*, float** }* [[STRUCTARG14]], i32 0, i32 0
 // ALL-NEXT:store i32* [[A_ADDR]], i32** [[GEP_A_ADDR15]], align 8
-// ALL-NEXT:[[GEP_B_ADDR16:%.*]] = getelementptr { { i32*, double*, 
float** }*, i32*, double*, float** }, { { i32*, double*, float** }*, i32*, 
double*, float** }* [[STRUCTARG14]], i32 0, i32 2
+// ALL-NEXT:[[GEP_B_ADDR16:%.*]] = getelementptr { i32*, double*, float** 
}, { i32*, double*, float** }* [[STRUCTARG14]], i32 0, i32 1
 // ALL-NEXT:store double* [[B_ADDR]], double** [[GEP_B_ADDR16]], align 8
-// ALL-NEXT:[[GEP_R_ADDR17:%.*]] = getelementptr { { i32*, double*, 
float** }*, i32*, double*, float** }, { { i32*, double*, float** }*, i32*, 
double*, float** }* [[STRUCTARG14]], i32 0, i32 3
+// ALL-NEXT:[[GEP_R_ADDR17:%.*]] = getelementptr { i32*, double*, 

[PATCH] D121061: [OpenMPIRBuilder] Allocate temporary at the correct block in a nested parallel

[William Moses via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
wsmoses marked an inline comment as done.
Closed by commit rG87ec6f41bba6: [OpenMPIRBuilder] Allocate temporary at the 
correct block in a nested parallel (authored by wsmoses).

Changed prior to commit:
  https://reviews.llvm.org/D121061?vs=413309&id=413321#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121061

Files:
  clang/test/OpenMP/irbuilder_nested_openmp_parallel_empty.c
  clang/test/OpenMP/irbuilder_nested_parallel_for.c
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Transforms/Utils/CodeExtractor.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/HotColdSplitting.cpp
  llvm/lib/Transforms/IPO/IROutliner.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  mlir/test/Target/LLVMIR/openmp-nested.mlir

Index: mlir/test/Target/LLVMIR/openmp-nested.mlir
===
--- /dev/null
+++ mlir/test/Target/LLVMIR/openmp-nested.mlir
@@ -0,0 +1,41 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+ 
+module {
+  llvm.func @printf(!llvm.ptr, ...) -> i32
+  llvm.mlir.global internal constant @str0("WG size of kernel = %d X %d\0A\00")
+
+  llvm.func @main(%arg0: i32, %arg1: !llvm.ptr>) -> i32 {
+omp.parallel   {
+  %0 = llvm.mlir.constant(1 : index) : i64
+  %1 = llvm.mlir.constant(10 : index) : i64
+  %2 = llvm.mlir.constant(0 : index) : i64
+  %4 = llvm.mlir.constant(0 : i32) : i32
+  %12 = llvm.alloca %0 x i64 : (i64) -> !llvm.ptr
+  omp.wsloop (%arg2) : i64 = (%2) to (%1) step (%0)  {
+omp.parallel   {
+  omp.wsloop (%arg3) : i64 = (%2) to (%0) step (%0)  {
+llvm.store %2, %12 : !llvm.ptr
+omp.yield
+  }
+  omp.terminator
+}
+%19 = llvm.load %12 : !llvm.ptr
+%20 = llvm.trunc %19 : i64 to i32
+%5 = llvm.mlir.addressof @str0 : !llvm.ptr>
+%6 = llvm.getelementptr %5[%4, %4] : (!llvm.ptr>, i32, i32) -> !llvm.ptr
+%21 = llvm.call @printf(%6, %20, %20) : (!llvm.ptr, i32, i32) -> i32
+omp.yield
+  }
+  omp.terminator
+}
+%a4 = llvm.mlir.constant(0 : i32) : i32
+llvm.return %a4 : i32
+  }
+
+}
+
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @[[inner1:.+]] to void (i32*, i32*, ...)*))
+
+// CHECK: define internal void @[[inner1]]
+// CHECK: %[[structArg:.+]] = alloca { i64* }
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @3, i32 1, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, { i64* }*)* @[[inner2:.+]] to void (i32*, i32*, ...)*), { i64* }* %[[structArg]])
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -246,9 +246,10 @@
  bool AggregateArgs, BlockFrequencyInfo *BFI,
  BranchProbabilityInfo *BPI, AssumptionCache *AC,
  bool AllowVarArgs, bool AllowAlloca,
- std::string Suffix)
+ BasicBlock *AllocationBlock, std::string Suffix)
 : DT(DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-  BPI(BPI), AC(AC), AllowVarArgs(AllowVarArgs),
+  BPI(BPI), AC(AC), AllocationBlock(AllocationBlock),
+  AllowVarArgs(AllowVarArgs),
   Blocks(buildExtractionBlockSet(BBs, DT, AllowVarArgs, AllowAlloca)),
   Suffix(Suffix) {}
 
@@ -257,7 +258,7 @@
  BranchProbabilityInfo *BPI, AssumptionCache *AC,
  std::string Suffix)
 : DT(&DT), AggregateArgs(AggregateArgs || AggregateArgsOpt), BFI(BFI),
-  BPI(BPI), AC(AC), AllowVarArgs(false),
+  BPI(BPI), AC(AC), AllocationBlock(nullptr), AllowVarArgs(false),
   Blocks(buildExtractionBlockSet(L.getBlocks(), &DT,
  /* AllowVarArgs */ false,
  /* AllowAlloca */ false)),
@@ -1189,9 +1190,10 @@
 
 // Allocate a struct at the beginning of this function
 StructArgTy = StructType::get(newFunction->getContext(), ArgTypes);
-Struct = new AllocaInst(StructArgTy, DL.getAllocaAddrSpace(), nullptr,
-"structArg",
-&codeReplacer->getParent()->front().front());
+Struct = new AllocaInst(
+StructArgTy, DL.getAllocaAddrSpace(), nullptr, "structArg",
+AllocationBlock ? &*AllocationBlock->getFirstInsertionPt()
+: &codeReplacer->getParen

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D118352#3359626 , @urnathan wrote:

> In D118352#3358864 , @ChuanqiXu 
> wrote:
>
>> If I don't misread the codes, it looks like `mangleModuleInitializer` is not 
>> called.
>
>
>
>> Now we could add test for partitions.
>
> Correct, it is not called as the global initializer pieces are not yet 
> implemented.  Let's not hold up this patch for that nor remove piece that 
> will become necessary at that point.

It's odd to see unused functions. I just worry about that other people might 
delete these functions as cleaning up (maybe there wouldn't be).

---

Although the general declarations in partitions wouldn't be mangled specially, 
I think it would be better to add test case to show that.


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

https://reviews.llvm.org/D118352

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

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D120540#3359454 , @iains wrote:

> In D120540#3359446 , @ChuanqiXu 
> wrote:
>
>> In D120540#3359394 , @iains wrote:
>>
>>> 1. I agree 100% that the driver needs to be able to process in "c++20 
>>> modules mode";
>>
>> Yeah, not all the projects could be able to run in c++20 mode. We use 
>> `-std=c++17` + `-fcoroutines-ts` for C++20 coroutine before.
>>
>>> there is some handling of sources that should be changed accordingly.
>>
>> If I don't misunderstand, I guess we don't. Since all the code about C++20 
>> modules are controlled by `CPlusPlusModules` variable. I think there 
>> wouldn't be codes controlled by `Cpp20` variables.
>
> Actually, the comment was intending to refer to the driver specifically - 
> since (for example) we have to disambiguate PCH jobs from C++20 Header Unit 
> jobs - in the Front End, as of now we have been using CPlusPlus modules as 
> the indicator for C++20 modules.  There is still scope for confusion if that 
> is set at the same time as alternates...

Oh, I didn't see Header Unit into details. I'm mainly focus on named module 
now. From my personal point, I feel header unit and named module are really not 
the same in many aspects so that we could handle them separately.

>>> 2. I believe that it is a general objective of the tooling folks (roughly 
>>> SG15 participants) that C++20 modules should (eventually) be considered 
>>> automatic for C++20+
>>
>> If I understand correctly, clang would enable C++20 modules by default if 
>> C++20 is enabled.
>
> Yes, exactly, as is the case right now.
>
>>> 3. There is at least one request from tooling folks that there should be an 
>>> option to disable modules (even when in C++20  mode); this is a practical 
>>> measure to avoid the case that it is impossible to build a TU because of 
>>> some potential modules-related bug ...
>>
>> I think `-fno-cxxmodules` could be the option now.
>
> so long as this is decoupled from any clang modules meanings?

Yes, `fcxx-modules` only refers to C++20 modules now.

>>> 4. IMO it becomes increasingly important to try and decouple the clang 
>>> modules from C++20 modules as much as possible.
>>
>> 100% agree. I think it should be beneficial to decouple them from command 
>> line, implementation and comments. (Now many comments are not precise. For 
>> example, when we talk about a `Module`. we are referring a module unit 
>> indeed. There are other examples.)
>
> It would be friendly to the user to reject command line options that are not 
> appropriate to the "current modules mode" - since there are ≈ 60+ 
> modules-related command line options it is already very easy to be confused.  
> To do this, the driver needs to establish the "current modules mode" early 
> (even before it builds jobs, since as noted above some jobs build differently 
> depending on the assumed mode).

Totally agreed. It would be very hard process to disambuguate them since there 
are already users..

>>> So .. I was going to suggest that we might introduce -fmodules= {none, 
>>> clang, c++20, ...} 
>>> with defaults picked:
>>>
>>>fmodules => clang (i.e. the current meaning)
>>>   
>>>   !fmodules && C++20+ => c++20 (i.e. the objective of (2) above
>>>   Where there are other flags that imply C++20 that can switch c++20 mode 
>>> as well
>>>   
>>>   otherwise the default would be "none"
>>>
>>> .. this provides for (3) ... since -std=c++20 -fmodules=none could be used.
>>>
>>> I do not have a patch for this proposal as of this time (my current patches 
>>> assume (2) but do not meet the objective of (3))
>>
>> The proposal is good to me though. I sent a patch 
>> (https://reviews.llvm.org/D113391) before to forbid the mixed use of clang 
>> modules and c++20 modules since it might be confusing if we use the 
>> combination `-fmodules -std=c++20`. But the comment shows that the current 
>> users of Clang Modules (mainly Google and Apple) wish a smooth switch from 
>> clang module to c++20 module. So I think the proposal which would break the 
>> current use cases would be not easy to land. 
>> (This patch wouldn't break any use case I think).
>
> Well, I think neither proposal breaks current use-cases - the selection of 
> defaults is designed to make current command lines do exactly the same as 
> they do now.
>
> My comments are not intended as a blocker for your patch -  but simply to 
> offer a suggestion for a more generic handling of the same objective.

Got it, thanks. My objective is relatively smaller. I just want to enable the 
use of C++20 modules for actual projects. (I heard that there are projects 
couldn't upgrade to C++20 due to a ABI break in std::string). Given the command 
line design of coroutines, I think this might be acceptable.


Repository:
  rG LLVM Github Monorepo

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

h

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@urnathan It looks @rsmith is not available this time. Would you like to accept 
this one?


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

https://reviews.llvm.org/D118034

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

[PATCH] D116015: [PowerPC] Add generic fnmsub intrinsic

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 41.
qiucf marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116015

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
  clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
  clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrVSX.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
@@ -98,49 +98,104 @@
 
 declare float @llvm.ppc.fnmadds(float, float, float)
 
-define dso_local double @fnmsub_t0(double %d, double %d2, double %d3) {
-; CHECK-PWR8-LABEL: fnmsub_t0:
+define dso_local float @fnmsub_f32(float %f, float %f2, float %f3) {
+; CHECK-PWR8-LABEL: fnmsub_f32:
 ; CHECK-PWR8:   # %bb.0: # %entry
-; CHECK-PWR8-NEXT:xsnmsubmdp 1, 2, 3
+; CHECK-PWR8-NEXT:xsnmsubasp 3, 1, 2
+; CHECK-PWR8-NEXT:fmr 1, 3
 ; CHECK-PWR8-NEXT:blr
 ;
-; CHECK-NOVSX-LABEL: fnmsub_t0:
+; CHECK-NOVSX-LABEL: fnmsub_f32:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:blr
+;
+; CHECK-PWR7-LABEL: fnmsub_f32:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-PWR7-NEXT:blr
+entry:
+  %0 = tail call float @llvm.ppc.fnmsub.f32(float %f, float %f2, float %f3)
+  ret float %0
+}
+
+declare float @llvm.ppc.fnmsub.f32(float, float, float)
+
+define dso_local double @fnmsub_f64(double %f, double %f2, double %f3) {
+; CHECK-PWR8-LABEL: fnmsub_f64:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xsnmsubadp 3, 1, 2
+; CHECK-PWR8-NEXT:fmr 1, 3
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: fnmsub_f64:
 ; CHECK-NOVSX:   # %bb.0: # %entry
 ; CHECK-NOVSX-NEXT:fnmsub 1, 1, 2, 3
 ; CHECK-NOVSX-NEXT:blr
 ;
-; CHECK-PWR7-LABEL: fnmsub_t0:
+; CHECK-PWR7-LABEL: fnmsub_f64:
 ; CHECK-PWR7:   # %bb.0: # %entry
-; CHECK-PWR7-NEXT:xsnmsubmdp 1, 2, 3
+; CHECK-PWR7-NEXT:xsnmsubadp 3, 1, 2
+; CHECK-PWR7-NEXT:fmr 1, 3
 ; CHECK-PWR7-NEXT:blr
 entry:
-  %0 = tail call double @llvm.ppc.fnmsub(double %d, double %d2, double %d3)
+  %0 = tail call double @llvm.ppc.fnmsub.f64(double %f, double %f2, double %f3)
   ret double %0
 }
 
-declare double @llvm.ppc.fnmsub(double, double, double)
+declare double @llvm.ppc.fnmsub.f64(double, double, double)
 
-define dso_local float @fnmsubs_t0(float %f, float %f2, float %f3) {
-; CHECK-PWR8-LABEL: fnmsubs_t0:
+define dso_local <4 x float> @fnmsub_v4f32(<4 x float> %f, <4 x float> %f2, <4 x float> %f3) {
+; CHECK-PWR8-LABEL: fnmsub_v4f32:
 ; CHECK-PWR8:   # %bb.0: # %entry
-; CHECK-PWR8-NEXT:xsnmsubmsp 1, 2, 3
+; CHECK-PWR8-NEXT:xvnmsubasp 36, 34, 35
+; CHECK-PWR8-NEXT:vmr 2, 4
 ; CHECK-PWR8-NEXT:blr
 ;
-; CHECK-NOVSX-LABEL: fnmsubs_t0:
+; CHECK-NOVSX-LABEL: fnmsub_v4f32:
 ; CHECK-NOVSX:   # %bb.0: # %entry
-; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 5, 9
+; CHECK-NOVSX-NEXT:fnmsubs 2, 2, 6, 10
+; CHECK-NOVSX-NEXT:fnmsubs 3, 3, 7, 11
+; CHECK-NOVSX-NEXT:fnmsubs 4, 4, 8, 12
 ; CHECK-NOVSX-NEXT:blr
 ;
-; CHECK-PWR7-LABEL: fnmsubs_t0:
+; CHECK-PWR7-LABEL: fnmsub_v4f32:
 ; CHECK-PWR7:   # %bb.0: # %entry
-; CHECK-PWR7-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-PWR7-NEXT:xvnmsubasp 36, 34, 35
+; CHECK-PWR7-NEXT:vmr 2, 4
 ; CHECK-PWR7-NEXT:blr
 entry:
-  %0 = tail call float @llvm.ppc.fnmsubs(float %f, float %f2, float %f3)
-  ret float %0
+  %0 = tail call <4 x float> @llvm.ppc.fnmsub.v4f32(<4 x float> %f, <4 x float> %f2, <4 x float> %f3)
+  ret <4 x float> %0
+}
+
+declare <4 x float> @llvm.ppc.fnmsub.v4f32(<4 x float>, <4 x float>, <4 x float>)
+
+define dso_local <2 x double> @fnmsub_v2f64(<2 x double> %f, <2 x double> %f2, <2 x double> %f3) {
+; CHECK-PWR8-LABEL: fnmsub_v2f64:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xvnmsubadp 36, 34, 35
+; CHECK-PWR8-NEXT:vmr 2, 4
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: fnmsub_v2f64:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fnmsub 1, 1, 3, 5
+; CHECK-NOVSX-NEXT:fnmsub 2, 2, 4, 6
+; CHECK-NOVSX-NEXT:blr
+;
+; CHECK-PWR7-LABEL: fnmsub_v2f64:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:xvnmsubadp 36, 34, 35
+; CHECK-PWR7-NEXT:vmr 2, 4
+; CHECK-PWR7-NEXT:blr
+entry:
+  %0 = tail call <2 x double> @llvm.ppc.fnmsub.v2f64(<2 x double> %f, <2 x double> %f2, <2 x double> %f3)
+  ret <2 x dou

[PATCH] D116015: [PowerPC] Add generic fnmsub intrinsic

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.c:98
 // CHECK-NEXT:store double [[D:%.*]], double* [[D_ADDR]], align 8
+// CHECK-COUNT-3:load double, double* [[D_ADDR]], align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, double* [[D_ADDR]], align 8

shchenz wrote:
> If we improve the check lines to CHECK-COUNT, do we still need the original 
> CHECKs?
Yes, otherwise we can't capture the right operands of `llvm.ppc.fnmsub.f64`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116015

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

[clang] b2497e5 - [PowerPC] Add generic fnmsub intrinsic

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2022-03-07T13:00:06+08:00
New Revision: b2497e54356d454d0e16d8f44086bf6db6aff0e3

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

LOG: [PowerPC] Add generic fnmsub intrinsic

Currently in Clang, we have two types of builtins for fnmsub operation:
one for float/double vector, they'll be transformed into IR operations;
one for float/double scalar, they'll generate corresponding intrinsics.

But for the vector version of builtin, the 3 op chain may be recognized
as expensive by some passes (like early cse). We need some way to keep
the fnmsub form until code generation.

This patch introduces ppc.fnmsub.* intrinsic to unify four fnmsub
intrinsics.

Reviewed By: shchenz

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCInstrInfo.td
llvm/lib/Target/PowerPC/PPCInstrVSX.td
llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 5b917ead9cd9c..acbeac326ece6 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -15778,6 +15778,8 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
 return Builder.CreateTrunc(LoadIntrinsic, Int16Ty);
   }
   // FMA variations
+  case PPC::BI__builtin_ppc_fnmsub:
+  case PPC::BI__builtin_ppc_fnmsubs:
   case PPC::BI__builtin_vsx_xvmaddadp:
   case PPC::BI__builtin_vsx_xvmaddasp:
   case PPC::BI__builtin_vsx_xvnmaddadp:
@@ -15816,6 +15818,8 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
   F, {X, Y, Builder.CreateFNeg(Z, "neg")});
 else
   return Builder.CreateCall(F, {X, Y, Builder.CreateFNeg(Z, "neg")});
+  case PPC::BI__builtin_ppc_fnmsub:
+  case PPC::BI__builtin_ppc_fnmsubs:
   case PPC::BI__builtin_vsx_xvnmsubadp:
   case PPC::BI__builtin_vsx_xvnmsubasp:
 if (Builder.getIsFPConstrained())
@@ -15824,10 +15828,9 @@ Value *CodeGenFunction::EmitPPCBuiltinExpr(unsigned 
BuiltinID,
   F, {X, Y, Builder.CreateFNeg(Z, "neg")}),
   "neg");
 else
-  return Builder.CreateFNeg(
-  Builder.CreateCall(F, {X, Y, Builder.CreateFNeg(Z, "neg")}),
-  "neg");
-}
+  return Builder.CreateCall(
+  CGM.getIntrinsic(Intrinsic::ppc_fnmsub, ResultType), {X, Y, Z});
+  }
 llvm_unreachable("Unknown FMA operation");
 return nullptr; // Suppress no-return warning
   }

diff  --git a/clang/test/CodeGen/PowerPC/builtins-ppc-fma.c 
b/clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
index 3f124e8c8299c..111302337954b 100644
--- a/clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
+++ b/clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
@@ -32,12 +32,8 @@ void test_fma(void) {
   // CHECK: <2 x double> @llvm.fma.v2f64(<2 x double> %{{.*}}, <2 x double> 
%{{.*}}, <2 x double> [[RESULT]])
 
   vf = __builtin_vsx_xvnmsubasp(vf, vf, vf);
-  // CHECK: [[RESULT:%[^ ]+]] = fneg <4 x float> %{{.*}}
-  // CHECK: [[RESULT2:%[^ ]+]] = call <4 x float> @llvm.fma.v4f32(<4 x float> 
%{{.*}}, <4 x float> %{{.*}}, <4 x float> [[RESULT]])
-  // CHECK: fneg <4 x float> [[RESULT2]]
+  // CHECK: call <4 x float> @llvm.ppc.fnmsub.v4f32(<4 x float> %{{.*}}, <4 x 
float> %{{.*}}, <4 x float> %{{.*}})
 
   vd = __builtin_vsx_xvnmsubadp(vd, vd, vd);
-  // CHECK: [[RESULT:%[^ ]+]] = fneg <2 x double> %{{.*}}
-  // CHECK: [[RESULT2:%[^ ]+]] = call <2 x double> @llvm.fma.v2f64(<2 x 
double> %{{.*}}, <2 x double> %{{.*}}, <2 x double> [[RESULT]])
-  // CHECK: fneg <2 x double> [[RESULT2]]
+  // CHECK: call <2 x double> @llvm.ppc.fnmsub.v2f64(<2 x double> %{{.*}}, <2 
x double> %{{.*}}, <2 x double> %{{.*}})
 }

diff  --git a/clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c 
b/clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
index 909210996064c..f09ba841e2202 100644
--- a/clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
+++ b/clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
@@ -142,9 +142,7 @@ void test_float(void) {
 
   vf = __builtin_vsx_xvnmsubasp(vf, vf, vf);
   // CHECK-LABEL: try-xvnmsubasp
-  // CHECK-UNCONSTRAINED: [[RESULT0:%[^ ]+]] = fneg <4 x float> %{{.*}}
-  // CHECK-UNCONSTRAINED: [[RESULT1:%[^ ]+]] = call <4 x float> 
@llvm.fma.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> 
[[RESULT0]])
-  // CHECK-UNCONSTRAINED: fneg

[PATCH] D116015: [PowerPC] Add generic fnmsub intrinsic

[Qiu Chaofan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb2497e54356d: [PowerPC] Add generic fnmsub intrinsic 
(authored by qiucf).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116015

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-fma.c
  clang/test/CodeGen/PowerPC/builtins-ppc-fpconstrained.c
  clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrVSX.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-math.ll
@@ -98,49 +98,104 @@
 
 declare float @llvm.ppc.fnmadds(float, float, float)
 
-define dso_local double @fnmsub_t0(double %d, double %d2, double %d3) {
-; CHECK-PWR8-LABEL: fnmsub_t0:
+define dso_local float @fnmsub_f32(float %f, float %f2, float %f3) {
+; CHECK-PWR8-LABEL: fnmsub_f32:
 ; CHECK-PWR8:   # %bb.0: # %entry
-; CHECK-PWR8-NEXT:xsnmsubmdp 1, 2, 3
+; CHECK-PWR8-NEXT:xsnmsubasp 3, 1, 2
+; CHECK-PWR8-NEXT:fmr 1, 3
 ; CHECK-PWR8-NEXT:blr
 ;
-; CHECK-NOVSX-LABEL: fnmsub_t0:
+; CHECK-NOVSX-LABEL: fnmsub_f32:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:blr
+;
+; CHECK-PWR7-LABEL: fnmsub_f32:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-PWR7-NEXT:blr
+entry:
+  %0 = tail call float @llvm.ppc.fnmsub.f32(float %f, float %f2, float %f3)
+  ret float %0
+}
+
+declare float @llvm.ppc.fnmsub.f32(float, float, float)
+
+define dso_local double @fnmsub_f64(double %f, double %f2, double %f3) {
+; CHECK-PWR8-LABEL: fnmsub_f64:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xsnmsubadp 3, 1, 2
+; CHECK-PWR8-NEXT:fmr 1, 3
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: fnmsub_f64:
 ; CHECK-NOVSX:   # %bb.0: # %entry
 ; CHECK-NOVSX-NEXT:fnmsub 1, 1, 2, 3
 ; CHECK-NOVSX-NEXT:blr
 ;
-; CHECK-PWR7-LABEL: fnmsub_t0:
+; CHECK-PWR7-LABEL: fnmsub_f64:
 ; CHECK-PWR7:   # %bb.0: # %entry
-; CHECK-PWR7-NEXT:xsnmsubmdp 1, 2, 3
+; CHECK-PWR7-NEXT:xsnmsubadp 3, 1, 2
+; CHECK-PWR7-NEXT:fmr 1, 3
 ; CHECK-PWR7-NEXT:blr
 entry:
-  %0 = tail call double @llvm.ppc.fnmsub(double %d, double %d2, double %d3)
+  %0 = tail call double @llvm.ppc.fnmsub.f64(double %f, double %f2, double %f3)
   ret double %0
 }
 
-declare double @llvm.ppc.fnmsub(double, double, double)
+declare double @llvm.ppc.fnmsub.f64(double, double, double)
 
-define dso_local float @fnmsubs_t0(float %f, float %f2, float %f3) {
-; CHECK-PWR8-LABEL: fnmsubs_t0:
+define dso_local <4 x float> @fnmsub_v4f32(<4 x float> %f, <4 x float> %f2, <4 x float> %f3) {
+; CHECK-PWR8-LABEL: fnmsub_v4f32:
 ; CHECK-PWR8:   # %bb.0: # %entry
-; CHECK-PWR8-NEXT:xsnmsubmsp 1, 2, 3
+; CHECK-PWR8-NEXT:xvnmsubasp 36, 34, 35
+; CHECK-PWR8-NEXT:vmr 2, 4
 ; CHECK-PWR8-NEXT:blr
 ;
-; CHECK-NOVSX-LABEL: fnmsubs_t0:
+; CHECK-NOVSX-LABEL: fnmsub_v4f32:
 ; CHECK-NOVSX:   # %bb.0: # %entry
-; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:fnmsubs 1, 1, 5, 9
+; CHECK-NOVSX-NEXT:fnmsubs 2, 2, 6, 10
+; CHECK-NOVSX-NEXT:fnmsubs 3, 3, 7, 11
+; CHECK-NOVSX-NEXT:fnmsubs 4, 4, 8, 12
 ; CHECK-NOVSX-NEXT:blr
 ;
-; CHECK-PWR7-LABEL: fnmsubs_t0:
+; CHECK-PWR7-LABEL: fnmsub_v4f32:
 ; CHECK-PWR7:   # %bb.0: # %entry
-; CHECK-PWR7-NEXT:fnmsubs 1, 1, 2, 3
+; CHECK-PWR7-NEXT:xvnmsubasp 36, 34, 35
+; CHECK-PWR7-NEXT:vmr 2, 4
 ; CHECK-PWR7-NEXT:blr
 entry:
-  %0 = tail call float @llvm.ppc.fnmsubs(float %f, float %f2, float %f3)
-  ret float %0
+  %0 = tail call <4 x float> @llvm.ppc.fnmsub.v4f32(<4 x float> %f, <4 x float> %f2, <4 x float> %f3)
+  ret <4 x float> %0
+}
+
+declare <4 x float> @llvm.ppc.fnmsub.v4f32(<4 x float>, <4 x float>, <4 x float>)
+
+define dso_local <2 x double> @fnmsub_v2f64(<2 x double> %f, <2 x double> %f2, <2 x double> %f3) {
+; CHECK-PWR8-LABEL: fnmsub_v2f64:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xvnmsubadp 36, 34, 35
+; CHECK-PWR8-NEXT:vmr 2, 4
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: fnmsub_v2f64:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fnmsub 1, 1, 3, 5
+; CHECK-NOVSX-NEXT:fnmsub 2, 2, 4, 6
+; CHECK-NOVSX-NEXT:blr
+;
+; CHECK-PWR7-LABEL: fnmsub_v2f64:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:xvnmsubadp 36, 34, 35
+; CHECK-PWR7-NEXT:vmr 2, 4
+; CHECK-PWR7-NEX

[PATCH] D120307: [X86] Add helper enum for ternary intrinsics

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.
Herald added a project: All.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120307

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

[PATCH] D119407: [PowerPC] [Clang] Add SSE4 and BMI compatible intrinsics implementation for PowerPC

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 413336.
qiucf added a comment.
Herald added a project: All.

Add P10  tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119407

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/ppc_wrappers/bmi2intrin.h
  clang/lib/Headers/ppc_wrappers/bmiintrin.h
  clang/lib/Headers/ppc_wrappers/emmintrin.h
  clang/lib/Headers/ppc_wrappers/immintrin.h
  clang/lib/Headers/ppc_wrappers/nmmintrin.h
  clang/lib/Headers/ppc_wrappers/pmmintrin.h
  clang/lib/Headers/ppc_wrappers/smmintrin.h
  clang/lib/Headers/ppc_wrappers/tmmintrin.h
  clang/lib/Headers/ppc_wrappers/x86gprintrin.h
  clang/lib/Headers/ppc_wrappers/x86intrin.h
  clang/lib/Headers/ppc_wrappers/xmmintrin.h
  clang/test/CodeGen/PowerPC/ppc-emmintrin.c
  clang/test/CodeGen/PowerPC/ppc-smmintrin.c
  clang/test/CodeGen/PowerPC/ppc-x86gprintrin.c
  clang/test/CodeGen/PowerPC/ppc-xmmintrin.c

Index: clang/test/CodeGen/PowerPC/ppc-xmmintrin.c
===
--- clang/test/CodeGen/PowerPC/ppc-xmmintrin.c
+++ clang/test/CodeGen/PowerPC/ppc-xmmintrin.c
@@ -383,12 +383,11 @@
 // CHECK: extractelement <4 x float> %{{[0-9a-zA-Z_.]+}}, i32 0
 
 // CHECK-LABEL: define available_externally signext i32 @_mm_cvtss_si32
-// CHECK-LE: %[[VEC:[0-9a-zA-Z_.]+]] = call { <4 x float>, i64, double } asm "xxsldwi ${0:x},${0:x},${0:x},3;\0Axscvspdp ${2:x},${0:x};\0Afctiw  $2,$2;\0Amfvsrd  $1,${2:x};\0A", "=^wa,=r,=f,0"
-// CHECK-BE: %[[VEC:[0-9a-zA-Z_.]+]] = call { <4 x float>, i64, double } asm "xscvspdp ${2:x},${0:x};\0Afctiw  $2,$2;\0Amfvsrd  $1,${2:x};\0A", "=^wa,=r,=f,0"
-// CHECK: extractvalue { <4 x float>, i64, double } %[[VEC]], 0
-// CHECK: extractvalue { <4 x float>, i64, double } %[[VEC]], 1
-// CHECK: extractvalue { <4 x float>, i64, double } %[[VEC]], 2
-// CHECK: trunc i64 %{{[0-9a-zA-Z_.]+}} to i32
+// CHECK-LE: %[[VEC:[0-9a-zA-Z_.]+]] = call { <4 x float>, i32, double } asm "xxsldwi ${0:x},${0:x},${0:x},3;\0Axscvspdp ${2:x},${0:x};\0Afctiw  $2,$2;\0Amfvsrd  $1,${2:x};\0A", "=^wa,=r,=f,0"
+// CHECK-BE: %[[VEC:[0-9a-zA-Z_.]+]] = call { <4 x float>, i32, double } asm "xscvspdp ${2:x},${0:x};\0Afctiw  $2,$2;\0Amfvsrd  $1,${2:x};\0A", "=^wa,=r,=f,0"
+// CHECK: extractvalue { <4 x float>, i32, double } %[[VEC]], 0
+// CHECK: extractvalue { <4 x float>, i32, double } %[[VEC]], 1
+// CHECK: extractvalue { <4 x float>, i32, double } %[[VEC]], 2
 
 // CHECK-LABEL: define available_externally i64 @_mm_cvtss_si64
 // CHECK-LE: %[[VEC:[0-9a-zA-Z_.]+]] = call { <4 x float>, i64, double } asm "xxsldwi ${0:x},${0:x},${0:x},3;\0Axscvspdp ${2:x},${0:x};\0Afctid  $2,$2;\0Amfvsrd  $1,${2:x};\0A", "=^wa,=r,=f,0"
Index: clang/test/CodeGen/PowerPC/ppc-x86gprintrin.c
===
--- /dev/null
+++ clang/test/CodeGen/PowerPC/ppc-x86gprintrin.c
@@ -0,0 +1,239 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-linux-gnu -mcpu=pwr7 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr7 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s
+
+// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 -mcpu=pwr7 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr7 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s
+
+#include 
+
+unsigned short us;
+unsigned ui;
+unsigned long long ul;
+
+void __attribute__((noinline))
+test_bmiintrin() {
+  __tzcnt_u16(us);
+  __andn_u32(ui, ui);
+  _bextr_u32(ui, ui, ui);
+  __bextr_u32(ui, ui);
+  __blsi_u32(ui);
+  _blsi_u32(ui);
+  __blsmsk_u32(ui);
+  _blsmsk_u32(ui);
+  __blsr_u32(ui);
+  _blsr_u32(ui);
+  __tzcnt_u32(ui);
+  _tzcnt_u32(ui);
+  __andn_u64(ul, ul);
+  _bextr_u64(ul, ui, ui);
+  __bextr_u64(ul, ul);
+  __blsi_u64(ul);
+  _blsi_u64(ul);
+  __blsmsk_u64(ul);
+  _blsmsk_u64(ul);
+  __blsr_u64(ul);
+  _blsr_u64(ul);
+  __tzcnt_u64(ul);
+  _tzcnt_u64(ul);
+}
+
+// CHECK-LABEL: @test_bmiintrin
+
+// CHECK-LABEL: define available_externally zeroext i16 @__tzcnt_u16(i16 noundef zeroext %{{[0-9a-zA-Z._]+}})
+// CHECK: %[[CONV:[0-9a-zA-Z._]+]] = zext i16 %{{[0-9a-zA-Z._]+}} to i32
+// CHECK: %[[CALL:[0-9a-zA-Z._]+]] = call i32 @llvm.cttz.i32(i32 %[[CONV]], i1 false)
+// CHECK: trunc i32 %[[CALL]] to i16
+
+// CHECK-LABEL: define available_externally zeroext i32 @__andn_u32(i32 noundef zeroext %{{[0-9a-zA-Z._

[PATCH] D119476: Generalize and harmonize sub-expression traversal

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.
Herald added a project: All.

Gentle weekly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119476

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

[PATCH] D119477: Ignore FullExpr when traversing cast sub-expressions

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.
Herald added a project: All.

Gentle weekly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119477

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

[PATCH] D120857: WIP [randstruct] Create basis for unit test module

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 413343.
void added a comment.

Fix assert from use of null pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120857

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Randstruct.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Randstruct.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/AST/RandstructTest.cpp

Index: clang/unittests/AST/RandstructTest.cpp
===
--- /dev/null
+++ clang/unittests/AST/RandstructTest.cpp
@@ -0,0 +1,413 @@
+//===- unittest/AST/RandstructTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains tests for Clang's structure field layout randomization.
+//
+//===--===//
+
+/*
+ * Build this test suite by running `make ASTTests` in the build folder.
+ *
+ * Run this test suite by running the following in the build folder:
+ * ` ./tools/clang/unittests/AST/ASTTests
+ * --gtest_filter=StructureLayoutRandomization*`
+ */
+
+#include "clang/AST/Randstruct.h"
+#include "gtest/gtest.h"
+
+#include "DeclMatcher.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Testing/CommandLineArgs.h"
+#include "clang/Tooling/Tooling.h"
+
+#include 
+
+using namespace clang::randstruct;
+
+namespace clang {
+namespace ast_matchers {
+
+static std::unique_ptr makeAST(const std::string &SourceCode,
+TestLanguage Lang) {
+  std::vector Args = getCommandLineArgsForTesting(Lang);
+  Args.push_back("-frandstruct-seed=1234567890abcdef");
+  auto AST = tooling::buildASTFromCodeWithArgs(SourceCode, Args, "input.cc");
+  return AST;
+}
+
+static RecordDecl *getRecordDeclFromAST(const ASTContext &C,
+const std::string &Name) {
+  return FirstDeclMatcher().match(C.getTranslationUnitDecl(),
+  recordDecl(hasName(Name)));
+}
+
+static std::vector getFieldNamesFromRecord(const RecordDecl *RD) {
+  std::vector Fields;
+  Fields.reserve(8);
+  for (auto *Field : RD->fields())
+Fields.push_back(Field->getNameAsString());
+  return Fields;
+}
+
+static bool isSubsequence(const std::vector &Seq,
+  const std::vector &Subseq) {
+  const auto SeqLen = Seq.size();
+  const auto SubLen = Subseq.size();
+
+  auto IsSubseq = false;
+  for (auto I = 0u; I < SeqLen; ++I) {
+if (Seq[I] == Subseq[0]) {
+  IsSubseq = true;
+  for (auto J = 0u; J + I < SeqLen && J < SubLen; ++J) {
+if (Seq[J + I] != Subseq[J]) {
+  IsSubseq = false;
+  break;
+}
+  }
+}
+  }
+  return IsSubseq;
+}
+
+#define RANDSTRUCT_TEST_SUITE_TEST StructureLayoutRandomizationTestSuiteTest
+
+TEST(RANDSTRUCT_TEST_SUITE_TEST, CanDetermineIfSubsequenceExists) {
+  const std::vector S0 = {"a", "b", "c", "d"};
+  ASSERT_TRUE(isSubsequence(S0, {"b", "c"}));
+  ASSERT_TRUE(isSubsequence(S0, {"a", "b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(S0, {"b", "c", "d"}));
+  ASSERT_TRUE(isSubsequence(S0, {"a"}));
+  ASSERT_FALSE(isSubsequence(S0, {"a", "d"}));
+}
+
+#define RANDSTRUCT_TEST StructureLayoutRandomization
+
+TEST(RANDSTRUCT_TEST, UnmarkedStructuresAreNotRandomized) {
+  std::string Code =
+  R"(
+struct dont_randomize_me {
+int potato;
+float tomato;
+long cabbage;
+};
+)";
+
+  const auto AST = makeAST(Code, Lang_C99);
+  const auto *RD =
+  getRecordDeclFromAST(AST->getASTContext(), "dont_randomize_me");
+  const std::vector Expected = {"potato", "tomato", "cabbage"};
+  const std::vector Actual = getFieldNamesFromRecord(RD);
+
+  ASSERT_EQ(Expected, Actual);
+}
+
+TEST(RANDSTRUCT_TEST, StructuresCanBeMarkedWithRandomizeLayoutAttr) {
+  std::string Code =
+  R"(
+struct marked {
+int bacon;
+long lettuc

[clang] 7b969b0 - [clang][parser] Stop dragging an EndLoc around when parsing attributes

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-03-07T08:16:39+01:00
New Revision: 7b969b0bb53e5dcf23e0ddba977031fb104b63ec

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

LOG: [clang][parser] Stop dragging an EndLoc around when parsing attributes

It's almost always entirely unused and if it is used, the end of the
attribute range can be used instead.

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

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParsePragma.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Parse/ParseTentative.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 64a8675a7c508..a3c1a15d333ce 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2686,34 +2686,30 @@ class Parser : public CodeCompletionHandler {
   /// Such situations should use the specific attribute parsing functionality.
   void ParseAttributes(unsigned WhichAttrKinds,
ParsedAttributesWithRange &Attrs,
-   SourceLocation *End = nullptr,
LateParsedAttrList *LateAttrs = nullptr);
   void ParseAttributes(unsigned WhichAttrKinds, ParsedAttributes &Attrs,
-   SourceLocation *End = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 ParsedAttributesWithRange AttrsWithRange(AttrFactory);
-ParseAttributes(WhichAttrKinds, AttrsWithRange, End, LateAttrs);
+ParseAttributes(WhichAttrKinds, AttrsWithRange, LateAttrs);
 Attrs.takeAllFrom(AttrsWithRange);
   }
   /// \brief Possibly parse attributes based on what syntaxes are desired,
   /// allowing for the order to vary.
   bool MaybeParseAttributes(unsigned WhichAttrKinds,
 ParsedAttributesWithRange &Attrs,
-SourceLocation *End = nullptr,
 LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.isOneOf(tok::kw___attribute, tok::kw___declspec) ||
 (standardAttributesAllowed() && isCXX11AttributeSpecifier())) {
-  ParseAttributes(WhichAttrKinds, Attrs, End, LateAttrs);
+  ParseAttributes(WhichAttrKinds, Attrs, LateAttrs);
   return true;
 }
 return false;
   }
   bool MaybeParseAttributes(unsigned WhichAttrKinds, ParsedAttributes &Attrs,
-SourceLocation *End = nullptr,
 LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.isOneOf(tok::kw___attribute, tok::kw___declspec) ||
 (standardAttributesAllowed() && isCXX11AttributeSpecifier())) {
-  ParseAttributes(WhichAttrKinds, Attrs, End, LateAttrs);
+  ParseAttributes(WhichAttrKinds, Attrs, LateAttrs);
   return true;
 }
 return false;
@@ -2722,10 +2718,9 @@ class Parser : public CodeCompletionHandler {
   void MaybeParseGNUAttributes(Declarator &D,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
-  ParsedAttributes attrs(AttrFactory);
-  SourceLocation endLoc;
-  ParseGNUAttributes(attrs, &endLoc, LateAttrs, &D);
-  D.takeAttributes(attrs, endLoc);
+  ParsedAttributesWithRange attrs(AttrFactory);
+  ParseGNUAttributes(attrs, LateAttrs, &D);
+  D.takeAttributes(attrs, attrs.Range.getEnd());
 }
   }
 
@@ -2735,11 +2730,10 @@ class Parser : public CodeCompletionHandler {
   /// This API is discouraged. Use the version that takes a
   /// ParsedAttributesWithRange instead.
   bool MaybeParseGNUAttributes(ParsedAttributes &Attrs,
-   SourceLocation *EndLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
   ParsedAttributesWithRange AttrsWithRange(AttrFactory);
-  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  ParseGNUAttributes(Attrs, LateAttrs);
   Attrs.takeAllFrom(AttrsWithRange);
   return true;
 }
@@ -2747,10 +2741,9 @@ class Parser : public CodeCompletionHandler {
   }
 
   bool MaybeParseGNUAttributes(ParsedAttributesWithRange &Attrs,
-   SourceLocation *EndLoc = nullptr,
LateParsedAttrList *LateAttrs = nullptr) {
 if (Tok.is(tok::kw___attribute)) {
-  ParseGNUAttributes(Attrs, EndLoc, LateAttrs);
+  ParseGNUAttributes(Attrs, LateAttrs);
   return true;
 }
 return false;
@@ -2762,16 +2755,14 @@ class Parser : public CodeCompletionHandler {
   /// This API is discouraged. Use the version that takes a
   /// ParsedAt

[PATCH] D120888: [clang] Stop dragging a EndLoc around when parsing attributes

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b969b0bb53e: [clang][parser] Stop dragging an EndLoc around 
when parsing attributes (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120888

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTentative.cpp

Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -1913,7 +1913,7 @@
   /*OuterMightBeMessageSend*/true))
   return TPResult::True;
 
-ParsedAttributes attrs(AttrFactory);
+ParsedAttributesWithRange attrs(AttrFactory);
 MaybeParseMicrosoftAttributes(attrs);
 
 // decl-specifier-seq
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -106,7 +106,7 @@
   // at the start of the statement. Thus, we're not using MaybeParseAttributes
   // here because we don't want to allow arbitrary orderings.
   ParsedAttributesWithRange Attrs(AttrFactory);
-  MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true);
+  MaybeParseCXX11Attributes(Attrs, /*MightBeObjCMessageSend*/ true);
   if (getLangOpts().OpenCL)
 MaybeParseGNUAttributes(Attrs);
 
@@ -1119,8 +1119,7 @@
 ConsumeToken();
 
   ParsedAttributesWithRange attrs(AttrFactory);
-  MaybeParseCXX11Attributes(attrs, nullptr,
-/*MightBeObjCMessageSend*/ true);
+  MaybeParseCXX11Attributes(attrs, /*MightBeObjCMessageSend*/ true);
 
   // If this is the start of a declaration, parse it as such.
   if (isDeclarationStatement()) {
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -341,7 +341,7 @@
 Token &FirstToken) override;
 
   /// A pool of attributes that were parsed in \#pragma clang attribute.
-  ParsedAttributes AttributesForPragmaAttribute;
+  ParsedAttributesWithRange AttributesForPragmaAttribute;
 };
 
 struct PragmaMaxTokensHereHandler : public PragmaHandler {
@@ -1365,12 +1365,13 @@
 namespace {
 struct PragmaAttributeInfo {
   enum ActionType { Push, Pop, Attribute };
-  ParsedAttributes &Attributes;
+  ParsedAttributesWithRange &Attributes;
   ActionType Action;
   const IdentifierInfo *Namespace = nullptr;
   ArrayRef Tokens;
 
-  PragmaAttributeInfo(ParsedAttributes &Attributes) : Attributes(Attributes) {}
+  PragmaAttributeInfo(ParsedAttributesWithRange &Attributes)
+  : Attributes(Attributes) {}
 };
 
 #include "clang/Parse/AttrSubMatchRulesParserStringSwitches.inc"
@@ -1640,7 +1641,7 @@
   /*IsReinject=*/false);
   ConsumeAnnotationToken();
 
-  ParsedAttributes &Attrs = Info->Attributes;
+  ParsedAttributesWithRange &Attrs = Info->Attributes;
   Attrs.clearListOnly();
 
   auto SkipToEnd = [this]() {
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1252,7 +1252,7 @@
   TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
   Actions.PushLambdaScope();
 
-  ParsedAttributes Attr(AttrFactory);
+  ParsedAttributesWithRange Attr(AttrFactory);
   if (getLangOpts().CUDA) {
 // In CUDA code, GNU attributes are allowed to appear immediately after the
 // "[...]", even if there is no "(...)" before the lambda body.
@@ -1355,7 +1355,8 @@
   DeclEndLoc = ESpecRange.getEnd();
 
 // Parse attribute-specifier[opt].
-MaybeParseCXX11Attributes(Attr, &DeclEndLoc);
+if (MaybeParseCXX11Attributes(Attr))
+  DeclEndLoc = Attr.Range.getEnd();
 
 // Parse OpenCL addr space attribute.
 if (Tok.isOneOf(tok::kw___private, tok::kw___global, tok::kw___local,
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4513,19 +4513,17 @@
 ///
 /// attribute-specifier-seq:
 ///   attribute-specifier-seq[opt] attribute-specifier
-void Parser::ParseCXX11Attributes(ParsedAttributesWithRange &attrs,
-  SourceLocation *endLoc) {
+void Parser::ParseCXX11Attributes(ParsedAttributesWithRange &attrs) {
   assert(standardAttributesAllowed());
 
-  SourceLocation StartLoc = Tok.g

[PATCH] D120307: [X86] Add helper enum for ternary intrinsics

[LuoYuanke via Phabricator via cfe-commits]

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

LGTM, but pls wait for 1 or 2 days to see if there are any comments from others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120307

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

[PATCH] D120989: Support debug info for alias variable

[Kavitha Natarajan via Phabricator via cfe-commits]

kavitha-natarajan updated this revision to Diff 413347.
kavitha-natarajan added a comment.

Rebased and applied pre-merge check formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/debug-info-alias.c

Index: clang/test/CodeGen/debug-info-alias.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-alias.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-DAG: [[ENTITY1:![0-9]+]] = distinct !DIGlobalVariable(name: "aliased_global"
+// CHECK-DAG: [[ENTITY2:![0-9]+]] = distinct !DIGlobalVariable(name: "aliased_global_2"
+// CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "__global_alias", scope: !2, entity: [[ENTITY1]]
+// CHECK-DAG: [[ENTITY3:![0-9]+]] = !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "global_alias_2", scope: !2, entity: [[ENTITY2]]
+// CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "__global_alias_2_alias", scope: !2, entity: [[ENTITY3]]
+
+int aliased_global = 1;
+extern int __attribute__((alias("aliased_global"))) __global_alias;
+
+// Recursive alias:
+int aliased_global_2 = 2;
+extern int __attribute__((alias("aliased_global_2"))) global_alias_2;
+extern int __attribute__((alias("global_alias_2"))) __global_alias_2_alias;
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1213,6 +1213,7 @@
 
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
+  const GlobalDecl getMangledNameDecl(StringRef);
 
   void EmitTentativeDefinition(const VarDecl *D);
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1498,6 +1498,16 @@
   return Result.first->first();
 }
 
+const GlobalDecl CodeGenModule::getMangledNameDecl(StringRef Name) {
+  auto it = MangledDeclNames.begin();
+  while (it != MangledDeclNames.end()) {
+if (it->second == Name)
+  return it->first;
+it++;
+  }
+  return GlobalDecl();
+}
+
 llvm::GlobalValue *CodeGenModule::GetGlobalValue(StringRef Name) {
   return getModule().getNamedValue(Name);
 }
@@ -5164,6 +5174,13 @@
   setTLSMode(GA, *VD);
 
   SetCommonAttributes(GD, GA);
+
+  // Emit global alias debug information.
+  if (const auto *VD = dyn_cast(D)) {
+if (CGDebugInfo *DI = getModuleDebugInfo()) {
+  DI->EmitGlobalAlias(cast(GA->getAliasee()), GD);
+}
+  }
 }
 
 void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -152,8 +152,10 @@
   llvm::DenseMap DIFileCache;
   llvm::DenseMap SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
-  /// using declarations) that aren't covered by other more specific caches.
+  /// using declarations and global alias variables) that aren't covered
+  /// by other more specific caches.
   llvm::DenseMap DeclCache;
+  llvm::DenseMap ImportedDeclCache;
   llvm::DenseMap NamespaceCache;
   llvm::DenseMap
   NamespaceAliasCache;
@@ -512,6 +514,9 @@
   /// Emit information about an external variable.
   void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
+  /// Emit information about global variable alias.
+  void EmitGlobalAlias(const llvm::GlobalValue *GV, const GlobalDecl Decl);
+
   /// Emit C++ using directive.
   void EmitUsingDirective(const UsingDirectiveDecl &UD);
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3911,6 +3911,16 @@
 return dyn_cast_or_null(N);
   }
 
+  // imported declaration
+  auto IE = ImportedDeclCache.find(D->getCanonicalDecl());
+
+  if (IE != ImportedDeclCache.end()) {
+auto N = IE->second;
+if (auto *GVE = dyn_cast_or_null(N))
+  return cast(GVE);
+return dyn_cast_or_null(N);
+  }
+
   // No definition for now. Emit a forward definition that might be
   // merged with a potential upcoming definition.
   if (const auto *FD = dyn_cast(D))
@@ -5467,6 +5477,48 @@
   Var->addDebugInfo(GVE);
 }
 
+void CGDebugInfo::EmitGlobalAlias(const llvm::GlobalValue *GV,
+  const GlobalDecl GD) {
+
+  assert(GV);
+
+  if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
+return;
+
+  const auto *D = cast(

[PATCH] D120989: Support debug info for alias variable

[Kavitha Natarajan via Phabricator via cfe-commits]

kavitha-natarajan added a comment.

In D120989#3362491 , @dblaikie wrote:

> In D120989#3362490 , @dblaikie 
> wrote:
>
>> Broad question about debug info for aliases: How's this proposed solution 
>> compare/contrast to GCC's behavior?
>
> Aaand, I see that was described in thhe patch description. Sorry for not 
> reading...
>
> Right, paging in all the context - this review hinges on whether gdb/lldb can 
> handle/be made to handle this sort of debug info correctly.

I have posted a gdb patch to handle this change for review:
https://sourceware.org/pipermail/gdb-patches/2022-March/186329.html

lldb can handle this debug info correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120989

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