[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

MaskRay added inline comments. Comment at: tools/lldb-mi/MICmdCmdGdbSet.cpp:447 +bool CMICmdCmdGdbSet::OptionFnBreakpoint( + const CMIUtilString::VecString_t &vrWords) { +bool bPending = false; clang-format Reposito

[Lldb-commits] [PATCH] D52953: [lldb-mi] Implement -gdb-set breakpoint pending on/off

MaskRay added a comment. And there is no reviewers set :) Repository: rL LLVM https://reviews.llvm.org/D52953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

MaskRay added a comment. If we decide to optimize DWARF garbage collection, something generic will be cool. This generic-abi thread has some discussion about that https://groups.google.com/d/msg/generic-abi/A-1rbP8hFCA/EDA7Sf3KBwAJ (e.g. using COMDAT but it seems challenging and it comes with

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

MaskRay added inline comments. Comment at: lld/ELF/MarkLive.cpp:192 + Sec->Live = true; + if (Sec->kind() != SectionBase::Kind::Regular && + Sec->kind() != SectionBase::Kind::Merge) This check can be changed to `!isa && !isa`. But do you just want to excl

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

MaskRay added a comment. In D54747#1342666 , @rocallahan wrote: > Here are some results for the rusoto test in > https://github.com/rust-lang/rust/issues/56068#issue-382175735: > > | LLD | Binary size in bytes | > | LLD 6.0.1

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

MaskRay added a comment. @rocallahan I find that people are discussing a generic approach in D59553 Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

MaskRay added a comment. Hi! The added lines have some spaces at line ends. I think using `clang-format-diff.py` would remove them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154542/new/ https://reviews.llvm.org/D154542 ___

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

MaskRay added a comment. In D151683#4486321 , @aaron.ballman wrote: > Aside from the failing precommit CI test case in C, I think this LGTM. I've > added @MaskRay as the code owner for the command line option changes in case > he's got concerns regardi

[Lldb-commits] [PATCH] D155018: [Support] Move StringExtras.h include from Error.h to Error.cpp (part 5)

MaskRay added a comment. Still, would be good to land the lldb commit separately. And ensure that you have performed the last-minute testing... Ensure that projects like `flang bolt` still build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155018

[Lldb-commits] [PATCH] D155178: [Support] Move StringExtras.h include from Error.h to Error.cpp (part 6)

MaskRay added subscribers: mysterymath, phosek. MaskRay added a comment. In D155178#4498911 , @thakis wrote: > http://45.33.8.238/linux/112305/step_4.txt looks like a missing include I've fixed it in 816141ce0eb899178dbcb6f0671875eb825b2f84

[Lldb-commits] [PATCH] D155178: [Support] Move StringExtras.h include from Error.h to Error.cpp (part 6)

MaskRay added a comment. In D155178#4499105 , @MaskRay wrote: > In D155178#4498911 , @thakis wrote: > >> http://45.33.8.238/linux/112305/step_4.txt looks like a missing include > > I've fixed it in 816141ce0eb8991

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

MaskRay added a comment. Reverse ping @philnik :) The `release/17.x` branch will be created soon (https://discourse.llvm.org/t/llvm-17-0-0-release-planning-and-update/71762). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151683/new/ https://revie

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

MaskRay added a comment. It seems that a lldb specific test is needed. Adding a new method to `llvm/include/llvm/MC/MCInstrAnalysis.h` is fine with me, though I haven't checked the semantics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156086/ne

[Lldb-commits] [PATCH] D156086: [lldb][NFC] Use MCInstrAnalysis when available in the disassembler plugin

MaskRay added inline comments. Comment at: lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp:23 + +class TestMCDisasmInstanceRISCV : public testing::Test { +public: Place all classes and test methods in an anonymous namespace. Com

[Lldb-commits] [PATCH] D157028: [llvm] Extract common `OptTable` bits into macros

MaskRay added a comment. I'll be away for a few days but I took a quick glance. This change looks reasonable. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157028/new/ https://reviews.llvm.org/D157028 _

[Lldb-commits] [PATCH] D157028: [llvm] Extract common `OptTable` bits into macros

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D157028#4558724 , @jansvoboda11 wrote: > Here's an example of a patch that changes the `OPTION` macro: D157029 >

[Lldb-commits] [PATCH] D157028: [llvm] Extract common `OptTable` bits into macros

MaskRay added inline comments. Comment at: lld/ELF/Driver.h:28 OPT_INVALID = 0, -#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID, +#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__), #include "Options.inc" lld/wasm lld/COFF lld/MachO

[Lldb-commits] [PATCH] D157497: feat: Migrate isArch16Bit

MaskRay added a comment. In D157497#4584621 , @Pivnoy wrote: > This discussion was the main motivation for this change. > https://discourse.llvm.org/t/rfc-refactor-triple-related-classes/70410/11 > As a result, Triple should become a data class, and its d

[Lldb-commits] [PATCH] D151567: [LLVM][Support] Report EISDIR when opening a directory on AIX

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Thanks for the patch, but there are two issues that should be fixed: (1) stat => fstat (2) change the subject to mean that this is not AIX specific (`[LLVM][Support] Report EISDIR

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added inline comments. Herald added a subscriber: JDevlieghere. Comment at: llvm/docs/DeveloperPolicy.rst:64 +.. FIXME: Edit for LLVM Issues in Github. + You can drop this and let others fix this paragraph. Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added inline comments. Comment at: libcxx/docs/index.rst:220 * `libc++abi Homepage `_ -* `LLVM Bugzilla `_ +* `LLVM Issues `_ * `libcxx-commits Mailing List`_ -

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added a subscriber: jhenderson. MaskRay added a comment. @jhenderson Comment at: libunwind/docs/index.rst:101 * `LLVM Homepage `_ -* `LLVM Bugzilla `_ +* `LLVM Issues `_ * `cfe-co

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added inline comments. Comment at: llvm/docs/CommandGuide/llvm-objcopy.rst:539 -To report bugs, please visit . +To report bugs, please visit . https://github.com/llvm/llvm-project/l

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added inline comments. Comment at: lld/docs/_templates/indexsidebar.html:3-4 lld bugs should be reported at the - LLVM https://bugs.llvm.org/";>Bugzilla. + LLVM https://github.com/llvm/llvm-project/issues/";>Issues. ChuanqiXu wrote: > MaskRay wrote:

[Lldb-commits] [PATCH] D116351: Update Bug report URL to Github Issues

MaskRay added inline comments. Comment at: libcxx/docs/index.rst:220 * `libc++abi Homepage `_ -* `LLVM Bugzilla `_ +* `LLVM Issues `_ * `libcxx-commits Mailing List`_ -

[Lldb-commits] [PATCH] D115960: Revert D109159 "[amdgpu] Enable selection of `s_cselect_b64`."

MaskRay reopened this revision. MaskRay added a subscriber: ldionne. MaskRay added a comment. (CC @ldionne @smeenai) The revert 859ebca744e634dcc89a2294ffa41574f947bd62 included many unintended changes. Repository: rG LLV

[Lldb-commits] [PATCH] D115960: Revert D109159 "[amdgpu] Enable selection of `s_cselect_b64`."

MaskRay added a comment. Please abandon this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115960/new/ https://reviews.llvm.org/D115960 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D119092: Cleanup LLVMDebugInfoCodeView headers

MaskRay accepted this revision. MaskRay 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/D119092/new/ https://reviews.llvm.org/D119092

[Lldb-commits] [PATCH] D119186: [lldb][gdb-remote] Fix linking of gdb-remote when LLVM_ENABLE_ZLIB is ON

MaskRay added a comment. In D119186#3303530 , @mceier wrote: > I need someone to merge it for me. Testing and merging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119186/new/ https://reviews.llvm.org/D1

[Lldb-commits] [PATCH] D119186: [lldb][gdb-remote] Fix linking of gdb-remote when LLVM_ENABLE_ZLIB is ON

This revision was automatically updated to reflect the committed changes. Closed by commit rG385f5c4d3379: [lldb][CMake] Fix linking of gdb-remote when LLVM_ENABLE_ZLIB is ON (authored by mceier, committed by MaskRay). Changed prior to commit: https://reviews.llvm.org/D119186?vs=406706&id=40671

[Lldb-commits] [PATCH] D119723: Cleanup LLVMDWARFDebugInfo

MaskRay accepted this revision. MaskRay added subscribers: hubert.reinterpretcast, Kai. MaskRay added a comment. This revision is now accepted and ready to land. > Plus llvm/Support/Errc.h not included by a bunch of > llvm/DebugInfo/DWARF/DWARF*.h files You may check whether we can just get rid

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

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119918/new/ https://reviews.llvm.org/D1199

[Lldb-commits] [PATCH] D120195: Cleanup llvm/DebugInfo/PDB headers

MaskRay added a comment. before: 1065515095 after: 1065629059 An increase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120195/new/ https://reviews.llvm.org/D120195 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D120195: Cleanup llvm/DebugInfo/PDB headers

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. It'd be good to test `-DLLVM_ENABLE_MODULES=on` build. Some files get pure new headers. I still think it is good thing to do it separately. There is a risk that someone may revert the change

[Lldb-commits] [PATCH] D120195: Cleanup llvm/DebugInfo/PDB headers

MaskRay added a comment. In D120195#3334697 , @serge-sans-paille wrote: > In D120195#943 , @MaskRay wrote: > >> It'd be good to test `-DLLVM_ENABLE_MODULES=on` build. > > Sure, I'll add that to my local test

[Lldb-commits] [PATCH] D121168: Cleanup includes: LLVMTarget

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: llvm/include/llvm/Target/TargetMachine.h:21 #include "llvm/IR/PassManager.h" -#include "llvm/Pass.h" #include "llvm/Support/CodeGen.h" ---

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

MaskRay added a comment. I like the idea of `all` for llvm-objdump, but it should be a separate change, with `-mattr=-all` tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127741/new/ https://reviews.llvm.org/D127741 _

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

MaskRay added a comment. In D128465#3612948 , @phosek wrote: > 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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

MaskRay added a comment. lld/ELF change should be dropped from this change. Don't use `config->endianness`. I feel sad that for little-endian users who don't use big-endian, every write now is slightly slower due to a check ;-) Comment at: clang/lib/Basic/Targets/RISCV.cpp:12

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

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

MaskRay added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.h:144 + +StringRef LayoutEndianness = Triple.isLittleEndian() ? "e" : "E"; + gbenyei wrote: > MaskRay wrote: > > You may use a `char` and possibly fold this into the expression below. >

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

MaskRay added a comment. In D128612#3618167 , @gbenyei wrote: > In D128612#3617955 , @MaskRay wrote: > >> lld/ELF change should be dropped from this change. Don't use >> `config->endianness`. >> I feel sad that f

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

MaskRay added a comment. Consider adding `have_zstd` to `compiler-rt/test/lit.common.cfg.py` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128465/new/ https://reviews.llvm.org/D128465 ___ lldb-commits ma

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

MaskRay accepted this revision. MaskRay added a comment. Please check both `LLVM_ENABLE_ZSTD={on,off}` work. Ideally, check that when zstd' cmake (`libzstd-dev` on Debian) is unavailable, `LLVM_ENABLE_ZSTD=on` works (there is no zstd functionality but the cmake invocation should succeed). ===

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

MaskRay added inline comments. Comment at: lld/test/lit.site.cfg.py.in:21 config.have_zlib = @LLVM_ENABLE_ZLIB@ +config.have_zstd = @LLVM_ENABLE_ZSTD@ config.have_libxar = @LLVM_HAVE_LIBXAR@ This needs a change in lld/test/CMakeLists.txt Looks like you haven't

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

MaskRay reopened this revision. MaskRay added a comment. This revision is now accepted and ready to land. This patch has changed a lot from what I have reviewed. The CMake change should be added along with `llvm::compression::zstd::*` functions. Otherwise the change just introduces some CMake var

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

MaskRay added inline comments. Comment at: lld/test/lit.site.cfg.py.in:21 config.have_zlib = @LLVM_ENABLE_ZLIB@ +config.have_zstd = @LLVM_ENABLE_ZSTD@ config.have_libxar = @LLVM_HAVE_LIBXAR@ MaskRay wrote: > This needs a change in lld/test/CMakeLists.txt > > L

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

MaskRay added a comment. As I mentioned, the proper approach is to add zstd functionality along with the CMake change, instead of adding CMake to all llvm-project components without a way to test them. Comment at: lld/test/lit.site.cfg.py.in:21 config.have_zlib = @LLVM_ENABL

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

MaskRay added a comment. In D128465#3646203 , @ckissane wrote: > In D128465#3642997 , @MaskRay wrote: > >> As I mentioned, the proper approach is to add zstd functionality along with >> the CMake change, instead

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

MaskRay added a comment. In D128465#3646561 , @ckissane wrote: > In D128465#3646258 , @MaskRay wrote: > >> In D128465#3646203 , @ckissane >> wrote: >> >>> In D128465#3642

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

MaskRay added a comment. The patch looks mostly good but please fix the subject and the summary. Note: if you fix the local commit message, you can use ` arc diff --head=HEAD 'HEAD^' --verbatim` to update the subject and the summary. Comment at: llvm/lib/Support/Compression.c

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

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: llvm/include/llvm/Support/Compression.h:54 + +void compress(StringRef InputBuffer, SmallVectorImpl &CompressedBuffer, + int Level = DefaultComp

[Lldb-commits] [PATCH] D129724: [lldb] Remove ELF .zdebug support

MaskRay created this revision. MaskRay added reviewers: ckissane, labath. Herald added subscribers: StephenFan, emaste. Herald added a reviewer: alexander-shaposhnikov. Herald added a project: All. MaskRay requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: l

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

MaskRay added a comment. Why was this reverted? Please make extensive tests especially for something dealing with the build system. When reverting a commit, briefly describe what happened. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128465/new/

[Lldb-commits] [PATCH] D129724: [lldb] Remove ELF .zdebug support

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1610 static SectionType GetSectionTypeFromName(llvm::StringRef Name) { if (Name.consume_front(".debug_") || Name.consume_front(".zdebug_")

[Lldb-commits] [PATCH] D129724: [lldb] Remove ELF .zdebug support

This revision was automatically updated to reflect the committed changes. MaskRay marked an inline comment as done. Closed by commit rGecfaf4801cd0: [lldb] Remove ELF .zdebug support (authored by MaskRay). Changed prior to commit: https://reviews.llvm.org/D129724?vs=444504&id=444707#toc Reposi

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

MaskRay added a comment. Instead of marking a differential `[OLD] ` (which may confuse readers whether this is to be abandoned), you can mark a differential `Request Reviews`/`Request Changes`. Then it will not be in the green state. I clicked "Reopen" to make it clear the patch hasn't relanded

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

MaskRay added a comment. In D128465#3660733 , @ckissane wrote: > In D128465#3660714 , @MaskRay wrote: > >> Instead of marking a differential `[OLD] ` (which may confuse readers >> whether this is to be abandoned)

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

MaskRay added a comment. In D130689#3704581 , @dyung wrote: > Your change is causing a build failure on the PS4 linux build bot using GCC > 9.3. Can you take a look? > https://lab.llvm.org/buildbot/#/builders/139/builds/26186 > > FAILED: > tools/clang

[Lldb-commits] [PATCH] D131422: [lldb] Remove include/lldb/lldb-private.h

MaskRay created this revision. MaskRay added reviewers: JDevlieghere, kastiglione. Herald added a subscriber: StephenFan. Herald added a project: All. MaskRay requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. The header from

[Lldb-commits] [PATCH] D131422: [lldb] Remove include/lldb/lldb-private.h

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcdeb50c32155: [lldb] Remove include/lldb/lldb-private.h (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

MaskRay added a comment. In D130689#3710291 , @aaron.ballman wrote: > In D130689#3710281 , @royjacobson > wrote: > >> In D130689#3709834 , @thieta wrote: >> >>> In D1306

[Lldb-commits] [PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

MaskRay closed this revision. MaskRay added a comment. a749e3295df4aee18a0ad723875a6501f30ac744 pushed by Aaron does not have a `Differential Revision:` line. Manual closing. Repository: rG LLVM Github Monorepo CHANGES SI

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

MaskRay added a comment. I vaguely recall that some lldb bots use a stand-alone build. Say I have a build directory /tmp/RelA with `-DLLVM_ENABLE_PROJECTS=clang` cmake -GNinja -S~/llvm-project/lldb -Bout/lldb -DCMAKE_PREFIX_PATH=/tmp/RelA will print CMake Warning at cmake/modules/LLDBConfi

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

MaskRay accepted this revision. MaskRay added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:315 + uint32_t endian = header.e_ident[EI_DATA]; + if(endian == ELFDATA2LSB) +return ArchSpec::eCore_ppc64le_generic;

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

MaskRay accepted this revision. MaskRay 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/D124673/new/ https://reviews.llvm.org/D124673

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb1aed14bfea0: [llvm][lldb] use FindLibEdit.cmake everywhere (authored by upsj, committed by MaskRay). Repository: rG LLVM Github Monorepo CHANGES

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

MaskRay added a comment. In D124673#3512037 , @paulkirth wrote: > Hi, Sorry for the late notification, but I think this change may not apply > correctly to all configs. > > We're seeing a breakage in Fuchsia's Clang CI builders: > https://luci-milo.apps

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

MaskRay added a comment. In D124673#3512190 , @phosek wrote: > In D124673#3512070 , @MaskRay wrote: > >> In D124673#3512037 , @paulkirth >> wrote: >> >>> Hi, Sorry for th

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

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Does this address the macOS build failure? 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

MaskRay added a comment. I am still seeing the zstdConfig.cmake zstd-config.cmake warning. @ckissane @phosek will you fix it? :) 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

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

[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] 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] 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] 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] 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] D137838: [RFC][Support] Move TargetParsers to new component

MaskRay added a comment. Moving target-specific parsers outside LLVMSupport LGTM. I even objected a bit when more stuff of this kind was introduced into LLVMSupport. +1 for adding temporary forwarding headers for now to avoid update all `#include` users. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

MaskRay added a comment. The name `llvm/lib/TargetParser` LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137838 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

MaskRay accepted this revision. MaskRay added a comment. Please check these builds all work: - default - `-DLLVM_LINK_LLVM_DYLIB=on` - `-DBUILD_SHARED_LIBS=on` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137838/new/ https://reviews.llvm.org/D137

[Lldb-commits] [PATCH] D140332: [ADT] Alias llvm::Optional to std::optional

MaskRay added a comment. Can you push `using OptionalFileEntryRef = CustomizableOptional;` and the renaming as a separate commit to make this patch smaller? There is a smaller that this rename may be reverted. 205c0589f918f95d2f2c586a01bea2716d73d603

[Lldb-commits] [PATCH] D140896: [WIP] Move from llvm::makeArrayRef to ArrayRef deduction guides

MaskRay accepted this revision. MaskRay added a comment. LGTM if `makeArrayRef` is kept in this patch. Do we foresee compiler bugs (for supported compilers) in this area? If there may be a risk, consider migrate a part of the code base first. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D140999: [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h

MaskRay accepted this revision. MaskRay added a comment. I don't think it is necessary to deprecate the old header then delete it after 16.0.0 is branched. llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. Perhaps only ldc `driver/targetmachine.cpp` uses the header. S

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added a comment. To enable smooth hash function migration in the future, we can explore the idea of adding `#ifdef EXPENSIVE_CHECKS\nshuffle` (see https://github.com/llvm/llvm-project/issues/34483). That means we should fix these tests properly to be non-dependent on the iteration order

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

MaskRay added a comment. edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable `CLANG_RESOURCE_DIR` but did not explain why. The patch introduced conditions in C++ code like std::string path_to_clang_dir = std::string(CLANG_RESOURCE_DIR).empty()

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added inline comments. Comment at: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp:638 - EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h"))); + EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.h"), testPath("A.cc"))); // Make

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: mlir/test/Transforms/print-op-graph.mlir:1 // RUN: mlir-opt -allow-unregistered-dialect -mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t 2>&1

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay added a comment. > `For posterity, the tests that fail on main are:` > > `Skipped : 43` > [...] You can remove these from the description. They are flaky tests unrelated to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/test/ObjectYAML/Offload/multiple_members.yaml:21 Value:"gfx908" Content: "cafefeed" Rebase:) I fixed obj2yaml Comment at: llvm/

[Lldb-commits] [PATCH] D143652: [lldb][DWARFASTParserClang] Attach linkage name to ctors/dtors if missing

MaskRay added a comment. This seems to cause many lldb failures https://lab.llvm.org/buildbot/#/builders/68/builds/47790 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143652/new/ https://reviews.llvm.org/D143652 __

[Lldb-commits] [PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

MaskRay added a comment. In D138539#4164313 , @steakhal wrote: > Oh, I forgot to mention why this change breaks the equality operator of > `llvm::StringSet`. It's because the `StringMap::operator==` will try to > compare the `value` as well, which is of

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

MaskRay added a comment. Herald added subscribers: bviyer, ekilmer, jplehr, thopre. @Ericson2314 @phosek What's the state of this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132608/new/ https://reviews.llvm.org/D132608 __

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

MaskRay added inline comments. Comment at: lldb/include/lldb/lldb-private-types.h:14 +#include "lldb/Target/RegisterFlags.h" #include "lldb/lldb-private.h" Is there a library layering violation? I assume that `lldb/Target/*.h` files depend on `lldb/include/l

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

MaskRay added inline comments. Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:19 +#include + chapuni wrote: > Odd layering... Good catch. LLVMDemangle cannot depend on other LLVM libraries. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

MaskRay added a comment. In D148384#4270505 , @dhoekwater wrote: > It looks like this breaks the build: > https://lab.llvm.org/buildbot#builders/119/builds/12865 > https://lab.llvm.org/buildbot#builders/123/builds/18388 > https://lab.llvm.org/buildbot

[Lldb-commits] [PATCH] D149784: [Damangle] convert rustDemangle to use std::string_view

MaskRay accepted this revision. MaskRay added a subscriber: labath. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/Utility/ConstString.h:186 operator llvm::StringRef() const { return GetStringRef(); } + // Implici

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

MaskRay added a comment. Herald added a subscriber: JDevlieghere. Thank you for making another try for the treewide change (which is admittedly very painful and not many people do such work). Can you include the original patch description to the summary? That is the main part and the message "Th

[Lldb-commits] [PATCH] D150996: LLVM_FALLTHROUGH => [[fallthrough]]. NFC

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150996/new/ https://reviews.llvm.org/D150996 __

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

MaskRay added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) { + if (!MangledName) +return false; Why is this change? The original contract is that `MangledName` m

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

MaskRay added inline comments. Comment at: llvm/lib/Demangle/Demangle.cpp:49 bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) { + if (!MangledName) +return false; nickdesaulniers wrote: > nickdesaulniers wrote: > > MaskRay wrote

  1   2   3   >