[Lldb-commits] [PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

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/D144509/new/ https://reviews.llvm.org/D144509 ___

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

phosek added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:17 + + set(oneValueArgs PREFIX SUBDIR) + cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN}) This variable is only used once on the following line, I'd just inline

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137724/new/ https://reviews.llvm.org/D137724 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137503/new/ https://reviews.llvm.org/D137503 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

phosek added a comment. In D128465#3826150 , @mgorny wrote: > Is anyone working on a solution that would work for people using a zstd > install method that's actually supported upstream? I have created D134990 , it'd be

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

phosek added a comment. In D128465#3800479 , @andrewrk wrote: > if(LLVM_ENABLE_ZSTD) > list(APPEND imported_libs zstd::libzstd_shared) > endif() > > This hard codes shared linking which breaks the use case of static linking > LLVM. This was addr

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

phosek added a comment. I agree with @tianshilei1992, I think we should avoid introducing new `CMAKE_` variables to avoid confusion. The same applies to module names, for example I don't think we should be introducing `GNUBinaryDirs` which can be easily confused for `GNUInstallDirs`. I would p

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

phosek added inline comments. Comment at: llvm/cmake/config-ix.cmake:149 + elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*") +find_package(zstd) + endif() Since there's no `Findzstd.cmake` module shipped with CMake, and we don't provide in LLVM, we should

[Lldb-commits] [PATCH] D133525: fix extra bytes when compressing for 32bit objcopy

phosek added a comment. Would it be possible to write a test that covers this case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133525/new/ https://reviews.llvm.org/D133525 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D131919: Move googletest to the third-party directory

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131919/new/ https://reviews.llvm.org/D131919 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

phosek added inline comments. Comment at: llvm/lib/Support/Compression.cpp:63 __msan_unpoison(CompressedBuffer.data(), CompressedSize); - CompressedBuffer.truncate(CompressedSize); + if (CompressedSize < CompressedBuffer.size()) +CompressedBuffer.truncate(CompressedSize)

[Lldb-commits] [PATCH] D128465: [llvm] cmake config groundwork to have ZSTD in LLVM

phosek added inline comments. Comment at: llvm/cmake/modules/FindZSTD.cmake:1 +# Copyright (c) Meta Platforms, Inc. and affiliates. +# MaskRay wrote: > How did you derive this? > > The file seems contributed by you (I don't think facebook/zstd has such a > file

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

phosek added a comment. I think this patch should be broken into at least two: 1. Refactor `llvm/include/llvm/Support/Compression.h` and `llvm/lib/Support/Compression.cpp` to introduce a generic interface and use it throughout the codebase. 2. zstd support in `llvm/include/llvm/Support/Compress

[Lldb-commits] [PATCH] D124673: [llvm][lldb] use FindLibEdit.cmake everywhere

phosek added a comment. In D124673#3512070 , @MaskRay wrote: > In D124673#3512037 , @paulkirth > wrote: > >> Hi, Sorry for the late notification, but I think this change may not apply >> correctly to all configs

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0c0f6cfb7b9f: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

phosek added a comment. Provide fallback for `TARGET_TRIPLE` and print a warning about deprecation and future removal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D119918

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

phosek updated this revision to Diff 409394. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D119918 Files: bolt/test/Unit/lit.site.cfg.py.in bolt/test/lit.site.cfg.py.in clang-tools-extra/clangd/test/lit.sit

[Lldb-commits] [PATCH] D119918: [CMake] Rename TARGET_TRIPLE to LLVM_TARGET_TRIPLE

phosek created this revision. phosek added reviewers: beanz, smeenai, ldionne, compnerd. Herald added subscribers: ayermolo, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, ar

[Lldb-commits] [PATCH] D117639: [cmake] Make include(GNUInstallDirs) always below project(..)

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117639/new/ https://reviews.llvm.org/D117639 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

phosek added inline comments. Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66 install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}" -DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir} +DESTINATION ${LIBCXX_INSTALL_HEA

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

phosek added inline comments. Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66 install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}" -DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir} +DESTINATION ${LIBCXX_INSTALL_HEA

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

phosek added inline comments. Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389 get_compiler_rt_target(${arch} target) -set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE) +set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/${CMAKE_I

[Lldb-commits] [PATCH] D94374: [CMake] Remove dead code setting policies to NEW

phosek accepted this revision. phosek added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94374/new/ https://reviews.llvm.org/D94374 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D85820: Use find_library for ncurses

This revision was automatically updated to reflect the committed changes. Closed by commit rG3c7bfbd6831b: [CMake] Use find_library for ncurses (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85820/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D85820: Use find_library for ncurses

phosek accepted this revision. phosek added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85820/new/ https://reviews.llvm.org/D85820 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mail

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

phosek added a comment. I have suggested an alternative in https://reviews.llvm.org/D85820#2237357 to try and fix this forward and I was waiting for @haampie's response before landing this. I can implement that today if you're OK fixing this forward? I think it'd be easier to do that than rever

[Lldb-commits] [PATCH] D85820: Use find_library for ncurses

phosek added a comment. In D85820#2235461 , @haampie wrote: > @gkistanova It's true that this change has lead to more issues I could ever > imagine, but I think the link you provided is the last remaining problem. > > Pinging @phosek for a similar issue w

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2231927 , @mati865 wrote: > @phosek in MSYS2 (targeting x86_64-w64-windows-gnu) Zlib works properly for > LLVM 10 but with master I'm now seeing: > > -- Constructing LLVMBuild project information > -- DEBUG zlib_libra

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG31e5f7120bdd: [CMake] Simplify CMake handling for zlib (authored by phosek). Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2203887 , @lxfind wrote: > In D79219#2201415 , @phosek wrote: > >> This is correct. That target is provided by `find_package(ZLIB)`. In >> LLVMConfig.cmake, we invoke `find_package

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. I have finally managed to reproduce the issue on my Windows machine, the latest version of the patch should address the issue. The previous logic would fail to find zlib altogether on Windows, which is why this issue haven't manifested before. `find_package` locates the

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2201994 , @kuhnel wrote: > This patch broke the Windows compilation on buildbot > and pre-merge > testing

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2201109 , @lxfind wrote: > @phosek, Under this change, now when I build LLVM (with a basic config `cmake > -G Ninja --LLVM_ENABLE_PROJECTS=clang ../llvm`), in file > `build_dir/lib/cmake/llvm/LLVMExports.cmake`, I see th

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was automatically updated to reflect the committed changes. Closed by commit rG1adc494bce44: [CMake] Simplify CMake handling for zlib (authored by phosek). Changed prior to commit: https://reviews.llvm.org/D79219?vs=283398&id=283434#toc Repository: rG LLVM Github Monorepo CHAN

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 284106. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/test/CMakeLists.txt lld/test/lit.si

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 284161. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/test/CMakeLists.txt lld/test/lit.si

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 283398. phosek marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. @kuhnel do you want to take a look or is it okay if I just submit the change? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 ___ lldb-commits mailing list lldb-commits@lists.ll

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. How can I test this change on pre-merge bots? I haven't seen any builds posted on this change before. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 ___

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGccbc1485b55f: [CMake] Simplify CMake handling for zlib (authored by phosek). Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek reopened this revision. phosek added a comment. This revision is now accepted and ready to land. Sorry about the breakage, that was an unintentional change. I have updated the patch and restored the original behavior on Windows. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

This revision was automatically updated to reflect the committed changes. Closed by commit rG64d99cc6abed: [CMake] Move find_package(ZLIB) to LLVMConfig (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84691/new/ https://reviews.l

[Lldb-commits] [PATCH] D84691: [CMake] Move find_package(ZLIB) to LLVMConfig

phosek created this revision. phosek added reviewers: smeenai, compnerd, mehdi_amini. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antia

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek marked 4 inline comments as done. phosek added inline comments. Comment at: mlir/examples/standalone/CMakeLists.txt:35 +endif() + include(TableGen) stephenneuendorffer wrote: > mehdi_amini wrote: > > mehdi_amini wrote: > > > I am a bit unsure that it is d

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was automatically updated to reflect the committed changes. Closed by commit rG1d09ecf36175: [CMake] Simplify CMake handling for zlib (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 280305. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/CMakeLists.txt

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was automatically updated to reflect the committed changes. Closed by commit rG10b1b4a231a4: [CMake] Simplify CMake handling for zlib (authored by phosek). Herald added subscribers: msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mge

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2152747 , @labath wrote: > I wouldn't mind separate (internal) cache variable, though I am somewhat > surprised by this problem. A (non-cache) variable set in one of the parent > scopes should still take precedence over

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

This revision was automatically updated to reflect the committed changes. Closed by commit rG8c1a79dc12f3: [CMake] Simplify CMake handling for zlib (authored by phosek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.o

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek reopened this revision. phosek added a comment. This revision is now accepted and ready to land. I had to revert this change because it broke bots that don't have zlib installed. What I haven't realized is that the shadowed variable will only be accessible from the same file. I could move

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 278216. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/CMakeLists.txt

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek marked an inline comment as done. phosek added inline comments. Comment at: llvm/cmake/config-ix.cmake:514 + if(ZLIB_FOUND) +set(LLVM_ENABLE_ZLIB "YES" CACHE STRING + "Use zlib for compression/decompression if available. Can be ON, OFF, or FORCE_ON"

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 274256. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/CMakeLists.txt

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 274255. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/CMakeLists.txt

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 263288. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.common.configured.in lld/CMakeLists.txt

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. In D79219#2029520 , @JDevlieghere wrote: > I'm in favor of this change. I'm not too happy with how this works in CMake, > I've expressed similar concerns when the FORCE_ON approach was suggested in > D71306

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek updated this revision to Diff 263284. phosek marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 Files: clang/CMakeLists.txt clang/test/CMakeLists.txt clang/test/lit.site.cfg.py.in compiler-rt/test/lit.

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79219/new/ https://reviews.llvm.org/D79219 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/c

[Lldb-commits] [PATCH] D79219: [CMake] Simplify CMake handling for zlib

phosek created this revision. phosek added reviewers: smeenai, compnerd. Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, hiraditya, mgorny. Herald added projects: clang, Sanitizers, LLDB, LLVM. Rather than handling zlib handling manually, use `find_package` from CMa

[Lldb-commits] [PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

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/D58791/new/ https://reviews.llvm.org/D58791 ___

[Lldb-commits] [PATCH] D64774: [DebugInfo] Move function from line table to the prologue (NFC)

phosek added a comment. This change broke lld: /b/s/w/ir/k/llvm-project/lld/ELF/InputFiles.cpp:323:28: error: conversion function from 'nullptr_t' to 'llvm::StringRef' invokes a deleted function it->second.file, nullptr, ^~~ /b/s/w/ir/k/llvm-proje

[Lldb-commits] [PATCH] D41099: [lldb] Switch to add_llvm_install_targets

phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D41099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lld