[PATCH] D143050: [Driver][Fuchsia] Remove relative vtable multilib

2023-02-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added a subscriber: abrachet.
Herald added a project: All.
phosek requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

We have made relative vtable the default for Fuchsia C++ ABI so this
is no longer needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143050

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

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -104,41 +104,6 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld 2>&1\
 // RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-ASAN-NOEXCEPT-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -fno-exceptions \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-NOEXCEPT-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -fsanitize=address \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -fno-exceptions -fsanitize=address \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-ASAN-NOEXCEPT-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fno-experimental-relative-c++-abi-vtables \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fno-experimental-relative-c++-abi-vtables \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fno-experimental-relative-c++-abi-vtables -fexperimental-relative-c++-abi-vtables \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86
 // RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fsanitize=hwaddress \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
@@ -149,16 +114,6 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: -fuse-ld=lld 2>&1\
 // RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-HWASAN-NOEXCEPT-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -fsanitize=hwaddress \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-HWASAN-X86
-// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -fno-exceptions -fsanitize=hwaddress \
-// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: -fuse-ld=lld 2>&1\
-// RUN: | FileCheck %s -check-prefixes=CHECK-MULTILIB-X86,CHECK-MULTILIB-RELATIVE-VTABLES-HWASAN-NOEXCEPT-X86
 
 // Test compat multilibs.
 // RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia -fc++-abi=itanium \
@@ -180,13 +135,7 @@
 // CHECK-MULTILIB-ASAN-X86: "-L{{.*}}{{/|}}..{{/|}}lib{{

[PATCH] D143051: [Clang][RISCV] Bump rvv intrinsics version to v0.11

2023-02-01 Thread Yueh-Ting (eop) Chen via Phabricator via cfe-commits
eopXD created this revision.
eopXD added reviewers: craig.topper, kito-cheng, asb, rogfer01, frasercrmck.
Herald added subscribers: luke, VincentWu, vkmr, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, shiva0217, niosHD, 
sabuasal, simoncook, johnrusso, rbar, arichardson.
Herald added a project: All.
eopXD requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay.
Herald added a project: clang.

The LLVM now supports v0.11 of the RVV intrinsics. Users can use the macro
`riscv_v_intrinsic` to distinguish what kind of intrinsics is supported in
the compiler.

Please refer to tag descriptions under

https://github.com/riscv-non-isa/rvv-intrinsic-doc/tags


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143051

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -267,7 +267,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 100{{$}}
@@ -281,7 +281,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 100{{$}}
@@ -294,7 +294,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64X-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64X-EXT: __riscv_vector 1
 // CHECK-ZVE64X-EXT: __riscv_zve32x 100{{$}}
@@ -305,7 +305,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32F-EXT %s
 // CHECK-ZVE32F-EXT: __riscv_v_elen 32
 // CHECK-ZVE32F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32F-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32F-EXT: __riscv_vector 1
 // CHECK-ZVE32F-EXT: __riscv_zve32f 100{{$}}
@@ -316,7 +316,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE32X-EXT %s
 // CHECK-ZVE32X-EXT: __riscv_v_elen 32
 // CHECK-ZVE32X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE32X-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE32X-EXT: __riscv_v_min_vlen 32
 // CHECK-ZVE32X-EXT: __riscv_vector 1
 // CHECK-ZVE32X-EXT: __riscv_zve32x 100{{$}}
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -197,8 +197,8 @@
 
   if (ISAInfo->hasExtension("zve32x")) {
 Builder.defineMacro("__riscv_vector");
-// Currently we support the v0.10 RISC-V V intrinsics.
-Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 10)));
+// Currently we support the v0.11 RISC-V V intrinsics.
+Builder.defineMacro("__riscv_v_intrinsic", Twine(getVersionValue(0, 11)));
   }
 }
 


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -267,7 +267,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64D-EXT %s
 // CHECK-ZVE64D-EXT: __riscv_v_elen 64
 // CHECK-ZVE64D-EXT: __riscv_v_elen_fp 64
-// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64D-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64D-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64D-EXT: __riscv_vector 1
 // CHECK-ZVE64D-EXT: __riscv_zve32f 100{{$}}
@@ -281,7 +281,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64F-EXT %s
 // CHECK-ZVE64F-EXT: __riscv_v_elen 64
 // CHECK-ZVE64F-EXT: __riscv_v_elen_fp 32
-// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 1{{$}}
+// CHECK-ZVE64F-EXT: __riscv_v_intrinsic 11000{{$}}
 // CHECK-ZVE64F-EXT: __riscv_v_min_vlen 64
 // CHECK-ZVE64F-EXT: __riscv_vector 1
 // CHECK-ZVE64F-EXT: __riscv_zve32f 100{{$}}
@@ -294,7 +294,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-ZVE64X-EXT %s
 // CHECK-ZVE64X-EXT: __riscv_v_elen 64
 // CHECK-ZVE64X-EXT: __riscv_v_elen_fp 0
-// CHECK-ZVE64X-EXT: __riscv_v_intrins

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-02-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

As discussed offline, we decided to stop spending effort on improving the 
cppreference_parser.py, instead, we allow human edits in the generated 
`StdSymbolMap.inc` and `CSymbolMap.inc`, it gives us more flexibility: sort the 
headers for multi-header symbol in a humn-desired order rather than suboptimal 
alphabet order; symbols (e.g. `std::experimental::filesystem`) that are not 
available in cppreference website; symbols (e.g. `PRIX16`) from a complicated 
cppreference HTML page which is hard to parse in cppreference_parser.py etc.

We have two major things in this patch:

1. extend the Stdlib API to support multiple-header symbols
2. emits multiple-header symbols in the *.inc files

For 1), this change is good. We need to make sure the change not break other 
users of the `.inc` files. clangd is the main user, the behavior will be 
changed in 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/index/CanonicalIncludes.cpp#L736
 as now we have duplicated qualified names (the ideal solution is to ignore 
multiple-header symbols, I think it is fine now if we just add a single 
`std::size_t` symbol, see my comment below)
For 2), I have some concerns, it looks like some multiple-headers symbols that 
are handled incorrectly by the `cppreference_parser.py` (see my other 
comments). I think the best way for us is to list them manually (no action 
required, I will make a followup)

I'd suggest to narrow the scope of the patch:

- keep 1) and add a unittest in 
`llvm-project/clang/unittests/Tooling/StandardLibraryTest.cpp`;
- drop all generated multi-header symbols in *.inc files, instead, we manually 
add a single one `std::size_t` symbol to the `StdSymbolMap.inc` for the 
StandardLibrary unittest. This means we need to revert the current change of 
`gen_std.py`;




Comment at: clang/include/clang/Tooling/Inclusions/CSymbolMap.inc:80
 SYMBOL(FOPEN_MAX, None, )
+SYMBOL(FP_FAST_FMA, None, )
+SYMBOL(FP_FAST_FMAF, None, )

These symbols seem to only have a single header, I think it is an improvement 
effect of  the new change of parsing logic in cppreference_parser.py.



Comment at: clang/include/clang/Tooling/Inclusions/CSymbolMap.inc:96
+SYMBOL(INT16_C, None, )
+SYMBOL(INT16_C, None, )
 SYMBOL(INT16_MAX, None, )

This type of symbol (`INTX_C`) seems not correct, they are only from  
-- (this is a limitation of our current cppreference parser (it can not handle 
the complex page: https://en.cppreference.com/w/c/types/integer)



Comment at: clang/include/clang/Tooling/Inclusions/CSymbolMap.inc:172
 SYMBOL(ONCE_FLAG_INIT, None, )
+SYMBOL(PRIX16, None, )
+SYMBOL(PRIX16, None, )

I think this kind of symbol (`PRIdN`, `PRIdLEASTN`, `PRIdFASTN`, `PRIdMAX`, 
`PRIdPTR` etc) is only from `` (another limitation of the 
cppreference parser). 



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:33
 #undef SYMBOL
   SymbolNames = new std::remove_reference_t[SymCount];
   SymbolHeaderIDs =

It is sad that we're allocating more memory for these two arrays now (as we 
have multiple SYMBOLs for the same qualified name). No action needed, can you 
add a comment saying that we're allocate more space here?



Comment at: clang/tools/include-mapping/test.py:56
 """
-self.assertEqual(_ParseSymbolPage(html, 'foo'), set(['']))
+self.assertEqual(_ParseSymbolPage(html, 'foo', ""), set(['']))
 

The test needs to update as well, as you remove the change of 
`_ParseSymbolPage`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142092

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


[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493863.
VitaNuo added a comment.

Minor improvements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

Files:
  clang/include/clang/Tooling/Inclusions/StandardLibrary.h
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp

Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,8 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -15,35 +17,54 @@
 namespace tooling {
 namespace stdlib {
 
-static llvm::StringRef *HeaderNames;
-static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
-static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
-static llvm::DenseMap *NamespaceSymbols;
 
-static int initialize() {
+struct SymbolHeaderMapping {
+  llvm::StringRef *HeaderNames;
+  std::pair *SymbolNames;
+  unsigned *SymbolHeaderIDs;
+  llvm::DenseMap HeaderIDs;
+  llvm::DenseMap NamespaceSymbols;
+};
+
+static llvm::DenseMap LanguageMappings;
+
+static int countSymbols(Lang Language) {
   unsigned SymCount = 0;
 #define SYMBOL(Name, NS, Header) ++SymCount;
+  switch (Language) {
+  case Lang::C:
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
+break;
+  case Lang::CXX:
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
+break;
+  }
 #undef SYMBOL
-  SymbolNames = new std::remove_reference_t[SymCount];
-  SymbolHeaderIDs =
-  new std::remove_reference_t[SymCount];
-  NamespaceSymbols = new std::remove_reference_t;
-  HeaderIDs = new std::remove_reference_t;
+  return SymCount;
+}
+
+static void initialize(Lang Language) {
+  SymbolHeaderMapping *Mapping = new SymbolHeaderMapping();
+  LanguageMappings.try_emplace(Language, Mapping);
+
+  unsigned SymCount = countSymbols(Language);
+  Mapping->SymbolNames =
+  new std::remove_reference_tSymbolNames)>[SymCount];
+  Mapping->SymbolHeaderIDs = new std::remove_reference_t<
+  decltype(*Mapping->SymbolHeaderIDs)>[SymCount];
 
   auto AddNS = [&](llvm::StringRef NS) -> NSSymbolMap & {
-auto R = NamespaceSymbols->try_emplace(NS, nullptr);
+auto R = Mapping->NamespaceSymbols.try_emplace(NS, nullptr);
 if (R.second)
   R.first->second = new NSSymbolMap();
 return *R.first->second;
   };
 
   auto AddHeader = [&](llvm::StringRef Header) -> unsigned {
-return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
+return Mapping->HeaderIDs.try_emplace(Header, Mapping->HeaderIDs.size())
+.first->second;
   };
 
   auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
@@ -51,8 +72,8 @@
 if (NS == "None")
   NS = "";
 
-SymbolNames[SymIndex] = {NS, Name};
-SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
+Mapping->SymbolNames[SymIndex] = {NS, Name};
+Mapping->SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
 
 NSSymbolMap &NSSymbols = AddNS(NS);
 NSSymbols.try_emplace(Name, SymIndex);
@@ -60,14 +81,24 @@
 ++SymIndex;
   };
 #define SYMBOL(Name, NS, Header) Add(#Name, #NS, #Header);
+  switch (Language) {
+  case Lang::C:
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
+break;
+  case Lang::CXX:
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
+break;
+  }
 #undef SYMBOL
 
-  HeaderNames = new llvm::StringRef[HeaderIDs->size()];
-  for (const auto &E : *HeaderIDs)
-HeaderNames[E.second] = E.first;
+  Mapping->HeaderNames = new llvm::StringRef[Mapping->HeaderIDs.size()];
+  for (const auto &E : Mapping->HeaderIDs)
+Mapping->HeaderNames[E.second] = E.first;
+}
 
+static int initialize() {
+  for (Lang Language : AllLangs)
+initialize(Language);
   return 0;
 }
 
@@ -76,38 +107,63 @@
   (void)Dummy;
 }
 
-std::optional Header::named(llvm::StringRef Name) {
+static SymbolHeaderMapping *GetMapping(Lang Language) {
+  auto MapIt = LanguageMappings.find(Language);
+  SymbolHeaderMapping *Mapping = MapIt->getSecond();
+  return Mapping;
+}
+
+std::optional Header::named(llvm::StringRef Name, Lang Language) {
   ensureInitialized();
-  auto It = HeaderIDs->find(Name);
-  if (It == HeaderIDs->end())
+  SymbolHeaderMapping *Mapping = GetMapping(Language);
+  auto It = Mapping->HeaderIDs.find(Name);
+  if (It == Mapping->HeaderIDs.end())
 return std::nullopt;
-  return Header(It->second);
+  return Header(It->second, Language);
+}
+
+llvm::StringRef Header::name() const {
+  SymbolHeaderMapping *Mapping = GetMapping(Language);
+  return Mapping->HeaderNames[ID];
+}
+llvm::StringRef Symbol::scope() const {
+  SymbolHeaderMapping *Mappi

[clang] 60ea6f3 - [ARM] Allow selecting hard-float ABI in integer-only MVE.

2023-02-01 Thread Simon Tatham via cfe-commits

Author: Simon Tatham
Date: 2023-02-01T09:05:12Z
New Revision: 60ea6f35a270d11c91770a2fc366888e7d3859f4

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

LOG: [ARM] Allow selecting hard-float ABI in integer-only MVE.

Armv8.1-M can be configured to support the integer subset of the MVE
vector instructions, and no floating point. In that situation, the FP
and vector registers still exist, and so do the load, store and move
instructions that transfer data in and out of them. So there's no
reason the hard floating point ABI can't be supported, and you might
reasonably want to use it, for the sake of intrinsics-based code
passing explicit MVE vector types between functions.

But the selection of the hard float ABI in the backend was gated on
Subtarget->hasVFP2Base(), which is false in the case of integer MVE
and no FP.

As a result, you'd silently get the soft float ABI even if you
deliberately tried to select it, e.g. with clang options such as
--target=arm-none-eabi -mfloat-abi=hard -march=armv8.1m.main+nofp+mve

The hard float ABI should have been gated on the weaker condition
Subtarget->hasFPRegs(), because the only requirement for being able to
pass arguments in the FP registers is that the registers themselves
should exist.

I haven't added a new test, because changing the existing
CodeGen/Thumb2/float-ops.ll test seemed sufficient. But I've added a
comment explaining why the results are expected to be what they are.

Reviewed By: lenary

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
llvm/docs/ReleaseNotes.rst
llvm/lib/Target/ARM/ARMFastISel.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/test/CodeGen/Thumb2/float-ops.ll

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c6139252e0c34..2b2ca8b2987f0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -163,6 +163,13 @@ DWARF Support in Clang
 Arm and AArch64 Support in Clang
 
 
+* The hard-float ABI is now available in Armv8.1-M configurations that
+  have integer MVE instructions (and therefore have FP registers) but
+  no scalar or vector floating point computation. Previously, trying
+  to select the hard-float ABI on such a target (via
+  ``-mfloat-abi=hard`` or a triple ending in ``hf``) would silently
+  use the soft-float ABI instead.
+
 Floating Point Support in Clang
 ---
 

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index d628257a76904..a3f02992048a4 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -71,6 +71,10 @@ Changes to the AMDGPU Backend
 Changes to the ARM Backend
 --
 
+- The hard-float ABI is now available in Armv8.1-M configurations that
+  have integer MVE instructions (and therefore have FP registers) but
+  no scalar or vector floating point computation.
+
 Changes to the AVR Backend
 --
 

diff  --git a/llvm/lib/Target/ARM/ARMFastISel.cpp 
b/llvm/lib/Target/ARM/ARMFastISel.cpp
index 62a090f4bca81..60a6e9ade9234 100644
--- a/llvm/lib/Target/ARM/ARMFastISel.cpp
+++ b/llvm/lib/Target/ARM/ARMFastISel.cpp
@@ -1842,7 +1842,7 @@ CCAssignFn 
*ARMFastISel::CCAssignFnForCall(CallingConv::ID CC,
   case CallingConv::CXX_FAST_TLS:
 // Use target triple & subtarget features to do actual dispatch.
 if (Subtarget->isAAPCS_ABI()) {
-  if (Subtarget->hasVFP2Base() &&
+  if (Subtarget->hasFPRegs() &&
   TM.Options.FloatABIType == FloatABI::Hard && !isVarArg)
 return (Return ? RetCC_ARM_AAPCS_VFP: CC_ARM_AAPCS_VFP);
   else

diff  --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp 
b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 8a28e6b4e4fd2..07fa829731563 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2081,7 +2081,7 @@ 
ARMTargetLowering::getEffectiveCallingConv(CallingConv::ID CC,
   case CallingConv::Tail:
 if (!Subtarget->isAAPCS_ABI())
   return CallingConv::ARM_APCS;
-else if (Subtarget->hasVFP2Base() && !Subtarget->isThumb1Only() &&
+else if (Subtarget->hasFPRegs() && !Subtarget->isThumb1Only() &&
  getTargetMachine().Options.FloatABIType == FloatABI::Hard &&
  !isVarArg)
   return CallingConv::ARM_AAPCS_VFP;

diff  --git a/llvm/test/CodeGen/Thumb2/float-ops.ll 
b/llvm/test/CodeGen/Thumb2/float-ops.ll
index 51f18afaf0a46..d2b1dd6f05a3f 100644
--- a/llvm/test/CodeGen/Thumb2/float-ops.ll
+++ b/llvm/test/CodeGen/Thumb2/float-ops.ll
@@ -83,7 +83,7 @@ entry:
 define float @rem_f(float %a, float %b) {
 entry:
 ; CHECK-LABEL: rem_f:
-; NONE: bl fmodf
+; NONE: {{b|bl}} fmodf
 ; HARD: b fmod

[PATCH] D142703: [ARM] Allow selecting hard-float ABI in integer-only MVE.

2023-02-01 Thread Simon Tatham 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 rG60ea6f35a270: [ARM] Allow selecting hard-float ABI in 
integer-only MVE. (authored by simon_tatham).

Changed prior to commit:
  https://reviews.llvm.org/D142703?vs=492722&id=493865#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142703

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/test/CodeGen/Thumb2/float-ops.ll

Index: llvm/test/CodeGen/Thumb2/float-ops.ll
===
--- llvm/test/CodeGen/Thumb2/float-ops.ll
+++ llvm/test/CodeGen/Thumb2/float-ops.ll
@@ -83,7 +83,7 @@
 define float @rem_f(float %a, float %b) {
 entry:
 ; CHECK-LABEL: rem_f:
-; NONE: bl fmodf
+; NONE: {{b|bl}} fmodf
 ; HARD: b fmodf
   %0 = frem float %a, %b
   ret float %0
@@ -92,16 +92,23 @@
 define double @rem_d(double %a, double %b) {
 entry:
 ; CHECK-LABEL: rem_d:
-; NONE: bl fmod
+; NONE: {{b|bl}} fmod
 ; HARD: b fmod
   %0 = frem double %a, %b
   ret double %0
 }
 
+; In the ONLYREGS case (where we have integer MVE but no floating
+; point), we still expect the hard float ABI, because we asked for it
+; in the triple, and since the FP registers exist, it's possible to
+; use them to pass arguments. So the generated code should load the
+; return value into s0, not r0. Similarly for the other load and store
+; tests.
 define float @load_f(ptr %a) {
 entry:
 ; CHECK-LABEL: load_f:
-; NONE: ldr r0, [r0]
+; NOREGS: ldr r0, [r0]
+; ONLYREGS: vldr s0, [r0]
 ; HARD: vldr s0, [r0]
   %0 = load float, ptr %a, align 4
   ret float %0
@@ -120,7 +127,8 @@
 define void @store_f(ptr %a, float %b) {
 entry:
 ; CHECK-LABEL: store_f:
-; NONE: str r1, [r0]
+; NOREGS: str r1, [r0]
+; ONLYREGS: vstr s0, [r0]
 ; HARD: vstr s0, [r0]
   store float %b, ptr %a, align 4
   ret void
@@ -130,7 +138,7 @@
 entry:
 ; CHECK-LABEL: store_d:
 ; NOREGS: strd r2, r3, [r0]
-; ONLYREGS: strd r2, r3, [r0]
+; ONLYREGS: vstr d0, [r0]
 ; HARD: vstr d0, [r0]
   store double %b, ptr %a, align 8
   ret void
@@ -230,7 +238,8 @@
 
 define float @bitcast_i_to_f(i32 %a) {
 ; CHECK-LABEL: bitcast_i_to_f:
-; NONE-NOT: mov
+; NOREGS-NOT: mov
+; ONLYREGS: vmov s0, r0
 ; HARD: vmov s0, r0
   %1 = bitcast i32 %a to float
   ret float %1
@@ -238,15 +247,17 @@
 
 define double @bitcast_i_to_d(i64 %a) {
 ; CHECK-LABEL: bitcast_i_to_d:
-; NONE-NOT: mov
+; NOREGS-NOT: mov
+; ONLYREGS: vmov d0, r0, r1
 ; HARD: vmov d0, r0, r1
-  %1 = bitcast i64 %a to double
+ %1 = bitcast i64 %a to double
   ret double %1
 }
 
 define i32 @bitcast_f_to_i(float %a) {
 ; CHECK-LABEL: bitcast_f_to_i:
-; NONE-NOT: mov
+; NOREGS-NOT: mov
+; ONLYREGS: vmov r0, s0
 ; HARD: vmov r0, s0
   %1 = bitcast float %a to i32
   ret i32 %1
@@ -254,7 +265,8 @@
 
 define i64 @bitcast_d_to_i(double %a) {
 ; CHECK-LABEL: bitcast_d_to_i:
-; NONE-NOT: mov
+; NOREGS-NOT: mov
+; ONLYREGS: vmov r0, r1, d0
 ; HARD: vmov r0, r1, d0
   %1 = bitcast double %a to i64
   ret i64 %1
@@ -264,8 +276,8 @@
 ; CHECK-LABEL: select_f:
 ; NOREGS: lslsr2, r2, #31
 ; NOREGS: moveq   r0, r1
-; ONLYREGS: lslsr2, r2, #31
-; ONLYREGS: vmovne.f32  s2, s0
+; ONLYREGS: lslsr0, r0, #31
+; ONLYREGS: vmovne.f32  s1, s0
 ; HARD: lslsr0, r0, #31
 ; VFP4-ALL: vmovne.f32  s1, s0
 ; VFP4-ALL: vmov.f32s0, s1
@@ -276,8 +288,9 @@
 
 define double @select_d(double %a, double %b, i1 %c) {
 ; CHECK-LABEL: select_d:
-; NONE: ldr{{(.w)?}} [[REG:r[0-9]+]], [sp]
-; NONE: ands[[REG]], [[REG]], #1
+; NOREGS: ldr{{(.w)?}} [[REG:r[0-9]+]], [sp]
+; NOREGS: ands[[REG]], [[REG]], #1
+; ONLYREGS: andsr0, r0, #1
 ; NOREGS-DAG: moveq   r0, r2
 ; NOREGS-DAG: moveq   r1, r3
 ; ONLYREGS-DAG: csel   r0, r0, r2
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2081,7 +2081,7 @@
   case CallingConv::Tail:
 if (!Subtarget->isAAPCS_ABI())
   return CallingConv::ARM_APCS;
-else if (Subtarget->hasVFP2Base() && !Subtarget->isThumb1Only() &&
+else if (Subtarget->hasFPRegs() && !Subtarget->isThumb1Only() &&
  getTargetMachine().Options.FloatABIType == FloatABI::Hard &&
  !isVarArg)
   return CallingConv::ARM_AAPCS_VFP;
Index: llvm/lib/Target/ARM/ARMFastISel.cpp
===
--- llvm/lib/Target/ARM/ARMFastISel.cpp
+++ llvm/lib/Target/ARM/ARMFastISel.cpp
@@ -1842,7 +1842,7 @@
   case CallingConv::CXX_FAST_TLS:
 // Use target triple & subtarget features to do actual dispatch.
 if (Subtarget->isAAPCS_ABI()) {
-  if (Subtarget->hasVFP2Base() &&
+  if (Subtarget->hasFPRegs() &&
  

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

2023-02-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision.
ChuanqiXu added reviewers: erichkeane, royjacobson, shafik, clang-language-wg.
ChuanqiXu added a project: clang.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

Close https://github.com/llvm/llvm-project/issues/60275

The root cause of issue 60275 is the imbalance of 
PushExpressionEvaluationContext() and PopExpressionEvaluationContext().

See 
https://github.com/llvm/llvm-project/blob/f1c4f927f7c15b5efdc3589c050fd0513bf6b303/clang/lib/Parse/Parser.cpp#L1396-L1437

We will  PushExpressionEvaluationContext() in ActOnStartOfFunctionDef() in line 
1396 and we should pop it in ActOnFinishFunctionBody later. However if we skip 
the function body in line 1402, the expression evaluation context will not be 
popped. Then here is the issue report. I fix the issue by inserting codes to 
pop the expression evaluation context  explicitly if the function body is 
skipped. Maybe this looks like an ad-hoc fix. But if we want to fix this in a 
pretty way, we should refactor the current framework for pushing and popping 
expression evaluation contexts. Currently there are 23 
PushExpressionEvaluationContext() callsities and 21 
PopExpressionEvaluationContext() callsites in the code. And it seems not easy 
to balance them well and fast. So I suggest to land this fix first. At least it 
can prevent the crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143053

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Modules/pr60275.cppm


Index: clang/test/Modules/pr60275.cppm
===
--- /dev/null
+++ clang/test/Modules/pr60275.cppm
@@ -0,0 +1,40 @@
+// Address: https://github.com/llvm/llvm-project/issues/60275
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple 
-emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp 
-fmodule-file=%t/a.pcm -emit-llvm -o - | FileCheck %t/b.cpp
+//--- foo.h
+
+consteval void global() {}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+
+//--- b.cpp
+#include "foo.h"
+import a;
+
+consteval int b() {
+   return 0;
+}
+
+struct bb {
+   int m = b();
+};
+
+void bbb() {
+   bb x;
+}
+
+// CHECK: define{{.*}}_ZN2bbC2Ev({{.*}}[[THIS:%.+]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[THIS_ADDR:%.*]] = alloca ptr
+// CHECK-NEXT:   store ptr [[THIS]], ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[M_ADDR:%.*]] = getelementptr{{.*}}%struct.bb, ptr 
[[THIS1]], i32 0, i32 0
+// CHECK-NEXT:   store i32 0, ptr [[M_ADDR]]
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
@@ -1404,6 +1405,17 @@
 if (BodyKind == Sema::FnBodyKind::Other)
   SkipFunctionBody();
 
+// ExpressionEvaluationContext is pushed in ActOnStartOfFunctionDef
+// and it would be popped in ActOnFinishFunctionBody.
+// We pop it explcitly here since ActOnFinishFunctionBody won't get called.
+//
+// Do not call PopExpressionEvaluationContext() if it is a lambda because
+// one is already popped when finishing the lambda in BuildLambdaExpr().
+//
+// FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+// and PopExpressionEvaluationContext().
+if (!isLambdaCallOperator(dyn_cast_if_present(Res)))
+  Actions.PopExpressionEvaluationContext();
 return Res;
   }
 


Index: clang/test/Modules/pr60275.cppm
===
--- /dev/null
+++ clang/test/Modules/pr60275.cppm
@@ -0,0 +1,40 @@
+// Address: https://github.com/llvm/llvm-project/issues/60275
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp -fmodule-file=%t/a.pcm -emit-llvm -o - | FileCheck %t/b.cpp
+//--- foo.h
+
+consteval void global() {}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+
+//--- b.cpp
+#include "foo.h"
+import a;
+
+consteval int b() {
+	return 0;
+}
+
+struct bb {
+	int m = b();
+};
+
+void bbb() {
+	bb x;
+}
+
+// CHECK: define{{.*}}_ZN2bbC2Ev({{.*}}[[THIS:%.+]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[THIS_ADDR:%.*]] = alloca ptr
+// CHECK-NEXT:   store ptr [[THIS]], ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]]
+// CHECK-NEXT:

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

2023-02-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1709
+
+// The enclosing namespace must be first, it gets a quality boost.
+if (auto Enclosing = SpecifiedScopes.EnclosingNamespace) {

i was actually suggesting to put this logic inside 
`SpecifiedScope::scopesForIndexQuery` any reason for only including it in this 
code path?



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1719
+  });
+llvm::copy_if(SpecifiedScopes.scopesForQualification(),
+  std::back_inserter(AccessibleScopes),

`AccessibleScopes` doesn't need any particular ordering. we can use 
`scopesForQualification` as-is.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:712
+std::vector EnclosingAtFrontForCompletion;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
+EnclosingAtFrontForIndex.push_back(EnclosingScope);

tom-anders wrote:
> kadircet wrote:
> > note that this is actually going to skip inline namespaces (and you're 
> > using that in the returned set)
> Hm I should probably fix this and add another regression test for this..?
yeah, let's just leave a fixme.
i also think we should be setting it in all cases, i can't think of a reason 
for only setting it in a single case. i believe current scope should always get 
a boost whenever it's part of query scopes.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1660
 // First scope is the (modified) enclosing scope.
 QueryScopes = Scopes.scopesForIndexQuery();
 ScopeProximity.emplace(QueryScopes);

tom-anders wrote:
> kadircet wrote:
> > we should be setting `AccessibleScopes` too
> Ah thanks, that's what caused the one failing test. I just copied over 
> QueryScopes here for now, looks like this doesn't need any special handling 
> for inline namespaces, does it?
> looks like this doesn't need any special handling for inline namespaces, does 
> it?

well it probably does, but there isn't much we can do. as this code path 
happens with only heuristic parsing of the main-file code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140915

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


[clang] f559e78 - [AArch64] Handle negative architecture features

2023-02-01 Thread David Green via cfe-commits

Author: David Green
Date: 2023-02-01T09:21:07Z
New Revision: f559e781b2bd918d8cac8a878639870a8f26196d

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

LOG: [AArch64] Handle negative architecture features

Currently negative architecture features passes to clang like -Xclang
-target-feature -Xclang -v9.3a will end up _enabling_ dependant target
features (like FEAT_MOPS). This patch fixes that by ensuring we don't
enable dependant target features when !Enabled.

Fixes #60375

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/test/CodeGen/aarch64-targetattr.c
clang/test/Preprocessor/aarch64-target-features.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 33f9d67ef0e9b..b670a2578f843 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -685,11 +685,15 @@ void 
AArch64TargetInfo::setFeatureEnabled(llvm::StringMap &Features,
   llvm::AArch64::ArchInfo::findBySubArch(Name);
 
   if (!ArchInfo)
-return; // Not an architecure, nothing more to do.
+return; // Not an architecture, nothing more to do.
+
+  // Disabling an architecture feature does not affect dependent features
+  if (!Enabled)
+return;
 
   for (const auto *OtherArch : llvm::AArch64::ArchInfos)
 if (ArchInfo->implies(*OtherArch))
-  Features[OtherArch->getSubArch()] = Enabled;
+  Features[OtherArch->getSubArch()] = true;
 
   // Set any features implied by the architecture
   std::vector CPUFeats;

diff  --git a/clang/test/CodeGen/aarch64-targetattr.c 
b/clang/test/CodeGen/aarch64-targetattr.c
index 9cdbf42c82e45..9664b723a2b2c 100644
--- a/clang/test/CodeGen/aarch64-targetattr.c
+++ b/clang/test/CodeGen/aarch64-targetattr.c
@@ -89,6 +89,11 @@ void noneon() {}
 __attribute__((target("no-simd")))
 void nosimd() {}
 
+// This isn't part of the standard interface, but test that -arch features 
should not apply anything else.
+// CHECK-LABEL: @minusarch() #18
+__attribute__((target("no-v9.3a")))
+void minusarch() {}
+
 // CHECK: attributes #0 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+lse,+neon,+ras,+rdm,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #1 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+v8.1a,+v8.2a,+v8a"
 }
 // CHECK: attributes #2 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+sve2,+v8.1a,+v8.2a,+v8a"
 }
@@ -107,3 +112,4 @@ void nosimd() {}
 // CHECK: attributes #15 = { {{.*}} "target-cpu"="neoverse-n1" 
"target-features"="+aes,+bf16,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a"
 "tune-cpu"="cortex-a710" }
 // CHECK: attributes #16 = { {{.*}} "branch-target-enforcement"="true" {{.*}} 
"target-features"="+aes,+bf16,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a"
 "tune-cpu"="cortex-a710" }
 // CHECK: attributes #17 = { {{.*}} "target-features"="-neon" }
+// CHECK: attributes #18 = { {{.*}} "target-features"="-v9.3a" }

diff  --git a/clang/test/Preprocessor/aarch64-target-features.c 
b/clang/test/Preprocessor/aarch64-target-features.c
index 5834bc11f40c8..2a2f7efe34130 100644
--- a/clang/test/Preprocessor/aarch64-target-features.c
+++ b/clang/test/Preprocessor/aarch64-target-features.c
@@ -569,6 +569,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a -x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+nomops  -x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops-x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// Check that -target-feature -v9.3a doesn't enable dependant features
+// RUN: %clang -target aarch64-arm-none-eabi -Xclang -target-feature -Xclang 
-v9.3a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS   %s
 // CHECK-MOPS: __ARM_FEATURE_MOPS 1
 // CHECK-NOMOPS-NOT: __ARM_FEATURE_MOPS 1
 



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


[PATCH] D142963: [AArch64] Handle negative architecture features

2023-02-01 Thread Dave Green 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 rGf559e781b2bd: [AArch64] Handle negative architecture 
features (authored by dmgreen).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142963

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-targetattr.c
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -569,6 +569,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a -x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+nomops  -x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops-x c 
-E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// Check that -target-feature -v9.3a doesn't enable dependant features
+// RUN: %clang -target aarch64-arm-none-eabi -Xclang -target-feature -Xclang 
-v9.3a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS   %s
 // CHECK-MOPS: __ARM_FEATURE_MOPS 1
 // CHECK-NOMOPS-NOT: __ARM_FEATURE_MOPS 1
 
Index: clang/test/CodeGen/aarch64-targetattr.c
===
--- clang/test/CodeGen/aarch64-targetattr.c
+++ clang/test/CodeGen/aarch64-targetattr.c
@@ -89,6 +89,11 @@
 __attribute__((target("no-simd")))
 void nosimd() {}
 
+// This isn't part of the standard interface, but test that -arch features 
should not apply anything else.
+// CHECK-LABEL: @minusarch() #18
+__attribute__((target("no-v9.3a")))
+void minusarch() {}
+
 // CHECK: attributes #0 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+lse,+neon,+ras,+rdm,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #1 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+v8.1a,+v8.2a,+v8a"
 }
 // CHECK: attributes #2 = { {{.*}} 
"target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+sve2,+v8.1a,+v8.2a,+v8a"
 }
@@ -107,3 +112,4 @@
 // CHECK: attributes #15 = { {{.*}} "target-cpu"="neoverse-n1" 
"target-features"="+aes,+bf16,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a"
 "tune-cpu"="cortex-a710" }
 // CHECK: attributes #16 = { {{.*}} "branch-target-enforcement"="true" {{.*}} 
"target-features"="+aes,+bf16,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a"
 "tune-cpu"="cortex-a710" }
 // CHECK: attributes #17 = { {{.*}} "target-features"="-neon" }
+// CHECK: attributes #18 = { {{.*}} "target-features"="-v9.3a" }
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -685,11 +685,15 @@
   llvm::AArch64::ArchInfo::findBySubArch(Name);
 
   if (!ArchInfo)
-return; // Not an architecure, nothing more to do.
+return; // Not an architecture, nothing more to do.
+
+  // Disabling an architecture feature does not affect dependent features
+  if (!Enabled)
+return;
 
   for (const auto *OtherArch : llvm::AArch64::ArchInfos)
 if (ArchInfo->implies(*OtherArch))
-  Features[OtherArch->getSubArch()] = Enabled;
+  Features[OtherArch->getSubArch()] = true;
 
   // Set any features implied by the architecture
   std::vector CPUFeats;


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -569,6 +569,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+nomops  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// Check that -target-feature -v9.3a doesn't enable dependant features
+// RUN: %clang -target aarch64-arm-none-eabi -Xclang -target-feature -Xclang -v9.3a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS   %s
 // CHECK-MOPS: __ARM_FEATURE_MOPS 1
 // CHECK-NOMOPS-NOT: __ARM_FEATURE_MOPS 1
 
Index: clang/test/CodeGen/aarch64-targetattr.c
===
--

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493869.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.

Add comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142092

Files:
  clang/include/clang/Tooling/Inclusions/CSymbolMap.inc
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/tools/include-mapping/cppreference_parser.py
  clang/tools/include-mapping/gen_std.py
  clang/tools/include-mapping/test.py

Index: clang/tools/include-mapping/test.py
===
--- clang/tools/include-mapping/test.py
+++ clang/tools/include-mapping/test.py
@@ -53,7 +53,7 @@
   
 
 """
-self.assertEqual(_ParseSymbolPage(html, 'foo'), set(['']))
+self.assertEqual(_ParseSymbolPage(html, 'foo', ""), set(['']))
 
 
   def testParseSymbolPage_MulHeaders(self):
@@ -94,7 +94,7 @@
   
 
 """
-self.assertEqual(_ParseSymbolPage(html, "foo"),
+self.assertEqual(_ParseSymbolPage(html, "foo", ""),
  set(['', '']))
 
 
@@ -121,7 +121,7 @@
 
 
 """
-self.assertEqual(_ParseSymbolPage(html, "foo"),
+self.assertEqual(_ParseSymbolPage(html, "foo", ""),
  set(['', '']))
 
   def testParseSymbolPage_MulSymbolsInSameTd(self):
@@ -145,9 +145,9 @@
 
 
 """
-self.assertEqual(_ParseSymbolPage(html, "int8_t"),
+self.assertEqual(_ParseSymbolPage(html, "int8_t", ""),
  set(['']))
-self.assertEqual(_ParseSymbolPage(html, "int16_t"),
+self.assertEqual(_ParseSymbolPage(html, "int16_t", ""),
  set(['']))
 
 
Index: clang/tools/include-mapping/gen_std.py
===
--- clang/tools/include-mapping/gen_std.py
+++ clang/tools/include-mapping/gen_std.py
@@ -14,10 +14,7 @@
 The generated files are located in clang/include/Tooling/Inclusions.
 
 Caveats and FIXMEs:
-  - only symbols directly in "std" namespace are added, we should also add std's
-subnamespace symbols (e.g. chrono).
-  - symbols with multiple variants or defined in multiple headers aren't added,
-e.g. std::move, std::swap
+  - symbols with multiple variants aren't added, e.g., std::move
 
 Usage:
   1. Install BeautifulSoup dependency, see instruction:
@@ -110,17 +107,13 @@
 os.stat(index_page_path).st_mtime).strftime('%Y-%m-%d')
   print(CODE_PREFIX % (args.symbols.upper(), cppreference_modified_date))
   for symbol in symbols:
-if len(symbol.headers) == 1:
-  # SYMBOL(unqualified_name, namespace, header)
-  print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
-symbol.headers[0]))
-elif len(symbol.headers) == 0:
+if len(symbol.headers) == 0:
   sys.stderr.write("No header found for symbol %s\n" % symbol.name)
 else:
-  # FIXME: support symbols with multiple headers (e.g. std::move).
-  sys.stderr.write("Ambiguous header for symbol %s: %s\n" % (
-  symbol.name, ', '.join(symbol.headers)))
-
-
+  symbol.headers = sorted(symbol.headers)
+  for header in symbol.headers:
+# SYMBOL(unqualified_name, namespace, header)
+print("SYMBOL(%s, %s, %s)" % (symbol.name, symbol.namespace,
+  header))
 if __name__ == '__main__':
   main()
Index: clang/tools/include-mapping/cppreference_parser.py
===
--- clang/tools/include-mapping/cppreference_parser.py
+++ clang/tools/include-mapping/cppreference_parser.py
@@ -47,7 +47,7 @@
 
   Returns a list of headers.
   """
-  headers = set()
+  symbol_headers = set()
   all_headers = set()
 
   soup = BeautifulSoup(symbol_page_html, "html.parser")
@@ -58,32 +58,48 @@
   #   Defined in header   .t-dsc-header
   #   decl2.t-dcl
   for table in soup.select('table.t-dcl-begin, table.t-dsc-begin'):
-current_headers = []
-was_decl = False
-for row in table.select('tr'):
-  if _HasClass(row, 't-dcl', 't-dsc'):
-was_decl = True
-# Symbols are in the first cell.
-found_symbols = row.find('td').stripped_strings
-if not symbol_name in found_symbols:
-  continue
-headers.update(current_headers)
-  elif _HasClass(row, 't-dsc-header'):
-# If we saw a decl since the last header, this is a new block of headers
-# for a new block of decls.
-if was_decl:
-  current_headers = []
-was_decl = False
+rows = table.select('tr')
+i = 0
+while i < len(rows):
+  start = i
+  current_headers = set()
+  while i < len(rows) and _HasClass(rows[i], 't-dsc-header'):
+row = rows[i]
 # There are also .t-dsc-header for "defined in namespace".
 if not "Defined in header " in row.text:
+  i = i + 1
 

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493870.
VitaNuo added a comment.

Revert all changes to generator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142092

Files:
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/tools/include-mapping/test.py

Index: clang/tools/include-mapping/test.py
===
--- clang/tools/include-mapping/test.py
+++ clang/tools/include-mapping/test.py
@@ -53,7 +53,7 @@
   
 
 """
-self.assertEqual(_ParseSymbolPage(html, 'foo'), set(['']))
+self.assertEqual(_ParseSymbolPage(html, 'foo', ""), set(['']))
 
 
   def testParseSymbolPage_MulHeaders(self):
@@ -94,7 +94,7 @@
   
 
 """
-self.assertEqual(_ParseSymbolPage(html, "foo"),
+self.assertEqual(_ParseSymbolPage(html, "foo", ""),
  set(['', '']))
 
 
@@ -121,7 +121,7 @@
 
 
 """
-self.assertEqual(_ParseSymbolPage(html, "foo"),
+self.assertEqual(_ParseSymbolPage(html, "foo", ""),
  set(['', '']))
 
   def testParseSymbolPage_MulSymbolsInSameTd(self):
@@ -145,9 +145,9 @@
 
 
 """
-self.assertEqual(_ParseSymbolPage(html, "int8_t"),
+self.assertEqual(_ParseSymbolPage(html, "int8_t", ""),
  set(['']))
-self.assertEqual(_ParseSymbolPage(html, "int16_t"),
+self.assertEqual(_ParseSymbolPage(html, "int16_t", ""),
  set(['']))
 
 
Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -17,7 +18,7 @@
 
 static llvm::StringRef *HeaderNames;
 static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
+static llvm::SmallVector *SymbolHeaderIDs;
 static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
@@ -29,6 +30,8 @@
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
 #undef SYMBOL
+  // Allocates more space than necessary since multiple SYMBOLs might correspond
+  // to the same qualified name.
   SymbolNames = new std::remove_reference_t[SymCount];
   SymbolHeaderIDs =
   new std::remove_reference_t[SymCount];
@@ -46,18 +49,26 @@
 return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
   };
 
-  auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
-  llvm::StringRef HeaderName) mutable {
+  auto Add = [&, SymIndex(-1)](llvm::StringRef Name, llvm::StringRef NS,
+   llvm::StringRef HeaderName) mutable {
 if (NS == "None")
   NS = "";
 
+// Determine whether the symbol is new. This relies on symbols being
+// ordered alpahbetically in the map.
+if (SymIndex >= 0 && SymbolNames[SymIndex].first == NS &&
+SymbolNames[SymIndex].second == Name) {
+  // Not a new symbol, use the same index.
+} else {
+  // First symbol or new symbol, increment next available index.
+  ++SymIndex;
+}
+
 SymbolNames[SymIndex] = {NS, Name};
-SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
+SymbolHeaderIDs[SymIndex].emplace_back(AddHeader(HeaderName));
 
 NSSymbolMap &NSSymbols = AddNS(NS);
 NSSymbols.try_emplace(Name, SymIndex);
-
-++SymIndex;
   };
 #define SYMBOL(Name, NS, Header) Add(#Name, #NS, #Header);
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
@@ -87,7 +98,7 @@
 llvm::StringRef Symbol::scope() const { return SymbolNames[ID].first; }
 llvm::StringRef Symbol::name() const { return SymbolNames[ID].second; }
 std::optional Symbol::named(llvm::StringRef Scope,
- llvm::StringRef Name) {
+llvm::StringRef Name) {
   ensureInitialized();
   if (NSSymbolMap *NSSymbols = NamespaceSymbols->lookup(Scope)) {
 auto It = NSSymbols->find(Name);
@@ -96,9 +107,14 @@
   }
   return std::nullopt;
 }
-Header Symbol::header() const { return Header(SymbolHeaderIDs[ID]); }
+
+Header Symbol::header() const { return Header(SymbolHeaderIDs[ID][0]); }
 llvm::SmallVector Symbol::headers() const {
-  return {header()}; // FIXME: multiple in case of ambiguity
+  llvm::SmallVector result;
+  for (auto HeaderID : SymbolHeaderIDs[ID]) {
+result.emplace_back(Header(HeaderID));
+  }
+  return result;
 }
 
 Recognizer::Recognizer() { ensureInitialized(); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493871.
VitaNuo added a comment.

Revert more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142092

Files:
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp


Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -17,7 +18,7 @@
 
 static llvm::StringRef *HeaderNames;
 static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
+static llvm::SmallVector *SymbolHeaderIDs;
 static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
@@ -29,6 +30,8 @@
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
 #undef SYMBOL
+  // Allocates more space than necessary since multiple SYMBOLs might 
correspond
+  // to the same qualified name.
   SymbolNames = new std::remove_reference_t[SymCount];
   SymbolHeaderIDs =
   new std::remove_reference_t[SymCount];
@@ -46,18 +49,26 @@
 return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
   };
 
-  auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
-  llvm::StringRef HeaderName) mutable {
+  auto Add = [&, SymIndex(-1)](llvm::StringRef Name, llvm::StringRef NS,
+   llvm::StringRef HeaderName) mutable {
 if (NS == "None")
   NS = "";
 
+// Determine whether the symbol is new. This relies on symbols being
+// ordered alpahbetically in the map.
+if (SymIndex >= 0 && SymbolNames[SymIndex].first == NS &&
+SymbolNames[SymIndex].second == Name) {
+  // Not a new symbol, use the same index.
+} else {
+  // First symbol or new symbol, increment next available index.
+  ++SymIndex;
+}
+
 SymbolNames[SymIndex] = {NS, Name};
-SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
+SymbolHeaderIDs[SymIndex].emplace_back(AddHeader(HeaderName));
 
 NSSymbolMap &NSSymbols = AddNS(NS);
 NSSymbols.try_emplace(Name, SymIndex);
-
-++SymIndex;
   };
 #define SYMBOL(Name, NS, Header) Add(#Name, #NS, #Header);
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
@@ -87,7 +98,7 @@
 llvm::StringRef Symbol::scope() const { return SymbolNames[ID].first; }
 llvm::StringRef Symbol::name() const { return SymbolNames[ID].second; }
 std::optional Symbol::named(llvm::StringRef Scope,
- llvm::StringRef Name) {
+llvm::StringRef Name) {
   ensureInitialized();
   if (NSSymbolMap *NSSymbols = NamespaceSymbols->lookup(Scope)) {
 auto It = NSSymbols->find(Name);
@@ -96,9 +107,14 @@
   }
   return std::nullopt;
 }
-Header Symbol::header() const { return Header(SymbolHeaderIDs[ID]); }
+
+Header Symbol::header() const { return Header(SymbolHeaderIDs[ID][0]); }
 llvm::SmallVector Symbol::headers() const {
-  return {header()}; // FIXME: multiple in case of ambiguity
+  llvm::SmallVector result;
+  for (auto HeaderID : SymbolHeaderIDs[ID]) {
+result.emplace_back(Header(HeaderID));
+  }
+  return result;
 }
 
 Recognizer::Recognizer() { ensureInitialized(); }


Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -17,7 +18,7 @@
 
 static llvm::StringRef *HeaderNames;
 static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
+static llvm::SmallVector *SymbolHeaderIDs;
 static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
@@ -29,6 +30,8 @@
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
 #undef SYMBOL
+  // Allocates more space than necessary since multiple SYMBOLs might correspond
+  // to the same qualified name.
   SymbolNames = new std::remove_reference_t[SymCount];
   SymbolHeaderIDs =
   new std::remove_reference_t[SymCount];
@@ -46,18 +49,26 @@
 return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
   };
 
-  auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
-  llvm::StringRef HeaderName) mutable {
+  auto Add 

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added a comment.

Reverted everything apart from library support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142092

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


[PATCH] D143054: [include-mapping] Regenerate the mappings from the 20220730 html book.

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo created this revision.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143054

Files:
  clang/include/clang/Tooling/Inclusions/CSymbolMap.inc
  clang/include/clang/Tooling/Inclusions/StdRemovedSymbolMap.inc
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc

Index: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
===
--- clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
+++ clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
@@ -6,45 +6,18 @@
 // This file was generated automatically by
 // clang/tools/include-mapping/gen_std.py, DO NOT EDIT!
 //
-// Generated from cppreference offline HTML book (modified on 2018-10-28).
+// Generated from cppreference offline HTML book (modified on 2022-07-30).
 //===--===//
 
-SYMBOL(Assignable, std::, )
-SYMBOL(Boolean, std::, )
-SYMBOL(Common, std::, )
-SYMBOL(CommonReference, std::, )
-SYMBOL(Constructible, std::, )
-SYMBOL(ConvertibleTo, std::, )
-SYMBOL(CopyConstructible, std::, )
-SYMBOL(Copyable, std::, )
-SYMBOL(DefaultConstructible, std::, )
-SYMBOL(DerivedFrom, std::, )
-SYMBOL(Destructible, std::, )
-SYMBOL(EqualityComparable, std::, )
-SYMBOL(EqualityComparableWith, std::, )
 SYMBOL(FILE, std::, )
-SYMBOL(Integral, std::, )
-SYMBOL(Invocable, std::, )
-SYMBOL(Movable, std::, )
-SYMBOL(MoveConstructible, std::, )
-SYMBOL(Predicate, std::, )
-SYMBOL(Regular, std::, )
-SYMBOL(RegularInvocable, std::, )
-SYMBOL(Relation, std::, )
-SYMBOL(Same, std::, )
-SYMBOL(Semiregular, std::, )
-SYMBOL(SignedIntegral, std::, )
-SYMBOL(StrictTotallyOrdered, std::, )
-SYMBOL(StrictTotallyOrderedWith, std::, )
-SYMBOL(StrictWeakOrder, std::, )
-SYMBOL(Swappable, std::, )
-SYMBOL(SwappableWith, std::, )
-SYMBOL(UniformRandomBitGenerator, std::, )
-SYMBOL(UnsignedIntegral, std::, )
 SYMBOL(_Exit, std::, )
 SYMBOL(accumulate, std::, )
 SYMBOL(acos, std::, )
+SYMBOL(acosf, std::, )
 SYMBOL(acosh, std::, )
+SYMBOL(acoshf, std::, )
+SYMBOL(acoshl, std::, )
+SYMBOL(acosl, std::, )
 SYMBOL(add_const, std::, )
 SYMBOL(add_const_t, std::, )
 SYMBOL(add_cv, std::, )
@@ -73,7 +46,10 @@
 SYMBOL(alignment_of, std::, )
 SYMBOL(alignment_of_v, std::, )
 SYMBOL(all_of, std::, )
+SYMBOL(allocate_at_least, std::, )
 SYMBOL(allocate_shared, std::, )
+SYMBOL(allocate_shared_for_overwrite, std::, )
+SYMBOL(allocation_result, std::, )
 SYMBOL(allocator, std::, )
 SYMBOL(allocator_arg, std::, )
 SYMBOL(allocator_arg_t, std::, )
@@ -83,15 +59,35 @@
 SYMBOL(apply, std::, )
 SYMBOL(arg, std::, )
 SYMBOL(array, std::, )
+SYMBOL(as_bytes, std::, )
 SYMBOL(as_const, std::, )
+SYMBOL(as_writable_bytes, std::, )
 SYMBOL(asctime, std::, )
 SYMBOL(asin, std::, )
+SYMBOL(asinf, std::, )
 SYMBOL(asinh, std::, )
+SYMBOL(asinhf, std::, )
+SYMBOL(asinhl, std::, )
+SYMBOL(asinl, std::, )
+SYMBOL(assignable_from, std::, )
+SYMBOL(assoc_laguerre, std::, )
+SYMBOL(assoc_laguerref, std::, )
+SYMBOL(assoc_laguerrel, std::, )
+SYMBOL(assoc_legendre, std::, )
+SYMBOL(assoc_legendref, std::, )
+SYMBOL(assoc_legendrel, std::, )
+SYMBOL(assume_aligned, std::, )
 SYMBOL(async, std::, )
 SYMBOL(at_quick_exit, std::, )
 SYMBOL(atan, std::, )
 SYMBOL(atan2, std::, )
+SYMBOL(atan2f, std::, )
+SYMBOL(atan2l, std::, )
+SYMBOL(atanf, std::, )
 SYMBOL(atanh, std::, )
+SYMBOL(atanhf, std::, )
+SYMBOL(atanhl, std::, )
+SYMBOL(atanl, std::, )
 SYMBOL(atexit, std::, )
 SYMBOL(atof, std::, )
 SYMBOL(atoi, std::, )
@@ -116,19 +112,28 @@
 SYMBOL(atomic_flag, std::, )
 SYMBOL(atomic_flag_clear, std::, )
 SYMBOL(atomic_flag_clear_explicit, std::, )
+SYMBOL(atomic_flag_notify_all, std::, )
+SYMBOL(atomic_flag_notify_one, std::, )
+SYMBOL(atomic_flag_test, std::, )
 SYMBOL(atomic_flag_test_and_set, std::, )
 SYMBOL(atomic_flag_test_and_set_explicit, std::, )
+SYMBOL(atomic_flag_test_explicit, std::, )
+SYMBOL(atomic_flag_wait, std::, )
+SYMBOL(atomic_flag_wait_explicit, std::, )
 SYMBOL(atomic_init, std::, )
-SYMBOL(atomic_is_lockfree, std::, )
+SYMBOL(atomic_is_lock_free, std::, )
 SYMBOL(atomic_load, std::, )
 SYMBOL(atomic_load_explicit, std::, )
+SYMBOL(atomic_notify_all, std::, )
+SYMBOL(atomic_notify_one, std::, )
 SYMBOL(atomic_ref, std::, )
 SYMBOL(atomic_signal_fence, std::, )
 SYMBOL(atomic_store, std::, )
 SYMBOL(atomic_store_explicit, std::, )
 SYMBOL(atomic_thread_fence, std::, )
+SYMBOL(atomic_wait, std::, )
+SYMBOL(atomic_wait_explicit, std::, )
 SYMBOL(atto, std::, )
-SYMBOL(auto_ptr, std::, )
 SYMBOL(back_insert_iterator, std::, )
 SYMBOL(back_inserter, std::, )
 SYMBOL(bad_alloc, std::, )
@@ -141,35 +146,54 @@
 SYMBOL(bad_typeid, std::, )
 SYMBOL(bad_variant_access, std::, )
 SYMBOL(bad_weak_ptr, std::, )
+SYMBOL(barrier, std::, )
 SYMBOL(basic_common_reference, std:

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

2023-02-01 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added a comment.

On AArch64 we have the following failures:

  Failed Tests (15):
Clang :: CXX/basic/basic.stc/basic.stc.dynamic/p2-nodef.cpp
Clang :: CodeCompletion/ordinary-name-cxx11.cpp
Clang :: CodeCompletion/ordinary-name.cpp
Clang :: CodeGenCXX/cxx20-module-std-subst-2b.cpp
Clang :: CodeGenCXX/cxx20-module-std-subst-2c.cpp
Clang :: Index/complete-preamble.cpp
Clang :: Index/load-namespaces.cpp
Clang :: Modules/implicit-declared-allocation-functions.cppm
Clang :: PCH/chain-friend-instantiation.cpp
Clang :: PCH/cxx1z-aligned-alloc.cpp
Clang :: SemaCXX/constructor-initializer.cpp
Clang :: SemaCXX/using-directive.cpp
Clang-Unit :: 
AST/./ASTTests/ParameterizedTests/DeclContextTest/removeDeclOfClassTemplateSpecialization/0
Clang-Unit :: 
StaticAnalyzer/./StaticAnalysisTests/CallDescription/RejectOverQualifiedNames
Clang-Unit :: 
StaticAnalyzer/./StaticAnalysisTests/CallDescription/SkipAnonimousNamespaces

None of which are caused by assertions but instead errors of the form:

  input.cc:12:7: error: reference to 'std' is ambiguous
std::container v;
^
  note: candidate found by name lookup is 'std'
  input.cc:3:15: note: candidate found by name lookup is 'my::std'
  namespace std {
^
  1 error generated.

  
/home/david.spickett/llvm-project/clang/test/CodeGenCXX/cxx20-module-std-subst-2b.cpp:14:18:
 error: reference to 'std' is ambiguous
  void f(str> &s) {
   ^
  
/home/david.spickett/llvm-project/clang/test/CodeGenCXX/cxx20-module-std-subst-2b.cpp:7:11:
 note: candidate found by name lookup is 'std'
  namespace std {
^

On Arm it's the exact same set of failures.

What would help you debug those? I don't know the background for the change but 
I can check specifics on the machine itself if needed.

Test log from AArch64: F26314570: tests.log 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142822

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


[PATCH] D140959: RFC: Multilib prototype

2023-02-01 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.

FWIW I tried this patch out with RISC-V multilibs, and it works a treat. It 
solves the multilib reuse problem and relaxes the order of non-standard 
extensions. I did have to copy some of the logic from BareMetal.cpp to Gnu.cpp 
as was using a GCC installation.

The `args` field for the YAML was unnecessary and worked (including 
--print-multi-lib) just fine without it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140959

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


[PATCH] D143025: [Fuchsia] Add llvm-mt and llvm-rc to clang bootstrap dependency

2023-02-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: clang/cmake/caches/Fuchsia.cmake:42-52
+foreach(variableName ${_FUCHSIA_BOOTSTRAP_PASSTHROUGH_STRINGS})
+  if(DEFINED ${variableName})
+set(BOOTSTRAP_${variableName} "${${variableName}}" CACHE STRING "")
+  endif()
+endforeach()
+
+foreach(variableName ${_FUCHSIA_BOOTSTRAP_PASSTHROUGH_BOOL})

You should be able to use the following to avoid having to separate the 
variables by type:
```
  foreach(variable ${PASSTHROUGH_VARIABLES})
get_property(is_value_set CACHE ${variable} PROPERTY VALUE SET)
if(${is_value_set})
  get_property(value CACHE ${variable} PROPERTY VALUE)
  get_property(type CACHE ${variable} PROPERTY TYPE)
  set(BOOTSTRAP_${variable} "${value}" CACHE ${type} "")
endif()
  endforeach()
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143025

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


[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

2023-02-01 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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142014

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


[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Can you add some tests in `StandardLibraryTest.cpp`?




Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:31
+
+static llvm::DenseMap LanguageMappings;
+

using a map here seems like an overkill, we have just 2 elements, I'd just use 
two separate variables (`CMapping`, and `CXXMapping`)



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:100
+static int initialize() {
+  for (Lang Language : AllLangs)
+initialize(Language);

nit: just `for (Lang Language: {Lang::C, Lang::CXX})` or two statements 
`initilize(Lang::C);` and `initialize(Lang::CXX);`. 



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:164
 
+  ensureInitialized();
+  SymbolHeaderMapping *Mapping = GetMapping(Language);

Do we need the `ensureInitialized()` here? looks like no needed, we have called 
it in the Recognizer constructor,



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:188
+  Lang RecognizerLang = Lang::CXX;
+  if (Language == clang::Language::C) {
+RecognizerLang = Lang::C;

nit: just use `LangOpts.CPlusPlus` to check the language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

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


[PATCH] D142757: [clang][driver] Emit an error for `/clang:-x`

2023-02-01 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon added inline comments.



Comment at: clang/test/Driver/x-args.c:12
+// RUN: not %clang_cl /TC /WX /clang:-xc /clang:-E /clang:-dM %s 2>&1 | 
FileCheck -check-prefix=CL %s
+// CL-NOT: '-x c' after last input file has no effect
+// CL: error: unsupported option '-x c'; did you mean '/TC' or '/TP'?

Fznamznon wrote:
> MaskRay wrote:
> > You can remove this `CL-NOT` negative pattern. Instead, use 
> > `--check-prefix=CL --implicit-check-not=error:` to check that there is no 
> > other error.
> I probably don't fully understand the suggestion, but there is a `error:` 
> that is being checked in this test. The one that this revision adds. It is 
> checked on the next line.
Is that ok if I submit the patch "as is"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142757

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


[PATCH] D140959: RFC: Multilib prototype

2023-02-01 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings abandoned this revision.
michaelplatings added a comment.

I've created a new stack of changes taking into account the feedback. Unlike 
this change which was strictly a prototype, the new changes should be suitable 
for detailed review and hopefully approval.

- https://reviews.llvm.org/D142878 - Add testing for Fuchsia multilib
- https://reviews.llvm.org/D142893 - Class for building MultilibSet
- https://reviews.llvm.org/D142905 - Change multilib selection algorithm
- https://reviews.llvm.org/D142932 - Multilib YAML parsing
- https://reviews.llvm.org/D142933 - Add -print-multi-selection-flags argument
- https://reviews.llvm.org/D142986 - Enable multilib.yaml in the BareMetal 
ToolChain

I'm working on further patches to enable multilib layering, but that needn't 
block the above changes.

Thanks for giving it a try @Joe. The new changes are by design much more 
limited, in order to avoid creating an unstable API. I'd be interested to learn 
from you whether it's suitable for what you need, possibly by adding more 
functionality. In particular you'll see that https://reviews.llvm.org/D142933 
has functionality specific to Arm extensions so something similar may be 
required for other architectures.

In D140959#4028638 , @tschuett wrote:

> Did you look at Yaml I/O ?

Thanks for the tip, I've used it in https://reviews.llvm.org/D142932 and it's a 
great improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140959

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


[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493886.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

Files:
  clang/include/clang/Tooling/Inclusions/StandardLibrary.h
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp

Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,8 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -15,35 +17,54 @@
 namespace tooling {
 namespace stdlib {
 
-static llvm::StringRef *HeaderNames;
-static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
-static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
-static llvm::DenseMap *NamespaceSymbols;
 
-static int initialize() {
+struct SymbolHeaderMapping {
+  llvm::StringRef *HeaderNames;
+  std::pair *SymbolNames;
+  unsigned *SymbolHeaderIDs;
+  llvm::DenseMap HeaderIDs;
+  llvm::DenseMap NamespaceSymbols;
+};
+
+static llvm::DenseMap LanguageMappings;
+
+static int countSymbols(Lang Language) {
   unsigned SymCount = 0;
 #define SYMBOL(Name, NS, Header) ++SymCount;
+  switch (Language) {
+  case Lang::C:
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
+break;
+  case Lang::CXX:
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
+break;
+  }
 #undef SYMBOL
-  SymbolNames = new std::remove_reference_t[SymCount];
-  SymbolHeaderIDs =
-  new std::remove_reference_t[SymCount];
-  NamespaceSymbols = new std::remove_reference_t;
-  HeaderIDs = new std::remove_reference_t;
+  return SymCount;
+}
+
+static void initialize(Lang Language) {
+  SymbolHeaderMapping *Mapping = new SymbolHeaderMapping();
+  LanguageMappings.try_emplace(Language, Mapping);
+
+  unsigned SymCount = countSymbols(Language);
+  Mapping->SymbolNames =
+  new std::remove_reference_tSymbolNames)>[SymCount];
+  Mapping->SymbolHeaderIDs = new std::remove_reference_t<
+  decltype(*Mapping->SymbolHeaderIDs)>[SymCount];
 
   auto AddNS = [&](llvm::StringRef NS) -> NSSymbolMap & {
-auto R = NamespaceSymbols->try_emplace(NS, nullptr);
+auto R = Mapping->NamespaceSymbols.try_emplace(NS, nullptr);
 if (R.second)
   R.first->second = new NSSymbolMap();
 return *R.first->second;
   };
 
   auto AddHeader = [&](llvm::StringRef Header) -> unsigned {
-return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
+return Mapping->HeaderIDs.try_emplace(Header, Mapping->HeaderIDs.size())
+.first->second;
   };
 
   auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
@@ -51,8 +72,8 @@
 if (NS == "None")
   NS = "";
 
-SymbolNames[SymIndex] = {NS, Name};
-SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
+Mapping->SymbolNames[SymIndex] = {NS, Name};
+Mapping->SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
 
 NSSymbolMap &NSSymbols = AddNS(NS);
 NSSymbols.try_emplace(Name, SymIndex);
@@ -60,14 +81,24 @@
 ++SymIndex;
   };
 #define SYMBOL(Name, NS, Header) Add(#Name, #NS, #Header);
+  switch (Language) {
+  case Lang::C:
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
+break;
+  case Lang::CXX:
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
+break;
+  }
 #undef SYMBOL
 
-  HeaderNames = new llvm::StringRef[HeaderIDs->size()];
-  for (const auto &E : *HeaderIDs)
-HeaderNames[E.second] = E.first;
+  Mapping->HeaderNames = new llvm::StringRef[Mapping->HeaderIDs.size()];
+  for (const auto &E : Mapping->HeaderIDs)
+Mapping->HeaderNames[E.second] = E.first;
+}
 
+static int initialize() {
+  for (Lang Language : AllLangs)
+initialize(Language);
   return 0;
 }
 
@@ -76,38 +107,62 @@
   (void)Dummy;
 }
 
-std::optional Header::named(llvm::StringRef Name) {
+static SymbolHeaderMapping *GetMapping(Lang Language) {
+  auto MapIt = LanguageMappings.find(Language);
+  SymbolHeaderMapping *Mapping = MapIt->getSecond();
+  return Mapping;
+}
+
+std::optional Header::named(llvm::StringRef Name, Lang Language) {
   ensureInitialized();
-  auto It = HeaderIDs->find(Name);
-  if (It == HeaderIDs->end())
+  SymbolHeaderMapping *Mapping = GetMapping(Language);
+  auto It = Mapping->HeaderIDs.find(Name);
+  if (It == Mapping->HeaderIDs.end())
 return std::nullopt;
-  return Header(It->second);
+  return Header(It->second, Language);
+}
+
+llvm::StringRef Header::name() const {
+  SymbolHeaderMapping *Mapping = GetMapping(Language);
+  return Mapping->HeaderNames[ID];
+}
+llvm::StringRef Symbol::scope() const {
+  SymbolHeaderMapping *

[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added a comment.

Thanks for the comments!




Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:31
+
+static llvm::DenseMap LanguageMappings;
+

hokein wrote:
> using a map here seems like an overkill, we have just 2 elements, I'd just 
> use two separate variables (`CMapping`, and `CXXMapping`)
what about the design idea that we might potentially want to extend this to 
multiple standards etc.? The idea is that it's extensible to `ObjC`, 
`OpenCL`... and so on and so forth, as has been discussed offline.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:100
+static int initialize() {
+  for (Lang Language : AllLangs)
+initialize(Language);

hokein wrote:
> nit: just `for (Lang Language: {Lang::C, Lang::CXX})` or two statements 
> `initilize(Lang::C);` and `initialize(Lang::CXX);`. 
yes, same argument as above. I remember extensive discussions about the idea 
that we might want to extend this to multiple language versions etc. in the 
future.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:164
 
+  ensureInitialized();
+  SymbolHeaderMapping *Mapping = GetMapping(Language);

hokein wrote:
> Do we need the `ensureInitialized()` here? looks like no needed, we have 
> called it in the Recognizer constructor,
you're right, not needed.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:188
+  Lang RecognizerLang = Lang::CXX;
+  if (Language == clang::Language::C) {
+RecognizerLang = Lang::C;

hokein wrote:
> nit: just use `LangOpts.CPlusPlus` to check the language.
There's `LangStandard::isCPlusPlus` method that I've just discovered. That's 
probably even better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

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


[PATCH] D143059: [NFC] Enable selecting multiple multilibs

2023-02-01 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added subscribers: luke, abrachet, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, sdardis.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay.
Herald added a project: clang.

This will enable layering multilibs on top of each other.
For example a multilib containing only a no-exceptions libc++ could be
layered on top of a multilib containing C libs. This avoids the need
to duplicate the C library for every libc++ variant.

This change doesn't expose the functionality externally, it only opens
the functionality up to be potentially used by ToolChain classes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143059

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CSKYToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MipsLinux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/fuchsia.cpp
  clang/unittests/Driver/FuchsiaTest.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -153,18 +153,18 @@
   Multilib("/bar", {}, {}, {"+bar"}),
   });
   Multilib::flags_list Flags1 = {"+foo", "-bar"};
-  Multilib Selection1;
+  llvm::SmallVector Selection1;
   ASSERT_TRUE(MS.select(Flags1, Selection1))
   << "Flag set was {\"+foo\"}, but selection not found";
-  ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
-  << "Selection picked " << Selection1 << " which was not expected";
+  ASSERT_TRUE(Selection1.back().gccSuffix() == "/foo")
+  << "Selection picked " << Selection1.back() << " which was not expected";
 
   Multilib::flags_list Flags2 = {"+foo", "+bar"};
-  Multilib Selection2;
+  llvm::SmallVector Selection2;
   ASSERT_TRUE(MS.select(Flags2, Selection2))
   << "Flag set was {\"+bar\"}, but selection not found";
-  ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
-  << "Selection picked " << Selection2 << " which was not expected";
+  ASSERT_TRUE(Selection2.back().gccSuffix() == "/bar")
+  << "Selection picked " << Selection2.back() << " which was not expected";
 }
 
 TEST(MultilibTest, SelectMultiple) {
@@ -172,17 +172,17 @@
   Multilib("/a", {}, {}, {"x"}),
   Multilib("/b", {}, {}, {"y"}),
   });
-  std::vector Selection;
+  llvm::SmallVector Selection;
 
-  Selection = MS.select({"x"});
+  ASSERT_TRUE(MS.select({"x"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y"});
+  ASSERT_TRUE(MS.select({"y"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/b", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y", "x"});
+  ASSERT_TRUE(MS.select({"y", "x"}, Selection));
   ASSERT_EQ(2u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
@@ -307,7 +307,7 @@
 
 static bool select(const std::vector &InFlags,
const MultilibSet &MS, const MultilibFlagMap &MFM,
-   Multilib &Selected) {
+   llvm::SmallVector &Selected) {
   Multilib::flags_list Flags(MFM.getFlags(InFlags));
   Flags.insert(InFlags.begin(), InFlags.end());
   return MS.select(Flags, Selected);
@@ -316,7 +316,7 @@
 TEST(MultilibTest, SelectSoft) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: s
@@ -336,7 +336,7 @@
 TEST(MultilibTest, SelectSoftFP) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: f
@@ -353,7 +353,7 @@
   // with hard float.
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: h
@@ -368,7 +368,7 @@
 TEST(MultilibTest, SelectFloatABI) {
   MultilibSet MS;
   MultilibFlagMap MFM;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parse(MS, MFM, R"(
 variants:
 - dir: s
@@ -389,11 +389,11 @@
   noMatchFlags: [hasfp]
 )"));
   select({"mfloat

[PATCH] D142963: [AArch64] Handle negative architecture features

2023-02-01 Thread Tomas Matheson via Phabricator via cfe-commits
tmatheson added a comment.

In D142963#4094545 , @andrewrk wrote:

> Speaking as the one who filed the motivating bug report, all of the above 
> behaviors are fine. The motivating use case is explicitly specifying a //full 
> set// of enabled/disabled features, leaving nothing changed by LLVM's own 
> dependency resolution. In this use case, LLVM would never see any of these 
> three scenarios as input.

This is not something that I would recommend that you do, at least until we 
have a coherent model for what enabling/disabling features via 
`-target-features` means with respect to backend features. Currently:

- Architecture features are treated specially in some cases,
- it's not clear what negative features mean with respect to dependent features,
- the order in which you specify the `-target-features` changes the results,
- etc.

We don't test any combinations that we don't expect out of the clang driver 
(these are the first clang tests ever added for negative architecture 
features). You might have more reliable results if you stick to only using 
negative features where they are needed to disable features that were 
implicitly enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142963

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


[PATCH] D141939: [SVE][Builtins] Lower X forms of binop arithmetic builtins to dedicated intrinsics.

2023-02-01 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141939

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


[PATCH] D141939: [SVE][Builtins] Lower X forms of binop arithmetic builtins to dedicated intrinsics.

2023-02-01 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM. Feel free to ignore the nit if you think the explicit `x_intrinsic` is 
better.




Comment at: clang/include/clang/Basic/arm_sve.td:762
 
-multiclass SInstZPZZ flags=[]> {
-  def _M   : SInst;
-  def _X   : SInst;
-  def _Z   : SInst;
-
-  def _N_M : SInst;
-  def _N_X : SInst;
-  def _N_Z : SInst;
-}
-
-defm SVABD_S  : SInstZPZZ<"svabd",  "csil", "aarch64_sve_sabd">;
-defm SVABD_U  : SInstZPZZ<"svabd",  "UcUsUiUl", "aarch64_sve_uabd">;
-defm SVADD: SInstZPZZ<"svadd",  "csilUcUsUiUl", "aarch64_sve_add">;
-defm SVDIV_S  : SInstZPZZ<"svdiv",  "il",   "aarch64_sve_sdiv">;
-defm SVDIV_U  : SInstZPZZ<"svdiv",  "UiUl", "aarch64_sve_udiv">;
-defm SVDIVR_S : SInstZPZZ<"svdivr", "il",   "aarch64_sve_sdivr">;
-defm SVDIVR_U : SInstZPZZ<"svdivr", "UiUl", "aarch64_sve_udivr">;
-defm SVMAX_S  : SInstZPZZ<"svmax",  "csil", "aarch64_sve_smax">;
-defm SVMAX_U  : SInstZPZZ<"svmax",  "UcUsUiUl", "aarch64_sve_umax">;
-defm SVMIN_S  : SInstZPZZ<"svmin",  "csil", "aarch64_sve_smin">;
-defm SVMIN_U  : SInstZPZZ<"svmin",  "UcUsUiUl", "aarch64_sve_umin">;
-defm SVMUL: SInstZPZZ<"svmul",  "csilUcUsUiUl", "aarch64_sve_mul">;
-defm SVMULH_S : SInstZPZZ<"svmulh", "csil", "aarch64_sve_smulh">;
-defm SVMULH_U : SInstZPZZ<"svmulh", "UcUsUiUl", "aarch64_sve_umulh">;
-defm SVSUB: SInstZPZZ<"svsub",  "csilUcUsUiUl", "aarch64_sve_sub">;
-defm SVSUBR   : SInstZPZZ<"svsubr", "csilUcUsUiUl", "aarch64_sve_subr">;
+multiclass SInstZPZZ flags=[]> {
+  def _M   : SInst;

nit is it worth adding a `bit hasUndefVariant` and doing `!if (hasUndefVariant, 
intrinsic # "_u", intrinsic)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141939

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


[PATCH] D143051: [Clang][RISCV] Bump rvv intrinsics version to v0.11

2023-02-01 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.

LGTM, it's great to having a version number to distinguish different interface 
version


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143051

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


[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Manuel Brito via Phabricator via cfe-commits
ManuelJBrito updated this revision to Diff 493904.
ManuelJBrito retitled this revision from "[clang] Add builtin_nondet" to 
"[clang] Add builtin_nondeterministic_value".
ManuelJBrito edited the summary of this revision.
ManuelJBrito added a comment.
Herald added a subscriber: mgrang.

- Change name to `__builtin_nondeterministic_value`
- Add struct support




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-nondeterministic-value.c

Index: clang/test/CodeGen/builtins-nondeterministic-value.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-nondeterministic-value.c
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+typedef _Bool bool4 __attribute__((ext_vector_type(4)));
+struct testStruct{
+  int x;
+  int y;
+};
+
+int clang_nondet_i( int x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca i32, align 4
+// CHECK: store i32 [[X:%.*]], ptr [[A]], align 4
+// CHECK: [[R:%.*]] = freeze i32 poison
+// CHECK: ret i32 [[R]]
+  return __builtin_nondeterministic_value(x);
+} 
+
+float clang_nondet_f( float x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca float, align 4
+// CHECK: store float [[X:%.*]], ptr [[A]], align 4
+// CHECK: [[R:%.*]] = freeze float poison
+// CHECK: ret float [[R]]
+  return __builtin_nondeterministic_value(x);
+}
+
+double clang_nondet_d( double x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca double, align 8
+// CHECK: store double [[X:%.*]], ptr [[A]], align 8
+// CHECK: [[R:%.*]] = freeze double poison
+// CHECK: ret double [[R]]
+  return __builtin_nondeterministic_value(x);
+} 
+
+_Bool clang_nondet_b( _Bool x) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca i8, align 1
+// CHECK: [[B:%.*]] = zext i1 %x to i8
+// CHECK: store i8 [[B]], ptr [[A]], align 1
+// CHECK: [[R:%.*]] = freeze i1 poison
+// CHECK: ret i1 [[R]]
+  return __builtin_nondeterministic_value(x);
+} 
+
+float4 clang_nondet_fv( float4 x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca <4 x float>, align 16
+// CHECK: store <4 x float> [[X:%.*]], ptr [[A]], align 16
+// CHECK: [[R:%.*]] = freeze <4 x float> poison
+// CHECK: ret <4 x float> [[R]]
+  return __builtin_nondeterministic_value(x);
+} 
+
+bool4 clang_nondet_bv( bool4 x ) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca i8, align 1
+// CHECK: [[V:%.*]] = shufflevector <4 x i1> [[X:%.*]], <4 x i1> poison, <8 x i32> 
+// CHECK: [[B:%.*]] = bitcast <8 x i1> [[V]] to i8
+// CHECK: store i8 [[B]], ptr [[A]], align 1
+// CHECK: [[R:%.*]] = freeze <4 x i1> poison
+// CHECK: ret <4 x i1> [[R]]
+  return __builtin_nondeterministic_value(x);
+} 
+
+void clang_nondet_struct(struct testStruct x) {
+// CHECK-LABEL: entry
+// CHECK: [[A:%.*]] = alloca [[S:%.*]], align 4
+// CHECK: [[B:%.*]] = alloca [[S]], align 4
+// CHECK: [[R:%.*]] = freeze [[S]] poison
+// CHECK: store [[S]] [[R]], ptr [[B]], align 4
+// CHECK: ret void
+  x = __builtin_nondeterministic_value(x);
+} 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2580,6 +2580,12 @@
 break;
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+if (SemaBuiltinNonDeterministicValue(TheCall))
+  return ExprError();
+break;
+  }
+
   // __builtin_elementwise_abs restricts the element type to signed integers or
   // floating point types only.
   case Builtin::BI__builtin_elementwise_abs: {
@@ -17799,6 +17805,16 @@
   return false;
 }
 
+bool Sema::SemaBuiltinNonDeterministicValue(CallExpr *TheCall) {
+  if (checkArgCount(*this, TheCall, 1))
+return true;
+
+  ExprResult Arg = TheCall->getArg(0);
+  QualType TyArg = Arg.get()->getType();
+  TheCall->setType(TyArg);
+  return false;
+}
+
 ExprResult Sema::SemaBuiltinMatrixTranspose(CallExpr *TheCall,
 ExprResult CallResult) {
   if (checkArgCount(*this, TheCall, 1))
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3060,6 +3060,22 @@
 return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_nondeterministic_value: {
+QualType QT = E->getArg(0)->getType();
+llvm::Type * Ty = ConvertType(QT); 
+
+Value *Result = PoisonValue::get(Ty);
+Result = Builder.CreateFreeze(Result);
+
+if(Ty->isStructTy()){
+  Address StructAddr = ReturnValue.getValue();
+  Builder.CreateStore(Result, StructAddr);
+  return RValue::getAggregate(StructAddr);
+}
+
+r

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D142985#4095701 , @uabelho wrote:

> I wrote
>  https://github.com/llvm/llvm-project/issues/60437

Great, I'll land it once the patch is accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142985

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


[PATCH] D142932: Multilib YAML parsing

2023-02-01 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added inline comments.



Comment at: clang/lib/Driver/Multilib.cpp:82
+  for (const Matcher &M : Matchers) {
+const std::regex Regex(M.Regex);
+if (std::find_if(InFlags.begin(), InFlags.end(),

Please use the LLVM regular expression engine (llvm/Support/Regex.h) instead of 
std::regex. It's more portable, e.g., I've seen bugs and performance issues 
with MSVC implementation that don't manifest on Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142932

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


[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

2023-02-01 Thread Paulo Matos via Phabricator via cfe-commits
pmatos marked 11 inline comments as done.
pmatos added inline comments.



Comment at: clang/lib/Basic/Targets/DirectX.h:45
 3, // hlsl_groupshared
+// Wasm address space values for this map are dummy
+20, // wasm_funcref

erichkeane wrote:
> pmatos wrote:
> > erichkeane wrote:
> > > Also apply to the rest of the targets.  Also, why is there only 1 added 
> > > here?  I thought we had 2 address spaces for Wasm?
> > Externref is a type in clang, doesn't need its own address space in clang, 
> > only when it's lowered to a opaque type in llvm it gets its own address 
> > space.
> Please fix the comment in my suggestion.
> 
> Also, your explanation doesn't make it clear to me why you added TWO entries 
> to this list for other targets, but only 1 here.
I have now fixed that. We just need one address space.



Comment at: clang/lib/Basic/Targets/WebAssembly.h:44-45
+0,  // ptr64
+10, // wasm_externref,
+20, // wasm_funcref
+};

erichkeane wrote:
> aaron.ballman wrote:
> > I'm not super familiar with the address space maps, but this sets of my 
> > spidey senses. All of the other address space maps end with:
> > 
> > ptr64, hlsl_groupshared, wasm_funcref
> > 
> > but this one is ending with:
> > 
> > ptr64, wasm_externref, wasm_funcref
> > 
> > which makes me think something's amiss here. Thoughts?
> That DEFINITELY looks suspicious to me as well, I noted above too, but either 
> we need to specify we're re-using the `hlsl_groupshared` for `wasm_externref` 
> address space (at which point, i wonder: why not re-use a different address 
> space?), or correct this.
Fixed this now. We only need the address space for funcref. The address space 
for externref is now necessary as it's its own type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

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


[PATCH] D140897: [clang][dataflow] Fix handling of `DeclRefExpr`s to `BindingDecl`s.

2023-02-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

In D140897#4094945 , @xazax.hun wrote:

> I hope we will be able to get rid of this `SkipPast` thing at some point by 
> looking at the value categories of the AST instead.

Thanks! Agreed: filed issue #60444.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140897

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


[PATCH] D142932: Multilib YAML parsing

2023-02-01 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added inline comments.



Comment at: clang/lib/Driver/Multilib.cpp:83
+const std::regex Regex(M.Regex);
+if (std::find_if(InFlags.begin(), InFlags.end(),
+ [&Regex](const std::string &F) {

Please use `llvm::find_if(InFlags, ...)`



Comment at: clang/lib/Driver/Multilib.cpp:153
+  static std::string validate(IO &io, MultilibFlagMap::Matcher &M) {
+if (M.MatchFlags.empty() && M.NoMatchFlags.empty())
+  return "value required for 'matchFlags' or 'noMatchFlags'";

I think it would make sense to validate regular expression syntax as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142932

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


[PATCH] D142710: [clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

2023-02-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

In D142710#4094934 , @xazax.hun wrote:

> This change looks good to me. I wonder, however, whether the behavior should 
> be parameterized in the future. E.g., whether the user of the analysis should 
> be able to make a decision whether the analysis should be pessimistic or 
> optimistic about unmodeled values.

Interesting idea. I think this goes along with other places where we are 
unsound. Here, we err on the side of soundness. but, in general, we should have 
a configuration mechanism for this.  FWIW, the only reason we have 
uninitialized values at this point is recursive types. We also limit the depth 
of structs, but that should be removed given my recent patch to only model 
relevant fields. I have an idea for lazy initialization of values that I think 
could solve the recursion issue. Together, we could remove this concept of 
unmodeled values altogether from the framework.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142710

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


[clang] 0256280 - [clang][dataflow] Fix handling of `DeclRefExpr`s to `BindingDecl`s.

2023-02-01 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2023-02-01T13:23:23Z
New Revision: 02562804d074a4d6e041e00572483fe25932186e

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

LOG: [clang][dataflow] Fix handling of `DeclRefExpr`s to `BindingDecl`s.

The invariants around `ReferenceValues` are subtle (arguably, too much so). That
includes that we need to take care not to double wrap them -- in cases where we
wrap a loc in an `ReferenceValue` we need to be sure that the pointee isn't
already a `ReferenceValue`.  `BindingDecl` introduces another situation in which
this can arise. Previously, the code did not properly handle `BindingDecl`,
resulting in double-wrapped values, which broke other invariants (at least, that
struct values have an `AggregateStorageLocation`).

This patch adjusts the interpretation of `DeclRefExpr` to take `BindingDecl`'s
peculiarities into account. It also fixes the two tests which should have caught
this issue but were themselves (subtly) buggy.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 0e6c484b67e7d..74dd59851ad44 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -108,7 +108,8 @@ static BoolValue &unpackValue(BoolValue &V, Environment 
&Env) {
 }
 
 // Unpacks the value (if any) associated with `E` and updates `E` to the new
-// value, if any unpacking occured.
+// value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
+// by skipping past the reference.
 static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
   // FIXME: this is too flexible: it _allows_ a reference, while it should
   // _require_ one, since lvalues should always be wrapped in `ReferenceValue`.
@@ -146,7 +147,9 @@ class TransferVisitor : public 
ConstStmtVisitor {
   if (LHSLoc == nullptr)
 break;
 
-  auto *RHSVal = Env.getValue(*RHS, SkipPast::Reference);
+  // No skipping should be necessary, because any lvalues should have
+  // already been stripped off in evaluating the LValueToRValue cast.
+  auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
   if (RHSVal == nullptr)
 break;
 
@@ -196,9 +199,17 @@ class TransferVisitor : public 
ConstStmtVisitor {
 if (DeclLoc == nullptr)
   return;
 
-if (VD->getType()->isReferenceType()) {
-  assert(isa_and_nonnull(Env.getValue((*DeclLoc))) &&
- "reference-typed declarations map to `ReferenceValue`s");
+// If the value is already an lvalue, don't double-wrap it.
+if (isa_and_nonnull(Env.getValue(*DeclLoc))) {
+  // We only expect to encounter a `ReferenceValue` for a reference type
+  // (always) or for `BindingDecl` (sometimes). For the latter, we can't
+  // rely on type, because their type does not indicate whether they are a
+  // reference type. The assert is not strictly necessary, since we don't
+  // depend on its truth to proceed. But, it verifies our assumptions,
+  // which, if violated, probably indicate a problem elsewhere.
+  assert((VD->getType()->isReferenceType() || isa(VD)) &&
+ "Only reference-typed declarations or `BindingDecl`s should map "
+ "to `ReferenceValue`s");
   Env.setStorageLocation(*S, *DeclLoc);
 } else {
   auto &Loc = Env.createStorageLocation(*S);
@@ -238,6 +249,7 @@ class TransferVisitor : public 
ConstStmtVisitor {
 if (D.getType()->isReferenceType()) {
   // Initializing a reference variable - do not create a reference to
   // reference.
+  // FIXME: reuse the ReferenceValue instead of creating a new one.
   if (auto *InitExprLoc =
   Env.getStorageLocation(*InitExpr, SkipPast::Reference)) {
 auto &Val =
@@ -264,6 +276,9 @@ class TransferVisitor : public 
ConstStmtVisitor {
 Env.setValue(Loc, *Val);
 }
 
+// `DecompositionDecl` must be handled after we've interpreted the loc
+// itself, because the binding expression refers back to the
+// `DecompositionDecl` (even though it has no written name).
 if (const auto *Decomp = dyn_cast(&D)) {
   // If VarDecl is a DecompositionDecl, evaluate each of its bindings. This
   // needs to be evaluated after initializing the values in the storage for
@@ -288,7 +303,8 @@ class TransferVisitor : public 
ConstStmtVisitor {
   // types. The holding var declarations appear *after* this statement,
   // so we have to create a location for them here to share with `B`. 
We
   // don't 

[PATCH] D140897: [clang][dataflow] Fix handling of `DeclRefExpr`s to `BindingDecl`s.

2023-02-01 Thread Yitzhak Mandelbaum 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 rG02562804d074: [clang][dataflow] Fix handling of 
`DeclRefExpr`s to `BindingDecl`s. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140897

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3893,6 +3893,8 @@
 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
 ASSERT_THAT(BazDecl, NotNull());
 
+// BindingDecls always map to references -- either lvalue or rvalue, so
+// we still need to skip here.
 const Value *BoundFooValue =
 Env1.getValue(*BoundFooDecl, SkipPast::Reference);
 ASSERT_THAT(BoundFooValue, NotNull());
@@ -3904,13 +3906,13 @@
 EXPECT_TRUE(isa(BoundBarValue));
 
 // Test that a `DeclRefExpr` to a `BindingDecl` works as expected.
-EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::None), BoundFooValue);
 
 const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
 
 // Test that `BoundFooDecl` retains the value we expect, after the join.
 BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
-EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::None), BoundFooValue);
   });
 }
 
@@ -3988,16 +3990,15 @@
 // works as expected. We don't test aliasing properties of the
 // reference, because we don't model `std::get` and so have no way to
 // equate separate references into the tuple.
-EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env1.getValue(*BazDecl, SkipPast::None), BoundFooValue);
 
 const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
 
 // Test that `BoundFooDecl` retains the value we expect, after the join.
 BoundFooValue = Env2.getValue(*BoundFooDecl, SkipPast::Reference);
-EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::Reference), BoundFooValue);
+EXPECT_EQ(Env2.getValue(*BazDecl, SkipPast::None), BoundFooValue);
   });
 }
-// TODO: ref binding
 
 TEST(TransferTest, BinaryOperatorComma) {
   std::string Code = R"(
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -108,7 +108,8 @@
 }
 
 // Unpacks the value (if any) associated with `E` and updates `E` to the new
-// value, if any unpacking occured.
+// value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
+// by skipping past the reference.
 static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
   // FIXME: this is too flexible: it _allows_ a reference, while it should
   // _require_ one, since lvalues should always be wrapped in `ReferenceValue`.
@@ -146,7 +147,9 @@
   if (LHSLoc == nullptr)
 break;
 
-  auto *RHSVal = Env.getValue(*RHS, SkipPast::Reference);
+  // No skipping should be necessary, because any lvalues should have
+  // already been stripped off in evaluating the LValueToRValue cast.
+  auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
   if (RHSVal == nullptr)
 break;
 
@@ -196,9 +199,17 @@
 if (DeclLoc == nullptr)
   return;
 
-if (VD->getType()->isReferenceType()) {
-  assert(isa_and_nonnull(Env.getValue((*DeclLoc))) &&
- "reference-typed declarations map to `ReferenceValue`s");
+// If the value is already an lvalue, don't double-wrap it.
+if (isa_and_nonnull(Env.getValue(*DeclLoc))) {
+  // We only expect to encounter a `ReferenceValue` for a reference type
+  // (always) or for `BindingDecl` (sometimes). For the latter, we can't
+  // rely on type, because their type does not indicate whether they are a
+  // reference type. The assert is not strictly necessary, since we don't
+  // depend on its truth to proceed. But, it verifies our assumptions,
+  // which, if violated, probably indicate a problem elsewhere.
+  assert((VD->getType()->isReferenceType() || isa(VD)) &&
+ "Only reference-typed declarations or `BindingDecl`s should map "
+ "to `ReferenceValue`s");
   Env.setStorageLocation(*S, *DeclLoc);
 } else {
   auto &Loc = Env.createStorageLocation(*S);
@@ -238,6 +249,7 @@
 

[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you!

Btw, if you change:

`Fix https://github.com/llvm/llvm-project/issues/58375`

into:

`Fixes #58375`

in the commit message, it will automatically close the github issue and 
associate it with this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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


[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

2023-02-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:243
+/// - Strict
+std::optional> Mode;
+

I think "Diagnostics.Mode" is too vague a name.

I expect this to be a rollout flag that we remove in the medium term (either 
deciding to stick to the old or new behavior), so maybe something 
ultra-specific like`AllowStalePreamble: yes/no`?
(Preamble is jargon, maybe there's something better, but again I don't think we 
should plan for this to be really "user-visible")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142890

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


[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

@aaron.ballman It works with the URL as well. The URL is preferred, because 
that way people don't have to reconstruct it when seeing the issue reference on 
phabricator, where it does not get auto-linked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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


[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

2023-02-01 Thread Paulo Matos via Phabricator via cfe-commits
pmatos marked an inline comment as done.
pmatos added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:4129
+  let Documentation = [WebAssemblyExportNameDocs];
+  let Subjects = SubjectList<[TypedefName], ErrorDiag>;
+}

aaron.ballman wrote:
> erichkeane wrote:
> > pmatos wrote:
> > > erichkeane wrote:
> > > > pmatos wrote:
> > > > > erichkeane wrote:
> > > > > > Note that it seems this is likely not going to work correctly on 
> > > > > > template type aliases!  You probably want to make sure that is 
> > > > > > acceptable to you.
> > > > > Documentation says about Subject:
> > > > > 
> > > > > ```
> > > > >   // The things to which an attribute can appertain
> > > > >   SubjectList Subjects;
> > > > > ```
> > > > > 
> > > > > Given this can be attached to function pointer types, is the subject 
> > > > > then just Type ?
> > > > IIRC, the list of subjects are the Decls and Stmt types that are 
> > > > possibly allowed.  I don't thnk it would be just 'Type' there.
> > > > 
> > > > As you have it, it is only allowed on TypedefNameDecl.
> > > > 
> > > > See the SubsetSubject's at the top of this file for some examples on 
> > > > how you could constrain a special matcher to only work on function 
> > > > pointer decls.
> > > Good point @erichkeane . What do you think of FunctionPointer 
> > > implementation here?
> > That seems acceptable to me.
> That makes sense to me as well, but FWIW, type attributes don't currently 
> have much of any tablegen automation (unlike decl and stmt attributes), so 
> the subject list isn't strictly necessary here. But it's still appreciated 
> because 1) it's documentation, and 2) we want to tablegen more and this helps 
> us with that goal.
@erichkeane Yes, you're right. Something like 

```
using IntIntFuncref = int(*)(int) __funcref;
```

doesn't work. Why is this not accepted? Is it due to the Subjects listed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

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


[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 493918.
VitaNuo added a comment.

Add a couple more test cases to StandardLibraryTest.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

Files:
  clang/include/clang/Tooling/Inclusions/StandardLibrary.h
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/unittests/Tooling/StandardLibraryTest.cpp

Index: clang/unittests/Tooling/StandardLibraryTest.cpp
===
--- clang/unittests/Tooling/StandardLibraryTest.cpp
+++ clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/Testing/CommandLineArgs.h"
 #include "clang/Testing/TestAST.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -17,6 +18,7 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 using ::testing::ElementsAre;
 
@@ -103,6 +105,56 @@
   EXPECT_EQ(Recognizer(Sec), std::nullopt);
 }
 
+TEST(StdlibTest, LanguageSeparationForSymbols) {
+  EXPECT_NE(std::nullopt, stdlib::Symbol::named("std::", "vector"));
+  EXPECT_NE(std::nullopt,
+stdlib::Symbol::named("std::", "vector", stdlib::Lang::CXX));
+  EXPECT_EQ(std::nullopt,
+stdlib::Symbol::named("std::", "vector", stdlib::Lang::C));
+
+  EXPECT_EQ(std::nullopt, stdlib::Symbol::named("", "int16_t"));
+  EXPECT_EQ(std::nullopt,
+stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX));
+  EXPECT_NE(std::nullopt,
+stdlib::Symbol::named("", "int16_t", stdlib::Lang::C));
+}
+
+TEST(StdlibTest, LanguageSeparationForHeaders) {
+  EXPECT_NE(std::nullopt, stdlib::Header::named("vector"));
+  EXPECT_NE(std::nullopt, stdlib::Header::named("vector", stdlib::Lang::CXX));
+  EXPECT_EQ(std::nullopt, stdlib::Header::named("vector", stdlib::Lang::C));
+
+  EXPECT_EQ(std::nullopt, stdlib::Header::named("stdint.h"));
+  EXPECT_EQ(std::nullopt, stdlib::Header::named("stdint.h", stdlib::Lang::CXX));
+  EXPECT_NE(std::nullopt, stdlib::Header::named("stdint.h", stdlib::Lang::C));
+}
+
+TEST(StdlibTest, LanguageSeparationForRecognizer) {
+  llvm::StringRef Code = R"cpp(
+ std::vector vec;
+  )cpp";
+  TestInputs Inputs(Code);
+  Inputs.Language = TestLanguage::Lang_CXX11;
+  TestAST AST(Inputs);
+  auto *Vec = cast(lookup(AST, "vec")).getType()->getAsCXXRecordDecl();
+
+  stdlib::Recognizer Recognizer;
+  EXPECT_EQ(Recognizer(Vec), stdlib::Symbol::named("std::", "vector"));
+  EXPECT_EQ(Recognizer(Vec),
+stdlib::Symbol::named("std::", "vector", stdlib::Lang::CXX));
+
+  Code = R"c(
+ int16_t var = 42;
+  )c";
+  Inputs = TestInputs(Code);
+  Inputs.Language = TestLanguage::Lang_C99;
+  AST = TestAST(Inputs);
+
+  auto *Var = cast(lookup(AST, "var")).getType()->getAsRecordDecl();
+  EXPECT_EQ(Recognizer(Var),
+stdlib::Symbol::named("", "int16_t", stdlib::Lang::C));
+}
+
 } // namespace
 } // namespace tooling
 } // namespace clang
Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -8,6 +8,8 @@
 
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/AST/Decl.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/LangStandard.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 
@@ -15,35 +17,54 @@
 namespace tooling {
 namespace stdlib {
 
-static llvm::StringRef *HeaderNames;
-static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
-static llvm::DenseMap *HeaderIDs;
 // Maps symbol name -> Symbol::ID, within a namespace.
 using NSSymbolMap = llvm::DenseMap;
-static llvm::DenseMap *NamespaceSymbols;
 
-static int initialize() {
+struct SymbolHeaderMapping {
+  llvm::StringRef *HeaderNames;
+  std::pair *SymbolNames;
+  unsigned *SymbolHeaderIDs;
+  llvm::DenseMap HeaderIDs;
+  llvm::DenseMap NamespaceSymbols;
+};
+
+static llvm::DenseMap LanguageMappings;
+
+static int countSymbols(Lang Language) {
   unsigned SymCount = 0;
 #define SYMBOL(Name, NS, Header) ++SymCount;
+  switch (Language) {
+  case Lang::C:
 #include "clang/Tooling/Inclusions/CSymbolMap.inc"
+break;
+  case Lang::CXX:
 #include "clang/Tooling/Inclusions/StdSymbolMap.inc"
+break;
+  }
 #undef SYMBOL
-  SymbolNames = new std::remove_reference_t[SymCount];
-  SymbolHeaderIDs =
-  new std::remove_reference_t[SymCount];
-  NamespaceSymbols = new std::remove_reference_t;
-  HeaderIDs = new std::remove_reference_t;
+  return SymCount;
+}
+
+static void initialize(Lang Language) {
+  SymbolHeaderMapping *Mapping = new SymbolHeaderMapping();
+  LanguageMappings.try_emplace(Language, Mapping);
+
+  unsigned SymCount = countSymbols(Language);
+  Mapping->Sy

[PATCH] D142992: [include-mapping] Implement language separation in stdlib recognizer library

2023-02-01 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added a comment.

Added a couple of test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142992

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


[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

2023-02-01 Thread Paulo Matos via Phabricator via cfe-commits
pmatos updated this revision to Diff 493919.
pmatos marked an inline comment as done.
pmatos added a comment.

Address some of the pending comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AddressSpaces.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Format/FormatToken.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/WebAssembly/wasm-funcref.c
  clang/test/Parser/wasm-funcref.c
  clang/test/SemaCXX/wasm-funcref.cpp
  clang/test/SemaTemplate/address_space-dependent.cpp

Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@
 
 template 
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388587)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388586)}}
 }
 
 template 
@@ -101,7 +101,7 @@
   car<1, 2, 3>(); // expected-note {{in instantiation of function template specialization 'car<1, 2, 3>' requested here}}
   HasASTemplateFields<1> HASTF;
   neg<-1>(); // expected-note {{in instantiation of function template specialization 'neg<-1>' requested here}}
-  correct<0x7FFFEA>();
+  correct<0x7FFFE9>();
   tooBig<8388650>(); // expected-note {{in instantiation of function template specialization 'tooBig<8388650L>' requested here}}
 
   __attribute__((address_space(1))) char *x;
Index: clang/test/SemaCXX/wasm-funcref.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/wasm-funcref.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fsyntax-only -verify -triple wasm32 -Wno-unused-value -target-feature +reference-types %s
+
+// Testing that funcrefs work on template aliases
+using IntIntFuncref = int(*)(int);// __funcref;
+
+int get(int);
+
+IntIntFuncref getFuncref() {
+return get;
+}
\ No newline at end of file
Index: clang/test/Parser/wasm-funcref.c
===
--- /dev/null
+++ clang/test/Parser/wasm-funcref.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -fsyntax-only -verify %s
+
+// Test that we trigger an error at parse time if using keyword funcref
+// while not using a wasm triple.
+typedef void (*__funcref funcref_t)(); // expected-error {{invalid use of __funcref keyword outside the WebAssembly triple}}
+typedef int (*__funcref fn_funcref_t)(int);// expected-error {{invalid use of __funcref keyword outside the WebAssembly triple}}
+typedef int (*fn_t)(int);
+
+static fn_funcref_t nullFuncref = 0;
Index: clang/test/CodeGen/WebAssembly/wasm-funcref.c
===
--- /dev/null
+++ clang/test/CodeGen/WebAssembly/wasm-funcref.c
@@ -0,0 +1,84 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple wasm32 -target-feature +reference-types -o - -emit-llvm %s | FileCheck %s
+
+typedef void (*__funcref funcref_t)();
+typedef int (*__funcref fn_funcref_t)(int);
+typedef int (*fn_t)(int);
+
+// Null funcref builtin call
+// CHECK-LABEL: @get_null(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call ptr addrspace(20) @llvm.wasm.ref.null.func()
+// CHECK-NEXT:ret ptr addrspace(20) [[TMP0]]
+//
+funcref_t get_null() {
+  return __builtin_wasm_ref_null_func();
+}
+
+// Call to null funcref builtin but requires cast since
+// default return value for builtin is a funcref with function type () -> ().
+// CHECK-LABEL: @get_null_ii(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call ptr addrspace(20) @llvm.wasm.ref.null.func()
+// CHECK-NEXT:ret ptr addrspace(20) [[TMP0]]
+//
+fn_funcref_t get_null_ii() {
+  return (fn_funcref_t) __builtin_wasm_ref_n

[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho resigned from this revision.
uabelho added a comment.

In D142985#4096297 , @jhuber6 wrote:

> In D142985#4095701 , @uabelho wrote:
>
>> I wrote
>>  https://github.com/llvm/llvm-project/issues/60437
>
> Great, I'll land it once the patch is accepted.

Thanks! Thumbs up from me, unfortunately I don't feel confident to review the 
actual code change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142985

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


[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

2023-02-01 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy added inline comments.



Comment at: clang/test/AST/ast-dump-overloaded-operators.cpp:27
 // CHECK-NEXT: | `-ParmVarDecl {{.*}}  col:19{{( imported)?}} 'E'
-// CHECK-NEXT: `-FunctionDecl {{.*}}  line:14:6{{( 
imported)?}} test 'void ()'
+// CHECK-NEXT:  -FunctionDecl {{.*}}  line:14:6{{( 
imported)?}} test 'void ()'
 // CHECK-NEXT:   `-CompoundStmt {{.*}} 

Why did a backtick disappear here and in many other locations? Is this somehow 
related to va_list handling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142822

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


[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D142826#4096355 , @nikic wrote:

> @aaron.ballman It works with the URL as well. The URL is preferred, because 
> that way people don't have to reconstruct it when seeing the issue reference 
> on phabricator, where it does not get auto-linked.

Oh, that's really good to know, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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


[clang] 95668c0 - [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Shivam Gupta via cfe-commits

Author: Shivam Gupta
Date: 2023-02-01T19:29:16+05:30
New Revision: 95668c0d97e6184729f3a3e9621a58d9edffb6b0

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

LOG: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

GCC added the -Wtype-limits warning group to -Wextra around
GCC 4.4 and the group has some very helpful extra warnings
like tautological comparison type limit warnings
(comparingan unsigned int to see if it's positive, etc).

Fix https://github.com/llvm/llvm-project/issues/58375

Reviewed By: #clang-vendors, thesamesam

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/test/Sema/tautological-constant-compare.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2b2ca8b2987f0..e5d09ce61778e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -61,6 +61,9 @@ Bug Fixes
   templates. This fixes
   `Issue 60344 `_.
 
+- ``-Wtype-limits`` was added to ``-Wextra`` for GCC compatibility. This fixes
+  `Issue 58375 `_.
+
 Improvements to Clang's diagnostics
 ^^^
 - We now generate a diagnostic for signed integer overflow due to unary minus

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 6c997c37cc5cd..39e6c94fe6e29 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -992,6 +992,7 @@ def Extra : DiagGroup<"extra", [
 EmptyInitStatement,
 StringConcatation,
 FUseLdPath,
+TypeLimits,
   ]>;
 
 def Most : DiagGroup<"most", [

diff  --git a/clang/test/Sema/tautological-constant-compare.c 
b/clang/test/Sema/tautological-constant-compare.c
index 04b8a1416be0b..59094186899dd 100644
--- a/clang/test/Sema/tautological-constant-compare.c
+++ b/clang/test/Sema/tautological-constant-compare.c
@@ -4,8 +4,8 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only 
-Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST 
-verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST 
-verify -x c++ %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -verify=silent %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -verify=silent -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -DTEST -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent 
%s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent 
-x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s



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


[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Shivam Gupta 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 rG95668c0d97e6: [Clang] Add -Wtype-limits to -Wextra for GCC 
compatibility (authored by xgupta).

Changed prior to commit:
  https://reviews.llvm.org/D142826?vs=493810&id=493924#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Sema/tautological-constant-compare.c


Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -4,8 +4,8 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only 
-Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST 
-verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST 
-verify -x c++ %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -verify=silent %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -verify=silent -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -DTEST -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra 
-Wno-sign-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent 
%s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent 
-x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -992,6 +992,7 @@
 EmptyInitStatement,
 StringConcatation,
 FUseLdPath,
+TypeLimits,
   ]>;
 
 def Most : DiagGroup<"most", [
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,9 @@
   templates. This fixes
   `Issue 60344 `_.
 
+- ``-Wtype-limits`` was added to ``-Wextra`` for GCC compatibility. This fixes
+  `Issue 58375 `_.
+
 Improvements to Clang's diagnostics
 ^^^
 - We now generate a diagnostic for signed integer overflow due to unary minus


Index: clang/test/Sema/tautological-constant-compare.c
===
--- clang/test/Sema/tautological-constant-compare.c
+++ clang/test/Sema/tautological-constant-compare.c
@@ -4,8 +4,8 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify -x c++ %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -verify=silent %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -verify=silent -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -DTEST -verify %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wextra -Wno-sign-compare -DTEST -verify -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wall -verify=silent -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify=silent %s
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -992,6 +992,7 @@
 EmptyInitStatement,
 StringConcatation,
 FUseLdPath,
+TypeLimits,
   ]>;
 
 def Most : DiagGroup<"most", [
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -61,6 +61,9 @@
   templates. This fixes
   `Issue 60344 `_.
 
+- ``-Wtype-limits`` was added to ``-Wextra`` for GCC compatibility. This fixes
+  `Issue 58375 `_.
+
 Improvements to Clang's diagnostics
 ^^^
 - We now generate a diagnostic for signed integer overflow due to unary minus

[PATCH] D142826: [Clang] Add -Wtype-limits to -Wextra for GCC compatibility

2023-02-01 Thread Shivam Gupta via Phabricator via cfe-commits
xgupta added a comment.

In D142826#4096348 , @aaron.ballman 
wrote:

> LGTM, thank you!

Thank you for reviewing and helping it to commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142826

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


[PATCH] D142384: [C++20] Fix a crash with modules.

2023-02-01 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 493926.
usaxena95 added a comment.

Moved to RecordDecl::field_begin. Assertion is no more valid in RecordDecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

Files:
  clang/lib/AST/Decl.cpp


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4769,7 +4769,8 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 


Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4769,7 +4769,8 @@
 RecordDecl::field_iterator RecordDecl::field_begin() const {
   if (hasExternalLexicalStorage() && !hasLoadedFieldsFromExternalStorage())
 LoadFieldsFromExternalStorage();
-
+  if (RecordDecl *D = getDefinition(); D && D != this)
+return D->field_begin();
   return field_iterator(decl_iterator(FirstDecl));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 accepted this revision.
tianshilei1992 added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142985

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


[PATCH] D142985: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Joseph Huber 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 rG9c4591d7f3ac: [LinkerWrapper] Fix memory issues due to 
unguarded accesses to global state (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142985

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -77,6 +77,9 @@
 /// Binary path for the CUDA installation.
 static std::string CudaBinaryPath;
 
+/// Mutex lock to protect writes to shared TempFiles in parallel.
+static std::mutex TempFilesMutex;
+
 /// Temporary files created by the linker wrapper.
 static std::list> TempFiles;
 
@@ -200,6 +203,7 @@
 
 /// Get a temporary filename suitable for output.
 Expected createOutputFile(const Twine &Prefix, StringRef Extension) 
{
+  std::scoped_lock Lock(TempFilesMutex);
   SmallString<128> OutputFile;
   if (SaveTemps) {
 (Prefix + "." + Extension).toNullTerminatedStringRef(OutputFile);
@@ -1047,6 +1051,7 @@
   return createFileError(*OutputOrErr, EC);
   }
 
+  std::scoped_lock Guard(ImageMtx);
   OffloadingImage TheImage{};
   TheImage.TheImageKind =
   Args.hasArg(OPT_embed_bitcode) ? IMG_Bitcode : IMG_Object;
@@ -1058,7 +1063,6 @@
Args.MakeArgString(LinkerArgs.getLastArgValue(OPT_arch_EQ))}};
   TheImage.Image = std::move(*FileOrErr);
 
-  std::lock_guard Guard(ImageMtx);
   Images[Kind].emplace_back(std::move(TheImage));
 }
 return Error::success();


Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -77,6 +77,9 @@
 /// Binary path for the CUDA installation.
 static std::string CudaBinaryPath;
 
+/// Mutex lock to protect writes to shared TempFiles in parallel.
+static std::mutex TempFilesMutex;
+
 /// Temporary files created by the linker wrapper.
 static std::list> TempFiles;
 
@@ -200,6 +203,7 @@
 
 /// Get a temporary filename suitable for output.
 Expected createOutputFile(const Twine &Prefix, StringRef Extension) {
+  std::scoped_lock Lock(TempFilesMutex);
   SmallString<128> OutputFile;
   if (SaveTemps) {
 (Prefix + "." + Extension).toNullTerminatedStringRef(OutputFile);
@@ -1047,6 +1051,7 @@
   return createFileError(*OutputOrErr, EC);
   }
 
+  std::scoped_lock Guard(ImageMtx);
   OffloadingImage TheImage{};
   TheImage.TheImageKind =
   Args.hasArg(OPT_embed_bitcode) ? IMG_Bitcode : IMG_Object;
@@ -1058,7 +1063,6 @@
Args.MakeArgString(LinkerArgs.getLastArgValue(OPT_arch_EQ))}};
   TheImage.Image = std::move(*FileOrErr);
 
-  std::lock_guard Guard(ImageMtx);
   Images[Kind].emplace_back(std::move(TheImage));
 }
 return Error::success();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9c4591d - [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

2023-02-01 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2023-02-01T08:12:10-06:00
New Revision: 9c4591d7f3acaa00318900bdba4b4ba26c99c666

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

LOG: [LinkerWrapper] Fix memory issues due to unguarded accesses to global state

There were intemittent errors in the linker wrapper when using the
sanitizers in parallel. First, this is because the `TempFiles` global
was not guarded when creating a new file. Second, even though the `Args`
list is passed as const, the internal state is mutable when adding a
string. So that needs to be guarded too.

Fixes https://github.com/llvm/llvm-project/issues/60437

Reviewed By: tianshilei1992

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

Added: 


Modified: 
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Removed: 




diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp 
b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 3c92d34571044..6981e124b1062 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -77,6 +77,9 @@ static StringRef ExecutableName;
 /// Binary path for the CUDA installation.
 static std::string CudaBinaryPath;
 
+/// Mutex lock to protect writes to shared TempFiles in parallel.
+static std::mutex TempFilesMutex;
+
 /// Temporary files created by the linker wrapper.
 static std::list> TempFiles;
 
@@ -200,6 +203,7 @@ std::string getMainExecutable(const char *Name) {
 
 /// Get a temporary filename suitable for output.
 Expected createOutputFile(const Twine &Prefix, StringRef Extension) 
{
+  std::scoped_lock Lock(TempFilesMutex);
   SmallString<128> OutputFile;
   if (SaveTemps) {
 (Prefix + "." + Extension).toNullTerminatedStringRef(OutputFile);
@@ -1047,6 +1051,7 @@ linkAndWrapDeviceFiles(SmallVectorImpl 
&LinkerInputFiles,
   return createFileError(*OutputOrErr, EC);
   }
 
+  std::scoped_lock Guard(ImageMtx);
   OffloadingImage TheImage{};
   TheImage.TheImageKind =
   Args.hasArg(OPT_embed_bitcode) ? IMG_Bitcode : IMG_Object;
@@ -1058,7 +1063,6 @@ linkAndWrapDeviceFiles(SmallVectorImpl 
&LinkerInputFiles,
Args.MakeArgString(LinkerArgs.getLastArgValue(OPT_arch_EQ))}};
   TheImage.Image = std::move(*FileOrErr);
 
-  std::lock_guard Guard(ImageMtx);
   Images[Kind].emplace_back(std::move(TheImage));
 }
 return Error::success();



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


[PATCH] D142872: Honor the fwrapv option when generating the inbounds GEP .

2023-02-01 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

I've found another bug in LLVM, fixed with 
https://github.com/llvm/llvm-project/commit/78f88082de3627ea04501c83a08f52cf1e60b4f7.
 After that change, the test case from 
https://github.com/llvm/llvm-project/issues/59580 appears to be handled 
correctly, it contains this in the final IR:

  br i1 icmp ugt (ptr getelementptr ([0 x i8], ptr @end, i64 0, i64 536870911), 
ptr @end), label %if.then, label %if.else9

As such, I'm not sure if we actually need any change in Clang, as it seems like 
this was ultimately due to LLVM bugs. At least we would need a different test 
case, as the `inbounds.c` test case is already getting compiled correctly 
without this patch.




Comment at: llvm/include/llvm/IR/ConstantFold.h:29
   template  class ArrayRef;
+  inline bool SignedWrap = false;
+

These changes shouldn't be needed anymore.


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

https://reviews.llvm.org/D142872

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


[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a subscriber: cor3ntin.
erichkeane added a comment.

@cor3ntin did a bunch of work here lately, so he's probably a better reviewer 
than I am.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143053

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


[PATCH] D143021: [OpenMP 5.2] Deprecate 'destroy' clause without argument for 'depobj' construct

2023-02-01 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:496
 
+def err_destroy_clause_without_argument
+: Error<"'destroy' clause without argument on '#pragma omp depobj' "

1. Usually we name them err_omp_... Also, move to other err_omp messages.
2. Better to use something like "Expected depobj with argument if destroy 
clause is specified" or something like this.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:17666
+  if (!InteropVar && (LangOpts.OpenMP >= 52 &&
+  DSAStack->getCurrentDirective() == OMPD_depobj)) {
+Diag(StartLoc, diag::err_destroy_clause_without_argument);

Can it be used with any other directive except for depobj?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143021

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


[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.

In D142401#4082356 , @cor3ntin wrote:

> In D142401#4074959 , @shafik wrote:
>
>> I could not find any tests that test for this warning.
>
> I'm unable to trigger the warning at all. I feel i should but just using that 
> function resolves the crash, probably because it allocates more stack in a 
> separate thread, and there is actually some code that avoids marking the same 
> object used twice.
> But the warning might still trigger on some platform, so i suspect the test i 
> added might, sometimes, trigger a warning. I'm not exactly sure how to handle 
> that

Tests for stack space are always really challenging because triggering the 
failure is so system specific. We only have one test for it 
(`test/SemaTemplate/stack-exhaustion.cpp`)

But do we need to run these on separate threads in all of those places to 
address the issue?

"and there is actually some code that avoids marking the same object used 
twice." makes me wonder if we're missing an early return somewhere that would 
reduce stack usage more and avoid needing to spin up threads for this. (The 
upside to this change is that the code compiles more often but the downside is 
that we're not addressing the memory pressure issue, just kicking the can down 
the road a bit.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Valentin Clement via Phabricator via cfe-commits
clementval added a comment.

The new test embed.f90 and embed-error.f90 are failing on my side. Do they 
assume a specific configuration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D141324: [clang] extend external_source_symbol attribute with the USR clause

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

That part is Ok now, thanks.  However, this still needs a release note, plus a 
test for spelling versions.


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

https://reviews.llvm.org/D141324

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


[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D142401#4096478 , @aaron.ballman 
wrote:

> In D142401#4082356 , @cor3ntin 
> wrote:
>
>> In D142401#4074959 , @shafik wrote:
>>
>>> I could not find any tests that test for this warning.
>>
>> I'm unable to trigger the warning at all. I feel i should but just using 
>> that function resolves the crash, probably because it allocates more stack 
>> in a separate thread, and there is actually some code that avoids marking 
>> the same object used twice.
>> But the warning might still trigger on some platform, so i suspect the test 
>> i added might, sometimes, trigger a warning. I'm not exactly sure how to 
>> handle that
>
> Tests for stack space are always really challenging because triggering the 
> failure is so system specific. We only have one test for it 
> (`test/SemaTemplate/stack-exhaustion.cpp`)
>
> But do we need to run these on separate threads in all of those places to 
> address the issue?
>
> "and there is actually some code that avoids marking the same object used 
> twice." makes me wonder if we're missing an early return somewhere that would 
> reduce stack usage more and avoid needing to spin up threads for this. (The 
> upside to this change is that the code compiles more often but the downside 
> is that we're not addressing the memory pressure issue, just kicking the can 
> down the road a bit.)

I agree for the most part, but the 'runWithSufficientStackSpace' doesn't run in 
a separate thread, it just wraps it in a function that makes our diagnostic for 
running out of memory better.  But I think you're right, we probably should 
have SOMETHING that lets us exit early or defer constructing the initializers 
or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Jan Sjödin via Phabricator via cfe-commits
jsjodin added a comment.

In D142244#4096492 , @clementval 
wrote:

> The new test embed.f90 and embed-error.f90 are failing on my side. Do they 
> assume a specific configuration?

No, it should work for any configuration as far as I know. How are you running 
the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070
+
+if(Ty->isStructTy()){
+  Address StructAddr = ReturnValue.getValue();

This gets REALLY complicated, you can't just create a store, this might end up 
hitting conversion operators/etc, and is subject to triviality/etc, and also 
probably needs to go through a constructor.  I suspect you're going to prefer 
to just decide this isn't a valid builtin for structs instead of getting bogged 
down in that mess.



Comment at: clang/lib/Sema/SemaChecking.cpp:17814
+  QualType TyArg = Arg.get()->getType();
+  TheCall->setType(TyArg);
+  return false;

So I think choosing the sub-set of types here is a much better idea, allowing 
any of the user-defined types is going to make your codegen for this builtin a 
mess.  IMO, just `isBuiltinType` + `isVectorType` is probably an acceptable 
list for now.  



Comment at: clang/test/CodeGen/builtins-nondeterministic-value.c:6
+struct testStruct{
+  int x;
+  int y;

If you're going to allow this, we need tests for unions, constructors, 
non-trivial types, copy assignment/etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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


[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D139774#4095522 , @vedgy wrote:

> In D139774#4094619 , @aaron.ballman 
> wrote:
>
>> Is there a middle ground where, instead of #2 for general temporary storage, 
>> we went with #2 but with compiler-specific directories instead of system 
>> directories. e.g., don't let the caller set the temp directory, but do let 
>> the caller set the preamble directory (which defaults to the temp directory) 
>> so long as it's set before invoking the compiler? This still won't let you 
>> change options mid-run, but it also seems like it should have less risk of 
>> affecting other components while still solving the thread safety issues. I'm 
>> not certain if it's any easier to implement, but I think it does save you 
>> from modifying `FileSystemOptions`. As a separate item, we could then 
>> consider adding a new C API to let you toggle the in-memory vs on-disk 
>> functionality after exploring that it won't cause other problems because 
>> nobody considered the possibility that it's not a stable value for the 
>> compiler invocation.
>
> OK, so I'm going to implement overriding the preamble directory in 
> `clang_createIndexWithPreambleStoragePath` (or `clang_createIndex2` or 
> `clang_createIndexExt` or?); try to keep it simple and not modify 
> `FileSystemOptions`; deal with the in-memory option separately later. Abandon 
> this revision now and create another one once ready?

That sounds like a good plan to me. I wonder if we want to name it something 
like `clang_createIndexWithOptions` (or something generic like that), and give 
it a versioned structure of options along the lines of:

  struct CIndexOptions {
uint32_t Size; // sizeof(struct CIndexOptions), used for option versioning
const char *PreambleStoragePath; // This pointer can be freed after 
creating the index
  };

and define the function to return an error if `Size < sizeof(struct 
CIndexOptions)`. This should allow us to add additional options to the 
structure without having to introduce a new constructor API each time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D142401#4096498 , @erichkeane 
wrote:

> In D142401#4096478 , @aaron.ballman 
> wrote:
>
>> In D142401#4082356 , @cor3ntin 
>> wrote:
>>
>>> In D142401#4074959 , @shafik 
>>> wrote:
>>>
 I could not find any tests that test for this warning.
>>>
>>> I'm unable to trigger the warning at all. I feel i should but just using 
>>> that function resolves the crash, probably because it allocates more stack 
>>> in a separate thread, and there is actually some code that avoids marking 
>>> the same object used twice.
>>> But the warning might still trigger on some platform, so i suspect the test 
>>> i added might, sometimes, trigger a warning. I'm not exactly sure how to 
>>> handle that
>>
>> Tests for stack space are always really challenging because triggering the 
>> failure is so system specific. We only have one test for it 
>> (`test/SemaTemplate/stack-exhaustion.cpp`)
>>
>> But do we need to run these on separate threads in all of those places to 
>> address the issue?
>>
>> "and there is actually some code that avoids marking the same object used 
>> twice." makes me wonder if we're missing an early return somewhere that 
>> would reduce stack usage more and avoid needing to spin up threads for this. 
>> (The upside to this change is that the code compiles more often but the 
>> downside is that we're not addressing the memory pressure issue, just 
>> kicking the can down the road a bit.)
>
> I agree for the most part, but the 'runWithSufficientStackSpace' doesn't run 
> in a separate thread, it just wraps it in a function that makes our 
> diagnostic for running out of memory better.  But I think you're right, we 
> probably should have SOMETHING that lets us exit early or defer constructing 
> the initializers or something.

That's not entirely accurate: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Stack.h#L40
 which calls into 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Stack.cpp#L66

It MIGHT get run in a new thread (if that's enabled) or it might not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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


[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Huh TIL!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Valentin Clement via Phabricator via cfe-commits
clementval added a comment.

In D142244#4096512 , @jsjodin wrote:

> No, it should work for any configuration as far as I know. How are you 
> running the test?

Just with `check-flang`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Jan Sjödin via Phabricator via cfe-commits
jsjodin added a comment.

In D142244#4096538 , @clementval 
wrote:

> In D142244#4096512 , @jsjodin wrote:
>
>> No, it should work for any configuration as far as I know. How are you 
>> running the test?
>
> Just with `check-flang`

How does it fail? Can you show the output? Also what CPU are you running on?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D133289#4036529 , @aaron.ballman 
wrote:

> I spoke with @to268 during office hours about the current status of the NB 
> comments for this feature in the C committee. For the moment, he's going to 
> pause work on this patch until the C committee weighs in on whether `auto` is 
> a type specifier or not. The committee meets the week of Jan 23, so we should 
> have a better understanding of direction by the week of Jan 30. If the C 
> committee turns `auto` into a type specifier, we may go back to the original 
> implementation of this feature which exposes the C++ feature in C (and add 
> diagnostics around the things that need to be constrained in C such as use in 
> a function signature).

The minutes from last week's meeting aren't available yet, but I can give an 
update from my personal notes. Alex Gilding wrote a proposal to change the 
specification mechanisms away from "storage class specifier with no type 
specifier" and to an actual type specifier in WG14 N3076 
(https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3076.pdf). WG14 briefly 
looked at the paper, but given how late in the cycle we are, we did not want to 
adopt new specification wording that is so drastically different from the 
existing wording. So we repaired the national body comments with WG14 N3079 
(https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3079.htm) instead.

> Does WG14 want something along the lines of N3076 in a future revision of the 
> standard? 10 (for)/0 (against)/7 (abstain) -- consensus

So we now have sufficient support within WG14 for Clang to expose our C++ 
implementation of `auto` in C (with additional diagnostics for functionality C 
doesn't yet support like `auto` in function signatures) instead of implementing 
it from scratch as a weird storage class specifier/missing type specifier. 
However, we did make some other repairs to the feature so the current wording 
in the working draft (WG14 N3054) is not the latest. Specifically, we adopted 
US-122 and US-123 changes from WG14 N3079.

WG14 is expecting to have another week of meetings to work on NB comments (week 
of Feb 13) and we are expecting to have a second round of NB comments. I think 
it's okay for us to restart work on this patch now that we have a direction, 
but we might want to hold off on landing the patch until after we have a better 
idea of the final form of the specification. If there are no comments about it 
in CD2, then we're fine, but it's possible for CD2 to make breaking changes to 
the feature, so it's mostly about being aware of that situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Valentin Clement via Phabricator via cfe-commits
clementval added a comment.

In D142244#4096546 , @jsjodin wrote:

> In D142244#4096538 , @clementval 
> wrote:
>
>> In D142244#4096512 , @jsjodin 
>> wrote:
>>
>>> No, it should work for any configuration as far as I know. How are you 
>>> running the test?
>>
>> Just with `check-flang`
>
> How does it fail? Can you show the output? Also what CPU are you running on?



  
  FAIL: Flang :: Driver/embed.f90 (2 of 1993)
   TEST 'Flang :: Driver/embed.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   llvm-project/build/bin/flang-new -fc1 -emit-llvm -o - 
-fembed-offload-object=llvm-project/flang/test/Driver/Inputs/hello.f90 
llvm-project/flang/test/Driver/embed.f90 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed.f90
  : 'RUN: at line 8';   llvm-project/build/bin/flang-new -fc1 -emit-llvm-bc -o 
llvm-project/build/tools/flang/test/Driver/Output/embed.f90.tmp.bc 
llvm-project/flang/test/Driver/embed.f90 2>&1
  : 'RUN: at line 9';   llvm-project/build/bin/flang-new -fc1 -emit-llvm -o - 
-fembed-offload-object=llvm-project/flang/test/Driver/Inputs/hello.f90 
llvm-project/build/tools/flang/test/Driver/Output/embed.f90.tmp.bc 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed.f90
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  llvm-project/flang/test/Driver/embed.f90:11:10: error: CHECK: expected string 
not found in input
  ! CHECK: @[[OBJECT_1:.+]] = private constant [61 x i8] c"program hello\0A 
write(*,*), \22Hello world!\22\0Aend program hello\0A", section 
".llvm.offloading", align 8, !exclude !0
   ^
  :1:1: note: scanning from here
  ; ModuleID = 'FIRModule'
  ^
  
  Input file: 
  Check file: llvm-project/flang/test/Driver/embed.f90
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
1: ; ModuleID = 'FIRModule' 
  check:11 X error: no match found
2: source_filename = "FIRModule" 
  check:11 ~~
3: target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
  check:11 
~
4: target triple = "x86_64-unknown-linux-gnu" 
  check:11 ~~~
5:  
  check:11 ~
6: @_QFECi = internal constant i32 1 
  check:11 ~~
.
.
.
  >>



  `
  FAIL: Flang :: Driver/embed-error.f90 (1 of 1993)
   TEST 'Flang :: Driver/embed-error.f90' FAILED 

  Script:
  --
  : 'RUN: at line 6';   not llvm-project/build/bin/flang-new -fc1 -emit-llvm -o 
- -fembed-offload-object=llvm-project/flang/test/Driver/Inputs/missing.f90 
llvm-project/flang/test/Driver/embed-error.f90 2>&1 | 
llvm-project/build/bin/FileCheck llvm-project/flang/test/Driver/embed-error.f90 
--check-prefix=ERROR
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  llvm-project/flang/test/Driver/embed-error.f90:8:10: error: ERROR: expected 
string not found in input
  ! ERROR: error: could not open
   ^
  :1:1: note: scanning from here
  ; ModuleID = 'FIRModule'
  ^
  :6:14: note: possible intended match here
  @_QFECi = internal constant i32 1
   ^
  
  Input file: 
  Check file: llvm-project/flang/test/Driver/embed-error.f90
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: ; ModuleID = 'FIRModule' 
  check:8'0 X error: no match found
 2: source_filename = "FIRModule" 
  check:8'0 ~~
 3: target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
  check:8'0 
~
 4: target triple = "x86_64-unknown-linux-gnu" 
  check:8'0 ~~~
 5:  
  check:8'0 ~
 6: @_QFECi = internal constant i32 1 
  check:8'0 ~~
  check:8'1  ? possible intended match
 7: @_QQEnvironmentDefaults = constant ptr null 
  check:8'0 
 8:  
  check:8'0 ~
 9: declare ptr @malloc(i64) 
  check:8'0 ~
10:  
  check:8'0 ~
11: declare void @free(ptr) 
  check:8'0 
 .
 .
 .

I run on `x86_64`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llv

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D133289#4037180 , @tschuett wrote:

> It surprised me that there are no type inference messages? The type of this 
> auto is double. I only found warnings of misuse of auto and codegen tests.

Can you give a code example of what you expected to see a diagnostic for but 
aren't getting one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2023-02-01 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

In D133289#4096588 , @aaron.ballman 
wrote:

> In D133289#4037180 , @tschuett 
> wrote:
>
>> It surprised me that there are no type inference messages? The type of this 
>> auto is double. I only found warnings of misuse of auto and codegen tests.
>
> Can you give a code example of what you expected to see a diagnostic for but 
> aren't getting one?

This is probably only for testing and not for Clang users:

  auto Float = 3.0; // note that Float is a float


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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


[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Manuel Brito via Phabricator via cfe-commits
ManuelJBrito added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070
+
+if(Ty->isStructTy()){
+  Address StructAddr = ReturnValue.getValue();

erichkeane wrote:
> This gets REALLY complicated, you can't just create a store, this might end 
> up hitting conversion operators/etc, and is subject to triviality/etc, and 
> also probably needs to go through a constructor.  I suspect you're going to 
> prefer to just decide this isn't a valid builtin for structs instead of 
> getting bogged down in that mess.
The motivation for this builtin was to match intel's definition of undefined in 
the lowering of intrinsics such as  
https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm256_castsi128_si256&expand=628&ig_expand=755,
  so struct support isn't critical.
So if everyone else agrees i'll drop struct support, do as you suggest and if 
there are use cases for nondeterministic values of other types add support for 
them later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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


[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

2023-02-01 Thread Manuel Klimek via Phabricator via cfe-commits
klimek created this revision.
klimek added a reviewer: sammccall.
Herald added a project: All.
klimek requested review of this revision.
Herald added a project: clang.

In preparation for configured macro replacements in formatting,
add the ability to insert tokens to FormatTokenSource, and implement
token insertion in IndexedTokenSource.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143070

Files:
  clang/lib/Format/FormatTokenSource.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTokenSourceTest.cpp
  clang/unittests/Format/TestLexer.h

Index: clang/unittests/Format/TestLexer.h
===
--- clang/unittests/Format/TestLexer.h
+++ clang/unittests/Format/TestLexer.h
@@ -71,7 +71,8 @@
 
   TokenList annotate(llvm::StringRef Code) {
 FormatTokenLexer Lex = getNewLexer(Code);
-auto Tokens = Lex.lex();
+auto Toks = Lex.lex();
+SmallVector Tokens(Toks.begin(), Toks.end());
 UnwrappedLineParser Parser(Style, Lex.getKeywords(), 0, Tokens, *this);
 Parser.parse();
 TokenAnnotator Annotator(Style, Lex.getKeywords());
Index: clang/unittests/Format/FormatTokenSourceTest.cpp
===
--- clang/unittests/Format/FormatTokenSourceTest.cpp
+++ clang/unittests/Format/FormatTokenSourceTest.cpp
@@ -30,6 +30,12 @@
 FormatToken *Tok = FormatTok;  \
 EXPECT_EQ((Tok)->Tok.getKind(), Kind) << *(Tok);   \
   } while (false);
+#define EXPECT_TOKEN_ID(FormatTok, Name)   \
+  do { \
+FormatToken *Tok = FormatTok;  \
+EXPECT_EQ((Tok)->Tok.getKind(), tok::identifier) << *(Tok);\
+EXPECT_EQ((Tok)->TokenText, Name) << *(Tok);   \
+  } while (false);
 
 TEST_F(IndexedTokenSourceTest, EmptyInput) {
   TokenList Tokens = lex("");
@@ -60,6 +66,8 @@
   EXPECT_TOKEN_KIND(Source.peekNextToken(), tok::eof);
   EXPECT_TOKEN_KIND(Source.getNextToken(), tok::eof);
   EXPECT_TOKEN_KIND(Source.getPreviousToken(), tok::semi);
+  EXPECT_TOKEN_KIND(Source.getNextToken(), tok::eof);
+  EXPECT_TOKEN_KIND(Source.getPreviousToken(), tok::semi);
 }
 
 TEST_F(IndexedTokenSourceTest, ResetPosition) {
@@ -73,6 +81,62 @@
   EXPECT_TOKEN_KIND(Source.setPosition(Position), tok::kw_int);
 }
 
+TEST_F(IndexedTokenSourceTest, InsertTokens) {
+  TokenList TokensA = lex("A1 A2");
+  TokenList TokensB = lex("B1 B2");
+  IndexedTokenSource Source(TokensA);
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensB), "B1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "B2");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A2");
+}
+
+TEST_F(IndexedTokenSourceTest, InsertTokensAtEOF) {
+  TokenList TokensA = lex("A1");
+  TokenList TokensB = lex("B1 B2");
+  IndexedTokenSource Source(TokensA);
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_KIND(Source.getNextToken(), tok::eof);
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensB), "B1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "B2");
+  EXPECT_TOKEN_KIND(Source.getNextToken(), tok::eof);
+}
+
+TEST_F(IndexedTokenSourceTest, InsertTokensRecursive) {
+  TokenList TokensA = lex("A1");
+  TokenList TokensB = lex("B1");
+  TokenList TokensC = lex("C1");
+  TokenList TokensD = lex("D1");
+  IndexedTokenSource Source(TokensA);
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  // A1
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensB), "B1");
+  // B1 A1
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensC), "C1");
+  // C1 B1 A1
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensD), "D1");
+  // D1 C1 B1 A1
+  EXPECT_TOKEN_ID(Source.getNextToken(), "C1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "B1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+}
+
+TEST_F(IndexedTokenSourceTest, InsertTokensRecursiveAtEndOfSequence) {
+  TokenList TokensA = lex("A1");
+  TokenList TokensB = lex("B1");
+  TokenList TokensC = lex("C1");
+  TokenList TokensD = lex("D1");
+  IndexedTokenSource Source(TokensA);
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensB), "B1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensC), "C1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+  EXPECT_TOKEN_ID(Source.insertTokens(TokensD), "D1");
+  EXPECT_TOKEN_ID(Source.getNextToken(), "A1");
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -89,7 +89,8 @@
 public:
 

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070
+
+if(Ty->isStructTy()){
+  Address StructAddr = ReturnValue.getValue();

ManuelJBrito wrote:
> erichkeane wrote:
> > This gets REALLY complicated, you can't just create a store, this might end 
> > up hitting conversion operators/etc, and is subject to triviality/etc, and 
> > also probably needs to go through a constructor.  I suspect you're going to 
> > prefer to just decide this isn't a valid builtin for structs instead of 
> > getting bogged down in that mess.
> The motivation for this builtin was to match intel's definition of undefined 
> in the lowering of intrinsics such as  
> https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm256_castsi128_si256&expand=628&ig_expand=755,
>   so struct support isn't critical.
> So if everyone else agrees i'll drop struct support, do as you suggest and if 
> there are use cases for nondeterministic values of other types add support 
> for them later?
I suspect I speak for all with my suggestion.  I don't think Aaron/Shafik are 
concerned with struct support here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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


[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3070
+
+if(Ty->isStructTy()){
+  Address StructAddr = ReturnValue.getValue();

erichkeane wrote:
> ManuelJBrito wrote:
> > erichkeane wrote:
> > > This gets REALLY complicated, you can't just create a store, this might 
> > > end up hitting conversion operators/etc, and is subject to 
> > > triviality/etc, and also probably needs to go through a constructor.  I 
> > > suspect you're going to prefer to just decide this isn't a valid builtin 
> > > for structs instead of getting bogged down in that mess.
> > The motivation for this builtin was to match intel's definition of 
> > undefined in the lowering of intrinsics such as  
> > https://www.intel.com/content/www/us/en/docs/intrinsics-guide/index.html#text=_mm256_castsi128_si256&expand=628&ig_expand=755,
> >   so struct support isn't critical.
> > So if everyone else agrees i'll drop struct support, do as you suggest and 
> > if there are use cases for nondeterministic values of other types add 
> > support for them later?
> I suspect I speak for all with my suggestion.  I don't think Aaron/Shafik are 
> concerned with struct support here.
If we reject the code, we can relax that restriction in the future without 
breaking code, so I think it's fine to not support structures. We can add 
support for them later if we have a need to do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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


[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment.

In D139774#4096527 , @aaron.ballman 
wrote:

> That sounds like a good plan to me. I wonder if we want to name it something 
> like `clang_createIndexWithOptions` (or something generic like that), and 
> give it a versioned structure of options along the lines of:
>
>   struct CIndexOptions {
> uint32_t Size; // sizeof(struct CIndexOptions), used for option versioning
> const char *PreambleStoragePath; // This pointer can be freed after 
> creating the index
>   };
>
> and define the function to return an error if `Size < sizeof(struct 
> CIndexOptions)`. This should allow us to add additional options to the 
> structure without having to introduce a new constructor API each time.

Would this be the recommended usage of such an API?

1. Call `clang_createIndexWithOptions`.
2. If it returns `nullptr` index, report an error in the application (e.g. 
print a warning or show an error in the UI) and fall back to code paths that 
support older Clang versions, beginning with calling the older constructor 
`clang_createIndex`.

Is assigning `sizeof(CIndexOptions)` to `Size` the API user's responsibility or 
should libclang define an inline function `clang_initializeCIndexOptions`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Jan Sjödin via Phabricator via cfe-commits
jsjodin added a comment.

In D142244#4096579 , @clementval 
wrote:

> In D142244#4096546 , @jsjodin wrote:
>
>> In D142244#4096538 , @clementval 
>> wrote:
>>
>>> In D142244#4096512 , @jsjodin 
>>> wrote:
>>>
 No, it should work for any configuration as far as I know. How are you 
 running the test?
>>>
>>> Just with `check-flang`
>>
>> How does it fail? Can you show the output? Also what CPU are you running on?
>
>
>
>   I run on `x86_64`

How did you configure your build? I'd like to reproduce your build and see if I 
can make it fail on my machine. Right now the test passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D143072: [NFC][clang] Fix static code analyzer complains

2023-02-01 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon created this revision.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Initialize values that end up uninitialized.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143072

Files:
  clang/lib/ARCMigrate/TransformActions.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp


Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -574,6 +574,7 @@
 D.diag_AlwaysFallThrough_HasNoReturn = 0;
 D.diag_AlwaysFallThrough_ReturnsNonVoid =
 diag::warn_falloff_nonvoid_coroutine;
+D.diag_NeverFallThroughOrReturn = 0;
 D.funMode = Coroutine;
 return D;
   }
Index: clang/lib/ARCMigrate/TransformActions.cpp
===
--- clang/lib/ARCMigrate/TransformActions.cpp
+++ clang/lib/ARCMigrate/TransformActions.cpp
@@ -45,7 +45,7 @@
 SourceLocation Loc;
 SourceRange R1, R2;
 StringRef Text1, Text2;
-Stmt *S;
+Stmt *S = nullptr;
 SmallVector DiagIDs;
   };
 


Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -574,6 +574,7 @@
 D.diag_AlwaysFallThrough_HasNoReturn = 0;
 D.diag_AlwaysFallThrough_ReturnsNonVoid =
 diag::warn_falloff_nonvoid_coroutine;
+D.diag_NeverFallThroughOrReturn = 0;
 D.funMode = Coroutine;
 return D;
   }
Index: clang/lib/ARCMigrate/TransformActions.cpp
===
--- clang/lib/ARCMigrate/TransformActions.cpp
+++ clang/lib/ARCMigrate/TransformActions.cpp
@@ -45,7 +45,7 @@
 SourceLocation Loc;
 SourceRange R1, R2;
 StringRef Text1, Text2;
-Stmt *S;
+Stmt *S = nullptr;
 SmallVector DiagIDs;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] ecb3cd0 - [Driver] Move PS4/PS5 header search path management to the driver

2023-02-01 Thread Paul Robinson via cfe-commits

Author: Paul Robinson
Date: 2023-02-01T07:40:30-08:00
New Revision: ecb3cd0946a47c419efda7b92cd6f6fcc30b6be9

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

LOG: [Driver] Move PS4/PS5 header search path management to the driver

This follows how OpenBSD, FreeBSD, and NetBSD now work. (See
D138183 and D140817 for those cases.)

It also tidies up some code duplication that wasn't exactly right.

Added: 


Modified: 
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/lib/Driver/ToolChains/PS4CPU.h
clang/lib/Lex/InitHeaderSearch.cpp
clang/test/Driver/ps4-ps5-header-search.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 643f815c5835a..fc5d46bc25605 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -8,6 +8,7 @@
 
 #include "PS4CPU.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -253,15 +254,14 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const 
llvm::Triple &Triple,
   // SDK include or lib directories. This behavior could be changed if
   // -Weverything or -Winvalid-or-nonexistent-directory options are passed.
   // If -isysroot was passed, use that as the SDK base path.
-  std::string PrefixDir;
   if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
-PrefixDir = A->getValue();
-if (!llvm::sys::fs::exists(PrefixDir))
-  D.Diag(clang::diag::warn_missing_sysroot) << PrefixDir;
+SDKRootDir = A->getValue();
+if (!llvm::sys::fs::exists(SDKRootDir))
+  D.Diag(clang::diag::warn_missing_sysroot) << SDKRootDir;
   } else
-PrefixDir = std::string(SDKDir.str());
+SDKRootDir = std::string(SDKDir.str());
 
-  SmallString<512> SDKIncludeDir(PrefixDir);
+  SmallString<512> SDKIncludeDir(SDKRootDir);
   llvm::sys::path::append(SDKIncludeDir, "target/include");
   if (!Args.hasArg(options::OPT_nostdinc) &&
   !Args.hasArg(options::OPT_nostdlibinc) &&
@@ -272,7 +272,7 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const 
llvm::Triple &Triple,
 << Twine(Platform, " system headers").str() << SDKIncludeDir;
   }
 
-  SmallString<512> SDKLibDir(SDKDir);
+  SmallString<512> SDKLibDir(SDKRootDir);
   llvm::sys::path::append(SDKLibDir, "target/lib");
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs) &&
@@ -287,6 +287,42 @@ toolchains::PS4PS5Base::PS4PS5Base(const Driver &D, const 
llvm::Triple &Triple,
   getFilePaths().push_back(std::string(SDKLibDir.str()));
 }
 
+void toolchains::PS4PS5Base::AddClangSystemIncludeArgs(
+const ArgList &DriverArgs,
+ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+
+  if (DriverArgs.hasArg(options::OPT_nostdinc))
+return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(D.ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+return;
+
+  // Add dirs specified via 'configure --with-c-include-dirs'.
+  StringRef CIncludeDirs(C_INCLUDE_DIRS);
+  if (!CIncludeDirs.empty()) {
+SmallVector dirs;
+CIncludeDirs.split(dirs, ":");
+for (StringRef dir : dirs) {
+  StringRef Prefix =
+llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+  addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
+}
+return;
+  }
+
+  addExternCSystemInclude(DriverArgs, CC1Args,
+  SDKRootDir + "/target/include");
+  addExternCSystemInclude(DriverArgs, CC1Args,
+  SDKRootDir + "/target/include_common");
+}
+
 Tool *toolchains::PS4CPU::buildAssembler() const {
   return new tools::PScpu::Assembler(*this);
 }

diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.h 
b/clang/lib/Driver/ToolChains/PS4CPU.h
index 954e7d8d8d684..0866a5daa4cc6 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.h
+++ b/clang/lib/Driver/ToolChains/PS4CPU.h
@@ -63,6 +63,9 @@ class LLVM_LIBRARY_VISIBILITY PS4PS5Base : public Generic_ELF 
{
  const llvm::opt::ArgList &Args, StringRef Platform,
  const char *EnvVar);
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const override;
   // No support for finding a C++ standard library yet.
   void addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args) const override 
{
@@ -111,6 +114,10 @@ class LLVM_LIBRARY_VISIBILITY PS4PS5Base : public 
Generic_ELF {
 
 pr

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D139774#4096638 , @vedgy wrote:

> In D139774#4096527 , @aaron.ballman 
> wrote:
>
>> That sounds like a good plan to me. I wonder if we want to name it something 
>> like `clang_createIndexWithOptions` (or something generic like that), and 
>> give it a versioned structure of options along the lines of:
>>
>>   struct CIndexOptions {
>> uint32_t Size; // sizeof(struct CIndexOptions), used for option 
>> versioning
>> const char *PreambleStoragePath; // This pointer can be freed after 
>> creating the index
>>   };
>>
>> and define the function to return an error if `Size < sizeof(struct 
>> CIndexOptions)`. This should allow us to add additional options to the 
>> structure without having to introduce a new constructor API each time.
>
> Would this be the recommended usage of such an API?
>
> 1. Call `clang_createIndexWithOptions`.
> 2. If it returns `nullptr` index, report an error in the application (e.g. 
> print a warning or show an error in the UI) and fall back to code paths that 
> support older Clang versions, beginning with calling the older constructor 
> `clang_createIndex`.

Yeah, I would imagine robust code to look like:

  CIndexOptions Opts;
  Opts.Size = sizeof(Opts);
  Opts.PreambleStoragePath = somePathPtr;
  CIndex Idx = clang_createIndexWithOptions(/*excludeDeclsFromPCH=*/1, 
/*displayDiagnostics=*/1, &Opts);
  if (!Idx) {
Idx = clang_createIndex(/*excludeDeclsFromPCH=*/1, 
/*displayDiagnostics=*/1);
if (!Idx)
  handleError();
  }

> Is assigning `sizeof(CIndexOptions)` to `Size` the API user's responsibility 
> or should libclang define an inline function `clang_initializeCIndexOptions`?

The user's responsibility. There's three scenarios when a field is added to the 
structure: 1) library and app are matching versions, 2) library is newer than 
app, 3) app is newer than library. #1 is the happy path most folks will 
hopefully be on. For #2 and #3, the app will either be sending more or less 
data than the library expects, but the library will know how much of the 
structure is valid based on the size field. If the size is too small and the 
library can't get data it needs, it can catch that and report an error instead 
of reading past a buffer. And if the size is too large, the library can ignore 
anything it doesn't know about or it can give an error due to not knowing how 
to support the semantics of the newer interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D143075: BareMetal ToolChain multilib layering

2023-02-01 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added a subscriber: abidh.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This enables layering baremetal multilibs on top of each other.
For example a multilib containing only a no-exceptions libc++ could be
layered on top of a multilib containing C libs. This avoids the need
to duplicate the C library for every libc++ variant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143075

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/Inputs/baremetal_multilib/multilib.yaml
  clang/test/Driver/baremetal-multilib.cpp

Index: clang/test/Driver/baremetal-multilib.cpp
===
--- clang/test/Driver/baremetal-multilib.cpp
+++ clang/test/Driver/baremetal-multilib.cpp
@@ -25,6 +25,23 @@
 // RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-DIRECTORY %s
 // CHECK-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8-m.main/fp
 
+// RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+// RUN:   | FileCheck --check-prefix=CHECK-LAYERED-MULTILIB %s
+// CHECK-LAYERED-MULTILIB: "{{.*}}clang{{.*}}" "-cc1" "-triple" "thumbv8.1m.main-none-unknown-eabihf"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include/c++/v1"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include/c++/v1"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include"
+// CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include"
+// CHECK-LAYERED-MULTILIB-NEXT: "-L{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/lib"
+// CHECK-LAYERED-MULTILIB-SAME: "-L{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/lib"
+
+// RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
+// RUN: --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+// RUN:   | FileCheck --check-prefix=CHECK-LAYERED-PRINT-MULTI-DIRECTORY %s
+// CHECK-LAYERED-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8.1-m.main/fp
+// CHECK-LAYERED-PRINT-MULTI-DIRECTORY-NEXT: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+
 // RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-lib 2>&1 \
 // RUN: --target=arm-none-eabi --sysroot= \
 // RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-LIB %s
@@ -38,6 +55,7 @@
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8-m.main/fp;@-target=thumbv8m.main-none-eabihf
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp;@-target=thumbv8.1m.main-none-eabihf
 // CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/nofp/mve;@-target=arm-none-eabihf@march=armv8.1m.main+nofp+mve
+// CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept;@-target=thumbv8.1m.main-none-eabihf@fno-exceptions
 
 // RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -x assembler -mexecute-only \
 // RUN: --target=arm-none-eabi --sysroot= %s -c -### 2>&1 \
Index: clang/test/Driver/Inputs/baremetal_multilib/multilib.yaml
===
--- clang/test/Driver/Inputs/baremetal_multilib/multilib.yaml
+++ clang/test/Driver/Inputs/baremetal_multilib/multilib.yaml
@@ -65,6 +65,14 @@
   flags: [target=thumbv8.1m.main-none-unknown-eabihf, march=+mve]
   printArgs: [--target=arm-none-eabihf, -march=armv8.1m.main+nofp+mve]
 
+# A specialisation of v8.1-m.main/fp without exceptions.
+# This layers over the top of the regular v8.1-m.main/fp so it doesn't
+# need to have its own include directory or C library, thereby saving
+# disk space.
+- dir: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+  flags: [target=thumbv8.1m.main-none-unknown-eabihf, hasfpu, fno-exceptions]
+  printArgs: [--target=thumbv8.1m.main-none-eabihf, -fno-exceptions]
+
 
 # The second section of the file is a map from auto-detected flags
 # to custom flags. The auto-detected flags can be printed out
Index: clang/lib/Driver/ToolChains/BareMetal.h
===
--- clang/lib/Driver/ToolChains/BareMetal.h
+++ clang/lib/Driver/ToolChains/BareMetal.h
@@ -70,6 +70,9 @@
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs) const;
   std::string computeSysRoot() const override;
+

[PATCH] D142388: [clang] Add builtin_nondeterministic_value

2023-02-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Please add a release note.

Thanks for this new builtin, as I was working on something very very similar - 
the implementation looks fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142388

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


[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment.

In D139774#4096695 , @aaron.ballman 
wrote:

> There's three scenarios when a field is added to the structure: 1) library 
> and app are matching versions, 2) library is newer than app, 3) app is newer 
> than library. #1 is the happy path most folks will hopefully be on. For #2 
> and #3, the app will either be sending more or less data than the library 
> expects, but the library will know how much of the structure is valid based 
> on the size field. If the size is too small and the library can't get data it 
> needs, it can catch that and report an error instead of reading past a 
> buffer. And if the size is too large, the library can ignore anything it 
> doesn't know about or it can give an error due to not knowing how to support 
> the semantics of the newer interface.

In the second case, the library ideally should assume that the missing struct 
members are not specified and behave as the corresponding older version. In the 
third case, the app can support older libclang versions by passing successively 
smaller structs until libclang returns a valid `CIndex`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[clang] d4fb829 - [clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

2023-02-01 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2023-02-01T15:57:09Z
New Revision: d4fb829b718059eb044843aea7b03d5b65556351

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

LOG: [clang][dataflow] Relax validity assumptions in 
`UncheckedOptionalAccessModel`.

Currently, the interpretation of `swap` calls in the optional model assumes the
optional arguments are modeled (and therefore have valid storage locations and
values). This assumption is incorrect, for example, in the case of unmodeled
optional fields (which can be missing either value or location). This patch
relaxes these assumptions, to return rather than assert when either argument is
not modeled.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 308dc25dad1fc..ef34492f7d851 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -521,48 +521,54 @@ void transferNulloptAssignment(const CXXOperatorCallExpr 
*E,
   transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
 }
 
-void transferSwap(const StorageLocation &OptionalLoc1,
-  const StorageLocation &OptionalLoc2,
-  LatticeTransferState &State) {
-  auto *OptionalVal1 = State.Env.getValue(OptionalLoc1);
-  assert(OptionalVal1 != nullptr);
+void transferSwap(const Expr &E1, SkipPast E1Skip, const Expr &E2,
+  Environment &Env) {
+  // We account for cases where one or both of the optionals are not modeled,
+  // either lacking associated storage locations, or lacking values associated
+  // to such storage locations.
+  auto *Loc1 = Env.getStorageLocation(E1, E1Skip);
+  auto *Loc2 = Env.getStorageLocation(E2, SkipPast::Reference);
+
+  if (Loc1 == nullptr) {
+if (Loc2 != nullptr)
+  Env.setValue(*Loc2, createOptionalValue(Env, Env.makeAtomicBoolValue()));
+return;
+  }
+  if (Loc2 == nullptr) {
+Env.setValue(*Loc1, createOptionalValue(Env, Env.makeAtomicBoolValue()));
+return;
+  }
 
-  auto *OptionalVal2 = State.Env.getValue(OptionalLoc2);
-  assert(OptionalVal2 != nullptr);
+  // Both expressions have locations, though they may not have corresponding
+  // values. In that case, we create a fresh value at this point. Note that if
+  // two branches both do this, they will not share the value, but it at least
+  // allows for local reasoning about the value. To avoid the above, we would
+  // need *lazy* value allocation.
+  // FIXME: allocate values lazily, instead of just creating a fresh value.
+  auto *Val1 = Env.getValue(*Loc1);
+  if (Val1 == nullptr)
+Val1 = &createOptionalValue(Env, Env.makeAtomicBoolValue());
 
-  State.Env.setValue(OptionalLoc1, *OptionalVal2);
-  State.Env.setValue(OptionalLoc2, *OptionalVal1);
+  auto *Val2 = Env.getValue(*Loc2);
+  if (Val2 == nullptr)
+Val2 = &createOptionalValue(Env, Env.makeAtomicBoolValue());
+
+  Env.setValue(*Loc1, *Val2);
+  Env.setValue(*Loc2, *Val1);
 }
 
 void transferSwapCall(const CXXMemberCallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
   assert(E->getNumArgs() == 1);
-
-  auto *OptionalLoc1 = State.Env.getStorageLocation(
-  *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer);
-  assert(OptionalLoc1 != nullptr);
-
-  auto *OptionalLoc2 =
-  State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
-  assert(OptionalLoc2 != nullptr);
-
-  transferSwap(*OptionalLoc1, *OptionalLoc2, State);
+  transferSwap(*E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer,
+   *E->getArg(0), State.Env);
 }
 
 void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
   assert(E->getNumArgs() == 2);
-
-  auto *OptionalLoc1 =
-  State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
-  assert(OptionalLoc1 != nullptr);
-
-  auto *OptionalLoc2 =
-  State.Env.getStorageLocation(*E->getArg(1), SkipPast::Reference);
-  assert(OptionalLoc2 != nullptr);
-
-  transferSwap(*OptionalLoc1, *OptionalLoc2, State);
+  transferSwap(*E->getArg(0), SkipPast::Reference, *E->getArg(1), State.Env);
 }
 
 BoolValue &evaluateEquality(Environment &Env, BoolValue &EqVal, BoolValue &LHS,

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp 
b/clang/

[PATCH] D142710: [clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

2023-02-01 Thread Yitzhak Mandelbaum 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 rGd4fb829b7180: [clang][dataflow] Relax validity assumptions 
in `UncheckedOptionalAccessModel`. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142710

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -11,7 +11,6 @@
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/DenseSet.h"
@@ -2124,6 +2123,139 @@
   )");
 }
 
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledLocLeft) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct L { $ns::$optional hd; L* tl; };
+
+void target() {
+  $ns::$optional foo = 3;
+  L bar;
+
+  // Any `tl` beyond the first is not modeled.
+  bar.tl->tl->hd.swap(foo);
+
+  bar.tl->tl->hd.value(); // [[unsafe]]
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledLocRight) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct L { $ns::$optional hd; L* tl; };
+
+void target() {
+  $ns::$optional foo = 3;
+  L bar;
+
+  // Any `tl` beyond the first is not modeled.
+  foo.swap(bar.tl->tl->hd);
+
+  bar.tl->tl->hd.value(); // [[unsafe]]
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueLeftSet) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct S { int x; };
+struct A { $ns::$optional late; };
+struct B { A f3; };
+struct C { B f2; };
+struct D { C f1; };
+
+void target() {
+  $ns::$optional foo = S{3};
+  D bar;
+
+  bar.f1.f2.f3.late.swap(foo);
+
+  bar.f1.f2.f3.late.value();
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueLeftUnset) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct S { int x; };
+struct A { $ns::$optional late; };
+struct B { A f3; };
+struct C { B f2; };
+struct D { C f1; };
+
+void target() {
+  $ns::$optional foo;
+  D bar;
+
+  bar.f1.f2.f3.late.swap(foo);
+
+  bar.f1.f2.f3.late.value(); // [[unsafe]]
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
+// fixme: use recursion instead of depth.
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueRightSet) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct S { int x; };
+struct A { $ns::$optional late; };
+struct B { A f3; };
+struct C { B f2; };
+struct D { C f1; };
+
+void target() {
+  $ns::$optional foo = S{3};
+  D bar;
+
+  foo.swap(bar.f1.f2.f3.late);
+
+  bar.f1.f2.f3.late.value();
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
+TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueRightUnset) {
+  ExpectDiagnosticsFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct S { int x; };
+struct A { $ns::$optional late; };
+struct B { A f3; };
+struct C { B f2; };
+struct D { C f1; };
+
+void target() {
+  $ns::$optional foo;
+  D bar;
+
+  foo.swap(bar.f1.f2.f3.late);
+
+  bar.f1.f2.f3.late.value(); // [[unsafe]]
+  foo.value(); // [[unsafe]]
+}
+  )");
+}
+
 TEST_P(UncheckedOptionalAccessTest, UniquePtrToOptional) {
   // We suppress diagnostics for optionals in smart pointers (other than
   // `optional` itself).
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -521,48 +521,54 @@
   transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
 }
 
-void transferSwap(const StorageLocation &OptionalLoc1,
-  const StorageLocation &OptionalLoc2,
-  LatticeTransferState &State) {
-  auto *OptionalVal1 = State.Env.getValue(OptionalLoc1);
-  assert(OptionalVal1 != nullptr);
+void transferSwap(const Ex

[PATCH] D139774: [libclang] Add API to set temporary directory location

2023-02-01 Thread Igor Kushnir via Phabricator via cfe-commits
vedgy added a comment.

Perhaps the struct should contain the value of `CINDEX_VERSION_MINOR` instead 
of the `sizeof`? This way, libclang can change the meaning of a struct member 
without changing the size of the struct. For example, replace 
`PreambleStoragePath` with `TemporaryDirectoryPath`. Such a change could even 
be quiet, because setting the more general temporary directory path would 
likely be welcome or at least acceptable to the API users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Valentin Clement via Phabricator via cfe-commits
clementval added a comment.

In D142244#4096648 , @jsjodin wrote:

> In D142244#4096579 , @clementval 
> wrote:
>
>> In D142244#4096546 , @jsjodin 
>> wrote:
>>
>>> In D142244#4096538 , @clementval 
>>> wrote:
>>>
 In D142244#4096512 , @jsjodin 
 wrote:

> No, it should work for any configuration as far as I know. How are you 
> running the test?

 Just with `check-flang`
>>>
>>> How does it fail? Can you show the output? Also what CPU are you running on?
>>
>>
>>
>>   I run on `x86_64`
>
> How did you configure your build? I'd like to reproduce your build and see if 
> I can make it fail on my machine. Right now the test passes.

My bad .. my `CompilerInvocation.cpp` was overwritten and missed part of this 
patch. It works fine. Sorry to have bothered you with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

2023-02-01 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann updated this revision to Diff 493955.
tahonermann added a comment.

Rebased. This now targets Clang 17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142639

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
  clang/test/SemaCXX/cxx2a-lambda-equals-this.cpp
  clang/test/SemaCXX/lambda-implicit-this-capture.cpp


Index: clang/test/SemaCXX/lambda-implicit-this-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-implicit-this-capture.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -verify=cxx11 %s
+// RUN: %clang_cc1 -std=c++2a -verify=cxx2a %s
+// RUN: %clang_cc1 -std=c++2a -verify=cxx2a-no-deprecated %s -Wno-deprecated
+// cxx11-no-diagnostics
+// cxx2a-no-deprecated-no-diagnostics
+
+struct A {
+  int i;
+  void f() {
+(void) [=] { // cxx2a-note {{add an explicit capture of 'this'}}
+  return i; // cxx2a-warning {{implicit capture of 'this' with a capture 
default of '=' is deprecated}}
+};
+  }
+};
Index: clang/test/SemaCXX/cxx2a-lambda-equals-this.cpp
===
--- clang/test/SemaCXX/cxx2a-lambda-equals-this.cpp
+++ clang/test/SemaCXX/cxx2a-lambda-equals-this.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -verify %s -Wdeprecated
+// RUN: %clang_cc1 -std=c++2a -verify %s
+// expected-no-diagnostics
 
 // This test does two things.
 // Deleting the copy constructor ensures that an [=, this] capture doesn't 
copy the object.
@@ -12,12 +13,3 @@
 L();
   }
 };
-
-struct B {
-  int i;
-  void f() {
-(void) [=] { // expected-note {{add an explicit capture of 'this'}}
-  return i; // expected-warning {{implicit capture of 'this' with a 
capture default of '=' is deprecated}}
-};
-  }
-};
Index: clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
===
--- clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
+++ clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -emit-llvm-only %s
-// RUN: %clang_cc1 -std=c++2a -verify -verify=expected-cxx2a -fsyntax-only 
-fblocks -emit-llvm-only %s
+// RUN: %clang_cc1 -std=c++2a -verify -verify=expected-cxx2a -fsyntax-only 
-fblocks -emit-llvm-only -Wno-deprecated-this-capture %s
 // RUN: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -emit-llvm-only 
-triple i386-windows-pc %s
-// RUN: %clang_cc1 -std=c++2a -verify -verify=expected-cxx2a -fsyntax-only 
-fblocks -emit-llvm-only -triple i386-windows-pc %s
+// RUN: %clang_cc1 -std=c++2a -verify -verify=expected-cxx2a -fsyntax-only 
-fblocks -emit-llvm-only -triple i386-windows-pc -Wno-deprecated-this-capture %s
 // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks 
-fdelayed-template-parsing %s -DDELAYED_TEMPLATE_PARSING
 // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks 
-fms-extensions %s -DMS_EXTENSIONS
 // DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks 
-fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS 
-DDELAYED_TEMPLATE_PARSING
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7906,7 +7906,7 @@
 "is a C++20 extension">, InGroup;
   def warn_deprecated_this_capture : Warning<
 "implicit capture of 'this' with a capture default of '=' is deprecated">,
-InGroup, DefaultIgnore;
+InGroup;
   def note_deprecated_this_capture : Note<
 "add an explicit capture of 'this' to capture '*this' by reference">;
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -69,6 +69,9 @@
 - We now generate a diagnostic for signed integer overflow due to unary minus
   in a non-constant expression context. This fixes
   `Issue 31643 `_
+- Clang now warns by default for C++20 and later about deprecated capture of
+  ``this`` with a capture default of ``=``. This warning can be disabled with
+  ``-Wno-deprecated-this-capture``.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/lambda-implicit-this-capture.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-implicit-this-capture.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -verify=cxx11 %s
+// RUN: %clang_cc1 -std=c++2a -verify=cxx2a %s
+// RUN: %clang_cc1 -std=c++2a -verify=cxx2a-no-deprecated %s -Wno-deprecated
+// 

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

2023-02-01 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis marked 2 inline comments as done.
TIFitis added inline comments.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1342-1382
+/// Create a constant string location from the MLIR Location information.
+static llvm::Constant *
+createSourceLocStrFromLocation(Location loc, llvm::OpenMPIRBuilder &builder,
+   StringRef name, uint32_t &strLen) {
+  if (auto fileLoc = loc.dyn_cast()) {
+StringRef fileName = fileLoc.getFilename();
+unsigned lineNo = fileLoc.getLine();

clementval wrote:
> TIFitis wrote:
> > clementval wrote:
> > > TIFitis wrote:
> > > > clementval wrote:
> > > > > TIFitis wrote:
> > > > > > clementval wrote:
> > > > > > > kiranchandramohan wrote:
> > > > > > > > clementval wrote:
> > > > > > > > > Instead of copy pasting this from 
> > > > > > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > > > > > >  can you extract it and put it in a common shared file so 
> > > > > > > > > bith translation can use the same code without duplication?
> > > > > > > > @raghavendhra put up a patch some time back and he faced some 
> > > > > > > > issues. It might be good to check with him or may be he can 
> > > > > > > > comment here.
> > > > > > > > https://reviews.llvm.org/D127037
> > > > > > > > https://discourse.llvm.org/t/rfc-for-refactoring-common-code-for-openacc-and-openmp/63833
> > > > > > > Just moving the three functions should be trivial. I'm not 
> > > > > > > talking about the processMapOperand.
> > > > > > I've moved `getSizeInBytes`.
> > > > > > 
> > > > > > The other two functions make use of `mlir::Location` and thus can't 
> > > > > > be moved trivially.
> > > > > > 
> > > > > > I can still try to move them by individually passing the elements 
> > > > > > of `mlir::Location` but that might not be ideal. Is that what you'd 
> > > > > > like?
> > > > > What about a new header file in 
> > > > > `mlir/include/mlir/Target/LLVMIR/Dialect/**` shared by 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp`
> > > > >  and 
> > > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp`.
> > > > >  That should be doable. 
> > > > `mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp` 
> > > > and 
> > > > `mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp` 
> > > > already have access to the common `mlir::Location` type.
> > > > 
> > > > Problem is that `OMPIRBuilder.cpp` is the only common file between them 
> > > >  where I can move the two functions to. Currently there are no 
> > > > `mlir/**` include files in `OMPIRBuilder.cpp` and it seems to me like a 
> > > > strict design choice to have it that way.
> > > The functions can be header only. Why do you need to put them in the 
> > > OMPIRBuilder.cpp? I think it is better than duplicate the exact same code 
> > > over. 
> > Sorry, I misunderstood you earlier.
> > I've added a new header file 
> > `mlir/include/mlir/Target/LLVMIR/Dialect/Utils.h`, this is my first attempt 
> > at adding a new header file, please let me know if you find any issues.
> Thanks! That's what I had in mind. We might want to check with MLIR folks if 
> `mlir::utils` is suited for that. I don't mind if it is `mlir::omp::builder` 
> or smth similar since it is related to the OMPIRBuilder.
Since the utils file is common to all the dialects I kept it as `mlir::utils`.

How do I get the opinion from people working in MLIR on this, can you suggest 
some reviewers whom I can add?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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


[PATCH] D142932: Multilib YAML parsing

2023-02-01 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 493968.
michaelplatings added a comment.

Incorporated changes requested by @miyuki


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142932

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -187,3 +187,357 @@
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
 }
+
+static void diagnosticCallback(const llvm::SMDiagnostic &D, void *Out) {
+  *reinterpret_cast(Out) = D.getMessage();
+}
+
+static bool parse(MultilibSet &MS, MultilibFlagMap &MFM,
+  std::string &Diagnostic, const char *Data) {
+  return MS.parse(llvm::MemoryBufferRef(Data, "TEST"), MFM, diagnosticCallback,
+  &Diagnostic);
+}
+
+static bool parse(MultilibSet &MS, MultilibFlagMap &MFM, const char *Data) {
+  return MS.parse(llvm::MemoryBufferRef(Data, "TEST"), MFM);
+}
+
+TEST(MultilibTest, ParseInvalid) {
+  std::string Diagnostic;
+
+  MultilibSet MS;
+  MultilibFlagMap MFM;
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+variants:
+- dir: /abc
+  flags: []
+  printArgs: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+variants:
+- flags: []
+  printArgs: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'dir'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+variants:
+- dir: .
+  printArgs: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'flags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+variants:
+- dir: .
+  flags: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("missing required key 'printArgs'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+flagMap:
+- regex: abc
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic)
+  .contains("value required for 'matchFlags' or 'noMatchFlags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, MFM, Diagnostic, R"(
+flagMap:
+- dir: .
+  regex: '('
+  printArgs: []
+
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("parentheses not balanced"))
+  << Diagnostic;
+}
+
+TEST(MultilibTest, Parse) {
+  MultilibSet MS;
+  MultilibFlagMap MFM;
+  EXPECT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: .
+  flags: []
+  printArgs: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: abc
+  flags: []
+  printArgs: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/abc", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: pqr
+  flags: []
+  printArgs: [-mfloat-abi=soft]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/pqr", MS.begin()->gccSuffix());
+  EXPECT_EQ(std::vector({"-mfloat-abi=soft"}),
+MS.begin()->getPrintArgs());
+
+  EXPECT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: pqr
+  flags: []
+  printArgs: [-mfloat-abi=soft, -fno-exceptions]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ(std::vector({"-mfloat-abi=soft", "-fno-exceptions"}),
+MS.begin()->getPrintArgs());
+
+  EXPECT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: a
+  flags: []
+  printArgs: []
+- dir: b
+  flags: []
+  printArgs: []
+)"));
+  EXPECT_EQ(2U, MS.size());
+}
+
+static bool select(const std::vector &InFlags,
+   const MultilibSet &MS, const MultilibFlagMap &MFM,
+   Multilib &Selected) {
+  Multilib::flags_list Flags(MFM.getFlags(InFlags));
+  Flags.insert(InFlags.begin(), InFlags.end());
+  return MS.select(Flags, Selected);
+}
+
+TEST(MultilibTest, SelectSoft) {
+  MultilibSet MS;
+  MultilibFlagMap MFM;
+  Multilib Selected;
+  ASSERT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: s
+  flags: [softabi]
+  printArgs: []
+flagMap:
+- regex: mfloat-abi=soft
+  matchFlags: [softabi]
+- regex: mfloat-abi=softfp
+  matchFlags: [softabi]
+)"));
+  EXPECT_TRUE(select({"mfloat-abi=soft"}, MS, MFM, Selected));
+  EXPECT_TRUE(select({"mfloat-abi=softfp"}, MS, MFM, Selected));
+  EXPECT_FALSE(select({"mfloat-abi=hard"}, MS, MFM, Selected));
+}
+
+TEST(MultilibTest, SelectSoftFP) {
+  MultilibSet MS;
+  MultilibFlagMap MFM;
+  Multilib Selected;
+  ASSERT_TRUE(parse(MS, MFM, R"(
+variants:
+- dir: f
+  flags: [mfloat-abi=softfp]
+  printArgs: []
+)"));
+  EXPECT_FALSE(select({"mfloat-abi=soft"}, MS, MFM, Selected));
+  EXPECT_TRUE(select({"mfloat-abi=softfp"}, MS, MFM, Selected));
+  EXPECT_FALSE(select({"mfloat-abi=hard"}, MS, MFM, Selected));
+}
+
+TEST(MultilibTest, SelectHard) {
+  // If hard float is all that's available then se

[PATCH] D142991: [clang-tidy] Add --fix-mode and --nolint-prefix options

2023-02-01 Thread Kyle Edwards via Phabricator via cfe-commits
KyleFromKitware updated this revision to Diff 493969.
KyleFromKitware added a comment.

Added tests and fixed assertion error in release build.


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

https://reviews.llvm.org/D142991

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-fixit-or-nolint.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp
  clang/include/clang/Basic/DiagnosticFrontendKinds.td

Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -180,6 +180,7 @@
   "qualifier 'const' is needed for variables in address space '%0'">;
 
 def note_fixit_applied : Note<"FIX-IT applied suggested code changes">;
+def note_fixit_added_nolint : Note<"FIX-IT added NOLINT to suppress warning">;
 def note_fixit_in_macro : Note<
 "FIX-IT unable to apply suggested code changes in a macro">;
 def note_fixit_failed : Note<
Index: clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-nolint.cpp
@@ -0,0 +1,16 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-use-trailing-return-type,readability-identifier-length' -fix -fix-mode=nolint -nolint-prefix='FIXME: ' -export-fixes=%t.yaml -- > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp %s
+// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
+// RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
+
+// CHECK: /* FIXME: NOLINTNEXTLINE(modernize-use-trailing-return-type) */
+// CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning
+// CHECK-YAML: ReplacementText: "/* FIXME: NOLINTNEXTLINE(modernize-use-trailing-return-type) */\n"
+int func() {
+  // CHECK: /* FIXME: NOLINTNEXTLINE(readability-identifier-length) */
+  // CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning
+  // CHECK-YAML: ReplacementText: "  /* FIXME: NOLINTNEXTLINE(readability-identifier-length) */\n"
+  int i = 0;
+  return i;
+}
Index: clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-fixit-or-nolint.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/fix-mode-fixit-or-nolint.cpp
@@ -0,0 +1,17 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,modernize-use-trailing-return-type,readability-identifier-length' -fix -fix-mode=fixit-or-nolint -export-fixes=%t.yaml -- > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp %s
+// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
+// RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
+
+int func() {
+  // CHECK: auto func() -> int {
+  // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+  // CHECK-YAML: ReplacementText: auto
+  // CHECK-YAML: ReplacementText: ' -> int'
+  // CHECK: /* NOLINTNEXTLINE(readability-identifier-length) */
+  // CHECK-MESSAGES: note: FIX-IT added NOLINT to suppress warning
+  // CHECK-YAML: ReplacementText: "  /* NOLINTNEXTLINE(readability-identifier-length) */\n"
+  int i = 0;
+  return i;
+}
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -138,6 +138,25 @@
 )"),
   cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt FixMode("fix-mode", cl::desc(R"(
+How to fix warnings:
+  - 'fixit' (default) applies fix-it if available
+  - 'nolint' adds a NOLINT comment to suppress the
+warning, prefixed with the value of the
+--nolint-prefix argument
+  - 'fixit-or-nolint' applies fix-it if available,
+otherwise adds a NOLINT comment like the 'nolint'
+option
+)"),
+cl::init("fixit"),
+cl::cat(ClangTidyCategory));
+
+static cl::opt NoLintPrefix("nolint-prefix", cl::desc(R"(
+Prefix to be added to NOLINT comments.
+)"),
+ cl::init(""),
+ cl::cat(ClangTidyCategory));
+
 static cl::opt FormatStyle("format-style", cl::desc(R"(
 Style for formatting code around applied fixes:
   - 'none' (default) turns off formatting
@@ -473,6 +492,19 @@
 return 1;
   }
 
+  FixType Type = FT_FixIt;
+  if (FixMode.getValue() == "fixit")
+Typ

[PATCH] D142384: [C++20] Fix a crash with modules.

2023-02-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

I don't see any issues with this, so happy to LGTM. @rsmith  any concerns on 
your side?
@usaxena95


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

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


[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

2023-02-01 Thread Jan Sjödin via Phabricator via cfe-commits
jsjodin added a comment.

In D142244#4096777 , @clementval 
wrote:

> In D142244#4096648 , @jsjodin wrote:
>
>> In D142244#4096579 , @clementval 
>> wrote:
>>
>>> In D142244#4096546 , @jsjodin 
>>> wrote:
>>>
 In D142244#4096538 , @clementval 
 wrote:

> In D142244#4096512 , @jsjodin 
> wrote:
>
>> No, it should work for any configuration as far as I know. How are you 
>> running the test?
>
> Just with `check-flang`

 How does it fail? Can you show the output? Also what CPU are you running 
 on?
>>>
>>>
>>>
>>>   I run on `x86_64`
>>
>> How did you configure your build? I'd like to reproduce your build and see 
>> if I can make it fail on my machine. Right now the test passes.
>
> My bad .. my `CompilerInvocation.cpp` was overwritten and missed part of this 
> patch. It works fine. Sorry to have bothered you with that.

No problem, glad it works for you. I'm working on another patch that is 
configuration dependent so I want to make sure there are no errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142244

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


[PATCH] D142569: [WIP][OpenMP] Introduce kernel argument

2023-02-01 Thread Shilei Tian via Phabricator via cfe-commits
tianshilei1992 updated this revision to Diff 493978.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142569

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/include/Debug.h
  openmp/libomptarget/DeviceRTL/include/Interface.h
  openmp/libomptarget/DeviceRTL/include/State.h
  openmp/libomptarget/DeviceRTL/src/Configuration.cpp
  openmp/libomptarget/DeviceRTL/src/Debug.cpp
  openmp/libomptarget/DeviceRTL/src/Kernel.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp
  openmp/libomptarget/include/DeviceEnvironment.h
  openmp/libomptarget/include/Environment.h
  openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
  openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

Index: openmp/libomptarget/plugins/cuda/src/rtl.cpp
===
--- openmp/libomptarget/plugins/cuda/src/rtl.cpp
+++ openmp/libomptarget/plugins/cuda/src/rtl.cpp
@@ -23,7 +23,7 @@
 #include 
 
 #include "Debug.h"
-#include "DeviceEnvironment.h"
+#include "Environment.h"
 #include "omptarget.h"
 #include "omptargetplugin.h"
 
Index: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
===
--- openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -37,7 +37,7 @@
 #include "internal.h"
 #include "rt.h"
 
-#include "DeviceEnvironment.h"
+#include "Environment.h"
 #include "get_elf_mach_gfx_name.h"
 #include "omptargetplugin.h"
 #include "print_tracing.h"
Index: openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
===
--- openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
+++ openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
@@ -17,7 +17,7 @@
 #include 
 
 #include "Debug.h"
-#include "DeviceEnvironment.h"
+#include "Environment.h"
 #include "GlobalHandler.h"
 #include "PluginInterface.h"
 #include "omptarget.h"
Index: openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
===
--- openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
+++ openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
@@ -17,7 +17,7 @@
 #include 
 
 #include "Debug.h"
-#include "DeviceEnvironment.h"
+#include "Environment.h"
 #include "GlobalHandler.h"
 #include "PluginInterface.h"
 
Index: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
===
--- openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
+++ openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
@@ -19,7 +19,7 @@
 #include 
 
 #include "Debug.h"
-#include "DeviceEnvironment.h"
+#include "Environment.h"
 #include "GlobalHandler.h"
 #include "JIT.h"
 #include "MemoryManager.h"
@@ -627,6 +627,11 @@
 
   /// Map of host pinned allocations used for optimize device transfers.
   PinnedAllocationMapTy PinnedAllocs;
+
+private:
+  /// Return the kernel environment object for kernel \p Name.
+  Expected
+  getKernelEnvironmentForKernel(StringRef Name, DeviceImageTy &Image);
 };
 
 /// Class implementing common functionalities of offload plugins. Each plugin
Index: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
===
--- openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -554,32 +554,43 @@
   return Plugin::success();
 }
 
-Expected
-GenericDeviceTy::getExecutionModeForKernel(StringRef Name,
-   DeviceImageTy &Image) {
-  // Create a metadata object for the exec mode global (auto-generated).
-  StaticGlobalTy ExecModeGlobal(Name.data(),
-"_exec_mode");
+Expected
+GenericDeviceTy::getKernelEnvironmentForKernel(StringRef Name,
+   DeviceImageTy &Image) {
+  // Create a metadata object for the kernel environment object.
+  StaticGlobalTy KernelEnv(Name.data(), "_kernel_info");
 
-  // Retrieve execution mode for the kernel.

[PATCH] D142710: [clang][dataflow] Relax validity assumptions in `UncheckedOptionalAccessModel`.

2023-02-01 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D142710#4096325 , @ymandel wrote:

> In D142710#4094934 , @xazax.hun 
> wrote:
>
>> This change looks good to me. I wonder, however, whether the behavior should 
>> be parameterized in the future. E.g., whether the user of the analysis 
>> should be able to make a decision whether the analysis should be pessimistic 
>> or optimistic about unmodeled values.
>
> Interesting idea. I think this goes along with other places where we are 
> unsound. Here, we err on the side of soundness. but, in general, we should 
> have a configuration mechanism for this.  FWIW, the only reason we have 
> uninitialized values at this point is recursive types. We also limit the 
> depth of structs, but that should be removed given my recent patch to only 
> model relevant fields. I have an idea for lazy initialization of values that 
> I think could solve the recursion issue. Together, we could remove this 
> concept of unmodeled values altogether from the framework.

Oh, sounds great! I do think lazy initialization will be really valuable to 
reduce the number of unmodeled values, but not entirely sure if we can 
completely eliminate them. In case we end up creating new locations (different 
from the earlier ones) in every iteration of the loop it might be harder to 
reach a fixed point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142710

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


[PATCH] D142933: Add -print-multi-selection-flags argument

2023-02-01 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.

So again, I took these patches for a spin with RISC-V. As I mentioned before, I 
was using a GCC installation, so the multilib selection happens inside Gnu.cpp 
(findRISCVBareMetalMultilibs). The main trouble I had was at that point we 
don't have a reference to the ToolChain, so calling `getMultiSelectionFlags` 
was not possible. I had a go at making this function static, and it was mostly 
fine as you could just pass in the Triple and Driver, however 
`getSanitizerArgs` was not possible. I'm not sure what the solution here is.

Multilibs for RISCV are straightforward, they just depend arch+abi, so I added 
the arch extensions and abi to the Result in `getMultiSelectionFlags`.

With that modified `getMultiSelectionFlags`, it worked well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

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


[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

2023-02-01 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/Parse/Parser.cpp:1415
+//
+// FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+// and PopExpressionEvaluationContext().

It does seem a bit ad-hoc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143053

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


[PATCH] D142878: Add testing for Fuchsia multilib

2023-02-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

Thanks, some of these are no longer needed which could simplify this change so 
I sent out D143050  to clean up the Fuchsia 
driver first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142878

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


  1   2   3   >