date:20221118

[clang] 7f608a2 - Revert "[opt][clang] Enable using -module-summary/-flto=thin with -S/-emit-llvm"

2022-11-18 Thread Mikhail Goncharov via cfe-commits


Author: Mikhail Goncharov
Date: 2022-11-18T09:25:45+01:00
New Revision: 7f608a2497c7578b9f3ca98014176ab95cffe3c0

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

LOG: Revert "[opt][clang] Enable using -module-summary/-flto=thin with 
-S/-emit-llvm"

This reverts commit 34ab474348e2623cc67abddb7bb662385297ac65.

as it has introduced circular dependency lib - analysis

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/test/CodeGen/split-lto-unit.c
llvm/include/llvm/IRPrinter/IRPrintingPasses.h
llvm/lib/IRPrinter/CMakeLists.txt
llvm/lib/IRPrinter/IRPrintingPasses.cpp
llvm/test/Bitcode/thinlto-function-summary.ll
llvm/tools/opt/NewPMDriver.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index d6a7a9800ec58..9c677140b4a5d 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -983,24 +983,19 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
 MPM.addPass(VerifierPass());
 
-  if (Action == Backend_EmitBC || Action == Backend_EmitLL) {
+  switch (Action) {
+  case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
+  if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
+ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
+if (!ThinLinkOS)
+  return;
+  }
   if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
 TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
  CodeGenOpts.EnableSplitLTOUnit);
-  if (Action == Backend_EmitBC) {
-if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
-  ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
-  if (!ThinLinkOS)
-return;
-}
-MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? 
&ThinLinkOS->os()
- : nullptr));
-  } else {
-MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists,
-/*EmitLTOSummary=*/true));
-  }
-
+  MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &ThinLinkOS->os()
+   : nullptr));
 } else {
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
@@ -1012,13 +1007,17 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
uint32_t(1));
   }
-  if (Action == Backend_EmitBC)
-MPM.addPass(BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists,
-  EmitLTOSummary));
-  else
-MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists,
-EmitLTOSummary));
+  MPM.addPass(
+  BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));
 }
+break;
+
+  case Backend_EmitLL:
+MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists));
+break;
+
+  default:
+break;
   }
 
   // Now that we have all of the passes ready, run them.

diff  --git a/clang/test/CodeGen/split-lto-unit.c 
b/clang/test/CodeGen/split-lto-unit.c
index b1560b61f3e30..941aebafd01b3 100644
--- a/clang/test/CodeGen/split-lto-unit.c
+++ b/clang/test/CodeGen/split-lto-unit.c
@@ -1,15 +1,12 @@
 // ; Check that -flto=thin without -fsplit-lto-unit has EnableSplitLTOUnit = 0
 // RUN: %clang_cc1 -flto=thin -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s
-// RUN: %clang_cc1 -flto=thin -emit-llvm < %s | FileCheck %s
 // CHECK: !{i32 1, !"EnableSplitLTOUnit", i32 0}
 //
 // ; Check that -flto=thin with -fsplit-lto-unit has EnableSplitLTOUnit = 1
 // RUN: %clang_cc1 -flto=thin -fsplit-lto-unit -emit-llvm-bc < %s | llvm-dis 
-o - | FileCheck %s --check-prefix=SPLIT
-// RUN: %clang_cc1 -flto=thin -fsplit-lto-unit -emit-llvm < %s | FileCheck %s 
--check-prefix=SPLIT
 // SPLIT: !{i32 1, !"EnableSplitLTOUnit", i32 1}
 //
 // ; Check that regular LTO has EnableSplitLTOUnit = 1
 // RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | 
llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" 
--check-prefix=SPLIT
-// RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm < %s | 
FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
 
 int main(void) {}

diff  --git a/llvm/include/llvm/IRPrinter/IRPrintingPasses.h 
b/llvm/include/llvm/IRPrinter/IRPrintingPasses.h
index 8eb7973365172..6cfa7a87af933 100644
--- a/llvm/include/llvm/IRPrinter/IRPrintingPasses.h
++

[clang-tools-extra] 68294af - [include-cleaner] Fix building with LLVM_LINK_LLVM_DYLIB=ON on mingw

2022-11-18 Thread Martin Storsjö via cfe-commits


Author: Martin Storsjö
Date: 2022-11-18T10:30:26+02:00
New Revision: 68294afa0836bb62be921e2143d147cdfdc8ba70

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

LOG: [include-cleaner] Fix building with LLVM_LINK_LLVM_DYLIB=ON on mingw

Make sure to use clang_target_link_libraries, which skips linking
against libraries and links against libclang-cpp instead, if
LLVM_LINK_LLVM_DYLIB is enabled.

This fixes errors like these:

ld.lld: error: duplicate symbol: clang::PPCallbacks::~PPCallbacks()
>>> defined at libclangLex.a(PPCallbacks.cpp.obj)
>>> defined at libclang-cpp.dll

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt 
b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index 617539ae0dba4..a71276f54b643 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -8,8 +8,10 @@ add_clang_library(clangIncludeCleaner
   Record.cpp
   Types.cpp
   WalkAST.cpp
+  )
 
-  LINK_LIBS
+clang_target_link_libraries(clangIncludeCleaner
+  PRIVATE
   clangAST
   clangBasic
   clangLex



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

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 476372.
hokein marked 4 inline comments as done.
hokein added a comment.

rebase and update based on the offline discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/CMakeLists.txt
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -17,6 +17,10 @@
 namespace {
 using testing::UnorderedElementsAre;
 
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
 TEST(FindIncludeHeaders, IWYU) {
   TestInputs Inputs;
   PragmaIncludes PI;
@@ -38,20 +42,34 @@
 #include "header1.h"
 #include "header2.h"
   )cpp";
-  Inputs.ExtraFiles["header1.h"] = R"cpp(
+  Inputs.ExtraFiles["header1.h"] = guard(R"cpp(
 // IWYU pragma: private, include "path/public.h"
-  )cpp";
-  Inputs.ExtraFiles["header2.h"] = R"cpp(
+  )cpp");
+  Inputs.ExtraFiles["header2.h"] = guard(R"cpp(
 #include "detail1.h" // IWYU pragma: export
 
 // IWYU pragma: begin_exports
 #include "detail2.h"
 // IWYU pragma: end_exports
 
+#include "detail3.h" // IWYU pragma: export
+#include "imp.inc"
+#include "imp_private.inc"
+
 #include "normal.h"
-  )cpp";
+
+  )cpp");
   Inputs.ExtraFiles["normal.h"] = Inputs.ExtraFiles["detail1.h"] =
-  Inputs.ExtraFiles["detail2.h"] = "";
+  Inputs.ExtraFiles["detail2.h"] = guard("");
+  Inputs.ExtraFiles["detail3.h"] = guard(R"cpp(
+#include "imp3.inc"
+  )cpp");
+  Inputs.ExtraFiles["imp3.inc"] = "";
+  Inputs.ExtraFiles["imp.inc"] = "";
+  Inputs.ExtraFiles["imp_private.inc"] = R"cpp(
+// IWYU pragma: private, include "public2.h"
+  )cpp";
+
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
   auto &FM = SM.getFileManager();
@@ -60,20 +78,36 @@
 return SM.translateFileLineCol(FM.getFile(FileName).get(),
/*Line=*/1, /*Col=*/1);
   };
+  auto PhysicalHeader = [&](llvm::StringRef FileName) {
+return FM.getFile(FileName).get();
+  };
 
   EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, &PI),
-  UnorderedElementsAre(Header("\"path/public.h\"")));
+  UnorderedElementsAre(Header("\"path/public.h\""),
+   PhysicalHeader("header1.h")));
 
   EXPECT_THAT(findHeaders(SourceLocFromFile("detail1.h"), SM, &PI),
-  UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
-   Header(FM.getFile("detail1.h").get(;
+  UnorderedElementsAre(PhysicalHeader("header2.h"),
+   PhysicalHeader("detail1.h")));
   EXPECT_THAT(findHeaders(SourceLocFromFile("detail2.h"), SM, &PI),
-  UnorderedElementsAre(Header(FM.getFile("header2.h").get()),
-   Header(FM.getFile("detail2.h").get(;
-
+  UnorderedElementsAre(PhysicalHeader("header2.h"),
+   PhysicalHeader("detail2.h")));
+  // Verify that we emit the exporter for the details3.h.
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp3.inc"), SM, &PI),
+  UnorderedElementsAre(PhysicalHeader("imp3.inc"),
+   PhysicalHeader("detail3.h"),
+   PhysicalHeader("header2.h")));
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp.inc"), SM, &PI),
+  UnorderedElementsAre(PhysicalHeader("header2.h"),
+   PhysicalHeader("imp.inc")));
+  // There is a IWYU priviate mapping in the non self-contained header, verify
+  // that we don't emit its includer.
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp_private.inc"), SM, &PI),
+  UnorderedElementsAre(PhysicalHeader("imp_private.inc"),
+   Header("\"public2.h\"")));
   EXPECT_THAT(findHeaders(SourceLocFromFile("normal.h"), SM, &PI),
-  UnorderedElementsAre(Header(FM.getFile("normal.h").get(;
+  UnorderedElementsAre(PhysicalHeader("normal.h")));
 }
 
 } // namespace
 } // namespace clang::include_cleaner
\ No newline at end of file
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/A

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

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



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39
 FileID FID = SM.getFileID(Loc.physical());
-const auto *FE = SM.getFileEntryForID(FID);
+const auto *FE = getResponsibleHeader(FID, SM, PI);
 if (!FE)

hokein wrote:
> kadircet wrote:
> > i am not sure if sequencing of this and rest of the IWYU pragma handling is 
> > the right actually.
> > 
> > e.g. consider a scenario in which:
> > private.h:
> > ```
> > // IWYU private, include "public.h"
> > void foo();
> > ```
> > private-impl.h:
> > ```
> > #pragma once
> > #include "private.h"
> > ```
> > 
> > we'll actually now use `private-impl.h` as provider for `foo` rather than 
> > `public.h`, a similar argument goes for export pragma. i think intent of 
> > the developer is a lot more explicit when we have these pragmas around, so 
> > we should prioritize them and then pick the self contained one, if it's 
> > still needed. WDYT?
> It looks reasonable to me. Thanks for raising this case, this is a good 
> point. I mostly considered the case where the self-contained file has the 
> IWYU private mapping.
> 
> My current thought is:
> 
> Overall algorithm: we check the original `FE` to see whether we can get any 
> IWYU mapping (private, export) headers, if there is any, we use these mapping 
> headers as final results; if not, we use the `selfContainedHeader(FE)` and 
> its IWYU mapping headers;
> 
> Below are the cases I considered -- 1), 2), 4) works fine, but the 3) is not 
> perfect (as we prioritize the IWYU mapping headers), but probably ok.
> 
> ```
>   // Case1: Private
>   // findHeaders("private1.inc") => "path/public1.h"
>   Inputs.ExtraFiles["private1.h"] = R"cpp(
> #pragma once
> #include "private1.inc"
>   )cpp");
>   Inputs.ExtraFiles["private1.inc"] = R"cpp(
> // IWYU pragma: private, include "path/public1.h"
>   )cpp";
> 
>   // Case2: Private
>   // findHeaders("private2.inc") => "path/public2.h"
>   Inputs.ExtraFiles["private2.h"] = R"cpp(
> #pragma once
> // IWYU pragma: private, include "path/public2.h"
> #include "private2.inc"
>   )cpp";
>   Inputs.ExtraFiles["private2.inc"] = "";
> 
>   // Case3: Export
>   // findHeaders("details1.inc") => "details1.inc", "export1.h"
>   Inputs.ExtraFiles["export1.h"] = R"cpp(
> #include "details1.inc" // IWYU pragma: export
>   )cpp";
>   Inputs.ExtraFiles["details1.inc"] = "";
> 
>   // Case4: Export
>   // findHeaders("details2.inc") => "export2.h", "export2_internal.h"
>   Inputs.ExtraFiles["export2.h"] =R"cpp(
> #pragma once
> #include "export2_internal.h" // IWYU pragma: export
>   )cpp";
>   Inputs.ExtraFiles["export2_internal.h"] = R"cpp(
> #pragma once
> #include "details2.inc"
>   )cpp";
>   Inputs.ExtraFiles["details2.inc"] = "";
> ```
> 
> Let me know what you think about it.
> 
based on our offline discussion, we're going to preserve all header candidates 
here (and with proper private/export signals attached, which will be in in a 
followup patch), and let the later step to select proper headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

[PATCH] D137885: [modules] Support zstd in .pcm file

2022-11-18 Thread Thorsten via Phabricator via cfe-commits

tschuett added a comment.

Release note?




Comment at: clang/lib/Serialization/ASTReader.cpp:1457
+  const llvm::compression::Format F =
+  Blob.size() >= 2 && memcmp(Blob.data(), "\x1f\x8b", 2) == 0
+  ? llvm::compression::Format::Zlib

Could you put the magic number into a named constant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137885

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

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12
 
   LINK_LIBS
   clangAST

Note that this file changed a bit further in 
68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase again.

(I tested this patch after rebasing, in a mingw+dylib build configuration, and 
it still builds correctly there.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

[PATCH] D138207: [clang][Parse][NFC] Remove unused CommaLocs parameters

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder closed this revision.
tbaeder added a comment.

Dang, looks like I forgot to mention the review in 
https://github.com/llvm/llvm-project/commit/e78a43dacafb8b97fc377a52f8f206e37d94d5d6.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138207

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

[clang] f102fe7 - Revert "Revert "[opt][clang] Enable using -module-summary/-flto=thin with -S/-emit-llvm""

2022-11-18 Thread Alexander Shaposhnikov via cfe-commits


Author: Alexander Shaposhnikov
Date: 2022-11-18T08:58:31Z
New Revision: f102fe73044062542f022182381c17172a999db2

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

LOG: Revert "Revert "[opt][clang] Enable using -module-summary/-flto=thin with 
-S/-emit-llvm""

This reverts commit 7f608a2497c7578b9f3ca98014176ab95cffe3c0
and removes the dependency of Object on IRPrinter.

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/test/CodeGen/split-lto-unit.c
llvm/include/llvm/IRPrinter/IRPrintingPasses.h
llvm/lib/IRPrinter/CMakeLists.txt
llvm/lib/IRPrinter/IRPrintingPasses.cpp
llvm/test/Bitcode/thinlto-function-summary.ll
llvm/tools/opt/NewPMDriver.cpp
utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 9c677140b4a5d..d6a7a9800ec58 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -983,19 +983,24 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
 MPM.addPass(VerifierPass());
 
-  switch (Action) {
-  case Backend_EmitBC:
+  if (Action == Backend_EmitBC || Action == Backend_EmitLL) {
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
-  if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
-ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
-if (!ThinLinkOS)
-  return;
-  }
   if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
 TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
  CodeGenOpts.EnableSplitLTOUnit);
-  MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &ThinLinkOS->os()
-   : nullptr));
+  if (Action == Backend_EmitBC) {
+if (!CodeGenOpts.ThinLinkBitcodeFile.empty()) {
+  ThinLinkOS = openOutputFile(CodeGenOpts.ThinLinkBitcodeFile);
+  if (!ThinLinkOS)
+return;
+}
+MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? 
&ThinLinkOS->os()
+ : nullptr));
+  } else {
+MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists,
+/*EmitLTOSummary=*/true));
+  }
+
 } else {
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
@@ -1007,17 +1012,13 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
uint32_t(1));
   }
-  MPM.addPass(
-  BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));
+  if (Action == Backend_EmitBC)
+MPM.addPass(BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists,
+  EmitLTOSummary));
+  else
+MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists,
+EmitLTOSummary));
 }
-break;
-
-  case Backend_EmitLL:
-MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists));
-break;
-
-  default:
-break;
   }
 
   // Now that we have all of the passes ready, run them.

diff  --git a/clang/test/CodeGen/split-lto-unit.c 
b/clang/test/CodeGen/split-lto-unit.c
index 941aebafd01b3..b1560b61f3e30 100644
--- a/clang/test/CodeGen/split-lto-unit.c
+++ b/clang/test/CodeGen/split-lto-unit.c
@@ -1,12 +1,15 @@
 // ; Check that -flto=thin without -fsplit-lto-unit has EnableSplitLTOUnit = 0
 // RUN: %clang_cc1 -flto=thin -emit-llvm-bc < %s | llvm-dis -o - | FileCheck %s
+// RUN: %clang_cc1 -flto=thin -emit-llvm < %s | FileCheck %s
 // CHECK: !{i32 1, !"EnableSplitLTOUnit", i32 0}
 //
 // ; Check that -flto=thin with -fsplit-lto-unit has EnableSplitLTOUnit = 1
 // RUN: %clang_cc1 -flto=thin -fsplit-lto-unit -emit-llvm-bc < %s | llvm-dis 
-o - | FileCheck %s --check-prefix=SPLIT
+// RUN: %clang_cc1 -flto=thin -fsplit-lto-unit -emit-llvm < %s | FileCheck %s 
--check-prefix=SPLIT
 // SPLIT: !{i32 1, !"EnableSplitLTOUnit", i32 1}
 //
 // ; Check that regular LTO has EnableSplitLTOUnit = 1
 // RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm-bc < %s | 
llvm-dis -o - | FileCheck %s --implicit-check-not="EnableSplitLTOUnit" 
--check-prefix=SPLIT
+// RUN: %clang_cc1 -flto -triple x86_64-pc-linux-gnu -emit-llvm < %s | 
FileCheck %s --implicit-check-not="EnableSplitLTOUnit" --check-prefix=SPLIT
 
 int main(void) {}

diff  --git a/llvm/include/llvm/IRPrinter/IRPrintingPasses.h 
b/llvm/include/llvm/IRPrinter/IRPrintingPasses.h
index 6cfa7a87af933..8eb7973365172 100644

[PATCH] D137768: [opt][clang] Enable using -module-summary /-flto=thin with -S / -emit-llvm

2022-11-18 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexander-shaposhnikov added a comment.

Recommitted  this diff in 
https://github.com/llvm/llvm-project/commit/f102fe73044062542f022182381c17172a999db2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137768

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

[PATCH] D138143: [FPEnv] Enable strict fp for AArch64 in clang

2022-11-18 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits

ostannard added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:760
+- Strict floating point has been enabled for AArch64, which means that
+  ``-ftrapping-math``, ``-frounding-math``, ``-ffp-model=strict``, and
+  ``-ffp-exception-behaviour=`` are now accepted.

Do we need tests for these command-line options? I only see tests for 
-ffp-exception-behavior= in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138143

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

[clang] c8ecbaa - [clang] Fix assert message

2022-11-18 Thread via cfe-commits


Author: serge-sans-paille
Date: 2022-11-18T10:10:42+01:00
New Revision: c8ecbaa2eba9de4338642bbfb6efd75b286878cb

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

LOG: [clang] Fix assert message

Added: 


Modified: 
clang/lib/Lex/Lexer.cpp

Removed: 




diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index b6ffb85cd2fa6..d45a6fb0a2f84 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -3517,8 +3517,8 @@ bool Lexer::Lex(Token &Result) {
 /// that the Flags of result have been cleared before calling this.
 bool Lexer::LexTokenInternal(Token &Result, bool TokAtPhysicalStartOfLine) {
 LexStart:
-  assert(!Result.needsCleaning() && "Result doesn't need cleaning");
-  assert(!Result.hasPtrData() && "Result has been reset");
+  assert(!Result.needsCleaning() && "Result needs cleaning");
+  assert(!Result.hasPtrData() && "Result has not been reset");
 
   // CurPtr - Cache BufferPtr in an automatic variable.
   const char *CurPtr = BufferPtr;



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

[PATCH] D137960: [Lexer] Speedup LexTokenInternal

2022-11-18 Thread serge via Phabricator via cfe-commits

serge-sans-paille added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:3520-3521
+LexStart:
+  assert(!Result.needsCleaning() && "Result doesn't need cleaning");
+  assert(!Result.hasPtrData() && "Result has been reset");
 

foad wrote:
> Messages are backwards. They should be like "Result needs cleaning!" and 
> "Result has not been reset!".
Fixed as c8ecbaa2eba9de4338642bbfb6efd75b286878cb. Thanks for pointing that out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137960

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

[PATCH] D136565: [clang] Instantiate alias templates with sugar

2022-11-18 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added a comment.

And another problem with this patch: there's another pattern (or multiple 
different patterns?) in the code, that result in around 3x clang memory usage 
increase after this patch. The result of `-print-stats` doesn't make it clear 
where the additional allocations come from:

  --- at_culprit2022-11-10 13:16:52.0 +0100
  +++ before_culprit2022-11-10 13:21:44.0 +0100
  @@ -1,340 +1,338 @@
  -
   STATISTICS:
   
   *** Semantic Analysis Stats:
   0 SFINAE diagnostics trapped.
   
   Number of memory regions: 600
   Bytes used: 13579328
   Bytes allocated: 13631488
   Bytes wasted: 52160 (includes alignment, etc)
   
   *** Analysis Based Warnings Stats:
   168155 functions analyzed (0 w/o CFGs).
 697398 CFG blocks built.
 4 average CFG blocks per function.
 503 max CFG blocks per function.
   0 functions analyzed for uninitialiazed variables
 0 variables analyzed.
 0 average variables per function.
 0 max variables per function.
 0 block visits.
 0 average block visits per function.
 0 max block visits per function.
   
   *** AST Context Stats:
  -  19535407 types total.
  +  19808684 types total.
   30 Decayed types, 48 each (1440 bytes)
   585 ConstantArray types, 56 each (32760 bytes)
   2792 DependentSizedArray types, 64 each (178688 bytes)
  -96 IncompleteArray types, 40 each (3840 bytes)
  +90 IncompleteArray types, 40 each (3600 bytes)
   38 Atomic types, 40 each (1520 bytes)
   32 Attributed types, 48 each (1536 bytes)
   62 Builtin types, 24 each (1488 bytes)
   1 Complex types, 40 each (40 bytes)
   64274 Decltype types, 40 each (2570960 bytes)
   17491 Auto types, 48 each (839568 bytes)
   114 DeducedTemplateSpecialization types, 48 each (5472 bytes)
  -798961 DependentName types, 48 each (38350128 bytes)
  -527 DependentTemplateSpecialization types, 48 each (25296 bytes)
  -5241210 Elaborated types, 48 each (251578080 bytes)
  -588059 FunctionProto types, 40 each (23522360 bytes)
  +798160 DependentName types, 48 each (38311680 bytes)
  +467 DependentTemplateSpecialization types, 48 each (22416 bytes)
  +4735944 Elaborated types, 48 each (227325312 bytes)
  +585169 FunctionProto types, 40 each (23406760 bytes)
   5631 InjectedClassName types, 40 each (225240 bytes)
   4684 MemberPointer types, 48 each (224832 bytes)
   2 ObjCObjectPointer types, 40 each (80 bytes)
   2 ObjCObject types, 40 each (80 bytes)
   1 ObjCInterface types, 48 each (48 bytes)
  -30517 PackExpansion types, 40 each (1220680 bytes)
  -2700 Paren types, 40 each (108000 bytes)
  -393866 Pointer types, 40 each (15754640 bytes)
  -208529 LValueReference types, 40 each (8341160 bytes)
  -176267 RValueReference types, 40 each (7050680 bytes)
  -814 SubstTemplateTypeParmPack types, 48 each (39072 bytes)
  -3432531 SubstTemplateTypeParm types, 40 each (137301240 bytes)
  +30567 PackExpansion types, 40 each (1222680 bytes)
  +2694 Paren types, 40 each (107760 bytes)
  +395432 Pointer types, 40 each (15817280 bytes)
  +207973 LValueReference types, 40 each (8318920 bytes)
  +178946 RValueReference types, 40 each (7157840 bytes)
  +764 SubstTemplateTypeParmPack types, 48 each (36672 bytes)
  +4213429 SubstTemplateTypeParm types, 40 each (168537160 bytes)
   1596 Enum types, 32 each (51072 bytes)
   577336 Record types, 32 each (18474752 bytes)
   5892736 TemplateSpecialization types, 40 each (235709440 bytes)
   1776985 TemplateTypeParm types, 40 each (71079400 bytes)
   2 TypeOfExpr types, 32 each (64 bytes)
   188177 Typedef types, 40 each (7527080 bytes)
   124634 UnaryTransform types, 48 each (5982432 bytes)
  -4068 Using types, 40 each (162720 bytes)
  +1787 Using types, 40 each (71480 bytes)
   57 Vector types, 40 each (2280 bytes)
  -Total bytes = 826368168
  +Total bytes = 833249832
   5151/505661 implicit default constructors created
   21741/525877 implicit copy constructors created
   18531/522651 implicit move constructors created
   9247/526033 implicit copy assignment operators created
   5875/517861 implicit move assignment operators created
   23158/524964 implicit destructors created
   
  -Number of memory regions: 1846
  -Bytes used: 11780764491
  -Bytes allocated: 12214755831
  -Bytes wasted: 433991340 (includes alignment, etc)
  +Number of memory regions: 1697
  +Bytes used: 4995782175
  +Bytes allocated: 5403406263
  +Bytes wasted: 407624088 (includes alignment, etc)
   
   *** Decl Stats:
 4825242 decls total.
   21328 AccessSpec decls, 40 each (853120 bytes)
   2 Empty decls, 40 each (80 bytes)
   1 ExternCContext decls, 72 each (72 bytes)
   4 FileScopeAsm decls, 56 each (224 bytes)
   7779 Friend decls, 64 each (497856 bytes)
   6 LifetimeExtendedTemporary decls, 72 each (432 bytes)
   1721 Linkage

[PATCH] D138270: [clang][Sema] Skip checking int expressions for overflow in constexpr functions

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

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

I'm not 100% sure if this is crazy or not, but it doesn't even seem to break 
any existing test cases.

We evaluate all integer expressions for overflows and diagnose them if 
possible. However, in the case of a constexpr function, we will evaluate the 
function (and all expressions it contains) anyway after parsing it, which will 
check for overflow again, so the previous checking is unnecessary.

I checked that not calling `CheckForIntOverflow()` at all breaks tests (it 
does).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138270

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15542,11 +15542,14 @@
   bool IsConstexpr) {
   llvm::SaveAndRestore ConstantContext(
   isConstantEvaluatedOverride, IsConstexpr || isa(E));
+
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
 CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+
+  if (!IsConstexpr && !E->isValueDependent() && !curFunctionDeclIsConstexpr())
 CheckForIntOverflow(E);
+
   DiagnoseMisalignedMembers();
 }
 
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1438,7 +1438,7 @@
 // Helper functions.
 
//===--===//
 
-DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) {
+DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) const {
   DeclContext *DC = CurContext;
 
   while (true) {
@@ -1458,7 +1458,7 @@
 /// getCurFunctionDecl - If inside of a function body, this returns a pointer
 /// to the function decl for the function being parsed.  If we're currently
 /// in a 'block', this returns the containing context.
-FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) {
+FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) const {
   DeclContext *DC = getFunctionLevelDeclContext(AllowLambda);
   return dyn_cast(DC);
 }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3503,13 +3503,18 @@
   void ActOnExitFunctionContext();
 
   /// If \p AllowLambda is true, treat lambda as function.
-  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false);
+  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false) const;
 
   /// Returns a pointer to the innermost enclosing function, or nullptr if the
   /// current context is not inside a function. If \p AllowLambda is true,
   /// this can return the call operator of an enclosing lambda, otherwise
   /// lambdas are skipped when looking for an enclosing function.
-  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false);
+  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false) const;
+
+  bool curFunctionDeclIsConstexpr() const {
+const FunctionDecl *FD = getCurFunctionDecl();
+return FD ? FD->isConstexpr() : false;
+  }
 
   /// getCurMethodDecl - If inside of a method body, this returns a pointer to
   /// the method decl for the method being parsed.  If we're currently


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15542,11 +15542,14 @@
   bool IsConstexpr) {
   llvm::SaveAndRestore ConstantContext(
   isConstantEvaluatedOverride, IsConstexpr || isa(E));
+
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
 CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+
+  if (!IsConstexpr && !E->isValueDependent() && !curFunctionDeclIsConstexpr())
 CheckForIntOverflow(E);
+
   DiagnoseMisalignedMembers();
 }
 
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1438,7 +1438,7 @@
 // Helper functions.
 //===--===//
 
-DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) {
+DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) const {
   DeclContext *DC = CurContext;
 
   while (true) {
@@ -1458,7 +1458,7 @@
 /// getCurFunctionDecl - If inside of a function body, this returns a pointer
 /// to the function decl for the function being parsed.  If we're currently
 /// in a 'b

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

thanks, mostly LG a bunch of suggestions for tests




Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:74
+  /// Returns true if the given file is a self-contained file.
+  bool isSelfContained(const FileEntry *File) const;
+

we also need tests for this



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:24
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!PI && FE)
+  return {Header(FE)};

can we rather write this as:
```
if (!PI)
  return FE ? {Header(FE)} : {};
```

i think it makes it more clear that in the absence of PI we're just terminating 
the traversal no matter what (rather than getting into the code below with 
possibly a null PI, because FE also happened to be null).



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:30
   // FIXME: compute transitive exporter headers.
   for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
 Results.push_back(Export);

nit: `Results.append(PI->getExporters(FE, SM.getFileManager()))`



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:85
 
   EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, &PI),
+  UnorderedElementsAre(Header("\"path/public.h\""),

tests and assertions here are getting a little bit hard to follow. do you mind 
splitting them into smaller chunks instead? each testing different parts of the 
traversal behaviour. one for private->public mappings, another for exports, 
another for non-self contained traversal. having another big integration test 
for testing each might also be fine, but it's really hard to reason about so i 
don't think that should be the main test in which we're testing the behaviour.

apart from that this might be easier to follow if we did some renaming:
- header1.h -> private1.h
- header2.h -> exporter.h (also probably move non-exported includes to a 
different header)
- detailx.h -> exportedx.h
- impx.inc -> fragmentx.inc



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:95
+   PhysicalHeader("detail2.h")));
+  // Verify that we emit the exporter for the details3.h.
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp3.inc"), SM, &PI),

`Verify that we emit exporters for each header on the path.`



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:105
+  // that we don't emit its includer.
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp_private.inc"), SM, &PI),
+  UnorderedElementsAre(PhysicalHeader("imp_private.inc"),

i think it would be better to check we stop traversal once we hit the pragma 
(i.e. have a non-self contained file included by another non-self contained 
file that also has a IWYU private mapping).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

[PATCH] D138258: clang/cmake: Fix incorrectly disabling tests when LLVM_EXTERNAL_LIT is used

2022-11-18 Thread Konrad Wilhelm Kleine via Phabricator via cfe-commits

kwk requested changes to this revision.
kwk added a comment.
This revision now requires changes to proceed.

As much as I would like this to be fixed. I vote against this patch because in 
`lld/CMakeLists.txt` there's an almost (if not entirely) identical piece of 
code 

 that screams to be outsourced into a `/cmake/Modules/FindLit.cmake` (to be 
created). I'll have a look at this and see if I can come up with a patch for 
this. Afterall `/cmake` is the central place to distribute share CMake code 
between subprojects, right @phosek (didn't you create `/cmake` in the first 
place?

Alternatively, we can copy the code from `lld/CMakeLists.txt` and outsource it 
afterwards so that the shared code becomes more obvious.

If there're any objections regarding outsourcing to `FindLit.cmake`, then I 
would only ask to copy the code from `lld/CMakeLists.txt`. There the 
`find_program` part sits below the if block.

Comment at: clang/CMakeLists.txt:97
 # Seek installed Lit.
-find_program(LLVM_LIT
- NAMES llvm-lit lit.py lit
- PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
- DOC "Path to lit.py")
+if (NOT LLVM_EXTERNAL_LIT)
+  find_program(LLVM_EXTERNAL_LIT

mgorny wrote:
> I don't think you need to do this if you rename the var, `find_program()` 
> doesn't seem to do any searching when the var is already set.
> 
> I've tested with a dummy CMakeLists:
> 
> ```
> set(FOO /bin/true)
> find_program(FOO NAMES foo)
> message(FATAL_ERROR ${FOO})
> ```
> 
> gives `/bin/true` for me.
> I don't think you need to do this if you rename the var, `find_program()` 
> doesn't seem to do any searching when the var is already set.

This might be true but it is counter intuitive to assume that `find_program` 
does nothing. So for readability I'd keep the `if (NOT LLVM_EXTERNAL_LIT)`. I 
know it is not needed but with the `if` it won't look like 
`find_program(LLVM_EXTERNAL_LIT` is the only/central place to set the variable 
`LLVM_EXTERNAL_LIT`. With the guard around it, it becomes more obvious that 
this is more of  a fallback.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138258

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

[PATCH] D138219: [include-cleaner] Show includes matched by refs in HTML report. Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/ecee6869e37af3db28089b64d8dce806/ra

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Thanks, I like the idea of marking missing-include refs.

I haven't read the code yet, and below are all my findings when I played with 
the demo html (some of them are unrelated to the current patch, just want to 
dump all):

1. size_t shows duplicated entries, line 465
2. we are missing refs in some using declarations (line 31, line32), but line 
33 does have a link which seems weird
3. UI related:
  - it would be nice to highlight the whole line, if user clicks the line link 
from the shown-up panel;
  - for `Included path/to/header.h`, I think adding the `""`/`<>` around the 
spelling string will be nicer;
  - for main-file symbols, showing (`Header  ASTTests.cpp`) is suboptimal, but 
I don't have better solution;
4. The handling of std symbols seems inconsistent:
  - click on a vector `push_back` will give the mapping `vector` header;
  - click on the type `std::vector` will give the `iosfwd` header;
5. I was confused why the type `Annotation` has the `Included` file, but not 
the method call `.code()`, since both shown-up panel are showing 
`Annotations.h` header, then I realized that the `code` is from the base class 
`llvm::Annotation`. This brings up a policy question (Not sure it has been 
discussed before): If we have already included the header of a subclass, and we 
have some base-class method calls via the subclass object, do we need to insert 
the header of the base class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138219

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

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair updated this revision to Diff 476387.
al45tair marked 2 inline comments as done.
al45tair added a comment.

Updated following comments from @ahatanak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjC/objc-asm-attribute-test.m


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin 
%s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple 
x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,6 +11,10 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
@@ -59,6 +63,10 @@
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] 
c"optionalProp\00"
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x 
i8] c"T@,?,&\00"
+// CHECK: @"_OBJC_$_PROP_LIST_MySecretNamespace.Protocol2" ={{.*}} 
getelementptr inbounds ([13 x i8], [13 x i8]* @OBJC_PROP_NAME_ATTR_, i32 0, i32 
0),{{.*}} getelementptr inbounds ([7 x i8], [7 x i8]* @OBJC_PROP_NAME_ATTR_.11, 
i32 0, i32 0)
+
 // CHECK: private unnamed_addr constant [42 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] 
c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7865,6 +7865,7 @@
 /// kPropertyWeak = 'W'  // 'weak' property
 /// kPropertyStrong = 'P'// property GC'able
 /// kPropertyNonAtomic = 'N' // property non-atomic
+/// kPropertyOptional = '?'  // property optional
 /// };
 /// @endcode
 std::string
@@ -7890,6 +7891,9 @@
   // closely resembles encoding of ivars.
   getObjCEncodingForPropertyType(PD->getType(), S);
 
+  if (PD->isOptional())
+S += ",?";
+
   if (PD->isReadOnly()) {
 S += ",R";
 if (PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_copy)


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,6 +11,10 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
@@ -59,6 +63,10 @@
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] c"optionalProp\00"
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x i8] c"T@,?,&\00"
+// CHECK: @"_OBJC_$_PROP_LIST_MySecretNamespace.Protocol2" ={{.*}} getelementptr inbounds ([13 x i8], [13 x i8]* @OBJC_PROP_NAME_ATTR_, i32 0, i32 0),{{.*}} getelementptr inbounds ([7 x i8], [7 x i8]* @OBJC_PROP_NAME_ATTR_.11, i32 0, i32 0)
+
 // CHECK: private unnamed_addr constant [42 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7865,6 +7865,7 @@
 /// kPropertyWeak = 'W'  // 'weak' property
 /// kPropertyStrong = 'P'// property GC'able
 /// kPropertyNonAtomic = 'N' // property non-atomic
+/// kPropertyOptional = '?'  // property optional
 /// };
 /// @endcode
 std::string
@@ -7890,6

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

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



Comment at: clang/test/CodeGenObjC/objc-asm-attribute-test.m:20
 
+@class Message;
+

ahatanak wrote:
> Do you need this change?
No, I think this is left-over from an earlier revision. I'll get rid of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair added a comment.

@theraven Any chance you could glance over this and reassure us that it isn't 
going to break the GNU runtime if we do this? (We're adding an extra attribute 
in the property attribute string so that we can detect `@optional` properties 
in ObjC protocols at runtime.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D111566: [SYCL] Fix function pointer address space

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added inline comments.
Herald added a project: All.



Comment at: clang/lib/AST/ASTContext.cpp:11579
+unsigned ASTContext::getTargetAddressSpace(QualType T) const {
+  return T->isFunctionType() ? getTargetInfo().getProgramAddressSpace()
+ : getTargetAddressSpace(T.getQualifiers());

Can we add a DataLayout aware function to CodeGen instead? That would avoid the 
need for getProgramAddressSpace() in TargetInfo?



Comment at: clang/lib/Basic/TargetInfo.cpp:153
   MaxOpenCLWorkGroupSize = 1024;
+  ProgramAddrSpace = 0;
 }

A bit late to this review (only just noticed it while merging) - but I don't 
like that we end up duplicating even more DataLayout information here - while 
it only affects AVR upstream, downstream CHERI and Morello have to duplicate 
this to Arm/RISC-V/MIPS as well now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111566

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

[PATCH] D138270: [clang][Sema] Skip checking int expressions for overflow in constexpr functions

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 476391.

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

https://reviews.llvm.org/D138270

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/unittests/Support/TimeProfilerTest.cpp


Index: clang/unittests/Support/TimeProfilerTest.cpp
===
--- clang/unittests/Support/TimeProfilerTest.cpp
+++ clang/unittests/Support/TimeProfilerTest.cpp
@@ -178,9 +178,7 @@
   ASSERT_TRUE(TraceGraph == R"(
 Frontend
 | EvaluateAsRValue ()
-| EvaluateForOverflow ()
 | EvaluateAsRValue ()
-| EvaluateForOverflow ()
 | isPotentialConstantExpr (slow_namespace::slow_func)
 | EvaluateAsBooleanCondition ()
 | | EvaluateAsRValue ()
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15542,11 +15542,14 @@
   bool IsConstexpr) {
   llvm::SaveAndRestore ConstantContext(
   isConstantEvaluatedOverride, IsConstexpr || isa(E));
+
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
 CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+
+  if (!IsConstexpr && !E->isValueDependent() && !curFunctionDeclIsConstexpr())
 CheckForIntOverflow(E);
+
   DiagnoseMisalignedMembers();
 }
 
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1438,7 +1438,7 @@
 // Helper functions.
 
//===--===//
 
-DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) {
+DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) const {
   DeclContext *DC = CurContext;
 
   while (true) {
@@ -1458,7 +1458,7 @@
 /// getCurFunctionDecl - If inside of a function body, this returns a pointer
 /// to the function decl for the function being parsed.  If we're currently
 /// in a 'block', this returns the containing context.
-FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) {
+FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) const {
   DeclContext *DC = getFunctionLevelDeclContext(AllowLambda);
   return dyn_cast(DC);
 }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3503,13 +3503,18 @@
   void ActOnExitFunctionContext();
 
   /// If \p AllowLambda is true, treat lambda as function.
-  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false);
+  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false) const;
 
   /// Returns a pointer to the innermost enclosing function, or nullptr if the
   /// current context is not inside a function. If \p AllowLambda is true,
   /// this can return the call operator of an enclosing lambda, otherwise
   /// lambdas are skipped when looking for an enclosing function.
-  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false);
+  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false) const;
+
+  bool curFunctionDeclIsConstexpr() const {
+const FunctionDecl *FD = getCurFunctionDecl();
+return FD ? FD->isConstexpr() : false;
+  }
 
   /// getCurMethodDecl - If inside of a method body, this returns a pointer to
   /// the method decl for the method being parsed.  If we're currently


Index: clang/unittests/Support/TimeProfilerTest.cpp
===
--- clang/unittests/Support/TimeProfilerTest.cpp
+++ clang/unittests/Support/TimeProfilerTest.cpp
@@ -178,9 +178,7 @@
   ASSERT_TRUE(TraceGraph == R"(
 Frontend
 | EvaluateAsRValue ()
-| EvaluateForOverflow ()
 | EvaluateAsRValue ()
-| EvaluateForOverflow ()
 | isPotentialConstantExpr (slow_namespace::slow_func)
 | EvaluateAsBooleanCondition ()
 | | EvaluateAsRValue ()
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15542,11 +15542,14 @@
   bool IsConstexpr) {
   llvm::SaveAndRestore ConstantContext(
   isConstantEvaluatedOverride, IsConstexpr || isa(E));
+
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
 CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+
+  if (!IsConstexpr && !E->isValueDependent() && !curFunctionDeclIsConstexpr())
 CheckForIntOverflow(E);
+
   DiagnoseMisalignedMembers();
 }
 
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1438,7 +1438,7 @@
 // Helper functions.
 //===

[clang] 326393a - [Driver] exclude recently added tests from Windows

2022-11-18 Thread Matt Jacobson via cfe-commits


Author: Matt Jacobson
Date: 2022-11-18T05:30:42-05:00
New Revision: 326393ae653189023b251f05009d86215ad30caf

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

LOG: [Driver] exclude recently added tests from Windows

Added: 
clang/test/Driver/freebsd-include-paths.c

Modified: 
clang/test/Driver/freebsd.c
clang/test/Driver/freebsd.cpp

Removed: 




diff  --git a/clang/test/Driver/freebsd-include-paths.c 
b/clang/test/Driver/freebsd-include-paths.c
new file mode 100644
index 0..872b2386ce9f1
--- /dev/null
+++ b/clang/test/Driver/freebsd-include-paths.c
@@ -0,0 +1,16 @@
+// UNSUPPORTED: system-windows
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
+
+// Check that the driver passes include paths to cc1 on FreeBSD in C++ mode.
+// RUN: %clang -### -xc++ %s --target=x86_64-unknown-freebsd13.1 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES-CXX
+// DRIVER-PASS-INCLUDES-CXX:  "-cc1" {{.*}}"-resource-dir" 
"[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-CXX-SAME: "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES-CXX-SAME: {{^}} "-internal-isystem" 
"[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES-CXX-SAME: {{^}} "-internal-externc-isystem" 
"/usr/include"

diff  --git a/clang/test/Driver/freebsd.c b/clang/test/Driver/freebsd.c
index 1091f3b0cff0d..87ce46d1c1c6e 100644
--- a/clang/test/Driver/freebsd.c
+++ b/clang/test/Driver/freebsd.c
@@ -214,9 +214,3 @@
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
 
-// Check that the driver passes include paths to cc1 on FreeBSD.
-// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
-// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"

diff  --git a/clang/test/Driver/freebsd.cpp b/clang/test/Driver/freebsd.cpp
index 6b1b09055ab84..56c3d3cba1fe5 100644
--- a/clang/test/Driver/freebsd.cpp
+++ b/clang/test/Driver/freebsd.cpp
@@ -40,11 +40,3 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" 
"[[SYSROOT]]usr/include/c++/v1"
-
-// Check that the driver passes include paths to cc1 on FreeBSD.
-// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
-// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "/usr/include/c++/v1"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" 
"[[RESOURCE]]{{/|}}include"
-// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"



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

[PATCH] D138274: Add version to all LLVM cmake package

2022-11-18 Thread Thomas Preud'homme via Phabricator via cfe-commits

thopre created this revision.
thopre added reviewers: smeenai, arsenm, stella.stamenova, stellaraccident, 
Ericson2314, pmccormick, beanz.
Herald added a reviewer: bollu.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini.
Herald added a reviewer: sscalpone.
Herald added projects: Flang, All.
thopre requested review of this revision.
Herald added subscribers: cfe-commits, stephenneuendorffer, nicolasvasilache, 
jdoerfert, wdng.
Herald added projects: clang, MLIR.

Add a version to non-LLVM cmake package so that users needing an exact
version match can use the version parameter to find_package. Also adjust
the find_package(LLVM) to use an exact version match as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138274

Files:
  clang/cmake/modules/CMakeLists.txt
  clang/cmake/modules/ClangConfig.cmake.in
  clang/cmake/modules/ClangConfigVersion.cmake.in
  flang/cmake/modules/CMakeLists.txt
  flang/cmake/modules/FlangConfig.cmake.in
  flang/cmake/modules/FlangConfigVersion.cmake.in
  lld/cmake/modules/CMakeLists.txt
  lld/cmake/modules/LLDConfig.cmake.in
  lld/cmake/modules/LLDConfigVersion.cmake.in
  mlir/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/MLIRConfig.cmake.in
  mlir/cmake/modules/MLIRConfigVersion.cmake.in
  polly/cmake/CMakeLists.txt
  polly/cmake/PollyConfig.cmake.in
  polly/cmake/PollyConfigVersion.cmake.in

Index: polly/cmake/PollyConfigVersion.cmake.in
===
--- /dev/null
+++ polly/cmake/PollyConfigVersion.cmake.in
@@ -0,0 +1,13 @@
+set(PACKAGE_VERSION "@PACKAGE_VERSION@")
+
+# LLVM is API-compatible only with matching major.minor versions
+# and patch versions not less than that requested.
+if("@LLVM_VERSION_MAJOR@.@LLVM_VERSION_MINOR@" VERSION_EQUAL
+"${PACKAGE_FIND_VERSION_MAJOR}.${PACKAGE_FIND_VERSION_MINOR}"
+   AND NOT "@LLVM_VERSION_PATCH@" VERSION_LESS "${PACKAGE_FIND_VERSION_PATCH}")
+  set(PACKAGE_VERSION_COMPATIBLE 1)
+  if("@LLVM_VERSION_PATCH@" VERSION_EQUAL
+  "${PACKAGE_FIND_VERSION_PATCH}")
+set(PACKAGE_VERSION_EXACT 1)
+  endif()
+endif()
Index: polly/cmake/PollyConfig.cmake.in
===
--- polly/cmake/PollyConfig.cmake.in
+++ polly/cmake/PollyConfig.cmake.in
@@ -2,7 +2,8 @@
 
 @POLLY_CONFIG_CODE@
 
-find_package(LLVM REQUIRED CONFIG
+set(LLVM_VERSION ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH})
+find_package(LLVM @LLVM_VERSION@ EXACT REQUIRED CONFIG
  HINTS "@POLLY_CONFIG_LLVM_CMAKE_DIR@")
 
 set(Polly_CMAKE_DIR ${CMAKE_CURRENT_LIST_DIR})
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -82,6 +82,10 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/PollyConfig.cmake.in
   ${polly_cmake_builddir}/PollyConfig.cmake
   @ONLY)
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/PollyConfigVersion.cmake.in
+  ${polly_cmake_builddir}/PollyConfigVersion.cmake
+  @ONLY)
 
 file(GENERATE
   OUTPUT ${polly_cmake_builddir}/${POLLY_EXPORTS_FILE_NAME}
@@ -138,6 +142,10 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/PollyConfig.cmake.in
   ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/PollyConfig.cmake
   @ONLY)
+configure_file(
+  ${CMAKE_CURRENT_SOURCE_DIR}/PollyConfigVersion.cmake.in
+  ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/PollyConfigVersion.cmake
+  @ONLY)
 file(GENERATE OUTPUT
   ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${POLLY_EXPORTS_FILE_NAME}
   CONTENT "${POLLY_EXPORTS}")
@@ -146,6 +154,7 @@
   install(
 FILES
 "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/PollyConfig.cmake"
+"${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/PollyConfigVersion.cmake"
 "${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/${POLLY_EXPORTS_FILE_NAME}"
 DESTINATION "${POLLY_INSTALL_PACKAGE_DIR}")
 endif ()
Index: mlir/cmake/modules/MLIRConfigVersion.cmake.in
===
--- /dev/null
+++ mlir/cmake/modules/MLIRConfigVersion.cmake.in
@@ -0,0 +1,13 @@
+set(PACKAGE_VERSION "@PACKAGE_VERSION@")
+
+# LLVM is API-compatible only with matching major.minor versions
+# and patch versions not less than that requested.
+if("@LLVM_VERSION_MAJOR@.@LLVM_VERSION_MINOR@" VERSION_EQUAL
+"${PACKAGE_FIND_VERSION_MAJOR}.${PACKAGE_FIND_VERSION_MINOR}"
+   AND NOT "@LLVM_VERSION_PATCH@" VERSION_LESS "${PACKAGE_FIND_VERSION_PATCH}")
+  set(PACKAGE_VERSION_COMPATIBLE 1)
+  if("@LLVM_VERSION_PATCH@" VERSION_EQUAL
+  "${PACKAGE_FIND_VERSION_PATCH}")
+set(PACKAGE_VERSION_EXACT 1)
+  endif()
+endif()
Index: mlir/cmake/modules/MLIRConfig.cmake.in
===
--- mlir/cmake/modules/MLIRConfig.cmake.in
+++ mlir/cmake/modules/MLIRConfig.cmak

[PATCH] D138275: [clang][Interp] Avoid leaking init maps of local primitive arrays

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This shows up when the interpretation is interrupted, in this case because 
we're reading an uninitialized value, and so we don't get to the `destroy` 
instruction for the scope.

This adds a `destroyAll()` that calls deallocates all the local variables of 
all the scopes in the function. This works, but I was wondering if it should 
instead be a `cleanupLeftovers()` that's only used by the caller _if_ the 
interpretation wasn't successful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138275

Files:
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/test/AST/Interp/cxx20.cpp


Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -304,3 +304,15 @@
   }
   static_assert(testInc2() == 1, "");
 };
+
+constexpr int NoLeakHere() {
+  int abc[2];
+
+  abc[0] = 1;
+  return abc[1]; // expected-note {{read of object outside its lifetime}} \
+ // ref-note {{read of uninitialized object}}
+}
+static_assert(NoLeakHere() == 3); // expected-error {{not an integral constant 
expression}} \
+  // expected-note {{in call to}} \
+  // ref-error {{not an integral constant 
expression}} \
+  // ref-note {{in call to}}
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -47,6 +47,11 @@
   /// Invokes the destructors for a scope.
   void destroy(unsigned Idx);
 
+  /// Invokes the destructors for all scopes. This is used in the
+  /// InterpFrame destructor to avoid memory leaks in case the
+  /// interpretation was not successful.
+  void destroyAll();
+
   /// Pops the arguments off the stack.
   void popArgs();
 
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -64,6 +64,7 @@
 }
 
 InterpFrame::~InterpFrame() {
+  destroyAll();
   for (auto &Param : Params)
 S.deallocate(reinterpret_cast(Param.second.get()));
 }
@@ -74,6 +75,16 @@
   }
 }
 
+void InterpFrame::destroyAll() {
+  if (!Func)
+return;
+  for (const Scope &Sc : Func->scopes()) {
+for (auto &Local : Sc.locals()) {
+  S.deallocate(reinterpret_cast(localBlock(Local.Offset)));
+}
+  }
+}
+
 void InterpFrame::popArgs() {
   for (PrimType Ty : Func->args_reverse())
 TYPE_SWITCH(Ty, S.Stk.discard());


Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -304,3 +304,15 @@
   }
   static_assert(testInc2() == 1, "");
 };
+
+constexpr int NoLeakHere() {
+  int abc[2];
+
+  abc[0] = 1;
+  return abc[1]; // expected-note {{read of object outside its lifetime}} \
+ // ref-note {{read of uninitialized object}}
+}
+static_assert(NoLeakHere() == 3); // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to}} \
+  // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -47,6 +47,11 @@
   /// Invokes the destructors for a scope.
   void destroy(unsigned Idx);
 
+  /// Invokes the destructors for all scopes. This is used in the
+  /// InterpFrame destructor to avoid memory leaks in case the
+  /// interpretation was not successful.
+  void destroyAll();
+
   /// Pops the arguments off the stack.
   void popArgs();
 
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -64,6 +64,7 @@
 }
 
 InterpFrame::~InterpFrame() {
+  destroyAll();
   for (auto &Param : Params)
 S.deallocate(reinterpret_cast(Param.second.get()));
 }
@@ -74,6 +75,16 @@
   }
 }
 
+void InterpFrame::destroyAll() {
+  if (!Func)
+return;
+  for (const Scope &Sc : Func->scopes()) {
+for (auto &Local : Sc.locals()) {
+  S.deallocate(reinterpret_cast(localBlock(Local.Offset)));
+}
+  }
+}
+
 void InterpFrame::popArgs() {
   for (PrimType Ty : Func->args_reverse())
 TYPE_SWITCH(Ty, S.Stk.discard());

[PATCH] D138276: TableGen: require tablegen cl::opts to be registered explicitly

2022-11-18 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle created this revision.
Herald added subscribers: libc-commits, Moerafaat, zero9178, bzcheeseman, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, hiraditya.
Herald added projects: libc-project, All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

We plan to include the TableGen library into the LLVM shared library.
This is a preparatory change to avoid polluting the command-line options
space of non-tablegen tools.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138276

Files:
  clang/utils/TableGen/TableGen.cpp
  libc/utils/HdrGen/Main.cpp
  libc/utils/HdrGen/PrototypeTestGen/PrototypeTestGen.cpp
  libc/utils/tools/WrapperGen/Main.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/include/llvm/TableGen/Main.h
  llvm/lib/TableGen/Main.cpp
  llvm/tools/llvm-shlib/CMakeLists.txt
  llvm/utils/TableGen/TableGen.cpp
  mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp

Index: mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
===
--- mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
+++ mlir/lib/Tools/mlir-tblgen/MlirTblgenMain.cpp
@@ -129,6 +129,7 @@
   llvm::cl::opt generator(
   "", llvm::cl::desc("Generator to run"), cl::location(::generator));
 
+  llvm::registerTableGenOptions();
   cl::ParseCommandLineOptions(argc, argv);
 
   return TableGenMain(argv[0], &mlirTableGenMain);
Index: llvm/utils/TableGen/TableGen.cpp
===
--- llvm/utils/TableGen/TableGen.cpp
+++ llvm/utils/TableGen/TableGen.cpp
@@ -286,6 +286,7 @@
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
+  registerTableGenOptions();
   cl::ParseCommandLineOptions(argc, argv);
 
   return TableGenMain(argv[0], &LLVMTableGenMain);
Index: llvm/tools/llvm-shlib/CMakeLists.txt
===
--- llvm/tools/llvm-shlib/CMakeLists.txt
+++ llvm/tools/llvm-shlib/CMakeLists.txt
@@ -19,7 +19,6 @@
 
   # Exclude libLLVMTableGen for the following reasons:
   #  - it is only used by internal *-tblgen utilities;
-  #  - it pollutes the global options space.
   list(REMOVE_ITEM LIB_NAMES "LLVMTableGen")
 
   if(LLVM_DYLIB_EXPORTED_SYMBOL_FILE)
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -26,36 +26,45 @@
 #include 
 using namespace llvm;
 
-static cl::opt
-OutputFilename("o", cl::desc("Output filename"), cl::value_desc("filename"),
-   cl::init("-"));
+namespace {
 
-static cl::opt
-DependFilename("d",
-   cl::desc("Dependency filename"),
-   cl::value_desc("filename"),
-   cl::init(""));
+struct TableGenOptions {
+  cl::opt OutputFilename{"o", cl::desc("Output filename"),
+  cl::value_desc("filename"),
+  cl::init("-")};
 
-static cl::opt
-InputFilename(cl::Positional, cl::desc(""), cl::init("-"));
+  cl::opt DependFilename{"d", cl::desc("Dependency filename"),
+  cl::value_desc("filename"), cl::init("")};
 
-static cl::list
-IncludeDirs("I", cl::desc("Directory of include files"),
-cl::value_desc("directory"), cl::Prefix);
+  cl::opt InputFilename{cl::Positional, cl::desc(""),
+ cl::init("-")};
 
-static cl::list
-MacroNames("D", cl::desc("Name of the macro to be defined"),
-cl::value_desc("macro name"), cl::Prefix);
+  cl::list IncludeDirs{"I", cl::desc("Directory of include files"),
+cl::value_desc("directory"), cl::Prefix};
 
-static cl::opt
-WriteIfChanged("write-if-changed", cl::desc("Only write output if it changed"));
+  cl::list MacroNames{"D",
+   cl::desc("Name of the macro to be defined"),
+   cl::value_desc("macro name"), cl::Prefix};
 
-static cl::opt
-TimePhases("time-phases", cl::desc("Time phases of parser and backend"));
+  cl::opt WriteIfChanged{"write-if-changed",
+   cl::desc("Only write output if it changed")};
 
-static cl::opt NoWarnOnUnusedTemplateArgs(
-"no-warn-on-unused-template-args",
-cl::desc("Disable unused template argument warnings."));
+  cl::opt TimePhases{"time-phases",
+   cl::desc("Time phases of parser and backend")};
+
+  cl::opt NoWarnOnUnusedTemplateArgs{
+  "no-warn-on-unused-template-args",
+  cl::desc("Disable unused template argument warnings.")};
+};
+
+TableGenOptions &getOpts() {
+  static TableGenOptions Opts;
+  return

[PATCH] D138278: TableGen: honor LLVM_LINK_LLVM_DYLIB by default

2022-11-18 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle created this revision.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, s.egerton, simoncook.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

Most TableGen tools have link dependency chains of the form

  ${project}-tblgen -> ${project}Support -> LLVMSupport

In LLVM_LINK_LLVM_DYLIB=ON builds, ${project}Support naturally links
aginst LLVMSupport implicitly by linking against libLLVM-*.so, and so
${project}-tblgen ends up with a dependency on the shared library.

Configuring the tool itself with DISABLE_LLVM_LINK_LLVM_DYLIB then
typically leads to LLVMSupport to be also included statically, leading
to duplicate definitions of symbols from LLVMSupport after the dynamic
linker has done its thing.

TableGen tools simply aren't that special: they can be linked dynamically
just fine, and so we do that to simplify the build setup.

The only exception to this rule is llvm-tblgen itself, which must be
statically linked to avoid circular dependencies of the form

  llvm-tblgen -> llvm-shlib -> tablegen-generated files -> llvm-tblgen


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138278

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt
  llvm/cmake/modules/CrossCompile.cmake
  llvm/cmake/modules/TableGen.cmake
  llvm/utils/TableGen/CMakeLists.txt
  mlir/lib/Support/CMakeLists.txt
  mlir/lib/TableGen/CMakeLists.txt
  mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
  mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
  mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt

Index: mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
===
--- mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
+++ mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
@@ -12,8 +12,6 @@
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/tblgen-lsp-server
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_LIBS PUBLIC
   MLIRLspServerSupportLib
   MLIRSupport
Index: mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
===
--- mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
+++ mlir/lib/Tools/mlir-tblgen/CMakeLists.txt
@@ -4,8 +4,6 @@
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/mlir-tblgen
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   TableGen
 
Index: mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
===
--- mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
+++ mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
@@ -4,8 +4,6 @@
   Lexer.cpp
   Parser.cpp
   
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   TableGen
Index: mlir/lib/TableGen/CMakeLists.txt
===
--- mlir/lib/TableGen/CMakeLists.txt
+++ mlir/lib/TableGen/CMakeLists.txt
@@ -1,14 +1,5 @@
-# This library is unusual, since mlir-tblgen depends on it, which is
-# built with DISABLE_LLVM_LINK_LLVM_DYLIB, this must also be built
-# with that option.  Otherwise builds with LLVM_BUILD_LLVM_DYLIB and
-# LLVM_LINK_LLVM_DYLIB fail.  (Note that even if this has no llvm
-# component dependencies, LLVM_LINK_LLVM_DYLIB tends to introduce a
-# dependence on libLLVM.so)  However, it must also be linkable against
-# libMLIR.so in some contexts (see unittests/Tablegen, for instance, which
-# has a dependence on MLIRIR, which must depend on libLLVM.so).  This works
-# in this special case because this library is static.
 
-llvm_add_library(MLIRTableGen STATIC
+llvm_add_library(MLIRTableGen
   Argument.cpp
   Attribute.cpp
   AttrOrTypeDef.cpp
@@ -30,8 +21,6 @@
   Trait.cpp
   Type.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/TableGen
 )
Index: mlir/lib/Support/CMakeLists.txt
===
--- mlir/lib/Support/CMakeLists.txt
+++ mlir/lib/Support/CMakeLists.txt
@@ -30,8 +30,6 @@
 add_llvm_library(MLIRSupportIndentedOstream
   IndentedOstream.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   )
Index: llvm/utils/TableGen/CMakeLists.txt
===
--- llvm/utils/TableGen/CMakeLists.txt
+++ llvm/utils/TableGen/CMakeLists.txt
@@ -3,6 +3,7 @@
 set(LLVM_LINK_COMPONENTS Support)
 
 add_tablegen(llvm-tblgen LLVM
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
   EXPORT LLVM
   AsmMatcherEmitter.cpp
Index: llvm/cmake/modules/TableGen.cmake

[PATCH] D137392: [clang][Interp] Explicitly handle RVO Pointer

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137392

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

[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136815

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

[PATCH] D136920: [clang][Interp] Array initialization via CXXConstructExpr

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136920

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

[PATCH] D137232: [clang][Interp] Support inc/dec operators on pointers

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D137232

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

[PATCH] D136457: [clang][Interp] Fix discarding non-primitive function call return values

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136457

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

[PATCH] D136694: [clang][Interp] Check that constructor calls initialize all record fields

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136694

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

[PATCH] D137235: [clang][Interp] Fix ImplicitValueInitExprs for pointer types

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137235

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D134859

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

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 476410.
hokein marked 2 inline comments as done.
hokein added a comment.

address comment: split tests, add a smoke test for 
PragmaInclude::isSelfContained.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/CMakeLists.txt
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -390,5 +390,24 @@
   EXPECT_TRUE(PI.getExporters(FM.getFile("bar.h").get(), FM).empty());
 }
 
+TEST_F(PragmaIncludeTest, SelfContained) {
+  Inputs.Code = R"cpp(
+  #include "guarded.h"
+ 
+  #include "unguarded.h"
+  )cpp";
+  Inputs.ExtraFiles["guarded.h"] = R"cpp(
+  #ifndef HEADER_H
+  #define HEADER_H
+
+  #endif HEADER_H
+  )cpp";
+  Inputs.ExtraFiles["unguarded.h"] = "";
+  TestAST Processed = build();
+  auto &FM = Processed.fileManager();
+  EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
+  EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,72 +8,148 @@
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang::include_cleaner {
 namespace {
 using testing::UnorderedElementsAre;
 
-TEST(FindIncludeHeaders, IWYU) {
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
+class FindHeadersTest : public testing::Test {
+protected:
   TestInputs Inputs;
   PragmaIncludes PI;
-  Inputs.MakeAction = [&PI] {
-struct Hook : public PreprocessOnlyAction {
-public:
-  Hook(PragmaIncludes *Out) : Out(Out) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-Out->record(CI);
-return true;
-  }
-
-  PragmaIncludes *Out;
+  std::unique_ptr AST;
+  FindHeadersTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public PreprocessOnlyAction {
+  public:
+Hook(PragmaIncludes *Out) : Out(Out) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  Out->record(CI);
+  return true;
+}
+
+PragmaIncludes *Out;
+  };
+  return std::make_unique(&PI);
 };
-return std::make_unique(&PI);
+  }
+  void buildAST() { AST = std::make_unique(Inputs); }
+
+  llvm::SmallVector findHeaders(llvm::StringRef FileName) {
+return include_cleaner::findHeaders(
+AST->sourceManager().translateFileLineCol(
+AST->fileManager().getFile(FileName).get(),
+/*Line=*/1, /*Col=*/1),
+AST->sourceManager(), &PI);
+  }
+  const FileEntry *physicalHeader(llvm::StringRef FileName) {
+return AST->fileManager().getFile(FileName).get();
   };
+};
 
+TEST_F(FindHeadersTest, IWYUPrivateToPublic) {
   Inputs.Code = R"cpp(
-#include "header1.h"
-#include "header2.h"
+#include "private.h"
   )cpp";
-  Inputs.ExtraFiles["header1.h"] = R"cpp(
+  Inputs.ExtraFiles["private.h"] = guard(R"cpp(
 // IWYU pragma: private, include "path/public.h"
+  )cpp");
+  buildAST();
+  EXPECT_THAT(findHeaders("private.h"),
+  UnorderedElementsAre(physicalHeader("private.h"),
+   Header("\"path/public.h\"")));
+}
+
+TEST_F(FindHeadersTest, IWYUExport) {
+  Inputs.Code = R"cpp(
+#include "exporter.h"
   )cpp";
-  Inputs.ExtraFiles["header2.h"] = R"cpp(
-#include "detail1.h" // IWYU pragma: export
+  Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+#include "exported1.h" // IWYU pragma: export
 
 // IWYU pragma: begin_exports
-#include "detail2.h"
+#include "exported2.h"
 // IWYU pragma: end_exports
 
 #include "normal.h"
+  )cpp");
+  Inputs.ExtraFiles["exported1.h"] = guard("");
+  Inputs.ExtraFiles["exported2.h"] = guard("");
+  Inputs.ExtraFiles["normal.h"] = guard("");
+
+  buildAST();
+  EXPECT_THAT(findHeaders("exported1.h"

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/CMakeLists.txt:12
 
   LINK_LIBS
   clangAST

mstorsjo wrote:
> Note that this file changed a bit further in 
> 68294afa0836bb62be921e2143d147cdfdc8ba70, so you may want to rebase again.
> 
> (I tested this patch after rebasing, in a mingw+dylib build configuration, 
> and it still builds correctly there.)
sure, and thank you very much for taking care of the mingw+dylib build!



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:24
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!PI && FE)
+  return {Header(FE)};

kadircet wrote:
> can we rather write this as:
> ```
> if (!PI)
>   return FE ? {Header(FE)} : {};
> ```
> 
> i think it makes it more clear that in the absence of PI we're just 
> terminating the traversal no matter what (rather than getting into the code 
> below with possibly a null PI, because FE also happened to be null).
I agree, we can do that, but but the sad bit is that we need to explicitly 
spell the return type in the conditional operator.



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:30
   // FIXME: compute transitive exporter headers.
   for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
 Results.push_back(Export);

kadircet wrote:
> nit: `Results.append(PI->getExporters(FE, SM.getFileManager()))`
the type of two container is different (Header vs FileEntry, I updated to 
explicitly spell the `Header` type in the push_back) -- if we want to get rid 
of the for-loop, we could use the `llvm::for_each`.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:85
 
   EXPECT_THAT(findHeaders(SourceLocFromFile("header1.h"), SM, &PI),
+  UnorderedElementsAre(Header("\"path/public.h\""),

kadircet wrote:
> tests and assertions here are getting a little bit hard to follow. do you 
> mind splitting them into smaller chunks instead? each testing different parts 
> of the traversal behaviour. one for private->public mappings, another for 
> exports, another for non-self contained traversal. having another big 
> integration test for testing each might also be fine, but it's really hard to 
> reason about so i don't think that should be the main test in which we're 
> testing the behaviour.
> 
> apart from that this might be easier to follow if we did some renaming:
> - header1.h -> private1.h
> - header2.h -> exporter.h (also probably move non-exported includes to a 
> different header)
> - detailx.h -> exportedx.h
> - impx.inc -> fragmentx.inc
that sounds fair enough, split into small tests.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:105
+  // that we don't emit its includer.
+  EXPECT_THAT(findHeaders(SourceLocFromFile("imp_private.inc"), SM, &PI),
+  UnorderedElementsAre(PhysicalHeader("imp_private.inc"),

kadircet wrote:
> i think it would be better to check we stop traversal once we hit the pragma 
> (i.e. have a non-self contained file included by another non-self contained 
> file that also has a IWYU private mapping).
I think this is covered by this case (though this case is quite simple) -- we 
started the traversing from the imp_private.inc, at the beginning we hit the 
the IWYU private mapping, we stop immediately. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2022-11-18 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D138263#3936007 , @owenpan wrote:

> I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, 
> then wouldn't it be better to do that?

But the `r_brace` has no `MatchingParen`, and I didn't want to go into that 
hole.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138263

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson created this revision.
arichardson added reviewers: erichkeane, rjmccall, aaron.ballman.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang language-level address spaces and LLVM pointer address spaces are
not the same thing (even though they will both have a numeric value of
zero in many cases). LangAS is a enum class to avoid implicit conversions,
but eba69b59d1a30dead07da2c279c8ecfd2b62ba9f 
 avoided 
the compiler error by
adding a static_cast<>. While touching this code, simplify it by using
CreatePointerBitCastOrAddrSpaceCast() which is already a no-op if the types
match.

This changes the code generation for spir64 to place the globals in
addrspace(4). I believe is correct, but it would be good for someone who is
familiar with the target to confirm.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138284

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenSYCL/unique_stable_name.cpp
  clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Index: clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE=addrspace(4) '
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE='
 
 
 template
@@ -38,7 +38,7 @@
   // Make sure the following 3 are the same between the host and device compile.
   // Note that these are NOT the same value as eachother, they differ by the
   // signature.
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
 }
Index: clang/test/CodeGenSYCL/unique_stable_name.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name.cpp
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple spir64-unknown-unknown-sycldevice -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
-// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr constant [[LAMBDA_K3_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ4mainEUlPZ4mainEUlvE_E_\00"
-// CHECK: @[[INT1:[^\w]+]] = private unnamed_addr constant [[INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSi\00"
-// CHECK: @[[STRING:[^\w]+]] = private unnamed_addr constant [[STRING_SIZE:\[[0-9]+ x i8\]]] c"_ZTSAppL_ZZ4mainE1jE_i\00",
-// CHECK: @[[INT2:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA_X:[^\w]+]] = private unnamed_addr constant [[LAMBDA_X_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE0_\00"
-// CHECK: @[[MACRO_Y:[^\w]+]] =  private unnamed_addr constant [[MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE1_\00"
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE2_\00", align 1
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE3_\00", align 1
-// CHECK: @[[MACRO_MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE4_\00"
-// CHECK: @[[MACRO_MACRO_Y:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE5_\00"
-// CHECK: @[[INT3:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA:[^\w]+]] = private unnamed_addr constant [[LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functio

[PATCH] D138143: [FPEnv] Enable strict fp for AArch64 in clang

2022-11-18 Thread John Brawn via Phabricator via cfe-commits

john.brawn added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:760
+- Strict floating point has been enabled for AArch64, which means that
+  ``-ftrapping-math``, ``-frounding-math``, ``-ffp-model=strict``, and
+  ``-ffp-exception-behaviour=`` are now accepted.

ostannard wrote:
> Do we need tests for these command-line options? I only see tests for 
> -ffp-exception-behavior= in this patch.
-ftrapping-math and -ffp-model=strict are both driver-only options that are 
converted into other cc1 options and tested in Driver/fp-model.c. 
-frounding-math is tested in a target-independent way in the 
CodeGen/fpconstrained* tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138143

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

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

thanks, lgtm!




Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47
+  }
+  void buildAST() { AST = std::make_unique(Inputs); }
+

nit: i'd actually rename this to `astWithIncludes` and take in a set of 
filenames, then set up `Inputs.Code` here, eg:
```
astWithIncludes({"foo.h", "bar.h"}); -> Inputs.Code = `#include 
"foo.h"\n#include "bar.h"`; AST = TestAST(Inputs);
```



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:100
+  UnorderedElementsAre(physicalHeader("normal.h")));
+}
+

also we should assert that there's nothing funky going on with `exporter.h` 
itself



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:128
+  buildAST();
+  // There is a IWYU priviate mapping in the non self-contained header, verify
+  // that we don't emit its includer.

s/priviate/private



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:400
+  Inputs.ExtraFiles["guarded.h"] = R"cpp(
+  #ifndef HEADER_H
+  #define HEADER_H

nit: might be easier to just have `#pragma once`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

Re: How can Autoconf help with the transition to stricter compilation defaults?

2022-11-18 Thread Paul Eggert via cfe-commits


On 11/17/22 13:35, Bruno Haible wrote:


Clang will surely not acquire knowledge about "every library", right,
only about the C library according to relevant standards (ISO C, POSIX)?


I don't know the Clang developers' plans. But if I wanted Clang to be 
picky then yes, I'd have it know about every library the program links 
to. It's some work but doable via techniques like -flto. Let's hope this 
isn't enabled by default.




There is a
documented way to ask Clang and GCC "assume a free-standing implementation,
not an environment that has the ISO C / POSIX / LSB / ... functions":
   1) -ffreestanding [1][2]
   2) -fno-builtin


Thanks, good suggestion. If this suffices to pacify Clang, we can use it 
in Autoconf: that is, we cause ./configure to link a simple program with 
-fno-builtin and if that works, then ./configure uses -fno-builtin when 
doing "does this function exist?" link-time tests.


This hack wouldn't suffice if Clang starts doing link-time type checking 
by default. However, link-time type checking doesn't seem to be on the 
Clang developers' to-do list, so we should be OK at least for now.

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

Re: How can Autoconf help with the transition to stricter compilation defaults?

2022-11-18 Thread Bruno Haible via cfe-commits

Paul Eggert wrote:
> > AC_CHECK_FUNC *should not*  just probe for linkability of a symbol
> 
> ... Autoconf cannot 
> be expected to know every signature of every function in every library.

Clang will surely not acquire knowledge about "every library", right,
only about the C library according to relevant standards (ISO C, POSIX)?

> ... In the meantime if 
> Clang becomes pickier by default it would be helpful if there were a 
> well-defined way to shut off Clang's pickiness.

To me it seems the problem is not Clang's pickiness, but rather the
assumptions that it makes about the target environment. There is a
documented way to ask Clang and GCC "assume a free-standing implementation,
not an environment that has the ISO C / POSIX / LSB / ... functions":
  1) -ffreestanding [1][2]
  2) -fno-builtin

Can we assume that these options will continue to work?

If so, all Autoconf needs to do is to pass these options to the compiler
in AC_CHECK_FUNC checks. Then we don't need to add knowledge about each
of the hundreds of standard libc functions into Autoconf.

Bruno

[1] https://clang.llvm.org/docs/ClangCommandLineReference.html
[2] https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/C-Dialect-Options.html

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

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 476425.
hokein marked 3 inline comments as done.
hokein added a comment.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/CMakeLists.txt
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -390,5 +390,21 @@
   EXPECT_TRUE(PI.getExporters(FM.getFile("bar.h").get(), FM).empty());
 }
 
+TEST_F(PragmaIncludeTest, SelfContained) {
+  Inputs.Code = R"cpp(
+  #include "guarded.h"
+
+  #include "unguarded.h"
+  )cpp";
+  Inputs.ExtraFiles["guarded.h"] = R"cpp(
+  #pragma once
+  )cpp";
+  Inputs.ExtraFiles["unguarded.h"] = "";
+  TestAST Processed = build();
+  auto &FM = Processed.fileManager();
+  EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
+  EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,72 +8,150 @@
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang::include_cleaner {
 namespace {
 using testing::UnorderedElementsAre;
 
-TEST(FindIncludeHeaders, IWYU) {
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
+class FindHeadersTest : public testing::Test {
+protected:
   TestInputs Inputs;
   PragmaIncludes PI;
-  Inputs.MakeAction = [&PI] {
-struct Hook : public PreprocessOnlyAction {
-public:
-  Hook(PragmaIncludes *Out) : Out(Out) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-Out->record(CI);
-return true;
-  }
-
-  PragmaIncludes *Out;
+  std::unique_ptr AST;
+  FindHeadersTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public PreprocessOnlyAction {
+  public:
+Hook(PragmaIncludes *Out) : Out(Out) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  Out->record(CI);
+  return true;
+}
+
+PragmaIncludes *Out;
+  };
+  return std::make_unique(&PI);
 };
-return std::make_unique(&PI);
+  }
+  void buildAST() { AST = std::make_unique(Inputs); }
+
+  llvm::SmallVector findHeaders(llvm::StringRef FileName) {
+return include_cleaner::findHeaders(
+AST->sourceManager().translateFileLineCol(
+AST->fileManager().getFile(FileName).get(),
+/*Line=*/1, /*Col=*/1),
+AST->sourceManager(), &PI);
+  }
+  const FileEntry *physicalHeader(llvm::StringRef FileName) {
+return AST->fileManager().getFile(FileName).get();
   };
+};
 
+TEST_F(FindHeadersTest, IWYUPrivateToPublic) {
   Inputs.Code = R"cpp(
-#include "header1.h"
-#include "header2.h"
+#include "private.h"
   )cpp";
-  Inputs.ExtraFiles["header1.h"] = R"cpp(
+  Inputs.ExtraFiles["private.h"] = guard(R"cpp(
 // IWYU pragma: private, include "path/public.h"
+  )cpp");
+  buildAST();
+  EXPECT_THAT(findHeaders("private.h"),
+  UnorderedElementsAre(physicalHeader("private.h"),
+   Header("\"path/public.h\"")));
+}
+
+TEST_F(FindHeadersTest, IWYUExport) {
+  Inputs.Code = R"cpp(
+#include "exporter.h"
   )cpp";
-  Inputs.ExtraFiles["header2.h"] = R"cpp(
-#include "detail1.h" // IWYU pragma: export
+  Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+#include "exported1.h" // IWYU pragma: export
 
 // IWYU pragma: begin_exports
-#include "detail2.h"
+#include "exported2.h"
 // IWYU pragma: end_exports
 
 #include "normal.h"
+  )cpp");
+  Inputs.ExtraFiles["exported1.h"] = guard("");
+  Inputs.ExtraFiles["exported2.h"] = guard("");
+  Inputs.ExtraFiles["normal.h"] = guard("");
+
+  buildAST();
+  EXPECT_THAT(findHeaders("exported1.h"),
+  UnorderedElementsAre(physicalHeader("exported1.h"),
+   physicalHeader("e

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:47
+  }
+  void buildAST() { AST = std::make_unique(Inputs); }
+

kadircet wrote:
> nit: i'd actually rename this to `astWithIncludes` and take in a set of 
> filenames, then set up `Inputs.Code` here, eg:
> ```
> astWithIncludes({"foo.h", "bar.h"}); -> Inputs.Code = `#include 
> "foo.h"\n#include "bar.h"`; AST = TestAST(Inputs);
> ```
I think hiding and synthesizing the code of main file inside a function will 
hurt the code readability (though the main-file code is boilerplate). As a 
reader, I will prefer much that the main code and the all #includes are defined 
in a single place which is inside the TEST_F. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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

[clang-tools-extra] 0cf8885 - [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2022-11-18T13:52:30+01:00
New Revision: 0cf888514454350cd97ab79cdb4a73e7f189eea0

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

LOG: [include-cleaner] Add self-contained file support for PragmaIncludes.

And use it findHeaders. findHeaders now finds all header candidates
given a symbol location (these headers will be attached with proper
signals, in a followup patch).

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/CMakeLists.txt
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
index 6a6e339a7f43f..d6a6a6aa417c5 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -70,6 +70,9 @@ class PragmaIncludes {
   llvm::SmallVector getExporters(const FileEntry *File,
 FileManager &FM) const;
 
+  /// Returns true if the given file is a self-contained file.
+  bool isSelfContained(const FileEntry *File) const;
+
 private:
   class RecordPragma;
   /// 1-based Line numbers for the #include directives of the main file that
@@ -94,11 +97,13 @@ class PragmaIncludes {
  llvm::SmallVector>
   IWYUExportBy;
 
+  /// Contains all non self-contained files detected during the parsing.
+  llvm::DenseSet NonSelfContainedFiles;
+
   /// Owns the strings.
   llvm::BumpPtrAllocator Arena;
 
   // FIXME: add support for clang use_instead pragma
-  // FIXME: add selfcontained file.
 };
 
 /// Recorded main-file parser events relevant to include-cleaner.

diff  --git a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt 
b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
index a71276f54b643..99f73836a5ad3 100644
--- a/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/lib/CMakeLists.txt
@@ -15,6 +15,7 @@ clang_target_link_libraries(clangIncludeCleaner
   clangAST
   clangBasic
   clangLex
+  clangToolingInclusions
   clangToolingInclusionsStdlib
   )
 

diff  --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp 
b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index c782ff89755cb..321d692d7a8a8 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -19,27 +19,30 @@ llvm::SmallVector findHeaders(const SymbolLocation 
&Loc,
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
 // FIXME: Handle macro locations.
-// FIXME: Handle non self-contained files.
 FileID FID = SM.getFileID(Loc.physical());
-const auto *FE = SM.getFileEntryForID(FID);
-if (!FE)
-  return {};
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!PI) {
+  return FE ? llvm::SmallVector{Header(FE)}
+: llvm::SmallVector();
+}
+while (FE) {
+  Results.push_back(Header(FE));
+  // FIXME: compute transitive exporter headers.
+  for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
+Results.push_back(Header(Export));
 
-Results = {Header(FE)};
-if (PI) {
-  // We treat the spelling header in the IWYU pragma as the final public
-  // header.
-  // FIXME: look for exporters if the public header is exported by another
-  // header.
   llvm::StringRef VerbatimSpelling = PI->getPublic(FE);
-  if (!VerbatimSpelling.empty())
-return {Header(VerbatimSpelling)};
+  if (!VerbatimSpelling.empty()) {
+Results.push_back(VerbatimSpelling);
+break;
+  }
+  if (PI->isSelfContained(FE) || FID == SM.getMainFileID())
+break;
 
-  // FIXME: compute transitive exporter headers.
-  for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
-Results.push_back(Export);
+  // Walkup the include stack for non self-contained headers.
+  FID = SM.getDecomposedIncludedLoc(FID).first;
+  FE = SM.getFileEntryForID(FID);
 }
-
 return Results;
   }
   case SymbolLocation::Standard: {

diff  --git a/clang-tools-extra/include-cleaner/lib/Record.cpp 
b/clang-tools-extra/include-cleaner/lib/Record.cpp
index 72db299114da8..16956b45198b6 1006

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-18 Thread Haojian Wu 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 rG0cf888514454: [include-cleaner] Add self-contained file 
support for PragmaIncludes. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/CMakeLists.txt
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -390,5 +390,21 @@
   EXPECT_TRUE(PI.getExporters(FM.getFile("bar.h").get(), FM).empty());
 }
 
+TEST_F(PragmaIncludeTest, SelfContained) {
+  Inputs.Code = R"cpp(
+  #include "guarded.h"
+
+  #include "unguarded.h"
+  )cpp";
+  Inputs.ExtraFiles["guarded.h"] = R"cpp(
+  #pragma once
+  )cpp";
+  Inputs.ExtraFiles["unguarded.h"] = "";
+  TestAST Processed = build();
+  auto &FM = Processed.fileManager();
+  EXPECT_TRUE(PI.isSelfContained(FM.getFile("guarded.h").get()));
+  EXPECT_FALSE(PI.isSelfContained(FM.getFile("unguarded.h").get()));
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,72 +8,150 @@
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang::include_cleaner {
 namespace {
 using testing::UnorderedElementsAre;
 
-TEST(FindIncludeHeaders, IWYU) {
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
+class FindHeadersTest : public testing::Test {
+protected:
   TestInputs Inputs;
   PragmaIncludes PI;
-  Inputs.MakeAction = [&PI] {
-struct Hook : public PreprocessOnlyAction {
-public:
-  Hook(PragmaIncludes *Out) : Out(Out) {}
-  bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-Out->record(CI);
-return true;
-  }
-
-  PragmaIncludes *Out;
+  std::unique_ptr AST;
+  FindHeadersTest() {
+Inputs.MakeAction = [this] {
+  struct Hook : public PreprocessOnlyAction {
+  public:
+Hook(PragmaIncludes *Out) : Out(Out) {}
+bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+  Out->record(CI);
+  return true;
+}
+
+PragmaIncludes *Out;
+  };
+  return std::make_unique(&PI);
 };
-return std::make_unique(&PI);
+  }
+  void buildAST() { AST = std::make_unique(Inputs); }
+
+  llvm::SmallVector findHeaders(llvm::StringRef FileName) {
+return include_cleaner::findHeaders(
+AST->sourceManager().translateFileLineCol(
+AST->fileManager().getFile(FileName).get(),
+/*Line=*/1, /*Col=*/1),
+AST->sourceManager(), &PI);
+  }
+  const FileEntry *physicalHeader(llvm::StringRef FileName) {
+return AST->fileManager().getFile(FileName).get();
   };
+};
 
+TEST_F(FindHeadersTest, IWYUPrivateToPublic) {
   Inputs.Code = R"cpp(
-#include "header1.h"
-#include "header2.h"
+#include "private.h"
   )cpp";
-  Inputs.ExtraFiles["header1.h"] = R"cpp(
+  Inputs.ExtraFiles["private.h"] = guard(R"cpp(
 // IWYU pragma: private, include "path/public.h"
+  )cpp");
+  buildAST();
+  EXPECT_THAT(findHeaders("private.h"),
+  UnorderedElementsAre(physicalHeader("private.h"),
+   Header("\"path/public.h\"")));
+}
+
+TEST_F(FindHeadersTest, IWYUExport) {
+  Inputs.Code = R"cpp(
+#include "exporter.h"
   )cpp";
-  Inputs.ExtraFiles["header2.h"] = R"cpp(
-#include "detail1.h" // IWYU pragma: export
+  Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+#include "exported1.h" // IWYU pragma: export
 
 // IWYU pragma: begin_exports
-#include "detail2.h"
+#include "exported2.h"
 // IWYU pragma: end_exports
 
 #include "normal.h"
+  )cpp");
+  Inputs.ExtraFiles["exported1.h"] = guard("");
+  Inputs.ExtraFiles["exported2.h"] = guard("");
+  Inputs.ExtraFiles["normal.h"] = guard("");
+
+  buildAST();
+  EXPECT_THAT(findHeaders

[clang-tools-extra] 3cf14a7 - [Support] Add support for attaching payloads to points and ranges

2022-11-18 Thread Tom Praschan via cfe-commits


Author: Tom Praschan
Date: 2022-11-18T15:00:23+01:00
New Revision: 3cf14a7bdce08f8fe03d987a2d99c6ea47c58c03

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

LOG: [Support] Add support for attaching payloads to points and ranges

This is useful where tests previously encoded information in the name
names of ranges and points. Currently, this is pretty limited because
names consist of only alphanumeric characters and '_'.

With this patch, we can keep the names simple and attach optional
payloads to ranges and points instead.

The new syntax should be fully backwards compatible (if I haven't missed
anything). I tested this against clangd unit tests and everything still passes.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/Annotations.cpp
clang-tools-extra/clangd/unittests/Annotations.h
llvm/include/llvm/Testing/Support/Annotations.h
llvm/lib/Testing/Support/Annotations.cpp
llvm/unittests/Support/AnnotationsTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/Annotations.cpp 
b/clang-tools-extra/clangd/unittests/Annotations.cpp
index edb0ea9a3af1b..32b916d54d870 100644
--- a/clang-tools-extra/clangd/unittests/Annotations.cpp
+++ b/clang-tools-extra/clangd/unittests/Annotations.cpp
@@ -13,41 +13,78 @@ namespace clang {
 namespace clangd {
 
 Position Annotations::point(llvm::StringRef Name) const {
-  return offsetToPosition(code(), Base::point(Name));
+  return pointWithPayload(Name).first;
+}
+
+std::pair
+Annotations::pointWithPayload(llvm::StringRef Name) const {
+  auto [BasePoint, Payload] = Base::pointWithPayload(Name);
+  return {offsetToPosition(code(), BasePoint), Payload};
 }
 
 std::vector Annotations::points(llvm::StringRef Name) const {
-  auto Offsets = Base::points(Name);
+  auto BasePoints = Base::points(Name);
 
   std::vector Ps;
-  Ps.reserve(Offsets.size());
-  for (size_t O : Offsets)
-Ps.push_back(offsetToPosition(code(), O));
+  Ps.reserve(BasePoints.size());
+  for (const auto Point : BasePoints)
+Ps.push_back(offsetToPosition(code(), Point));
+
+  return Ps;
+}
+
+std::vector>
+Annotations::pointsWithPayload(llvm::StringRef Name) const {
+  auto BasePoints = Base::pointsWithPayload(Name);
+
+  std::vector> Ps;
+  Ps.reserve(BasePoints.size());
+  for (const auto &[Point, Payload] : BasePoints)
+Ps.push_back({offsetToPosition(code(), Point), Payload});
 
   return Ps;
 }
 
-static clangd::Range toLSPRange(llvm::StringRef Code, Annotations::Range R) {
+static clangd::Range toLSPRange(llvm::StringRef Code,
+llvm::Annotations::Range R) {
   clangd::Range LSPRange;
   LSPRange.start = offsetToPosition(Code, R.Begin);
   LSPRange.end = offsetToPosition(Code, R.End);
   return LSPRange;
 }
 
-clangd::Range Annotations::range(llvm::StringRef Name) const {
-  return toLSPRange(code(), Base::range(Name));
+Range Annotations::range(llvm::StringRef Name) const {
+  return rangeWithPayload(Name).first;
 }
 
-std::vector Annotations::ranges(llvm::StringRef Name) const {
+std::pair
+Annotations::rangeWithPayload(llvm::StringRef Name) const {
+  auto [BaseRange, Payload] = Base::rangeWithPayload(Name);
+  return {toLSPRange(code(), BaseRange), Payload};
+}
+
+std::vector Annotations::ranges(llvm::StringRef Name) const {
   auto OffsetRanges = Base::ranges(Name);
 
   std::vector Rs;
   Rs.reserve(OffsetRanges.size());
-  for (Annotations::Range R : OffsetRanges)
+  for (const auto &R : OffsetRanges)
 Rs.push_back(toLSPRange(code(), R));
 
   return Rs;
 }
 
+std::vector>
+Annotations::rangesWithPayload(llvm::StringRef Name) const {
+  auto OffsetRanges = Base::rangesWithPayload(Name);
+
+  std::vector> Rs;
+  Rs.reserve(OffsetRanges.size());
+  for (const auto &[R, Payload] : OffsetRanges)
+Rs.push_back({toLSPRange(code(), R), Payload});
+
+  return Rs;
+}
+
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/unittests/Annotations.h 
b/clang-tools-extra/clangd/unittests/Annotations.h
index 9c296810e44dd..60a2d62dda9c3 100644
--- a/clang-tools-extra/clangd/unittests/Annotations.h
+++ b/clang-tools-extra/clangd/unittests/Annotations.h
@@ -27,10 +27,18 @@ class Annotations : public llvm::Annotations {
   using llvm::Annotations::Annotations;
 
   Position point(llvm::StringRef Name = "") const;
+  std::pair
+  pointWithPayload(llvm::StringRef Name = "") const;
   std::vector points(llvm::StringRef Name = "") const;
+  std::vector>
+  pointsWithPayload(llvm::StringRef Name = "") const;
 
   clangd::Range range(llvm::StringRef Name = "") const;
+  std::pair
+  rangeWithPayload(llvm::StringRef Name = "") const;
   std::vector ranges(llvm::StringRef Name = "") const;
+  std::vector>
+  ranges

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

2022-11-18 Thread Tom Praschan via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
tom-anders marked 6 inline comments as done.
Closed by commit rG3cf14a7bdce0: [Support] Add support for attaching payloads 
to points and ranges (authored by tom-anders).

Changed prior to commit:
  https://reviews.llvm.org/D137909?vs=476230&id=476429#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137909

Files:
  clang-tools-extra/clangd/unittests/Annotations.cpp
  clang-tools-extra/clangd/unittests/Annotations.h
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/unittests/Support/AnnotationsTest.cpp

Index: llvm/unittests/Support/AnnotationsTest.cpp
===
--- llvm/unittests/Support/AnnotationsTest.cpp
+++ llvm/unittests/Support/AnnotationsTest.cpp
@@ -12,6 +12,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::Pair;
 using ::testing::ResultOf;
 using ::testing::UnorderedElementsAre;
 
@@ -105,6 +106,38 @@
   ElementsAre(range(8, 9), range(7, 10), range(1, 10)));
 }
 
+TEST(AnnotationsTest, Payload) {
+  // // A single unnamed point or range with unspecified payload
+  EXPECT_THAT(llvm::Annotations("a$^b").pointWithPayload(), Pair(1u, ""));
+  EXPECT_THAT(llvm::Annotations("a$[[b]]cdef").rangeWithPayload(),
+  Pair(range(1, 2), ""));
+
+  // A single unnamed point or range with empty payload
+  EXPECT_THAT(llvm::Annotations("a$()^b").pointWithPayload(), Pair(1u, ""));
+  EXPECT_THAT(llvm::Annotations("a$()[[b]]cdef").rangeWithPayload(),
+  Pair(range(1, 2), ""));
+
+  // A single unnamed point or range with payload.
+  EXPECT_THAT(llvm::Annotations("a$(foo)^b").pointWithPayload(),
+  Pair(1u, "foo"));
+  EXPECT_THAT(llvm::Annotations("a$(foo)[[b]]cdef").rangeWithPayload(),
+  Pair(range(1, 2), "foo"));
+
+  // A single named point or range with payload
+  EXPECT_THAT(llvm::Annotations("a$name(foo)^b").pointWithPayload("name"),
+  Pair(1u, "foo"));
+  EXPECT_THAT(
+  llvm::Annotations("a$name(foo)[[b]]cdef").rangeWithPayload("name"),
+  Pair(range(1, 2), "foo"));
+
+  // Multiple named points with payload.
+  llvm::Annotations Annotated("a$p1(p1)^bcd$p2(p2)^123$p1^345");
+  EXPECT_THAT(Annotated.points(), IsEmpty());
+  EXPECT_THAT(Annotated.pointsWithPayload("p1"),
+  ElementsAre(Pair(1u, "p1"), Pair(7u, "")));
+  EXPECT_THAT(Annotated.pointWithPayload("p2"), Pair(4u, "p2"));
+}
+
 TEST(AnnotationsTest, Named) {
   // A single named point or range.
   EXPECT_EQ(llvm::Annotations("a$foo^b").point("foo"), 1u);
@@ -144,6 +177,8 @@
   EXPECT_DEATH(llvm::Annotations("ff[[fdfd"), "unmatched \\[\\[");
   EXPECT_DEATH(llvm::Annotations("ff[[fdjsfjd]]xxx]]"), "unmatched \\]\\]");
   EXPECT_DEATH(llvm::Annotations("ff$fdsfd"), "unterminated \\$name");
+  EXPECT_DEATH(llvm::Annotations("ff$("), "unterminated payload");
+  EXPECT_DEATH(llvm::Annotations("ff$name("), "unterminated payload");
 #endif
 }
 } // namespace
Index: llvm/lib/Testing/Support/Annotations.cpp
===
--- llvm/lib/Testing/Support/Annotations.cpp
+++ llvm/lib/Testing/Support/Annotations.cpp
@@ -28,27 +28,35 @@
 require(Assertion, Msg, Text);
   };
   llvm::Optional Name;
-  llvm::SmallVector, 8> OpenRanges;
+  llvm::Optional Payload;
+  llvm::SmallVector OpenRanges;
 
   Code.reserve(Text.size());
   while (!Text.empty()) {
 if (Text.consume_front("^")) {
-  Points[Name.value_or("")].push_back(Code.size());
+  All.push_back(
+  {Code.size(), size_t(-1), Name.value_or(""), Payload.value_or("")});
+  Points[Name.value_or("")].push_back(All.size() - 1);
   Name = llvm::None;
+  Payload = llvm::None;
   continue;
 }
 if (Text.consume_front("[[")) {
-  OpenRanges.emplace_back(Name.value_or(""), Code.size());
+  OpenRanges.push_back(
+  {Code.size(), size_t(-1), Name.value_or(""), Payload.value_or("")});
   Name = llvm::None;
+  Payload = llvm::None;
   continue;
 }
 Require(!Name, "$name should be followed by ^ or [[");
 if (Text.consume_front("]]")) {
   Require(!OpenRanges.empty(), "unmatched ]]");
-  Range R;
-  R.Begin = OpenRanges.back().second;
-  R.End = Code.size();
-  Ranges[OpenRanges.back().first].push_back(R);
+
+  const Annotation &NewRange = OpenRanges.back();
+  All.push_back(
+  {NewRange.Begin, Code.size(), NewRange.Name, NewRange.Payload});
+  Ranges[NewRange.Name].push_back(All.size() - 1);
+
   OpenRanges.pop_back();
   continue;
 }
@@ -56,6 +64,13 @@
   Name =
   Text.take_while([](char C) { return llvm::isAlnum(C) || C == '_'; });
   Text = Text.drop_front(Name->size());
+
+  if (Text.consume_front("(

[PATCH] D138287: [clang][RISCV] Drop caching from RVVType as it introduces data races

2022-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added reviewers: kito-cheng, ilya-biryukov.
Herald added subscribers: sunshaoce, VincentWu, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

As brought up in https://reviews.llvm.org/D124730#inline-1326968, this
logic is used in places that are supposed to be thread-safe. Unfortunately
putting a single mutex around these maps also wouldn't solve the issue because
RVVType itself is mutable. So I see no other option but to drop the caches &
pass-by-reference logic completely, this likely has performance implications.
Whoever owns this probably should figure out a thread-safe place to store these
instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138287

Files:
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -177,7 +177,7 @@
 
   // Cast pointer operand of vector load intrinsic.
   for (const auto &I : enumerate(RVVI->getInputTypes())) {
-if (I.value()->isPointer()) {
+if (I.value().isPointer()) {
   assert(RVVI->getIntrinsicTypes().front() == -1 &&
  "RVVI should be vector load intrinsic.");
   OS << "  Ops[" << I.index() << "] = Builder.CreateBitCast(Ops[";
@@ -342,7 +342,7 @@
   printHeaderCode(OS);
 
   auto printType = [&](auto T) {
-OS << "typedef " << T->getClangBuiltinStr() << " " << T->getTypeStr()
+OS << "typedef " << T.getClangBuiltinStr() << " " << T.getTypeStr()
<< ";\n";
   };
 
Index: clang/lib/Support/RISCVVIntrinsicUtils.cpp
===
--- clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -803,42 +803,11 @@
   return Types;
 }
 
-// Compute the hash value of RVVType, used for cache the result of computeType.
-static uint64_t computeRVVTypeHashValue(BasicType BT, int Log2LMUL,
-PrototypeDescriptor Proto) {
-  // Layout of hash value:
-  // 0   816  2432  40
-  // | Log2LMUL + 3  | BT  | Proto.PT | Proto.TM | Proto.VTM |
-  assert(Log2LMUL >= -3 && Log2LMUL <= 3);
-  return (Log2LMUL + 3) | (static_cast(BT) & 0xff) << 8 |
- ((uint64_t)(Proto.PT & 0xff) << 16) |
- ((uint64_t)(Proto.TM & 0xff) << 24) |
- ((uint64_t)(Proto.VTM & 0xff) << 32);
-}
-
-Optional RVVType::computeType(BasicType BT, int Log2LMUL,
+Optional RVVType::computeType(BasicType BT, int Log2LMUL,
   PrototypeDescriptor Proto) {
-  // Concat BasicType, LMUL and Proto as key
-  static std::unordered_map LegalTypes;
-  static std::set IllegalTypes;
-  uint64_t Idx = computeRVVTypeHashValue(BT, Log2LMUL, Proto);
-  // Search first
-  auto It = LegalTypes.find(Idx);
-  if (It != LegalTypes.end())
-return &(It->second);
-
-  if (IllegalTypes.count(Idx))
-return llvm::None;
-
-  // Compute type and record the result.
   RVVType T(BT, Log2LMUL, Proto);
-  if (T.isValid()) {
-// Record legal type index and value.
-LegalTypes.insert({Idx, T});
-return &(LegalTypes[Idx]);
-  }
-  // Record illegal type index.
-  IllegalTypes.insert(Idx);
+  if (T.isValid())
+return T;
   return llvm::None;
 }
 
@@ -892,9 +861,9 @@
 
 std::string RVVIntrinsic::getBuiltinTypeStr() const {
   std::string S;
-  S += OutputType->getBuiltinStr();
+  S += OutputType.getBuiltinStr();
   for (const auto &T : InputTypes) {
-S += T->getBuiltinStr();
+S += T.getBuiltinStr();
   }
   return S;
 }
@@ -905,7 +874,7 @@
   SmallVector SuffixStrs;
   for (auto PD : PrototypeDescriptors) {
 auto T = RVVType::computeType(Type, Log2LMUL, PD);
-SuffixStrs.push_back((*T)->getShortStr());
+SuffixStrs.push_back((*T).getShortStr());
   }
   return join(SuffixStrs, "_");
 }
Index: clang/lib/Sema/SemaRISCVVectorLookup.cpp
===
--- clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -354,16 +354,15 @@
bool IsOverload) {
   ASTContext &Context = S.Context;
   RVVIntrinsicDef &IDef = IntrinsicList[Index];
-  RVVTypes Sigs = IDef.Signature;
+  RVVTypes &Sigs = IDef.Signature;
   size_t SigLength = Sigs.size();
-  RVVType *ReturnType =

[PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

2022-11-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

In D124730#3916786 , @kadircet wrote:

> Hi @kito-cheng, can you please let us know if you're working on a fix here 
> (and whether it seems to be close)? otherwise i am planning to revert this 
> and consequent changes, as it's clearly introducing data races.

FYI, dropping all the caching and pass-by-pointer logic in 
https://reviews.llvm.org/D138287


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124730

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

[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via ${}

2022-11-18 Thread Tom Praschan via Phabricator via cfe-commits

tom-anders added a comment.

Thanks again! Landed this with your suggestions incorporated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137909

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

[PATCH] D138289: [clang][Parse] Remove constant expression from if condition

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

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

`MaybeTypeCast` here is not a variable, it's an enum member with value 1.

I'm just not sure if this should be using `isTypeCast` instead or not. Let's 
see what the CI says.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138289

Files:
  clang/lib/Parse/ParseExpr.cpp


Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3168,7 +3168,7 @@
 InMessageExpressionRAIIObject InMessage(*this, false);
 
 Result = ParseExpression(MaybeTypeCast);
-if (!getLangOpts().CPlusPlus && MaybeTypeCast && Result.isUsable()) {
+if (!getLangOpts().CPlusPlus && Result.isUsable()) {
   // Correct typos in non-C++ code earlier so that implicit-cast-like
   // expressions are parsed correctly.
   Result = Actions.CorrectDelayedTyposInExpr(Result);


Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3168,7 +3168,7 @@
 InMessageExpressionRAIIObject InMessage(*this, false);
 
 Result = ParseExpression(MaybeTypeCast);
-if (!getLangOpts().CPlusPlus && MaybeTypeCast && Result.isUsable()) {
+if (!getLangOpts().CPlusPlus && Result.isUsable()) {
   // Correct typos in non-C++ code earlier so that implicit-cast-like
   // expressions are parsed correctly.
   Result = Actions.CorrectDelayedTyposInExpr(Result);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

2022-11-18 Thread Tom Praschan via Phabricator via cfe-commits

tom-anders created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders updated this revision to Diff 476438.
tom-anders added a comment.
tom-anders retitled this revision from "[clangd] Prototype for adding enclosing 
function in references results" to "[clangd] Add extension for adding context 
(enclosing function or class) in references results".
tom-anders published this revision for review.
tom-anders marked 2 inline comments as done.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Add tests, add client capability, add new ReferenceLocation type




Comment at: clang-tools-extra/clangd/Protocol.h:218
+  /// reference occurs
+  std::string container;
+

Location is a common vocabulary type.
If we're going to add an extension that relates to a particular method, we 
should define a new type that's wire-compatible if that's at all feasible. 
Could be a subclass of Location or just a clone.

(If this is just a shortcut taken while drafting this patch, please ignore me!)



Comment at: clang-tools-extra/clangd/XRefs.cpp:1479
+
+Index->lookup(ContainerLookup, [&](const Symbol &Container) {
+  auto Ref = RefIndexForContainer.find(Container.ID);

Being able to get all the containers in one ~constant-time index query is 
great! May still not want to have this on-by-default when the client doesn't 
advertise support, as it's a fixed nontrivial latency hit for remote index.

Also we should skip this query when the set of IDs is empty. That can go here 
or (maybe better) in the remote index client.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1479
+
+Index->lookup(ContainerLookup, [&](const Symbol &Container) {
+  auto Ref = RefIndexForContainer.find(Container.ID);

sammccall wrote:
> Being able to get all the containers in one ~constant-time index query is 
> great! May still not want to have this on-by-default when the client doesn't 
> advertise support, as it's a fixed nontrivial latency hit for remote index.
> 
> Also we should skip this query when the set of IDs is empty. That can go here 
> or (maybe better) in the remote index client.
> Being able to get all the containers in one ~constant-time index query is 
> great! May still not want to have this on-by-default when the client doesn't 
> advertise support, as it's a fixed nontrivial latency hit for remote index.
Makes sense, we could either add a ClientCapability for this (Looks like we 
already have some capabilities related to extensions, so this wouldn't be a 
novelty), or it could just be a command line flag. Any preferences? 

> Also we should skip this query when the set of IDs is empty. That can go here 
> or (maybe better) in the remote index client.
Agreed, probably more foolproof to let the index do this (otherwise the next 
person will forget about that as well probably)


Relevant issue: https://github.com/clangd/clangd/issues/177


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137894

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/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  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
@@ -302,6 +302,7 @@
 MATCHER_P(sym, Name, "") { return arg.Name == Name; }
 
 MATCHER_P(rangeIs, R, "") { return arg.Loc.range == R; }
+MATCHER_P(contextIs, C, "") { return arg.Loc.context.value_or("") == C; }
 MATCHER_P(attrsAre, A, "") { return arg.Attributes == A; }
 MATCHER_P(hasID, ID, "") { return arg.ID == ID; }
 
@@ -1900,28 +1901,30 @@
 
   auto AST = TU.build();
   std::vector> ExpectedLocations;
-  for (const auto &R : T.ranges())
-ExpectedLocations.push_back(AllOf(rangeIs(R), attrsAre(0u)));
+  for (const auto &[R, Context] : T.rangesWithPayload())
+ExpectedLocations.push_back(
+AllOf(rangeIs(R), contextIs(Context), attrsAre(0u)));
   // $def is actually shorthand for both definition and declaration.
   // If we have cases that are definition-only, we should change this.
-  for (const auto &R : T.ranges("def"))
-ExpectedLocations.push_back(
-AllOf(rangeIs(R), attrsAre(ReferencesResult::Definition |
-   ReferencesResult::Declaration)));
-  for (const auto &R : T.ranges("decl"))
-ExpectedLocations.push_back(
-AllOf(rangeIs(R), attrsAre(ReferencesResult::Declaration)));
-  for (const auto &R : T.ranges("overridedecl"))
+  for (const

[PATCH] D137894: [clangd] Add extension for adding context (enclosing function or class) in references results

2022-11-18 Thread Tom Praschan via Phabricator via cfe-commits

tom-anders added inline comments.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1939
 int main() {
-  int $def[[foo]];
-  [[^foo]] = 2;
-  int test1 = [[foo]];
+  int $def(main)[[foo]];
+  $(main)[[^foo]] = 2;

@nridge See https://reviews.llvm.org/D137909 for this new syntax


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137894

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a reviewer: bader.
aaron.ballman added a subscriber: bader.
aaron.ballman added a comment.

Adding @bader as SYCL code owner. The changes look reasonable to me, but 
codegen is not my area of expertise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D138284#3936830 , @aaron.ballman 
wrote:

> Adding @bader as SYCL code owner. The changes look reasonable to me, but 
> codegen is not my area of expertise.

Yeah, same to me.  I wrote this originally, but I think the address-space stuff 
was added in the downstream by someone else (though I can't find proof of it 
now!).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

2022-11-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+  const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+  Descriptor *D =
+  P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is());

tbaeder wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > Double checking that `Src.is()` is correct here. That's 
> > > being passed as the `IsTemporary` argument. I presume the logic is that 
> > > if the decl type is an expression then it has to be a temporary, but 
> > > that's not entirely true. Consider C where a compound literal is *not* a 
> > > temporary but actually creates an lvalue: `(int){12}`. Will that be an 
> > > issue?
> > See suggested comment below in InterpBlock.
> That interpretation is correct and I don't know what to do about compound 
> literals yet.
My intuition is that `IsTemporary` should be set from 
`Expr::isTemporaryObject()` (or isa `MaterializeTemporaryExpr` maybe).

At the very least, you can add a .c test file with a compound literal in a 
_Static_assert and a FIXME comment (in the test and here in the code).




Comment at: clang/lib/AST/Interp/Descriptor.h:74
+  };
+  static constexpr MetadataSize NoMetadata = MetadataSize{0};
+

tbaeder wrote:
> tbaeder wrote:
> > shafik wrote:
> > > erichkeane wrote:
> > > > add a line: 
> > > > `static constexpr MetadataSize InlineDescriptor = 
> > > > MetadataSize{sizeof(InlineDescriptor)};` 
> > > > and you can use this instead of depending on 'sizeof' all over the 
> > > > place.
> > > It feels weird to call this `NoMetadata` but we will pass this as an 
> > > argument to a function with a parameter of `MetaDataSize`. So I am 
> > > expecting a size but I am getting no meta data instead and it looks like 
> > > a mistake. 
> > > 
> > > Maybe a better name would be `ZeroMetaDataSize`?
> > I kinda get what your point is, but I don't think this is confusing.
> I'm a bit concerned about the naming here; `InlineDescriptor` is already a 
> type name, but calling it `MetadataSizeInlineDesc` is pretty long and callers 
> already have to prepend a `Descriptor::` to that. :/
Rather than using a sentinel value, should we be using `Optional`?



Comment at: clang/lib/AST/Interp/Interp.h:950
 return false;
   if (auto *FD = Ptr.getField()) {
 Ptr.deref() = Value.truncate(FD->getBitWidthValue(S.getCtx()));

tbaeder wrote:
> aaron.ballman wrote:
> > Do we need to `initialize()` here as well?
> I think so, but bitfields are currently completely untested.
That should be rectified. :-) It seems like that's reasonable to do as part of 
this patch given it's mostly just two changes here and a few test cases?



Comment at: clang/lib/AST/Interp/InterpBlock.h:77-86
+  char *data() {
+// Data might contain metadata as well.
+size_t DataOffset = Desc->getMetadataSize();
+return reinterpret_cast(reinterpret_cast(this) +
+sizeof(Block) + DataOffset);
+  }
+

tbaeder wrote:
> aaron.ballman wrote:
> > Should we have `const`-correct overloads to provide read-only access?
> IIRC I added that at least once in a different patch but I abandoned that and 
> haven't had a use for a const overload since.
My worry is that retrofitting const correctness will eventually become too much 
of a chore and so we'll be adding yet another const-incorrect interface to the 
compiler. Seeing as how this is being reworked from scratch, I think we should 
use the opportunity to get better const-correctness, but maybe that ship has 
sailed?



Comment at: clang/lib/AST/Interp/InterpBlock.h:97
   void invokeCtor() {
-std::memset(data(), 0, getSize());
+std::memset(rawData(), 0, Desc->getAllocSize());
 if (Desc->CtorFn)

tbaeder wrote:
> aaron.ballman wrote:
> > Why do we want to overwrite the metadata here?
> This is only called after creating an new block, so nothing is being 
> overwritten, the metadata hasn't been filled-in yet.
Sounds like a good reason not to memset over that block then; it's useless work 
that will be thrown away anyway (I worry we may come to rely on this zero init 
accidentally)?



Comment at: clang/lib/AST/Interp/Program.cpp:334
 Descriptor *ElemDesc =
-createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
+createDescriptor(D, ElemTy.getTypePtr(), Descriptor::NoMetadata,
+ IsConst, IsTemporary);

tbaeder wrote:
> aaron.ballman wrote:
> > Why does this one get no metadata while the rest of the descriptors created 
> > do?
> This is the descriptor for the array elements. The metadata passed to this 
> function is only being used for the outermost descriptor, i.e. we use it a 
> few lines below for the actual array but we don't use it for the array 
> elements.
Don't we need the m

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

2022-11-18 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D126818#3935740 , @rjmccall wrote:

> I'm too often slow to actually apply edits to the ABI document.  There's been 
> plenty of time for feedback on this one; go ahead and act like it's accepted.

Ah! I see!  I thought last time you had held this patch up on waiting for 
someone to do the work in the ABI group.  Thank you for the update!

So this should be ready for review now.  It is much smaller than the previous 
version, since we started setting the bit for 
'FriendConstraintRefersToEnclosingTemplate' during declaration creation.

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

https://reviews.llvm.org/D126818

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

[clang-tools-extra] 6527201 - [clang-tidy] Remove a duplicated if-branch in the unused-using-decl

2022-11-18 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2022-11-18T15:07:46+01:00
New Revision: 6527201414cf9479639594a7a1f80558ec09d106

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

LOG: [clang-tidy] Remove a duplicated if-branch in the unused-using-decl
check, NFC

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
index 02aa786aa8449..244cf719b5a8c 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -107,9 +107,6 @@ void UnusedUsingDeclsCheck::check(const 
MatchFinder::MatchResult &Result) {
 } else if (const auto *Specialization =
dyn_cast(Used)) {
   removeFromFoundDecls(Specialization->getSpecializedTemplate());
-} else if (const auto *FD = dyn_cast(Used)) {
-  if (const auto *FDT = FD->getPrimaryTemplate())
-removeFromFoundDecls(FDT);
 } else if (const auto *ECD = dyn_cast(Used)) {
   if (const auto *ET = ECD->getType()->getAs())
 removeFromFoundDecls(ET->getDecl());



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

[PATCH] D138289: [clang][Parse] Remove constant expression from if condition

2022-11-18 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

This code was introduced in 
https://github.com/llvm/llvm-project/commit/5a5319062300166a747807339349766341a2c2b2
 and does not appear to have had precommit review (that I could find). It was 
resolving https://bugs.llvm.org/show_bug.cgi?id=23101, and the last comment on 
that issue suggests this was intending to look at `isTypeCast`.

I expect precommit CI will not care about the removal because it is a no-op 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138289

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder marked 4 inline comments as done.
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+  const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+  Descriptor *D =
+  P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is());

aaron.ballman wrote:
> tbaeder wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > Double checking that `Src.is()` is correct here. That's 
> > > > being passed as the `IsTemporary` argument. I presume the logic is that 
> > > > if the decl type is an expression then it has to be a temporary, but 
> > > > that's not entirely true. Consider C where a compound literal is *not* 
> > > > a temporary but actually creates an lvalue: `(int){12}`. Will that be 
> > > > an issue?
> > > See suggested comment below in InterpBlock.
> > That interpretation is correct and I don't know what to do about compound 
> > literals yet.
> My intuition is that `IsTemporary` should be set from 
> `Expr::isTemporaryObject()` (or isa `MaterializeTemporaryExpr` maybe).
> 
> At the very least, you can add a .c test file with a compound literal in a 
> _Static_assert and a FIXME comment (in the test and here in the code).
> 
The `Src.is<>` is not a change introduced by this patch though.

As for a test, it'll just be rejected because we don't implement visiting 
`CompoundLiteral`s at all right now.



Comment at: clang/lib/AST/Interp/Interp.h:950
 return false;
   if (auto *FD = Ptr.getField()) {
 Ptr.deref() = Value.truncate(FD->getBitWidthValue(S.getCtx()));

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Do we need to `initialize()` here as well?
> > I think so, but bitfields are currently completely untested.
> That should be rectified. :-) It seems like that's reasonable to do as part 
> of this patch given it's mostly just two changes here and a few test cases?
I can add the `initialize()` calls but I don't think adding bitfield test cases 
makes sense for this patch (pretty sure they will fail in unexpected ways 
anyway).



Comment at: clang/lib/AST/Interp/InterpBlock.h:77-86
+  char *data() {
+// Data might contain metadata as well.
+size_t DataOffset = Desc->getMetadataSize();
+return reinterpret_cast(reinterpret_cast(this) +
+sizeof(Block) + DataOffset);
+  }
+

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Should we have `const`-correct overloads to provide read-only access?
> > IIRC I added that at least once in a different patch but I abandoned that 
> > and haven't had a use for a const overload since.
> My worry is that retrofitting const correctness will eventually become too 
> much of a chore and so we'll be adding yet another const-incorrect interface 
> to the compiler. Seeing as how this is being reworked from scratch, I think 
> we should use the opportunity to get better const-correctness, but maybe that 
> ship has sailed?
I don't think the ship has sailed at all. Is your argument that the function 
should be `const` because it can be? It's slightly problematic because they end 
up returning `const char *` as well. I can add the two overloads without 
problem, just not sure if they end up being used at all right now.



Comment at: clang/lib/AST/Interp/Program.cpp:334
 Descriptor *ElemDesc =
-createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
+createDescriptor(D, ElemTy.getTypePtr(), Descriptor::NoMetadata,
+ IsConst, IsTemporary);

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Why does this one get no metadata while the rest of the descriptors 
> > > created do?
> > This is the descriptor for the array elements. The metadata passed to this 
> > function is only being used for the outermost descriptor, i.e. we use it a 
> > few lines below for the actual array but we don't use it for the array 
> > elements.
> Don't we need the metadata to track whether the array elements have been 
> initialized? e.g., it's not the array object that gets initialized, it's the 
> elements that do, and we'd want to catch things like:
> ```
> consteval int func() {
>   int a[10];
>   return a[2];
> }
> ```
That's what the `InitMap` is for: it's just a bitmap storing one bit per 
element of a primitive array for its initialization state.


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

https://reviews.llvm.org/D135750

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

[PATCH] D138289: [clang][Parse] Remove constant expression from if condition

2022-11-18 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 476452.

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

https://reviews.llvm.org/D138289

Files:
  clang/lib/Parse/ParseExpr.cpp


Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3168,7 +3168,7 @@
 InMessageExpressionRAIIObject InMessage(*this, false);
 
 Result = ParseExpression(MaybeTypeCast);
-if (!getLangOpts().CPlusPlus && MaybeTypeCast && Result.isUsable()) {
+if (!getLangOpts().CPlusPlus && isTypeCast && Result.isUsable()) {
   // Correct typos in non-C++ code earlier so that implicit-cast-like
   // expressions are parsed correctly.
   Result = Actions.CorrectDelayedTyposInExpr(Result);


Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3168,7 +3168,7 @@
 InMessageExpressionRAIIObject InMessage(*this, false);
 
 Result = ParseExpression(MaybeTypeCast);
-if (!getLangOpts().CPlusPlus && MaybeTypeCast && Result.isUsable()) {
+if (!getLangOpts().CPlusPlus && isTypeCast && Result.isUsable()) {
   // Correct typos in non-C++ code earlier so that implicit-cast-like
   // expressions are parsed correctly.
   Result = Actions.CorrectDelayedTyposInExpr(Result);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

2022-11-18 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added inline comments.



Comment at: utils/bazel/llvm-project-overlay/llvm/BUILD.bazel:918
 ":Core",
+":IRPrinter",
 ":IRReader",

Per the follow on fix to D137768, I guess this one was unnecessary. Can you 
check if there are any others added that are similarly unnecessary and can be 
removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138081

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

[PATCH] D138221: [HIP] Fix lld failure when devie object is empty

2022-11-18 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 476454.
yaxunl added a comment.

need to specify osabi for elf64_amdgpu


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

https://reviews.llvm.org/D138221

Files:
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/hip-toolchain-device-only.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  lld/ELF/Driver.cpp


Index: lld/ELF/Driver.cpp
===
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -176,12 +176,15 @@
   .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
   .Case("elf64_sparc", {ELF64BEKind, EM_SPARCV9})
   .Case("msp430elf", {ELF32LEKind, EM_MSP430})
+  .Case("elf64_amdgpu", {ELF64LEKind, EM_AMDGPU})
   .Default({ELFNoneKind, EM_NONE});
 
   if (ret.first == ELFNoneKind)
 error("unknown emulation: " + emul);
   if (ret.second == EM_MSP430)
 osabi = ELFOSABI_STANDALONE;
+  else if (ret.second == EM_AMDGPU)
+osabi = ELFOSABI_AMDGPU_HSA;
   return std::make_tuple(ret.first, ret.second, osabi);
 }
 
Index: clang/test/Driver/hip-toolchain-no-rdc.hip
===
--- clang/test/Driver/hip-toolchain-no-rdc.hip
+++ clang/test/Driver/hip-toolchain-no-rdc.hip
@@ -59,7 +59,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-m" "elf64_amdgpu" 
"--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 //
@@ -82,7 +82,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" 
"-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 //
@@ -122,7 +122,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" 
"-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_803:.*out]]" [[OBJ_DEV_B_803]]
 
 //
@@ -145,7 +145,7 @@
 // CHECK-NOT: {{".*opt"}}
 // CHECK-NOT: {{".*llc"}}
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" 
"-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_B_900:.*out]]" [[OBJ_DEV_B_900]]
 
 //
Index: clang/test/Driver/hip-toolchain-device-only.hip
===
--- clang/test/Driver/hip-toolchain-device-only.hip
+++ clang/test/Driver/hip-toolchain-device-only.hip
@@ -12,7 +12,7 @@
 // CHECK-SAME: "-target-cpu" "gfx803"
 // CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_803:".*o"]] "-x" "hip"
 
-// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD: ".*lld.*"]] "-flavor" "gnu" "-m" "elf64_amdgpu" 
"--no-undefined" "-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
@@ -21,7 +21,7 @@
 // CHECK-SAME: "-target-cpu" "gfx900"
 // CHECK-SAME: {{.*}} "-o" [[OBJ_DEV_A_900:".*o"]] "-x" "hip"
 
-// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK: [[LLD]] "-flavor" "gnu" "-m" "elf64_amdgpu" "--no-undefined" 
"-shared"
 // CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
 
 // CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -111,7 +111,12 @@
  const llvm::opt::ArgList &Args) const 
{
   // Construct lld command.
   // The output from ld.lld is an HSA code object file.
-  ArgStringList LldArgs{"-flavor", "gnu", "--no-undefined", "-shared",
+  ArgStringList LldArgs{"-flavor",
+"gnu",
+"-m",
+"elf64_amdgpu",
+"--no-undefined",
+"-shared",
 "-plugin-opt=-amdgpu-internalize-symbols"};
 
   auto &TC = getToolChain();


Index: lld/ELF/Driver.cpp
===
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -176,12 +176,15 @@
   .Case("elf_iamcu", {ELF32LEKind, EM_IAMCU})
   .Case("elf64_sparc", {ELF64BEKind, EM_SPARCV9})
   .Case("msp430elf", {ELF32LEKind, EM_MSP430})
+  .Case("elf64_amdgpu", {ELF64LEKind, EM_AMDGPU})
   .Default({ELFNoneKind, EM_NONE});
 
   if (ret.first == ELFNoneKind)
 error("unknown emulation: " + emul);
   if (ret.second == EM_MSP430)
 osabi = ELFOSABI_STANDALONE;
+  else if (ret.second == EM_AMDGPU)
+osabi = ELFOSABI_AMDGPU_HSA;
   return std::make_tuple(ret.f

[PATCH] D138247: PR58819: Correct linkage and mangling of lambdas in inline static member initializers

2022-11-18 Thread Paul Robinson via Phabricator via cfe-commits

probinson added a comment.

If this is specifically for C++17, I believe Sony doesn't officially support 
that yet although I am checking.

It looks like this is only part of the fix for #58819? The original report also 
had a `static int n` with an internal-linkage name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138247

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

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-18 Thread Oliver Stöneberg via Phabricator via cfe-commits

firewave added a comment.

The crash is gone.

The false positive with the `[m]` capture is still present with `-std=c++11`.

Here's another false positive:

  #include 
  
  void f(const std::string&);
  
  int main() {
  std::string s;
  f(s.empty() ? "<>" : s);
  }



  input.cpp:7:26: warning: Parameter 's' is copied on last use, consider moving 
it instead. [performance-unnecessary-copy-on-last-use]
  f(s.empty() ? "<>" : s);
   ^
   std::move( )

I still have another false positive with static variables but have not gotten 
around reducing it yet.

I posted the false negatives in the GitHub ticket as those do not relate to 
getting this approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexey Bader via Phabricator via cfe-commits

bader added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

> This changes the code generation for spir64 to place the globals in 
> addrspace(4). I believe is correct, but it would be good for someone who is 
> familiar with the target to confirm.

Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for 
spir* targets.
`addrspace(4)` represents "generic" address space, which is a placeholder for a 
specific address space. If we leave it `addrspace(4)` for global definition, 
the compiler won't be able to infer genuine address space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson created this revision.
arichardson added a reviewer: clang.
Herald added subscribers: steakhal, kosarev, mattd, gchakrabarti, asavonic, 
martong, kerbowa, atanasyan, jrtc27, jvesely, sdardis.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: NoQ.
Herald added a project: All.
arichardson requested review of this revision.
Herald added subscribers: cfe-commits, jholewinski.
Herald added a project: clang.

Mixing LLVM and Clang address spaces can result in subtle bugs, and there
is no need for this hook to use the LLVM IR level address spaces.
Most of this change is just replacing zero with LangAS::Default,
but it also allows us to remove a few calls to getTargetAddressSpace().

This also removes a stale comment+workaround in
CGDebugInfo::CreatePointerLikeType(): ASTContext::getTypeSize() does
return the expected size for ReferenceType (and handles address spaces).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo &TInfo = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter &br, const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,7 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back(&Size);
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16720,7 +16720,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::De

[PATCH] D138210: [clang] Require parameter pack to be last argument in concepts.

2022-11-18 Thread Luke Nihlen via Phabricator via cfe-commits

luken-google updated this revision to Diff 476462.
luken-google added a comment.

Adding release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138210

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -766,3 +766,24 @@
   __iterator_traits_member_pointer_or_arrow_or_void> f;
 }
 }// namespace InheritedFromPartialSpec
+
+namespace GH48182 {
+template // expected-error{{template 
parameter pack must be the last template parameter}}
+concept invalid = true;
+
+template requires invalid // expected-error{{use of undeclared 
identifier 'invalid'}}
+no errors are printed
+;
+
+static_assert(invalid also here ; // expected-error{{use of undeclared 
identifier 'invalid'}}
+
+int foo() {
+bool b;
+b = invalid not just in declarations; // expected-error{{expected ';' 
after expression}}
+   // expected-error@-1{{use of 
undeclared identifier 'invalid'}}
+   // expected-error@-2{{expected 
';' after expression}}
+   // expected-error@-3{{use of 
undeclared identifier 'just'}}
+   // expected-error@-4{{unknown 
type name 'in'}}
+return b;
+}
+} // namespace GH48182
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8923,17 +8923,31 @@
 return nullptr;
   }
 
-  if (TemplateParameterLists.front()->size() == 0) {
+  TemplateParameterList *Params = TemplateParameterLists.front();
+
+  if (Params->size() == 0) {
 Diag(NameLoc, diag::err_concept_no_parameters);
 return nullptr;
   }
 
+  for (TemplateParameterList::const_iterator ParamIt = Params->begin(),
+ ParamEnd = Params->end();
+   ParamIt != ParamEnd; ++ParamIt) {
+Decl const *Param = *ParamIt;
+if (Param->isParameterPack()) {
+  if (++ParamIt == ParamEnd)
+break;
+  Diag(Param->getLocation(),
+   diag::err_template_param_pack_must_be_last_template_parameter);
+  return nullptr;
+}
+  }
+
   if (DiagnoseUnexpandedParameterPack(ConstraintExpr))
 return nullptr;
 
-  ConceptDecl *NewDecl = ConceptDecl::Create(Context, DC, NameLoc, Name,
- TemplateParameterLists.front(),
- ConstraintExpr);
+  ConceptDecl *NewDecl =
+  ConceptDecl::Create(Context, DC, NameLoc, Name, Params, ConstraintExpr);
 
   if (NewDecl->hasAssociatedConstraints()) {
 // C++2a [temp.concept]p4:
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -638,6 +638,8 @@
   ([temp.func.order]p6.2.1 is not implemented, matching GCC).
 - Implemented `P0857R0 
`_,
   which specifies constrained lambdas and constrained template 
*template-parameter*\s.
+- Required parameter pack to be provided at the end of the concept parameter 
list. This
+  fixes `Issue 48182 `_.
 
 - Do not hide templated base members introduced via using-decl in derived class
   (useful specially for constrained members). Fixes `GH50886 
`_.


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -766,3 +766,24 @@
   __iterator_traits_member_pointer_or_arrow_or_void> f;
 }
 }// namespace InheritedFromPartialSpec
+
+namespace GH48182 {
+template // expected-error{{template parameter pack must be the last template parameter}}
+concept invalid = true;
+
+template requires invalid // expected-error{{use of undeclared identifier 'invalid'}}
+no errors are printed
+;
+
+static_assert(invalid also here ; // expected-error{{use of undeclared identifier 'invalid'}}
+
+int foo() {
+bool b;
+b = invalid not just in declarations; // expected-error{{expected ';' after expression}}
+   // expected-error@-1{{use of undeclared identifier 'invalid'}}
+   // expected-error@-2{{expected ';' after expression}}
+   // expected-error@-3{{use of undeclared identifier 'just'}}
+   // expected-error@-4{

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson created this revision.
arichardson added reviewers: eandrews, dylanmckay, bader, rjmccall.
Herald added subscribers: jrtc27, luismarques, s.egerton, Jim, PkmX, atanasyan, 
simoncook, kristof.beyls, sdardis.
Herald added a project: All.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This value was added to clang/Basic in D111566 
, but is only used during
codegen, where we can use the LLVM IR DataLayout instead. I noticed this
because the downstream CHERI targets would have to also set this value
for AArch64/RISC-V/MIPS. Instead of duplicating more information between
LLVM IR and Clang, this patch moves getTargetAddressSpace(QualType T) to
CodeGenTypes, where we can consult the DataLayout.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3195,7 +3195,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3758,7 +3758,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,7 +3897,7 @@
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+DeclTy, getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy = QC.strip(NativePar

[clang] 096c033 - [clang][sema][NFC] Make some local variables const

2022-11-18 Thread Timm Bäder via cfe-commits


Author: Timm Bäder
Date: 2022-11-18T16:19:09+01:00
New Revision: 096c033634d230fdb5eae4a0c3777e629cce0d82

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

LOG: [clang][sema][NFC] Make some local variables const

We don't modify them. Also use auto here since we already wrote the full
type in the cast.

Added: 


Modified: 
clang/lib/Sema/SemaTemplate.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 08ff4f4aecb5e..bb419e79a347a 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -109,7 +109,7 @@ NamedDecl *Sema::getAsTemplateNameDecl(NamedDecl *D,
 return D;
   }
 
-  if (CXXRecordDecl *Record = dyn_cast(D)) {
+  if (const auto *Record = dyn_cast(D)) {
 // C++ [temp.local]p1:
 //   Like normal (non-template) classes, class templates have an
 //   injected-class-name (Clause 9). The injected-class-name
@@ -126,8 +126,7 @@ NamedDecl *Sema::getAsTemplateNameDecl(NamedDecl *D,
   if (Record->getDescribedClassTemplate())
 return Record->getDescribedClassTemplate();
 
-  if (ClassTemplateSpecializationDecl *Spec
-= dyn_cast(Record))
+  if (const auto *Spec = dyn_cast(Record))
 return Spec->getSpecializedTemplate();
 }
 



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

[clang] 25bd9e1 - [clang][parse] Avoid creating StmtVectors every loop iteration

2022-11-18 Thread Timm Bäder via cfe-commits


Author: Timm Bäder
Date: 2022-11-18T16:19:09+01:00
New Revision: 25bd9e10098a91623b63952967b6561d77f43c15

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

LOG: [clang][parse] Avoid creating StmtVectors every loop iteration

Added: 


Modified: 
clang/lib/Parse/ParseStmt.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 8610abb849476..6a50f83894304 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -37,8 +37,8 @@ StmtResult Parser::ParseStatement(SourceLocation 
*TrailingElseLoc,
 
   // We may get back a null statement if we found a #pragma. Keep going until
   // we get an actual statement.
+  StmtVector Stmts;
   do {
-StmtVector Stmts;
 Res = ParseStatementOrDeclaration(Stmts, StmtCtx, TrailingElseLoc);
   } while (!Res.isInvalid() && !Res.get());
 



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

[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits

doru1004 created this revision.
doru1004 added reviewers: ABataev, jdoerfert, carlo.bertolli, ronl, dhruvachak, 
gregrodgers.
doru1004 added a project: OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
doru1004 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Starting with OpenMP 5.2 the map type on target enter/exit data can be elided. 
When the map type is not specified it defaults to `to` for `target enter data` 
and to `from` for `target exit data`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138297

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_enter_data_ast_print.cpp
  clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
  clang/test/OpenMP/target_exit_data_ast_print.cpp
  clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp

Index: clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+#pragma omp target exit data map(i_def)
+
+#pragma omp target exit data map(from: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT: T i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def){{$}}
+// CHECK-NEXT: #pragma omp target exit data map(from: i){{$}}
+
+// CHECK: template<> int tmain(int argc, int *argv) {
+// CHECK-NEXT: int i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+// CHECK: template<> char tmain(char argc, char *argv) {
+// CHECK-NEXT: char i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+int main (int argc, char **argv) {
+  int b_def, b;
+  static int a_def, a;
+// CHECK: static int a_def, a;
+
+#pragma omp target exit data map(a_def)
+// CHECK:  #pragma omp target exit data map(from: a_def)
+  a_def=2;
+// CHECK-NEXT: a_def = 2;
+
+#pragma omp target exit data map(from: a)
+// CHECK:  #pragma omp target exit data map(from: a)
+  a=2;
+// CHECK-NEXT: a = 2;
+
+#pragma omp target exit data map(b_def)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b_def)
+
+#pragma omp target exit data map(from: b)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b)
+
+  return tmain(argc, &argc) + tmain(argv[0][0], argv[0]);
+}
+
+#endif
Index: clang/test/OpenMP/target_exit_data_ast_print.cpp
===
--- clang/test/OpenMP/target_exit_data_ast_print.cpp
+++ clang/test/OpenMP/target_exit_data_ast_print.cpp
@@ -6,6 +6,10 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
 // RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
Index: clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck --check-prefix=CHECK --check-prefix=CHECK-52 %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+
+#pragma omp target enter data map(i_def)
+
+#pragma omp target enter data map(to: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT:

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added a comment.

In D138284#3936846 , @erichkeane 
wrote:

> In D138284#3936830 , @aaron.ballman 
> wrote:
>
>> Adding @bader as SYCL code owner. The changes look reasonable to me, but 
>> codegen is not my area of expertise.
>
> Yeah, same to me.  I wrote this originally, but I think the address-space 
> stuff was added in the downstream by someone else (though I can't find proof 
> of it now!).

(Unfortunately) that was me. Not the best design, but it catches lots of subtle 
run-time errors at compile time. Has been extremely useful for the downstream 
CHERI fork.




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

bader wrote:
> > This changes the code generation for spir64 to place the globals in 
> > addrspace(4). I believe is correct, but it would be good for someone who is 
> > familiar with the target to confirm.
> 
> Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for 
> spir* targets.
> `addrspace(4)` represents "generic" address space, which is a placeholder for 
> a specific address space. If we leave it `addrspace(4)` for global 
> definition, the compiler won't be able to infer genuine address space.
Okay that's interesting - I guess it means we should not be using 
`getConstantAddressSpace()` here? Or getConstantAddressSpace() should actually 
return a value that maps to `addrspace(1)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits

doru1004 updated this revision to Diff 476467.

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

https://reviews.llvm.org/D138297

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_enter_data_ast_print.cpp
  clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
  clang/test/OpenMP/target_exit_data_ast_print.cpp
  clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp

Index: clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_exit_data_ast_print_openmp52.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+#pragma omp target exit data map(i_def)
+
+#pragma omp target exit data map(from: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT: T i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def){{$}}
+// CHECK-NEXT: #pragma omp target exit data map(from: i){{$}}
+
+// CHECK: template<> int tmain(int argc, int *argv) {
+// CHECK-NEXT: int i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+// CHECK: template<> char tmain(char argc, char *argv) {
+// CHECK-NEXT: char i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target exit data map(from: i_def)
+// CHECK-NEXT: #pragma omp target exit data map(from: i)
+
+int main (int argc, char **argv) {
+  int b_def, b;
+  static int a_def, a;
+// CHECK: static int a_def, a;
+
+#pragma omp target exit data map(a_def)
+// CHECK:  #pragma omp target exit data map(from: a_def)
+  a_def=2;
+// CHECK-NEXT: a_def = 2;
+
+#pragma omp target exit data map(from: a)
+// CHECK:  #pragma omp target exit data map(from: a)
+  a=2;
+// CHECK-NEXT: a = 2;
+
+#pragma omp target exit data map(b_def)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b_def)
+
+#pragma omp target exit data map(from: b)
+// CHECK-NEXT:  #pragma omp target exit data map(from: b)
+
+  return tmain(argc, &argc) + tmain(argv[0][0], argv[0]);
+}
+
+#endif
Index: clang/test/OpenMP/target_exit_data_ast_print.cpp
===
--- clang/test/OpenMP/target_exit_data_ast_print.cpp
+++ clang/test/OpenMP/target_exit_data_ast_print.cpp
@@ -6,6 +6,10 @@
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
 
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
 // RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
Index: clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_enter_data_ast_print_openmp52.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=52 -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -x c++ -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=52 -std=c++11 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck --check-prefix=CHECK --check-prefix=CHECK-52 %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+T tmain(T argc, T *argv) {
+  T i_def, i;
+
+  i = argc;
+
+#pragma omp target enter data map(i_def)
+
+#pragma omp target enter data map(to: i)
+
+  return 0;
+}
+
+// CHECK: template  T tmain(T argc, T *argv) {
+// CHECK-NEXT: T i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target enter data map(to: i_def){{$}}
+// CHECK-NEXT: #pragma omp target enter data map(to: i){{$}}
+
+// CHECK: template<> int tmain(int argc, int *argv) {
+// CHECK-NEXT: int i_def, i;
+// CHECK-NEXT: i = argc;
+// CHECK-NEXT: #pragma omp target enter data map(to: i_def){{$}}
+// CHECK-NEXT: #pragma omp target enter data map(to: i)
+
+// CHECK: template<> char tmain(char argc, char *argv) {
+// CHECK-NEXT: char

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson added inline comments.

Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

arichardson wrote:
> bader wrote:
> > > This changes the code generation for spir64 to place the globals in 
> > > addrspace(4). I believe is correct, but it would be good for someone who 
> > > is familiar with the target to confirm.
> > 
> > Globals must reside in `sycl_global` namespace, which is `addrspace(1)` for 
> > spir* targets.
> > `addrspace(4)` represents "generic" address space, which is a placeholder 
> > for a specific address space. If we leave it `addrspace(4)` for global 
> > definition, the compiler won't be able to infer genuine address space.
> Okay that's interesting - I guess it means we should not be using 
> `getConstantAddressSpace()` here? Or getConstantAddressSpace() should 
> actually return a value that maps to `addrspace(1)`?
Ah it looks like we should be using 
`CodeGenModule::GetGlobalConstantAddressSpace()` instead of 
`getTarget().getConstantAddressSpace()`. Is that correct?

```
LangAS CodeGenModule::GetGlobalConstantAddressSpace() const {
  // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
  if (LangOpts.OpenCL)
return LangAS::opencl_constant;
  if (LangOpts.SYCLIsDevice)
return LangAS::sycl_global;
  if (LangOpts.HIP && LangOpts.CUDAIsDevice && getTriple().isSPIRV())
// For HIPSPV map literals to cuda_device (maps to CrossWorkGroup in SPIR-V)
// instead of default AS (maps to Generic in SPIR-V). Otherwise, we end up
// with OpVariable instructions with Generic storage class which is not
// allowed (SPIR-V V1.6 s3.42.8). Also, mapping literals to SPIR-V
// UniformConstant storage class is not viable as pointers to it may not be
// casted to Generic pointers which are used to model HIP's "flat" pointers.
return LangAS::cuda_device;
  if (auto AS = getTarget().getConstantAddressSpace())
return *AS;
  return LangAS::Default;
}
```

Another problem appears to be that the default implementation of 
getConstantAddressSpace() returns `LangAS::Default` instead of None, so the 
.value_or() will never be used.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson updated this revision to Diff 476470.
arichardson added a comment.

Fix to use addrspace(1)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenSYCL/unique_stable_name.cpp
  clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Index: clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE=addrspace(1) '
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s '-D$ADDRSPACE='
 
 
 template
@@ -38,7 +38,7 @@
   // Make sure the following 3 are the same between the host and device compile.
   // Note that these are NOT the same value as eachother, they differ by the
   // signature.
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
-  // CHECK: private unnamed_addr constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUlvE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUliE_\00"
+  // CHECK: private unnamed_addr [[$ADDRSPACE]]constant [17 x i8] c"_ZTSZ4mainEUldE_\00"
 }
Index: clang/test/CodeGenSYCL/unique_stable_name.cpp
===
--- clang/test/CodeGenSYCL/unique_stable_name.cpp
+++ clang/test/CodeGenSYCL/unique_stable_name.cpp
@@ -1,22 +1,22 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple spir64-unknown-unknown-sycldevice -fsycl-is-device -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s
-// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr constant [[LAMBDA_K3_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ4mainEUlPZ4mainEUlvE_E_\00"
-// CHECK: @[[INT1:[^\w]+]] = private unnamed_addr constant [[INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSi\00"
-// CHECK: @[[STRING:[^\w]+]] = private unnamed_addr constant [[STRING_SIZE:\[[0-9]+ x i8\]]] c"_ZTSAppL_ZZ4mainE1jE_i\00",
-// CHECK: @[[INT2:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA_X:[^\w]+]] = private unnamed_addr constant [[LAMBDA_X_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE0_\00"
-// CHECK: @[[MACRO_Y:[^\w]+]] =  private unnamed_addr constant [[MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE1_\00"
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE2_\00", align 1
-// CHECK: @{{.*}} = private unnamed_addr constant [32 x i8] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE3_\00", align 1
-// CHECK: @[[MACRO_MACRO_X:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE4_\00"
-// CHECK: @[[MACRO_MACRO_Y:[^\w]+]] = private unnamed_addr constant [[MACRO_MACRO_SIZE]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE5_\00"
-// CHECK: @[[INT3:[^\w]+]] = private unnamed_addr constant [[INT_SIZE]] c"_ZTSi\00"
-// CHECK: @[[LAMBDA:[^\w]+]] = private unnamed_addr constant [[LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZZ4mainENKUlvE0_clEvEUlvE_\00"
-// CHECK: @[[LAMBDA_IN_DEP_INT:[^\w]+]] = private unnamed_addr constant [[DEP_INT_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIiEvvEUlvE_\00",
-// CHECK: @[[LAMBDA_IN_DEP_X:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ28lambda_in_dependent_functionIZZ4mainENKUlvE0_clEvEUlvE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_NO_DEP:[^\w]+]] = private unnamed_addr constant [[NO_DEP_LAMBDA_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ13lambda_no_depIidEvT_T0_EUlidE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA1_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUliE_ZZ4mainENKS0_clEvEUldE_EvvEUlvE_\00",
-// CHECK: @[[LAMBDA_TWO_DEP2:[^\w]+]] = private unnamed_addr constant [[DEP_LAMBDA2_SIZE:\[[0-9]+ x i8\]]] c"_ZTSZ14lambda_two_depIZZ4mainENKUlvE0_clEvEUldE_ZZ4mainENKS0_clEvEUliE_EvvEUlvE_\00",
+// CHECK: @[[LAMBDA_KERNEL3:[^\w]+]] = private unnamed_addr addrspace(1) constant [[LAMBDA_K3_

[clang] 2656fb3 - [clang][NFC] Remove unused parameter from ParseConstantExpression

2022-11-18 Thread Timm Bäder via cfe-commits


Author: Timm Bäder
Date: 2022-11-18T16:34:52+01:00
New Revision: 2656fb39451ff19400b76bba93f7f6a27879784c

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

LOG: [clang][NFC] Remove unused parameter from ParseConstantExpression

Added: 


Modified: 
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseExpr.cpp

Removed: 




diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 9e49ade8d633..cb57cee47fef 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1763,7 +1763,7 @@ class Parser : public CodeCompletionHandler {
   ExprResult ParseExpression(TypeCastState isTypeCast = NotTypeCast);
   ExprResult ParseConstantExpressionInExprEvalContext(
   TypeCastState isTypeCast = NotTypeCast);
-  ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast);
+  ExprResult ParseConstantExpression();
   ExprResult ParseCaseExpression(SourceLocation CaseLoc);
   ExprResult ParseConstraintExpression();
   ExprResult

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index caa608e16a53..20ce560514e9 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -208,14 +208,14 @@ 
Parser::ParseConstantExpressionInExprEvalContext(TypeCastState isTypeCast) {
   return Actions.ActOnConstantExpression(Res);
 }
 
-ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast) {
+ExprResult Parser::ParseConstantExpression() {
   // C++03 [basic.def.odr]p2:
   //   An expression is potentially evaluated unless it appears where an
   //   integral constant expression is required (see 5.19) [...].
   // C++98 and C++11 have no such rule, but this is only a defect in C++98.
   EnterExpressionEvaluationContext ConstantEvaluated(
   Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
-  return ParseConstantExpressionInExprEvalContext(isTypeCast);
+  return ParseConstantExpressionInExprEvalContext(NotTypeCast);
 }
 
 ExprResult Parser::ParseCaseExpression(SourceLocation CaseLoc) {



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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexey Bader via Phabricator via cfe-commits

bader added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+  Context.getTargetInfo().getConstantAddressSpace().value_or(
+  LangAS::Default));
   llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(

arichardson wrote:
> arichardson wrote:
> > bader wrote:
> > > > This changes the code generation for spir64 to place the globals in 
> > > > addrspace(4). I believe is correct, but it would be good for someone 
> > > > who is familiar with the target to confirm.
> > > 
> > > Globals must reside in `sycl_global` namespace, which is `addrspace(1)` 
> > > for spir* targets.
> > > `addrspace(4)` represents "generic" address space, which is a placeholder 
> > > for a specific address space. If we leave it `addrspace(4)` for global 
> > > definition, the compiler won't be able to infer genuine address space.
> > Okay that's interesting - I guess it means we should not be using 
> > `getConstantAddressSpace()` here? Or getConstantAddressSpace() should 
> > actually return a value that maps to `addrspace(1)`?
> Ah it looks like we should be using 
> `CodeGenModule::GetGlobalConstantAddressSpace()` instead of 
> `getTarget().getConstantAddressSpace()`. Is that correct?
> 
> 
> ```
> LangAS CodeGenModule::GetGlobalConstantAddressSpace() const {
>   // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
>   if (LangOpts.OpenCL)
> return LangAS::opencl_constant;
>   if (LangOpts.SYCLIsDevice)
> return LangAS::sycl_global;
>   if (LangOpts.HIP && LangOpts.CUDAIsDevice && getTriple().isSPIRV())
> // For HIPSPV map literals to cuda_device (maps to CrossWorkGroup in 
> SPIR-V)
> // instead of default AS (maps to Generic in SPIR-V). Otherwise, we end up
> // with OpVariable instructions with Generic storage class which is not
> // allowed (SPIR-V V1.6 s3.42.8). Also, mapping literals to SPIR-V
> // UniformConstant storage class is not viable as pointers to it may not 
> be
> // casted to Generic pointers which are used to model HIP's "flat" 
> pointers.
> return LangAS::cuda_device;
>   if (auto AS = getTarget().getConstantAddressSpace())
> return *AS;
>   return LangAS::Default;
> }
> ```
> 
> Another problem appears to be that the default implementation of 
> getConstantAddressSpace() returns `LangAS::Default` instead of None, so the 
> .value_or() will never be used.
> Ah it looks like we should be using 
> CodeGenModule::GetGlobalConstantAddressSpace() instead of 
> getTarget().getConstantAddressSpace(). Is that correct?

Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson updated this revision to Diff 476472.
arichardson added a comment.

clang-format

I could also use {} instead of `LangAS::Default`, but the latter seems more 
obvious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138295

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumCXXABI.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/AST/MicrosoftCXXABI.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/Mips.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/CodeGen/PatternInit.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
  clang/unittests/CodeGen/TestCompiler.h

Index: clang/unittests/CodeGen/TestCompiler.h
===
--- clang/unittests/CodeGen/TestCompiler.h
+++ clang/unittests/CodeGen/TestCompiler.h
@@ -48,7 +48,7 @@
 std::make_shared(compiler.getTargetOpts(;
 
 const clang::TargetInfo &TInfo = compiler.getTarget();
-PtrSize = TInfo.getPointerWidth(0) / 8;
+PtrSize = TInfo.getPointerWidth(clang::LangAS::Default) / 8;
 
 compiler.createFileManager();
 compiler.createSourceManager(compiler.getFileManager());
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp
@@ -72,7 +72,7 @@
 public:
   WalkAST(BugReporter &br, const CheckerBase *checker, AnalysisDeclContext *ac)
   : BR(br), Checker(checker), AC(ac), ASTC(AC->getASTContext()),
-PtrWidth(ASTC.getTargetInfo().getPointerWidth(0)) {}
+PtrWidth(ASTC.getTargetInfo().getPointerWidth(LangAS::Default)) {}
 
   // Statement visitor methods.
   void VisitChildren(Stmt *S);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7288,7 +7288,8 @@
 
   // Add address space to type based on its attributes.
   LangAS ASIdx = LangAS::Default;
-  uint64_t PtrWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  uint64_t PtrWidth =
+  S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (PtrWidth == 32) {
 if (Attrs[attr::Ptr64])
   ASIdx = LangAS::ptr64;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2661,7 +2661,9 @@
   // tree? Or should the consumer just recalculate the value?
   // FIXME: Using a dummy value will interact poorly with attribute enable_if.
   IntegerLiteral Size(
-  Context, llvm::APInt::getZero(Context.getTargetInfo().getPointerWidth(0)),
+  Context,
+  llvm::APInt::getZero(
+  Context.getTargetInfo().getPointerWidth(LangAS::Default)),
   Context.getSizeType(), SourceLocation());
   AllocArgs.push_back(&Size);
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16720,7 +16720,7 @@
   // The type of __null will be int or long, depending on the size of
   // pointers on the target.
   QualType Ty;
-  unsigned pw = Context.getTargetInfo().getPointerWidth(0);
+  unsigned pw = Context.getTargetInfo().getPointerWidth(LangAS::Default);
   if (pw == Context.getTargetInfo().getIntWidth())
 Ty = Context.IntTy;
   else if (pw == Context.getTargetInfo().getLongWidth())
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4506,7 +4506,7 @@
 break;
   case 7:
 if (Str == "pointer")
-  DestWidth = S.Context.getTargetInfo().getPointerWidth(0);
+  DestWidth = S.Context.getTargetInfo().getPointerWidth(LangAS::Default);
 break;
   case 11:
 if (Str == "unwind_word")
Index: clang/lib/Sema/SemaChecking.cpp
=

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Alexander Richardson via Phabricator via cfe-commits

arichardson updated this revision to Diff 476473.
arichardson added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h

Index: clang/lib/CodeGen/CodeGenTypes.h
===
--- clang/lib/CodeGen/CodeGenTypes.h
+++ clang/lib/CodeGen/CodeGenTypes.h
@@ -305,7 +305,7 @@
   bool isRecordBeingLaidOut(const Type *Ty) const {
 return RecordsBeingLaidOut.count(Ty);
   }
-
+  unsigned getTargetAddressSpace(QualType T) const;
 };
 
 }  // end namespace CodeGen
Index: clang/lib/CodeGen/CodeGenTypes.cpp
===
--- clang/lib/CodeGen/CodeGenTypes.cpp
+++ clang/lib/CodeGen/CodeGenTypes.cpp
@@ -655,7 +655,7 @@
 const ReferenceType *RTy = cast(Ty);
 QualType ETy = RTy->getPointeeType();
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -665,7 +665,7 @@
 llvm::Type *PointeeType = ConvertTypeForMem(ETy);
 if (PointeeType->isVoidTy())
   PointeeType = llvm::Type::getInt8Ty(getLLVMContext());
-unsigned AS = Context.getTargetAddressSpace(ETy);
+unsigned AS = getTargetAddressSpace(ETy);
 ResultType = llvm::PointerType::get(PointeeType, AS);
 break;
   }
@@ -958,3 +958,13 @@
 bool CodeGenTypes::isZeroInitializable(const RecordDecl *RD) {
   return getCGRecordLayout(RD).isZeroInitializable();
 }
+
+unsigned CodeGenTypes::getTargetAddressSpace(QualType T) const {
+  // Return the address space for the type. If the type is a
+  // function type without an address space qualifier, the
+  // program address space is used. Otherwise, the target picks
+  // the best address space based on the type information
+  return T->isFunctionType() && !T.hasAddressSpace()
+ ? getDataLayout().getProgramAddressSpace()
+ : getContext().getTargetAddressSpace(T.getAddressSpace());
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3196,7 +3196,7 @@
   // See if there is already something with the target's name in the module.
   llvm::GlobalValue *Entry = GetGlobalValue(AA->getAliasee());
   if (Entry) {
-unsigned AS = getContext().getTargetAddressSpace(VD->getType());
+unsigned AS = getTypes().getTargetAddressSpace(VD->getType());
 auto Ptr = llvm::ConstantExpr::getBitCast(Entry, DeclTy->getPointerTo(AS));
 return ConstantAddress(Ptr, DeclTy, Alignment);
   }
@@ -3759,7 +3759,7 @@
   if (getTarget().supportsIFunc()) {
 ResolverType = llvm::FunctionType::get(
 llvm::PointerType::get(DeclTy,
-   Context.getTargetAddressSpace(FD->getType())),
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
   }
   else {
@@ -3897,8 +3897,8 @@
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
 llvm::Type *ResolverType = llvm::FunctionType::get(
-llvm::PointerType::get(
-DeclTy, getContext().getTargetAddressSpace(FD->getType())),
+llvm::PointerType::get(DeclTy,
+   getTypes().getTargetAddressSpace(FD->getType())),
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -3128,7 +3128,7 @@
   const Type *NonQualTy = QC.strip(NativeParamType);
   QualType NativePointeeTy = cast(NonQualTy)->getPointeeType();
   unsigned NativePointeeAddrSpace =
-  CGF.getContext().getTargetAddressSpace(NativePointeeTy);
+  CGF.getTypes().getTargetAddressSpace(NativePointeeTy);
   QualType TargetTy = TargetParam->getType();
   llvm::Value *TargetAddr = CGF.EmitLoadOfScalar(
   LocalAddr, /*Volatile=*/false, TargetTy, SourceLocation());
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ c

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

2022-11-18 Thread v1nh1shungry via Phabricator via cfe-commits

v1nh1shungry created this revision.
v1nh1shungry added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,25 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for decltype, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (T->isDecltypeType())
+  addTypeHint(D->getLocation(),
+  llvm::cast(T)->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,25 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for decltype, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (T->isDecltypeType())
+  addTypeHint(D->getLocation(),
+  llvm::cast(T)->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137986: [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

2022-11-18 Thread David Tenty via Phabricator via cfe-commits

daltenty accepted this revision.
daltenty added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137986

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

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

2022-11-18 Thread Trass3r via Phabricator via cfe-commits

Trass3r added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:315
+  addTypeHint(D->getLocation(),
+  llvm::cast(T)->getUnderlyingType(),
+  /*Prefix=*/": ");

Should use dyn_cast instead of is+cast I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

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

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

2022-11-18 Thread Alexey Bader via Phabricator via cfe-commits

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

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138284

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

[PATCH] D138249: [WebAssembly] Update relaxed-simd instruction names

2022-11-18 Thread Marat Dukhan via Phabricator via cfe-commits

maratyszcza added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:180
 TARGET_BUILTIN(__builtin_wasm_relaxed_min_f64x2, "V2dV2dV2d", "nc", 
"relaxed-simd")
-TARGET_BUILTIN(__builtin_wasm_relaxed_max_f64x2, "V2dV2dV2d", "nc", 
"relaxed-simd")
+TARGET_BUILTIN(__builtin_wasm_relaxed_max_f64x2, "V2dV2dV2d", "nC", 
"relaxed-simd")
 

Typo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138249

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

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

2022-11-18 Thread v1nh1shungry via Phabricator via cfe-commits

v1nh1shungry updated this revision to Diff 476485.
v1nh1shungry added a comment.

Apply the comment's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T))
+  addTypeHint(D->getLocation(), DT->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1141,6 +1141,10 @@
   ExpectedHint{": int &", "z"});
 }
 
+TEST(TypeHints, Decltype) {
+  assertTypeHints("decltype(0) $i[[i]];", ExpectedHint{": int", "i"});
+}
+
 TEST(TypeHints, NoQualifiers) {
   assertTypeHints(R"cpp(
 namespace A {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -296,17 +296,24 @@
   return true;
 }
 
-if (D->getType()->getContainedAutoType()) {
-  if (!D->getType()->isDependentType()) {
+auto T = D->getType();
+if (T->getContainedAutoType()) {
+  if (!T->isDependentType()) {
 // Our current approach is to place the hint on the variable
 // and accordingly print the full type
 // (e.g. for `const auto& x = 42`, print `const int&`).
 // Alternatively, we could place the hint on the `auto`
 // (and then just print the type deduced for the `auto`).
-addTypeHint(D->getLocation(), D->getType(), /*Prefix=*/": ");
+addTypeHint(D->getLocation(), T, /*Prefix=*/": ");
   }
 }
 
+// Show type hint for `decltype(expr)`, e.g.
+// for `decltype(0) i;`, show `decltype(0) i: int;`
+if (const auto *DT = llvm::dyn_cast(T))
+  addTypeHint(D->getLocation(), DT->getUnderlyingType(),
+  /*Prefix=*/": ");
+
 // Handle templates like `int foo(auto x)` with exactly one instantiation.
 if (auto *PVD = llvm::dyn_cast(D)) {
   if (D->getIdentifier() && PVD->getType()->isDependentType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)` in variable declarations

2022-11-18 Thread v1nh1shungry via Phabricator via cfe-commits

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

@Trass3r Thank you for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

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

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2022-11-18 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D138263#3936536 , 
@HazardyKnusperkeks wrote:

> In D138263#3936007 , @owenpan wrote:
>
>> I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, 
>> then wouldn't it be better to do that?
>
> But the `r_brace` has no `MatchingParen`, and I didn't want to go into that 
> hole.

One thing I thought about was adding a new `BlockKind`. But maybe I can look 
into the code that sets that and can add the `MatchingParen`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138263

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

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

2022-11-18 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews added subscribers: erichkeane, aaron.ballman.
eandrews added a comment.

Functionally this looks ok to me. However I am not sure if CodeGenTypes is the 
'right' place for this function to live, considering that other functions with 
similar functionality are in ASTContext - including overloads of 
getTargetAddressSpace( ). @erichkeane @aaron.ballman could you please take a 
look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138296

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

[PATCH] D138249: [WebAssembly] Update relaxed-simd instruction names

2022-11-18 Thread Thomas Lively via Phabricator via cfe-commits

tlively added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:180
 TARGET_BUILTIN(__builtin_wasm_relaxed_min_f64x2, "V2dV2dV2d", "nc", 
"relaxed-simd")
-TARGET_BUILTIN(__builtin_wasm_relaxed_max_f64x2, "V2dV2dV2d", "nc", 
"relaxed-simd")
+TARGET_BUILTIN(__builtin_wasm_relaxed_max_f64x2, "V2dV2dV2d", "nC", 
"relaxed-simd")
 

maratyszcza wrote:
> Typo?
Yep, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138249

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

[PATCH] D138249: [WebAssembly] Update relaxed-simd instruction names

2022-11-18 Thread Thomas Lively via Phabricator via cfe-commits

tlively updated this revision to Diff 476490.
tlively added a comment.

- Fix type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138249

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -794,17 +794,17 @@
 # CHECK: i32x4.relaxed_trunc_f64x2_u_zero # encoding: [0xfd,0x84,0x02]
 i32x4.relaxed_trunc_f64x2_u_zero
 
-# CHECK: f32x4.relaxed_fma # encoding: [0xfd,0x85,0x02]
-f32x4.relaxed_fma
+# CHECK: f32x4.relaxed_madd # encoding: [0xfd,0x85,0x02]
+f32x4.relaxed_madd
 
-# CHECK: f32x4.relaxed_fms # encoding: [0xfd,0x86,0x02]
-f32x4.relaxed_fms
+# CHECK: f32x4.relaxed_nmadd # encoding: [0xfd,0x86,0x02]
+f32x4.relaxed_nmadd
 
-# CHECK: f64x2.relaxed_fma # encoding: [0xfd,0x87,0x02]
-f64x2.relaxed_fma
+# CHECK: f64x2.relaxed_madd # encoding: [0xfd,0x87,0x02]
+f64x2.relaxed_madd
 
-# CHECK: f64x2.relaxed_fms # encoding: [0xfd,0x88,0x02]
-f64x2.relaxed_fms
+# CHECK: f64x2.relaxed_nmadd # encoding: [0xfd,0x88,0x02]
+f64x2.relaxed_nmadd
 
 # CHECK: i8x16.relaxed_laneselect # encoding: [0xfd,0x89,0x02]
 i8x16.relaxed_laneselect
@@ -833,10 +833,10 @@
 # CHECK: i16x8.relaxed_q15mulr_s # encoding: [0xfd,0x91,0x02]
 i16x8.relaxed_q15mulr_s
 
-# CHECK: i16x8.dot_i8x16_i7x16_s # encoding: [0xfd,0x92,0x02]
-i16x8.dot_i8x16_i7x16_s
+# CHECK: i16x8.relaxed_dot_i8x16_i7x16_s # encoding: [0xfd,0x92,0x02]
+i16x8.relaxed_dot_i8x16_i7x16_s
 
-# CHECK: i32x4.dot_i8x16_i7x16_add_s # encoding: [0xfd,0x93,0x02]
-i32x4.dot_i8x16_i7x16_add_s
+# CHECK: i32x4.relaxed_dot_i8x16_i7x16_add_s # encoding: [0xfd,0x93,0x02]
+i32x4.relaxed_dot_i8x16_i7x16_add_s
 
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -184,9 +184,9 @@
 ; CHECK-NEXT: .functype laneselect_v16i8 (v128, v128, v128) -> (v128){{$}}
 ; CHECK-NEXT: i8x16.relaxed_laneselect $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
 ; CHECK-NEXT: return $pop[[R]]{{$}}
-declare <16 x i8> @llvm.wasm.laneselect.v16i8(<16 x i8>, <16 x i8>, <16 x i8>)
+declare <16 x i8> @llvm.wasm.relaxed.laneselect.v16i8(<16 x i8>, <16 x i8>, <16 x i8>)
 define <16 x i8> @laneselect_v16i8(<16 x i8> %a, <16 x i8> %b, <16 x i8> %c) {
-  %v = call <16 x i8> @llvm.wasm.laneselect.v16i8(
+  %v = call <16 x i8> @llvm.wasm.relaxed.laneselect.v16i8(
 <16 x i8> %a, <16 x i8> %b, <16 x i8> %c
   )
   ret <16 x i8> %v
@@ -360,9 +360,9 @@
 ; CHECK-NEXT: .functype laneselect_v8i16 (v128, v128, v128) -> (v128){{$}}
 ; CHECK-NEXT: i16x8.relaxed_laneselect $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
 ; CHECK-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.laneselect.v8i16(<8 x i16>, <8 x i16>, <8 x i16>)
+declare <8 x i16> @llvm.wasm.relaxed.laneselect.v8i16(<8 x i16>, <8 x i16>, <8 x i16>)
 define <8 x i16> @laneselect_v8i16(<8 x i16> %a, <8 x i16> %b, <8 x i16> %c) {
-  %v = call <8 x i16> @llvm.wasm.laneselect.v8i16(
+  %v = call <8 x i16> @llvm.wasm.relaxed.laneselect.v8i16(
 <8 x i16> %a, <8 x i16> %b, <8 x i16> %c
   )
   ret <8 x i16> %v
@@ -382,11 +382,11 @@
 
 ; CHECK-LABEL: dot_i8x16_i7x16_s_i16x8:
 ; CHECK-NEXT: .functype dot_i8x16_i7x16_s_i16x8 (v128, v128) -> (v128){{$}}
-; CHECK-NEXT: i16x8.dot_i8x16_i7x16_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; CHECK-NEXT: i16x8.relaxed_dot_i8x16_i7x16_s $push[[R:[0-9]+]]=, $0, $1{{$}}
 ; CHECK-NEXT: return $pop[[R]]{{$}}
-declare <8 x i16> @llvm.wasm.dot.i8x16.i7x16.signed(<16 x i8>, <16 x i8>)
+declare <8 x i16> @llvm.wasm.relaxed.dot.i8x16.i7x16.signed(<16 x i8>, <16 x i8>)
 define <8 x i16> @dot_i8x16_i7x16_s_i16x8(<16 x i8> %a, <16 x i8> %b) {
-  %v = call <8 x i16> @llvm.wasm.dot.i8x16.i7x16.signed(
+  %v = call <8 x i16> @llvm.wasm.relaxed.dot.i8x16.i7x16.signed(
 <16 x i8> %a, <16 x i8> %b
   )
   ret <8 x i16> %v
@@ -542,9 +542,9 @@
 ; CHECK-NEXT: .functype laneselect_v4i32 (v128, v128, v128) -> (v128){{$}}
 ; CHECK-NEXT: i32x4.relaxed_laneselect $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
 ; CHECK-NEXT: return $pop[[R]]{{$}}
-declare <4 x i32> @llvm.wasm.laneselect.v4i32(<4 x i32>, <4 x i32>, <4 x i32>)
+declare <4 x i32> @llvm.wasm.relaxed.laneselect.v4i32(<4 x i32>, <4 x i32>, <4 x i32>)
 define <4 x i32> @laneselect_v4i32(<4 x i32> %a, <4 x i32> %b, <4 x i32> %c) {
-  %v = call <4 x i32> @llv

[PATCH] D135411: Add generic KCFI operand bundle lowering

2022-11-18 Thread Sami Tolvanen via Phabricator via cfe-commits

samitolvanen updated this revision to Diff 476491.
samitolvanen added a comment.

Dropped the unneeded constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135411

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChain.cpp
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation/KCFI.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/CMakeLists.txt
  llvm/lib/Transforms/Instrumentation/KCFI.cpp
  llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
  llvm/test/Transforms/KCFI/kcfi.ll
  llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn
@@ -20,6 +20,7 @@
 "InstrOrderFile.cpp",
 "InstrProfiling.cpp",
 "Instrumentation.cpp",
+"KCFI.cpp",
 "MemProfiler.cpp",
 "MemorySanitizer.cpp",
 "PGOInstrumentation.cpp",
Index: llvm/test/Transforms/KCFI/kcfi.ll
===
--- /dev/null
+++ llvm/test/Transforms/KCFI/kcfi.ll
@@ -0,0 +1,21 @@
+; RUN: opt -S -passes=kcfi %s | FileCheck %s
+
+; CHECK-LABEL: define void @f1(
+define void @f1(ptr noundef %x) {
+  ; CHECK:  %[[#GEPI:]] = getelementptr inbounds i32, ptr %x, i32 -1
+  ; CHECK-NEXT: %[[#LOAD:]] = load i32, ptr %[[#GEPI]], align 4
+  ; CHECK-NEXT: %[[#ICMP:]] = icmp ne i32 %[[#LOAD]], 12345678
+  ; CHECK-NEXT: br i1 %[[#ICMP]], label %[[#TRAP:]], label %[[#CALL:]], !prof ![[#WEIGHTS:]]
+  ; CHECK:  [[#TRAP]]:
+  ; CHECK-NEXT: call void @llvm.trap()
+  ; CHECK-NEXT: br label %[[#CALL]]
+  ; CHECK:  [[#CALL]]:
+  ; CHECK-NEXT: call void %x()
+  ; CHECK-NOT:  [ "kcfi"(i32 12345678) ]
+  call void %x() [ "kcfi"(i32 12345678) ]
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"kcfi", i32 1}
+; CHECK: ![[#WEIGHTS]] = !{!"branch_weights", i32 1, i32 1048575}
Index: llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
===
--- /dev/null
+++ llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll
@@ -0,0 +1,12 @@
+; RUN: not opt -S -passes=kcfi %s 2>&1 | FileCheck %s
+
+; CHECK: error: -fpatchable-function-entry=N,M, where M>0 is not compatible with -fsanitize=kcfi on this target
+define void @f1(ptr noundef %x) #0 {
+  call void %x() [ "kcfi"(i32 12345678) ]
+  ret void
+}
+
+attributes #0 = { "patchable-function-prefix"="1" }
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"kcfi", i32 1}
Index: llvm/lib/Transforms/Instrumentation/KCFI.cpp
===
--- /dev/null
+++ llvm/lib/Transforms/Instrumentation/KCFI.cpp
@@ -0,0 +1,111 @@
+//===-- KCFI.cpp - Generic KCFI operand bundle lowering -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This pass emits generic KCFI indirect call checks for targets that don't
+// support lowering KCFI operand bundles in the back-end.
+//
+//===--===//
+
+#include "llvm/Transforms/Instrumentation/KCFI.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/DiagnosticPrinter.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalObject.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "kcfi"
+
+STATISTIC(NumKCFIChecks, "Number of kcfi operands transformed into checks");
+
+namespace {
+class DiagnosticInfoKCFI : public DiagnosticInfo {
+  const Twine &Msg;
+
+public:
+  DiagnosticInfoKCFI(const Twine &DiagMsg,
+ DiagnosticSeverity Severity = DS_Error)
+  : DiagnosticInfo(DK_Linker, Severity), Msg(DiagMsg) {}
+  void print(DiagnosticPrinter &DP) const override { DP << Msg; }
+};
+} // namespace
+
+PreservedAnalyses KCFIPass::run(Function &F, FunctionAnalysisManager &AM) {
+  Module &M = *F.getParent();
+  if (!M.getModuleFlag("kcfi"))
+return PreservedAnalyses::all();
+
+  // Find call in

[clang] fa7477e - [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

2022-11-18 Thread Xing Xue via cfe-commits


Author: Xing Xue
Date: 2022-11-18T11:36:56-05:00
New Revision: fa7477eb87fd262898e78db983c7b0951b87641c

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

LOG: [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and 
__builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

Summary:
AIX library functions frexpl(), ldexpl(), and modfl() are for 128-bit IBM long 
double, i.e. __ibm128. Other *l() functions, e.g., acosl(), are for 64-bit long 
double. The AIX Clang compiler currently maps builtin functions 
__builtin_frexpl(), __builtin_ldexpl(), and __builtin_modfl() to frexpl(), 
ldexpl(), and modfl() in 64-bit long double mode which results in seg-faults or 
incorrect return values. This patch changes to map __builtin_frexpl(), 
__builtin_ldexpl(), and __builtin_modfl() to double version lib functions 
frexp(), ldexp() and modf() in 64-bit long double mode.

Reviewed by: hubert.reinterpretcast, daltenty

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

Added: 
clang/test/CodeGen/aix-builtin-mapping.c

Modified: 
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 50b0eaed82ec6..6000478bf545e 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -106,6 +106,15 @@ llvm::Constant *CodeGenModule::getBuiltinLibFunction(const 
FunctionDecl *FD,
   {Builtin::BI__builtin_nexttowardf128, "__nexttowardieee128"},
   };
 
+  // The AIX library functions frexpl, ldexpl, and modfl are for 128-bit
+  // IBM 'long double' (i.e. __ibm128). Map to the 'double' versions
+  // if it is 64-bit 'long double' mode.
+  static SmallDenseMap AIXLongDouble64Builtins{
+  {Builtin::BI__builtin_frexpl, "frexp"},
+  {Builtin::BI__builtin_ldexpl, "ldexp"},
+  {Builtin::BI__builtin_modfl, "modf"},
+  };
+
   // If the builtin has been declared explicitly with an assembler label,
   // use the mangled name. This 
diff ers from the plain label on platforms
   // that prefix labels.
@@ -118,6 +127,12 @@ llvm::Constant *CodeGenModule::getBuiltinLibFunction(const 
FunctionDecl *FD,
 &getTarget().getLongDoubleFormat() == &llvm::APFloat::IEEEquad() &&
 F128Builtins.find(BuiltinID) != F128Builtins.end())
   Name = F128Builtins[BuiltinID];
+else if (getTriple().isOSAIX() &&
+ &getTarget().getLongDoubleFormat() ==
+ &llvm::APFloat::IEEEdouble() &&
+ AIXLongDouble64Builtins.find(BuiltinID) !=
+ AIXLongDouble64Builtins.end())
+  Name = AIXLongDouble64Builtins[BuiltinID];
 else
   Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
   }

diff  --git a/clang/test/CodeGen/aix-builtin-mapping.c 
b/clang/test/CodeGen/aix-builtin-mapping.c
new file mode 100644
index 0..57ea558652e1a
--- /dev/null
+++ b/clang/test/CodeGen/aix-builtin-mapping.c
@@ -0,0 +1,22 @@
+// AIX library functions frexpl, ldexpl, and modfl are for 128-bit IBM
+// 'long double' (i.e. __ibm128). Check that the compiler generates
+// calls to the 'double' versions for corresponding builtin functions in
+// 64-bit 'long double' mode.
+
+// RUN: %clang_cc1 -triple powerpc-ibm-aix -mlong-double-64 -emit-llvm -o - %s 
| FileCheck -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix -mlong-double-64 -emit-llvm -o - 
%s | FileCheck -check-prefix=CHECK %s
+
+int main()
+{
+  int DummyInt;
+  long double DummyLongDouble;
+  long double returnValue;
+
+  returnValue = __builtin_modfl(1.0L, &DummyLongDouble);
+  returnValue = __builtin_frexpl(0.0L, &DummyInt);
+  returnValue = __builtin_ldexpl(1.0L, 1);
+}
+
+// CHECK: %call = call double @modf(double noundef 1.00e+00, ptr noundef 
%DummyLongDouble) #3
+// CHECK: %call1 = call double @frexp(double noundef 0.00e+00, ptr noundef 
%DummyInt) #3
+// CHECK: %call2 = call double @ldexp(double noundef 1.00e+00, i32 noundef 
{{(signext )?}}1) #4



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

[PATCH] D137986: [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

2022-11-18 Thread Xing Xue via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa7477eb87fd: [Clang][CodeGen][AIX] Map __builtin_frexpl, 
__builtin_ldexpl, and… (authored by xingxue).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137986

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aix-builtin-mapping.c


Index: clang/test/CodeGen/aix-builtin-mapping.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-builtin-mapping.c
@@ -0,0 +1,22 @@
+// AIX library functions frexpl, ldexpl, and modfl are for 128-bit IBM
+// 'long double' (i.e. __ibm128). Check that the compiler generates
+// calls to the 'double' versions for corresponding builtin functions in
+// 64-bit 'long double' mode.
+
+// RUN: %clang_cc1 -triple powerpc-ibm-aix -mlong-double-64 -emit-llvm -o - %s 
| FileCheck -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix -mlong-double-64 -emit-llvm -o - 
%s | FileCheck -check-prefix=CHECK %s
+
+int main()
+{
+  int DummyInt;
+  long double DummyLongDouble;
+  long double returnValue;
+
+  returnValue = __builtin_modfl(1.0L, &DummyLongDouble);
+  returnValue = __builtin_frexpl(0.0L, &DummyInt);
+  returnValue = __builtin_ldexpl(1.0L, 1);
+}
+
+// CHECK: %call = call double @modf(double noundef 1.00e+00, ptr noundef 
%DummyLongDouble) #3
+// CHECK: %call1 = call double @frexp(double noundef 0.00e+00, ptr noundef 
%DummyInt) #3
+// CHECK: %call2 = call double @ldexp(double noundef 1.00e+00, i32 noundef 
{{(signext )?}}1) #4
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -106,6 +106,15 @@
   {Builtin::BI__builtin_nexttowardf128, "__nexttowardieee128"},
   };
 
+  // The AIX library functions frexpl, ldexpl, and modfl are for 128-bit
+  // IBM 'long double' (i.e. __ibm128). Map to the 'double' versions
+  // if it is 64-bit 'long double' mode.
+  static SmallDenseMap AIXLongDouble64Builtins{
+  {Builtin::BI__builtin_frexpl, "frexp"},
+  {Builtin::BI__builtin_ldexpl, "ldexp"},
+  {Builtin::BI__builtin_modfl, "modf"},
+  };
+
   // If the builtin has been declared explicitly with an assembler label,
   // use the mangled name. This differs from the plain label on platforms
   // that prefix labels.
@@ -118,6 +127,12 @@
 &getTarget().getLongDoubleFormat() == &llvm::APFloat::IEEEquad() &&
 F128Builtins.find(BuiltinID) != F128Builtins.end())
   Name = F128Builtins[BuiltinID];
+else if (getTriple().isOSAIX() &&
+ &getTarget().getLongDoubleFormat() ==
+ &llvm::APFloat::IEEEdouble() &&
+ AIXLongDouble64Builtins.find(BuiltinID) !=
+ AIXLongDouble64Builtins.end())
+  Name = AIXLongDouble64Builtins[BuiltinID];
 else
   Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
   }


Index: clang/test/CodeGen/aix-builtin-mapping.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-builtin-mapping.c
@@ -0,0 +1,22 @@
+// AIX library functions frexpl, ldexpl, and modfl are for 128-bit IBM
+// 'long double' (i.e. __ibm128). Check that the compiler generates
+// calls to the 'double' versions for corresponding builtin functions in
+// 64-bit 'long double' mode.
+
+// RUN: %clang_cc1 -triple powerpc-ibm-aix -mlong-double-64 -emit-llvm -o - %s | FileCheck -check-prefix=CHECK %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix -mlong-double-64 -emit-llvm -o - %s | FileCheck -check-prefix=CHECK %s
+
+int main()
+{
+  int DummyInt;
+  long double DummyLongDouble;
+  long double returnValue;
+
+  returnValue = __builtin_modfl(1.0L, &DummyLongDouble);
+  returnValue = __builtin_frexpl(0.0L, &DummyInt);
+  returnValue = __builtin_ldexpl(1.0L, 1);
+}
+
+// CHECK: %call = call double @modf(double noundef 1.00e+00, ptr noundef %DummyLongDouble) #3
+// CHECK: %call1 = call double @frexp(double noundef 0.00e+00, ptr noundef %DummyInt) #3
+// CHECK: %call2 = call double @ldexp(double noundef 1.00e+00, i32 noundef {{(signext )?}}1) #4
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -106,6 +106,15 @@
   {Builtin::BI__builtin_nexttowardf128, "__nexttowardieee128"},
   };
 
+  // The AIX library functions frexpl, ldexpl, and modfl are for 128-bit
+  // IBM 'long double' (i.e. __ibm128). Map to the 'double' versions
+  // if it is 64-bit 'long double' mode.
+  static SmallDenseMap AIXLongDouble64Builtins{
+  {Builtin::BI__builtin_frexpl, "frexp"},
+  {Builtin::BI__builtin_ldexpl, "ldexp"},
+  {Builtin::BI__builtin_modfl, "modf"},
+  };
+
   // If the builtin has been declared explicitly with an asse

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-18 Thread Fabian Keßler via Phabricator via cfe-commits

Febbe added a comment.

In D137205#3936944 , @firewave wrote:

> The crash is gone.
>
> The false positive with the `[m]` capture is still present with `-std=c++11`.

Does only a warning appear? Or also a fix? Currently, only a warning should 
appear, but this is not fixable in c++11.

> Here's another false positive:
>
>   #include 
>   
>   void f(const std::string&);
>   
>   int main() {
>   std::string s;
>   f(s.empty() ? "<>" : s);
>   }
>
>
>
>   input.cpp:7:26: warning: Parameter 's' is copied on last use, consider 
> moving it instead. [performance-unnecessary-copy-on-last-use]
>   f(s.empty() ? "<>" : s);
>^
>std::move( )

Thanks for the minimal examples. Need to look into the AST's.

> I still have another false positive with static variables but have not gotten 
> around reducing it yet.
>
> I posted the false negatives in the GitHub ticket as those do not relate to 
> getting this approved.

Also thanks, I've looked over them, seems like there is a problem with the 
detection in the CFG regarding self assignments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

[PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-18 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson accepted this revision.
tejohnson 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/D137217/new/

https://reviews.llvm.org/D137217

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

[PATCH] D138297: [Clang][OpenMP] Add default map type for target enter/exit data

2022-11-18 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:4367-4371
+  if (getLangOpts().OpenMP >= 52)
+if (DKind == OMPD_target_enter_data)
+  Data.ExtraModifier = OMPC_MAP_to;
+else if (DKind == OMPD_target_exit_data)
+  Data.ExtraModifier = OMPC_MAP_from;

Add braces, otherwise the compiler generates a warning


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

https://reviews.llvm.org/D138297

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

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-18 Thread Oliver Stöneberg via Phabricator via cfe-commits

firewave added a comment.

In D137205#3937298 , @Febbe wrote:

> Does only a warning appear? Or also a fix? Currently, only a warning should 
> appear, but this is not fixable in c++11.

There's no fix-it. I didn't realize that before.

Maybe this should also be added to the documentation as a known limitation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137205

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

1 2 >

1 - 100 of 199 matches

Mail list logo