[PATCH] D146582: [clang] Fix wrong highlight for _Complex

2023-03-22 Thread Ilyas Mustafazade via Phabricator via cfe-commits 
1lyasm updated this revision to Diff 507254.
1lyasm added a comment.

Deleted comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146582

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1252,6 +1252,8 @@
 
   unsigned DK = ExtraInitsIsError ? diag::err_excess_initializers
   : diag::ext_excess_initializers;
+  if (T->isAnyComplexType() && Index < IList->getNumInits() - 1)
+   ++Index;
   SemaRef.Diag(IList->getInit(Index)->getBeginLoc(), DK)
   << initKind << IList->getInit(Index)->getSourceRange();
 }


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -1252,6 +1252,8 @@
 
   unsigned DK = ExtraInitsIsError ? diag::err_excess_initializers
   : diag::ext_excess_initializers;
+  if (T->isAnyComplexType() && Index < IList->getNumInits() - 1)
+	++Index;
   SemaRef.Diag(IList->getInit(Index)->getBeginLoc(), DK)
   << initKind << IList->getInit(Index)->getSourceRange();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146582: [clang] Fix wrong highlight for _Complex

2023-03-22 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

Sorry but this is already being worked on in https://reviews.llvm.org/D146503.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146582

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

[PATCH] D146503: Fix highlighting issue with _complex and initialization list with more than 2 items

2023-03-22 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a subscriber: aaron.ballman.
tbaeder added a comment.

LGTM but I'll wait for @aaron.ballman to maybe comment on the diagnostic 
differences in C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146503

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

[PATCH] D146582: [clang] Fix wrong highlight for _Complex

2023-03-22 Thread Ilyas Mustafazade via Phabricator via cfe-commits 
1lyasm added a comment.

In D146582#4212300 , @tbaeder wrote:

> Sorry but this is already being worked on in https://reviews.llvm.org/D146503.

Ok, did not know, closing this. Thanks for mentioning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146582

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

[clang] 558b46f - [analyzer] Fix crashing getSValFromInitListExpr for nested initlists

2023-03-22 Thread Balazs Benics via cfe-commits 

Author: Balazs Benics
Date: 2023-03-22T08:43:09+01:00
New Revision: 558b46fde2db2215794336bbd08e411fee5240d7

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

LOG: [analyzer] Fix crashing getSValFromInitListExpr for nested initlists

In the following example, we will end up hitting the `llvm_unreachable()`:
https://godbolt.org/z/5sccc95Ec
```lang=C++
enum class E {};
const E glob[] = {{}};
void initlistWithinInitlist() {
  clang_analyzer_dump(glob[0]); // crashes at loading from `glob[0]`
}
```

We should just return `std::nullopt` instead for these cases.
It's better than crashing.

Reviewed By: xazax.hun

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initialization.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 46948c12617c0..49855305cecc0 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1849,8 +1849,12 @@ std::optional 
RegionStoreManager::getSValFromInitListExpr(
 // Go to the nested initializer list.
 ILE = IL;
   }
-  llvm_unreachable(
-  "Unhandled InitListExpr sub-expressions or invalid offsets.");
+
+  assert(ILE);
+
+  // FIXME: Unhandeled InitListExpr sub-expression, possibly constructing an
+  //enum?
+  return std::nullopt;
 }
 
 /// Returns an SVal, if possible, for the specified position in a string

diff  --git a/clang/test/Analysis/initialization.cpp 
b/clang/test/Analysis/initialization.cpp
index e5b94ea7d0a2b..e624ef5bae9e9 100644
--- a/clang/test/Analysis/initialization.cpp
+++ b/clang/test/Analysis/initialization.cpp
@@ -249,3 +249,10 @@ void glob_array_parentheses1() {
   clang_analyzer_eval(glob_arr9[1][2] == 7); // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_arr9[1][3] == 0); // expected-warning{{TRUE}}
 }
+
+enum class E {};
+const E glob[] = {{}};
+void initlistWithinInitlist() {
+  // no-crash
+  clang_analyzer_dump(glob[0]); // expected-warning-re {{reg_${{[0-9]+
+}



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

[PATCH] D146538: [analyzer] Fix crashing getSValFromInitListExpr for nested initlists

2023-03-22 Thread Balázs Benics via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG558b46fde2db: [analyzer] Fix crashing 
getSValFromInitListExpr for nested initlists (authored by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146538

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.cpp


Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -249,3 +249,10 @@
   clang_analyzer_eval(glob_arr9[1][2] == 7); // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_arr9[1][3] == 0); // expected-warning{{TRUE}}
 }
+
+enum class E {};
+const E glob[] = {{}};
+void initlistWithinInitlist() {
+  // no-crash
+  clang_analyzer_dump(glob[0]); // expected-warning-re {{reg_${{[0-9]+
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1849,8 +1849,12 @@
 // Go to the nested initializer list.
 ILE = IL;
   }
-  llvm_unreachable(
-  "Unhandled InitListExpr sub-expressions or invalid offsets.");
+
+  assert(ILE);
+
+  // FIXME: Unhandeled InitListExpr sub-expression, possibly constructing an
+  //enum?
+  return std::nullopt;
 }
 
 /// Returns an SVal, if possible, for the specified position in a string


Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -249,3 +249,10 @@
   clang_analyzer_eval(glob_arr9[1][2] == 7); // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_arr9[1][3] == 0); // expected-warning{{TRUE}}
 }
+
+enum class E {};
+const E glob[] = {{}};
+void initlistWithinInitlist() {
+  // no-crash
+  clang_analyzer_dump(glob[0]); // expected-warning-re {{reg_${{[0-9]+
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1849,8 +1849,12 @@
 // Go to the nested initializer list.
 ILE = IL;
   }
-  llvm_unreachable(
-  "Unhandled InitListExpr sub-expressions or invalid offsets.");
+
+  assert(ILE);
+
+  // FIXME: Unhandeled InitListExpr sub-expression, possibly constructing an
+  //enum?
+  return std::nullopt;
 }
 
 /// Returns an SVal, if possible, for the specified position in a string
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142905: [Driver] Change multilib selection algorithm

2023-03-22 Thread Petr Hosek via Phabricator via cfe-commits 
phosek added inline comments.



Comment at: clang/lib/Driver/MultilibBuilder.cpp:88
 
+static Multilib::flags_list getPrintOptions(const Multilib::flags_list &Flags) 
{
+  // Derive print options from flags.

Would it be possible to move this method directly into `Multilib` to avoid 
having to pass around the callback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142905

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

[PATCH] D146608: [CMake] Build runtimes for riscv64-unknown-fuchsia

2023-03-22 Thread Petr Hosek via Phabricator via cfe-commits 
phosek updated this revision to Diff 507260.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146608

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -204,7 +204,7 @@
 set(BUILTINS_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE 
PATH "")
   endforeach()
 
-  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia)
+  foreach(target 
x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
@@ -276,12 +276,12 @@
 
   set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
-  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia" CACHE 
STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia" 
CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
 endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -204,7 +204,7 @@
 set(BUILTINS_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
   endforeach()
 
-  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia)
+  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
@@ -276,12 +276,12 @@
 
   set(LLVM_RUNTIME_MULTILIBS "asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
-  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
 endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44745: [HWASan] Port HWASan to Linux x86-64 (clang)

2023-03-22 Thread Mingjie Xu via Phabricator via cfe-commits 
Enna1 added a comment.
Herald added subscribers: yaneury, supersymetrie, Chia-hungDuan, pengfei.
Herald added a project: All.

Hi, I'm curious about why hwasan requires PIE. Is there any documents mentioned 
this? Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D44745

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

[PATCH] D146538: [analyzer] Fix crashing getSValFromInitListExpr for nested initlists

2023-03-22 Thread Balázs Benics via Phabricator via cfe-commits 
steakhal added a comment.

Speaking of issue tracking, I think it would be much more valuable to our users 
if we had a proper release notes section for CSA. Right now it is so ad-hoc.
That is also true for crash fixes and backports. Maybe we should put more 
emphasis on that part for the next major release.
I'm not sure how to enforce it though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146538

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

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

2023-03-22 Thread Dominik Adamski via Phabricator via cfe-commits 
domada updated this revision to Diff 507269.
domada retitled this revision from "[WIP][Flang][AMDGPU][OpenMP] Save target 
features in OpenMP MLIR  dialect" to "[Flang][AMDGPU][OpenMP] Save target 
features in OpenMP MLIR  dialect".
domada added a comment.

Patch rebased


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

https://reviews.llvm.org/D145579

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendActions.h
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/target-cpu-features.f90
  flang/test/Lower/OpenMP/target_cpu_features.f90
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1437,6 +1437,40 @@
   return false;
 }
 
+// Set the omp.target_cpu attribute on the module with the specified string
+void OpenMPDialect::setTargetCpu(Operation *module, llvm::StringRef cpu) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu"}),
+  mlir::StringAttr::get(module->getContext(), cpu));
+}
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpu(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+void OpenMPDialect::setTargetCpuFeatures(Operation *module,
+ llvm::StringRef cpuFeatures) {
+  module->setAttr(mlir::StringAttr::get(module->getContext(),
+llvm::Twine{"omp.target_cpu_features"}),
+  mlir::StringAttr::get(module->getContext(), cpuFeatures));
+}
+
+// Return the value of the omp.target_cpu_features attribute stored in the
+// module if it exists, otherwise return empty by default
+std::string OpenMPDialect::getTargetCpuFeatures(Operation *module) {
+  if (Attribute targetCpu = module->getAttr("omp.target_cpu_features"))
+if (targetCpu.isa())
+  return targetCpu.dyn_cast().getValue().str();
+  return llvm::Twine{""}.str();
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -36,6 +36,21 @@
 // Return the value of the omp.is_device attribute stored in the module if it
 // exists, otherwise return false by default
 static bool getIsDevice(Operation* module);
+
+// Set the omp.target_cpu attribute on the module with the specified string
+static void setTargetCpu(Operation* module, StringRef cpu);
+
+// Return the value of the omp.target_cpu attribute stored in the module if it
+// exists, otherwise return empty by default
+static std::string getTargetCpu(Operation* module);
+
+// Set the omp.target_cpu_features attribute on the module with
+// the specified string
+static void setTargetCpuFeatures(Operation* module, StringRef cpuFeatures);
+
+// Return the value of the omp.target_cpu_features attribute stored in
+// the module if it exists, otherwise return empty by default
+static std::string getTargetCpuFeatures(Operation* module);
   }];
 }
 
Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -251,3 +251,212 @@
 
   return T.isAMDGCN() ? getArchNameAMDGCN(ProcKind) : getArchNameR600(ProcKind);
 }
+
+void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
+  StringMap &Features) {
+  // XXX - What does the member GPU mean if device name string passed here?
+  if (T.isAMDGCN()) {
+switch (parseArchAMDGCN(GPU)) {
+case GK_GFX1103:
+case GK_GFX1102:
+case GK_GFX1101:
+case GK_GFX1100:
+  Features["ci-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot8-insts"] = true;
+  Features["dot9-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] =

[PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-03-22 Thread Pavel Kosov via Phabricator via cfe-commits 
kpdev42 updated this revision to Diff 507272.
kpdev42 added a comment.

Update patch after landing D145057 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/types/TestEmptyBase.py
  lldb/test/API/types/empty_base_type.cpp

Index: lldb/test/API/types/empty_base_type.cpp
===
--- /dev/null
+++ lldb/test/API/types/empty_base_type.cpp
@@ -0,0 +1,23 @@
+struct C
+{
+ long c,d;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct A : B,C
+{
+ long a,b;
+} _a;
+
+
+int main() {
+  return _a.a; // Set breakpoint here.
+}
Index: lldb/test/API/types/TestEmptyBase.py
===
--- /dev/null
+++ lldb/test/API/types/TestEmptyBase.py
@@ -0,0 +1,42 @@
+"""
+Test that recursive types are handled correctly.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class EmptyBaseTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Find the line number to break for main.c.
+self.line = line_number('empty_base_type.cpp',
+'// Set breakpoint here.')
+
+self.sources = {
+'CXX_SOURCES': 'empty_base_type.cpp'}
+
+def test(self):
+"""Test that recursive structs are displayed correctly."""
+self.build(dictionary=self.sources)
+self.setTearDownCleanup(dictionary=self.sources)
+self.run_expr()
+
+def run_expr(self):
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"empty_base_type.cpp",
+self.line,
+num_expected_locations=-1,
+loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.runCmd("expression _a")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1460,6 +1460,8 @@
   if (!result)
 return;
 
+  const clang::CXXBaseSpecifier *prev_base =
+  base_classes.empty() ? nullptr : base_classes.back().get();
   base_classes.push_back(std::move(result));
 
   if (is_virtual) {
@@ -1476,6 +1478,20 @@
 // be removed from LayoutRecordType() in the external
 // AST source in clang.
   } else {
+if (prev_base) {
+  clang::CXXRecordDecl *prev_base_decl =
+  prev_base->getType()->getAsCXXRecordDecl();
+  if (prev_base_decl && !prev_base_decl->isEmpty()) {
+auto it = layout_info.base_offsets.find(prev_base_decl);
+assert(it != layout_info.base_offsets.end());
+if (it->second.getQuantity() == member_byte_offset) {
+  prev_base_decl->markEmpty();
+  for (auto *field : prev_base_decl->fields())
+field->addAttr(clang::NoUniqueAddressAttr::Create(
+ast->getASTContext(), clang::SourceRange()));
+}
+  }
+}
 layout_info.base_offsets.insert(std::make_pair(
 ast->GetAsCXXRecordDecl(base_class_clang_type.GetOpaqueQualType()),
 clang::CharUnits::fromQuantity(member_byte_offset)));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146376: Update static_assert message for redundant cases

2023-03-22 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder added a comment.

Since we only handle `BinaryOperator`s in the toplevel assertion expression, I 
think it should just be safe to never diagnose for them if it's an `BO_LOr` 
opcode, so you should be able to just check for that in 
`DiagnoseStaticAssertDetails()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146376

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

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

2023-03-22 Thread Dominik Adamski via Phabricator via cfe-commits 
domada added a comment.

D146612  presents the lowering from MLIR 
attributes to LLVM IR.


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

https://reviews.llvm.org/D145579

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

[PATCH] D145579: [Flang][AMDGPU][OpenMP] Save target features in OpenMP MLIR dialect

2023-03-22 Thread Andrzej Warzynski via Phabricator via cfe-commits 
awarzynski added a comment.

Really nice to see some shared code being elevated out of Clang into LLVM, 
thanks!

I've only reviewed on the Flang driver changes. I will let the OpenMP experts 
to review the remaining bits. All in all looks good, I've only made some small 
suggestions.

Thanks for working on this!




Comment at: flang/lib/Frontend/FrontendActions.cpp:93
 
+std::string CodeGenAction::getAllTargetFeatures() {
+  std::string allFeaturesStr;

This method could be simplified by following 
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code.
 For example:

```
std::string CodeGenAction::getAllTargetFeatures()  {
  if (!triple.isAMDGPU()) {
allFeaturesStr = llvm::join(targetOpts.featuresAsWritten.begin(),
targetOpts.featuresAsWritten.end(), ",");
return allFeaturesStr;
  }

  // The logic for AMDGPU
  // Perhaps add a dedicated hook: getExplicitAndImplicitAMDGPUTargetFeatures()
} 
```

Btw, this method does different things depending on the triple. Perhaps rename 
it as something more generic, e.g. `getTargetFeatures`? I think that the 
current name, `getAllTargetFeatures`, is a bit misleading (i.e. what does "all" 
mean?).

Also:
* make it `static`
* document


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

https://reviews.llvm.org/D145579

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

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme added a comment.

In D146514#4209931 , @xazax.hun wrote:

> This fix looks good for me for this particular problem, but I wonder whether 
> the solution is general enough. In case the analysis figures out that a call 
> would not return (e.g., the `value` method is called on a provably empty 
> optional, and it would throw instead of returning), would this approach still 
> work? Would the analysis update the `BlockReachable` bitvector on demand?

No, I think this is a different case.

`BlockReachable` is intended to encode reachability solely as defined by the 
structure of the CFG. The whole problem we're trying to solve in this patch is 
that the CFG understands that `noreturn` functions don't return and therefore 
eliminates any successor edges from calls to `noreturn` functions. This means 
that some of the CFG nodes become unreachable; specifically, in our case, the 
RHS of the `&&` and `||` become unreachable, and so the dataflow analysis never 
computes a value for them. We therefore need to make sure that we don't try to 
query the value of the RHS in this case.

The case of calling `value` on a provably empty optional is different: The CFG 
does not understand that such a call does not return and therefore generates 
successor edges as it would for any other function call.

A side effect of the logic that this patch adds is that we get some neat 
inference results, as demonstrated in the newly added test. However, generating 
these results isn't the main goal of this patch; the patch is merely intended 
to ensure that we can process CFGs containing unreachable nodes caused by 
`noreturn` functions.

It would certainly be nice to generate similar inference results in the example 
that you mention (`value` on a provably empty optional), but that would require 
more work and a different approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146514

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

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme updated this revision to Diff 507287.
mboehme added a comment.

Reworded comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146514

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  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
@@ -5104,4 +5104,70 @@
   });
 }
 
+// Repro for a crash that used to occur when we call a `noreturn` function
+// within one of the operands of a `&&` or `||` operator.
+TEST(TransferTest, NoReturnFunctionInsideShortCircuitedBooleanOp) {
+  std::string Code = R"(
+__attribute__((noreturn)) int doesnt_return();
+bool some_condition();
+void target(bool b1, bool b2) {
+  // Neither of these should crash. In addition, if we don't terminate the
+  // program, we know that the operators need to trigger the short-circuit
+  // logic, so `NoreturnOnRhsOfAnd` will be false and `NoreturnOnRhsOfOr`
+  // will be true.
+  bool NoreturnOnRhsOfAnd = b1 && doesnt_return() > 0;
+  bool NoreturnOnRhsOfOr = b2 || doesnt_return() > 0;
+
+  // Calling a `noreturn` function on the LHS of an `&&` or `||` makes the
+  // entire expression unreachable. So we know that in both of the following
+  // cases, if `target()` terminates, the `else` branch was taken.
+  bool NoreturnOnLhsMakesAndUnreachable = false;
+  if (some_condition())
+ doesnt_return() > 0 && some_condition();
+  else
+ NoreturnOnLhsMakesAndUnreachable = true;
+
+  bool NoreturnOnLhsMakesOrUnreachable = false;
+  if (some_condition())
+ doesnt_return() > 0 || some_condition();
+  else
+ NoreturnOnLhsMakesOrUnreachable = true;
+
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+// Check that [[p]] is reachable with a non-false flow condition.
+EXPECT_FALSE(Env.flowConditionImplies(Env.getBoolLiteralValue(false)));
+
+auto &B1 = getValueForDecl(ASTCtx, Env, "b1");
+EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(B1)));
+
+auto &NoreturnOnRhsOfAnd =
+getValueForDecl(ASTCtx, Env, "NoreturnOnRhsOfAnd");
+EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(NoreturnOnRhsOfAnd)));
+
+auto &B2 = getValueForDecl(ASTCtx, Env, "b2");
+EXPECT_TRUE(Env.flowConditionImplies(B2));
+
+auto &NoreturnOnRhsOfOr =
+getValueForDecl(ASTCtx, Env, "NoreturnOnRhsOfOr");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnRhsOfOr));
+
+auto &NoreturnOnLhsMakesAndUnreachable = getValueForDecl(
+ASTCtx, Env, "NoreturnOnLhsMakesAndUnreachable");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnLhsMakesAndUnreachable));
+
+auto &NoreturnOnLhsMakesOrUnreachable = getValueForDecl(
+ASTCtx, Env, "NoreturnOnLhsMakesOrUnreachable");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnLhsMakesOrUnreachable));
+  });
+}
+
 } // namespace
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -389,6 +389,20 @@
 ///   `Name` must be unique in `ASTCtx`.
 const ValueDecl *findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name);
 
+/// Returns the value (of type `ValueT`) for the given identifier.
+/// `ValueT` must be a subclass of `Value` and must be of the appropriate type.
+///
+/// Requirements:
+///
+///   `Name` must be unique in `ASTCtx`.
+template 
+ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env,
+llvm::StringRef Name) {
+  const ValueDecl *VD = findValueDecl(ASTCtx, Name);
+  assert(VD != nullptr);
+  return *cast(Env.getValue(*VD, SkipPast::None));
+}
+
 /// Creates and owns constraints which are boolean values.
 class ConstraintContext {
 public:
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeEr

[clang] ee2cd60 - [dataflow] Log flow condition to the correct stream.

2023-03-22 Thread Sam McCall via cfe-commits 

Author: Sam McCall
Date: 2023-03-22T10:57:21+01:00
New Revision: ee2cd606abd98380bc71974863354a0d54ccfab3

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

LOG: [dataflow] Log flow condition to the correct stream.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 970b17be224db..702aaff9c7e71 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -264,7 +264,8 @@ class DataflowAnalysisContext {
   /// `Val2` imposed by the flow condition.
   bool equivalentBoolValues(BoolValue &Val1, BoolValue &Val2);
 
-  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token);
+  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token,
+  llvm::raw_ostream &OS = 
llvm::dbgs());
 
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 44053246bb744..a1b813982502b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -342,7 +342,8 @@ BoolValue 
&DataflowAnalysisContext::buildAndSubstituteFlowConditionWithCache(
   return substituteBoolValue(*ConstraintsIt->second, SubstitutionsCache);
 }
 
-void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token) {
+void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token,
+llvm::raw_ostream &OS) {
   llvm::DenseSet Constraints = {&Token};
   llvm::DenseSet VisitedTokens;
   addTransitiveFlowConditionConstraints(Token, Constraints, VisitedTokens);
@@ -350,7 +351,7 @@ void 
DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token) {
   llvm::DenseMap AtomNames = {
   {&getBoolLiteralValue(false), "False"},
   {&getBoolLiteralValue(true), "True"}};
-  llvm::dbgs() << debugString(Constraints, AtomNames);
+  OS << debugString(Constraints, AtomNames);
 }
 
 const ControlFlowContext *

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 46fb7bd2fd5e9..e3bde37ea68f7 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -812,7 +812,7 @@ void Environment::dump(raw_ostream &OS) const {
   }
 
   OS << "FlowConditionToken:\n";
-  DACtx->dumpFlowCondition(*FlowConditionToken);
+  DACtx->dumpFlowCondition(*FlowConditionToken, OS);
 }
 
 void Environment::dump() const {



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

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme updated this revision to Diff 507288.
mboehme added a comment.

Reworded comment again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146514

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  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
@@ -5104,4 +5104,70 @@
   });
 }
 
+// Repro for a crash that used to occur when we call a `noreturn` function
+// within one of the operands of a `&&` or `||` operator.
+TEST(TransferTest, NoReturnFunctionInsideShortCircuitedBooleanOp) {
+  std::string Code = R"(
+__attribute__((noreturn)) int doesnt_return();
+bool some_condition();
+void target(bool b1, bool b2) {
+  // Neither of these should crash. In addition, if we don't terminate the
+  // program, we know that the operators need to trigger the short-circuit
+  // logic, so `NoreturnOnRhsOfAnd` will be false and `NoreturnOnRhsOfOr`
+  // will be true.
+  bool NoreturnOnRhsOfAnd = b1 && doesnt_return() > 0;
+  bool NoreturnOnRhsOfOr = b2 || doesnt_return() > 0;
+
+  // Calling a `noreturn` function on the LHS of an `&&` or `||` makes the
+  // entire expression unreachable. So we know that in both of the following
+  // cases, if `target()` terminates, the `else` branch was taken.
+  bool NoreturnOnLhsMakesAndUnreachable = false;
+  if (some_condition())
+ doesnt_return() > 0 && some_condition();
+  else
+ NoreturnOnLhsMakesAndUnreachable = true;
+
+  bool NoreturnOnLhsMakesOrUnreachable = false;
+  if (some_condition())
+ doesnt_return() > 0 || some_condition();
+  else
+ NoreturnOnLhsMakesOrUnreachable = true;
+
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+// Check that [[p]] is reachable with a non-false flow condition.
+EXPECT_FALSE(Env.flowConditionImplies(Env.getBoolLiteralValue(false)));
+
+auto &B1 = getValueForDecl(ASTCtx, Env, "b1");
+EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(B1)));
+
+auto &NoreturnOnRhsOfAnd =
+getValueForDecl(ASTCtx, Env, "NoreturnOnRhsOfAnd");
+EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(NoreturnOnRhsOfAnd)));
+
+auto &B2 = getValueForDecl(ASTCtx, Env, "b2");
+EXPECT_TRUE(Env.flowConditionImplies(B2));
+
+auto &NoreturnOnRhsOfOr =
+getValueForDecl(ASTCtx, Env, "NoreturnOnRhsOfOr");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnRhsOfOr));
+
+auto &NoreturnOnLhsMakesAndUnreachable = getValueForDecl(
+ASTCtx, Env, "NoreturnOnLhsMakesAndUnreachable");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnLhsMakesAndUnreachable));
+
+auto &NoreturnOnLhsMakesOrUnreachable = getValueForDecl(
+ASTCtx, Env, "NoreturnOnLhsMakesOrUnreachable");
+EXPECT_TRUE(Env.flowConditionImplies(NoreturnOnLhsMakesOrUnreachable));
+  });
+}
+
 } // namespace
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -389,6 +389,20 @@
 ///   `Name` must be unique in `ASTCtx`.
 const ValueDecl *findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name);
 
+/// Returns the value (of type `ValueT`) for the given identifier.
+/// `ValueT` must be a subclass of `Value` and must be of the appropriate type.
+///
+/// Requirements:
+///
+///   `Name` must be unique in `ASTCtx`.
+template 
+ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env,
+llvm::StringRef Name) {
+  const ValueDecl *VD = findValueDecl(ASTCtx, Name);
+  assert(VD != nullptr);
+  return *cast(Env.getValue(*VD, SkipPast::None));
+}
+
 /// Creates and owns constraints which are boolean values.
 class ConstraintContext {
 public:
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/

[PATCH] D146527: [dataflow] Log flow condition to the correct stream.

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee2cd606abd9: [dataflow] Log flow condition to the correct 
stream. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146527

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -812,7 +812,7 @@
   }
 
   OS << "FlowConditionToken:\n";
-  DACtx->dumpFlowCondition(*FlowConditionToken);
+  DACtx->dumpFlowCondition(*FlowConditionToken, OS);
 }
 
 void Environment::dump() const {
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -342,7 +342,8 @@
   return substituteBoolValue(*ConstraintsIt->second, SubstitutionsCache);
 }
 
-void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token) {
+void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token,
+llvm::raw_ostream &OS) {
   llvm::DenseSet Constraints = {&Token};
   llvm::DenseSet VisitedTokens;
   addTransitiveFlowConditionConstraints(Token, Constraints, VisitedTokens);
@@ -350,7 +351,7 @@
   llvm::DenseMap AtomNames = {
   {&getBoolLiteralValue(false), "False"},
   {&getBoolLiteralValue(true), "True"}};
-  llvm::dbgs() << debugString(Constraints, AtomNames);
+  OS << debugString(Constraints, AtomNames);
 }
 
 const ControlFlowContext *
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -264,7 +264,8 @@
   /// `Val2` imposed by the flow condition.
   bool equivalentBoolValues(BoolValue &Val1, BoolValue &Val2);
 
-  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token);
+  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token,
+  llvm::raw_ostream &OS = 
llvm::dbgs());
 
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -812,7 +812,7 @@
   }
 
   OS << "FlowConditionToken:\n";
-  DACtx->dumpFlowCondition(*FlowConditionToken);
+  DACtx->dumpFlowCondition(*FlowConditionToken, OS);
 }
 
 void Environment::dump() const {
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -342,7 +342,8 @@
   return substituteBoolValue(*ConstraintsIt->second, SubstitutionsCache);
 }
 
-void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token) {
+void DataflowAnalysisContext::dumpFlowCondition(AtomicBoolValue &Token,
+llvm::raw_ostream &OS) {
   llvm::DenseSet Constraints = {&Token};
   llvm::DenseSet VisitedTokens;
   addTransitiveFlowConditionConstraints(Token, Constraints, VisitedTokens);
@@ -350,7 +351,7 @@
   llvm::DenseMap AtomNames = {
   {&getBoolLiteralValue(false), "False"},
   {&getBoolLiteralValue(true), "True"}};
-  llvm::dbgs() << debugString(Constraints, AtomNames);
+  OS << debugString(Constraints, AtomNames);
 }
 
 const ControlFlowContext *
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -264,7 +264,8 @@
   /// `Val2` imposed by the flow condition.
   bool equivalentBoolValues(BoolValue &Val1, BoolValue &Val2);
 
-  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token);
+  LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token,
+  llvm::raw_ostream &OS = llvm::dbgs());
 
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
___

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme marked an inline comment as done.
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:174-175
+  if (RHSVal == nullptr) {
+// If the RHS isn't reachable, this implies that if we end up 
evaluating
+// this BinaryOperator, the value of the LHS must have triggered the
+// short-circuit logic. This implies that the value of the entire

ymandel wrote:
> nit. reworded (using A => B => C  is the same as A & B => C).  but, your call.
Thanks, this is easier to understand. Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146514

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

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme added a comment.

After some offline discussion with reviewers, we've come to the conclusion that 
`AnalysisContext` isn't a sufficiently clear abstraction to warrant putting it 
in a header.

For the time being, I'll therefore pursue the less ambitious approach of simply 
eliminating the `StmtToEnvMap` interface and instead turning it into a concrete 
class with the implementation that used to be in `StmtToEnvMapImpl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

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

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme updated this revision to Diff 507297.
mboehme added a comment.

Pursue a less ambitious approach, as described in the previous comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

Files:
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -40,27 +40,6 @@
 namespace clang {
 namespace dataflow {
 
-class StmtToEnvMapImpl : public StmtToEnvMap {
-public:
-  StmtToEnvMapImpl(
-  const ControlFlowContext &CFCtx,
-  llvm::ArrayRef>
-  BlockToState)
-  : CFCtx(CFCtx), BlockToState(BlockToState) {}
-
-  const Environment *getEnvironment(const Stmt &S) const override {
-auto BlockIt = CFCtx.getStmtToBlock().find(&ignoreCFGOmittedNodes(S));
-assert(BlockIt != CFCtx.getStmtToBlock().end());
-const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
-assert(State);
-return &State->Env;
-  }
-
-private:
-  const ControlFlowContext &CFCtx;
-  llvm::ArrayRef> BlockToState;
-};
-
 /// Returns the index of `Block` in the successors of `Pred`.
 static int blockIndexInPredecessor(const CFGBlock &Pred,
const CFGBlock &Block) {
@@ -263,7 +242,7 @@
 TypeErasedDataflowAnalysisState PredState = *MaybePredState;
 if (Analysis.builtinOptions()) {
   if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-const StmtToEnvMapImpl StmtToEnv(AC.CFCtx, AC.BlockStates);
+const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
 auto [Cond, CondValue] =
 TerminatorVisitor(StmtToEnv, PredState.Env,
   blockIndexInPredecessor(*Pred, Block))
@@ -298,7 +277,7 @@
   AnalysisContext &AC) {
   const Stmt *S = Elt.getStmt();
   assert(S != nullptr);
-  transfer(StmtToEnvMapImpl(AC.CFCtx, AC.BlockStates), *S, InputState.Env);
+  transfer(StmtToEnvMap(AC.CFCtx, AC.BlockStates), *S, InputState.Env);
 }
 
 /// Built-in transfer function for `CFGInitializer`.
Index: clang/include/clang/Analysis/FlowSensitive/Transfer.h
===
--- clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -17,6 +17,7 @@
 #include "clang/AST/Stmt.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
 
 namespace clang {
 namespace dataflow {
@@ -24,12 +25,24 @@
 /// Maps statements to the environments of basic blocks that contain them.
 class StmtToEnvMap {
 public:
-  virtual ~StmtToEnvMap() = default;
-
-  /// Returns the environment of the basic block that contains `S` or nullptr 
if
-  /// there isn't one.
-  /// FIXME: Ensure that the result can't be null and return a const reference.
-  virtual const Environment *getEnvironment(const Stmt &S) const = 0;
+  StmtToEnvMap(const ControlFlowContext &CFCtx,
+   llvm::ArrayRef>
+   BlockToState)
+  : CFCtx(CFCtx), BlockToState(BlockToState) {}
+
+  /// Returns the environment of the basic block that contains `S`.
+  /// The result is guaranteed never to be null.
+  virtual const Environment *getEnvironment(const Stmt &S) const {
+auto BlockIt = CFCtx.getStmtToBlock().find(&ignoreCFGOmittedNodes(S));
+assert(BlockIt != CFCtx.getStmtToBlock().end());
+const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
+assert(State);
+return &State->Env;
+  }
+
+private:
+  const ControlFlowContext &CFCtx;
+  llvm::ArrayRef> BlockToState;
 };
 
 /// Evaluates `S` and updates `Env` accordingly.


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -40,27 +40,6 @@
 namespace clang {
 namespace dataflow {
 
-class StmtToEnvMapImpl : public StmtToEnvMap {
-public:
-  StmtToEnvMapImpl(
-  const ControlFlowContext &CFCtx,
-  llvm::ArrayRef>
-  BlockToState)
-  : CFCtx(CFCtx), BlockToState(BlockToState) {}
-
-  const Environment *getEnvironment(const Stmt &S) const override {
-auto BlockIt = CFCtx.getStmtToBlock().find(&ignoreCFGOmittedNodes(S));
-assert(BlockIt != CFCtx.getStmtToBlock().end());
-const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
-assert(State);
-return &State->Env;
-  }
-
-

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme added a comment.

@gribozavr2 @xazax.hun Just wanted to confirm that you still regard this as 
ready to land now that I've changed the patch as we discussed offline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

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

[PATCH] D146401: [clang-format] Don't squash Verilog escaped identifiers

2023-03-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTestVerilog.cpp:897
 }
+} // namespace
+} // namespace test

is this correct? do you have an anonymous namespace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146401

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

[PATCH] D146288: clang-tidy: Detect use-after-move in CXXCtorInitializer

2023-03-22 Thread Marco Falke via Phabricator via cfe-commits 
MarcoFalke updated this revision to Diff 507304.
MarcoFalke added a comment.

NFC


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

https://reviews.llvm.org/D146288

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp
@@ -369,6 +369,18 @@
 };
 a.foo();
   }
+  // Don't warn if 'a' is a copy inside a synchronous lambda
+  {
+A a;
+A copied{[a] mutable { return std::move(a); }()};
+a.foo();
+  }
+  // False negative (should warn if 'a' is a ref inside a synchronous lambda)
+  {
+A a;
+A moved{[&a] mutable { return std::move(a); }()};
+a.foo();
+  }
   // Warn if the use consists of a capture that happens after a move.
   {
 A a;
@@ -1367,6 +1379,120 @@
 }
 } // namespace UnevalContext
 
+class CtorInit {
+public:
+  CtorInit(std::string val)
+  : a{val.empty()},// fine
+s{std::move(val)},
+b{val.empty()}
+  // CHECK-NOTES: [[@LINE-1]]:11: warning: 'val' used after it was moved
+  // CHECK-NOTES: [[@LINE-3]]:9: note: move occurred here
+  {}
+
+private:
+  bool a;
+  std::string s;
+  bool b;
+};
+
+class CtorInitLambda {
+public:
+  CtorInitLambda(std::string val)
+  : a{val.empty()},// fine
+s{std::move(val)},
+b{[&] { return val.empty(); }()},
+// CHECK-NOTES: [[@LINE-1]]:12: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:9: note: move occurred here
+c{[] {
+  std::string str{};
+  std::move(str);
+  return str.empty();
+  // CHECK-NOTES: [[@LINE-1]]:18: warning: 'str' used after it was moved
+  // CHECK-NOTES: [[@LINE-3]]:11: note: move occurred here
+}()} {
+std::move(val);
+// CHECK-NOTES: [[@LINE-1]]:15: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-13]]:9: note: move occurred here
+std::string val2{};
+std::move(val2);
+val2.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val2' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+  }
+
+private:
+  bool a;
+  std::string s;
+  bool b;
+  bool c;
+  bool d{};
+};
+
+class CtorInitOrder {
+public:
+  CtorInitOrder(std::string val)
+  : a{val.empty()}, // fine
+b{val.empty()},
+// CHECK-NOTES: [[@LINE-1]]:11: warning: 'val' used after it was moved
+s{std::move(val)} {} // wrong order
+  // CHECK-NOTES: [[@LINE-1]]:9: note: move occurred here
+  // CHECK-NOTES: [[@LINE-4]]:11: note: the use happens in a later loop iteration than the move
+
+private:
+  bool a;
+  std::string s;
+  bool b;
+};
+
+struct Obj {};
+struct CtorD {
+  CtorD(Obj b);
+};
+
+struct CtorC {
+  CtorC(Obj b);
+};
+
+struct CtorB {
+  CtorB(Obj &b);
+};
+
+struct CtorA : CtorB, CtorC, CtorD {
+  CtorA(Obj b) : CtorB{b}, CtorC{std::move(b)}, CtorD{b} {}
+  // CHECK-NOTES: [[@LINE-1]]:55: warning: 'b' used after it was moved
+  // CHECK-NOTES: [[@LINE-2]]:34: note: move occurred here
+};
+
+struct Base {
+  Base(Obj b) : bb{std::move(b)} {}
+  template  Base(Call &&c) : bb{c()} {};
+
+  Obj bb;
+};
+
+struct Derived : Base, CtorC {
+  Derived(Obj b)
+  : Base{[&] mutable { return std::move(b); }()},
+// False negative: The lambda/std::move was executed, so it should warn
+// below
+CtorC{b} {}
+};
+
+struct Derived2 : Base, CtorC {
+  Derived2(Obj b)
+  : Base{[&] mutable { return std::move(b); }},
+// This was a move, but it doesn't warn below, because it can't know if
+// the lambda/std::move was actually called
+CtorC{b} {}
+};
+
+struct Derived3 : Base, CtorC {
+  Derived3(Obj b)
+  : Base{[c = std::move(b)] mutable { return std::move(c); }}, CtorC{b} {}
+  // CHECK-NOTES: [[@LINE-1]]:74: warning: 'b' used after it was moved
+  // CHECK-NOTES: [[@LINE-2]]:19: note: move occurred here
+};
+
 class PR38187 {
 public:
   PR38187(std::string val) : val_(std::move(val)) {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -162,6 +162,10 @@
   ` check.
   Global options of the same name should be used instead.
 
+- Improved :doc:`bugprone-use-after-move
+  ` check to also cover constructor
+  initializers.
+
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`google-build-namespaces
   ` check.
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-too

[PATCH] D146247: [clang-format] Support TypeScript satisfies operator

2023-03-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

1. your diff is messed up
2. you have to get an accept before anyone will commit for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146247

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

[PATCH] D146288: clang-tidy: Detect use-after-move in CXXCtorInitializer

2023-03-22 Thread Marco Falke via Phabricator via cfe-commits 
MarcoFalke marked 2 inline comments as done.
MarcoFalke added a comment.

Minor NFC, I plan to land this tomorrow or Friday


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

https://reviews.llvm.org/D146288

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

[PATCH] D146247: [clang-format] Support TypeScript satisfies operator

2023-03-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

Can you add a github issue and reference it here




Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1445
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_Unknown);
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_StartOfName);
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_Unknown);

I'd like to personally see this become a TT_JsSatisfies


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146247

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

[PATCH] D145813: [clang-format] Respect Cpp11BraceListStyle when aligning array of structures

2023-03-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20273
+  Style.ColumnLimit = 20;
+  EXPECT_EQ(
+  "demo = std::array<\n"

hch12907 wrote:
> MyDeveloperDay wrote:
> > any reason you didn't use verifyFormat here?
> No particular reason - I simply copied other tests and modified them to suit 
> the Cpp11BracedListStyle == false case, and one of those tests used EXPECT_EQ.
> 
> Should I change it to verifyFormat?
yes please as this tests the code getting messup and putting it back to normal



Comment at: clang/unittests/Format/FormatTest.cpp:20276
+  "struct test, 3>{\n"
+  "  test{\n"
+  "   56,23,\n"

just because we go over the limit do you think its correct that it pushes this 
back?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145813

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

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs and fields

2023-03-22 Thread Jirui Wu via Phabricator via cfe-commits 
JiruiWu marked 2 inline comments as done.
JiruiWu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5811
+// For alignment adjusted HFAs, cap the argument alignment to 16, otherwise
+// set it to 8 according to the AAPCS64 document.
 unsigned Align =

tmatheson wrote:
> Does the similar code added in D100853 need updated too?
No, because this patch is on AArch64 and https://reviews.llvm.org/D100853 is on 
AArch32. The alignment is capped to 16 in AArch64 and capped to 8 in AArch32.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5814
 getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity();
-unsigned BaseAlign = getContext().getTypeAlignInChars(Base).getQuantity();
-Align = (Align > BaseAlign && Align >= 16) ? 16 : 0;
+Align = (Align >= 16) ? 16 : 8;
 return ABIArgInfo::getDirect(

tmatheson wrote:
> Does this code definitely only apply when the ABI is AAPCS64, or should there 
> be a check for that somewhere here? I can't tell whether the `if` on line 
> 5806 is sufficient.
The code on line 5814 only applies when the ABI is AAPCS64 because it is in the 
`if` block that starts on line 5805 and ends on line 5818. As a result, the 
`if` on line 5806 is sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146242

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

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

2023-03-22 Thread Michael Halkenhäuser via Phabricator via cfe-commits 
mhalk added a comment.

In D145591#4182360 , @jhuber6 wrote:

> I'm not a fan of the same warning being copied in 24 places. Why do we set 
> `LangOpts.IsOpenMP` on the GPU compilation side, couldn't we just filter out 
> the `-fopenmp` or whatever it is for the HIP job?

If your concern was on the clang / SemA side, I found appropriate helper 
functions to reduce the number of checks (`if (getLangOpts().HIP) { Diag(...) 
}`) from 19 to 4.
But e.g. in the test we will still emit 24 messages, for affected directives.

I doubt reducing the number of diagnostic messages is worth the effort and esp. 
cost during compilation when tackled e.g. via clang-tooling?! (unsure)
The idea being: emission of a single diagnostic message and pointing out all 
(24) affected locations.

I may be completely wrong, but I suspect that users affected by "using HIP 
language but OpenMP target offloading behavior is altered" will (want to) 
disable this particular diagnostic once they are "aware".




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8635
+  "HIP does not support OpenMP target directives; directive has been ignored">,
+  InGroup;
+

>>! In D145591#4182945, @jdoerfert wrote:
> I would emit an error. A warning only if we can ensure the code does 
> something sensible. Right now, I doubt that is the case, similarly I doubt we 
> actually ignore things.

@yaxunl Just wanted to point out: If we added `DefaultError` here, the SemA 
would emit an error as preferred by Johannes.
But one could still disable it via `-Wno-hip-omp-target-directives` even with 
`-Werror` (or also demote it via `-Wno-error=hip-omp-target-directives`).

Would that be acceptable for everyone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

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

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

2023-03-22 Thread Tom Eccles via Phabricator via cfe-commits 
tblah updated this revision to Diff 507321.
tblah added a comment.

Thanks for the suggestion

- Added a test to check that hlfir is not output without the flag
- clang-formatted CLOptions.inc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146278

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Tools/CLOptions.inc
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/mlir-pass-pipeline.f90
  flang/test/Fir/basic-program.fir
  flang/test/HLFIR/flang-experimental-hlfir-flag.f90

Index: flang/test/HLFIR/flang-experimental-hlfir-flag.f90
===
--- /dev/null
+++ flang/test/HLFIR/flang-experimental-hlfir-flag.f90
@@ -0,0 +1,19 @@
+! Test -flang-experimental-hlfir flag
+! RUN: %flang_fc1 -flang-experimental-hlfir -emit-fir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck %s --check-prefix NO-HLFIR
+
+subroutine test(a, res)
+  real :: a(:), res
+  res = SUM(a)
+end subroutine
+! CHECK-LABEL: func.func @_QPtest
+! CHECK:   %[[A:.*]]: !fir.box>
+! CHECK:   %[[RES:.*]]: !fir.ref
+! CHECK-DAG: %[[A_VAR:.*]]:2 = hlfir.declare %[[A]]
+! CHECK-DAG: %[[RES_VAR:.*]]:2 = hlfir.declare %[[RES]]
+! CHECK-NEXT:%[[SUM_RES:.*]] = hlfir.sum %[[A_VAR]]#0
+! CHECK-NEXT:hlfir.assign %[[SUM_RES]] to %[[RES_VAR]]#0
+! CHECK-NEXT:hlfir.destroy %[[SUM_RES]]
+! CHECK-NEXT:return
+! CHECK-NEXT:  }
+! NO-HLFIR-NOT: hlfir.
Index: flang/test/Fir/basic-program.fir
===
--- flang/test/Fir/basic-program.fir
+++ flang/test/Fir/basic-program.fir
@@ -16,7 +16,11 @@
 
 // PASSES: Pass statistics report
 
-// PASSES:  CSE
+// PASSES:Canonicalizer
+// PASSES-NEXT:   LowerHLFIRIntrinsics
+// PASSES-NEXT:   BufferizeHLFIR
+// PASSES-NEXT:   ConvertHLFIRtoFIR
+// PASSES-NEXT:   CSE
 // PASSES-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
Index: flang/test/Driver/mlir-pass-pipeline.f90
===
--- flang/test/Driver/mlir-pass-pipeline.f90
+++ flang/test/Driver/mlir-pass-pipeline.f90
@@ -12,6 +12,10 @@
 ! ALL: Pass statistics report
 
 ! ALL: Fortran::lower::VerifierPass
+! O2-NEXT: Canonicalizer
+! ALL-NEXT: LowerHLFIRIntrinsics
+! ALL-NEXT: BufferizeHLFIR
+! ALL-NEXT: ConvertHLFIRtoFIR
 ! ALL-NEXT: CSE
 ! Ideally, we need an output with only the pass names, but
 ! there is currently no way to get that, so in order to
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -41,6 +41,8 @@
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flang-experimental-exec
 ! CHECK-NEXT:Enable support for generating executables (experimental)
+! CHECK-NEXT: -flang-experimental-hlfir
+! CHECK-NEXT:Use HLFIR lowering (experimental)
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -flto= Set LTO mode
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -818,6 +818,11 @@
 success = false;
   }
 
+  // -flang-experimental-hlfir
+  if (args.hasArg(clang::driver::options::OPT_flang_experimental_hlfir)) {
+res.loweringOpts.setLowerToHighLevelFIR(true);
+  }
+
   success &= parseFrontendArgs(res.getFrontendOpts(), args, diags);
   parseTargetArgs(res.getTargetOpts(), args);
   parsePreprocessorArgs(res.getPreprocessorOpts(), args);
Index: flang/include/flang/Tools/CLOptions.inc
===
--- flang/include/flang/Tools/CLOptions.inc
+++ flang/include/flang/Tools/CLOptions.inc
@@ -14,6 +14,7 @@
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "mlir/Transforms/Passes.h"
 #include "flang/Optimizer/CodeGen/CodeGen.h"
+#include "flang/Optimizer/HLFIR/Passes.h"
 #include "flang/Optimizer/Transforms/Passes.h"
 #include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Support/CommandLine.h"
@@ -72,7 +73,8 @@
 "rewrite boxed procedures");
 #endif
 
-DisableOption(ExternalNameConversion, "external-name-interop", "convert names with external convention");
+DisableOption(ExternalNameConversion, "external-name-interop",
+"convert names with external convention");
 
 /// Generic for adding a pass to the pass manager if it is not di

[PATCH] D146244: [clangd] Show used symbols on #include line hover.

2023-03-22 Thread Viktoriia Bakalova via Phabricator via cfe-commits 
VitaNuo updated this revision to Diff 507326.
VitaNuo marked 10 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146244

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -10,6 +10,7 @@
 #include "Annotations.h"
 #include "Config.h"
 #include "Hover.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -20,6 +21,7 @@
 
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -2978,6 +2980,67 @@
 EXPECT_EQ(Case.HI.present().asMarkdown(), Case.ExpectedMarkdown);
 }
 
+TEST(Hover, UsedSymbols) {
+  struct {
+Annotations Code;
+const std::function ExpectedBuilder;
+  } Cases[] = {{Annotations(R"cpp(
+#inclu^de "foo.h"
+int var = foo();
+  )cpp"),
+[](HoverInfo &HI) { HI.UsedSymbolNames = {"foo"}; }},
+   {Annotations("[[#incl^ude \"foo.h\"]]"),
+[](HoverInfo &HI) { HI.UsedSymbolNames = {}; }},
+   {Annotations(R"cpp(
+#include "foo.h"  
+#include ^"bar.h"
+int fooVar = foo();
+int barVar = bar();
+int foobarVar = foobar();
+X x;
+  )cpp"),
+[](HoverInfo &HI) {
+  HI.UsedSymbolNames = {"X", "bar", "foobar"};
+}},
+   {Annotations(R"cpp(
+#in^clude 
+int fooVar = foo();
+  )cpp"),
+[](HoverInfo &HI) { HI.UsedSymbolNames = {"foo"}; }},
+   {Annotations(R"cpp(
+#in^clude "public.h"
+#include "private.h"
+int fooVar = foo();
+  )cpp"),
+[](HoverInfo &HI) { HI.UsedSymbolNames = {"foo"}; }}
+
+  };
+  for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Code.code());
+
+TestTU TU;
+TU.Filename = "foo.cpp";
+TU.Code = Case.Code.code();
+TU.AdditionalFiles["foo.h"] = guard("int foo();");
+TU.AdditionalFiles["bar.h"] = guard("int bar(); int foobar(); class X {};");
+TU.AdditionalFiles["private.h"] = guard(R"cpp(
+// IWYU pragma: private, include "public.h"
+int foo(); 
+  )cpp");
+TU.AdditionalFiles["public.h"] = guard("");
+TU.AdditionalFiles["system/foo"] = guard("int foo();");
+TU.ExtraArgs.push_back("-isystem" + testPath("system"));
+
+auto AST = TU.build();
+auto H = getHover(AST, Case.Code.point(), format::getLLVMStyle(), nullptr);
+ASSERT_TRUE(H);
+HoverInfo Expected;
+Case.ExpectedBuilder(Expected);
+SCOPED_TRACE(H->present().asMarkdown());
+EXPECT_EQ(H->UsedSymbolNames, Expected.UsedSymbolNames);
+  }
+}
+
 TEST(Hover, DocsFromIndex) {
   Annotations T(R"cpp(
   template  class X {};
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -78,6 +78,9 @@
 /// representation. The spelling contains the ""<> characters.
 std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
 include_cleaner::Header Provider);
+
+std::vector
+collectMacroReferences(ParsedAST &AST);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -124,27 +124,6 @@
   return true;
 }
 
-std::vector
-collectMacroReferences(ParsedAST &AST) {
-  const auto &SM = AST.getSourceManager();
-  //  FIXME: !!this is a hacky way to collect macro references.
-  std::vector Macros;
-  auto &PP = AST.getPreprocessor();
-  for (const syntax::Token &Tok :
-   AST.getTokens().spelledTokens(SM.g

[PATCH] D146244: [clangd] Show used symbols on #include line hover.

2023-03-22 Thread Viktoriia Bakalova via Phabricator via cfe-commits 
VitaNuo added a comment.

Thanks for the comments!




Comment at: clang-tools-extra/clangd/Hover.cpp:1158
+  const SourceManager &SM = AST.getSourceManager();
+  std::set UsedSymbolNames;
+  include_cleaner::walkUsed(

hokein wrote:
> just want to check the intention, we're using the `set` here because we want 
> an alphabet order of `UsedSymbolNames`. If yes, then looks good (probably add 
> a comment in the field `UsedSymbolNames` of HoverInfo).
Actually no, we're using set primarily to deduplicate symbol names. Otherwise, 
many symbols have multiple references each, and the resulting list of used 
symbols ends up having a lot of duplicates.

I didn't know a set would order used symbols names alphabetically, but if so, 
that's a nice side effect :)





Comment at: clang-tools-extra/clangd/Hover.cpp:1171
+for (const include_cleaner::Header &H : Providers) {
+  switch (H.kind()) {
+  case include_cleaner::Header::Physical:

hokein wrote:
> since we expose `convertIncludes` in your previous patch, we can construct a 
> `include_cleaner::Includes` struct from the parssing `Inc`, and use the 
> `match()` method to match the header.
Yeah, good idea, this makes the function considerably shorter.



Comment at: clang-tools-extra/clangd/Hover.cpp:1448
+
+P.appendCode((*UsedSymbolNames)[UsedSymbolNames->size() - 1]);
+if (UsedSymbolNames->size() > 5) {

hokein wrote:
> if the UsedSymbolNames.size() > 5, we will show the last element rather than 
> the fifth one, my reading of the code this is not intended, right?
Right, sorry. That was only covering the case of UsedSymbolNames.size() <= 5.



Comment at: clang-tools-extra/clangd/Hover.h:116
+  // from a #include'd file that are used in the main file.
+  std::optional> UsedSymbolNames;
 

hokein wrote:
> we can use just a vector, we can use `.empty()` to the unused-include case.
> 
> `llvm::SmallVector` is preferred, per 
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h.
Ok for `std::optional` removal, but I wouldn't want to use `llvm::SmallVector`. 
`HoverInfo` does not contain any LLVM types atm (most of the fields are either 
`std::string` or `std::vector`), so I am not sure we want to introduce a 
divergence for unclear benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146244

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

[PATCH] D146503: Fix highlighting issue with _complex and initialization list with more than 2 items

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Thanks for this!

One thing that's missing is test coverage for the highlight itself, as that was 
what the original issue was about. You should add a test that uses 
`-fcaret-diagnostics-max-lines` and `FileCheck` so that you can test that the 
caret shows up where expected (along the lines of 
https://github.com/llvm/llvm-project/blob/main/clang/test/Misc/caret-diags-multiline.cpp).

You should also add a release note about the fix to 
`clang/docs/ReleaseNotes.rst`




Comment at: clang/test/Sema/complex-init-list.c:37
 struct teststruct invalid1 = { 1, 2 }; // expected-warning {{excess elements}}
-_Complex float invalid2 = { 1, 2, 3 }; // expected-warning {{excess elements}}
+_Complex float invalid2 = { 1, 2, 3 }; // expected-warning {{specifying real 
and imaginary components is an extension}} expected-warning {{excess elements 
in scalar initializer}}
 _Complex float invalid3 = {}; // expected-error {{scalar initializer cannot be 
empty}} expected-warning {{GNU empty initializer}}

This makes it a bit easier to see that there are two diagnostics being fired 
(even though we're not consistently doing that in this test file).



Comment at: clang/test/Sema/complex-init-list.c:51-52
+// initialization list
+_Complex double cd = {1.0, 2.0, 3.0}; // expected-warning {{specifying real 
and imaginary components is an extension}} expected-warning {{excess elements 
in scalar initializer}}
+_Complex float cf = {1.1f, 2.2f, 3.3f, 4.4f}; // expected-warning {{specifying 
real and imaginary components is an extension}} expected-warning {{excess 
elements in scalar initializer}}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146503

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

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Dmitri Gribenko via Phabricator via cfe-commits 
gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:35
+  /// The result is guaranteed never to be null.
+  virtual const Environment *getEnvironment(const Stmt &S) const {
+auto BlockIt = CFCtx.getStmtToBlock().find(&ignoreCFGOmittedNodes(S));

Drop virtual?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

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

[PATCH] D144115: [clang] Extend pragma dump to support expressions

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParsePragma.cpp:15-17
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/Preprocessor.h"

Endill wrote:
> aaron.ballman wrote:
> > 
> I'm using `warn_pragma_debug_unexpected_argument` and 
> `warn_pragma_debug_missing_argument` from there as well, and not just added 
> diagnostics. What should I do about that? Copying them over to 
> `DiagnosticParseKinds.td` doesn't work.
Ah, those should move into `DiagnosticCommonKinds.td` under the `// Parse && 
Lex` heading, then they should be available from here as well without the extra 
include.



Comment at: clang/lib/Parse/ParsePragma.cpp:731-736
+} else if (E.get()->isTypeDependent()) {
+  PP.Diag(StartLoc, diag::warn_pragma_debug_type_dependent_argument)
+<< SourceRange(StartLoc, Tok.getLocation());
+} else if (E.get()->isValueDependent()) {
+  PP.Diag(StartLoc, diag::warn_pragma_debug_value_dependent_argument)
+<< SourceRange(StartLoc, Tok.getLocation());

Endill wrote:
> aaron.ballman wrote:
> > 
> I see quite a lot of items inside `ExprDependence`. Are you sure we don't 
> miss any corner cases by using `isValueDependent()` to select between value- 
> and type-dependent diagnostic text? 
I was trying to think of a case where we'd run into problems, but I can't come 
up with one, so I believe we're fine here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144115

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

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

2023-03-22 Thread Qiongsi Wu via Phabricator via cfe-commits 
qiongsiwu1 added a comment.

Ping for review/comments. Thanks so much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

2023-03-22 Thread Andrzej Warzynski via Phabricator via cfe-commits 
awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: flang/test/HLFIR/flang-experimental-hlfir-flag.f90:19
+! CHECK-NEXT:  }
+! NO-HLFIR-NOT: hlfir.

[ultra nit] I would separate with an empty line for better readability. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146278

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

[PATCH] D146513: Add missing nearbyintf16 builtin functions

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a subscriber: cfe-commits.
aaron.ballman added a comment.

Please also add a release note and add documentation for the builtin.


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

https://reviews.llvm.org/D146513

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

[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added reviewers: rjmccall, serge-sans-paille.
aaron.ballman added a comment.

> Where do you suggest I add the documentation?

Somewhere around 
https://clang.llvm.org/docs/LanguageExtensions.html#extensions-to-specify-floating-point-flags
 where we document `clang fp eval_method`.


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

https://reviews.llvm.org/D146148

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

[PATCH] D146557: [MLIR][OpenMP] Added OMPIRBuilder support for use_device_ptr clause

2023-03-22 Thread Akash Banerjee via Phabricator via cfe-commits 
TIFitis updated this revision to Diff 507337.
TIFitis added a comment.

Removed use_dev_ptr clause related code. This patch only refactors the map 
clause prcoessing now.
Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/test/Target/LLVMIR/omptarget-llvm.mlir

Index: mlir/test/Target/LLVMIR/omptarget-llvm.mlir
===
--- mlir/test/Target/LLVMIR/omptarget-llvm.mlir
+++ mlir/test/Target/LLVMIR/omptarget-llvm.mlir
@@ -12,32 +12,24 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 4]
 // CHECK-LABEL: define void @_QPopenmp_target_data() {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
-// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
-// CHECK: %[[VAL_3:.*]] = alloca i32, i64 1, align 4
-// CHECK: br label %[[VAL_4:.*]]
-// CHECK:   entry:; preds = %[[VAL_5:.*]]
-// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_6]], align 8
-// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_7]], align 8
-// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %[[VAL_8]], align 4
+// CHECK: %[[VAL_2:.*]] = alloca i32, i64 1, align 4
+// CHECK: br label %[[VAL_3:.*]]
+// CHECK:   entry:; preds = %[[VAL_4:.*]]
+// CHECK: %[[VAL_5:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_5]], align 8
+// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_6]], align 8
+// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_7]], ptr %[[VAL_8]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: store i32 99, ptr %[[VAL_2]], align 4
 // CHECK: %[[VAL_9:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
 // CHECK: %[[VAL_10:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_11:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr %[[VAL_11]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
-// CHECK: br label %[[VAL_12:.*]]
-// CHECK:   omp.data.region:  ; preds = %[[VAL_4]]
-// CHECK: store i32 99, ptr %[[VAL_3]], align 4
-// CHECK: br label %[[VAL_13:.*]]
-// CHECK:   omp.region.cont:  ; preds = %[[VAL_12]]
-// CHECK: %[[VAL_14:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: %[[VAL_15:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_16:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_14]], ptr %[[VAL_15]], ptr %[[VAL_16]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
 // CHECK: ret void
 
 // -
@@ -56,33 +48,25 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 2]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64]  [i64 8]
 // CHECK-LABEL: define void @_QPopenmp_target_data_region
 // CHECK: (ptr %[[ARG_0:.*]]) {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: %[[VAL_1:.*]]

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

2023-03-22 Thread Dominik Adamski via Phabricator via cfe-commits 
domada updated this revision to Diff 507333.
domada retitled this revision from "[Flang][AMDGPU][OpenMP] Save target 
features in OpenMP MLIR  dialect" to "[Flang][AMDGPU] Add support for AMDGPU to 
Flang driver".
domada edited the summary of this revision.
domada added a comment.

Applied remarks.
Moved OpenMP changes to https://reviews.llvm.org/D146612 .


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

https://reviews.llvm.org/D145579

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/target-cpu-features.f90
  llvm/include/llvm/TargetParser/TargetParser.h
  llvm/lib/TargetParser/TargetParser.cpp

Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -251,3 +251,212 @@
 
   return T.isAMDGCN() ? getArchNameAMDGCN(ProcKind) : getArchNameR600(ProcKind);
 }
+
+void AMDGPU::fillAMDGPUFeatureMap(StringRef GPU, const Triple &T,
+  StringMap &Features) {
+  // XXX - What does the member GPU mean if device name string passed here?
+  if (T.isAMDGCN()) {
+switch (parseArchAMDGCN(GPU)) {
+case GK_GFX1103:
+case GK_GFX1102:
+case GK_GFX1101:
+case GK_GFX1100:
+  Features["ci-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot8-insts"] = true;
+  Features["dot9-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] = true;
+  Features["gfx9-insts"] = true;
+  Features["gfx10-insts"] = true;
+  Features["gfx10-3-insts"] = true;
+  Features["gfx11-insts"] = true;
+  break;
+case GK_GFX1036:
+case GK_GFX1035:
+case GK_GFX1034:
+case GK_GFX1033:
+case GK_GFX1032:
+case GK_GFX1031:
+case GK_GFX1030:
+  Features["ci-insts"] = true;
+  Features["dot1-insts"] = true;
+  Features["dot2-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot6-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] = true;
+  Features["gfx9-insts"] = true;
+  Features["gfx10-insts"] = true;
+  Features["gfx10-3-insts"] = true;
+  Features["s-memrealtime"] = true;
+  Features["s-memtime-inst"] = true;
+  break;
+case GK_GFX1012:
+case GK_GFX1011:
+  Features["dot1-insts"] = true;
+  Features["dot2-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot6-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot10-insts"] = true;
+  [[fallthrough]];
+case GK_GFX1013:
+case GK_GFX1010:
+  Features["dl-insts"] = true;
+  Features["ci-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["gfx8-insts"] = true;
+  Features["gfx9-insts"] = true;
+  Features["gfx10-insts"] = true;
+  Features["s-memrealtime"] = true;
+  Features["s-memtime-inst"] = true;
+  break;
+case GK_GFX940:
+  Features["gfx940-insts"] = true;
+  Features["fp8-insts"] = true;
+  [[fallthrough]];
+case GK_GFX90A:
+  Features["gfx90a-insts"] = true;
+  [[fallthrough]];
+case GK_GFX908:
+  Features["dot3-insts"] = true;
+  Features["dot4-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot6-insts"] = true;
+  Features["mai-insts"] = true;
+  [[fallthrough]];
+case GK_GFX906:
+  Features["dl-insts"] = true;
+  Features["dot1-insts"] = true;
+  Features["dot2-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot10-insts"] = true;
+  [[fallthrough]];
+case GK_GFX90C:
+case GK_GFX909:
+case GK_GFX904:
+case GK_GFX902:
+case GK_GFX900:
+  Features["gfx9-insts"] = true;
+  [[fallthrough]];
+case GK_GFX810:
+case GK_GFX805:
+case GK_GFX803:
+case GK_GFX802:
+case GK_GFX801:
+  Features["gfx8-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["s-memrealtime"] = true;
+  [[fallthrough]];
+case GK_GFX705:
+case GK_GFX704:
+case GK_GFX703:
+case GK_GFX702:
+case GK_GFX701:
+case GK_GFX700:
+  Features["ci-insts"] = true;
+  [[fallthrough]];
+case GK_GFX602:
+case GK_GFX601:
+case GK_GFX600:
+  Features["s-memtime-inst"] = true;
+  break;
+case GK_NONE:
+  break;
+default:
+  llvm_unreachable("Unhandled GPU!");
+}
+  } else {
+if (GPU.empty(

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel accepted this revision.
ymandel added a comment.

thanks!




Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:36-40
+auto BlockIt = CFCtx.getStmtToBlock().find(&ignoreCFGOmittedNodes(S));
+assert(BlockIt != CFCtx.getStmtToBlock().end());
+const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
+assert(State);
+return &State->Env;

Nit: why inline vs putting the definition in Transfer.cpp? I don't have a good 
sense for the size cutoff on this decision but if it's only used there, then 
might make sense just to keep the header leaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

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

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
sammccall updated this revision to Diff 507339.
sammccall added a comment.

Use a process-shared counter for HTML output filenames to avoid clobbering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

Files:
  clang/include/clang/Analysis/FlowSensitive/Logger.h
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.css
  clang/lib/Analysis/FlowSensitive/HTMLLogger.js
  clang/lib/Analysis/FlowSensitive/bundle_resources.py
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -9,6 +9,7 @@
 
 namespace clang::dataflow::test {
 namespace {
+using testing::HasSubstr;
 
 struct TestLattice {
   int Elements = 0;
@@ -83,19 +84,24 @@
   void logText(llvm::StringRef Text) override { OS << Text << "\n"; }
 };
 
-TEST(LoggerTest, Sequence) {
+AnalysisInputs makeInputs() {
   const char *Code = R"cpp(
 int target(bool b, int p, int q) {
   return b ? p : q;
 }
 )cpp";
+  static std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"};
 
   auto Inputs = AnalysisInputs(
   Code, ast_matchers::hasName("target"),
   [](ASTContext &C, Environment &) { return TestAnalysis(C); });
-  std::vector Args = {
-  "-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"};
   Inputs.ASTBuildArgs = Args;
+  return Inputs;
+}
+
+TEST(LoggerTest, Sequence) {
+  auto Inputs = makeInputs();
   std::string Log;
   TestLogger Logger(Log);
   Inputs.BuiltinOptions.Log = &Logger;
@@ -148,5 +154,27 @@
 )");
 }
 
+TEST(LoggerTest, HTML) {
+  auto Inputs = makeInputs();
+  std::vector Logs;
+  auto Logger = Logger::html([&]() {
+Logs.emplace_back();
+return std::make_unique(Logs.back());
+  });
+  Inputs.BuiltinOptions.Log = Logger.get();
+
+  ASSERT_THAT_ERROR(checkDataflow(std::move(Inputs),
+[](const AnalysisOutputs &) {}),
+llvm::Succeeded());
+
+  // Simple smoke tests: we can't meaningfully test the behavior.
+  ASSERT_THAT(Logs, testing::SizeIs(1));
+  EXPECT_THAT(Logs[0], HasSubstr("function updateSelection")) << "embeds JS";
+  EXPECT_THAT(Logs[0], HasSubstr("html {")) << "embeds CSS";
+  EXPECT_THAT(Logs[0], HasSubstr("b (ImplicitCastExpr")) << "has CFG elements";
+  EXPECT_THAT(Logs[0], HasSubstr(" !selection[k])) return; // no data to show
+if (!keys.some(k => k in changes)) return; // nothing changed
+
+let templateId = 'template-' + keys.map(k => selection[k]).join('_');
+let template = document.getElementById(templateId);
+if (template == null) {
+  console.error("missing template ", templateId);
+  return;
+}
+for (child of template.content.children) {
+  var slot = document.getElementById(child.id);
+  slot.replaceChildren(...child.cloneNode(/*deep=*/true).childNodes);
+}
+  }
+
+  updateData(['bb']);
+  updateData(['iter', 'elt']);
+
+  for (var k in changes)
+applyClassIf(k + '-select', classSelector(changes[k]));
+}
+
+// Handle a mouse event on a region containing selectable items.
+// This might end up changing the hover state or the selection state.
+//
+// targetSelector describes what target HTML element is selectable.
+// targetToID specifies how to determine the selection from it:
+//   hover: a function from target to the class name to highlight
+//   bb: a function from target to the basic-block name to select (BB4)A
+//   elt: a function from target to the CFG element name to select (BB4.5)
+//   iter: a function from target to the BB iteration to select (BB4:2)
+// If an entry is missing, the selection is unmodified.
+// If an entry is null, the selection is always cleared.
+function mouseEventHandler(event, targetSelector, targetToID) {
+  var target = event.type == "mouseout" ? null : event.target.closest(targetSelector);
+  let selTarget = k => (target && targetToID[k]) ? targetToID[k](target) : null;
+  if (event.type == "click") {
+let newSel = {};
+for (var k in targetToID) {
+  if (k == 'hover') continue;
+  let t = selTarget(k);
+  newSel[k] = t;
+}
+updateSelection(newSel);
+  } else if ("hover" in targetToID) {
+applyClassIf("hover", classSelector(selTarget("hover")));
+  }
+}
+function watch(rootSelector, targetSelector, targetToID) {
+  var root = document.querySelector(rootSelector);
+  console.log(root, rootSelector);
+  for (event of ['mouseout', 'mousemove', 'click'])
+root.addEventListener(event, e => mouseEventHandler(e, targetSelector, targetToID));
+}
+function watchAll(lastIter) {
+  watch('

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-03-22 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

@alexander-shaposhnikov : Are you working on @rsmith 's suggestion?  I finally 
have time to look into it, but dont want to repeat effort if you're already 1/2 
way into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

2023-03-22 Thread Dominik Adamski via Phabricator via cfe-commits 
domada marked an inline comment as done.
domada added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:93
 
+std::string CodeGenAction::getAllTargetFeatures() {
+  std::string allFeaturesStr;

awarzynski wrote:
> This method could be simplified by following 
> https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code.
>  For example:
> 
> ```
> std::string CodeGenAction::getAllTargetFeatures()  {
>   if (!triple.isAMDGPU()) {
> allFeaturesStr = llvm::join(targetOpts.featuresAsWritten.begin(),
> targetOpts.featuresAsWritten.end(), ",");
> return allFeaturesStr;
>   }
> 
>   // The logic for AMDGPU
>   // Perhaps add a dedicated hook: 
> getExplicitAndImplicitAMDGPUTargetFeatures()
> } 
> ```
> 
> Btw, this method does different things depending on the triple. Perhaps 
> rename it as something more generic, e.g. `getTargetFeatures`? I think that 
> the current name, `getAllTargetFeatures`, is a bit misleading (i.e. what does 
> "all" mean?).
> 
> Also:
> * make it `static`
> * document
Hi,
thanks for the review. I applied your remarks. I also moved OpenMP related 
changes to the child review.


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

https://reviews.llvm.org/D145579

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

[clang] acf6a32 - [flang] add -flang-experimental-hlfir flag to flang-new

2023-03-22 Thread Tom Eccles via cfe-commits 

Author: Tom Eccles
Date: 2023-03-22T13:36:54Z
New Revision: acf6a3224955779724a35a383d63c48af2163171

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

LOG: [flang] add -flang-experimental-hlfir flag to flang-new

This flag instructs flang-new to use the new HLFIR lowering. It is
marked as experimental and not included in --help.

This was added to make it more convenient to test the performance of
code generated by the HLFIR lowering.

Extra diffs are from running clang-format on CLOptions.inc (which was
being forced by CI).

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

Added: 
flang/test/HLFIR/flang-experimental-hlfir-flag.f90

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Flang.cpp
flang/include/flang/Tools/CLOptions.inc
flang/lib/Frontend/CompilerInvocation.cpp
flang/test/Driver/driver-help-hidden.f90
flang/test/Driver/mlir-pass-pipeline.f90
flang/test/Fir/basic-program.fir

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a05e61ac0e92f..b50dfd6f35510 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5080,6 +5080,10 @@ def flang_experimental_exec : Flag<["-"], 
"flang-experimental-exec">,
   Flags<[FlangOption, FlangOnlyOption, NoXarchOption, HelpHidden]>,
   HelpText<"Enable support for generating executables (experimental)">;
 
+def flang_experimental_hlfir : Flag<["-"], "flang-experimental-hlfir">,
+  Flags<[FlangOption, FC1Option, FlangOnlyOption, NoXarchOption, HelpHidden]>,
+  HelpText<"Use HLFIR lowering (experimental)">;
+
 
//===--===//
 // FLangOption + CoreOption + NoXarchOption
 
//===--===//

diff  --git a/clang/lib/Driver/ToolChains/Flang.cpp 
b/clang/lib/Driver/ToolChains/Flang.cpp
index 0a4a0de99b89f..23083ff3795b5 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -65,6 +65,9 @@ void Flang::addOtherOptions(const ArgList &Args, 
ArgStringList &CmdArgs) const {
   if (stackArrays &&
   !stackArrays->getOption().matches(options::OPT_fno_stack_arrays))
 CmdArgs.push_back("-fstack-arrays");
+
+  if (Args.hasArg(options::OPT_flang_experimental_hlfir))
+CmdArgs.push_back("-flang-experimental-hlfir");
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {

diff  --git a/flang/include/flang/Tools/CLOptions.inc 
b/flang/include/flang/Tools/CLOptions.inc
index 9324686138717..30581624d0dc5 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -14,6 +14,7 @@
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "mlir/Transforms/Passes.h"
 #include "flang/Optimizer/CodeGen/CodeGen.h"
+#include "flang/Optimizer/HLFIR/Passes.h"
 #include "flang/Optimizer/Transforms/Passes.h"
 #include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Support/CommandLine.h"
@@ -72,7 +73,8 @@ DisableOption(BoxedProcedureRewrite, 
"boxed-procedure-rewrite",
 "rewrite boxed procedures");
 #endif
 
-DisableOption(ExternalNameConversion, "external-name-interop", "convert names 
with external convention");
+DisableOption(ExternalNameConversion, "external-name-interop",
+"convert names with external convention");
 
 /// Generic for adding a pass to the pass manager if it is not disabled.
 template 
@@ -211,6 +213,20 @@ inline void 
createDefaultFIROptimizerPassPipeline(mlir::PassManager &pm,
   pm.addPass(mlir::createCSEPass());
 }
 
+/// Create a pass pipeline for lowering from HLFIR to FIR
+///
+/// \param pm - MLIR pass manager that will hold the pipeline definition
+/// \param optLevel - optimization level used for creating FIR optimization
+///   passes pipeline
+inline void createHLFIRToFIRPassPipeline(
+mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) 
{
+  if (optLevel.isOptimizingForSpeed())
+pm.addPass(mlir::createCanonicalizerPass());
+  pm.addPass(hlfir::createLowerHLFIRIntrinsicsPass());
+  pm.addPass(hlfir::createBufferizeHLFIRPass());
+  pm.addPass(hlfir::createConvertHLFIRtoFIRPass());
+}
+
 #if !defined(FLANG_EXCLUDE_CODEGEN)
 inline void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm,
 llvm::OptimizationLevel optLevel = defaultOptLevel,
@@ -218,8 +234,7 @@ inline void 
createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm,
   fir::addBoxedProcedurePass(pm);
   pm.addNestedPass(
   fir::createAbstractResultOnFuncOptPass());
-  pm.addNestedPass(
-  fir::createAbstractResultOnGlobalOptPass());
+  pm.addNestedPass(fir::createAbstractResultOnGlobalOptPass());

[PATCH] D146278: [flang] add -flang-experimental-hlfir flag to flang-new

2023-03-22 Thread Tom Eccles 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 rGacf6a3224955: [flang] add -flang-experimental-hlfir flag to 
flang-new (authored by tblah).

Changed prior to commit:
  https://reviews.llvm.org/D146278?vs=507321&id=507343#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146278

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Tools/CLOptions.inc
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/mlir-pass-pipeline.f90
  flang/test/Fir/basic-program.fir
  flang/test/HLFIR/flang-experimental-hlfir-flag.f90

Index: flang/test/HLFIR/flang-experimental-hlfir-flag.f90
===
--- /dev/null
+++ flang/test/HLFIR/flang-experimental-hlfir-flag.f90
@@ -0,0 +1,20 @@
+! Test -flang-experimental-hlfir flag
+! RUN: %flang_fc1 -flang-experimental-hlfir -emit-fir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -o - %s | FileCheck %s --check-prefix NO-HLFIR
+
+subroutine test(a, res)
+  real :: a(:), res
+  res = SUM(a)
+end subroutine
+! CHECK-LABEL: func.func @_QPtest
+! CHECK:   %[[A:.*]]: !fir.box>
+! CHECK:   %[[RES:.*]]: !fir.ref
+! CHECK-DAG: %[[A_VAR:.*]]:2 = hlfir.declare %[[A]]
+! CHECK-DAG: %[[RES_VAR:.*]]:2 = hlfir.declare %[[RES]]
+! CHECK-NEXT:%[[SUM_RES:.*]] = hlfir.sum %[[A_VAR]]#0
+! CHECK-NEXT:hlfir.assign %[[SUM_RES]] to %[[RES_VAR]]#0
+! CHECK-NEXT:hlfir.destroy %[[SUM_RES]]
+! CHECK-NEXT:return
+! CHECK-NEXT:  }
+
+! NO-HLFIR-NOT: hlfir.
Index: flang/test/Fir/basic-program.fir
===
--- flang/test/Fir/basic-program.fir
+++ flang/test/Fir/basic-program.fir
@@ -16,7 +16,11 @@
 
 // PASSES: Pass statistics report
 
-// PASSES:  CSE
+// PASSES:Canonicalizer
+// PASSES-NEXT:   LowerHLFIRIntrinsics
+// PASSES-NEXT:   BufferizeHLFIR
+// PASSES-NEXT:   ConvertHLFIRtoFIR
+// PASSES-NEXT:   CSE
 // PASSES-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
Index: flang/test/Driver/mlir-pass-pipeline.f90
===
--- flang/test/Driver/mlir-pass-pipeline.f90
+++ flang/test/Driver/mlir-pass-pipeline.f90
@@ -12,6 +12,10 @@
 ! ALL: Pass statistics report
 
 ! ALL: Fortran::lower::VerifierPass
+! O2-NEXT: Canonicalizer
+! ALL-NEXT: LowerHLFIRIntrinsics
+! ALL-NEXT: BufferizeHLFIR
+! ALL-NEXT: ConvertHLFIRtoFIR
 ! ALL-NEXT: CSE
 ! Ideally, we need an output with only the pass names, but
 ! there is currently no way to get that, so in order to
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -41,6 +41,8 @@
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flang-experimental-exec
 ! CHECK-NEXT:Enable support for generating executables (experimental)
+! CHECK-NEXT: -flang-experimental-hlfir
+! CHECK-NEXT:Use HLFIR lowering (experimental)
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -flto= Set LTO mode
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -818,6 +818,11 @@
 success = false;
   }
 
+  // -flang-experimental-hlfir
+  if (args.hasArg(clang::driver::options::OPT_flang_experimental_hlfir)) {
+res.loweringOpts.setLowerToHighLevelFIR(true);
+  }
+
   success &= parseFrontendArgs(res.getFrontendOpts(), args, diags);
   parseTargetArgs(res.getTargetOpts(), args);
   parsePreprocessorArgs(res.getPreprocessorOpts(), args);
Index: flang/include/flang/Tools/CLOptions.inc
===
--- flang/include/flang/Tools/CLOptions.inc
+++ flang/include/flang/Tools/CLOptions.inc
@@ -14,6 +14,7 @@
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "mlir/Transforms/Passes.h"
 #include "flang/Optimizer/CodeGen/CodeGen.h"
+#include "flang/Optimizer/HLFIR/Passes.h"
 #include "flang/Optimizer/Transforms/Passes.h"
 #include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Support/CommandLine.h"
@@ -72,7 +73,8 @@
 "rewrite boxed procedures");
 #endif
 
-DisableOption(ExternalNameConversion, "external-name-interop", "convert names with external convention");
+DisableOption(ExternalNameConversion, "extern

[PATCH] D146530: [clang][diagnostics]Removed "sorry" from all the required files

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a subscriber: AryanGodara.
aaron.ballman added a comment.

This appears to be the same efforts that have been going on in 
https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so that 
there's only one patch for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146530

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

[PATCH] D145579: [Flang][AMDGPU] Add support for AMDGPU to Flang driver

2023-03-22 Thread Andrzej Warzynski via Phabricator via cfe-commits 
awarzynski added a comment.

A few more comments, but mostly nits. Btw, is this patch sufficient to generate 
code for AMDGPU? Or, put differently, what's the level of support atm?




Comment at: clang/lib/Driver/ToolChains/Flang.cpp:107
 break;
+  case llvm::Triple::r600:
+  case llvm::Triple::amdgcn:

Should there be a test for this triple as well?



Comment at: flang/lib/Frontend/FrontendActions.cpp:134-137
+  if (!triple.isAMDGPU()) {
+return llvm::join(targetOpts.featuresAsWritten.begin(),
+  targetOpts.featuresAsWritten.end(), ",");
+  }

[nit] I would probably flip this as:
```
if (triple.isAMDGPU()) {
  // Clang does not append all target features to the clang -cc1 invocation.
  // Some AMDGPU features are passed implicitly by the Clang frontend.
  // That's why we need to extract implicit AMDGPU target features and add
  // them to the target features specified by the user
  return getExplicitAndImplicitAMDGPUTargetFeatures(ci, targetOpts, triple);
  }
``` 

I know that I suggested the opposite in my previous comment, but you have 
simplified this since :) In any case, feel free to ignore.



Comment at: flang/lib/Frontend/FrontendActions.cpp:139-142
+  // Clang does not append all target features to the clang -cc1 invocation.
+  // Some AMDGPU features are passed implicitly by the Clang frontend.
+  // That's why we need to extract implicit AMDGPU target features and add
+  // them to the target features specified by the user

[nit] IMHO this is documenting an implementation detail that would be more 
relevant inside `getExplicitAndImplicitAMDGPUTargetFeatures`.

More importantly, you are suggesting that the driver is doing whatever it is 
doing "because that's what Clang does". Consistency with Clang is important 
(you could call it out in the commit message) :) However, it would be even 
nicer to understand the actual rationale behind these implicit features. Any 
idea? Perhaps there are some clues in git history?

Also, perhaps a TODO to share this code with Clang? (to make it even clearer 
that the frontend driver aims for full consistency with Clang here).


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

https://reviews.llvm.org/D145579

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

[clang] 893ce57 - docs: add some documentation on Windows SDK search

2023-03-22 Thread Saleem Abdulrasool via cfe-commits 

Author: Saleem Abdulrasool
Date: 2023-03-22T09:46:36-04:00
New Revision: 893ce5759fe2e450dc637d4c76e779f883535882

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

LOG: docs: add some documentation on Windows SDK search

Add some documentation on the flags and the process by which clang
identifies the headers and libraries for the Windows environment.  It
should identify the flags and their interactions as well as the order in
which the various sources of information are consulted.

Differential Revision: https://reviews.llvm.org/D146165
Reviewed By: hans, mstorjo

Added: 


Modified: 
clang/docs/UsersManual.rst

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 77b1c938c6ad4..031d8a9b624d5 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -4487,3 +4487,126 @@ If the user is using the static CRT (``/MT``), then 
diff erent runtimes are used
 to produce DLLs and EXEs. To link a DLL, pass
 ``clang_rt.asan_dll_thunk-x86_64.lib``. To link an EXE, pass
 ``-wholearchive:clang_rt.asan-x86_64.lib``.
+
+Windows System Headers and Library Lookup
+^
+
+clang-cl uses a set of 
diff erent approaches to locate the right system libraries
+to link against when building code.  The Windows environment uses libraries 
from
+three distinct sources:
+
+1. Windows SDK
+2. UCRT (Universal C Runtime)
+3. Visual C++ Tools (VCRuntime)
+
+The Windows SDK provides the import libraries and headers required to build
+programs against the Windows system packages.  Underlying the Windows SDK is 
the
+UCRT, the universal C runtime.
+
+This 
diff erence is best illustrated by the various headers that one would find
+in the 
diff erent categories.  The WinSDK would contain headers such as
+`WinSock2.h` which is part of the Windows API surface, providing the Windows
+socketing interfaces for networking.  UCRT provides the C library headers,
+including e.g. `stdio.h`.  Finally, the Visual C++ tools provides the 
underlying
+Visual C++ Runtime headers such as `stdint.h` or `crtdefs.h`.
+
+There are various controls that allow the user control over where clang-cl will
+locate these headers.  The default behaviour for the Windows SDK and UCRT is as
+follows:
+
+1. Consult the command line.
+
+Anything the user specifies is always given precedence.  The following
+extensions are part of the clang-cl toolset:
+
+- `/winsysroot:`
+
+The `/winsysroot:` is used as an equivalent to `-sysroot` on Unix
+environments.  It allows the control of an alternate location to be treated
+as a system root.  When specified, it will be used as the root where the
+`Windows Kits` is located.
+
+- `/winsdkversion:`
+- `/winsdkdir:`
+
+If `/winsysroot:` is not specified, the `/winsdkdir:` argument is consulted
+as a location to identify where the Windows SDK is located.  Contrary to
+`/winsysroot:`, `/winsdkdir:` is expected to be the complete path rather
+than a root to locate `Windows Kits`.
+
+The `/winsdkversion:` flag allows the user to specify a version identifier
+for the SDK to prefer.  When this is specified, no additional validation is
+performed and this version is preferred.  If the version is not specified,
+the highest detected version number will be used.
+
+2. Consult the environment.
+
+TODO: This is not yet implemented.
+
+This will consult the environment variables:
+
+- `WindowsSdkDir`
+- `UCRTVersion`
+
+3. Fallback to the registry.
+
+If no arguments are used to indicate where the SDK is present, and the
+compiler is running on Windows, the registry is consulted to locate the
+installation.
+
+The Visual C++ Toolset has a slightly more elaborate mechanism for detection.
+
+1. Consult the command line.
+
+- `/winsysroot:`
+
+The `/winsysroot:` is used as an equivalent to `-sysroot` on Unix
+environments.  It allows the control of an alternate location to be treated
+as a system root.  When specified, it will be used as the root where the
+`VC` directory is located.
+
+- `/vctoolsdir:`
+- `/vctoolsversion:`
+
+If `/winsysroot:` is not specified, the `/vctoolsdir:` argument is 
consulted
+as a location to identify where the Visual C++ Tools are located.  If
+`/vctoolsversion:` is specified, that version is preferred, otherwise, the
+highest version detected is used.
+
+2. Consult the environment.
+
+- `/external:[VARIABLE]`
+
+  This specifies a user identified environment variable which is treated as
+  a path delimiter (`;`) separated list of paths to map into `-imsvc`
+  arguments which are treated as `-isystem`.
+
+- `INCLUDE` and `EXTERNAL_INCLUDE`
+
+

[PATCH] D146165: docs: add some documentation on Windows SDK search

2023-03-22 Thread Saleem Abdulrasool via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG893ce5759fe2: docs: add some documentation on Windows SDK 
search (authored by compnerd).

Changed prior to commit:
  https://reviews.llvm.org/D146165?vs=506978&id=507346#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146165

Files:
  clang/docs/UsersManual.rst

Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -4487,3 +4487,126 @@
 to produce DLLs and EXEs. To link a DLL, pass
 ``clang_rt.asan_dll_thunk-x86_64.lib``. To link an EXE, pass
 ``-wholearchive:clang_rt.asan-x86_64.lib``.
+
+Windows System Headers and Library Lookup
+^
+
+clang-cl uses a set of different approaches to locate the right system libraries
+to link against when building code.  The Windows environment uses libraries from
+three distinct sources:
+
+1. Windows SDK
+2. UCRT (Universal C Runtime)
+3. Visual C++ Tools (VCRuntime)
+
+The Windows SDK provides the import libraries and headers required to build
+programs against the Windows system packages.  Underlying the Windows SDK is the
+UCRT, the universal C runtime.
+
+This difference is best illustrated by the various headers that one would find
+in the different categories.  The WinSDK would contain headers such as
+`WinSock2.h` which is part of the Windows API surface, providing the Windows
+socketing interfaces for networking.  UCRT provides the C library headers,
+including e.g. `stdio.h`.  Finally, the Visual C++ tools provides the underlying
+Visual C++ Runtime headers such as `stdint.h` or `crtdefs.h`.
+
+There are various controls that allow the user control over where clang-cl will
+locate these headers.  The default behaviour for the Windows SDK and UCRT is as
+follows:
+
+1. Consult the command line.
+
+Anything the user specifies is always given precedence.  The following
+extensions are part of the clang-cl toolset:
+
+- `/winsysroot:`
+
+The `/winsysroot:` is used as an equivalent to `-sysroot` on Unix
+environments.  It allows the control of an alternate location to be treated
+as a system root.  When specified, it will be used as the root where the
+`Windows Kits` is located.
+
+- `/winsdkversion:`
+- `/winsdkdir:`
+
+If `/winsysroot:` is not specified, the `/winsdkdir:` argument is consulted
+as a location to identify where the Windows SDK is located.  Contrary to
+`/winsysroot:`, `/winsdkdir:` is expected to be the complete path rather
+than a root to locate `Windows Kits`.
+
+The `/winsdkversion:` flag allows the user to specify a version identifier
+for the SDK to prefer.  When this is specified, no additional validation is
+performed and this version is preferred.  If the version is not specified,
+the highest detected version number will be used.
+
+2. Consult the environment.
+
+TODO: This is not yet implemented.
+
+This will consult the environment variables:
+
+- `WindowsSdkDir`
+- `UCRTVersion`
+
+3. Fallback to the registry.
+
+If no arguments are used to indicate where the SDK is present, and the
+compiler is running on Windows, the registry is consulted to locate the
+installation.
+
+The Visual C++ Toolset has a slightly more elaborate mechanism for detection.
+
+1. Consult the command line.
+
+- `/winsysroot:`
+
+The `/winsysroot:` is used as an equivalent to `-sysroot` on Unix
+environments.  It allows the control of an alternate location to be treated
+as a system root.  When specified, it will be used as the root where the
+`VC` directory is located.
+
+- `/vctoolsdir:`
+- `/vctoolsversion:`
+
+If `/winsysroot:` is not specified, the `/vctoolsdir:` argument is consulted
+as a location to identify where the Visual C++ Tools are located.  If
+`/vctoolsversion:` is specified, that version is preferred, otherwise, the
+highest version detected is used.
+
+2. Consult the environment.
+
+- `/external:[VARIABLE]`
+
+  This specifies a user identified environment variable which is treated as
+  a path delimiter (`;`) separated list of paths to map into `-imsvc`
+  arguments which are treated as `-isystem`.
+
+- `INCLUDE` and `EXTERNAL_INCLUDE`
+
+  The path delimiter (`;`) separated list of paths will be mapped to
+  `-imsvc` arguments which are treated as `-isystem`.
+
+- `LIB` (indirectly)
+
+  The linker `link.exe` or `lld-link.exe` will honour the environment
+  variable `LIB` which is a path delimiter (`;`) set of paths to consult for
+  the import libraries to use when linking the final target.
+
+The following environment variables will be consulted and used to form paths
+to validate and load content from as appropriate:
+
+

[PATCH] D146530: [clang][diagnostics]Removed "sorry" from all the required files

2023-03-22 Thread Priyanshi Agarwal via Phabricator via cfe-commits 
ipriyanshi1708 added a comment.

In D146530#4213045 , @aaron.ballman 
wrote:

> This appears to be the same efforts that have been going on in 
> https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so 
> that there's only one patch for this?

Hi! I am an Outreachy applicant, firstly how can I coordinate with @AryanGodara 
and secondly how will it be counted as my contribution and then can I record it 
in my final application? Or I will have to look for some other issues for 
Outreachy?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146530

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

[PATCH] D146530: [clang][diagnostics]Removed "sorry" from all the required files

2023-03-22 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D146530#4213076 , @ipriyanshi1708 
wrote:

> In D146530#4213045 , @aaron.ballman 
> wrote:
>
>> This appears to be the same efforts that have been going on in 
>> https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so 
>> that there's only one patch for this?
>
> Hi! I am an Outreachy applicant, firstly how can I coordinate with 
> @AryanGodara and secondly how will it be counted as my contribution and then 
> can I record it in my final application? Or I will have to look for some 
> other issues for Outreachy?

Welcome! You can comment on the other review to see if the author would like to 
collaborate with you on a solution. We have the ability to land commits with 
co-author tags (please request it explicitly when someone approves the patch so 
that the person landing the work knows to do this), so that should get you both 
the attribution in GitHub that you did the work (no idea if Outreachy has rules 
around co-authoring though). You might have to look for other issues for 
Outreachy, depending on what they require.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146530

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

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

2023-03-22 Thread Tom Honermann via Phabricator via cfe-commits 
tahonermann added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:5890-5892
+  TargetOpts.EABIVersion = llvm::EABI::Default;
+  // Initialize CodeObjectVersion with default i.e., 4
+  TargetOpts.CodeObjectVersion = TargetOptions::CodeObjectVersionKind::COV_4;

It is odd that these values aren't preserved by the AST writer. I wonder if 
that is intentional.

It took me a while, but I eventually found that these data members are assigned 
via an inclusion of `clang/Driver/Options.inc` somewhere.

Rather than assigning values here, I think we would be better off adding member 
initializers in the class definition. But I think we do need to look into why 
the AST writer isn't preserving the values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146604

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

[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-03-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits 
zahiraam updated this revision to Diff 507360.

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

https://reviews.llvm.org/D146148

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/Inputs/math.h
  clang/test/Sema/abi-check-1.cpp
  clang/test/Sema/abi-check-2.cpp
  clang/test/Sema/abi-check-3.cpp

Index: clang/test/Sema/abi-check-3.cpp
===
--- /dev/null
+++ clang/test/Sema/abi-check-3.cpp
@@ -0,0 +1,151 @@
+// RUN: %clang_cc1  -isystem %S/Inputs -triple x86_64-linux-gnu \
+// RUN: -emit-llvm -o - %s | FileCheck %s
+
+// RUN: not %clang_cc1  -isystem %S/Inputs -triple x86_64-linux-gnu \
+// RUN: -ffp-eval-method=source -emit-obj -o %t %s 2>&1 \
+// RUN: | FileCheck -check-prefix=ERROR-1 %s
+
+// RUN: not %clang_cc1  -isystem %S/Inputs -triple x86_64-linux-gnu \
+// RUN: -ffp-eval-method=double -emit-obj -o %t %s 2>&1 \
+// RUN: | FileCheck -check-prefix=ERROR-2 %s
+
+// RUN: %clang_cc1  -isystem %S/Inputs -triple x86_64-linux-gnu \
+// RUN: -ffp-eval-method=extended -emit-llvm -o - %s \
+// RUN: | FileCheck -check-prefix=CHECK-EXT %s
+
+#include 
+
+float foo1() {
+#pragma clang fp eval_method(extended)
+  float a;
+  double b;
+  // CHECK: alloca float
+  // CHECK: alloca double
+  return a - b;
+}
+
+float foo2() {
+#pragma clang fp eval_method(extended)
+  float_t a; 
+  double_t b; 
+  // CHECK: alloca float
+  // CHECK: alloca double
+  // CHECK-EXT: alloca x86_fp80
+  // CHECK-EXT: alloca x86_fp80
+  // ERROR-1: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-1: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-2: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  // ERROR-2: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  return a - b;
+}
+
+void foo3() {
+#pragma clang fp eval_method(extended)
+  char buff[sizeof(float_t)];
+  char bufd[sizeof(double_t)];
+  // CHECK:  alloca [4 x i8]
+  // CHECK:  alloca [8 x i8]
+  // CHECK-DBL: alloca [8 x i8]
+  // CHECK-DBL: alloca [8 x i8]
+  // ERROR-1: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-1: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-2: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  // ERROR-2: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  buff[1] = bufd[2];
+}
+
+float foo4() {
+#pragma clang fp eval_method(extended)
+  typedef float_t FT;
+  typedef double_t DT;
+  FT a;
+  DT b;
+  // CHECK: alloca float
+  // CHECK: alloca double
+  // CHECK-EXT: alloca x86_fp80
+  // CHECK-EXT: alloca x86_fp80
+  // ERROR-1: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-1: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-2: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  // ERROR-2: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=double' is used
+  return a - b;
+}
+
+int foo5() {
+#pragma clang fp eval_method(extended)
+  int t = _Generic( 1.0L, float_t:1, default:0);
+  int v = _Generic( 1.0L, double_t:1, default:0);
+  // CHECK: alloca i32
+  // CHECK: alloca i32
+  // ERROR-1: error: float_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eval-method=source' is used
+  // ERROR-1: error: double_t type definition cannot be modified inside a scope containing '#pragma clang fp eval_method(extended)' when command line option 'ffp-eva

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

2023-03-22 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8635
+  "HIP does not support OpenMP target directives; directive has been ignored">,
+  InGroup;
+

mhalk wrote:
> >>! In D145591#4182945, @jdoerfert wrote:
> > I would emit an error. A warning only if we can ensure the code does 
> > something sensible. Right now, I doubt that is the case, similarly I doubt 
> > we actually ignore things.
> 
> @yaxunl Just wanted to point out: If we added `DefaultError` here, the SemA 
> would emit an error as preferred by Johannes.
> But one could still disable it via `-Wno-hip-omp-target-directives` even with 
> `-Werror` (or also demote it via `-Wno-error=hip-omp-target-directives`).
> 
> Would that be acceptable for everyone?
LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

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

[PATCH] D146625: [dataflow] handle missing case in value debug strings

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall published this revision for review.
sammccall added a reviewer: gribozavr2.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146625

Files:
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
@@ -27,7 +27,14 @@
   auto B = Ctx.atom();
 
   auto Expected = R"(B0)";
-  debugString(*B);
+  EXPECT_THAT(debugString(*B), StrEq(Expected));
+}
+
+TEST(BoolValueDebugStringTest, Top) {
+  ConstraintContext Ctx;
+  auto B = Ctx.top();
+
+  auto Expected = R"(top)";
   EXPECT_THAT(debugString(*B), StrEq(Expected));
 }
 
Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -106,6 +106,10 @@
   S = getAtomName(&cast(B));
   break;
 }
+case Value::Kind::TopBool: {
+  S = "top";
+  break;
+}
 case Value::Kind::Conjunction: {
   auto &C = cast(B);
   auto L = debugString(C.getLeftSubValue(), Depth + 1);


Index: clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
@@ -27,7 +27,14 @@
   auto B = Ctx.atom();
 
   auto Expected = R"(B0)";
-  debugString(*B);
+  EXPECT_THAT(debugString(*B), StrEq(Expected));
+}
+
+TEST(BoolValueDebugStringTest, Top) {
+  ConstraintContext Ctx;
+  auto B = Ctx.top();
+
+  auto Expected = R"(top)";
   EXPECT_THAT(debugString(*B), StrEq(Expected));
 }
 
Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -106,6 +106,10 @@
   S = getAtomName(&cast(B));
   break;
 }
+case Value::Kind::TopBool: {
+  S = "top";
+  break;
+}
 case Value::Kind::Conjunction: {
   auto &C = cast(B);
   auto L = debugString(C.getLeftSubValue(), Depth + 1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146417: [clangd] Fix AddUsing in the face of typo-correction

2023-03-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet closed this revision.
kadircet added a comment.

relanded in 35c2aac6e3957c2e82bf92269039fa02bab0e1d9 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146417

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

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

2023-03-22 Thread Sergio Afonso via Phabricator via cfe-commits 
skatrak updated this revision to Diff 507365.
skatrak marked 6 inline comments as done.
skatrak added a comment.

Changes to tests and path manipulation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146075

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Driver/save-temps.f90

Index: flang/test/Driver/save-temps.f90
===
--- flang/test/Driver/save-temps.f90
+++ flang/test/Driver/save-temps.f90
@@ -53,3 +53,36 @@
 ! NO-TEMPS: "-fc1"
 ! NO-TEMPS-SAME: "-S"
 ! NO-TEMPS-SAME: "-o" "save-temps.s"
+
+!--
+! MLIR generation
+!--
+! RUN: not %flang_fc1 -emit-llvm-bc -save-temps=#invalid-dir -o - %s 2>&1 | FileCheck %s -check-prefix=MLIR-ERROR
+! MLIR-ERROR: error: Saving MLIR temp file failed
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: pushd %t && %flang_fc1 -emit-llvm-bc -save-temps=cwd -o - %s 2>&1
+! RUN: FileCheck %s -input-file=save-temps-fir.mlir -check-prefix=MLIR-FIR
+! RUN: FileCheck %s -input-file=save-temps-llvmir.mlir -check-prefix=MLIR-LLVMIR
+! RUN: popd
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: pushd %t && %flang_fc1 -emit-llvm-bc -save-temps -o - %s 2>&1
+! RUN: FileCheck %s -input-file=save-temps-fir.mlir -check-prefix=MLIR-FIR
+! RUN: FileCheck %s -input-file=save-temps-llvmir.mlir -check-prefix=MLIR-LLVMIR
+! RUN: popd
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -emit-llvm-bc -save-temps=obj -o %t/out.bc %s 2>&1
+! RUN: FileCheck %s -input-file=%t/save-temps-fir.mlir -check-prefix=MLIR-FIR
+! RUN: FileCheck %s -input-file=%t/save-temps-llvmir.mlir -check-prefix=MLIR-LLVMIR
+
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -emit-llvm-bc -save-temps=%t -o - %s 2>&1
+! RUN: FileCheck %s -input-file=%t/save-temps-fir.mlir -check-prefix=MLIR-FIR
+! RUN: FileCheck %s -input-file=%t/save-temps-llvmir.mlir -check-prefix=MLIR-LLVMIR
+
+! MLIR-FIR: func.func @{{.*}}main() {
+! MLIR-LLVMIR: llvm.func @{{.*}}main() {
+
+end program
Index: flang/test/Driver/frontend-forwarding.f90
===
--- flang/test/Driver/frontend-forwarding.f90
+++ flang/test/Driver/frontend-forwarding.f90
@@ -15,7 +15,8 @@
 ! RUN: -fassociative-math \
 ! RUN: -freciprocal-math \
 ! RUN: -fpass-plugin=Bye%pluginext \
-! RUN: -mllvm -print-before-all\
+! RUN: -mllvm -print-before-all \
+! RUN: -save-temps=obj \
 ! RUN: -P \
 ! RUN:   | FileCheck %s
 
@@ -34,3 +35,4 @@
 ! CHECK: "-fconvert=little-endian"
 ! CHECK: "-fpass-plugin=Bye
 ! CHECK: "-mllvm" "-print-before-all"
+! CHECK: "-save-temps=obj"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -170,6 +170,8 @@
 ! HELP-FC1-NEXT: -pic-level   Value for __PIC__
 ! HELP-FC1-NEXT: -plugin  Use the named plugin action instead of the default action (use "help" to list available options)
 ! HELP-FC1-NEXT: -P Disable linemarker output in -E mode
+! HELP-FC1-NEXT: -save-temps=Save intermediate compilation results.
+! HELP-FC1-NEXT: -save-tempsSave intermediate compilation results
 ! HELP-FC1-NEXT: -std=   Language standard to compile for
 ! HELP-FC1-NEXT: -S Only run preprocess and compilation steps
 ! HELP-FC1-NEXT: -target-cpu Target a specific cpu type
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -13,6 +13,7 @@
 #include "flang/Frontend/FrontendActions.h"
 #include "flang/Common/default-kinds.h"
 #include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/CompilerInvocation.h"
 #include "flang/Frontend/FrontendOptions.h"
 #include "flang/Frontend/PreprocessorOptions.h"
 #include "flang/Lower/Bridge.h"
@@ -39,6 +40,7 @@
 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticFrontend.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
@@ -52,10 +54,14 @@
 #include "llvm/Passes/PassPlugin.h"
 #include "llvm/Passes/StandardInstrumentations.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

2023-03-22 Thread Sergio Afonso via Phabricator via cfe-commits 
skatrak added a comment.

Thanks again for helping improving this patch. I made most of the changes 
requested, and tried to clarify a bit better the approach followed for this 
implementation, to see if it is acceptable or if some more fundamental changes 
have to be done.




Comment at: flang/lib/Frontend/FrontendActions.cpp:103
+  std::error_code ec;
+  llvm::ToolOutputFile out(outDir + outFile, ec, llvm::sys::fs::OF_Text);
+  if (ec)

awarzynski wrote:
> Why not just [[ 
> https://github.com/llvm/llvm-project/blob/3b1951aceb8c58acd3d5c5ba2d042ad3d03c13c4/flang/lib/Frontend/FrontendActions.cpp#L793
>  | print ]] the MLIR module?
I'm not sure I completely understand the issue here, by looking at the code you 
linked to. Maybe it's related to the fact that this function is implemented by 
creating new files apart from the main output of the current compiler 
invocation. I developed the idea a bit more in the reply to your comment on 
"save-temps.f90".



Comment at: flang/test/Driver/save-temps.f90:14
 ! CHECK-NEXT: "-o" "save-temps.o"
 ! CHECK-NEXT: "-o" "a.out"
 

awarzynski wrote:
> Why are there no MLIR files here? Same comment for other invocations.
This is because the general way in which -save-temps works is different from 
what's implemented in this patch for MLIR in flang. In the other cases, the 
driver splits the work into several frontend invocations, where each step 
generally produces the input of the next. `-save-temps` makes sure these 
intermediate files are kept where the user specified and not deleted.

In this patch, instead of modifying the driver to create a frontend invocation 
to produce MLIR (generating the *-fir.mlir file), another one to optimize/lower 
that (generating the *-llvmir.mlir file), and a third one to translate lowered 
MLIR into LLVM IR, we just forward the flag to the frontend, which creates 
extra MLIR files at particular spots of the codegen process if the flag is set. 
Hence, MLIR files don't show in the output of `flang-new -###`.



Comment at: flang/test/Driver/save-temps.f90:61
+! RUN: rm -rf %t && mkdir -p %t
+! RUN: %flang_fc1 -emit-llvm-bc -save-temps=obj -o %t/out.bc %s 2>&1
+! RUN: FileCheck %s -input-file=%t/save-temps-FIR.mlir -check-prefix=MLIR-FIR

awarzynski wrote:
> What happens in this case: `-save-temps`? (no dir is specified)
The same as if `-save-temps=cwd` is specified. The test is now updated to check 
all supported options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146075

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-03-22 Thread Alexander Shaposhnikov via Phabricator via cfe-commits 
alexander-shaposhnikov added a comment.

@erichkeane - yes, I'm working on it, I hope to have a new version ~soon 
(within a few days).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

2023-03-22 Thread Gábor Horváth via Phabricator via cfe-commits 
xazax.hun accepted this revision.
xazax.hun added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146507

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-03-22 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

In D146178#4213235 , 
@alexander-shaposhnikov wrote:

> @erichkeane - yes, I'm working on it, I hope to have a new version ~soon 
> (within a few days).

Ok, thanks for the update then!  I'll do just enough looking into it to be able 
to review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-03-22 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:728
  bool SkipForSpecialization = false) {
   MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
   ND, /*Final=*/false, /*Innermost=*/nullptr, /*RelativeToPrimary=*/true,

Note all uses of THIS function are probably wrong based on what Richard says?  
I think even the friend-constraint-depends-on-enclosing-template thing is 
wrong?  Is that right @rsmith ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D145642: [clang-format] Annotate lambdas with requires clauses.

2023-03-22 Thread Utkarsh Saxena via Phabricator via cfe-commits 
usaxena95 added a comment.

> starting a line with an opening paren is pretty weird!)

I do not think this is weird. On the contrary, this is more readable to me and 
separates the requires clause from the parameters list. For example this one 
looks so much better:

  // trunk.
  template 
  void func() {
auto x = []
  requires Foo && Foo(T t, U u) requires BarBar && BarBar ||
   BarBar
{
  return u + t;
};
  }

  // patch.
  template  void func() {
auto x = []
  requires Foo && Foo
(T t, U u)
  requires BarBar && BarBar || BarBar
{ return u + t; };
  }

Discussion on wrapping the lambda body with single statement. FWIW: I do not 
think this is a regression and we are fixing things as seen in my first example.

Another point:
While testing this patch, the following still fails to recognise. Might be 
something special with `true`.

  auto y = [&]
  requires true(Callable && callable)
{ static_cast(callable); };




Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1352
+
+  // Both at once? Probably not even valid.
+  Tokens = annotate("[]  requires Foo (T t) requires Bar 
{}");

This is valid and is accepted by the compilers https://godbolt.org/z/EPbrWbrsv


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145642

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

[PATCH] D144569: [Clang][OpenMP] Fix accessing of aligned arrays in offloaded target regions

2023-03-22 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits 
doru1004 closed this revision.
doru1004 added a comment.

Commit: 65a0d669b4625c34775436a6d3643d15bbc2465a 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144569

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

[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

2023-03-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Similar to D146426 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146634

Files:
  clang/lib/Index/USRGeneration.cpp


Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -262,6 +262,12 @@
 
   // Mangle in type information for the arguments.
   for (auto *PD : D->parameters()) {
+// FIXME: Make sure we don't have nullptrs in parameter lists of function
+// decls.
+if (!PD) {
+  IgnoreResults = true;
+  return;
+}
 Out << '#';
 VisitType(PD->getType());
   }


Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -262,6 +262,12 @@
 
   // Mangle in type information for the arguments.
   for (auto *PD : D->parameters()) {
+// FIXME: Make sure we don't have nullptrs in parameter lists of function
+// decls.
+if (!PD) {
+  IgnoreResults = true;
+  return;
+}
 Out << '#';
 VisitType(PD->getType());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

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

In D146514#4212528 , @mboehme wrote:

> No, I think this is a different case.

Sorry, I might have been unclear. I am 100% agree that these are different 
cases. I was wondering whether we can/should have a single code path supporting 
both. But I do not insist since we do not seem to have precedent for the 
scenario I mentioned and do not want to block anything based on hypotheticals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146514

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

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

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

While I am OK with this approach, I wonder if it would be too much work to 
split the HTML generation and emitting data up. E.g., the logger could emit 
YAML or JSON that could be parsed by a python script to create the HTML (or the 
HTML itself could use JS to parse it and generate the DOM).
This would have a couple of advantages like:

- People could add additional visualizations or other tools like querying 
program states more easily just consuming the JSON/YAML file
- Other static analysis tools could produce similar YAML/JSON so your 
visualization could potentially work for other analysis tools. E.g., I think 
people were working on a dataflow analysis framework for MLIR.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

2023-03-22 Thread Gábor Horváth via Phabricator via cfe-commits 
xazax.hun added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:186
+  Logger &logger() const {
+return DACtx->getOptions().Log ? *DACtx->getOptions().Log : Logger::null();
+  }

If we already have a `NullLogger`, I wonder if making `DACtx->getOptions().Log` 
a reference that points to NullLogger when logging is disabled would be less 
confusing (and fewer branches).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144730

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

[PATCH] D146530: [clang][diagnostics]Removed "sorry" from all the required files

2023-03-22 Thread Priyanshi Agarwal via Phabricator via cfe-commits 
ipriyanshi1708 added a comment.

In D146530#4213078 , @aaron.ballman 
wrote:

> In D146530#4213076 , 
> @ipriyanshi1708 wrote:
>
>> In D146530#4213045 , 
>> @aaron.ballman wrote:
>>
>>> This appears to be the same efforts that have been going on in 
>>> https://reviews.llvm.org/D146041. Can you coordinate with @AryanGodara so 
>>> that there's only one patch for this?
>>
>> Hi! I am an Outreachy applicant, firstly how can I coordinate with 
>> @AryanGodara and secondly how will it be counted as my contribution and then 
>> can I record it in my final application? Or I will have to look for some 
>> other issues for Outreachy?
>
> Welcome! You can comment on the other review to see if the author would like 
> to collaborate with you on a solution. We have the ability to land commits 
> with co-author tags (please request it explicitly when someone approves the 
> patch so that the person landing the work knows to do this), so that should 
> get you both the attribution in GitHub that you did the work (no idea if 
> Outreachy has rules around co-authoring though). You might have to look for 
> other issues for Outreachy, depending on what they require.

Ok thanks! I will surely ask him to collaborate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146530

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

[PATCH] D146041: Fix weirdly apologetic diagnostic messages

2023-03-22 Thread Priyanshi Agarwal via Phabricator via cfe-commits 
ipriyanshi1708 added a comment.

Hi @AryanGodara , I am an Outreachy applicant. I was also working on this issue 
and also created a patch for this issue i.e., https://reviews.llvm.org/D146530 
. Can we collaborate to solve this issue so that only one patch will be there 
for a issue as told by @aaron.ballman .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146041

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

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

2023-03-22 Thread Gábor Horváth via Phabricator via cfe-commits 
xazax.hun added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Logger.cpp:17
+Logger &Logger::null() {
+  struct NullLogger : Logger {};
+  static auto *Instance = new NullLogger();

Adding `final`? Just in case it can help with devirtualization. 



Comment at: clang/lib/Analysis/FlowSensitive/Logger.cpp:23
+namespace {
+struct TextualLogger : Logger {
+  llvm::raw_ostream &OS;

`final`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144730

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

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Adrian Prantl via Phabricator via cfe-commits 
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

So this attribute will lower into a `DW_AT_trampoline("target_func_name")` 
attribute on the `DW_TAG_subprogram` of the function definition?

The debug info parts LGTM. It would be nice to hear some extra confirmation 
that we're fine with this specific attribute name.




Comment at: llvm/include/llvm/IR/DIBuilder.h:802
+DITemplateParameterArray TParams = nullptr,
+DITypeArray ThrownTypes = nullptr, StringRef TargetFuncName = "");
 

Why did you need to add this? Does `flang` use a different createMethod() 
method?
I assume yes, since it the doxygen comment explicitly calls out C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-03-22 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

The user isn't modifying the `float_t` type definition, they're using it.  I 
think the diagnostic should say something like `cannot use type 'float_t' 
within '#pragma clang fp eval_method'; type is set according to the default 
eval method for the translation unit`.

Is there any way we can infer an attribute for these typedefs when they're 
declared, then diagnose it in `DiagnoseUseOfDecl`?  Some sort of 
"available_only_in_default_eval_method" attribute?


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

https://reviews.llvm.org/D146148

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

[PATCH] D145646: [clang][driver] Enable '-flto' on AVR

2023-03-22 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits 
gulfem added a comment.

We started seeing a test failure in `avr-ld.c`.

  /b/s/w/ir/x/w/llvm-llvm-project/clang/test/Driver/avr-ld.c:48:11: error: 
  // LINKP: {{".*ld.*"}} {{.*}} "--defsym=__DATA_REGION_ORIGIN__=0x800100" 
"-plugin" {{.*}} "-plugin-opt=mcpu=atmega328"
^
  :1:1: note: scanning from here
  Fuchsia clang version 17.0.0 (https://llvm.googlesource.com/llvm-project 
0d37efdbc599e61ce2a0418723a66d6b45aea8d7)
  ^
  :6:239: note: possible intended match here

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8785950851645447937/overview


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145646

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

[PATCH] D146338: [MSVC compatibility][dllimport/dllexport][PS] Allow dllexport/dllimport for classes with UniqueExternalLinkage

2023-03-22 Thread Wolfgang Pieb via Phabricator via cfe-commits 
wolfgangp added a comment.

In D146338#4206222 , @hans wrote:

>> In D145271  it was suggested that we drop 
>> the attribute in such contexts, and this is effectively what happens. The 
>> compiler does not produce any exported definitions (or import any symbols) 
>> for such classes. The patch is simply to suppress the diagnostic for MSVC 
>> mode and Playstation.



> With the current patch, we still end up with the attribute on the base class 
> in the AST. Also, does this make the compiler accept dllexport of classes in 
> anonymous namespaces? I'm not sure we want that.
> Is it not possible to check the linkage and bail out in 
> `Sema::propagateDLLAttrToBaseClassTemplate` instead?

Actually, the compiler would (still) not accept explicit dll* attributes on the 
class in either function scopes or anon namespaces. This case is about classes 
with internal linkage deriving from exported/imported template classes that 
have been instantiated with a class with internal linkage. In the description, 
the classes B and C are such classes. They both derive from A, which is 
imported and instantiated with B and C as template parameters.

However, I think I have found a way to drop the attribute. I think it's 
equivalent to just suppressing the error message, but it's probably cleaner.


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

https://reviews.llvm.org/D146338

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

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

2023-03-22 Thread Michael Halkenhäuser via Phabricator via cfe-commits 
mhalk updated this revision to Diff 507409.
mhalk added a comment.

Patch rework, implementing the mentioned changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/SemaOpenMP/hip-omp-mix.cpp

Index: clang/test/SemaOpenMP/hip-omp-mix.cpp
===
--- /dev/null
+++ clang/test/SemaOpenMP/hip-omp-mix.cpp
@@ -0,0 +1,165 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 %s -x c++ -fopenmp -fsyntax-only -verify=host
+// host-no-diagnostics
+
+// RUN: %clang_cc1 %s -x hip -fopenmp -fsyntax-only -verify=device
+// device-error@#01 {{HIP does not support OpenMP target directives}}
+// device-error@#02 {{HIP does not support OpenMP target directives}}
+// device-error@#03 {{HIP does not support OpenMP target directives}}
+// device-error@#04 {{HIP does not support OpenMP target directives}}
+// device-error@#05 {{HIP does not support OpenMP target directives}}
+// device-error@#06 {{HIP does not support OpenMP target directives}}
+// device-error@#07 {{HIP does not support OpenMP target directives}}
+// device-error@#08 {{HIP does not support OpenMP target directives}}
+// device-error@#09 {{HIP does not support OpenMP target directives}}
+// device-error@#10 {{HIP does not support OpenMP target directives}}
+// device-error@#11 {{HIP does not support OpenMP target directives}}
+// device-error@#12 {{HIP does not support OpenMP target directives}}
+// device-error@#13 {{HIP does not support OpenMP target directives}}
+// device-error@#14 {{HIP does not support OpenMP target directives}}
+// device-error@#15 {{HIP does not support OpenMP target directives}}
+// device-error@#16 {{HIP does not support OpenMP target directives}}
+// device-error@#17 {{HIP does not support OpenMP target directives}}
+// device-error@#18 {{HIP does not support OpenMP target directives}}
+// device-error@#19 {{HIP does not support OpenMP target directives}}
+// device-error@#20 {{HIP does not support OpenMP target directives}}
+// device-error@#21 {{HIP does not support OpenMP target directives}}
+// device-error@#22 {{HIP does not support OpenMP target directives}}
+// device-error@#23 {{HIP does not support OpenMP target directives}}
+// device-error@#24 {{HIP does not support OpenMP target directives}}
+
+void test01() {
+#pragma omp target // #01
+  ;
+}
+
+
+void test02() {
+#pragma omp target parallel // #02
+  ;
+}
+
+void test03() {
+#pragma omp target parallel for // #03
+  for (int i = 0; i < 1; ++i);
+}
+
+void test04(int x) {
+#pragma omp target data map(x) // #04
+  ;
+}
+
+void test05(int * x, int n) {
+#pragma omp target enter data map(to:x[:n]) // #05
+}
+
+void test06(int * x, int n) {
+#pragma omp target exit data map(from:x[:n]) // #06
+}
+
+void test07(int * x, int n) {
+#pragma omp target update to(x[:n]) // #07
+}
+
+#pragma omp declare target (test07) // #08
+void test08() {
+
+}
+
+#pragma omp begin declare target // #09
+void test09_1() {
+
+}
+
+void test09_2() {
+
+}
+#pragma omp end declare target
+
+void test10(int n) {
+  #pragma omp target parallel // #10
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test11(int n) {
+  #pragma omp target parallel for // #11
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test12(int n) {
+  #pragma omp target parallel for simd // #12
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test13(int n) {
+  #pragma omp target parallel loop // #13
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test14(int n) {
+  #pragma omp target simd // #14
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test15(int n) {
+  #pragma omp target teams // #15
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test16(int n) {
+  #pragma omp target teams distribute // #16
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test17(int n) {
+  #pragma omp target teams distribute simd // #17
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test18(int n) {
+  #pragma omp target teams loop // #18
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test19(int n) {
+  #pragma omp target teams distribute parallel for // #19
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test20(int n) {
+  #pragma omp target teams distribute parallel for simd // #20
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test21() {
+#pragma omp target // #21
+  {
+#pragma omp teams // #22
+{}
+  }
+}
+
+void test22() {
+#pragma omp target // #23
+#pragma omp teams // #24
+  {}
+}
+
+void test23() {
+// host code
+#pragma omp teams
+  {}
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6119,6 +6119,11 @@
 BindKind, StartLoc))

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
sammccall added a comment.

In D146591#4213319 , @xazax.hun wrote:

> While I am OK with this approach, I wonder if it would be too much work to 
> split the HTML generation and emitting data up. E.g., the logger could emit 
> YAML or JSON that could be parsed by a python script to create the HTML (or 
> the HTML itself could use JS to parse it and generate the DOM).

This is definitely doable. I think the main downside is more JS complexity: 
emitting JSON is easy, turning it into a view means some sort of templating 
(micro)framework, probably.
(I find it important that the output is directly viewable without running extra 
tools, and we're not pulling in big deps. So this probably means some custom JS)

> This would have a couple of advantages like:
>
> - People could add additional visualizations or other tools like querying 
> program states more easily just consuming the JSON/YAML file
> - Other static analysis tools could produce similar YAML/JSON so your 
> visualization could potentially work for other analysis tools. E.g., I think 
> people were working on a dataflow analysis framework for MLIR.

Definitely.
To realize these advantages, the data model needs to be generic enough and 
stable enough that it's actually useful with different producers/consumers.
(e.g. does the MLIR dataflow framework have the same idea about how the 
analysis timeline relates to the CFG and elements within it? And if we want to 
show the clang AST or the StorageLocation/Value graph, what's the 
non-clang::dataflow-coupled way to model that?)

This seems useful but also a pretty large project in itself - I don't think I 
have the experience or bandwidth to take it on. (My primary goal is nullability 
analysis, I need to spend some more time on that directly to understand deeper 
requirements about debugging these things).

One halfway point is to move the data into JSON, move most of the HTML into a 
static file, and use JS to mash them together - without exposing the JSON 
separately or attempting to properly decouple it from the 
generator/visualization, but leaving us closer to doing that later. Do you 
think that's valuable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D146376: Update static_assert message for redundant cases

2023-03-22 Thread Krishna Narayanan via Phabricator via cfe-commits 
Krishna-13-cyber updated this revision to Diff 507413.
Krishna-13-cyber added a comment.

Have worked on the top level with Boolean literals unlike the previous diffs.I 
have updated the test case as well with this new upcoming change.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146376

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/static-assert.cpp


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -259,7 +259,6 @@
 return !b;
   }
   static_assert(invert(true) == invert(false), ""); // expected-error 
{{failed}} \
-// expected-note 
{{evaluates to 'false == true'}}
 
   /// No notes here since we compare a bool expression with a bool literal.
   static_assert(invert(true) == true, ""); // expected-error {{failed}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -16728,6 +16728,10 @@
 if (!UsefulToPrintExpr(LHS) && !UsefulToPrintExpr(RHS))
   return;
 
+// Don't print obvious boolean literals.
+if (LHS->getType()->isBooleanType() && RHS->getType()->isBooleanType())
+  return;
+
 struct {
   const clang::Expr *Cond;
   Expr::EvalResult Result;


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -259,7 +259,6 @@
 return !b;
   }
   static_assert(invert(true) == invert(false), ""); // expected-error {{failed}} \
-// expected-note {{evaluates to 'false == true'}}
 
   /// No notes here since we compare a bool expression with a bool literal.
   static_assert(invert(true) == true, ""); // expected-error {{failed}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -16728,6 +16728,10 @@
 if (!UsefulToPrintExpr(LHS) && !UsefulToPrintExpr(RHS))
   return;
 
+// Don't print obvious boolean literals.
+if (LHS->getType()->isBooleanType() && RHS->getType()->isBooleanType())
+  return;
+
 struct {
   const clang::Expr *Cond;
   Expr::EvalResult Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146155: [clang][NFC] Fix location of 2>&1 in a few -print tests

2023-03-22 Thread Louis Dionne via Phabricator via cfe-commits 
ldionne added a reviewer: aaron.ballman.
ldionne added a comment.

Gentle ping, this was likely a copy-paste error.

Adding Aaron in case he has time to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146155

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

[PATCH] D146376: Update static_assert message for redundant cases

2023-03-22 Thread Christopher Di Bella via Phabricator via cfe-commits 
cjdb added inline comments.



Comment at: clang/test/SemaCXX/static-assert.cpp:261-265
   static_assert(invert(true) == invert(false), ""); // expected-error 
{{failed}} \
-// expected-note 
{{evaluates to 'false == true'}}
 
   /// No notes here since we compare a bool expression with a bool literal.
   static_assert(invert(true) == true, ""); // expected-error {{failed}}

We should also have a test for conjunctions, disjunctions, and negations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146376

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

[PATCH] D146155: [clang][NFC] Fix location of 2>&1 in a few -print tests

2023-03-22 Thread Petr Hosek via Phabricator via cfe-commits 
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146155

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

[PATCH] D146644: [documentation]Fixed Random Typos

2023-03-22 Thread Priyanshi Agarwal via Phabricator via cfe-commits 
ipriyanshi1708 created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
ipriyanshi1708 added reviewers: samtebbs, aaron.ballman.
ipriyanshi1708 published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146644

Files:
  llvm/lib/Support/FileUtilities.cpp
  llvm/test/DebugInfo/macro_link.ll
  llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp


Index: llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
===
--- llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
+++ llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
@@ -7,7 +7,7 @@
 
//===--===//
 //
 // This tool verifies Control Flow Integrity (CFI) instrumentation by static
-// binary anaylsis. See the design document in /docs/CFIVerify.rst for more
+// binary analysis. See the design document in /docs/CFIVerify.rst for more
 // information.
 //
 // This tool is currently incomplete. It currently only does disassembly for
Index: llvm/test/DebugInfo/macro_link.ll
===
--- llvm/test/DebugInfo/macro_link.ll
+++ llvm/test/DebugInfo/macro_link.ll
@@ -1,6 +1,6 @@
 ; RUN: llvm-link %s %s -S -o -| FileCheck %s
 
-; This test checks that DIMacro and DIMacroFile comaprison works correctly.
+; This test checks that DIMacro and DIMacroFile comparison works correctly.
 
 ; CHECK: !llvm.dbg.cu = !{[[CU1:![0-9]*]], [[CU2:![0-9]*]]}
 
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -169,7 +169,7 @@
 
 /// DiffFilesWithTolerance - Compare the two files specified, returning 0 if 
the
 /// files match, 1 if they are different, and 2 if there is a file error.  This
-/// function differs from DiffFiles in that you can specify an absolete and
+/// function differs from DiffFiles in that you can specify an absolute and
 /// relative FP error that is allowed to exist.  If you specify a string to 
fill
 /// in for the error option, it will set the string to an error message if an
 /// error occurs, allowing the caller to distinguish between a failed diff and 
a


Index: llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
===
--- llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
+++ llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
@@ -7,7 +7,7 @@
 //===--===//
 //
 // This tool verifies Control Flow Integrity (CFI) instrumentation by static
-// binary anaylsis. See the design document in /docs/CFIVerify.rst for more
+// binary analysis. See the design document in /docs/CFIVerify.rst for more
 // information.
 //
 // This tool is currently incomplete. It currently only does disassembly for
Index: llvm/test/DebugInfo/macro_link.ll
===
--- llvm/test/DebugInfo/macro_link.ll
+++ llvm/test/DebugInfo/macro_link.ll
@@ -1,6 +1,6 @@
 ; RUN: llvm-link %s %s -S -o -| FileCheck %s
 
-; This test checks that DIMacro and DIMacroFile comaprison works correctly.
+; This test checks that DIMacro and DIMacroFile comparison works correctly.
 
 ; CHECK: !llvm.dbg.cu = !{[[CU1:![0-9]*]], [[CU2:![0-9]*]]}
 
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -169,7 +169,7 @@
 
 /// DiffFilesWithTolerance - Compare the two files specified, returning 0 if the
 /// files match, 1 if they are different, and 2 if there is a file error.  This
-/// function differs from DiffFiles in that you can specify an absolete and
+/// function differs from DiffFiles in that you can specify an absolute and
 /// relative FP error that is allowed to exist.  If you specify a string to fill
 /// in for the error option, it will set the string to an error message if an
 /// error occurs, allowing the caller to distinguish between a failed diff and a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Augusto Noronha via Phabricator via cfe-commits 
augusto2112 added a comment.

In D146595#4213412 , @aprantl wrote:

> So this attribute will lower into a `DW_AT_trampoline("target_func_name")` 
> attribute on the `DW_TAG_subprogram` of the function definition?

Exactly, there's already functionality to lower the `targetFuncName` in a 
`DISubprogram` into dwarf as a `DW_AT_trampoline`. This threads the clang 
attribute down to LLVM IR.




Comment at: llvm/include/llvm/IR/DIBuilder.h:802
+DITemplateParameterArray TParams = nullptr,
+DITypeArray ThrownTypes = nullptr, StringRef TargetFuncName = "");
 

aprantl wrote:
> Why did you need to add this? Does `flang` use a different createMethod() 
> method?
> I assume yes, since it the doxygen comment explicitly calls out C++.
Yes, there's an existing `createFunction` which already takes in a 
`TargetFuncName`, which I assume is all that `flang` uses. I'm extending this 
to `createMethod` so we support this attribute on methods as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

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

In D146591#4213614 , @sammccall wrote:

> (e.g. does the MLIR dataflow framework have the same idea about how the 
> analysis timeline relates to the CFG and elements within it? And if we want 
> to show the clang AST or the StorageLocation/Value graph, what's the 
> non-clang::dataflow-coupled way to model that?)

These are good questions.

In D146591#4213614 , @sammccall wrote:

> Do you think that's valuable?

In case it does not add too much complexity on your side I think it is 
valuable. In case someone wants to use the same visualization for their tool it 
would be on them to figure out how to generalize the visualization part. On the 
other hand, even if we never get new users for the visualization, a JSON file 
will make it easier to build new diagnostic tools for the dataflow framework. 
One could even do one-off python scripts to help debugging a specific issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146591

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

[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

2023-03-22 Thread Jon Phillips via Phabricator via cfe-commits 
jp4a50 added a comment.

In D146101#4208962 , @MyDeveloperDay 
wrote:

> What I'd really like to see, is... in the event that 
> `ContinuationIndentWidth` is set and I do NOTset 
> `DesignatedInitializerIndentWidth`   , then 
> `DesignatedInitializerIndentWidth`  would inherit from that (not the default 
> as you have here),  if I set ContinuationIndentWidthto 3 
> DesignatedInitializerIndentWidth should be 3

Agree that this makes the most sense and, luckily, that is exactly the 
behaviour I implemented in my most recent diff!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

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

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
sammccall updated this revision to Diff 507418.
sammccall marked 3 inline comments as done.
sammccall added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144730

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Logger.h
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/Logger.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -0,0 +1,152 @@
+#include "TestingSupport.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang::dataflow::test {
+namespace {
+
+struct TestLattice {
+  int Elements = 0;
+  int Branches = 0;
+  int Joins = 0;
+
+  LatticeJoinEffect join(const TestLattice &Other) {
+if (Joins < 3) {
+  ++Joins;
+  Elements += Other.Elements;
+  Branches += Other.Branches;
+  return LatticeJoinEffect::Changed;
+}
+return LatticeJoinEffect::Unchanged;
+  }
+  friend bool operator==(const TestLattice &LHS, const TestLattice &RHS) {
+return std::tie(LHS.Elements, LHS.Branches, LHS.Joins) ==
+   std::tie(RHS.Elements, RHS.Branches, RHS.Joins);
+  }
+};
+
+class TestAnalysis : public DataflowAnalysis {
+public:
+  using DataflowAnalysis::DataflowAnalysis;
+
+  static TestLattice initialElement() { return TestLattice{}; }
+  void transfer(const CFGElement &, TestLattice &L, Environment &E) {
+E.logger().log([](llvm::raw_ostream &OS) { OS << "transfer()"; });
+++L.Elements;
+  }
+  void transferBranch(bool Branch, const Stmt *S, TestLattice &L,
+  Environment &E) {
+E.logger().log([&](llvm::raw_ostream &OS) {
+  OS << "transferBranch(" << Branch << ")";
+});
+++L.Branches;
+  }
+};
+
+class TestLogger : public Logger {
+public:
+  TestLogger(std::string &S) : OS(S) {}
+
+private:
+  llvm::raw_string_ostream OS;
+
+  void beginAnalysis(const ControlFlowContext &,
+ TypeErasedDataflowAnalysis &) override {
+logText("beginAnalysis()");
+  }
+  void endAnalysis() override { logText("\nendAnalysis()"); }
+
+  void enterBlock(const CFGBlock &B) override {
+OS << "\nenterBlock(" << B.BlockID << ")\n";
+  }
+  void enterElement(const CFGElement &E) override {
+// we don't want the trailing \n
+std::string S;
+llvm::raw_string_ostream SS(S);
+E.dumpToStream(SS);
+
+OS << "enterElement(" << llvm::StringRef(S).trim() << ")\n";
+  }
+  void recordState(TypeErasedDataflowAnalysisState &S) override {
+const TestLattice &L = llvm::any_cast(S.Lattice.Value);
+OS << "recordState(Elements=" << L.Elements << ", Branches=" << L.Branches
+   << ", Joins=" << L.Joins << ")\n";
+  }
+  /// Records that the analysis state for the current block is now final.
+  void blockConverged() override { logText("blockConverged()"); }
+
+  void logText(llvm::StringRef Text) override { OS << Text << "\n"; }
+};
+
+TEST(LoggerTest, Sequence) {
+  const char *Code = R"cpp(
+int target(bool b, int p, int q) {
+  return b ? p : q;
+}
+)cpp";
+
+  auto Inputs = AnalysisInputs(
+  Code, ast_matchers::hasName("target"),
+  [](ASTContext &C, Environment &) { return TestAnalysis(C); });
+  std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing", "-std=c++17"};
+  Inputs.ASTBuildArgs = Args;
+  std::string Log;
+  TestLogger Logger(Log);
+  Inputs.BuiltinOptions.Log = &Logger;
+
+  ASSERT_THAT_ERROR(checkDataflow(std::move(Inputs),
+[](const AnalysisOutputs &) {}),
+llvm::Succeeded());
+
+  EXPECT_EQ(Log, R"(beginAnalysis()
+
+enterBlock(4)
+recordState(Elements=0, Branches=0, Joins=0)
+enterElement(b)
+transfer()
+recordState(Elements=1, Branches=0, Joins=0)
+enterElement(b (ImplicitCastExpr, LValueToRValue, _Bool))
+transfer()
+recordState(Elements=2, Branches=0, Joins=0)
+
+enterBlock(3)
+transferBranch(0)
+recordState(Elements=2, Branches=1, Joins=0)
+enterElement(q)
+transfer()
+recordState(Elements=3, Branches=1, Joins=0)
+
+enterBlock(2)
+transferBranch(1)
+recordState(Elements=2, Branches=1, Joins=0)
+enterElement(p)
+transfer()
+recordState(Elements=3, B

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

2023-03-22 Thread Sam McCall via Phabricator via cfe-commits 
sammccall added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:186
+  Logger &logger() const {
+return DACtx->getOptions().Log ? *DACtx->getOptions().Log : Logger::null();
+  }

xazax.hun wrote:
> If we already have a `NullLogger`, I wonder if making 
> `DACtx->getOptions().Log` a reference that points to NullLogger when logging 
> is disabled would be less confusing (and fewer branches).
Made the DataAnalysisContext constructor fill in this field with NullLogger if 
there isn't anything else, to avoid the branch. (Added a comment on the field).

It's still a pointer because of the ergonomic problems with ref fields in 
structs like this (can't initialize incrementally), and because we need a value 
for "no programmatic logging requested, feel free to respect the -dataflow-log 
flag" and using Logger::null() as a sentinel seems pretty surprising.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144730

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

[PATCH] D146595: [clang] Add clang trampoline attribute

2023-03-22 Thread Augusto Noronha via Phabricator via cfe-commits 
augusto2112 updated this revision to Diff 507422.
augusto2112 added a comment.
Herald added a subscriber: jdoerfert.

Fix pragma-attribute-supported-attributes-list.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-trampoline-method.cpp
  clang/test/CodeGen/attr-trampoline.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/CodeGen/MachineOutliner.cpp
  llvm/lib/IR/DIBuilder.cpp

Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -881,7 +881,7 @@
 unsigned LineNo, DISubroutineType *Ty, unsigned VIndex, int ThisAdjustment,
 DIType *VTableHolder, DINode::DIFlags Flags,
 DISubprogram::DISPFlags SPFlags, DITemplateParameterArray TParams,
-DITypeArray ThrownTypes) {
+DITypeArray ThrownTypes, StringRef TargetFuncName) {
   assert(getNonCompileUnitScope(Context) &&
  "Methods should have both a Context and a context that isn't "
  "the compile unit.");
@@ -891,7 +891,7 @@
   /*IsDistinct=*/IsDefinition, VMContext, cast(Context), Name,
   LinkageName, F, LineNo, Ty, LineNo, VTableHolder, VIndex, ThisAdjustment,
   Flags, SPFlags, IsDefinition ? CUNode : nullptr, TParams, nullptr,
-  nullptr, ThrownTypes);
+  nullptr, ThrownTypes, nullptr, TargetFuncName);
 
   if (IsDefinition)
 AllSubprograms.push_back(SP);
Index: llvm/lib/CodeGen/MachineOutliner.cpp
===
--- llvm/lib/CodeGen/MachineOutliner.cpp
+++ llvm/lib/CodeGen/MachineOutliner.cpp
@@ -799,7 +799,8 @@
 0, /* Line 0 is reserved for compiler-generated code. */
 DINode::DIFlags::FlagArtificial /* Compiler-generated code. */,
 /* Outlined code is optimized code by definition. */
-DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized);
+DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized, nullptr,
+nullptr, nullptr, nullptr, SP->getTargetFuncName());
 
 // Don't add any new variables to the subprogram.
 DB.finalizeSubprogram(OutlinedSP);
Index: llvm/include/llvm/IR/DIBuilder.h
===
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -790,15 +790,16 @@
 /// \param SPFlags   Additional flags specific to subprograms.
 /// \param TParams   Function template parameters.
 /// \param ThrownTypes   Exception types this function may throw.
-DISubprogram *
-createMethod(DIScope *Scope, StringRef Name, StringRef LinkageName,
- DIFile *File, unsigned LineNo, DISubroutineType *Ty,
- unsigned VTableIndex = 0, int ThisAdjustment = 0,
- DIType *VTableHolder = nullptr,
- DINode::DIFlags Flags = DINode::FlagZero,
- DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagZero,
- DITemplateParameterArray TParams = nullptr,
- DITypeArray ThrownTypes = nullptr);
+/// \param TargetFuncName The name of the target function if this is
+///   a trampoline.
+DISubprogram *createMethod(
+DIScope *Scope, StringRef Name, StringRef LinkageName, DIFile *File,
+unsigned LineNo, DISubroutineType *Ty, unsigned VTableIndex = 0,
+int ThisAdjustment = 0, DIType *VTableHolder = nullptr,
+DINode::DIFlags Flags = DINode::FlagZero,
+DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagZero,
+DITemplateParameterArray TParams = nullptr,
+DITypeArray ThrownTypes = nullptr, StringRef TargetFuncName = "");
 
 /// Create common block entry for a Fortran common block.
 /// \param Scope   Scope of this common block.
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -185,6 +185,7 @@
 // CHECK-NEXT: TargetClones (SubjectMatchRule_function)
 // CHECK-NEXT: TargetVersion (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
+// CHECK-NEXT: Trampoline (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
 // CHECK-NEXT: Uninitialized (SubjectMatchRule_variable_is_local)
 // CHECK-NEXT: UnsafeBufferUsage (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-trampoline.c
===

[clang] 77044a4 - [CMake] Build runtimes for riscv64-unknown-fuchsia

2023-03-22 Thread Petr Hosek via cfe-commits 

Author: Petr Hosek
Date: 2023-03-22T17:30:14Z
New Revision: 77044a47b4dec308e02c796e7951ab1745a7f53c

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

LOG: [CMake] Build runtimes for riscv64-unknown-fuchsia

This is necessary to have a complete RISC-V toolchain for Fuchsia.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index c874d8cacd197..037cb67e82189 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -204,7 +204,7 @@ if(FUCHSIA_SDK)
 set(BUILTINS_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE 
PATH "")
   endforeach()
 
-  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia)
+  foreach(target 
x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
@@ -276,12 +276,12 @@ if(FUCHSIA_SDK)
 
   set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
-  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia" CACHE 
STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia" 
CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
 endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")



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

[PATCH] D146608: [CMake] Build runtimes for riscv64-unknown-fuchsia

2023-03-22 Thread Petr Hosek 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 rG77044a47b4de: [CMake] Build runtimes for 
riscv64-unknown-fuchsia (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146608

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -204,7 +204,7 @@
 set(BUILTINS_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE 
PATH "")
   endforeach()
 
-  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia)
+  foreach(target 
x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
@@ -276,12 +276,12 @@
 
   set(LLVM_RUNTIME_MULTILIBS 
"asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
-  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia" CACHE 
STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia" 
CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS 
"x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE 
STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS 
"aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
 endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -204,7 +204,7 @@
 set(BUILTINS_${target}_CMAKE_SYSROOT ${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
   endforeach()
 
-  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia)
+  foreach(target x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia)
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
 set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
@@ -276,12 +276,12 @@
 
   set(LLVM_RUNTIME_MULTILIBS "asan;noexcept;compat;asan+noexcept;hwasan;hwasan+noexcept" CACHE STRING "")
 
-  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia" CACHE STRING "")
-  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_compat_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_asan+noexcept_TARGETS "x86_64-unknown-fuchsia;aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan_TARGETS "aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
+  set(LLVM_RUNTIME_MULTILIB_hwasan+noexcept_TARGETS "aarch64-unknown-fuchsia;riscv64-unknown-fuchsia" CACHE STRING "")
 endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
_

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

2023-03-22 Thread Sindhu Chittireddy via Phabricator via cfe-commits 
schittir updated this revision to Diff 507425.

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

https://reviews.llvm.org/D146604

Files:
  clang/include/clang/Basic/TargetOptions.h


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus


Index: clang/include/clang/Basic/TargetOptions.h
===
--- clang/include/clang/Basic/TargetOptions.h
+++ clang/include/clang/Basic/TargetOptions.h
@@ -45,7 +45,7 @@
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
@@ -88,7 +88,7 @@
 COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_4;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146042: [clang-format] Fix numerous issues with "LambdaBodyIndentation: OuterScope" option

2023-03-22 Thread Jon Phillips via Phabricator via cfe-commits 
jp4a50 added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:21918-21927
+  verifyFormat("Result doSomething(Promise promise) {\n"
+   "  return promise.then(\n"
+   "  [this, obj = std::move(s)](int evaluated) mutable {\n"
+   "return someObject.startAsyncAction().then(\n"
+   "[this, &obj](Result result) mutable {\n"
+   "  result.processMore();\n"
+   "});\n"

MyDeveloperDay wrote:
> This is a completely different function what happen to the std::vector 
> argument? 
The changes to shorten various types and variable names was to avoid lines (in 
the code sample) being broken onto multiple newlines where they weren't before 
because I changed the column limit from 100 to 60 (so leaving the code sample 
changed would have led to additional line breaks).

I changed the column limit from 100 to 60 purely because I thought the 100 
column limit made the tests unnecessarily verbose - there were lots of 
characters to read for seemingly no benefit.



Comment at: clang/unittests/Format/FormatTest.cpp:21916
 
   // Lambdas with different indentation styles.
+  Style = getLLVMStyleWithColumns(60);

MyDeveloperDay wrote:
> jp4a50 wrote:
> > MyDeveloperDay wrote:
> > > Why are you changing existing tests
> > Sorry about the rather large diff here.
> > 
> > I have made the following changes to existing tests (all for what I think 
> > are good reasons):
> > 
> >   - Reduced the column limit and then reduced the verbosity of the code 
> > samples in the test to make them more readable. Before the column limit was 
> > 100 and the code samples were very long. I found this made them 
> > unnecessarily difficult to read, especially because some of the lines of 
> > the code sample would wrap before a newline in the code itself, which was 
> > confusing.
> >   - Changed a number of tests written in the form `EXPECT_EQ(code, 
> > format(otherCode))` to simply `verifyFormat(code)` because the latter is 
> > much shorter and easier to read. If you can think of a good reason to 
> > favour the former over the latter then let me know but it just seemed like 
> > unnecessary duplication to me.
> >   - Some of the tests weren't syntactically valid C++ e.g. the tests at 
> > line 21939 which I have changed to have a valid (rather than incomplete) 
> > trailing return type.
> >   - The macro test actually captured a bug so I had to change the code 
> > sample there to reflect the now fixed behaviour.
> > 
> > I also added one or two more tests at the bottom to capture more complex 
> > cases like when more than one argument to a function call is a lambda. 
> > 
> > 
> refactoring the tests is one thing, but not at the same time you are 
> modifying how they are formatted. Could it be a separate commit so we can 
> actually see what is changing?
> 
> I don't deny what is there before doesn't look 100% correct, but I've always 
> worked on the Beyoncé rule  (If you liked it you should have put a test on 
> it), meaning... someone put a test there to assert some form of formatting 
> behaviour, you are now changing that to be your desired behaviour, that break 
> the rule (IMHO)
> 
> we have to agree what was there before wasn't correct, but your changes don't 
> give me that confidence, 
> refactoring the tests is one thing, but not at the same time you are 
> modifying how they are formatted. Could it be a separate commit so we can 
> actually see what is changing?

Sure! I assume that this implies two separate Phabricator reviews/diffs? Or is 
there somehow a way to show two "commits" within a single review? Sorry, I'm 
not used to using Phrabricator.

> I don't deny what is there before doesn't look 100% correct, but I've always 
> worked on the Beyoncé rule  (If you liked it you should have put a test on 
> it), meaning... someone put a test there to assert some form of formatting 
> behaviour, you are now changing that to be your desired behaviour, that break 
> the rule (IMHO)

The macro example I was referring to as a "bug" actually had a comment below it 
which seemed to acknowledge an issue with the previous implementation. Now I'm 
not 100% sure that that comment was referring to the behaviour in the macro 
example but what does seem clear is that the behaviour shown in the macro test 
case is not desirable in the case of using `OuterScope`. I just can't see how 
that behaviour could be justified as being intentional/desirable. If you 
wanted, I could reach out to the original author of that test and ask them 
since they still work at the same company.

> we have to agree what was there before wasn't correct, but your changes don't 
> give me that confidence, 

I'd appreciate if you could elaborate on this - perhaps you're referring mainly 
to the macro example but I'm not sure.


Repository:
  rG LLVM Github Monorepo

[PATCH] D72940: Add a support for clang tidy to import another configurations files from .clang-tidy

2023-03-22 Thread Richard via Phabricator via cfe-commits 
LegalizeAdulthood resigned from this revision.
LegalizeAdulthood added a comment.
This revision now requires review to proceed.
Herald added a reviewer: njames93.
Herald added a subscriber: PiotrZSL.
Herald added a project: All.

No action taken, removing myself as a reviewer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72940

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

[PATCH] D130630: [clang-tidy] Add readability-nested-ifs check

2023-03-22 Thread Richard via Phabricator via cfe-commits 
LegalizeAdulthood added a comment.
Herald added a subscriber: PiotrZSL.

How does this check interact with macros that expand to `if` statements?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130630

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2023-03-22 Thread Richard via Phabricator via cfe-commits 
LegalizeAdulthood added a comment.
Herald added a subscriber: PiotrZSL.

I'm not certain that the result of the transformation is more "readable"; is 
this check intended to aid conformance to a style guide?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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

  1   2   3   >