[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 marked an inline comment as done. Ericson2314 added inline comments. Comment at: clang/CMakeLists.txt:100 set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin) - set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLV

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 updated this revision to Diff 448459. Ericson2314 added a comment. Rebase so libcxx and friends are hopefully fixed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 Files: bolt/runtime/CMakeL

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 updated this revision to Diff 448458. Ericson2314 added a comment. `WARNING` -> `DEPRECATION` in `message` call Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 Files: bolt/runtime/CMakeLists

[Lldb-commits] [PATCH] D130822: Fixed loads of typos

clementval requested changes to this revision. clementval added inline comments. This revision now requires changes to proceed. Comment at: flang/docs/Extensions.md:157 with each other. -* Values for whole anonymous parent components in structure constructors (e.g., `EXTEND

[Lldb-commits] [PATCH] D130822: Fixed loads of typos

GabrielRavier added a comment. Well, I assumed if I split it up per project I would have been told to merge all the patches into a big one to avoid having a bunch of small ones essentially all doing the same thing, but then again this also makes sense, I'll split it up. Comm

[Lldb-commits] [PATCH] D130822: Fixed loads of typos

philnik added a comment. Could you split this up per project? Large patches like this are really hard to review, since there is no single person/small group that can approve everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130822/new/ htt

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

sebastian-ne accepted this revision. sebastian-ne added a comment. Looks good to me, thanks! Comment at: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h:70 /* Multilib suffix for libdir. */ +#define CLANG_INSTALL_LIBDIR_BASENAME "lib" T

[Lldb-commits] [PATCH] D131928: [libcxx][spaceship][doc] Repair links and clean up spaceship progress doc

mumbleskates updated this revision to Diff 453183. mumbleskates added a comment. Herald added subscribers: cfe-commits, llvm-commits, libc-commits, openmp-commits, lldb-commits, Sanitizers, anlunx, mtrofin, Enna1, bzcheeseman, kosarev, pmatos, asb, pcwang-thead, arjunp, sdasgup3, luke957, carlosg

[Lldb-commits] [PATCH] D131928: [libcxx][spaceship][doc] Repair links and clean up spaceship progress doc

mumbleskates updated this revision to Diff 453184. mumbleskates added a comment. Herald added subscribers: Michael137, yota9, ayermolo, StephenFan, sstefan1, JDevlieghere, kbarton. Herald added a reviewer: JDevlieghere. Herald added a reviewer: jdoerfert. Herald added a reviewer: aaron.ballman. He

[Lldb-commits] [PATCH] D131928: [libcxx][spaceship][doc] Repair links and clean up spaceship progress doc

mumbleskates updated this revision to Diff 453186. mumbleskates added a comment. - Merge branch 'main' into doc2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131928/new/ https://reviews.llvm.org/D131928 Files: libcxx/docs/Status/SpaceshipProjec

[Lldb-commits] [PATCH] D131928: [libcxx][spaceship][doc] Repair links and clean up spaceship progress doc

mumbleskates added a comment. Sorry! Something has gone horribly wrong with my usage of arcanist and it diffed against a very old version of `main`. i will try to repair the reviewers list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131928/new/

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 added a comment. Anyone have any idea what this Debian test failure is about? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 ___ lldb-commits maili

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

sebastian-ne added a comment. In D130586#3731021 , @Ericson2314 wrote: > Anyone have any idea what this Debian test failure is about? I haven’t seen a passing debian pre-merge check for months now, so I usually ignore it (the failures seems to be relat

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 updated this revision to Diff 453328. Ericson2314 marked an inline comment as done. Ericson2314 added a comment. Rebase, fix outdated comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 Fi

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 added a comment. OK I was worried the debuginfod one was unique to this PR; glad to hear it isn't. I'll give it a shot then, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 __

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

nathanchance added a comment. I can no longer do a two stage build on Fedora after this change. $ cmake \ -B build/stage1 \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_C_COMPILER=$(command -v clang) \ -DCMAKE_CXX_COMPILER=$(command -v clang++) \ -DLLVM_ENABLE_PROJECTS="clang;lld" \ -DLLVM_

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

craig.topper added a comment. I seem to be unable to pass `check-clang` after this. A lot of tests fail because they can't find headers they need. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf7a33090a910: [cmake] Use `CMAKE_INSTALL_LIBDIR` too (authored by

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

smeenai added a comment. This is breaking our build setup. It causes e.g. the Clang resource headers to be installed to `lib64/clang` instead of `lib/clang`. Given this and the other breakages, can we revert for now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 added a comment. Sorry. I have reverted this. I see why the `lib` and `lib64` mixup could be caused by this, but I am baffled how the missing headers could be. Headers search paths should be unaffected? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

thakis added a comment. When this relands and it's not too much trouble, it'd be nice if you could reland d3a1dbc4907b59690f9013cdb6221573ca4233f1 in the commit that relands this. Repository: rG LLVM Github Monorepo CHAN

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

tstellar added a comment. In D130586#3734012 , @Ericson2314 wrote: > Sorry. I have reverted this. I see why the `lib` and `lib64` mixup could be > caused by this, but I am baffled how the missing headers could be. Headers > search paths should be unaf

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 updated this revision to Diff 454212. Ericson2314 added a comment. Rebase; Include d3a1dbc4907b59690f9013cdb6221573ca4233f1 as requested Thanks @thakis for letting me know. Repository: rG LLVM Github Monorepo

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

Ericson2314 created this revision. Ericson2314 added reviewers: sebastian-ne, beanz, phosek, ldionne. Herald added subscribers: libc-commits, libcxx-commits, Enna1, bzcheeseman, pmatos, asb, ayermolo, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

sebastian-ne added a comment. This is probably caused by using `CMAKE_CFG_INTDIR` (indirectly) in more places. Seems like it expands to `$(Configuration)` for Visual Studio. Searching more about it, it’s only set at build time, but not at install time, which is a problem as well. On top of all,

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

arichardson added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:6 +if (NOT DEFINED CMAKE_BINARY_BINDIR) + set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin") +endif() I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR`

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

sebastian-ne added a comment. I’m not sure if it’s the case for all places (as `CMAKE_CFG_INTDIR` is not defined at install time), but I think the `CMAKE_BINARY_LIBDIR` introduced here serves the same purpose as the already existing `LLVM_LIBRARY_DIR`. Same for `CMAKE_BINARY_INCLUDEDIR`, which i

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

CarlosAlbertoEnciso added a comment. @Ericson2314 It seems that these changes break building on Windows using Visual Studio 2019. There are 2 issues: - CMake now creates a directory `$(Configuration)` in the root build directory. $(Configuration)\lib\cmake\llvm Location before the changes:

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

gchatelet added a comment. It also broke `llvm-exegesis` for 32 bits targets, see https://github.com/llvm/llvm-project/issues/57348 Reverting changes for the files in `tools/llvm-exegesis/` solves the compilation issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

Ericson2314 added a comment. Let's just revert this. I'll do it later today if no one beats me to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132316/new/ https://reviews.llvm.org/D132316 ___ lldb-c

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

CarlosAlbertoEnciso added a comment. In D132316#3748845 , @Ericson2314 wrote: > Let's just revert this. I'll do it later today if no one beats me to it. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

Re: [Lldb-commits] [PATCH] D132624: [LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`

Thank you for that hint, Jim! This infrastructure indeed looks very interesting for my use case. I will most likely use the HistoryThread class to represent the backtrace of the logical coroutine threads. Is my understanding correct, that `GetExtendedBacktraceThreads` only allows me to append addi

[Lldb-commits] [PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

Ericson2314 created this revision. Ericson2314 added reviewers: phosek, sebastian-ne, beanz. Herald added subscribers: bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jac

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

compnerd added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:3 + get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME) +endif() + Should this perhaps be moved further down near the usage? Comment at: cma

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

ldionne added a comment. I think I like the spirit of this change, which seems to be to move us closer to `CMAKE_foo` variables and further from `LLVM_foo` variables for equivalent functionality. I have a comment, but this essentially LGTM. The libc++ CI failures (in particular the bootstrappin

[Lldb-commits] [PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

Ericson2314 updated this revision to Diff 459968. Ericson2314 added a comment. Rebase on top of D133828 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132316/new/ https://reviews.llvm.org/D132316 Files: bolt/lib

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

Ericson2314 added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:3 + get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME) +endif() + compnerd wrote: > Should this perhaps be moved further down near the usage? Sure ==

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

Ericson2314 updated this revision to Diff 459985. Ericson2314 marked 4 inline comments as done. Ericson2314 added a comment. Rebase, avoid sed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132608/new/ https://reviews.llvm.org/D132608 Files: bolt

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

MaskRay added inline comments. Comment at: llvm/include/llvm/Support/Compression.h:48 + +constexpr int NoCompression = -5; +constexpr int BestSpeedCompression = 1; MaskRay wrote: > I missed the values here. Why is -5 picked for NoCompression? What does it > mean

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

Ericson2314 updated this revision to Diff 460099. Ericson2314 added a comment. Fix typo in compiler-rt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132608/new/ https://reviews.llvm.org/D132608 Files: bolt/CMakeLists.txt clang/CMakeLists.txt

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

Ericson2314 updated this revision to Diff 460191. Ericson2314 added a comment. Fix misspelled variable `CLANG_CURRENT_SOURCE_DIR` -> `CLANG_SOURCE_DIR` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132608/new/ https://reviews.llvm.org/D132608 File

[Lldb-commits] [PATCH] D133890: [CMake] Do these replacements to make use of D132608

Ericson2314 created this revision. Ericson2314 added reviewers: phosek, ldionne, sebastian-ne, beanz. Herald added subscribers: libc-commits, bzcheeseman, ayermolo, sdasgup3, abrachet, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufen

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

ldionne added inline comments. Comment at: libcxx/CMakeLists.txt:421 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(default_install_path "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1") +else() ldionne wrote: > Instead, I'd do th

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

Ericson2314 added inline comments. Comment at: libcxx/CMakeLists.txt:421 +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(default_install_path "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1") +else() ldionne wrote: > ldionne wrote:

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

Ericson2314 updated this revision to Diff 460222. Ericson2314 added a comment. Rebase, fix some issues (by the looks of it) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130586/new/ https://reviews.llvm.org/D130586 Files: clang/CMakeLists.txt

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

tianshilei1992 added a comment. Is it a good idea to define variables starting with `CMAKE`? That might be confusing for later maintenance by other developers because chances are that they will think those variables are CMake provided variables, try to look up into CMake document to see what th

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

Ericson2314 added a comment. @tianshilei1992 that is a fair point. I would be open to calling them something else. I just don't want to call them `LLVM_*` because that would be confusing since there is (a) LLVM in particular (b) the monorepo / project as a whole, and this variable are about *n

[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] D133890: [CMake] Do these replacements to make use of D132608

compnerd added a comment. I don't see anything wrong with this change per se, but I'm conflicted on the name of the variable. These are not standard variables but are encroaching on the CMake namespace. What happens if upstream decides to use these names? I think that we should keep the name

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

andrewrk added a comment. 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. Also LLVM needs to now include a Findzstd.cmake file or else we get this error: CMake Error at cmake/

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

MaskRay added a comment. zstd provides GNU Makefile, CMake, and Meson. The CMake files are only installed when configured with CMake. Debian and Ubuntu lack such files. The pkg-config file libzstd.pc is probably the most portable file. (I have also used it for a binutils-gdb patch.) I think we

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

andrewrk added a comment. Compiler infrastructure should not assume the existence of a Linux distribution. The portable way is simple and can easily be supported. One should be able to install `$prefix/lib/libzstd.a` and `$prefix/include/zstd.h`, then have that prefix searched as part of the st

[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] D128465: [llvm] add zstd to `llvm::compression` namespace

mgorny added a comment. Is anyone working on a solution that would work for people using a zstd install method that's actually supported upstream? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128465/new/ https://reviews.llvm.org/D128465

[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] D131919: Move googletest to the third-party directory

tstellar updated this revision to Diff 468784. tstellar marked 4 inline comments as done. tstellar added a comment. Herald added a subscriber: zero9178. Rebase and fix some missed path updates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131919/ne

Re: [Lldb-commits] [lldb] 3e03873 - [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux

> For example the type `const char *` also has a summary provider, and I'd hope that this feature does not depend on the specific way in which that summary is computed. Yea, it would be nice to have this on Linux. Will try to use a type whose format is compatible across platforms Am Mi., 2. Nov.

Re: [Lldb-commits] [lldb] 3e03873 - [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux

That's a known issue for the NativePDB plugin: https://github.com/llvm/llvm-project/issues/51933. Because currently the plugin internally looks up function names by full names. Even if you try to set a breakpoint with a full function name, the plugin itself only receives the basename. On Wed, Nov

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

MaskRay added a comment. Herald added a subscriber: Moerafaat. > We held off on this before as `LLVM_LIBDIR_SUFFIX` conflicted with it. Now we > return this. > > I imagine this is too potentially-breaking to make LLVM 15. That's fine. ... These sentences are no longer relevant and should be remo

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

sylvestre.ledru added a comment. Maybe add it to the release notes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137337/new/ https://reviews.llvm.org/D137337 ___ lldb-commits mailing list lldb-commits@l

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

serge-sans-paille created this revision. serge-sans-paille added reviewers: MaskRay, tstellar, mgorny, sylvestre.ledru. Herald added subscribers: libc-commits, libcxx-commits, Moerafaat, zero9178, Enna1, bzcheeseman, kosarev, ayermolo, sdasgup3, wenzhicui, wrengr, cota, StephenFan, teijeong, rdzh

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

jhenderson added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: Michael137, sstefan1, JDevlieghere. This is going to be impossible to cleanly review as-is. Could it be broken into lots of smaller chunks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

glider added subscribers: pcc, glider. glider added inline comments. Comment at: clang/docs/DataFlowSanitizerDesign.rst:54 - /// Returns whether the given label label contains the label elem. + /// Returns whether the given label contains the label elem. int dfsan_has_labe

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

serge-sans-paille updated this revision to Diff 472945. serge-sans-paille added a comment. + Release Note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137337/new/ https://reviews.llvm.org/D137337 Files: bolt/CMakeLists.txt bolt/include/bolt/RuntimeLibs/RuntimeLibraryVariables.inc.

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

foad added a comment. I committed the lib/Target/AMDGPU parts as 5073ae2a883f . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 __

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

Meinersbur added a comment. Changes in `/polly/` look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

aartbik added a comment. LGTM for sparse changes (note that you could have made your life a bit easier if you had broken this revision up at least over different projects, getting a global "LGTM" from somebody may be a bit hard now ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

ldionne accepted this revision. ldionne added a comment. Thanks for the fixes. LGTM for `libcxx/`, `libcxxabi/` and `libunwind/`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 __

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

DavidSpickett added a comment. Looked at the lldb changes, some comments for you. If you want to get a "looks good" for those please submit a separate review with only the lldb parts and I'll review that instead. As others said, appreciate the effort but the review process doesn't scale well t

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

tianshilei1992 added a comment. Changes to `openmp` look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 ___ lldb-commits mailing list lldb-commits@

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

sivachandra accepted this revision. sivachandra added a comment. Changes in the libc directory LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137338/new/ https://reviews.llvm.org/D137338 ___ lldb-co

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

mgorny added inline comments. Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32 SmallString<128> LibPath = llvm::sys::path::parent_path(Dir); - llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX); + llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR); if (!

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

tianshilei1992 added a comment. Can you verify if projects enabled by `LLVM_ENABLE_RUNTIMES` (not `LLVM_ENABLE_PROJECTS`) will still be installed properly, aka in LLVM's lib directory? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137337/new/ https://reviews.llvm.org/D137337

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

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

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

int3 added a comment. lld/MachO changes lgtm Comment at: lld/MachO/Driver.cpp:1386 // ld64 does something really weird. It attempts to realign the value to the -// page size, but assumes the the page size is 4K. This doesn't work with +// page size, but assumes th

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

aaron.ballman added a comment. The clang parts generally LG, but I spotted a few things. Thank you for this cleanup effort! Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:467 - /// Return a matcher that that points to the same implementation, but sets th

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

curdeius added inline comments. Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) { - // Handle C++ base classes. Non-virtual bases can treated a a kind of + // Handle C++ base classes. Non-virtual bases can treated a

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) { - // Handle C++ base classes. Non-virtual bases can treated a a kind of + // Handle C++ base classes. Non-virtual bases can treat

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

tstellar added a comment. Herald added a subscriber: Moerafaat. @probinson Does this latest update look better? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131919/new/ https://reviews.llvm.org/D131919

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

probinson accepted this revision. probinson added a comment. Yes, 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-commit

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

Rageking8 added a comment. I am ok with you guys taking the parts of this revision that you reviewed and directly commiting them to the repo, just like how the person above did it. This is to break the revision up to smaller parts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

Ericson2314 added inline comments. Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32 SmallString<128> LibPath = llvm::sys::path::parent_path(Dir); - llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX); + llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);

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

thesamesam created this revision. thesamesam added reviewers: mgorny, aaron.ballman. Herald added a reviewer: bollu. Herald added subscribers: libcxx-commits, Enna1, pengfei. Herald added projects: libunwind, All. Herald added a reviewer: libunwind. thesamesam requested review of this revision. Her

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

thesamesam added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: sstefan1, JDevlieghere. Note that we of course don't have to bother doing this for C++ sources and test programs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[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] D137503: [CMake] Fix -Wstrict-prototypes

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for this! If we do a 15.0.5, I think these changes should probably go into there as well (WDYT?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137503/new/ https:/

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

MaskRay added a comment. In D137503#3910651 , @aaron.ballman wrote: > LGTM, thank you for this! If we do a 15.0.5, I think these changes should > probably go into there as well (WDYT?) I support this. This will make llvm-project 15.0.5 buildable by mor

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

aaron.ballman added a comment. In D137338#3907281 , @Rageking8 wrote: > I am ok with you guys taking the parts of this revision that you reviewed and > directly commiting them to the repo, just like how the person above did it. > This is to break the re

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

Ericson2314 added a comment. I have done the deduping @phosek requested, and changed the variable names from `CMAKE_*` to `LLVMPROJ_*` which hopefully satisfies everyone's criteria. Happy with other non `LLVM_` options too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

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

Ericson2314 updated this revision to Diff 473679. Ericson2314 added a comment. Herald added a subscriber: Moerafaat. Dedup code, rename variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132608/new/ https://reviews.llvm.org/D132608 Files: b

[Lldb-commits] [PATCH] D133890: [CMake] Do these replacements to make use of D132608

Ericson2314 updated this revision to Diff 473684. Ericson2314 added a comment. Herald added a subscriber: Moerafaat. Rebase, rename variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133890/new/ https://reviews.llvm.org/D133890 Files: bolt/

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

Rageking8 added a comment. In D137338#3912135 , @aaron.ballman wrote: > In D137338#3907281 , @Rageking8 > wrote: > >> I am ok with you guys taking the parts of this revision that you reviewed >> and directly co

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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG32a2af44e1e8: [CMake] Fix -Wstrict-prototypes (authored by thesame

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

Mordante created this revision. Herald added a reviewer: bollu. Herald added subscribers: Moerafaat, zero9178, Enna1, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiag

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

Mordante added a comment. In D137724#3917616 , @thieta wrote: > I think this is fine as we have discussed before. But I really dislike the > code duplication for the check. We could put it in a include() I guess - but > maybe it's not worth it. I wante

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

thieta accepted this revision as: thieta. thieta added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: sstefan1, JDevlieghere. I think this is fine as we have discussed before. But I really dislike the code duplication for the check. We could put it in a include() I guess

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

MaskRay accepted this revision as: MaskRay. MaskRay added a comment. I think `if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)` checks for standalone builds is not necessary. The check in `llvm/CMakeLists.txt` suffices. It's unlikely the users will use different cmake versions to configure

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

Mordante added a comment. In D137724#3917644 , @MaskRay wrote: > I think `if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)` checks for > standalone builds is not necessary. The check in `llvm/CMakeLists.txt` > suffices. > It's unlikely the users w

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG59052468c3e3: Move googletest to the third-party directory (authored by tstellar). Changed prior to commit: https://reviews.llvm.org/D131919?vs=46

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

ldionne added a comment. LGTM (sorry, it looks like I approved on behalf of all libc++ vendors but that wasn't my intention). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137724/new/ https://reviews.llvm.org/D137724 _

  1   2   >