date:20220113

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:19610
getLLVMStyleWithColumns(35));
-  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"",
-   getLLVMStyleWithColumns(31));
+  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"", getLLVMStyleWithColumns(31));
   verifyFormat("// Однажды в студёную зимнюю пору...",

Oops. Fixed post-commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[clang] 3cf86c3 - Revert unrelated change from: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via cfe-commits


Author: Marek Kurdej
Date: 2022-01-14T08:43:59+01:00
New Revision: 3cf86c36112fd1b059c8aead3d04656c542195ce

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

LOG: Revert unrelated change from: [clang-format] Fix break being added to 
macro define with ColumnLimit: 0

Added: 


Modified: 
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d22f1a19ce7c..9d1273184807 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -19607,13 +19607,15 @@ TEST_F(FormatTest, HandlesUTF8BOM) {
 TEST_F(FormatTest, CountsUTF8CharactersProperly) {
   verifyFormat("\"Однажды в студёную зимнюю пору...\"",
getLLVMStyleWithColumns(35));
-  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"", getLLVMStyleWithColumns(31));
+  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"",
+   getLLVMStyleWithColumns(31));
   verifyFormat("// Однажды в студёную зимнюю пору...",
getLLVMStyleWithColumns(36));
   verifyFormat("// 一 二 三 四 五 六 七 八 九 十", getLLVMStyleWithColumns(32));
   verifyFormat("/* Однажды в студёную зимнюю пору... */",
getLLVMStyleWithColumns(39));
-  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */", getLLVMStyleWithColumns(35));
+  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */",
+   getLLVMStyleWithColumns(35));
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
@@ -19633,20 +19635,21 @@ TEST_F(FormatTest, SplitsUTF8Strings) {
 "\"пору,\"",
 format("\"Однажды, в студёную зимнюю пору,\"",
getLLVMStyleWithColumns(13)));
-  EXPECT_EQ("\"一 二 三 \"\n"
-"\"四 五六 \"\n"
-"\"七 八 九 \"\n"
-"\"十\"",
-format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
   EXPECT_EQ(
-  "\"一\t\"\n"
-  "\"二 \t\"\n"
-  "\"三 四 \"\n"
-  "\"五\t\"\n"
-  "\"六 \t\"\n"
-  "\"七 \"\n"
-  "\"八九十\tqq\"",
-  format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"", getLLVMStyleWithColumns(11)));
+  "\"一 二 三 \"\n"
+  "\"四 五六 \"\n"
+  "\"七 八 九 \"\n"
+  "\"十\"",
+  format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
+  EXPECT_EQ("\"一\t\"\n"
+"\"二 \t\"\n"
+"\"三 四 \"\n"
+"\"五\t\"\n"
+"\"六 \t\"\n"
+"\"七 \"\n"
+"\"八九十\tqq\"",
+format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
+   getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aa\"\n"
@@ -19691,16 +19694,16 @@ TEST_F(FormatTest, SplitsUTF8BlockComments) {
 format("/* Гляжу, поднимается медленно в гору\n"
" * Лошадка, везущая хворосту воз. */",
getLLVMStyleWithColumns(13)));
-  EXPECT_EQ("/* 一二三\n"
-" * 四五六七\n"
-" * 八  九\n"
-" * 十  */",
-format("/* 一二三 四五六七 八  九 十  */", getLLVMStyleWithColumns(9)));
+  EXPECT_EQ(
+  "/* 一二三\n"
+  " * 四五六七\n"
+  " * 八  九\n"
+  " * 十  */",
+  format("/* 一二三 四五六七 八  九 十  */", getLLVMStyleWithColumns(9)));
   EXPECT_EQ("/* 퓣퓮퓼퓽 픣픬픲픯\n"
 " * 핓핪핥핖\n"
 " * 햀핿핱-ퟠ */",
-format("/* 퓣퓮퓼퓽 픣픬픲픯 핓핪핥핖 햀핿핱-ퟠ */",
-   getLLVMStyleWithColumns(12)));
+format("/* 퓣퓮퓼퓽 픣픬픲픯 핓핪핥핖 햀핿핱-ퟠ */", getLLVMStyleWithColumns(12)));
 }
 
 #endif // _MSC_VER



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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

futuarmo wrote:
> owenpan wrote:
> > Please remove the newline and re-run `FormatTests`.
> FormatTests pass again
Done when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG47a9eb2117aa: [clang-format] Fix break being added to macro 
define with ColumnLimit: 0 (authored by futuarmo, committed by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D116859?vs=399394=399906#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,13 @@
Style);
 }
 
+TEST_F(FormatTest, FormatsMacrosWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOONG() LOOONG()",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
@@ -19600,15 +19607,13 @@
 TEST_F(FormatTest, CountsUTF8CharactersProperly) {
   verifyFormat("\"ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ...\"",
getLLVMStyleWithColumns(35));
-  verifyFormat("\"ä¸ äº ä¸ å äº å ä¸ å« ä¹ å\"",
-   getLLVMStyleWithColumns(31));
+  verifyFormat("\"ä¸ äº ä¸ å äº å ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(31));
   verifyFormat("// ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ...",
getLLVMStyleWithColumns(36));
   verifyFormat("// ä¸ äº ä¸ å äº å ä¸ å« ä¹ å", getLLVMStyleWithColumns(32));
   verifyFormat("/* ÐÐ´Ð½Ð°Ð¶Ð´Ñ Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ... */",
getLLVMStyleWithColumns(39));
-  verifyFormat("/* ä¸ äº ä¸ å äº å ä¸ å« ä¹ å */",
-   getLLVMStyleWithColumns(35));
+  verifyFormat("/* ä¸ äº ä¸ å äº å ä¸ å« ä¹ å */", getLLVMStyleWithColumns(35));
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
@@ -19628,21 +19633,20 @@
 "\"Ð¿Ð¾ÑÑ,\"",
 format("\"ÐÐ´Ð½Ð°Ð¶Ð´Ñ, Ð² ÑÑÑÐ´ÑÐ½ÑÑ Ð·Ð¸Ð¼Ð½ÑÑ Ð¿Ð¾ÑÑ,\"",
getLLVMStyleWithColumns(13)));
+  EXPECT_EQ("\"ä¸ äº ä¸ \"\n"
+"\"å äºå \"\n"
+"\"ä¸ å« ä¹ \"\n"
+"\"å\"",
+format("\"ä¸ äº ä¸ å äºå ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(11)));
   EXPECT_EQ(
-  "\"ä¸ äº ä¸ \"\n"
-  "\"å äºå \"\n"
-  "\"ä¸ å« ä¹ \"\n"
-  "\"å\"",
-  format("\"ä¸ äº ä¸ å äºå ä¸ å« ä¹ å\"", getLLVMStyleWithColumns(11)));
-  EXPECT_EQ("\"ä¸\t\"\n"
-"\"äº \t\"\n"
-"\"ä¸ å \"\n"
-"\"äº\t\"\n"
-"\"å \t\"\n"
-"\"ä¸ \"\n"
-"\"å«ä¹å\tqq\"",
-format("\"ä¸\täº \tä¸ å äº\tå \tä¸ å«ä¹å\tqq\"",
-   getLLVMStyleWithColumns(11)));
+  "\"ä¸\t\"\n"
+  "\"äº \t\"\n"
+  "\"ä¸ å \"\n"
+  "\"äº\t\"\n"
+  "\"å \t\"\n"
+  "\"ä¸ \"\n"
+  "\"å«ä¹å\tqq\"",
+  format("\"ä¸\täº \tä¸ å äº\tå \tä¸ å«ä¹å\tqq\"", getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aa\"\n"
@@ -19687,16 +19691,16 @@
 format("/* ÐÐ»ÑÐ¶Ñ, Ð¿Ð¾Ð´Ð½Ð¸Ð¼Ð°ÐµÑÑÑ Ð¼ÐµÐ´Ð»ÐµÐ½Ð½Ð¾ Ð² Ð³Ð¾ÑÑ\n"
" * ÐÐ¾ÑÐ°Ð´ÐºÐ°, Ð²ÐµÐ·ÑÑÐ°Ñ ÑÐ²Ð¾ÑÐ¾ÑÑÑ Ð²Ð¾Ð·. */",
getLLVMStyleWithColumns(13)));
-  EXPECT_EQ(
-  "/* ä¸äºä¸\n"
-  " * åäºåä¸\n"
-  " * å«  ä¹\n"
-  " * å  */",
-  format("/* ä¸äºä¸ åäºåä¸ å«  ä¹ å  */", getLLVMStyleWithColumns(9)));
+  EXPECT_EQ("/* ä¸äºä¸\n"
+" * åäºåä¸\n"
+" * å«  ä¹\n"
+" * å  */",
+format("/* ä¸äºä¸ åäºåä¸ å«  ä¹ å  */", getLLVMStyleWithColumns(9)));
   EXPECT_EQ("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯\n"
 " * ððªð¥ð\n"
 " * ðð¿ð±-ð  */",
-format("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯ ððªð¥ð ðð¿ð±-ð  */", getLLVMStyleWithColumns(12)));
+format("/* ð£ð®ð¼ð½ ð£ð¬ð²ð¯ ððªð¥ð ðð¿ð±-ð  */",
+   getLLVMStyleWithColumns(12)));
 }
 
 #endif // _MSC_VER
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@
 // different LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if

[clang] 47a9eb2 - [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Marek Kurdej via cfe-commits


Author: Armen Khachkinaev
Date: 2022-01-14T08:42:22+01:00
New Revision: 47a9eb2117aa7c61d8d2a9bf81a91e76a059a035

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

LOG: [clang-format] Fix break being added to macro define with ColumnLimit: 0

Fix for #[[ https://github.com/llvm/llvm-project/issues/49164 | 49164 ]] issue.

Reviewed By: MyDeveloperDay, HazardyKnusperkeks, curdeius, owenpan

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 31f5de6733627..9b1d4ecf3 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -485,7 +485,8 @@ bool ContinuationIndenter::mustBreak(const LineState 
) {
 // 
diff erent LineFormatter would be used otherwise.
 if (Previous.ClosesTemplateDeclaration)
   return Style.AlwaysBreakTemplateDeclarations != FormatStyle::BTDS_No;
-if (Previous.is(TT_FunctionAnnotationRParen))
+if (Previous.is(TT_FunctionAnnotationRParen) &&
+State.Line->Type != LT_PreprocessorDirective)
   return true;
 if (Previous.is(TT_LeadingJavaAnnotation) && Current.isNot(tok::l_paren) &&
 Current.isNot(TT_LeadingJavaAnnotation))

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 0332821aa6b0c..d22f1a19ce7cb 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -4804,6 +4804,13 @@ TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
Style);
 }
 
+TEST_F(FormatTest, FormatsMacrosWithZeroColumnWidth) {
+  FormatStyle ZeroColumn = getLLVMStyleWithColumns(0);
+
+  verifyFormat("#define A LOOONG() LOOONG()",
+   ZeroColumn);
+}
+
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
   verifyFormat("#define A \\\n"
"  f({ \\\n"
@@ -19600,15 +19607,13 @@ TEST_F(FormatTest, HandlesUTF8BOM) {
 TEST_F(FormatTest, CountsUTF8CharactersProperly) {
   verifyFormat("\"Однажды в студёную зимнюю пору...\"",
getLLVMStyleWithColumns(35));
-  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"",
-   getLLVMStyleWithColumns(31));
+  verifyFormat("\"一 二 三 四 五 六 七 八 九 十\"", getLLVMStyleWithColumns(31));
   verifyFormat("// Однажды в студёную зимнюю пору...",
getLLVMStyleWithColumns(36));
   verifyFormat("// 一 二 三 四 五 六 七 八 九 十", getLLVMStyleWithColumns(32));
   verifyFormat("/* Однажды в студёную зимнюю пору... */",
getLLVMStyleWithColumns(39));
-  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */",
-   getLLVMStyleWithColumns(35));
+  verifyFormat("/* 一 二 三 四 五 六 七 八 九 十 */", getLLVMStyleWithColumns(35));
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
@@ -19628,21 +19633,20 @@ TEST_F(FormatTest, SplitsUTF8Strings) {
 "\"пору,\"",
 format("\"Однажды, в студёную зимнюю пору,\"",
getLLVMStyleWithColumns(13)));
+  EXPECT_EQ("\"一 二 三 \"\n"
+"\"四 五六 \"\n"
+"\"七 八 九 \"\n"
+"\"十\"",
+format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
   EXPECT_EQ(
-  "\"一 二 三 \"\n"
-  "\"四 五六 \"\n"
-  "\"七 八 九 \"\n"
-  "\"十\"",
-  format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
-  EXPECT_EQ("\"一\t\"\n"
-"\"二 \t\"\n"
-"\"三 四 \"\n"
-"\"五\t\"\n"
-"\"六 \t\"\n"
-"\"七 \"\n"
-"\"八九十\tqq\"",
-format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
-   getLLVMStyleWithColumns(11)));
+  "\"一\t\"\n"
+  "\"二 \t\"\n"
+  "\"三 四 \"\n"
+  "\"五\t\"\n"
+  "\"六 \t\"\n"
+  "\"七 \"\n"
+  "\"八九十\tqq\"",
+  format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"", getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aa\"\n"
@@ -19687,16 +19691,16 @@ TEST_F(FormatTest, SplitsUTF8BlockComments) {
 format("/* Гляжу, поднимается медленно в гору\n"
" * Лошадка, везущая хворосту воз. */",
getLLVMStyleWithColumns(13)));
-  EXPECT_EQ(
-  "/* 一二三\n"
-  " * 四五六七\n"
-  " * 八  九\n"
-  " * 十  */",
-  format("/* 一二三 四五六七 八  九 十  */", getLLVMStyleWithColumns(9)));
+  EXPECT_EQ("/* 一二三\n"
+" * 四五六七\n"
+" * 八  九\n"
+" * 十  */",
+format("/* 一二三 四五六七 八  九 十  */", getLLVMStyleWithColumns(9)));
   EXPECT_EQ("/* 퓣퓮퓼퓽 픣픬픲픯\n"
 " * 핓핪핥핖\n"
 " * 햀핿핱-ퟠ */",
-format("/* 퓣퓮퓼퓽

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits

futuarmo added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

owenpan wrote:
> Please remove the newline and re-run `FormatTests`.
FormatTests pass again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[clang] 4f4340e - [NFC] [Coroutines] Refactor implementation in checkFinalSuspendNoThrow

2022-01-13 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2022-01-14T15:37:01+08:00
New Revision: 4f4340ee2af36909db77aeedb1d22c2ba52d2dfa

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

LOG: [NFC] [Coroutines] Refactor implementation in checkFinalSuspendNoThrow

Now when we are checking if the expression `co_await
promise.final_suspend()` is not throw, we would check unconditionally
for its child expressions recursively. It takes unnecessary time. And
the compiler would complains if the implementation in final_suspend()
may throw even if the higher level function signature marked noexcept
already.

This fixes bug48453 too.

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index decfb6a5d67d6..e7e60b7e7daf6 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -663,32 +663,32 @@ static void checkNoThrow(Sema , const Stmt *E,
   ThrowingDecls.insert(D);
 }
   };
-  auto SC = E->getStmtClass();
-  if (SC == Expr::CXXConstructExprClass) {
-auto const *Ctor = cast(E)->getConstructor();
+
+  if (auto *CE = dyn_cast(E)) {
+CXXConstructorDecl *Ctor = CE->getConstructor();
 checkDeclNoexcept(Ctor);
 // Check the corresponding destructor of the constructor.
-checkDeclNoexcept(Ctor->getParent()->getDestructor(), true);
-  } else if (SC == Expr::CallExprClass || SC == Expr::CXXMemberCallExprClass ||
- SC == Expr::CXXOperatorCallExprClass) {
-if (!cast(E)->isTypeDependent()) {
-  checkDeclNoexcept(cast(E)->getCalleeDecl());
-  auto ReturnType = 
cast(E)->getCallReturnType(S.getASTContext());
-  // Check the destructor of the call return type, if any.
-  if (ReturnType.isDestructedType() ==
-  QualType::DestructionKind::DK_cxx_destructor) {
-const auto *T =
-cast(ReturnType.getCanonicalType().getTypePtr());
-checkDeclNoexcept(
-dyn_cast(T->getDecl())->getDestructor(), true);
-  }
+checkDeclNoexcept(Ctor->getParent()->getDestructor(), /*IsDtor=*/true);
+  } else if (auto *CE = dyn_cast(E)) {
+if (CE->isTypeDependent())
+  return;
+
+checkDeclNoexcept(CE->getCalleeDecl());
+QualType ReturnType = CE->getCallReturnType(S.getASTContext());
+// Check the destructor of the call return type, if any.
+if (ReturnType.isDestructedType() ==
+QualType::DestructionKind::DK_cxx_destructor) {
+  const auto *T =
+  cast(ReturnType.getCanonicalType().getTypePtr());
+  checkDeclNoexcept(dyn_cast(T->getDecl())->getDestructor(),
+/*IsDtor=*/true);
+}
+  } else
+for (const auto *Child : E->children()) {
+  if (!Child)
+continue;
+  checkNoThrow(S, Child, ThrowingDecls);
 }
-  }
-  for (const auto *Child : E->children()) {
-if (!Child)
-  continue;
-checkNoThrow(S, Child, ThrowingDecls);
-  }
 }
 
 bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {

diff  --git 
a/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
index 131bae0d294fe..b501d537b7a8d 100644
--- a/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
@@ -11,32 +11,26 @@ struct coroutine_traits { using promise_type = typename 
Ret::promise_type; };
 
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *); // expected-note 2 {{must be 
declared with 'noexcept'}}
+  static coroutine_handle from_address(void *);
+  void *address() const noexcept;
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle); // expected-note 2 {{must 
be declared with 'noexcept'}}
+  coroutine_handle(coroutine_handle);
+  void *address() const noexcept;
 };
 
-struct suspend_never {
-  bool await_ready() { return true; }   // expected-note 2 {{must be 
declared with 'noexcept'}}
+struct suspend_always {
+  bool await_ready() { return false; }  // expected-note 2 {{must be 
declared with 'noexcept'}}
   void await_suspend(coroutine_handle<>) {} // expected-note 2 {{must be 
declared with 'noexcept'}}
   void await_resume() {}// expected-note 2 {{must be 
declared with 'noexcept'}}
-  ~suspend_never() noexcept(false); // expected-note 2 {{must be 
declared with 'noexcept'}}
-};
-
-struct suspend_always {
-  bool await_ready() { return false; }
-  void await_suspend(coroutine_handle<>) {}
-  void await_resume() {}
-

[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

2022-01-13 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie added a comment.

Could you please commit it on my behalf (Ella Ma )?
Thanks a lot.


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

https://reviews.llvm.org/D116329

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

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

2022-01-13 Thread Peixin Qiao via Phabricator via cfe-commits

peixin added a comment.

BTW, please rebase on main. There is one conflict about function 
`getOrCreateSrcLocStr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

2022-01-13 Thread Peixin Qiao via Phabricator via cfe-commits

peixin added a comment.
Herald added a subscriber: awarzynski.

@Meinersbur Here is the c++ code test. Without the chunk size specified, the 
running result using OMPIRBuilder is correct.

  #include
  
  int main() {
int i, N = 10;
float x = 0.0;
  
#pragma omp for schedule(static, 2)
for(i = 3; i <= N; i++) {
  x = x + i;
}
  
std::cout << "x = " << x << std::endl;
  
return 0;
  }

  $ clang++ chunk-1.cpp -fopenmp -fopenmp-enable-irbuilder && ./a.out
  x = 7
  $ clang++ chunk-1.cpp -fopenmp && ./a.out
  x = 52


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

2022-01-13 Thread Sam McCall via Phabricator via cfe-commits

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

Thanks, LG!


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

https://reviews.llvm.org/D116329

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

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

2022-01-13 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added a comment.

In D116966#3242170 , @aaronpuchert 
wrote:

> This is only observable on Windows, right?

Thanks for your reply. Yes, PLUGIN_TOOL argument is used on Windows only, so 
this problems can't be observed on non-Windows systems and this patch affects 
only Windows build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116966

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

[PATCH] D99484: [cmake] Use `GNUInstallDirs` to support custom installation dirs.

2022-01-13 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 updated this revision to Diff 399883.
Ericson2314 added a comment.

Rebase after landing the polly change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libompd/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -90,10 +92,10 @@
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
 DESTINATION lib/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -290,7 +290,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/polly"
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 
+include(ExtendPath)
 include(FindPrefixFromConfig)
 
 set(LLVM_INSTALL_PACKAGE_DIR "lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
@@ -83,17 +84,18 @@
 # Generate PollyConfig.cmake for the install tree.
 unset(POLLY_EXPORTS)
 find_prefix_from_config(POLLY_CONFIG_CODE POLLY_INSTALL_PREFIX "${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LLVM_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+extend_path(POLLY_CONFIG_LLVM_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
+extend_path(POLLY_CONFIG_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${POLLY_INSTALL_PACKAGE_DIR}")
+extend_path(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}" "lib${LLVM_LIBDIR_SUFFIX}")
+extend_path(base_includedir "\${POLLY_INSTALL_PREFIX}" "${CMAKE_INSTALL_INCLUDEDIR}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
-"\${POLLY_INSTALL_PREFIX}/include"
-"\${POLLY_INSTALL_PREFIX}/include/polly"
+"${base_includedir}"
+"${base_includedir}/polly"
 )
 else()
   set(POLLY_CONFIG_INCLUDE_DIRS
-"\${POLLY_INSTALL_PREFIX}/include"
+"${base_includedir}"
 ${ISL_INCLUDE_DIRS}
 )
 endif()
@@ -110,12 +112,12 @@
 foreach(tgt IN LISTS POLLY_CONFIG_EXPORTED_TARGETS)
   get_target_property(tgt_type ${tgt} TYPE)
   if (tgt_type STREQUAL "EXECUTABLE")
-set(tgt_prefix "bin/")
+set(tgt_prefix

[PATCH] D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

2022-01-13 Thread David Majnemer 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 rG072e2a7c67b7: [MS] Implement on-demand TLS initialization 
for Microsoft CXX ABI (authored by momo5502, committed by majnemer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115456

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/ms-thread_local.cpp

Index: clang/test/CodeGenCXX/ms-thread_local.cpp
===
--- clang/test/CodeGenCXX/ms-thread_local.cpp
+++ clang/test/CodeGenCXX/ms-thread_local.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s  -std=c++1y -triple=i686-pc-win32 -ftls-model=local-dynamic -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LD
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.25 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.20 -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LEGACY
+// RUN: %clang_cc1 %s -std=c++1y -triple=i686-pc-win32 -fms-compatibility-version=19.25 -ftls-model=local-dynamic -emit-llvm -o - | FileCheck %s -check-prefix=CHECK-LD
 
 struct A {
   A();
@@ -17,14 +18,22 @@
 
 // CHECK-DAG: @"?b@@3UA@@A" = dso_local thread_local global %struct.A zeroinitializer, align 1
 // CHECK-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-DAG: @__tls_guard = external dso_local thread_local global i8
 // CHECK-LD-DAG: @"?b@@3UA@@A" = dso_local thread_local(localdynamic) global %struct.A zeroinitializer, align 1
 // CHECK-LD-DAG: @"__tls_init$initializer$" = internal constant void ()* @__tls_init, section ".CRT$XDU"
+// CHECK-LD-DAG: @__tls_guard = external dso_local thread_local global i8
+// CHECK-LEGACY-NOT: @__tls_guard = external dso_local thread_local global i8
 thread_local A b;
 
-// CHECK-LABEL: define internal void @__tls_init()
-// CHECK: call void @"??__Eb@@YAXXZ"
-// CHECK-LD-LABEL: define internal void @__tls_init()
-// CHECK-LD: call void @"??__Eb@@YAXXZ"
+// CHECK-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: declare dso_local void @__dyn_tls_on_demand_init()
+// CHECK-LEGACY-NOT: declare dso_local void @__dyn_tls_on_demand_init()
+
+// CHECK-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK: call void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: define dso_local void @"?f@@YA?AUA@@XZ"(%struct.A* noalias sret(%struct.A) align 1 %agg.result)
+// CHECK-LD: call void @__dyn_tls_on_demand_init()
+// CHECK-LEGACY-NOT: call void @__dyn_tls_on_demand_init()
 
 thread_local A  = b;
 thread_local A  = c;
@@ -35,6 +44,22 @@
   return c;
 }
 
+// CHECK-LABEL: define dso_local i32 @"?g@@YAHXZ"()
+// CHECK-NOT: call void @__dyn_tls_on_demand_init()
+// CHECK-LD-LABEL: define dso_local i32 @"?g@@YAHXZ"()
+// CHECK-LD-NOT: call void @__dyn_tls_on_demand_init()
+
+thread_local int e = 2;
+
+int g() {
+  return e;
+}
+
+// CHECK-LABEL: define internal void @__tls_init()
+// CHECK: call void @"??__Eb@@YAXXZ"
+// CHECK-LD-LABEL: define internal void @__tls_init()
+// CHECK-LD: call void @"??__Eb@@YAXXZ"
+
 // CHECK: !llvm.linker.options = !{![[dyn_tls_init:[0-9]+]]}
 // CHECK: ![[dyn_tls_init]] = !{!"/include:___dyn_tls_init@12"}
 // CHECK-LD: !llvm.linker.options = !{![[dyn_tls_init:[0-9]+]]}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -401,7 +401,9 @@
   ArrayRef CXXThreadLocalInitVars) override;
 
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {
-return false;
+return getContext().getLangOpts().isCompatibleWithMSVC(
+   LangOptions::MSVC2019_5) &&
+   (!isEmittedWithConstantInitializer(VD) || mayNeedDestruction(VD));
   }
   LValue EmitThreadLocalVarDeclLValue(CodeGenFunction , const VarDecl *VD,
   QualType LValType) override;
@@ -2397,11 +2399,97 @@
   }
 }
 
+static llvm::GlobalValue *getTlsGuardVar(CodeGenModule ) {
+  // __tls_guard comes from the MSVC runtime and reflects
+  // whether TLS has been initialized for a particular thread.
+  // It is set from within __dyn_tls_init by the runtime.
+  // Every library and executable has its own variable.
+  llvm::Type *VTy = llvm::Type::getInt8Ty(CGM.getLLVMContext());
+  llvm::Constant *TlsGuardConstant =
+  CGM.CreateRuntimeVariable(VTy, "__tls_guard");
+  llvm::GlobalValue *TlsGuard =

[clang] 072e2a7 - [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

2022-01-13 Thread David Majnemer via cfe-commits


Author: Maurice Heumann
Date: 2022-01-13T21:23:23-08:00
New Revision: 072e2a7c67b74b8bde35f78ac0b0b13a269380f8

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

LOG: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

TLS initializers, for example constructors of thread-local variables, don't 
necessarily get called. If a thread was created before a module is loaded, the 
module's TLS initializers are not executed for this particular thread.

This is why Microsoft added support for dynamic TLS initialization. Before 
every use of thread-local variables, a check is added that runs the module's 
TLS initializers on-demand.

To do this, the method `__dyn_tls_on_demand_init` gets called. Internally, it 
simply calls `__dyn_tls_init`.

No additional TLS initializer that sets the guard needs to be emitted, as the 
guard always gets set by `__dyn_tls_init`.
The guard is also checked again within `__dyn_tls_init`. This makes our check 
redundant, however, as Microsoft's compiler also emits this check, the 
behaviour is adopted here.

Reviewed By: majnemer

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.h
clang/lib/CodeGen/CGCXXABI.cpp
clang/lib/CodeGen/CGCXXABI.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/ms-thread_local.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 35b33c2e09716..09afa641acf9b 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -124,6 +124,7 @@ class LangOptions : public LangOptionsBase {
 MSVC2017_5 = 1912,
 MSVC2017_7 = 1914,
 MSVC2019 = 1920,
+MSVC2019_5 = 1925,
 MSVC2019_8 = 1928,
   };
 

diff  --git a/clang/lib/CodeGen/CGCXXABI.cpp b/clang/lib/CodeGen/CGCXXABI.cpp
index 9714730e3c4bf..0b441e382f11c 100644
--- a/clang/lib/CodeGen/CGCXXABI.cpp
+++ b/clang/lib/CodeGen/CGCXXABI.cpp
@@ -154,6 +154,51 @@ void CGCXXABI::setCXXABIThisValue(CodeGenFunction , 
llvm::Value *ThisPtr) {
   CGF.CXXABIThisValue = ThisPtr;
 }
 
+bool CGCXXABI::mayNeedDestruction(const VarDecl *VD) const {
+  if (VD->needsDestruction(getContext()))
+return true;
+
+  // If the variable has an incomplete class type (or array thereof), it
+  // might need destruction.
+  const Type *T = VD->getType()->getBaseElementTypeUnsafe();
+  if (T->getAs() && T->isIncompleteType())
+return true;
+
+  return false;
+}
+
+bool CGCXXABI::isEmittedWithConstantInitializer(
+const VarDecl *VD, bool InspectInitForWeakDef) const {
+  VD = VD->getMostRecentDecl();
+  if (VD->hasAttr())
+return true;
+
+  // All later checks examine the initializer specified on the variable. If
+  // the variable is weak, such examination would not be correct.
+  if (!InspectInitForWeakDef && (VD->isWeak() || VD->hasAttr()))
+return false;
+
+  const VarDecl *InitDecl = VD->getInitializingDeclaration();
+  if (!InitDecl)
+return false;
+
+  // If there's no initializer to run, this is constant initialization.
+  if (!InitDecl->hasInit())
+return true;
+
+  // If we have the only definition, we don't need a thread wrapper if we
+  // will emit the value as a constant.
+  if (isUniqueGVALinkage(getContext().GetGVALinkageForVariable(VD)))
+return !mayNeedDestruction(VD) && InitDecl->evaluateValue();
+
+  // Otherwise, we need a thread wrapper unless we know that every
+  // translation unit will emit the value as a constant. We rely on the
+  // variable being constant-initialized in every translation unit if it's
+  // constant-initialized in any translation unit, which isn't actually
+  // guaranteed by the standard but is necessary for sanity.
+  return InitDecl->hasConstantInitialization();
+}
+
 void CGCXXABI::EmitReturnFromThunk(CodeGenFunction ,
RValue RV, QualType ResultType) {
   assert(!CGF.hasAggregateEvaluationKind(ResultType) &&

diff  --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index e94e19bab0ad1..b96222b3ce280 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -79,6 +79,18 @@ class CGCXXABI {
 
   ASTContext () const { return CGM.getContext(); }
 
+  bool mayNeedDestruction(const VarDecl *VD) const;
+
+  /// Determine whether we will definitely emit this variable with a constant
+  /// initializer, either because the language semantics demand it or because
+  /// we know that the initializer is a constant.
+  // For weak definitions, any initializer available in the current translation
+  // is not necessarily reflective of the initializer used; such initializers
+  // are ignored unless if

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

2022-01-13 Thread Vitaly Buka via Phabricator via cfe-commits

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

LGTM with few nits




Comment at: clang/lib/CodeGen/BackendUtil.cpp:362
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel,
+ CGOpts.SanitizeMemoryParamRetval != 0}));
 

we use implicit cast mostly, e.g. addAddressSanitizerPasses



Comment at: clang/test/CodeGen/param-retval-eager-checks.c:1
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - 
%s | \
+// RUN: FileCheck %s --check-prefix=CLEAN

Would you like to remove "| \" ?
80 char limit is not enforced on tests, multi line RUN: is even harder to read 
then long line



Comment at: clang/test/CodeGen/param-retval-eager-checks.c:1
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - 
%s | \
+// RUN: FileCheck %s --check-prefix=CLEAN

vitalybuka wrote:
> Would you like to remove "| \" ?
> 80 char limit is not enforced on tests, multi line RUN: is even harder to 
> read then long line
This dir contains tests for a lot of different componets
can you please add msan in name

e.g. "msan-param-retval.c"



Comment at: clang/test/CodeGen/param-retval-eager-checks.c:5
+// RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-Xclang -enable-noundef-analysis -mllvm -msan-eager-checks -o - %s | \
+// RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER

we probably don't need mllvm test here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116633

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

[PATCH] D116736: [Clang] Add __builtin_reduce_or and __builtin_reduce_and

2022-01-13 Thread Jun Zhang via Phabricator via cfe-commits

junaire added a comment.

> @junaire did you already get commit access or should I commit this change on 
> your behalf?

Yeah, I already have commit access, just waiting for your approval ;D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116736

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

[PATCH] D117271: [AArch64] clang support for Armv8.8/9.3 MOPS

2022-01-13 Thread Son Tuan Vu via Phabricator via cfe-commits

tyb0807 created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
tyb0807 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This introduces clang command line support for the new Armv8.8-A and
Armv9.3-A instructions for standardising memcpy, memset and memmove
operations, which was previously introduced into LLVM in
https://reviews.llvm.org/D116157.

Patch by Lucas Prates, Tomas Matheson and Son Tuan Vu.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117271

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/Driver/aarch64-mops.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp


Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1519,6 +1519,7 @@
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
+  {"mops", "nomops", "+mops", "-mops"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -116,6 +116,8 @@
 Features.push_back("+sme-i64");
   if (Extensions & AArch64::AEK_HBC)
 Features.push_back("+hbc");
+  if (Extensions & AArch64::AEK_MOPS)
+Features.push_back("+mops");
 
   return true;
 }
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -70,6 +70,7 @@
   AEK_SMEF64 =  1ULL << 38,
   AEK_SMEI64 =  1ULL << 39,
   AEK_HBC = 1ULL << 40,
+  AEK_MOPS =1ULL << 41,
 };
 
 enum class ArchKind {
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,6 +145,7 @@
 AARCH64_ARCH_EXT_NAME("sme-f64",  AArch64::AEK_SMEF64,  "+sme-f64",
  "-sme-f64")
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64",
  "-sme-i64")
 AARCH64_ARCH_EXT_NAME("hbc",  AArch64::AEK_HBC, "+hbc",
  "-hbc")
+AARCH64_ARCH_EXT_NAME("mops", AArch64::AEK_MOPS,"+mops",   
  "-mops")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME
Index: clang/test/Driver/aarch64-mops.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-mops.c
@@ -0,0 +1,6 @@
+// Test that target feature mops is implemented and available correctly
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops %s 
2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+mops"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 
2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// NO_MOPS: "-target-feature" "-mops"
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasHBC;
+  bool HasMOPS;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -544,6 +544,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasHBC = false;
+  HasMOPS = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 


Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1519,6 +1519,7 @@
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
+  {"mops", "nomops", "+mops", "-mops"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -116,6 +116,8 @@
 Features.push_back("+sme-i64");
   if (Extensions & AArch64::AEK_HBC)
 Features.push_back("+hbc");
+  if (Extensions & AArch64::AEK_MOPS)
+

[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

2022-01-13 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei added a comment.

It caused by bug in AutoUpgrade.cpp, fixed on D117270 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115942

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2022-01-13 Thread Owen Pan via Phabricator via cfe-commits

owenpan updated this revision to Diff 399869.
owenpan added a comment.

- Fixed a bug that missed braces in nested blocks.
- Added a test case.


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

https://reviews.llvm.org/D116316

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18781,6 +18781,7 @@
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(ReflowComments);
+  CHECK_PARSE_BOOL(RemoveBracesLLVM);
   CHECK_PARSE_BOOL(SortUsingDeclarations);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
@@ -23226,6 +23227,367 @@
   verifyFormat("struct Z : X {};", Style);
 }
 
+TEST_F(FormatTest, RemoveBraces) {
+  FormatStyle Style = getLLVMStyle();
+  Style.RemoveBracesLLVM = true;
+
+  // The following eight test cases are fully-braced versions of the examples at
+  // "llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-
+  // statement-bodies-of-if-else-loop-statements".
+
+  // 1. Omit the braces, since the body is simple and clearly associated with
+  // the if.
+  verifyFormat("if (isa(D))\n"
+   "  handleFunctionDecl(D);\n"
+   "else if (isa(D))\n"
+   "  handleVarDecl(D);",
+   "if (isa(D)) {\n"
+   "  handleFunctionDecl(D);\n"
+   "} else if (isa(D)) {\n"
+   "  handleVarDecl(D);\n"
+   "}",
+   Style);
+
+  // 2. Here we document the condition itself and not the body.
+  verifyFormat("if (isa(D)) {\n"
+   "  // It is necessary that we explain the situation with this\n"
+   "  // surprisingly long comment, so it would be unclear\n"
+   "  // without the braces whether the following statement is in\n"
+   "  // the scope of the `if`.\n"
+   "  // Because the condition is documented, we can't really\n"
+   "  // hoist this comment that applies to the body above the\n"
+   "  // if.\n"
+   "  handleOtherDecl(D);\n"
+   "}",
+   Style);
+
+  // 3. Use braces on the outer `if` to avoid a potential dangling else
+  // situation.
+  verifyFormat("if (isa(D)) {\n"
+   "  for (auto *A : D.attrs())\n"
+   "if (shouldProcessAttr(A))\n"
+   "  handleAttr(A);\n"
+   "}",
+   "if (isa(D)) {\n"
+   "  for (auto *A : D.attrs()) {\n"
+   "if (shouldProcessAttr(A)) {\n"
+   "  handleAttr(A);\n"
+   "}\n"
+   "  }\n"
+   "}",
+   Style);
+
+  // 4. Use braces for the `if` block to keep it uniform with the else block.
+  verifyFormat("if (isa(D)) {\n"
+   "  handleFunctionDecl(D);\n"
+   "} else {\n"
+   "  // In this else case, it is necessary that we explain the\n"
+   "  // situation with this surprisingly long comment, so it\n"
+   "  // would be unclear without the braces whether the\n"
+   "  // following statement is in the scope of the `if`.\n"
+   "  handleOtherDecl(D);\n"
+   "}",
+   Style);
+
+  // 5. This should also omit braces.  The `for` loop contains only a single
+  // statement, so it shouldn't have braces.  The `if` also only contains a
+  // single simple statement (the for loop), so it also should omit braces.
+  verifyFormat("if (isa(D))\n"
+   "  for (auto *A : D.attrs())\n"
+   "handleAttr(A);",
+   "if (isa(D)) {\n"
+   "  for (auto *A : D.attrs()) {\n"
+   "handleAttr(A);\n"
+   "  }\n"
+   "}",
+   Style);
+
+  // 6. Use braces for the outer `if` since the nested `for` is braced.
+  verifyFormat("if (isa(D)) {\n"
+   "  for (auto *A : D.attrs()) {\n"
+   "// In this for loop body, it is necessary that we explain\n"
+   "// the situation with this surprisingly long comment,\n"
+   "// forcing braces on the `for` block.\n"
+   "handleAttr(A);\n"
+   "  }\n"
+   "}",
+   Style);
+
+  // 7. Use braces on the outer block because there are more than two levels of
+  // nesting.
+  verifyFormat("if (isa(D)) {\n"
+   "  for (auto *A : D.attrs())\n"
+   "for

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-13 Thread Florin Iucha via Phabricator via cfe-commits

0x8000- added a comment.

+1

Let's get this in :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D117093: [C++20] [Modules] Exit early if export decl is invalid

2022-01-13 Thread Chuanqi Xu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f8916cfdd94: [C++20] [Modules] Exit early if export decl is 
not valid (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117093

Files:
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/export-in-non-modules.cpp


Index: clang/test/Modules/export-in-non-modules.cpp
===
--- /dev/null
+++ clang/test/Modules/export-in-non-modules.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+export struct Unit { // expected-error {{export declaration can only be used 
within a module interface unit after the module declaration}}
+  bool operator<(const Unit &);
+};
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -527,21 +527,30 @@
   // Set this temporarily so we know the export-declaration was braced.
   D->setRBraceLoc(LBraceLoc);
 
+  CurContext->addDecl(D);
+  PushDeclContext(S, D);
+
   // C++2a [module.interface]p1:
   //   An export-declaration shall appear only [...] in the purview of a module
   //   interface unit. An export-declaration shall not appear directly or
   //   indirectly within [...] a private-module-fragment.
   if (ModuleScopes.empty() || !ModuleScopes.back().Module->isModulePurview()) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+D->setInvalidDecl();
+return D;
   } else if (!ModuleScopes.back().ModuleInterface) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
 Diag(ModuleScopes.back().BeginLoc,
  diag::note_not_module_interface_add_export)
 << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+D->setInvalidDecl();
+return D;
   } else if (ModuleScopes.back().Module->Kind ==
  Module::PrivateModuleFragment) {
 Diag(ExportLoc, diag::err_export_in_private_module_fragment);
 Diag(ModuleScopes.back().BeginLoc, diag::note_private_module_fragment);
+D->setInvalidDecl();
+return D;
   }
 
   for (const DeclContext *DC = CurContext; DC; DC = DC->getLexicalParent()) {
@@ -553,7 +562,7 @@
 Diag(ND->getLocation(), diag::note_anonymous_namespace);
 // Don't diagnose internal-linkage declarations in this region.
 D->setInvalidDecl();
-break;
+return D;
   }
 
   //   A declaration is exported if it is [...] a namespace-definition
@@ -572,10 +581,10 @@
 Diag(ExportLoc, diag::err_export_within_export);
 if (ED->hasBraces())
   Diag(ED->getLocation(), diag::note_export);
+D->setInvalidDecl();
+return D;
   }
 
-  CurContext->addDecl(D);
-  PushDeclContext(S, D);
   D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::VisibleWhenImported);
   return D;
 }


Index: clang/test/Modules/export-in-non-modules.cpp
===
--- /dev/null
+++ clang/test/Modules/export-in-non-modules.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+export struct Unit { // expected-error {{export declaration can only be used within a module interface unit after the module declaration}}
+  bool operator<(const Unit &);
+};
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -527,21 +527,30 @@
   // Set this temporarily so we know the export-declaration was braced.
   D->setRBraceLoc(LBraceLoc);
 
+  CurContext->addDecl(D);
+  PushDeclContext(S, D);
+
   // C++2a [module.interface]p1:
   //   An export-declaration shall appear only [...] in the purview of a module
   //   interface unit. An export-declaration shall not appear directly or
   //   indirectly within [...] a private-module-fragment.
   if (ModuleScopes.empty() || !ModuleScopes.back().Module->isModulePurview()) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+D->setInvalidDecl();
+return D;
   } else if (!ModuleScopes.back().ModuleInterface) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
 Diag(ModuleScopes.back().BeginLoc,
  diag::note_not_module_interface_add_export)
 << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+D->setInvalidDecl();
+return D;
   } else if (ModuleScopes.back().Module->Kind ==
  Module::PrivateModuleFragment) {
 Diag(ExportLoc, diag::err_export_in_private_module_fragment);
 Diag(ModuleScopes.back().BeginLoc, diag::note_private_module_fragment);
+D->setInvalidDecl();
+return D;
   }
 
   for (const DeclContext *DC = CurContext; DC; DC = DC->getLexicalParent()) {
@@ -553,7 +562,7 @@

[clang] 4f8916c - [C++20] [Modules] Exit early if export decl is not valid

2022-01-13 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2022-01-14T10:21:42+08:00
New Revision: 4f8916cfdd941db2a4c4cdad6e5bc549532277a2

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

LOG: [C++20] [Modules] Exit early if export decl is not valid

This patch fixes a crash due to following simple program:
> export struct Unit {
>bool operator<(const Unit&);
> };

It would crash since the compiler would set the module ownership for
Unit. And the declaration with a module ownership is assumed to own a
module. But here isn't one. So here is the crash.

This patch fixes this by exiting early if it finds the export decl is
already invalid.

Reviewed By: aaron.ballman

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

Added: 
clang/test/Modules/export-in-non-modules.cpp

Modified: 
clang/lib/Sema/SemaModule.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index a4b9f3c242c1c..996063f83e946 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -527,21 +527,30 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation 
ExportLoc,
   // Set this temporarily so we know the export-declaration was braced.
   D->setRBraceLoc(LBraceLoc);
 
+  CurContext->addDecl(D);
+  PushDeclContext(S, D);
+
   // C++2a [module.interface]p1:
   //   An export-declaration shall appear only [...] in the purview of a module
   //   interface unit. An export-declaration shall not appear directly or
   //   indirectly within [...] a private-module-fragment.
   if (ModuleScopes.empty() || !ModuleScopes.back().Module->isModulePurview()) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+D->setInvalidDecl();
+return D;
   } else if (!ModuleScopes.back().ModuleInterface) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
 Diag(ModuleScopes.back().BeginLoc,
  diag::note_not_module_interface_add_export)
 << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+D->setInvalidDecl();
+return D;
   } else if (ModuleScopes.back().Module->Kind ==
  Module::PrivateModuleFragment) {
 Diag(ExportLoc, diag::err_export_in_private_module_fragment);
 Diag(ModuleScopes.back().BeginLoc, diag::note_private_module_fragment);
+D->setInvalidDecl();
+return D;
   }
 
   for (const DeclContext *DC = CurContext; DC; DC = DC->getLexicalParent()) {
@@ -553,7 +562,7 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation 
ExportLoc,
 Diag(ND->getLocation(), diag::note_anonymous_namespace);
 // Don't diagnose internal-linkage declarations in this region.
 D->setInvalidDecl();
-break;
+return D;
   }
 
   //   A declaration is exported if it is [...] a namespace-definition
@@ -572,10 +581,10 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation 
ExportLoc,
 Diag(ExportLoc, diag::err_export_within_export);
 if (ED->hasBraces())
   Diag(ED->getLocation(), diag::note_export);
+D->setInvalidDecl();
+return D;
   }
 
-  CurContext->addDecl(D);
-  PushDeclContext(S, D);
   D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::VisibleWhenImported);
   return D;
 }

diff  --git a/clang/test/Modules/export-in-non-modules.cpp 
b/clang/test/Modules/export-in-non-modules.cpp
new file mode 100644
index 0..c10d863d27005
--- /dev/null
+++ b/clang/test/Modules/export-in-non-modules.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+export struct Unit { // expected-error {{export declaration can only be used 
within a module interface unit after the module declaration}}
+  bool operator<(const Unit &);
+};



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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

2022-01-13 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

Thanks! Does this have any impact on binary size, as a proxy for more thorough 
performance testing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D117199: [RISCV] Add missing namespace (NFC)

2022-01-13 Thread Shao-Ce SUN via Phabricator via cfe-commits

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

OK, LGTM. Thanks! @fakepaper56 @khchen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117199

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

2022-01-13 Thread Dwayne Robinson via Phabricator via cfe-commits

fdwr added a comment.

> I think no one just had thought about it. :)

@HazardyKnusperkeks Happy new year. Just in case it gets forgotten, I'm 
squeaking on behalf of a few stackoverflowers  (#52158077 
,
 #50689027 
).
 Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

2022-01-13 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie updated this revision to Diff 399841.
OikawaKirie added a comment.

Updated as suggested.


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

https://reviews.llvm.org/D116329

Files:
  clang/test/Tooling/clang-check-analyze-save-temps.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -208,27 +208,37 @@
   ClangTool Tool(OptionsParser.getCompilations(),
  OptionsParser.getSourcePathList());
 
-  // Clear adjusters because -fsyntax-only is inserted by the default chain.
-  Tool.clearArgumentsAdjusters();
-
-  // Reset output path if is provided by user.
-  Tool.appendArgumentsAdjuster(
-  Analyze ? [&](const CommandLineArguments , StringRef File) {
-  auto Ret = getClangStripOutputAdjuster()(Args, File);
-  if (!AnalyzerOutput.empty()) {
-Ret.emplace_back("-o");
-Ret.emplace_back(AnalyzerOutput);
-  }
-  return Ret;
-}
-  : getClangStripOutputAdjuster());
-
-  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
-
-  // Running the analyzer requires --analyze. Other modes can work with the
-  // -fsyntax-only option.
-  Tool.appendArgumentsAdjuster(getInsertArgumentAdjuster(
-  Analyze ? "--analyze" : "-fsyntax-only", ArgumentInsertPosition::BEGIN));
+  if (Analyze) {
+// Set output path if is provided by user.
+//
+// As the original -o options have been removed by default via the
+// strip-output adjuster, we only need to add the analyzer -o options here
+// when it is provided by users.
+if (!AnalyzerOutput.empty())
+  Tool.appendArgumentsAdjuster(
+  getInsertArgumentAdjuster(CommandLineArguments{"-o", AnalyzerOutput},
+ArgumentInsertPosition::END));
+
+// Running the analyzer requires --analyze. Other modes can work with the
+// -fsyntax-only option.
+//
+// The syntax-only adjuster is installed by default.
+// Good: It also strips options that trigger extra output, like 
-save-temps.
+// Bad:  We don't want the -fsyntax-only when executing the static 
analyzer.
+//
+// To enable the static analyzer, we first strip all -fsyntax-only options
+// and then add an --analyze option to the front.
+Tool.appendArgumentsAdjuster(
+[&](const CommandLineArguments , StringRef /*unused*/) {
+  CommandLineArguments AdjustedArgs;
+  for (const std::string  : Args)
+if (Arg != "-fsyntax-only")
+  AdjustedArgs.emplace_back(Arg);
+  return AdjustedArgs;
+});
+Tool.appendArgumentsAdjuster(
+getInsertArgumentAdjuster("--analyze", ArgumentInsertPosition::BEGIN));
+  }
 
   ClangCheckActionFactory CheckFactory;
   std::unique_ptr FrontendFactory;
Index: clang/test/Tooling/clang-check-analyze-save-temps.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-analyze-save-temps.cpp
@@ -0,0 +1,19 @@
+// Check whether output generation options (like -save-temps) will not affect
+// the execution of the analyzer.
+
+// RUN: clang-check -analyze %s -- -save-temps -c -Xclang -verify
+
+// Check whether redundant -fsyntax-only options will affect the execution of
+// the analyzer.
+
+// RUN: clang-check -analyze %s -- \
+// RUN:   -fsyntax-only -c -fsyntax-only -Xclang -verify 2>&1 | \
+// RUN:   FileCheck %s --allow-empty
+
+// CHECK-NOT: argument unused during compilation: '--analyze'
+
+void a(int *x) {
+  if (x) {
+  }
+  *x = 47; // expected-warning {{Dereference of null pointer}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -208,27 +208,37 @@
   ClangTool Tool(OptionsParser.getCompilations(),
  OptionsParser.getSourcePathList());
 
-  // Clear adjusters because -fsyntax-only is inserted by the default chain.
-  Tool.clearArgumentsAdjusters();
-
-  // Reset output path if is provided by user.
-  Tool.appendArgumentsAdjuster(
-  Analyze ? [&](const CommandLineArguments , StringRef File) {
-  auto Ret = getClangStripOutputAdjuster()(Args, File);
-  if (!AnalyzerOutput.empty()) {
-Ret.emplace_back("-o");
-Ret.emplace_back(AnalyzerOutput);
-  }
-  return Ret;
-}
-  : getClangStripOutputAdjuster());
-
-  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
-
-  // Running the analyzer requires --analyze. Other modes can work with the
-  // -fsyntax-only option.
-

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

2022-01-13 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks updated this revision to Diff 399840.
aeubanks added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

Files:
  clang/lib/CodeGen/Address.h


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -14,30 +14,37 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
-  llvm::Type *ElementType;
-  CharUnits Alignment;
+  // Int portion stores upper 3 bits of the log of the alignment.
+  llvm::PointerIntPair Pointer;
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
 
 protected:
   Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
   Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
+  : Pointer(pointer), ElementType(elementType) {
 assert(pointer != nullptr && "Pointer cannot be null");
 assert(elementType != nullptr && "Element type cannot be null");
 assert(llvm::cast(pointer->getType())
->isOpaqueOrPointeeTypeMatches(elementType) &&
"Incorrect pointer element type");
-assert(!alignment.isZero() && "Alignment cannot be zero");
+assert(alignment.isPowerOfTwo() && "Alignment cannot be zero");
+auto AlignLog = llvm::Log2_64(alignment.getQuantity());
+assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits");
+Pointer.setInt(AlignLog >> 3);
+ElementType.setInt(AlignLog & 7);
   }
 
   // Deprecated: Use constructor with explicit element type instead.
@@ -46,11 +53,11 @@
 Alignment) {}
 
   static Address invalid() { return Address(nullptr); }
-  bool isValid() const { return Pointer != nullptr; }
+  bool isValid() const { return Pointer.getPointer() != nullptr; }
 
   llvm::Value *getPointer() const {
 assert(isValid());
-return Pointer;
+return Pointer.getPointer();
   }
 
   /// Return the type of the pointer value.
@@ -61,7 +68,7 @@
   /// Return the type of the values stored in this address.
   llvm::Type *getElementType() const {
 assert(isValid());
-return ElementType;
+return ElementType.getPointer();
   }
 
   /// Return the address space that this address resides in.
@@ -77,19 +84,21 @@
   /// Return the alignment of this pointer.
   CharUnits getAlignment() const {
 assert(isValid());
-return Alignment;
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1 << AlignLog);
   }
 
   /// Return address with different pointer, but same element type and
   /// alignment.
   Address withPointer(llvm::Value *NewPointer) const {
-return Address(NewPointer, ElementType, Alignment);
+return Address(NewPointer, ElementType.getPointer(), getAlignment());
   }
 
   /// Return address with different alignment, but same pointer and element
   /// type.
   Address withAlignment(CharUnits NewAlignment) const {
-return Address(Pointer, ElementType, NewAlignment);
+return Address(Pointer.getPointer(), ElementType.getPointer(),
+   NewAlignment);
   }
 };
 


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -14,30 +14,37 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
-  llvm::Type *ElementType;
-  CharUnits Alignment;
+  // Int portion stores upper 3 bits of the log of the alignment.
+  llvm::PointerIntPair Pointer;
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
 
 protected:
   Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
   Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
+  : Pointer(pointer), ElementType(elementType) {
 assert(pointer != nullptr && "Pointer cannot be null");
 assert(elementType != nullptr && "Element type cannot be null");
 assert(llvm::cast(pointer->getType())
->isOpaqueOrPointeeTypeMatches(elementType)

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-13 Thread Christopher Di Bella via Phabricator via cfe-commits

cjdb added a comment.

> That said, I think this particular check is more mature and closer to landing 
> than the other one. Based on the amount of effort in this patch already, 
> unless there are strong objections, I think we should base the "make this 
> const if it can be made so" checks on this one.

Ack. How about we worry about D107873  once 
this patch lands, and once I've had a chance to rebase it on top of this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

2022-01-13 Thread Arthur Eubanks via Phabricator via cfe-commits

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

This mitigates the extra memory caused by D115725 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117262

Files:
  clang/lib/CodeGen/Address.h


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -14,30 +14,36 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
-  llvm::Type *ElementType;
-  CharUnits Alignment;
+  llvm::PointerIntPair Pointer;
+  llvm::PointerIntPair ElementType;
+  // CharUnits Alignment;
 
 protected:
   Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
   Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
+  : Pointer(pointer), ElementType(elementType) {
 assert(pointer != nullptr && "Pointer cannot be null");
 assert(elementType != nullptr && "Element type cannot be null");
 assert(llvm::cast(pointer->getType())
->isOpaqueOrPointeeTypeMatches(elementType) &&
"Incorrect pointer element type");
-assert(!alignment.isZero() && "Alignment cannot be zero");
+assert(alignment.isPowerOfTwo() && "Alignment cannot be zero");
+auto AlignLog = llvm::Log2_64(alignment.getQuantity());
+assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits");
+Pointer.setInt(AlignLog >> 3);
+ElementType.setInt(AlignLog & 7);
   }
 
   // Deprecated: Use constructor with explicit element type instead.
@@ -46,11 +52,11 @@
 Alignment) {}
 
   static Address invalid() { return Address(nullptr); }
-  bool isValid() const { return Pointer != nullptr; }
+  bool isValid() const { return Pointer.getPointer() != nullptr; }
 
   llvm::Value *getPointer() const {
 assert(isValid());
-return Pointer;
+return Pointer.getPointer();
   }
 
   /// Return the type of the pointer value.
@@ -61,7 +67,7 @@
   /// Return the type of the values stored in this address.
   llvm::Type *getElementType() const {
 assert(isValid());
-return ElementType;
+return ElementType.getPointer();
   }
 
   /// Return the address space that this address resides in.
@@ -77,19 +83,21 @@
   /// Return the alignment of this pointer.
   CharUnits getAlignment() const {
 assert(isValid());
-return Alignment;
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1 << AlignLog);
   }
 
   /// Return address with different pointer, but same element type and
   /// alignment.
   Address withPointer(llvm::Value *NewPointer) const {
-return Address(NewPointer, ElementType, Alignment);
+return Address(NewPointer, ElementType.getPointer(), getAlignment());
   }
 
   /// Return address with different alignment, but same pointer and element
   /// type.
   Address withAlignment(CharUnits NewAlignment) const {
-return Address(Pointer, ElementType, NewAlignment);
+return Address(Pointer.getPointer(), ElementType.getPointer(),
+   NewAlignment);
   }
 };
 


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -14,30 +14,36 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
-  llvm::Type *ElementType;
-  CharUnits Alignment;
+  llvm::PointerIntPair Pointer;
+  llvm::PointerIntPair ElementType;
+  // CharUnits Alignment;
 
 protected:
   Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
   Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
+  : Pointer(pointer), ElementType(elementType) {
 assert(pointer != nullptr && "Pointer cannot be null");
 assert(elementType != nullptr && "Element type cannot be null");
 assert(llvm::cast(pointer->getType())
->isOpaqueOrPointeeTypeMatches(elementType) &&
"Incorrect pointer element type");
-assert(!alignment.isZero() && "Alignment cannot be zero");
+

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Owen Pan via Phabricator via cfe-commits

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4810
+
+  verifyFormat("#define A LOOONG() LOOONG()\n",
+   ZeroColumn);

Please remove the newline and re-run `FormatTests`.



Comment at: clang/unittests/Format/FormatTest.cpp:4810-4811
+
+  verifyFormat("#define STRINGIFY(t) #t\n"
+   "#define MAKEVERSIONSTRING(x, y, z, build) STRINGIFY(x) \".\" 
STRINGIFY(y) \".\" STRINGIFY(z) \".\" STRINGIFY(build)\n",
+   ZeroColumn);

curdeius wrote:
> HazardyKnusperkeks wrote:
> > curdeius wrote:
> > > Please test with something simpler.
> > Did this show the bugged behavior without the patch?
> > 
> > Regarding the `// clang-format off` there is the question what do we want 
> > in the tests? Show the long lines as the long lines, then we need to turn 
> > it off some times. Or do we want to keep the limit and break the strings, 
> > thus making it harder to read for the human?
> > Did this show the bugged behavior without the patch?
> Yes. I checked with a pretty fresh master.
> 
> > Regarding the `// clang-format off` there is the question what do we want 
> > in the tests? Show the long lines as the long lines, then we need to turn 
> > it off some times. Or do we want to keep the limit and break the strings, 
> > thus making it harder to read for the human?
> I'm pretty much against adding `// clang-format off` too, it hurts more than 
> it helps IMO.
> Regarding the `// clang-format off` there is the question what do we want in 
> the tests? Show the long lines as the long lines, then we need to turn it off 
> some times. Or do we want to keep the limit and break the strings, thus 
> making it harder to read for the human?

I prefer the latter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

2022-01-13 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a subscriber: aaron.ballman.
aaronpuchert added a comment.

This is only observable on Windows, right? Then perhaps @aaron.ballman can 
comment on this.

The change looks legitimate to me, but I don't work on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116966

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

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

2022-01-13 Thread Kevin Athey via Phabricator via cfe-commits

kda added a comment.

PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116633

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

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

2022-01-13 Thread Kevin Athey via Phabricator via cfe-commits

kda updated this revision to Diff 399826.
kda added a comment.
Herald added a subscriber: ormris.

enable eager-checks with -fsanitize-memory-param-retval


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116633

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/param-retval-eager-checks.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- /dev/null
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -0,0 +1,12 @@
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// CHECK: "-fsanitize-memory-param-retval"
+
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fsanitize-memory-param-retval"
+
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
Index: clang/test/CodeGen/param-retval-eager-checks.c
===
--- /dev/null
+++ clang/test/CodeGen/param-retval-eager-checks.c
@@ -0,0 +1,19 @@
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: FileCheck %s --check-prefix=CLEAN
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -o - %s | \
+// RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -mllvm -msan-eager-checks -o - %s | \
+// RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: FileCheck %s --check-prefixes=CLEAN
+// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -Xclang -enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
+// RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
+
+void bar(int x) {
+}
+
+// CLEAN:   define dso_local void @bar(i32 %x) #0 {
+// NOUNDEF: define dso_local void @bar(i32 noundef %x) #0 {
+// CLEAN:@__msan_param_tls
+// NOUNDEF_ONLY: @__msan_param_tls
+// EAGER-NOT:@__msan_param_tls
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -641,10 +641,14 @@
 Args.hasFlag(options::OPT_fsanitize_memory_use_after_dtor,
  options::OPT_fno_sanitize_memory_use_after_dtor,
  MsanUseAfterDtor);
+MsanParamRetval = Args.hasFlag(
+options::OPT_fsanitize_memory_param_retval,
+options::OPT_fno_sanitize_memory_param_retval, MsanParamRetval);
 NeedPIE |= !(TC.getTriple().isOSLinux() &&
  TC.getTriple().getArch() == llvm::Triple::x86_64);
   } else {
 MsanUseAfterDtor = false;
+MsanParamRetval = false;
   }
 
   if (AllAddedKinds & SanitizerKind::Thread) {
@@ -1096,6 +1100,9 @@
   if (MsanUseAfterDtor)
 CmdArgs.push_back("-fsanitize-memory-use-after-dtor");
 
+  if (MsanParamRetval)
+CmdArgs.push_back("-fsanitize-memory-param-retval");
+
   // FIXME: Pass these parameters as function attributes, not as -llvm flags.
   if (!TsanMemoryAccess) {
 CmdArgs.push_back("-mllvm");
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -358,7 +358,8 @@
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
   PM.add(createMemorySanitizerLegacyPassPass(
-  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel,
+ CGOpts.SanitizeMemoryParamRetval != 0}));
 
   // MemorySanitizer inserts complex

[PATCH] D116844: [Driver][Fuchsia] -r: imply -nostdlib like GCC

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe289561205e8: [Driver][Fuchsia] -r: imply -nostdlib like GCC 
(authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116844

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -72,6 +72,8 @@
 // CHECK-RELOCATABLE-NOT: "-pie"
 // CHECK-RELOCATABLE-NOT: "--build-id"
 // CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l
+// CHECK-RELOCATABLE-NOT: crt{{[^.]+}}.o
 
 // RUN: %clang %s -### --target=x86_64-unknown-fuchsia -nodefaultlibs 
-fuse-ld=lld 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -109,7 +109,8 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("Scrt1.o")));
 }
@@ -131,7 +132,8 @@
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_static))
   CmdArgs.push_back("-Bdynamic");
 


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -72,6 +72,8 @@
 // CHECK-RELOCATABLE-NOT: "-pie"
 // CHECK-RELOCATABLE-NOT: "--build-id"
 // CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l
+// CHECK-RELOCATABLE-NOT: crt{{[^.]+}}.o
 
 // RUN: %clang %s -### --target=x86_64-unknown-fuchsia -nodefaultlibs -fuse-ld=lld 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -109,7 +109,8 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("Scrt1.o")));
 }
@@ -131,7 +132,8 @@
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_static))
   CmdArgs.push_back("-Bdynamic");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e289561 - [Driver][Fuchsia] -r: imply -nostdlib like GCC

2022-01-13 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2022-01-13T15:49:19-08:00
New Revision: e289561205e844ab403c1c9e6814b2e44fb8d415

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

LOG: [Driver][Fuchsia] -r: imply -nostdlib like GCC

Similar to D116843.

Reviewed By: phosek

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 0dbe979668018..bd1600d060c87 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -109,7 +109,8 @@ void fuchsia::Linker::ConstructJob(Compilation , const 
JobAction ,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("Scrt1.o")));
 }
@@ -131,7 +132,8 @@ void fuchsia::Linker::ConstructJob(Compilation , const 
JobAction ,
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_static))
   CmdArgs.push_back("-Bdynamic");
 

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index ecf811d9401ba..fdf7f4ef3f11b 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -72,6 +72,8 @@
 // CHECK-RELOCATABLE-NOT: "-pie"
 // CHECK-RELOCATABLE-NOT: "--build-id"
 // CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l
+// CHECK-RELOCATABLE-NOT: crt{{[^.]+}}.o
 
 // RUN: %clang %s -### --target=x86_64-unknown-fuchsia -nodefaultlibs 
-fuse-ld=lld 2>&1 \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \



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

[PATCH] D116844: [Driver][Fuchsia] -r: imply -nostdlib like GCC

2022-01-13 Thread Petr Hosek via Phabricator via cfe-commits

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

Thank you! I was going to send the same change but you beat me to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116844

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

[PATCH] D116174: [clang] support relative roots to vfs overlays

2022-01-13 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

Each VFS may have its own working directory, so it could be surprising if we 
use the OS working directory instead of that.  This is complicated by the fact 
the VFS working directory may not be set yet during parsing the yaml (I haven't 
checked).  I'm not really sure what to recommend here. If we do change this, we 
should document the new behaviour in the doc comment for RedirectingFileSystem 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116174

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

[PATCH] D116844: [Driver][Fuchsia] -r: imply -nostdlib like GCC

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

This is a similar patch:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116844

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

[PATCH] D116174: [clang] support relative roots to vfs overlays

2022-01-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added subscribers: vsapsai, benlangmuir.
dexonsmith added a comment.

In D116174#3206895 , @keith wrote:

> This LGTM but I'm interested to hear from folks with more knowledge about the 
> VFS' desired behavior to comment on if this will have any unexpected side 
> effects

I haven't reviewed the patch, but looking at the description I don't have 
concerns with this.

@benlangmuir or @vsapsai, any thoughts from you?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116174

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D97446#3241828 , @JonChesterfield 
wrote:

> I'd much refer we put variables in arrays corresponding to their intended 
> lifespan instead of the binary format they're destined for.

I agree that LLVM features should generally mean something consistent across 
targets, and frontends should use the appropriate feature for the semantics 
they require.  Here the problem is that, for better or worse, the source 
language has different semantics on different targets.  Historically, that 
really has been driven by object format and the corresponding presumed linker 
behavior; I don't know where to begin answering the question of whether we 
should treat it as a platform difference instead when targeting a non-standard 
object format.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2093
+ "Only globals with definition can force usage.");
+  if (getTriple().isOSBinFormatELF())
+LLVMCompilerUsed.emplace_back(GV);

JonChesterfield wrote:
> ^ this specifically looks wrong, which array the variable goes in should be 
> based on what the variable is used for or what the programmer asked for, not 
> on the binary format used by the OS (is that even a unique test? One can run 
> elf or coff on windows iirc)
`addUsedOrCompilerUsedGlobal` is carefully used on `__attribute__((used))` and 
the related keep-static-consts.cpp.

`__attribute__((used))`  has a semantic split on ELF and non-ELF, so the 
difference is unavoidable. For other constructs we try to use the precise 
LLVMCompilerUsed or LLVMUsed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

2022-01-13 Thread Tommaso Bonvicini via Phabricator via cfe-commits

MuAlphaOmegaEpsilon updated this revision to Diff 399793.
MuAlphaOmegaEpsilon added a comment.

Update code after review and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-parameters.c
  clang/test/SemaCXX/warn-unused-parameters.cpp


Index: clang/test/SemaCXX/warn-unused-parameters.cpp
===
--- clang/test/SemaCXX/warn-unused-parameters.cpp
+++ clang/test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,7 @@
   auto l = []() { return sizeof...(s); };
   return l();
 }
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char* c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/test/Sema/warn-unused-parameters.c
===
--- clang/test/Sema/warn-unused-parameters.c
+++ clang/test/Sema/warn-unused-parameters.c
@@ -28,3 +28,5 @@
 // CHECK-everything-error: 5 errors generated
 // CHECK-everything-no-unused: 5 warnings generated
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,10 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FD->hasAttr())
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -286,3 +286,7 @@
   f([](int I) { return; });
 }
 } // namespace lambda
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
@@ -15,3 +15,5 @@
 // ===
 void h(i, c, d) int i; char *c, *d; {} // Don't mess with K style
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
@@ -22,4 +22,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused
 // CHECK-FIXES: {{^}}  F(int  /*j*/) : i() {}{{$}}
 };
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
 }
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -31,10 +31,11 @@
 } // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl()))
-  .bind("function"),
-  this);
+  Finder->addMatcher(functionDecl(isDefinition(), hasBody(stmt()),
+  hasAnyParameter(decl()),
+  unless(hasAttr(attr::Kind::Naked)))
+ .bind("function"),
+ this);
 }
 
 template 


Index: clang/test/SemaCXX/warn-unused-parameters.cpp

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

In D97446#3241735 , @MaskRay wrote:

> For your comment it appears an issue in the internalisation pass. It is 
> orthogonal to this patch.
> Do you have a reduced example with reproduce instructions for the issue? I 
> know very little about OpenMP.
> Well, I assume we can continuation the discussion in that OpenMP thread.

Internalize.cpp is fairly clear that it treats the two arrays differently, 
copying the corresponding part. Perhaps orthogonal to but exposed by this patch.

  // We must assume that globals in llvm.used have a reference that not even
  // the linker can see, so we don't internalize them.
  // For llvm.compiler.used the situation is a bit fuzzy. The assembler and
  // linker can drop those symbols. If this pass is running as part of LTO,
  // one might think that it could just drop llvm.compiler.used. The problem
  // is that even in LTO llvm doesn't see every reference. For example,
  // we don't see references from function local inline assembly. To be
  // conservative, we internalize symbols in llvm.compiler.used, but we
  // keep llvm.compiler.used so that the symbol is not deleted by llvm.
  for (GlobalValue *V : Used) {
AlwaysPreserved.insert(V->getName());
  }

>> It's possible that the check in internalize that skips over llvm.used and 
>> not llvm.compiler.used is itself a bug, and the intent is for llvm.used to 
>> be identical to llvm.compiler.used, but if that's the case we should delete 
>> the llvm.compiler.used array.
>
> ...
> I agree that in many cases user don't need more fine-grained GC precision and 
> probably just add both used/retain to not bother think about the problem.
> These people may find the split strange.
>
> llvm.compiler.used (the underlying mechanism of `__attribute__((used))`) is 
> useful in instrumentations.
> There are quite few cases that the compiler does not fully discard 
> definitions and has to defer it to the linker.
> I have changed some instrumentations (PGO/SanitizerCoverage/other sanitizers) 
> to downgrade llvm.used to llvm.compiler.used to improve section based linker 
> garbage collection for all of PE-COFF, Mach-O, and ELF.
> There has been decent object size decrease (at least single digit percent, 
> 10+% for some).

Uh, yes. Discarding things that previously were not discarded will make things 
smaller. The users I'm advocating for here are the ones who would have 
preferred we not discard the things that they asked us to keep and that we used 
to keep. Perhaps they will be few in number, and at least there's a workaround 
to be discovered.

In D97446#3241767 , @rjmccall wrote:

> I can understand how we got here, but it's a bad place to have ended up.
> ...elided...
> At the end of the day, I don't think we have much choice but to follow GCC's 
> lead on ELF platforms.  They get to define what these attributes mean, and if 
> they want to make weaker guarantees on ELF, that's their decision.

That's compelling, thank you for articulating it.

I think we have an outstanding issue to resolve in internalize, where the 
assumption behind this patch that the two forms are equivalent on elf does not 
hold.

I'd much refer we put variables in arrays corresponding to their intended 
lifespan instead of the binary format they're destined for. I don't know if the 
OS in question is certain to match the linker output either, seems possible to 
compile on an OS that usually uses one format but emit code in a different one.

Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2093
+ "Only globals with definition can force usage.");
+  if (getTriple().isOSBinFormatELF())
+LLVMCompilerUsed.emplace_back(GV);

^ this specifically looks wrong, which array the variable goes in should be 
based on what the variable is used for or what the programmer asked for, not on 
the binary format used by the OS (is that even a unique test? One can run elf 
or coff on windows iirc)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D116659: [llvm][clang][vfs] NFC: Simplify directory iteration

2022-01-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added a comment.

In D116659#3241727 , @benlangmuir 
wrote:

>> I don't know, I'm a bit skeptical we want to make it so easy to ignore 
>> errors so easily. I'd rather require clients to explicitly ignore the error.
>
> That's my gut feeling too. I suspect a bunch of this code should be 
> explicitly ignoring `ENOENT`, but not ignoring e.g. `EACCESS`, `EMFILE `, 
> etc.  It seems like a bug that we're ignoring all errors right now.
>
> If we really want to ignore all the iteration errors, we should do that 
> consistently in llvm::sys::fs not just the wrapper in the VFS layer.

If the main goal is to enable range-based-for, a simpler option than I 
suggested previously might be to have an iterator that dereferences to 
`ErrorOr`. Could be an iterator adaptor that takes any 
`increment(EC)`-style iterator (either `llvm::sys::fs` or `llvm::vfs`). Maybe 
`dir_range` could return `ErrorOr>` to expose the 
error-on-construction explicitly as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116659

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

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

2022-01-13 Thread Kevin Athey via Phabricator via cfe-commits

kda updated this revision to Diff 399784.
kda added a comment.
Herald added subscribers: luke957, s.egerton, simoncook.

Drop attribute changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116633

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize-memory-param-retval.c


Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- /dev/null
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -0,0 +1,12 @@
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// CHECK: "-fsanitize-memory-param-retval"
+
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory 
-fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fsanitize-memory-param-retval"
+
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory 
-fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -641,10 +641,14 @@
 Args.hasFlag(options::OPT_fsanitize_memory_use_after_dtor,
  options::OPT_fno_sanitize_memory_use_after_dtor,
  MsanUseAfterDtor);
+MsanParamRetval = Args.hasFlag(
+options::OPT_fsanitize_memory_param_retval,
+options::OPT_fno_sanitize_memory_param_retval, MsanParamRetval);
 NeedPIE |= !(TC.getTriple().isOSLinux() &&
  TC.getTriple().getArch() == llvm::Triple::x86_64);
   } else {
 MsanUseAfterDtor = false;
+MsanParamRetval = false;
   }
 
   if (AllAddedKinds & SanitizerKind::Thread) {
@@ -1096,6 +1100,9 @@
   if (MsanUseAfterDtor)
 CmdArgs.push_back("-fsanitize-memory-use-after-dtor");
 
+  if (MsanParamRetval)
+CmdArgs.push_back("-fsanitize-memory-param-retval");
+
   // FIXME: Pass these parameters as function attributes, not as -llvm flags.
   if (!TsanMemoryAccess) {
 CmdArgs.push_back("-mllvm");
Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -33,6 +33,7 @@
   int CoverageFeatures = 0;
   int MsanTrackOrigins = 0;
   bool MsanUseAfterDtor = true;
+  bool MsanParamRetval = false;
   bool CfiCrossDso = false;
   bool CfiICallGeneralizePointers = false;
   bool CfiCanonicalJumpTables = false;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1669,6 +1669,13 @@
   NormalizedValuesScope<"llvm::AsanDtorKind">,
   NormalizedValues<["None", "Global"]>,
   MarshallingInfoEnum, "Global">;
+defm sanitize_memory_param_retval
+  : BoolOption<"f", "sanitize-memory-param-retval",
+  CodeGenOpts<"SanitizeMemoryParamRetval">,
+  DefaultFalse,
+  PosFlag, NegFlag,
+  BothFlags<[], " detection of uninitialized parameters and return 
values">>,
+Group;
 // Note: This flag was introduced when it was necessary to distinguish between
 //   ABI for correct codegen.  This is no longer needed, but the flag is
 //   not removed since targeting either ABI will behave the same.
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -231,6 +231,9 @@
 ENUM_CODEGENOPT(SanitizeAddressDtor, llvm::AsanDtorKind, 2,
 llvm::AsanDtorKind::Global)  ///< Set how ASan global
  ///< destructors are emitted.
+CODEGENOPT(SanitizeMemoryParamRetval, 1, 0) ///< Enable detection of 
uninitialized
+///< parameters and return values
+///< in MemorySanitizer
 CODEGENOPT(SanitizeMemoryUseAfterDtor, 1, 0)

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

I can understand how we got here, but it's a bad place to have ended up.

Toolchains that do dead-stripping need a way to prevent it for specific objects 
and functions.  Windows and Darwin toolchains have historically done aggressive 
dead-stripping in both the compiler and linker, so they've always had this 
need.  ELF toolchains have historically emitted code and restricted the linker 
in ways that, together, largely inhibit link-time dead-stripping (except of 
specific things like C++ entities with vague linkage).  As a result, ELF has 
gotten away with not providing a fine-grained way to prevent link-time 
dead-stripping for a long time, when otherwise that would have been a major 
blocker.

There are basically three broad categories of reasons why programs need to use 
features like this:

1. Some system at runtime needs to be able to find the entity "reflectively" 
(generally, because it's in a section with a special name or will be looked up 
by its symbol name).  Usually this is intended as a kind of passive global 
registration, and so dead-stripping needs to be completely blocked for the 
entity.
2. Some system at runtime that finds the entity "reflectively" will access it 
in weird ways that the compiler can't hope to understand.  The attribute is not 
being used to prevent dead-stripping (unless the entity is *also* a passive 
registration), but to block compiler analysis and optimization if the value 
*does* end up being used.  This tends to be a language-implementation thing 
more than something that a library would do.
3. The entity isn't actually unreferenced, but some portion of the toolchain is 
just unable to see that.  The most important example of this is a reference 
from inline assembly.

If I were a GCC developer reacting to the addition of `retain` to ELF, I would 
have given `__attribute__((used))` the stronger semantics, forcing the entity 
to be retained at link-time, and I would have introduced a weaker attribute (or 
attributes) that people could use selectively in the second and third cases if 
they wanted better dead-stripping.  To me, that's conservatively living up to 
user expectations about what they're trying to achieve with 
`__attribute__((used))` while giving them an opportunity to be more precise if 
it's useful; and it also unifies the semantics across object formats now that 
it's possible to do so.  But that's not generally how GCC developers do things; 
they tend to treat the current compiler behavior as an inviolate contract down 
to a very low level, and if you want different behavior, you need to use 
different options.  I can understand and sympathize with that approach, even if 
I think it also tends to create situations like this one.

At the end of the day, I don't think we have much choice but to follow GCC's 
lead on ELF platforms.  They get to define what these attributes mean, and if 
they want to make weaker guarantees on ELF, that's their decision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D97446#3241617 , @JonChesterfield 
wrote:

>> Modulo optimizer bugs, __attribute__((used)) hasn't changed semantics.
>> If your downstream project does not handle llvm.compiler.used, maybe handle 
>> it now :)
>
> There seems to be some confusion here. The 'downstream project' is openmp, 
> which has worked around this regression in D117211 
>  and D117231 
> .
>
> Before this patch, `__attribute__((used))` mapped onto llvm.used, and the 
> variables so annotated made it all the way to the compiled artefact. After 
> this patch, it is mapped onto llvm.compiler.used, gets hit by an 
> internalisation pass and ends up in the compiled output but missing from the 
> symbol table. That is itself presumably a bug, as the linker should have 
> completely discarded it, with much the same effect.

For your comment it appears an issue in the internalisation pass. It is 
orthogonal to this patch.
Do you have a reduced example with reproduce instructions for the issue? I know 
very little about OpenMP.

> With this patch applied, what's the remaining use case for 
> `__attribute__((used))`? It can no longer be used to keep something in the 
> final executable, so it seems s/used/retain/g is the recommendation for 
> programs that previously used that attribute.
>
> It's possible that the check in internalize that skips over llvm.used and not 
> llvm.compiler.used is itself a bug, and the intent is for llvm.used to be 
> identical to llvm.compiler.used, but if that's the case we should delete the 
> llvm.compiler.used array.

A `__attribute__((used))` object can be referenced by a relocation. The 
relocation (from A to B) establishes a dependency relation in the linker: if 
the section defining A is retained, the section defining B should be retained 
as well.
This can be used by inline assembly (the compiler may not know the symbol 
references in assembly), and some code doing manual relocations.

llvm.compiler.used (the underlying mechanism of `__attribute__((used))`) is 
useful in instrumentations.
There are quite few cases that the compiler does not fully discard definitions 
and has to defer it to the linker.
I have changed some instrumentations (PGO/SanitizerCoverage/other sanitizers) 
to downgrade llvm.used to llvm.compiler.used to improve section based linker 
garbage collection for all of PE-COFF, Mach-O, and ELF.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D116659: [llvm][clang][vfs] NFC: Simplify directory iteration

2022-01-13 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

> I don't know, I'm a bit skeptical we want to make it so easy to ignore errors 
> so easily. I'd rather require clients to explicitly ignore the error.

That's my gut feeling too. I suspect a bunch of this code should be explicitly 
ignoring `ENOENT`, but not ignoring e.g. `EACCESS`, `EMFILE `, etc.  It seems 
like a bug that we're ignoring all errors right now.

If we really want to ignore all the iteration errors, we should do that 
consistently in llvm::sys::fs not just the wrapper in the VFS layer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116659

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

[PATCH] D116673: [Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 instructions

2022-01-13 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

In D116673#3237873 , @JackAKirk wrote:

>>> I can land the patch on your behalf. Are you OK to use the name/email in 
>>> this patch or do you prefer to use a different email for the LLVM commit?
>
> Thanks very much.  Yes the name/email in the patch is fine.

Note for the future. It would make it easier to land the patch if it was 
submitted to phabricator as a git patch, according to the instructions here: 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Review tracker would then include info about relevant base commit, author, 
commit message, etc. and it can be easily applied with `arc patch` to my tree. 
Using plain diff requires a bit of extra manual work to transfer all of that 
from the review tracker to the git commit.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116673

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

[PATCH] D116673: [Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 instructions

2022-01-13 Thread Artem Belevich via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbef3eb83442a: [Clang][NVPTX]Add NVPTX intrinsics and 
builtins for CUDA PTX cvt sm80… (authored by JackAKirk, committed by tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116673

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGen/builtins-nvptx.c
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTX.h
  llvm/lib/Target/NVPTX/NVPTXInstrInfo.td
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/test/CodeGen/NVPTX/convert-sm80.ll

Index: llvm/test/CodeGen/NVPTX/convert-sm80.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/convert-sm80.ll
@@ -0,0 +1,136 @@
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | FileCheck %s
+
+
+; CHECK-LABEL: cvt_rn_bf16x2_f32
+define i32 @cvt_rn_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.bf16x2.f32
+  %val = call i32 @llvm.nvvm.ff2bf16x2.rn(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_bf16x2_f32
+define i32 @cvt_rn_relu_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.relu.bf16x2.f32
+%val = call i32 @llvm.nvvm.ff2bf16x2.rn.relu(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rz_bf16x2_f32
+define i32 @cvt_rz_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.bf16x2.f32
+  %val = call i32 @llvm.nvvm.ff2bf16x2.rz(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_bf16x2_f32
+define i32 @cvt_rz_relu_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.relu.bf16x2.f32
+%val = call i32 @llvm.nvvm.ff2bf16x2.rz.relu(float %f1, float %f2);
+
+ret i32 %val
+}
+
+declare i32 @llvm.nvvm.ff2bf16x2.rn(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rn.relu(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rz(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rz.relu(float, float)
+
+; CHECK-LABEL: cvt_rn_f16x2_f32
+define <2 x half> @cvt_rn_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.f16x2.f32
+  %val = call <2 x half> @llvm.nvvm.ff2f16x2.rn(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_f16x2_f32
+define <2 x half> @cvt_rn_relu_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.relu.f16x2.f32
+%val = call <2 x half> @llvm.nvvm.ff2f16x2.rn.relu(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rz_f16x2_f32
+define <2 x half> @cvt_rz_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.f16x2.f32
+  %val = call <2 x half> @llvm.nvvm.ff2f16x2.rz(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_f16x2_f32
+define <2 x half> @cvt_rz_relu_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.relu.f16x2.f32
+%val = call <2 x half> @llvm.nvvm.ff2f16x2.rz.relu(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+declare <2 x half> @llvm.nvvm.ff2f16x2.rn(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rn.relu(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rz(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rz.relu(float, float)
+
+; CHECK-LABEL: cvt_rn_bf16_f32
+define i16 @cvt_rn_bf16_f32(float %f1) {
+
+; CHECK: cvt.rn.bf16.f32
+  %val = call i16 @llvm.nvvm.f2bf16.rn(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_bf16_f32
+define i16 @cvt_rn_relu_bf16_f32(float %f1) {
+
+; CHECK: cvt.rn.relu.bf16.f32
+%val = call i16 @llvm.nvvm.f2bf16.rn.relu(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rz_bf16_f32
+define i16 @cvt_rz_bf16_f32(float %f1) {
+
+; CHECK: cvt.rz.bf16.f32
+  %val = call i16 @llvm.nvvm.f2bf16.rz(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_bf16_f32
+define i16 @cvt_rz_relu_bf16_f32(float %f1) {
+
+; CHECK: cvt.rz.relu.bf16.f32
+%val = call i16 @llvm.nvvm.f2bf16.rz.relu(float %f1);
+
+ret i16 %val
+}
+
+declare i16 @llvm.nvvm.f2bf16.rn(float)
+declare i16 @llvm.nvvm.f2bf16.rn.relu(float)
+declare i16 @llvm.nvvm.f2bf16.rz(float)
+declare i16 @llvm.nvvm.f2bf16.rz.relu(float)
+
+; CHECK-LABEL: cvt_rna_tf32_f32
+define i32 @cvt_rna_tf32_f32(float %f1) {
+
+; CHECK: cvt.rna.tf32.f32
+  %val = call i32 @llvm.nvvm.f2tf32.rna(float %f1);
+
+ret i32 %val
+}
+
+declare i32 @llvm.nvvm.f2tf32.rna(float)
Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1046,6 +1046,38 @@
 def : Pat<(int_nvvm_ui2f_rp Int32Regs:$a),
   (CVT_f32_u32 Int32Regs:$a, CvtRP)>;
 
+def : Pat<(int_nvvm_ff2bf16x2_rn Float32Regs:$a, Float32Regs:$b),
+  (CVT_bf16x2_f32 Float32Regs:$a, Float32Regs:$b, CvtRN)>;
+def : Pat<(int_nvvm_ff2bf16x2_rn_relu Float32Regs:$a, Float32Regs:$b),
+  (CVT_bf16x2_f32 Float32Regs:$a, Float32Regs:$b, CvtRN_RELU)>;
+def : Pat<(int_nvvm_ff2bf16x2_rz

[clang] bef3eb8 - [Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 instructions

2022-01-13 Thread Artem Belevich via cfe-commits


Author: Jack Kirk
Date: 2022-01-13T13:29:48-08:00
New Revision: bef3eb83442a2f30c761a03793bd56c961f49cdd

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

LOG: [Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 
instructions

Adds NVPTX intrinsics and builtins for CUDA PTX cvt instructions for sm80
architectures and above. Requires ptx 7.0.

PTX ISA description of cvt instructions :
https://docs.nvidia.com/cuda/parallel-thread-execution/index.html#data-movement-and-conversion-instructions-cvt

Signed-off-by: JackAKirk 

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

Added: 
llvm/test/CodeGen/NVPTX/convert-sm80.ll

Modified: 
clang/include/clang/Basic/BuiltinsNVPTX.def
clang/test/CodeGen/builtins-nvptx.c
llvm/include/llvm/IR/IntrinsicsNVVM.td
llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
llvm/lib/Target/NVPTX/NVPTX.h
llvm/lib/Target/NVPTX/NVPTXInstrInfo.td
llvm/lib/Target/NVPTX/NVPTXIntrinsics.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsNVPTX.def 
b/clang/include/clang/Basic/BuiltinsNVPTX.def
index 025fef05c8e0..6b94dd857300 100644
--- a/clang/include/clang/Basic/BuiltinsNVPTX.def
+++ b/clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -402,6 +402,23 @@ BUILTIN(__nvvm_ull2d_rp, "dULLi", "")
 BUILTIN(__nvvm_f2h_rn_ftz, "Usf", "")
 BUILTIN(__nvvm_f2h_rn, "Usf", "")
 
+TARGET_BUILTIN(__nvvm_ff2bf16x2_rn, "ZUiff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2bf16x2_rn_relu, "ZUiff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2bf16x2_rz, "ZUiff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2bf16x2_rz_relu, "ZUiff", "", AND(SM_80,PTX70))
+
+TARGET_BUILTIN(__nvvm_ff2f16x2_rn, "V2hff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2f16x2_rn_relu, "V2hff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2f16x2_rz, "V2hff", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_ff2f16x2_rz_relu, "V2hff", "", AND(SM_80,PTX70))
+
+TARGET_BUILTIN(__nvvm_f2bf16_rn, "ZUsf", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_f2bf16_rn_relu, "ZUsf", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_f2bf16_rz, "ZUsf", "", AND(SM_80,PTX70))
+TARGET_BUILTIN(__nvvm_f2bf16_rz_relu, "ZUsf", "", AND(SM_80,PTX70))
+
+TARGET_BUILTIN(__nvvm_f2tf32_rna, "ZUif", "", AND(SM_80,PTX70))
+
 // Bitcast
 
 BUILTIN(__nvvm_bitcast_f2i, "if", "")

diff  --git a/clang/test/CodeGen/builtins-nvptx.c 
b/clang/test/CodeGen/builtins-nvptx.c
index ec0f74291ad4..1e31aaa70988 100644
--- a/clang/test/CodeGen/builtins-nvptx.c
+++ b/clang/test/CodeGen/builtins-nvptx.c
@@ -754,4 +754,40 @@ __device__ void 
nvvm_async_copy(__attribute__((address_space(3))) void* dst, __a
   __nvvm_cp_async_wait_all();
   #endif
   // CHECK: ret void
-}
\ No newline at end of file
+}
+
+// CHECK-LABEL: nvvm_cvt_sm80
+__device__ void nvvm_cvt_sm80() {
+#if __CUDA_ARCH__ >= 800
+  // CHECK_PTX70_SM80: call i32 @llvm.nvvm.ff2bf16x2.rn(float 1.00e+00, 
float 1.00e+00)
+  __nvvm_ff2bf16x2_rn(1, 1);
+  // CHECK_PTX70_SM80: call i32 @llvm.nvvm.ff2bf16x2.rn.relu(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2bf16x2_rn_relu(1, 1);
+  // CHECK_PTX70_SM80: call i32 @llvm.nvvm.ff2bf16x2.rz(float 1.00e+00, 
float 1.00e+00)
+  __nvvm_ff2bf16x2_rz(1, 1);
+  // CHECK_PTX70_SM80: call i32 @llvm.nvvm.ff2bf16x2.rz.relu(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2bf16x2_rz_relu(1, 1);
+
+  // CHECK_PTX70_SM80: call <2 x half> @llvm.nvvm.ff2f16x2.rn(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2f16x2_rn(1, 1);
+  // CHECK_PTX70_SM80: call <2 x half> @llvm.nvvm.ff2f16x2.rn.relu(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2f16x2_rn_relu(1, 1);
+  // CHECK_PTX70_SM80: call <2 x half> @llvm.nvvm.ff2f16x2.rz(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2f16x2_rz(1, 1);
+  // CHECK_PTX70_SM80: call <2 x half> @llvm.nvvm.ff2f16x2.rz.relu(float 
1.00e+00, float 1.00e+00)
+  __nvvm_ff2f16x2_rz_relu(1, 1);
+
+  // CHECK_PTX70_SM80: call i16 @llvm.nvvm.f2bf16.rn(float 1.00e+00)
+  __nvvm_f2bf16_rn(1);
+  // CHECK_PTX70_SM80: call i16 @llvm.nvvm.f2bf16.rn.relu(float 1.00e+00)
+  __nvvm_f2bf16_rn_relu(1);
+  // CHECK_PTX70_SM80: call i16 @llvm.nvvm.f2bf16.rz(float 1.00e+00)
+  __nvvm_f2bf16_rz(1);
+  // CHECK_PTX70_SM80: call i16 @llvm.nvvm.f2bf16.rz.relu(float 1.00e+00)
+  __nvvm_f2bf16_rz_relu(1);
+
+  // CHECK_PTX70_SM80: call i32 @llvm.nvvm.f2tf32.rna(float 1.00e+00)
+  __nvvm_f2tf32_rna(1);
+#endif
+  // CHECK: ret void
+}

diff  --git a/llvm/include/llvm/IR/IntrinsicsNVVM.td 
b/llvm/include/llvm/IR/IntrinsicsNVVM.td
index 6f55d1ef730e..41b28db56c75 100644
--- a/llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ b/llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -1185,6 +1185,36 @@ let TargetPrefix = "nvvm" in {
   def int_nvvm_f2h_rn

[PATCH] D117107: [clangd] Elide even more checks in SelectionTree.

2022-01-13 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG07f9fb8b5141: [clangd] Elide even more checks in 
SelectionTree. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D117107?vs=399643=399767#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117107

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -201,6 +201,13 @@
   )cpp",
   nullptr,
   },
+  {
+  R"cpp(
+#define TARGET void foo()
+[[TAR^GET{ return; }]]
+  )cpp",
+  "FunctionDecl",
+  },
   {
   R"cpp(
 struct S { S(const char*); };
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -274,22 +274,37 @@
 for (unsigned I = 0; I < Sel.size(); ++I) {
   if (shouldIgnore(Sel[I]) || PPIgnored[I])
 continue;
-  SpelledTokens.emplace_back();
-  Tok  = SpelledTokens.back();
+  SelectedSpelled.emplace_back();
+  Tok  = SelectedSpelled.back();
   S.Offset = SM.getFileOffset(Sel[I].location());
   if (S.Offset >= SelBegin && S.Offset + Sel[I].length() <= SelEnd)
 S.Selected = SelectionTree::Complete;
   else
 S.Selected = SelectionTree::Partial;
 }
+MaybeSelectedExpanded = computeMaybeSelectedExpandedTokens(Buf);
   }
 
   // Test whether a consecutive range of tokens is selected.
   // The tokens are taken from the expanded token stream.
   SelectionTree::Selection
   test(llvm::ArrayRef ExpandedTokens) const {
-if (SpelledTokens.empty())
+if (ExpandedTokens.empty())
   return NoTokens;
+if (SelectedSpelled.empty())
+  return SelectionTree::Unselected;
+// Cheap (pointer) check whether any of the tokens could touch selection.
+// In most cases, the node's overall source range touches ExpandedTokens,
+// or we would have failed mayHit(). However now we're only considering
+// the *unclaimed* spans of expanded tokens.
+// This is a significant performance improvement when a lot of nodes
+// surround the selection, including when generated by macros.
+if (MaybeSelectedExpanded.empty() ||
+() > () ||
+() < ()) {
+  return SelectionTree::Unselected;
+}
+
 SelectionTree::Selection Result = NoTokens;
 while (!ExpandedTokens.empty()) {
   // Take consecutive tokens from the same context together for efficiency.
@@ -312,14 +327,14 @@
   // If it returns false, test() will return NoTokens or Unselected.
   // If it returns true, test() may return any value.
   bool mayHit(SourceRange R) const {
-if (SpelledTokens.empty())
+if (SelectedSpelled.empty() || MaybeSelectedExpanded.empty())
   return false;
 // If the node starts after the selection ends, it is not selected.
 // Tokens a macro location might claim are >= its expansion start.
 // So if the expansion start > last selected token, we can prune it.
 // (This is particularly helpful for GTest's TEST macro).
 if (auto B = offsetInSelFile(getExpansionStart(R.getBegin(
-  if (*B > SpelledTokens.back().Offset)
+  if (*B > SelectedSpelled.back().Offset)
 return false;
 // If the node ends before the selection begins, it is not selected.
 SourceLocation EndLoc = R.getEnd();
@@ -328,12 +343,72 @@
 // In the rare case that the expansion range is a char range, EndLoc is
 // ~one token too far to the right. We may fail to prune, that's OK.
 if (auto E = offsetInSelFile(EndLoc))
-  if (*E < SpelledTokens.front().Offset)
+  if (*E < SelectedSpelled.front().Offset)
 return false;
 return true;
   }
 
 private:
+  // Plausible expanded tokens that might be affected by the selection.
+  // This is an overestimate, it may contain tokens that are not selected.
+  // The point is to allow cheap pruning in test()
+  llvm::ArrayRef
+  computeMaybeSelectedExpandedTokens(const syntax::TokenBuffer ) {
+if (SelectedSpelled.empty())
+  return {};
+
+bool StartInvalid = false;
+const syntax::Token *Start = llvm::partition_point(
+Toks.expandedTokens(),
+[&, First = SelectedSpelled.front().Offset](const syntax::Token ) {
+  if (Tok.kind() == tok::eof)
+return false;
+  // Implausible if upperbound(Tok) < First.
+  SourceLocation Loc = Tok.location();
+  auto Offset = offsetInSelFile(Loc);
+

[clang-tools-extra] 07f9fb8 - [clangd] Elide even more checks in SelectionTree.

2022-01-13 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-01-13T22:27:51+01:00
New Revision: 07f9fb8b51417ec3e6f46508e1b5ef78287b32ad

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

LOG: [clangd] Elide even more checks in SelectionTree.

During pop() we convert nodes into spans of expanded syntax::Tokens.
If we precompute a range of plausible (expanded) tokens, then we can do an
extremely cheap approximate hit-test against it, because syntax::Tokens are
ordered by pointer.

This would seem not to buy anything (we don't enter nodes unless they overlap
the selection), but in fact the spans we have are for *newly* claimed ranges
(i.e. those unclaimed by any child node).

So if you have:
   { { [[2+2]]; } }
then all of the CompoundStmts pass the hit test and are pushed, but we skip
full hit-testing of the brackets during pop() as they lie outside the range.

This is ~10x average speedup for selectiontree on a bad case I've seen
(large gtest file).

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 011e24ee70b2..8fb5918c5d00 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -274,22 +274,37 @@ class SelectionTester {
 for (unsigned I = 0; I < Sel.size(); ++I) {
   if (shouldIgnore(Sel[I]) || PPIgnored[I])
 continue;
-  SpelledTokens.emplace_back();
-  Tok  = SpelledTokens.back();
+  SelectedSpelled.emplace_back();
+  Tok  = SelectedSpelled.back();
   S.Offset = SM.getFileOffset(Sel[I].location());
   if (S.Offset >= SelBegin && S.Offset + Sel[I].length() <= SelEnd)
 S.Selected = SelectionTree::Complete;
   else
 S.Selected = SelectionTree::Partial;
 }
+MaybeSelectedExpanded = computeMaybeSelectedExpandedTokens(Buf);
   }
 
   // Test whether a consecutive range of tokens is selected.
   // The tokens are taken from the expanded token stream.
   SelectionTree::Selection
   test(llvm::ArrayRef ExpandedTokens) const {
-if (SpelledTokens.empty())
+if (ExpandedTokens.empty())
   return NoTokens;
+if (SelectedSpelled.empty())
+  return SelectionTree::Unselected;
+// Cheap (pointer) check whether any of the tokens could touch selection.
+// In most cases, the node's overall source range touches ExpandedTokens,
+// or we would have failed mayHit(). However now we're only considering
+// the *unclaimed* spans of expanded tokens.
+// This is a significant performance improvement when a lot of nodes
+// surround the selection, including when generated by macros.
+if (MaybeSelectedExpanded.empty() ||
+() > () ||
+() < ()) {
+  return SelectionTree::Unselected;
+}
+
 SelectionTree::Selection Result = NoTokens;
 while (!ExpandedTokens.empty()) {
   // Take consecutive tokens from the same context together for efficiency.
@@ -312,14 +327,14 @@ class SelectionTester {
   // If it returns false, test() will return NoTokens or Unselected.
   // If it returns true, test() may return any value.
   bool mayHit(SourceRange R) const {
-if (SpelledTokens.empty())
+if (SelectedSpelled.empty() || MaybeSelectedExpanded.empty())
   return false;
 // If the node starts after the selection ends, it is not selected.
 // Tokens a macro location might claim are >= its expansion start.
 // So if the expansion start > last selected token, we can prune it.
 // (This is particularly helpful for GTest's TEST macro).
 if (auto B = offsetInSelFile(getExpansionStart(R.getBegin(
-  if (*B > SpelledTokens.back().Offset)
+  if (*B > SelectedSpelled.back().Offset)
 return false;
 // If the node ends before the selection begins, it is not selected.
 SourceLocation EndLoc = R.getEnd();
@@ -328,12 +343,72 @@ class SelectionTester {
 // In the rare case that the expansion range is a char range, EndLoc is
 // ~one token too far to the right. We may fail to prune, that's OK.
 if (auto E = offsetInSelFile(EndLoc))
-  if (*E < SpelledTokens.front().Offset)
+  if (*E < SelectedSpelled.front().Offset)
 return false;
 return true;
   }
 
 private:
+  // Plausible expanded tokens that might be affected by the selection.
+  // This is an overestimate, it may contain tokens that are not selected.
+  // The point is to allow cheap pruning in test()
+  llvm::ArrayRef
+  computeMaybeSelectedExpandedTokens(const syntax::TokenBuffer ) {
+if (SelectedSpelled.empty())
+  return {};
+
+bool StartInvalid = false;
+const syntax::Token

[PATCH] D116925: [clangd] Suppress warning about system_header pragma when editing headers

2022-01-13 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG004acbb47d61: [clangd] Suppress warning about system_header 
pragma when editing headers (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116925

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -771,6 +771,17 @@
 Not(WithFix(_);
 }
 
+TEST(DiagnosticsTest, PragmaSystemHeader) {
+  Annotations Test("#pragma clang [[system_header]]\n");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "#pragma system_header ignored in main file";
+  TU.Filename = "TestTU.h";
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(ClangdTest, MSAsm) {
   // Parsing MS assembly tries to use the target MCAsmInfo, which we don't link.
   // We used to crash here. Now clang emits a diagnostic, which we filter out.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -277,19 +277,20 @@
"unreachable-code", "unused-variable",
"typecheck_bool_condition",
"unexpected_friend", "warn_alloca"));
-  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable,
-Conf.Diagnostics.Suppress));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+  diag::warn_unreachable, Conf.Diagnostics.Suppress, LangOptions()));
   // Subcategory not respected/suppressed.
-  EXPECT_FALSE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable_break,
- Conf.Diagnostics.Suppress));
-  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unused_variable,
-Conf.Diagnostics.Suppress));
+  EXPECT_FALSE(isBuiltinDiagnosticSuppressed(
+  diag::warn_unreachable_break, Conf.Diagnostics.Suppress, LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+  diag::warn_unused_variable, Conf.Diagnostics.Suppress, LangOptions()));
   EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_typecheck_bool_condition,
-Conf.Diagnostics.Suppress));
-  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_unexpected_friend,
-Conf.Diagnostics.Suppress));
-  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_alloca,
-Conf.Diagnostics.Suppress));
+Conf.Diagnostics.Suppress,
+LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+  diag::err_unexpected_friend, Conf.Diagnostics.Suppress, LangOptions()));
+  EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
+  diag::warn_alloca, Conf.Diagnostics.Suppress, LangOptions()));
 
   Frag.Diagnostics.Suppress.emplace_back("*");
   EXPECT_TRUE(compileAndApply());
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -350,7 +350,8 @@
   PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic ) {
 if (Cfg.Diagnostics.SuppressAll ||
-isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress))
+isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
+  *CI.getLangOpts()))
   return DiagnosticsEngine::Ignored;
 switch (Info.getID()) {
 case diag::warn_no_newline_eof:
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -445,7 +445,8 @@
 ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
   const clang::Diagnostic ) {
   if (Cfg.Diagnostics.SuppressAll ||
-  isBuiltinDiagnosticSuppressed(Info.getID(),

[clang-tools-extra] 004acbb - [clangd] Suppress warning about system_header pragma when editing headers

2022-01-13 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-01-13T22:24:05+01:00
New Revision: 004acbb47d61ffcc52da0c85ef6e6747a7cc7c14

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

LOG: [clangd] Suppress warning about system_header pragma when editing headers

Not sure it's OK to suppress this in clang itself - if we're building a PCH
or module, maybe it matters?

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index f3482c14184f..044ef1934426 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -879,7 +879,14 @@ void StoreDiags::flushLastDiag() {
 }
 
 bool isBuiltinDiagnosticSuppressed(unsigned ID,
-   const llvm::StringSet<> ) {
+   const llvm::StringSet<> ,
+   const LangOptions ) {
+  // Don't complain about header-only stuff in mainfiles if it's a header.
+  // FIXME: would be cleaner to suppress in clang, once we decide whether the
+  //behavior should be to silently-ignore or respect the pragma.
+  if (ID == diag::pp_pragma_sysheader_in_main_file && LangOpts.IsHeaderFile)
+return true;
+
   if (const char *CodePtr = getDiagnosticCode(ID)) {
 if (Suppress.contains(normalizeSuppressedCode(CodePtr)))
   return true;

diff  --git a/clang-tools-extra/clangd/Diagnostics.h 
b/clang-tools-extra/clangd/Diagnostics.h
index 99de0e1a611e..718a5583a36d 100644
--- a/clang-tools-extra/clangd/Diagnostics.h
+++ b/clang-tools-extra/clangd/Diagnostics.h
@@ -180,7 +180,8 @@ class StoreDiags : public DiagnosticConsumer {
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
 bool isBuiltinDiagnosticSuppressed(unsigned ID,
-   const llvm::StringSet<> );
+   const llvm::StringSet<> ,
+   const LangOptions &);
 /// Take a user-specified diagnostic code, and convert it to a normalized form
 /// stored in the config and consumed by isBuiltinDiagnosticsSuppressed.
 ///

diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 42552e9831a0..9c64c645130b 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -445,7 +445,8 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs ,
 ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
   const clang::Diagnostic ) {
   if (Cfg.Diagnostics.SuppressAll ||
-  isBuiltinDiagnosticSuppressed(Info.getID(), 
Cfg.Diagnostics.Suppress))
+  isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
+Clang->getLangOpts()))
 return DiagnosticsEngine::Ignored;
 
   auto It = OverriddenSeverity.find(Info.getID());

diff  --git a/clang-tools-extra/clangd/Preamble.cpp 
b/clang-tools-extra/clangd/Preamble.cpp
index 34f3caa5e34b..f3328df3f51f 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -350,7 +350,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
   PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic ) {
 if (Cfg.Diagnostics.SuppressAll ||
-isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress))
+isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
+  *CI.getLangOpts()))
   return DiagnosticsEngine::Ignored;
 switch (Info.getID()) {
 case diag::warn_no_newline_eof:

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 661784256af8..53406c09e92d 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -277,19 +277,20 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
"unreachable-code", "unused-variable",
"typecheck_bool_condition",
"unexpected_friend", "warn_alloca"));
-

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

2022-01-13 Thread Lambert Clara via Phabricator via cfe-commits

belkiss marked an inline comment as done.
belkiss added a comment.

And thanks @aganea, @hans & @rnk!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116861

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

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

2022-01-13 Thread Lambert Clara via Phabricator via cfe-commits

belkiss updated this revision to Diff 399764.
belkiss added a comment.

Simplify test, remove the inline specific CHECK-NOT since we have the 
implicit-check-not in FileCheck args checking that no errors have occured in 
the whole file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116861

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp


Index: compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx -fsanitize=alignment %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not="runtime error" 
-allow-empty
+// Disable with msan and tsan because they also override global new
+// UNSUPPORTED: ubsan-msan, ubsan-tsan
+
+#include 
+#include 
+#include 
+
+void *operator new(std::size_t count) {
+  constexpr const size_t offset = 8;
+
+  // allocate a bit more so we can safely offset it
+  void *ptr = std::malloc(count + offset);
+
+  // verify malloc returned 16 bytes aligned mem
+  static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ == 16,
+"Global new doesn't return 16 bytes aligned memory!");
+  assert((reinterpret_cast(ptr) &
+  (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
+
+  return static_cast(ptr) + offset;
+}
+
+struct Param {
+  void *_cookie1;
+  void *_cookie2;
+};
+
+static_assert(alignof(Param) == 8, "Param struct alignment must be 8 bytes!");
+
+int main() { Param *p = new Param; }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -1731,8 +1731,8 @@
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-result.getPointer(), allocType, result.getAlignment(),
-SkippedChecks, numElements);
+result.getPointer(), allocType, allocAlign, SkippedChecks,
+numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
  allocSizeWithoutCookie);


Index: compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp
@@ -0,0 +1,32 @@
+// RUN: %clangxx -fsanitize=alignment %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not="runtime error" -allow-empty
+// Disable with msan and tsan because they also override global new
+// UNSUPPORTED: ubsan-msan, ubsan-tsan
+
+#include 
+#include 
+#include 
+
+void *operator new(std::size_t count) {
+  constexpr const size_t offset = 8;
+
+  // allocate a bit more so we can safely offset it
+  void *ptr = std::malloc(count + offset);
+
+  // verify malloc returned 16 bytes aligned mem
+  static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ == 16,
+"Global new doesn't return 16 bytes aligned memory!");
+  assert((reinterpret_cast(ptr) &
+  (__STDCPP_DEFAULT_NEW_ALIGNMENT__ - 1)) == 0);
+
+  return static_cast(ptr) + offset;
+}
+
+struct Param {
+  void *_cookie1;
+  void *_cookie2;
+};
+
+static_assert(alignof(Param) == 8, "Param struct alignment must be 8 bytes!");
+
+int main() { Param *p = new Param; }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -1731,8 +1731,8 @@
   SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
 E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-result.getPointer(), allocType, result.getAlignment(),
-SkippedChecks, numElements);
+result.getPointer(), allocType, allocAlign, SkippedChecks,
+numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
  allocSizeWithoutCookie);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116443: [clangd] Implement textDocument/typeDefinition

2022-01-13 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sammccall marked 4 inline comments as done.
Closed by commit rG71a082f72674: [clangd] Implement textDocument/typeDefinition 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D116443?vs=396785=399761#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116443

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/type-definition.test
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -38,10 +38,12 @@
 namespace {
 
 using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
+using ::testing::Not;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedElementsAreArray;
 using ::testing::UnorderedPointwise;
@@ -1781,6 +1783,69 @@
   << Test;
 }
 
+TEST(FindType, All) {
+  Annotations HeaderA(R"cpp(
+struct [[Target]] { operator int() const; };
+struct Aggregate { Target a, b; };
+Target t;
+
+template  class smart_ptr {
+  T& operator*();
+  T* operator->();
+  T* get();
+};
+  )cpp");
+  auto TU = TestTU::withHeaderCode(HeaderA.code());
+  for (const llvm::StringRef Case : {
+   "str^uct Target;",
+   "T^arget x;",
+   "Target ^x;",
+   "a^uto x = Target{};",
+   "namespace m { Target tgt; } auto x = m^::tgt;",
+   "Target funcCall(); auto x = ^funcCall();",
+   "Aggregate a = { {}, ^{} };",
+   "Aggregate a = { ^.a=t, };",
+   "struct X { Target a; X() : ^a() {} };",
+   "^using T = Target; ^T foo();",
+   "^template  Target foo();",
+   "void x() { try {} ^catch(Target e) {} }",
+   "void x() { ^throw t; }",
+   "int x() { ^return t; }",
+   "void x() { ^switch(t) {} }",
+   "void x() { ^delete (Target*)nullptr; }",
+   "Target& ^tref = t;",
+   "void x() { ^if (t) {} }",
+   "void x() { ^while (t) {} }",
+   "void x() { ^do { } while (t); }",
+   "^auto x = []() { return t; };",
+   "Target* ^tptr = ",
+   "Target ^tarray[3];",
+   }) {
+Annotations A(Case);
+TU.Code = A.code().str();
+ParsedAST AST = TU.build();
+
+ASSERT_GT(A.points().size(), 0u) << Case;
+for (auto Pos : A.points())
+  EXPECT_THAT(findType(AST, Pos),
+  ElementsAre(Sym("Target", HeaderA.range(), HeaderA.range(
+  << Case;
+  }
+
+  // FIXME: We'd like these cases to work. Fix them and move above.
+  for (const llvm::StringRef Case : {
+   "smart_ptr ^tsmart;",
+   }) {
+Annotations A(Case);
+TU.Code = A.code().str();
+ParsedAST AST = TU.build();
+
+EXPECT_THAT(findType(AST, A.point()),
+Not(Contains(Sym("Target", HeaderA.range(), HeaderA.range()
+<< Case;
+  }
+}
+
 void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) {
   Annotations T(Test);
   auto TU = TestTU::withCode(T.code());
Index: clang-tools-extra/clangd/test/type-definition.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/type-definition.test
@@ -0,0 +1,32 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,
+  "text":"class X {};\nauto x = X{};"
+}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/typeDefinition","params":{
+  "textDocument":{"uri":"test:///main.cpp"},
+  "position":{"line":1,"character":5}
+}}
+#  CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 7,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 6,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index:

[clang-tools-extra] 71a082f - [clangd] Implement textDocument/typeDefinition

2022-01-13 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-01-13T22:15:10+01:00
New Revision: 71a082f72674ceba0797748543dcef3620b22822

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

LOG: [clangd] Implement textDocument/typeDefinition

This reuses the type=>decl mapping from go-to-definition on auto.
(Which could stand some improvement, but that can happen later).

Fixes https://github.com/clangd/clangd/issues/367

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

Added: 
clang-tools-extra/clangd/test/type-definition.test

Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 47bde7d0edcfd..5b1d046872655 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -560,6 +560,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
,
   {"declarationProvider", true},
   {"definitionProvider", true},
   {"implementationProvider", true},
+  {"typeDefinitionProvider", true},
   {"documentHighlightProvider", true},
   {"documentLinkProvider",
llvm::json::Object{
@@ -1278,6 +1279,21 @@ void ClangdLSPServer::onReference(const ReferenceParams 
,
   });
 }
 
+void ClangdLSPServer::onGoToType(const TextDocumentPositionParams ,
+ Callback> Reply) {
+  Server->findType(
+  Params.textDocument.uri.file(), Params.position,
+  [Reply = std::move(Reply)](
+  llvm::Expected> Types) mutable {
+if (!Types)
+  return Reply(Types.takeError());
+std::vector Response;
+for (const LocatedSymbol  : *Types)
+  Response.push_back(Sym.PreferredDeclaration);
+return Reply(std::move(Response));
+  });
+}
+
 void ClangdLSPServer::onGoToImplementation(
 const TextDocumentPositionParams ,
 Callback> Reply) {
@@ -1448,6 +1464,7 @@ void ClangdLSPServer::bindMethods(LSPBinder ,
   Bind.method("textDocument/signatureHelp", this, 
::onSignatureHelp);
   Bind.method("textDocument/definition", this, 
::onGoToDefinition);
   Bind.method("textDocument/declaration", this, 
::onGoToDeclaration);
+  Bind.method("textDocument/typeDefinition", this, 
::onGoToType);
   Bind.method("textDocument/implementation", this, 
::onGoToImplementation);
   Bind.method("textDocument/references", this, ::onReference);
   Bind.method("textDocument/switchSourceHeader", this, 
::onSwitchSourceHeader);

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index d2b447e75dbad..02c2a5c721e1d 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -121,6 +121,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
  Callback>);
   void onGoToDefinition(const TextDocumentPositionParams &,
 Callback>);
+  void onGoToType(const TextDocumentPositionParams &,
+  Callback>);
   void onGoToImplementation(const TextDocumentPositionParams &,
 Callback>);
   void onReference(const ReferenceParams &, Callback>);

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 69f37662c4ab7..29d5d9dfe1d1d 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -810,6 +810,17 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
 Transient);
 }
 
+void ClangdServer::findType(llvm::StringRef File, Position Pos,
+Callback> CB) {
+  auto Action =
+  [Pos, CB = std::move(CB)](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::findType(InpAST->AST, Pos));
+  };
+  WorkScheduler->runWithAST("FindType", File, std::move(Action));
+}
+
 void ClangdServer::findImplementations(
 PathRef File, Position Pos, Callback> CB) {
   auto Action = [Pos, CB = std::move(CB),

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index 14be307e6fda8..4bef653cbf82d 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -282,6 +282,10 @@ class ClangdServer {
   void findImplementations(PathRef File, Position Pos,

[PATCH] D117246: [OpenMP] Add support for linking AMDGPU images

2022-01-13 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, gregrodgers, JonChesterfield, ronlieb.
Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, 
nhaehnle, jvesely, kzhuravl.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a project: clang.

This patch adds support for linking AMDGPU images using the LLD binary.
AMDGPU files are always bitcode images and will always use the LTO
backend. Additionally we now pass the default architecture found with
the `amdgpu-arch` tool to the argument list.

Depends on D117156 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117246

Files:
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -512,7 +512,7 @@
   // Create a new file to write the linked device image to.
   SmallString<128> TempFile;
   if (std::error_code EC = sys::fs::createTemporaryFile(
-  TheTriple.getArchName() + "-" + Arch, "cubin", TempFile))
+  "lto-" + TheTriple.getArchName() + "-" + Arch, "cubin", TempFile))
 return createFileError(TempFile, EC);
   TempFiles.push_back(static_cast(TempFile));
 
@@ -590,6 +590,50 @@
   return static_cast(TempFile);
 }
 } // namespace nvptx
+namespace amdgcn {
+Expected link(ArrayRef InputFiles,
+   ArrayRef LinkerArgs, Triple TheTriple,
+   StringRef Arch) {
+  // AMDGPU uses the lld binary to link device object files.
+  ErrorOr LLDPath =
+  sys::findProgramByName("lld", sys::path::parent_path(LinkerExecutable));
+  if (!LLDPath)
+LLDPath = sys::findProgramByName("lld");
+  if (!LLDPath)
+return createStringError(LLDPath.getError(),
+ "Unable to find 'lld' in path");
+
+  // Create a new file to write the linked device image to.
+  SmallString<128> TempFile;
+  if (std::error_code EC = sys::fs::createTemporaryFile(
+  TheTriple.getArchName() + "-" + Arch + "-image", "out", TempFile))
+return createFileError(TempFile, EC);
+  TempFiles.push_back(static_cast(TempFile));
+
+  SmallVector CmdArgs;
+  CmdArgs.push_back(*LLDPath);
+  CmdArgs.push_back("-flavor");
+  CmdArgs.push_back("gnu");
+  CmdArgs.push_back("--no-undefined");
+  CmdArgs.push_back("-shared");
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(TempFile);
+
+  // Copy system library paths used by the host linker.
+  for (StringRef Arg : LinkerArgs)
+if (Arg.startswith("-L"))
+  CmdArgs.push_back(Arg);
+
+  // Add extracted input files.
+  for (StringRef Input : InputFiles)
+CmdArgs.push_back(Input);
+
+  if (sys::ExecuteAndWait(*LLDPath, CmdArgs))
+return createStringError(inconvertibleErrorCode(), "'lld' failed");
+
+  return static_cast(TempFile);
+}
+} // namespace amdgcn
 
 Expected linkDevice(ArrayRef InputFiles,
  ArrayRef LinkerArgs,
@@ -599,7 +643,7 @@
   case Triple::nvptx64:
 return nvptx::link(InputFiles, LinkerArgs, TheTriple, Arch);
   case Triple::amdgcn:
-// TODO: AMDGCN linking support.
+return amdgcn::link(InputFiles, LinkerArgs, TheTriple, Arch);
   case Triple::x86:
   case Triple::x86_64:
 // TODO: x86 linking support.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -289,10 +289,22 @@
 
   const OptTable  = getDriver().getOpts();
 
-  if (DeviceOffloadKind != Action::OFK_OpenMP) {
-for (Arg *A : Args) {
-  DAL->append(A);
+  if (DeviceOffloadKind == Action::OFK_OpenMP) {
+for (Arg *A : Args)
+  if (!llvm::is_contained(*DAL, A))
+DAL->append(A);
+
+std::string Arch = DAL->getLastArgValue(options::OPT_march_EQ).str();
+if (Arch.empty()) {
+  checkSystemForAMDGPU(Args, *this, Arch);
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), Arch);
 }
+
+return DAL;
+  }
+
+  for (Arg *A : Args) {
+DAL->append(A);
   }
 
   if (!BoundArch.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116443: [clangd] Implement textDocument/typeDefinition

2022-01-13 Thread Sam McCall via Phabricator via cfe-commits

sammccall marked 6 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1883
+  QualType VisitIfStmt(const IfStmt *S) { return type(S->getCond()); }
+  QualType VisitCaseStmt(const CaseStmt *S) { return type(S->getLHS()); }
+  QualType VisitCXXForRangeStmt(const CXXForRangeStmt *S) {

usaxena95 wrote:
> I think this would be useful for enumerations primarily. Would it work as of 
> now (would we return the enum definition loc) ?
Yes, it does work (tested manually).



Comment at: clang-tools-extra/clangd/XRefs.cpp:1925-1927
+// If we targeted something function-like, prefer its return type.
+if (auto FT = Type->getAs())
+  Type = FT->getReturnType();

usaxena95 wrote:
> Can this be accommodated in `typeForNode` ?
> It would be nicer if we keep this method only for plumbing.
Agree. It's not convenient to do it exactly in typeForNode though, as we have 
to wrap every return with the unwrapping logic.
I added `unwrapFindType` to do this, and that's a natural place to fix 
array/pointer cases too while here.



Comment at: clang-tools-extra/clangd/XRefs.h:110
+///
+/// For example, given `foo(b^ar())` where bar() returns unique_ptr,
+/// this should return the definition of class Foo.

usaxena95 wrote:
> This is sounds confusing to me about the current behaviour. 
> We would return `unique_ptr` here atm. Maybe just have simple `returns 
> Foo` here as of now.
Whoops, you're right. I'd love to have this behavior but decided not to bite it 
off in this patch.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1805
+   "namespace m { Target tgt; } auto x = m^::tgt;",
+   "Target funcCall(); auto x = ^funcCall();",
+   "Aggregate a = { {}, ^{} };",

usaxena95 wrote:
> Can you add a lambda as well
> `"au^to x = []() -> Target {return Target{};}"`
Done. This isn't handled yet (lambdas are CXXRecordType, not FunctionType) so 
added it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116443

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

[PATCH] D116659: [llvm][clang][vfs] NFC: Simplify directory iteration

2022-01-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added a subscriber: benlangmuir.
dexonsmith added a comment.

In D116659#3237061 , @jansvoboda11 
wrote:

> I agree that introducing new iterator and implementing `iterator_range<...> 
> FileSystem::dir_range()` is better solution than a macro.
>
> I'm not sure the `dir_range` function needs to take an `std::error_code` 
> out-param though. The error code is only used to stop the iteration, clients 
> don't use it for any other purpose. I think the new iterator could handle 
> error codes completely internally (by advancing to the end), providing better 
> ergonomics. WDYT?
>
> I might create a follow-up patches for //recursive// VFS-based iteration and 
> also enable range-based for loops in code using `llvm::sys::fs` instead of 
> the VFS.

I don't know, I'm a bit skeptical we want to make it so easy to ignore errors 
so easily. I'd rather require clients to explicitly ignore the error.

@benlangmuir, any thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116659

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

> Modulo optimizer bugs, __attribute__((used)) hasn't changed semantics.
> If your downstream project does not handle llvm.compiler.used, maybe handle 
> it now :)

There seems to be some confusion here. The 'downstream project' is openmp, 
which has worked around this regression in D117211 
 and D117231 
.

Before this patch, __attribute__((used)) mapped onto llvm.used, and the 
variables so annotated made it all the way to the compiled artefact. After this 
patch, it is mapped onto llvm.compiler.used, gets hit by an internalisation 
pass and ends up in the compiled output but missing from the symbol table. That 
is itself presumably a bug, as the linker should have completely discarded it, 
with much the same effect.

With this patch applied, what's the remaining use case for 
__attribute__((used))? It can no longer be used to keep something in the final 
executable, so it seems s/sed used/retain/g is the recommendation for programs 
that previously used that attribute.

It's possible that the check in internalize that skips over llvm.used and not 
llvm.compiler.used is itself a bug, and the intent is for llvm.used to be 
identical to llvm.compiler.used, but if that's the case we should delete the 
llvm.compiler.used array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D117119: [clangd][clang-tidy] Remove uses of `std::vector`

2022-01-13 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117119

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D97446#3241416 , @JonChesterfield 
wrote:

> It looks deeply wrong to be marking globals as either llvm.used or 
> llvm.compiler.used based on the output file format. It should be based on the 
> (purpose of) the global.
>
> In D97446#3241142 , @rnk wrote:
>
>> This change was implemented so that llvm.used could prevent section GC on 
>> ELF, to make its semantics consistent with llvm.used on COFF and MachO. 
>> Essentially, llvm.used behaves differently on ELF now so to prevent behavior 
>> changes for users of `__attribute__((used))`, it was migrated to 
>> `llvm.compiler.used` on ELF targets. This is consistent with GCC's behavior.
>
> Is this sentence inverted? llvm.used should prevent sections from being 
> discarded. If it doesn't at present, that's a bug in the linker.

I agree with rnk.
The bug can be on the compiler side which does not give enough information to 
the linker.
Before `retain` was added to GCC and binutils supported SHF_GNU_RETAIN, the ELF 
world was in that state.

> llvm.compiler.used should generally be discarded by whatever part of the 
> compiler wanted the variable

The compiler cannot discard it per LangRef 
https://llvm.org/docs/LangRef.html#the-llvm-compiler-used-global-variable

> but if it makes it to the linker, the linker should throw it away. Because it 
> was only used by the compiler. It's possible some users marked things as 
> 'used' but wanted them thrown away, but it seems more likely that users 
> weren't using gc-sections if it broke their application by throwing away 
> things they asked to keep.

Correct.

>> This should only change behavior for you if you depend on the details of the 
>> LLVM IR being emitted, or perhaps if you use LTO, where GlobalOpt will 
>> behave differently. I don't think these use cases are as important as 
>> consistent ELF section GC behavior.
>>
>> So, apologies for changing the LLVM IR emitted for this attribute, but I 
>> think it's unlikely we will change our minds and revert this a year later.
>
> I'm hopeful I can change your minds now. The current modelling in the 
> compiler doesn't match the stated intent.
>
> In D97446#3241195 , @MaskRay wrote:
>
>> llvm.compiler.used hasn't been changed.
>
> True, but attribute((used)) has. I only noticed this patch because it broke 
> some test cases in rocm, but it'll break some of my own code too. Moving a 
> variable from llvm.used to llvm.compiler.used changes whether internalise 
> skips the variable.

Modulo optimizer bugs, `__attribute__((used))` hasn't changed semantics.
If your downstream project does not handle llvm.compiler.used, maybe handle it 
now :)

I apologize that previous Clang probably very rarely emitted llvm.compiler.used 
and it started to do it more often now.

>> The text focuses on the semantics, and for practical reasons refers to the 
>> toolchain support.
>> Before GCC 11/binutils 2.36 there was just no portable way making a 
>> definition not discarded by ld --gc-sections.
>
> Sure there was. Don't pass gc-sections to the linker, or don't compile with 
> ffunction-sections to get a close approximation.
>
>>> edit: Would making attribute((used)) imply attribute((retain)) on elf 
>>> targets achieve the objective of this patch without breaking code that 
>>> expects 'used' to mean "don't throw this away"?
>>
>> This would make semantics less orthogonal and incompatible with GCC.
>> On COFF and Mach-O, there have been Clang-specific (not GCC) use cases 
>> relying on attribute((used)) implying GC roots.
>> On ELF, there was none before the toolchain support.
>>
>> Every llvm.used usage I can find in the wild does intend to have the GC 
>> semantics, and not having it on ELF was actually a bug and has been fixed by 
>> the patch series.
>
> I'm absolutely sure that people mark things as attribute((used)) to stop the 
> toolchain discarding them. I think we're in agreement there, but differ in 
> our assessment of popularity of gc-sections.
>
> Are we missing a category here?
>
> llvm.compiler.used <- the compiler uses the global, and may discard it. If it 
> doesn't, the linker should discard it
> llvm.linker.used <- the linker uses this global, and may discard it. The 
> compiler should leave it alone aside from passing it to the linker
> llvm.used <- some unspecified thing uses the global, the compiler and linker 
> should leave it alone aside from embedding it in the linked output

I think the llvm.compiler.used interpretation diverges from LangRef and how we 
teach optimizers to respect llvm.compiler.used.
Then llvm.linker.used shall not be needed.

> If we have to map 'attribute((used))' onto the new llvm.linker.used and 
> 'attribute((retain))' onto llvm.used that's a shame, but at least it keeps 
> the naming weirdness localised to the language front end.

I have checked the documentation of many GCC

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

2022-01-13 Thread Michael Kruse via Phabricator via cfe-commits

Meinersbur added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2587
+  // Check for unsupported clauses
+  if (S.clauses().size() > 0) {
+// Currently no clause is supported





Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2591
+  }
+  // Check if we have a statement with the ordered directive.
+  // Visit the statement hierarchy to find a compound statement




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

https://reviews.llvm.org/D114379

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

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

2022-01-13 Thread Michael Kruse via Phabricator via cfe-commits

Meinersbur added a comment.

[testing] Could you add a test for `applySimd` into `OpenMPIRBuilderTests.cpp`, 
so we have a test that is independent of Clang?




Comment at: clang/test/OpenMP/irbuilder_simd.cpp:3
+// expected-no-diagnostics 
+// RUN: %clang_cc1 -fopenmp-enable-irbuilder -verify -fopenmp 
-fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECKTWOLOOPS 
+// expected-no-diagnostics

The invocation is identical to the RUN on line 1. `CHECKTWOLOOPS` is not needed 
and can also be made as `CHECK`.



Comment at: clang/test/OpenMP/irbuilder_simd.cpp:4
+// RUN: %clang_cc1 -fopenmp-enable-irbuilder -verify -fopenmp 
-fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s -check-prefix=CHECKTWOLOOPS 
+// expected-no-diagnostics
+

A single `expected-no-diagnostics` is sufficient



Comment at: clang/test/OpenMP/irbuilder_simd.cpp:67-69
+// CHECK: ![[META0:[0-9]+]] = !{i32 1, !"wchar_size", i32 4}
+// CHECK-NEXT: ![[META1:[0-9]+]]  = !{i32 7, !"openmp", i32 45}
+// CHECK-NEXT: ![[META2:[0-9]+]]  =

If you added these CHECK lines by hand, remove those that are not relevant



Comment at: clang/test/OpenMP/irbuilder_simd.cpp:71
+// CHECK-NEXT: ![[META3:[0-9]+]] = distinct !{}
+// CHECK-NEXT: ![[META4:[0-9]+]]  = distinct !{![[META4:[0-9]+]], 
![[META5:[0-9]+]], ![[META6:[0-9]+]]}
+// CHECK-NEXT: ![[META5:[0-9]+]]  = !{!"llvm.loop.parallel_accesses", 
![[META3:[0-9]+]]}

Do not add a regex specification for any but the first occurance. A regex 
specification will "overwrite" the previous match, i.e. the different 
occurrences of `META4` would not need to be the same.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2120
 
+/// Attach metadata llvm.access.group to the memref instructions of \p block
+static void addSimdMetadata(BasicBlock *Block,





Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2124
+  for (Instruction  : *Block) {
+if (I.mayReadFromMemory() || I.mayWriteToMemory()) {
+  I.setMetadata(LLVMContext::MD_access_group, AccessGroup);





Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2125
+if (I.mayReadFromMemory() || I.mayWriteToMemory()) {
+  I.setMetadata(LLVMContext::MD_access_group, AccessGroup);
+}

The instruction may already have an access group assigned e.g. from a nested 
`#pragma clang loop vectorize(assume_safety)`. This would overwrite the access 
group. See `LoopInfoStack::InsertHelper` (`CGLoopInfo.cpp`) on how to assign 
multiple access groups, or add a TODO.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2156
+  DominatorTreeAnalysis DTA;
+  DominatorTree & = DTA.run(*F, FAM);
+  LoopAnalysis LIA;

Compiler warning: `warning C4189: 'DT': local variable is initialized but not 
referenced`



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2162
+
+  llvm::SmallSet Reachable; 
+

OpenMPIRBuilder is part of the `llvm` namespace, `llvm::` is not necessary.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2166
+  // can be found.
+  for (BasicBlock *Block:L->getBlocks()) {
+if (Block == CanonicalLoop->getCond() || Block == 
CanonicalLoop->getHeader()) continue;

Could you add a todo note such as: 
```
// TODO: Generalize getting all blocks inside a CanonicalizeLoopInfo, 
preferably without running any passes.
```



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2173-2175
+  for (BasicBlock *BB : Reachable) {
+addSimdMetadata(BB, AccessGroup);
+  }

[style]



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2181
+  ConstantInt::getTrue(Type::getInt1Ty(Ctx)));
+  addLoopMetadata(
+  CanonicalLoop,

The loop might already have a `llvm.loop.parallel_accesses`, in which case 
those two lists could be combined.

I don't think its very relevant, but maybe add a TODO?


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

https://reviews.llvm.org/D114379

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread Armen Khachkinaev via Phabricator via cfe-commits

futuarmo added a comment.

@curdeius, all tests pass on fresh build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D116328#3241486 , 
@LegalizeAdulthood wrote:

> In D116328#3241329 , @aaron.ballman 
> wrote:
>
>> In D116328#3223344 , 
>> @LegalizeAdulthood wrote:
>>
>>> My takeaway:
>>>
>>> - if `has` isn't expensive, I can either ditch this public matcher or move 
>>> it to be a private matcher used in my check
>>
>> [...] if you profiled something and notice a measurable difference
>> between `has()` and `hasSubstatement()` in practice, that would
>> be really good for the community to know.
>
> Do we have some sort of benchmarking facility in place, or do I
> have to homebrew something?

I've always homebrewed it, but if we have better facilities these days, I'd 
love to know about them!


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

https://reviews.llvm.org/D116328

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

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-13 Thread Richard via Phabricator via cfe-commits

LegalizeAdulthood added a comment.

In D116328#3241329 , @aaron.ballman 
wrote:

> In D116328#3223344 , 
> @LegalizeAdulthood wrote:
>
>> My takeaway:
>>
>> - if `has` isn't expensive, I can either ditch this public matcher or move 
>> it to be a private matcher used in my check
>
> [...] if you profiled something and notice a measurable difference
> between `has()` and `hasSubstatement()` in practice, that would
> be really good for the community to know.

Do we have some sort of benchmarking facility in place, or do I
have to homebrew something?


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

https://reviews.llvm.org/D116328

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Jon Chesterfield via Phabricator via cfe-commits

JonChesterfield added a comment.

It looks deeply wrong to be marking global as either llvm.used or 
llvm.compiler.used based on the output file format. It should be based on the 
(purpose of) the global.

In D97446#3241142 , @rnk wrote:

> This change was implemented so that llvm.used could prevent section GC on 
> ELF, to make its semantics consistent with llvm.used on COFF and MachO. 
> Essentially, llvm.used behaves differently on ELF now so to prevent behavior 
> changes for users of `__attribute__((used))`, it was migrated to 
> `llvm.compiler.used` on ELF targets. This is consistent with GCC's behavior.

Is this sentence inverted? llvm.used should prevent sections from being 
discarded. If it doesn't at present, that's a bug in the linker. 
llvm.compiler.used should generally be discarded by whatever part of the 
compiler wanted the variable, but if it makes it to the linker, the linker 
should throw it away. Because it was only used by the compiler. It's possible 
some users marked things as 'used' but wanted them thrown away, but it seems 
more likely that users weren't using gc-sections if it broke their application.
 by throwing away things they asked to keep.

> This should only change behavior for you if you depend on the details of the 
> LLVM IR being emitted, or perhaps if you use LTO, where GlobalOpt will behave 
> differently. I don't think these use cases are as important as consistent ELF 
> section GC behavior.
>
> So, apologies for changing the LLVM IR emitted for this attribute, but I 
> think it's unlikely we will change our minds and revert this a year later.

I'm hopeful I can change your minds now. The current modelling in the compiler 
doesn't match the stated intent.

In D97446#3241195 , @MaskRay wrote:

> llvm.compiler.used hasn't been changed.

True, but attribute((used)) has.

> The text focuses on the semantics, and for practical reasons refers to the 
> toolchain support.
> Before GCC 11/binutils 2.36 there was just no portable way making a 
> definition not discarded by ld --gc-sections.

Sure there was. Don't pass gc-sections to the linker, or don't compile with 
ffunction-sections to get a close approximation.

>> edit: Would making attribute((used)) imply attribute((retain)) on elf 
>> targets achieve the objective of this patch without breaking code that 
>> expects 'used' to mean "don't throw this away"?
>
> This would make semantics less orthogonal and incompatible with GCC.
> On COFF and Mach-O, there have been Clang-specific (not GCC) use cases 
> relying on attribute((used)) implying GC roots.
> On ELF, there was none before the toolchain support.
>
> Every llvm.used usage I can find in the wild does intend to have the GC 
> semantics, and not having it on ELF was actually a bug and has been fixed by 
> the patch series.

I'm absolutely sure that people mark things as attribute((used)) to stop the 
toolchain discarding them. I think we're in agreement there, but differ in our 
assessment of popularity of gc-sections.

Are we missing a category here?

llvm.compiler.used <- the compiler uses the global, and may discard it. If it 
doesn't, the linker should discard it
llvm.linker.used <- the linker uses this global, and may discard it. The 
compiler should leave it alone aside from passing it to the linker
llvm.used <- some unspecified thing uses the global, the compiler and linker 
should leave it alone aside from embedding it in the linked output

If we have to map 'attribute((used))' onto the new llvm.linker.used and 
'attribute((retain))' onto llvm.used that's a shame, but at least it keeps the 
naming weirdness localised to the language front end.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D117093: [C++20] [Modules] Exit early if export decl is invalid

2022-01-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!




Comment at: clang/lib/Sema/SemaModule.cpp:530-531
 
+  CurContext->addDecl(D);
+  PushDeclContext(S, D);
+

ChuanqiXu wrote:
> aaron.ballman wrote:
> > Am I understanding properly that this moved up here so that the for loop on 
> > line 553 can traverse the new context?
> > 
> > If so, can it be moved down to immediately before the for loop?
> The intention to move these up is to make sure D could be put in the context 
> even error detected. It wouldn't affect the traverse on line 556 since an 
> ExportDecl wouldn't be a NamespaceDecl.
Ah yes, that makes far more sense. Thanks!



Comment at: clang/lib/Sema/SemaModule.cpp:539-550
+return D;
   } else if (!ModuleScopes.back().ModuleInterface) {
 Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
 Diag(ModuleScopes.back().BeginLoc,
  diag::note_not_module_interface_add_export)
 << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+return D;

ChuanqiXu wrote:
> aaron.ballman wrote:
> > It seems a bit suspicious to me that we call `D->getInvalidDecl()` below 
> > within the for loop, but all the other places we leave it as a valid 
> > declaration despite it causing error diagnostics.
> Yeah, it is intentional to call `D->setInvalidDecl()` this loop. It would 
> suppress more diagnostic messages. But I feel it is good to call 
> `D->setInvalidDecl()` in other places.
Thanks, I agree that we want to mark the decl invalid rather than leave it 
seeming valid.


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

https://reviews.llvm.org/D117093

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

[PATCH] D117107: [clangd] Elide even more checks in SelectionTree.

2022-01-13 Thread Haojian Wu via Phabricator via cfe-commits

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

Thanks, this looks great!




Comment at: clang-tools-extra/clangd/Selection.cpp:364
+return *Offset < First;
+  log("Start {0}", Tok.dumpForTests(SM));
+  StartInvalid = true;

nit: remove this debug code.



Comment at: clang-tools-extra/clangd/Selection.cpp:390
+return *Offset <= Last;
+  log("End {0}", Tok.dumpForTests(SM));
+  EndInvalid = true;

and here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117107

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

[PATCH] D116843: [Driver][Gnu] -r: imply -nostdlib like GCC

2022-01-13 Thread Fangrui Song 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 rG64da6eb06570: [Driver][Gnu] -r: imply -nostdlib like GCC 
(authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116843

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-cross.cpp


Index: clang/test/Driver/linux-cross.cpp
===
--- clang/test/Driver/linux-cross.cpp
+++ clang/test/Driver/linux-cross.cpp
@@ -210,3 +210,14 @@
 // RUN: %clang -### %s --target=i686-linux-musl -mx32 --sysroot= \
 // RUN:   --stdlib=platform --rtlib=platform 2>&1 | FileCheck %s 
--check-prefix=MUSL_X32
 // MUSL_X32: "-dynamic-linker" "/lib/ld-musl-x32.so.1"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-linux-gnu 
--sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin 
-resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform -r 2>&1 | FileCheck %s 
--check-prefix=RELOCATABLE
+// RELOCATABLE:  "-internal-isystem"
+// RELOCATABLE-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// RELOCATABLE:  "-L
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -487,7 +487,8 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!isAndroid && !IsIAMCU) {
   const char *crt1 = nullptr;
   if (!Args.hasArg(options::OPT_shared)) {
@@ -563,7 +564,8 @@
   getToolChain().addProfileRTLibs(Args, CmdArgs);
 
   if (D.CCCIsCXX() &&
-  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (ToolChain.ShouldLinkCXXStdlib(Args)) {
   bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
  !Args.hasArg(options::OPT_static);
@@ -578,7 +580,7 @@
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
-  if (!Args.hasArg(options::OPT_nostdlib)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_r)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
   if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--start-group");


Index: clang/test/Driver/linux-cross.cpp
===
--- clang/test/Driver/linux-cross.cpp
+++ clang/test/Driver/linux-cross.cpp
@@ -210,3 +210,14 @@
 // RUN: %clang -### %s --target=i686-linux-musl -mx32 --sysroot= \
 // RUN:   --stdlib=platform --rtlib=platform 2>&1 | FileCheck %s --check-prefix=MUSL_X32
 // MUSL_X32: "-dynamic-linker" "/lib/ld-musl-x32.so.1"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform -r 2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE:  "-internal-isystem"
+// RELOCATABLE-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// RELOCATABLE:  "-L
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -487,7 +487,8 @@
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!isAndroid && !IsIAMCU) {
   const char *crt1 = nullptr;
   if (!Args.hasArg(options::OPT_shared)) {
@@ -563,7 +564,8 @@
   getToolChain().addProfileRTLibs(Args, CmdArgs);
 
   if (D.CCCIsCXX() &&
-  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (ToolChain.ShouldLinkCXXStdlib(Args)) {
   bool

[clang] 64da6eb - [Driver][Gnu] -r: imply -nostdlib like GCC

2022-01-13 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2022-01-13T11:25:23-08:00
New Revision: 64da6eb06570adf38c96075de0116f6d29d5ba81

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

LOG: [Driver][Gnu] -r: imply -nostdlib like GCC

See `gcc -dumpspecs` that -r essentially implies -nostdlib and suppresses
default -l* and crt*.o. The behavior makes sense because otherwise there will be
assuredly conflicting definitions when the relocatable output is linked into the
final executable/shared object.

Reviewed By: thesamesam, phosek

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/linux-cross.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 7aeadd84dfee8..fbb1af4dbe22b 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -487,7 +487,8 @@ void tools::gnutools::Linker::ConstructJob(Compilation , 
const JobAction ,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!isAndroid && !IsIAMCU) {
   const char *crt1 = nullptr;
   if (!Args.hasArg(options::OPT_shared)) {
@@ -563,7 +564,8 @@ void tools::gnutools::Linker::ConstructJob(Compilation , 
const JobAction ,
   getToolChain().addProfileRTLibs(Args, CmdArgs);
 
   if (D.CCCIsCXX() &&
-  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (ToolChain.ShouldLinkCXXStdlib(Args)) {
   bool OnlyLibstdcxxStatic = Args.hasArg(options::OPT_static_libstdcxx) &&
  !Args.hasArg(options::OPT_static);
@@ -578,7 +580,7 @@ void tools::gnutools::Linker::ConstructJob(Compilation , 
const JobAction ,
   // Silence warnings when linking C code with a C++ '-stdlib' argument.
   Args.ClaimAllArgs(options::OPT_stdlib_EQ);
 
-  if (!Args.hasArg(options::OPT_nostdlib)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_r)) {
 if (!Args.hasArg(options::OPT_nodefaultlibs)) {
   if (IsStatic || IsStaticPIE)
 CmdArgs.push_back("--start-group");

diff  --git a/clang/test/Driver/linux-cross.cpp 
b/clang/test/Driver/linux-cross.cpp
index 98e4b7cb1baa8..20d069112acf2 100644
--- a/clang/test/Driver/linux-cross.cpp
+++ b/clang/test/Driver/linux-cross.cpp
@@ -210,3 +210,14 @@
 // RUN: %clang -### %s --target=i686-linux-musl -mx32 --sysroot= \
 // RUN:   --stdlib=platform --rtlib=platform 2>&1 | FileCheck %s 
--check-prefix=MUSL_X32
 // MUSL_X32: "-dynamic-linker" "/lib/ld-musl-x32.so.1"
+
+/// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-linux-gnu 
--sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN:   -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin 
-resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform -r 2>&1 | FileCheck %s 
--check-prefix=RELOCATABLE
+// RELOCATABLE:  "-internal-isystem"
+// RELOCATABLE-SAME: {{^}} 
"[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
+// RELOCATABLE:  "-L
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/x86_64-linux-gnu/10"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^.]+}}.o



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

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

2022-01-13 Thread Lambert Clara via Phabricator via cfe-commits

belkiss added inline comments.



Comment at: 
compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp:33
+int main() {
+  // CHECK-NOT: runtime error: constructor call on misaligned address 
[[PTR:0x[0-9a-f]*]] for type 'Param', which requires 16 byte alignment
+  Param *p = new Param;

rnk wrote:
> I suggest simplifying this to `CHECK-NOT: runtime error`, since no errors are 
> expected. This is mostly to improve test debuggability anyway, the test will 
> fail if it exits non-zero.
I'll remove it altogether from here, since I pass 
`--implicit-check-not="runtime error"` to the FileCheck at the top of the file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116861

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

[PATCH] D116328: [ast-matchers] Add hasSubstatement() traversal matcher for caseStmt(), defaultStmt(), labelStmt()

2022-01-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D116328#3223344 , 
@LegalizeAdulthood wrote:

> My takeaway:
>
> - if `has` isn't expensive, I can either ditch this public matcher or move it 
> to be a private matcher used in my check

I don't think `has()` is particularly expensive, so I think you should be able 
to use it directly. However, if you profiled something and notice a measurable 
difference between `has()` and `hasSubstatement()` in practice, that would be 
really good for the community to know.

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

https://reviews.llvm.org/D116328

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

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

2022-01-13 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a subscriber: rjmccall.
rnk added a comment.

I think the main thing here is that the diagnostic is incorrect: It claims that 
the type requires `__STDC_DEFAULT_NEW_ALIGNMENT__` (16) byte alignment, but it 
doesn't, it only requires 8. That's confusing.

The next question is, does LLVM exploit `__STDCPP_DEFAULT_NEW_ALIGNMENT__` for 
optimization purposes? IMO, UBSan should agree with LLVM: it is supposed to aid 
in the early detection of UB, rather than having to figure that out later after 
optimizations have run. If LLVM doesn't exploit it, I don't see a need to 
report an error. After some experimentation, I have convinced myself that LLVM 
doesn't know or exploit allocator alignment:
https://godbolt.org/z/oo6rzKn3c

Finally, we should ask ourselves, is it worth adding a special diagnostic to 
check that all operator new calls return aligned pointers? I think the answer 
is no. Users are more likely to heed sanitizer errors when they point to 
actual, practical issues rather than hypothetical ones about more highly 
aligned types allocated at other call sites.

Let me ask @rjmccall for a second opinion, since he wrote the `Address` 
alignment-tracking overhaul code.




Comment at: 
compiler-rt/test/ubsan/TestCases/TypeCheck/global-new-alignment.cpp:33
+int main() {
+  // CHECK-NOT: runtime error: constructor call on misaligned address 
[[PTR:0x[0-9a-f]*]] for type 'Param', which requires 16 byte alignment
+  Param *p = new Param;

I suggest simplifying this to `CHECK-NOT: runtime error`, since no errors are 
expected. This is mostly to improve test debuggability anyway, the test will 
fail if it exits non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116861

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

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

2022-01-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/test/Preprocessor/init.c:1523
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BITINT_MAXWIDTH__ 8388608
 // WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8

aaron.ballman wrote:
> erichkeane wrote:
> > Actually why are we testing WEBASSEMBLY/AArch64 only?  I get that these 
> > two both do a 'next', but I would presume we'd want a similar test that 
> > ends up being for ALL platforms.  Its unfortunate the way that this test is 
> > setup, but could we perhaps have a 'triple-less' (or a test that just has a 
> > massive number of triples?) test of some sort that just validates this 
> > value?
> I agree that'd be nice, but it seems pretty orthogonal to this patch too. The 
> "init" tests definitely need some love because they're incredibly onerous. 
> But I'd prefer that be done another time.
> 
> As for why only here -- the value is identical for all targets currently, so 
> adding tests for other targets would be a great idea, but not really test 
> much of value. However, I can add the lines to the other targets easily 
> enough if you think there's value.
Sure, yeah, I see the lack of value.  I guess if anyone ever tries to make it 
target-specific we can make them do the additional testing instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117238

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

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

2022-01-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/Preprocessor/init.c:1523
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BITINT_MAXWIDTH__ 8388608
 // WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8

erichkeane wrote:
> Actually why are we testing WEBASSEMBLY/AArch64 only?  I get that these 
> two both do a 'next', but I would presume we'd want a similar test that ends 
> up being for ALL platforms.  Its unfortunate the way that this test is setup, 
> but could we perhaps have a 'triple-less' (or a test that just has a massive 
> number of triples?) test of some sort that just validates this value?
I agree that'd be nice, but it seems pretty orthogonal to this patch too. The 
"init" tests definitely need some love because they're incredibly onerous. But 
I'd prefer that be done another time.

As for why only here -- the value is identical for all targets currently, so 
adding tests for other targets would be a great idea, but not really test much 
of value. However, I can add the lines to the other targets easily enough if 
you think there's value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117238

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

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

2022-01-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/test/Preprocessor/init.c:1523
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BITINT_MAXWIDTH__ 8388608
 // WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8

Actually why are we testing WEBASSEMBLY/AArch64 only?  I get that these two 
both do a 'next', but I would presume we'd want a similar test that ends up 
being for ALL platforms.  Its unfortunate the way that this test is setup, but 
could we perhaps have a 'triple-less' (or a test that just has a massive number 
of triples?) test of some sort that just validates this value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117238

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

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

2022-01-13 Thread Erich Keane via Phabricator via cfe-commits

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

CFE changes LGTM, the limits.h/.cpp changes LOOK right, but please give others 
a chance to take a look.




Comment at: clang/lib/Sema/SemaType.cpp:2270
 
+  // The max size of a _BitInt is exposed to the user via the
+  // __BITINT_MAXWIDTH__ macro from InitPreprocessor.cpp. If someday a target

Thanks for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117238

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

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

2022-01-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, jyknight, rsmith.
Herald added a subscriber: dschuff.
aaron.ballman requested review of this revision.
Herald added a subscriber: aheejin.
Herald added a project: clang.

Part of the `_BitInt` feature in C2x 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2763.pdf) is a new macro in 
limits.h named `BITINT_MAXWIDTH` that can be used to determine the maximum 
width of a bit-precise integer type. This macro must expand to a value that is 
at least as large as `ULLONG_WIDTH`.

This adds an implementation-defined macro named `__BITINT_MAXWIDTH__` to 
specify that value, which is used by limits.h for the standard macro. This 
gives us a migration path in the future should a target ever need to support 
something different than `llvm::IntegerType::MAX_INT_BITS`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117238

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/limits.h
  clang/lib/Sema/SemaType.cpp
  clang/test/Headers/limits.cpp
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1520,6 +1520,7 @@
 // WEBASSEMBLY-NEXT:#define __ATOMIC_RELEASE 3
 // WEBASSEMBLY-NEXT:#define __ATOMIC_SEQ_CST 5
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BITINT_MAXWIDTH__ 8388608
 // WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
 // WEBASSEMBLY-NEXT:#define __CHAR16_TYPE__ unsigned short
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -40,6 +40,7 @@
 // AARCH64-NEXT: #define __ATOMIC_SEQ_CST 5
 // AARCH64:  #define __BIGGEST_ALIGNMENT__ 16
 // AARCH64_BE-NEXT: #define __BIG_ENDIAN__ 1
+// AARCH64-NEXT: #define __BITINT_MAXWIDTH__ 8388608
 // AARCH64-NEXT: #define __BOOL_WIDTH__ 8
 // AARCH64_BE-NEXT: #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
 // AARCH64_LE-NEXT: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
Index: clang/test/Headers/limits.cpp
===
--- clang/test/Headers/limits.cpp
+++ clang/test/Headers/limits.cpp
@@ -74,8 +74,11 @@
 _Static_assert(ULLONG_WIDTH / CHAR_BIT == sizeof(unsigned long long));
 _Static_assert(LLONG_WIDTH == ULLONG_WIDTH);
 _Static_assert(LLONG_WIDTH / CHAR_BIT == sizeof(signed long long));
+
+_Static_assert(BITINT_MAXWIDTH >= ULLONG_WIDTH);
 #else
 /* None of these are defined. */
 int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH,
-UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH;
+UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH,
+BITINT_MAXWIDTH;
 #endif
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2267,6 +2267,10 @@
 return QualType();
   }
 
+  // The max size of a _BitInt is exposed to the user via the
+  // __BITINT_MAXWIDTH__ macro from InitPreprocessor.cpp. If someday a target
+  // needs to pick a different max value, be sure to update
+  // InitializePredefinedMacros() as well.
   if (NumBits > llvm::IntegerType::MAX_INT_BITS) {
 Diag(Loc, diag::err_bit_int_max_size)
 << IsUnsigned << llvm::IntegerType::MAX_INT_BITS;
Index: clang/lib/Headers/limits.h
===
--- clang/lib/Headers/limits.h
+++ clang/lib/Headers/limits.h
@@ -78,6 +78,8 @@
 #define LONG_WIDTH   __LONG_WIDTH__
 #define ULLONG_WIDTH __LLONG_WIDTH__
 #define LLONG_WIDTH  __LLONG_WIDTH__
+
+#define BITINT_MAXWIDTH __BITINT_MAXWIDTH__
 #endif
 
 #ifdef __CHAR_UNSIGNED__  /* -funsigned-char */
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -25,6 +25,7 @@
 #include "clang/Serialization/ASTReader.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DerivedTypes.h"
 using namespace clang;
 
 static bool MacroBodyEndsInBackslash(StringRef MacroBody) {
@@ -914,6 +915,12 @@
   Builder.defineMacro("__LONG_WIDTH__", Twine(TI.getLongWidth()));
   Builder.defineMacro("__LLONG_WIDTH__", Twine(TI.getLongLongWidth()));
 
+  // The max size of a _BitInt is limited in SemaType.cpp to this value. If
+  // someday a target needs to pick a different value for this macro, be sure
+  // to update Sema::BuildBitIntType() as well.
+  Builder.defineMacro("__BITINT_MAXWIDTH__",
+

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D97446#3241142 , @rnk wrote:

> In D97446#3240309 , @JonChesterfield 
> wrote:
>
>> If I had a red buildbot to reference I would have reverted - the commit 
>> message does not make it clear that this is a breaking change. This broke a 
>> debugging hook in openmp, which is apparently not tested, and will break any 
>> application code that uses compiler.used on a target that uses elf.

llvm.compiler.used hasn't been changed.

>> Linked docs at https://reviews.llvm.org/D97447 suggest applications can get 
>> back to a working state by marking things as attribute((used)) 
>> attribute((retain)), presumably guarded by a test for whether retain exists. 
>> I think this would be another point in a codebase that has to distinguish 
>> between gcc and clang versions when picking attributes.
>>
>> edit: further reading suggests retain turned up in gcc 11 and requires 
>> binutils 2.36, with semantics similar to but distinct from used. It's a 
>> linker section discard directive, so the 'garbage collection roots' in this 
>> context may refer to bits of an elf instead of the language runtime feature.

The text focuses on the semantics, and for practical reasons refers to the 
toolchain support.
Before GCC 11/binutils 2.36 there was just no portable way making a definition 
not discarded by ld --gc-sections.
The patch series added the support but obviously cannot alter the fact that the 
toolchain was catching up.

>> I have fixed openmp by marking variables as retain but breaking applications 
>> that are relying on used (by discarding the variables) remains bad. Is this 
>> breakage already shipping with gcc, thus the ship has sailed, or can we keep 
>> backwards compat here?
>>
>> edit: Would making attribute((used)) imply attribute((retain)) on elf 
>> targets achieve the objective of this patch without breaking code that 
>> expects 'used' to mean "don't throw this away"?

That would make semantics less orthogonal and incompatible with GCC.
On COFF and Mach-O, there have been use cases relying on attribute((used)) 
implying GC roots.
On ELF, there was none before the toolchain support.
Every llvm.used usage I can find in the wild does intend to have the GC 
semantics, and not having it on ELF was actually a bug and has been fixed by 
the patch series.




Comment at: clang/lib/CodeGen/CGDecl.cpp:445
   if (D.hasAttr())
-CGM.addUsedGlobal(var);
+CGM.addUsedOrCompilerUsedGlobal(var);
 

rnk wrote:
> JonChesterfield wrote:
> > I think this (and the corresponding line in codgen) is incorrect.
> > 
> > Previously, attribute((used)) marked something as 'used', so it makes it 
> > all the way to the linker.
> > 
> > After this change, anything that answers getTriple().isOSBinFormatELF() 
> > with true will emit ((used)) as compiler.used, which means it gets deleted 
> > earlier. In particular, amdgpu uses elf and the openmp runtime marks some 
> > symbols used, which are now getting deleted by clang during internalise.
> > 
> > Lots of targets presumably use 'elf' as the target binary format though, so 
> > I expect this to have broken user facing code on all of them.
> > 
> This is the same behavior they would get in a native link if they used 
> `-ffunction-sections` / `--gc-sections`, so you have described the desired 
> behavior change: it makes LTO internalize+globalopt consistent with native 
> links.
llvm.used traditionally did not have GC root semantics on ELF platforms, 
actually it was identical to llvm.compiler.used (modulo possible bugs). This is 
the intended change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits

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

Assuming all other tests pass, I'm ok


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116859

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

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1362
+  LangStandard::lang_cxx17);
+}
+

xazax.hun wrote:
> I think this changed from 14 to 17 in the last revision. Is this intentional?
Not intentional. Thanks for catching it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

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

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev updated this revision to Diff 399714.
sgatev marked 2 inline comments as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -15,6 +15,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -37,7 +38,8 @@
 class TransferTest : public ::testing::Test {
 protected:
   template 
-  void runDataflow(llvm::StringRef Code, Matcher Match) {
+  void runDataflow(llvm::StringRef Code, Matcher Match,
+   LangStandard::Kind Std = LangStandard::lang_cxx17) {
 test::checkDataflow(
 Code, "target",
 [](ASTContext , Environment &) { return NoopAnalysis(C); },
@@ -45,7 +47,9 @@
  std::pair>>
  Results,
  ASTContext ) { Match(Results, ASTCtx); },
-{"-fsyntax-only", "-std=c++17"});
+{"-fsyntax-only",
+ "-std=" +
+ std::string(LangStandard::getLangStandardForKind(Std).getName())});
   }
 };
 
@@ -1281,4 +1285,308 @@
   });
 }
 
+TEST_F(TransferTest, TemporaryObject) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+
+void target() {
+  A Foo = A();
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc =
+cast(>getChild(*BarDecl));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(>getChild(*BarDecl));
+EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
+  });
+}
+
+TEST_F(TransferTest, ElidableConstructor) {
+  // This test is effectively the same as TransferTest.TemporaryObject, but
+  // the code is compiled as C++ 14.
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+
+void target() {
+  A Foo = A();
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc =
+cast(>getChild(*BarDecl));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(>getChild(*BarDecl));
+EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
+  },
+  LangStandard::lang_cxx14);
+}
+
+TEST_F(TransferTest, AssignmentOperator) {
+  std::string Code = R"(
+struct A {
+  int Baz;
+};
+
+void target() {
+  A Foo;
+  A Bar;
+  // [[p1]]
+  Foo = Bar;
+  // [[p2]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
+const Environment  = Results[0].second.Env;
+const Environment  = Results[1].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooLoc1 = cast(
+Env1.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc1 = cast(
+

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1362
+  LangStandard::lang_cxx17);
+}
+

I think this changed from 14 to 17 in the last revision. Is this intentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

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

[PATCH] D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal

2022-01-13 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

In D97446#3240309 , @JonChesterfield 
wrote:

> I don't know the context of this patch but changing attribute((used)) to put 
> things under compiler.used is definitely a breaking change. Please introduce 
> a new attribute if necessary for garbage collection purposes instead of 
> breaking this.

This change was implemented so that llvm.used could prevent section GC on ELF, 
to make its semantics consistent with llvm.used on COFF and MachO. Essentially, 
llvm.used behaves differently on ELF now so to prevent behavior changes for 
users of `__attribute__((used))`, it was migrated to `llvm.compiler.used` on 
ELF targets. This is consistent with GCC's behavior.

This should only change behavior for you if you depend on the details of the 
LLVM IR being emitted, or perhaps if you use LTO, where GlobalOpt will behave 
differently. I don't think these use cases are as important as consistent ELF 
section GC behavior.

So, apologies for changing the LLVM IR emitted for this attribute, but I think 
it's unlikely we will change our minds and revert this a year later.

> If I had a red buildbot to reference I would have reverted - the commit 
> message does not make it clear that this is a breaking change. This broke a 
> debugging hook in openmp, which is apparently not tested, and will break any 
> application code that uses compiler.used on a target that uses elf.
>
> Linked docs at https://reviews.llvm.org/D97447 suggest applications can get 
> back to a working state by marking things as attribute((used)) 
> attribute((retain)), presumably guarded by a test for whether retain exists. 
> I think this would be another point in a codebase that has to distinguish 
> between gcc and clang versions when picking attributes.
>
> edit: further reading suggests retain turned up in gcc 11 and requires 
> binutils 2.36, with semantics similar to but distinct from used. It's a 
> linker section discard directive, so the 'garbage collection roots' in this 
> context may refer to bits of an elf instead of the language runtime feature.
>
> I have fixed openmp by marking variables as retain but breaking applications 
> that are relying on used (by discarding the variables) remains bad. Is this 
> breakage already shipping with gcc, thus the ship has sailed, or can we keep 
> backwards compat here?
>
> edit: Would making attribute((used)) imply attribute((retain)) on elf targets 
> achieve the objective of this patch without breaking code that expects 'used' 
> to mean "don't throw this away"?






Comment at: clang/lib/CodeGen/CGDecl.cpp:445
   if (D.hasAttr())
-CGM.addUsedGlobal(var);
+CGM.addUsedOrCompilerUsedGlobal(var);
 

JonChesterfield wrote:
> I think this (and the corresponding line in codgen) is incorrect.
> 
> Previously, attribute((used)) marked something as 'used', so it makes it all 
> the way to the linker.
> 
> After this change, anything that answers getTriple().isOSBinFormatELF() with 
> true will emit ((used)) as compiler.used, which means it gets deleted 
> earlier. In particular, amdgpu uses elf and the openmp runtime marks some 
> symbols used, which are now getting deleted by clang during internalise.
> 
> Lots of targets presumably use 'elf' as the target binary format though, so I 
> expect this to have broken user facing code on all of them.
> 
This is the same behavior they would get in a native link if they used 
`-ffunction-sections` / `--gc-sections`, so you have described the desired 
behavior change: it makes LTO internalize+globalopt consistent with native 
links.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97446

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

[PATCH] D117177: [NFC][DFSan] Update DataFlowSanitizer user docs for -dfsan-conditional-callbacks, added in https://reviews.llvm.org/D116207

2022-01-13 Thread Andrew via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG529f098789d3: [NFC][DFSan] Update DataFlowSanitizer user 
docs for -dfsan-conditional… (authored by browneee).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117177

Files:
  clang/docs/DataFlowSanitizer.rst


Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -214,6 +214,25 @@
   void __dfsan_mem_transfer_callback(dfsan_label *Start, size_t Len);
   void __dfsan_cmp_callback(dfsan_label CombinedLabel);
 
+* ``-dfsan-conditional-callbacks`` -- An experimental feature that inserts
+  callbacks for control flow conditional expressions.
+  This can be used to find where tainted values can control execution.
+
+  In addition to this compilation flag, a callback handler must be registered
+  using ``dfsan_set_conditional_callback(my_callback);``, where my_callback is
+  a function with a signature matching
+  ``void my_callback(dfsan_label l, dfsan_origin o);``.
+  This signature is the same when origin tracking is disabled - in this case
+  the dfsan_origin passed in it will always be 0.
+
+  The callback will only be called when a tainted value reaches a conditional
+  expression for control flow (such as an if's condition).
+  The callback will be skipped for conditional expressions inside signal
+  handlers, as this is prone to deadlock. Tainted values used in conditional
+  expressions inside signal handlers will instead be aggregated via bitwise
+  or, and can be accessed using
+  ``dfsan_label dfsan_get_labels_in_signal_conditional();``.
+
 * ``-dfsan-track-origins`` -- Controls how to track origins. When its value is
   0, the runtime does not track origins. When its value is 1, the runtime 
tracks
   origins at memory store operations. When its value is 2, the runtime tracks


Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -214,6 +214,25 @@
   void __dfsan_mem_transfer_callback(dfsan_label *Start, size_t Len);
   void __dfsan_cmp_callback(dfsan_label CombinedLabel);
 
+* ``-dfsan-conditional-callbacks`` -- An experimental feature that inserts
+  callbacks for control flow conditional expressions.
+  This can be used to find where tainted values can control execution.
+
+  In addition to this compilation flag, a callback handler must be registered
+  using ``dfsan_set_conditional_callback(my_callback);``, where my_callback is
+  a function with a signature matching
+  ``void my_callback(dfsan_label l, dfsan_origin o);``.
+  This signature is the same when origin tracking is disabled - in this case
+  the dfsan_origin passed in it will always be 0.
+
+  The callback will only be called when a tainted value reaches a conditional
+  expression for control flow (such as an if's condition).
+  The callback will be skipped for conditional expressions inside signal
+  handlers, as this is prone to deadlock. Tainted values used in conditional
+  expressions inside signal handlers will instead be aggregated via bitwise
+  or, and can be accessed using
+  ``dfsan_label dfsan_get_labels_in_signal_conditional();``.
+
 * ``-dfsan-track-origins`` -- Controls how to track origins. When its value is
   0, the runtime does not track origins. When its value is 1, the runtime tracks
   origins at memory store operations. When its value is 2, the runtime tracks
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 529f098 - [NFC][DFSan] Update DataFlowSanitizer user docs for -dfsan-conditional-callbacks, added in https://reviews.llvm.org/D116207

2022-01-13 Thread Andrew Browne via cfe-commits


Author: Andrew Browne
Date: 2022-01-13T10:05:45-08:00
New Revision: 529f098789d334de9cd06e913ad9525719fa0774

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

LOG: [NFC][DFSan] Update DataFlowSanitizer user docs for 
-dfsan-conditional-callbacks, added in https://reviews.llvm.org/D116207

Reviewed By: morehouse

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

Added: 


Modified: 
clang/docs/DataFlowSanitizer.rst

Removed: 




diff  --git a/clang/docs/DataFlowSanitizer.rst 
b/clang/docs/DataFlowSanitizer.rst
index cce50dc913185..b4136a935dd28 100644
--- a/clang/docs/DataFlowSanitizer.rst
+++ b/clang/docs/DataFlowSanitizer.rst
@@ -214,6 +214,25 @@ labels of just ``v1`` and ``v2``.
   void __dfsan_mem_transfer_callback(dfsan_label *Start, size_t Len);
   void __dfsan_cmp_callback(dfsan_label CombinedLabel);
 
+* ``-dfsan-conditional-callbacks`` -- An experimental feature that inserts
+  callbacks for control flow conditional expressions.
+  This can be used to find where tainted values can control execution.
+
+  In addition to this compilation flag, a callback handler must be registered
+  using ``dfsan_set_conditional_callback(my_callback);``, where my_callback is
+  a function with a signature matching
+  ``void my_callback(dfsan_label l, dfsan_origin o);``.
+  This signature is the same when origin tracking is disabled - in this case
+  the dfsan_origin passed in it will always be 0.
+
+  The callback will only be called when a tainted value reaches a conditional
+  expression for control flow (such as an if's condition).
+  The callback will be skipped for conditional expressions inside signal
+  handlers, as this is prone to deadlock. Tainted values used in conditional
+  expressions inside signal handlers will instead be aggregated via bitwise
+  or, and can be accessed using
+  ``dfsan_label dfsan_get_labels_in_signal_conditional();``.
+
 * ``-dfsan-track-origins`` -- Controls how to track origins. When its value is
   0, the runtime does not track origins. When its value is 1, the runtime 
tracks
   origins at memory store operations. When its value is 2, the runtime tracks



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

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev updated this revision to Diff 399707.
sgatev marked 2 inline comments as done.
sgatev added a comment.

Address reviewers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -15,6 +15,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -37,7 +38,8 @@
 class TransferTest : public ::testing::Test {
 protected:
   template 
-  void runDataflow(llvm::StringRef Code, Matcher Match) {
+  void runDataflow(llvm::StringRef Code, Matcher Match,
+   LangStandard::Kind Std = LangStandard::lang_cxx17) {
 test::checkDataflow(
 Code, "target",
 [](ASTContext , Environment &) { return NoopAnalysis(C); },
@@ -45,7 +47,9 @@
  std::pair>>
  Results,
  ASTContext ) { Match(Results, ASTCtx); },
-{"-fsyntax-only", "-std=c++17"});
+{"-fsyntax-only",
+ "-std=" +
+ std::string(LangStandard::getLangStandardForKind(Std).getName())});
   }
 };
 
@@ -1281,4 +1285,308 @@
   });
 }
 
+TEST_F(TransferTest, TemporaryObject) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+
+void target() {
+  A Foo = A();
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc =
+cast(>getChild(*BarDecl));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(>getChild(*BarDecl));
+EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
+  });
+}
+
+TEST_F(TransferTest, ElidableConstructor) {
+  // This test is effectively the same as TransferTest.TemporaryObject, but
+  // the code is compiled as C++ 14.
+  std::string Code = R"(
+struct A {
+  int Bar;
+};
+
+void target() {
+  A Foo = A();
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment  = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc =
+cast(>getChild(*BarDecl));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(>getChild(*BarDecl));
+EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
+  },
+  LangStandard::lang_cxx17);
+}
+
+TEST_F(TransferTest, AssignmentOperator) {
+  std::string Code = R"(
+struct A {
+  int Baz;
+};
+
+void target() {
+  A Foo;
+  A Bar;
+  // [[p1]]
+  Foo = Bar;
+  // [[p2]]
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
+const Environment  = Results[0].second.Env;
+const Environment  = Results[1].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooLoc1 = cast(
+Env1.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc1 = cast(
+

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev marked an inline comment as done.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:114
+} else if (S->getCastKind() == CK_NoOp) {
+  auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+  if (SubExprLoc == nullptr)

xazax.hun wrote:
> An alternative way to handle Noop operations would be to make location lookup 
> function always skip certain nodes, so we do not need to store locations for 
> those subexpressions. I don't have a strong feeling for either solution, this 
> is fine as it is, just wanted to be sure that both were considered. 
It makes sense to consider it. Right now I don't have signals that one is 
better than the other. I think the current implementation fits the general 
model a bit better. Nevertheless, I added a FIXME to keep this option in mind.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:277
+  void VisitCallExpr(const CallExpr *S) {
+if (S->isCallToStdMove()) {
+  assert(S->getNumArgs() == 1);

xazax.hun wrote:
> Interesting, I only see `isCallToStdMove` in the CallExpr API, although I 
> imagine, `std::forward` has a similar level of importance. 
I haven't looked into `std::forward` yet, but happy to do that in the next 
patch.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:39
 protected:
+  enum class CppVersion {
+k14 = 14,

xazax.hun wrote:
> Shouldn't we piggyback on `clang::LangStandard::Kind`? If it is not easy to 
> convert that to the appropriate command line flag feel free to ignore this.
Makes sense. Thanks for pointing this out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

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

[PATCH] D116748: [AArch64][ARM][Clang] PerfMon Extension Added

2022-01-13 Thread Dave Green via Phabricator via cfe-commits

dmgreen accepted this revision.
dmgreen added a comment.

LGTM. Thanks for the update.




Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:147
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64", 
"-sme-i64")
+AARCH64_ARCH_EXT_NAME("pmuv3",  AArch64::AEK_PERFMON, "+perfmon", 
"-perfmon")
 #undef AARCH64_ARCH_EXT_NAME

The formatting is a little off, for the subsequent columns.


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

https://reviews.llvm.org/D116748

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

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

2022-01-13 Thread Eli Friedman via Phabricator via cfe-commits

efriedma abandoned this revision.
efriedma added a comment.
Herald added a subscriber: libcxx-commits.

A very similar patch was merged in e0d01294bc124211a8ffb55e69162eb34a242680 
 .


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D41316

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

[PATCH] D116750: [clang][lex] Keep references to `DirectoryLookup` objects up-to-date

2022-01-13 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp added a comment.

Great catch and thanks for the revert!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116750

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

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

2022-01-13 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a reviewer: sepavloff.
hans added a comment.

I'm not familiar enough with the code to review it (though it sounds right to 
me). Looks like it was written by Serge originally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116861

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

[PATCH] D117218: [clang][dataflow] Add transfer functions for constructors

2022-01-13 Thread Gábor Horváth via Phabricator via cfe-commits

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



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:114
+} else if (S->getCastKind() == CK_NoOp) {
+  auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
+  if (SubExprLoc == nullptr)

An alternative way to handle Noop operations would be to make location lookup 
function always skip certain nodes, so we do not need to store locations for 
those subexpressions. I don't have a strong feeling for either solution, this 
is fine as it is, just wanted to be sure that both were considered. 



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:277
+  void VisitCallExpr(const CallExpr *S) {
+if (S->isCallToStdMove()) {
+  assert(S->getNumArgs() == 1);

Interesting, I only see `isCallToStdMove` in the CallExpr API, although I 
imagine, `std::forward` has a similar level of importance. 



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:39
 protected:
+  enum class CppVersion {
+k14 = 14,

Shouldn't we piggyback on `clang::LangStandard::Kind`? If it is not easy to 
convert that to the appropriate command line flag feel free to ignore this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117218

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

[PATCH] D116511: [clang-cl] Support the /HOTPATCH flag

2022-01-13 Thread Hans Wennborg via Phabricator via cfe-commits

hans added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp:13
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s 
--check-prefix=HOTPATCH
+// ERR-HOTPATCH: error: unsupported option '/hotpatch' for target
+// HOTPATCH: S_COMPILE3 [size = [[#]]]

aganea wrote:
> hans wrote:
> > Does MSVC error for ARM/ARM64 too, or does it just ignore the flag?
> It prints a warning:
> ```
> D:\git\llvm-project>cl /c main.cpp /hotpatch
> Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30138 for ARM64
> Copyright (C) Microsoft Corporation.  All rights reserved.
> 
> cl : Command line warning D9002 : ignoring unknown option '/hotpatch'
> main.cpp
> ```
> But in our case, the issue is that `PATCHABLE_OP` isn't supported on ARM 
> backend, so it ends up asserting in 
> `D:/git/llvm-project/release/lib/Target/AArch64/AArch64GenMCCodeEmitter.inc` 
> in `AArch64MCCodeEmitter::getBinaryCodeForInstr`. There's perhaps a (better?) 
> way for shortcutting the use of `/hotpatch` on ARM, instead of erroring-out 
> like today.
> 
> Should we be more clear in the message, saying that hotpatching is supported 
> but we don't the flag? Or just consume and ignore it?
I think the most user-friendly thing would be to consume and ignore it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116511

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

1 2 >

1 - 100 of 195 matches

Mail list logo