date:20230527

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

2023-05-27 Thread Mark de Wever via Phabricator via lldb-commits

Mordante accepted this revision.
Mordante marked 3 inline comments as done.
Mordante added a comment.

In D151344#4368926 , @MaskRay wrote:

> Thank you for making another try for the treewide change (which is admittedly 
> very painful and not many people do such work).

Thanks. And it indeed takes quite a lot of effort.

> Can you include the original patch description to the summary? That is the 
> main part and the message "This reverts commit " is secondary. Readers should 
> not need to trace through the revert history to obtain the original 
> motivation.

Good suggestion I've updated the message.

Since @glandium and @hans seem happy with the changes I'll land this patch.




Comment at: libunwind/src/CMakeLists.txt:28-35
 
 # See add_asm_sources() in compiler-rt for explanation of this workaround.
 # CMake doesn't work correctly with assembly on AIX. Workaround by compiling
 # as C files as well.
 if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR
-   (MINGW AND CMAKE_VERSION VERSION_LESS 3.17) OR
-   (${CMAKE_SYSTEM_NAME} MATCHES "AIX"))
+   (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
   set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)

h-vetinari wrote:
> mstorsjo wrote:
> > h-vetinari wrote:
> > > Shouldn't it be possible to remove that entire block (as it only fires 
> > > for CMake <3.20)?
> > The intention was to do such cleanups in a later patch, as mentioned in the 
> > original commit message in https://reviews.llvm.org/D144509. (I guess it 
> > would be good to bring the original commit message along with the reland 
> > attempts too.)
> Well sure, but "workarounds" is a broad term. It makes sense to not try to 
> clean up everything that newer CMake enables, but this PR does remove a lot 
> of (all?) other lines with `CMAKE_VERSION VERSION_LESS ` where x<3.20. On 
> top of that, since this line is already being changed (which is why I noticed 
> in the first place), I'd suggest to just remove it. But I don't feel strongly 
> about this, just thought I'd point it out.
Yes the intention is to do these cleanups later. The patch does more than I'd 
like already. However that is due to the original patch and followups being 
reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

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

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

2023-05-27 Thread Mark de Wever via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rGcbaa3597aaf6: Reland "[CMake] Bumps minimum version to 
3.20.0. (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build-py/tests/functional/exec/CMakeLists.txt
  cmake/Modules/CMakePolicy.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/crt/CMakeLists.txt
  flang/CMakeLists.txt
  flang/lib/Decimal/CMakeLists.txt
  flang/runtime/CMakeLists.txt
  libc/CMakeLists.txt
  libc/examples/hello_world/CMakeLists.txt
  libclc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm-libgcc/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/CMake.rst
  llvm/docs/GettingStarted.rst
  llvm/docs/ReleaseNotes.rst
  mlir/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/cmake/DetectTestCompiler/CMakeLists.txt
  openmp/docs/SupportAndFAQ.rst
  openmp/libompd/src/CMakeLists.txt
  openmp/libomptarget/plugins/remote/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/Modules/FindOpenMPTarget.cmake
  openmp/tools/Modules/README.rst
  polly/CMakeLists.txt
  pstl/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -1,16 +1,13 @@
 # This file handles building LLVM runtime sub-projects.
-cmake_minimum_required(VERSION 3.13.4)
-if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-  message(WARNING
-"Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-"minimum version of CMake required to build LLVM will become 3.20.0, and "
-"using an older CMake will become an error. Please upgrade your CMake to "
-"at least 3.20.0 now to avoid issues in the future!")
-endif()
-project(Runtimes C CXX ASM)
+cmake_minimum_required(VERSION 3.20.0)
 
 # Add path for custom and the LLVM build's modules to the CMake module path.
 set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
+include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
+  NO_POLICY_SCOPE)
+
+project(Runtimes C CXX ASM)
+
 list(INSERT CMAKE_MODULE_PATH 0
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===--===##
-cmake_minimum_required(VERSION 3.13.4)
+cmake_minimum_required(VERSION 3.20.0)
 
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,14 +1,7 @@
 # Check if this is a in tree build.
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
-  cmake_minimum_required(VERSION 3.13.4)
-  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
-message(WARNING
-  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
-  "minimum version of CMake required to build LLVM will become 3.20.0, and "
-  "using an older CMake will become an error. Please upgrade your CMake to "
-  "at least 3.20.0 now to avoid issues in the future!")
-  endif()
+  cmake_minimum_required(VERSION 3.20.0)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
Index: openmp/tools/Modules/README.rst
===
--- openmp/tools/Modules/README.rst
+++ openmp/tools/Modules/README.rst
@@ -26,7 +26,7 @@
 
 .. code-block:: cmake
 
-  cmake_minimum_required(VERSION 3.13.4)
+  cmake_minimum_required(VERSION 3.20.0)
   project(offloadTest VERSION 1.0 LANGUAGES CXX)
 
   list(APPEND CMAKE_MODULE_PATH "${PATH_TO_OPENMP_INSTALL}/lib/cmake/openmp")
@@ -37,7 +37,7 @@
   target_link_libraries(offload PRIVATE OpenMPTarget::OpenMPTarget_NVPTX)
   target_sources(offload PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/Main.cpp)
 
-Using this module requires at least CMake version 3.13.4. Supported languages
+Using this module requires at least CMake version 3.20.0. Supported languages
 are C and C++ with Fortran support planned in the future. If your application
 requires building for a specif

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

2023-05-27 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp:1001
 
-bool ObjCLanguage::GetFormatterPrefixSuffix(ValueObject &valobj,
-ConstString type_hint,
-std::string &prefix,
-std::string &suffix) {
-  static ConstString g_CFBag("CFBag");
-  static ConstString g_CFBinaryHeap("CFBinaryHeap");
-
-  static ConstString g_NSNumberChar("NSNumber:char");
-  static ConstString g_NSNumberShort("NSNumber:short");
-  static ConstString g_NSNumberInt("NSNumber:int");
-  static ConstString g_NSNumberLong("NSNumber:long");
-  static ConstString g_NSNumberInt128("NSNumber:int128_t");
-  static ConstString g_NSNumberFloat("NSNumber:float");
-  static ConstString g_NSNumberDouble("NSNumber:double");
-
-  static ConstString g_NSData("NSData");
-  static ConstString g_NSArray("NSArray");
-  static ConstString g_NSString("NSString");
-  static ConstString g_NSStringStar("NSString*");
-
-  if (type_hint.IsEmpty())
-return false;
-
-  prefix.clear();
-  suffix.clear();
-
-  if (type_hint == g_CFBag || type_hint == g_CFBinaryHeap) {
-prefix = "@";
-return true;
-  }
-
-  if (type_hint == g_NSNumberChar) {
-prefix = "(char)";
-return true;
-  }
-  if (type_hint == g_NSNumberShort) {
-prefix = "(short)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt) {
-prefix = "(int)";
-return true;
-  }
-  if (type_hint == g_NSNumberLong) {
-prefix = "(long)";
-return true;
-  }
-  if (type_hint == g_NSNumberInt128) {
-prefix = "(int128_t)";
-return true;
-  }
-  if (type_hint == g_NSNumberFloat) {
-prefix = "(float)";
-return true;
-  }
-  if (type_hint == g_NSNumberDouble) {
-prefix = "(double)";
-return true;
-  }
-
-  if (type_hint == g_NSData || type_hint == g_NSArray) {
-prefix = "@\"";
-suffix = "\"";
-return true;
-  }
-
-  if (type_hint == g_NSString || type_hint == g_NSStringStar) {
-prefix = "@";
-return true;
-  }
+std::pair
+ObjCLanguage::GetFormatterPrefixSuffix(llvm::StringRef type_hint) {

We can make this whole map const and remove the explicit call_once by folding 
the `insert` calls into the ctor:

```
static constexpr llvm::StringRef empty;
static const llvm::StringMap<
std::pair>
g_affix_map = {{"CFBag", std::make_pair("@", empty)},
   {"CFBinaryHeap", std::make_pair("@", empty)},
   ..., };
```

If you're so inclined, you can even get rid of the final make_pair calls:

```
static const llvm::StringMap<
std::pair>
g_affix_map = {{"CFBag", {"@", empty}},
   {"CFBinaryHeap", {"@", empty}}};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151603

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

[Lldb-commits] [PATCH] D151615: [lldb] Change GetChildMemberWithName to take a StringRef, not ConstString

2023-05-27 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added reviewers: bulbazord, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

`GetChildMemberWithName` does not need a `ConstString`. This change makes the 
function
take a `StringRef` instead, which alleviates the need for callers to construct a
`ConstString`. I don't expect this change to improve performance, only 
ergonomics.

This is in support of Alex's effort to replace `ConstString` where appropriate.

There are related `ValueObject` functions that can also be changed, if this is 
accepted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151615

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -514,7 +514,7 @@
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
 lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
-  exception_sp->GetChildMemberWithName(ConstString("reserved"), true);
+  exception_sp->GetChildMemberWithName("reserved", true);
   if (!reserved_dict)
 return FailExceptionParsing("Failed to get 'reserved' member.");
 
@@ -567,18 +567,15 @@
 
   if (!return_addresses)
 return FailExceptionParsing("Failed to get return addresses.");
-  auto frames_value =
-  return_addresses->GetChildMemberWithName(ConstString("_frames"), true);
+  auto frames_value = return_addresses->GetChildMemberWithName("_frames", true);
   if (!frames_value)
 return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
-  auto count_value =
-  return_addresses->GetChildMemberWithName(ConstString("_cnt"), true);
+  auto count_value = return_addresses->GetChildMemberWithName("_cnt", true);
   if (!count_value)
 return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
-  auto ignore_value =
-  return_addresses->GetChildMemberWithName(ConstString("_ignore"), true);
+  auto ignore_value = return_addresses->GetChildMemberWithName("_ignore", true);
   if (!ignore_value)
 return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -138,12 +138,11 @@
   //we will obtain the name from this pointer.
   // 5) a free function. A pointer to the function will stored after the vtable
   //we will obtain the name from this pointer.
-  ValueObjectSP member_f_(
-  valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
+  ValueObjectSP member_f_(valobj_sp->GetChildMemberWithName("__f_", true));
 
   if (member_f_) {
 ValueObjectSP sub_member_f_(
-   member_f_->GetChildMemberWithName(ConstString("__f_"), true));
+member_f_->GetChildMemberWithName("__f_", true));
 
 if (sub_member_f_)
 member_f_ = sub_member_f_;
Index: lldb/source/Plugins/Language/CPlusPlus/LibS

[Lldb-commits] [PATCH] D151615: [lldb] Change GetChildMemberWithName to take a StringRef, not ConstString

2023-05-27 Thread Alex Langford via Phabricator via lldb-commits

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

After reading though the code and thinking about it, I believe this change is 
appropriate. It may or may not have a measurable impact on performance, but my 
bet is that it doesn't make it worse. I suppose you could stress test the C++ 
formatters if you really wanted to know though. Either way, thank you for doing 
this work!

We should probably change `CompilerType::GetIndexOfChildMemberWithName` to take 
a `StringRef` instead of a `const char *` as a follow-up. You're having to call 
`str()` and then getting the `const char *` out of that to correctly call the 
function which is the right thing to do but is also a little wasteful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151615

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

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

[Lldb-commits] [PATCH] D151603: [lldb][NFCI] Refactor Language::GetFormatterPrefixSuffix

[Lldb-commits] [PATCH] D151615: [lldb] Change GetChildMemberWithName to take a StringRef, not ConstString

[Lldb-commits] [PATCH] D151615: [lldb] Change GetChildMemberWithName to take a StringRef, not ConstString

5 matches

Site Navigation

Mail list logo

Footer information