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

2023-02-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2023-02-10 Thread Fangrui Song via Phabricator via lldb-commits
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)

2023-03-02 Thread Fangrui Song via Phabricator via lldb-commits
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

2023-04-12 Thread Fangrui Song via Phabricator via lldb-commits
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

2023-04-13 Thread Fangrui Song via Phabricator via lldb-commits
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

2023-04-14 Thread Fangrui Song via Phabricator via lldb-commits
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

2023-04-14 Thread Fangrui Song via Phabricator via lldb-commits
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] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Drive-by: std::filesystem is unavailable for GCC<8. The minimum GCC version is 5, so std::filesystem isn't suitable. You may use`include/llvm/Support/FileSystem.h` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105732/new/ https://reviews.llvm.org/D105732

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:148 +std::string proc_fd_path = "/proc/self/fd"; +std::filesystem::path fp(proc_fd_path); /proc/self/fd is generally unavailable on non-Linux OSes. CHANGES

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. LGTM. Some nits Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:161 +// stdin/stdout/stderr +if ((fd > 2) && !info.GetFileActionForFD(fd) && fd != error_fd) + files_to_close.push_b

[Lldb-commits] [PATCH] D105732: [lldb] Update logic to close inherited file descriptors.

2021-08-17 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:148 + +std::string proc_fd_path = "/proc/self/fd"; +std::error_code EC; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-25 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/test/API/functionalities/dyld-launch-linux/Makefile:1 +CXX_SOURCES := main.cpp +LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)" ``` CXX_SOURCES := main.cpp DYLIB_NAME := signal_file DYLIB_CXX_SOURCES := signal_file.cpp

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-25 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:16 +exe = "/lib64/ld-linux-x86-64.so.2" +if(os.path.exists(exe)): +target = self.dbg.CreateTarget(exe) ==

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

2021-08-25 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:293 + // ld.so saves empty file name for the executable file in the link map. + // When executable is run using ld.so, we need to be update executable path. + if (name.em

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

2021-09-09 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. `ninja clang` on Windows works. `check-llvm-tools` has a few `REQUIRES: system-windows` tests and I am running them. Test invocation is bit slow. Repository: rG LLVM Github Monorepo CHAN

[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files

2021-09-13 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:120 + if (error.Fail()) { +error.SetErrorString("Unable to convert the csd string to UTF16."); +return error; https://llvm.org/docs/CodingStandards

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

2021-09-14 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a subscriber: felixonmars. MaskRay added a comment. Hi Luis, is this still needed after D86292 ? Or are there missing pieces? @felixonmars reported that https://archriscv.felixc.at/.status/logs/lldb.log still failed to build on riscv64 Arch Linux. I

[Lldb-commits] [PATCH] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

2021-09-14 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I fixed this in e69d359841b6358f1d17569212ef8cf91244ca11 and fixed some style issues. --- This needs extra care. While Clang -Wformat flags printf("%llu", (size_t)3); warning: format specifi

[Lldb-commits] [PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

2021-09-23 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109975/new/ https://reviews.llvm.org/D109975 _

[Lldb-commits] [PATCH] D111454: Move TargetRegistry.(h|cpp) from Support to MC

2021-10-22 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111454/new/ https://reviews.llvm.org/D111454 ___ lldb-commits mailing list lldb-commits@list

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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-01-04 Thread Fangrui Song via Phabricator via lldb-commits
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`."

2022-01-05 Thread Fangrui Song via Phabricator via lldb-commits
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`."

2022-02-01 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-15 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-16 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-19 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-19 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-02-23 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-03-09 Thread Fangrui Song via Phabricator via lldb-commits
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] D111509: [clang] use getCommonSugar in an assortment of places

2022-09-08 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Since the fix was not merged, I will revert this soon. I saw a clang segfault with `clang++ libstdc++-v3/src/c++98/complex_io.cc` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111509/new/ https://reviews.llvm.org/D111509

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-09-08 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D111509#3778882 , @mizvekov wrote: > In D111509#3778879 , @MaskRay wrote: > >> Since the fix was not merged, I will revert this soon. >> >> I saw a clang segfault with `clang++ libstdc+

[Lldb-commits] [PATCH] D133530: [lldb] Add zstd support

2022-09-08 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lldb/test/Shell/ObjectFile/ELF/compressed-sections-zstd.yaml:19 +Content: deadbeefbaadf00d +## The legacy .zdebug format is not supported. + - Name:.zdebug_info Delete `.zdebug`. It is unrelated

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

2022-09-08 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. See `test/tools/llvm-objcopy/ELF/compress-debug-sections-zstd.test`. Use a similar file for ELFCLASS32 which runs `llvm-objcopy --compress-debug-sections=zstd` then `llvm-objcopy --decompress-debug-sections`. Then compare the output with the original with just one `ll

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

2022-09-08 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp:509 + const ElfType OutputElfType = + getOutputElfType(Config.OutputArch.value_or(MachineInfo())); + const bool Is64Bit = This is incorrect. Config.OutputArch is usually unset (

[Lldb-commits] [PATCH] D133530: [lldb] Add zstd support

2022-09-13 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. Ping for unresolved issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133530/new/ https://reviews.llvm.org/D133530 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D132300: [clang][lldb][cmake] Use new `*_INSTALL_LIBDIR_BASENAME` CPP macro

2022-09-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In the long term we should just remove the `CLANG_INSTALL_LIBDIR_BASENAME` customization. This is supposed for GCC multilib lib32 lib64 names but we don't necessarily use it for Clang + compiler-rt files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2022-09-21 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. D134385 should fix the problem:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133525/new/ https://reviews.llvm.org/D133525 ___ lldb-commits ma

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

2022-10-06 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. I missed https://reviews.llvm.org/D134668 . See my comment there I don't think the change is necessary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135400/new/ https://reviews.llvm.org/D135400 ___ lldb-commits mail

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

2022-10-06 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Driver/debug-options-aranges.c:6 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s -// CHECK: --pl

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

2022-10-27 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. It'll be good if we have a wiki describing how downstream users invoke cmake, so that large cmake refactoring can be verified beforehand. (like usage verification https://github.com/opencollab/llvm-toolchain-integration-test-suite, but for cmake invocations) CHANGES S

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

2022-11-04 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. Considering https://discourse.llvm.org/t/should-we-continue-embed-the-full-llvm-version-in-lib-clang/62094 and this thread, I think overall people favor this patch. If a distribution

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D137217#3926601 , @tejohnson wrote: > @MaskRay wondering if this is a good change to make for ELF as well, wdyt? Yes, I think this is a good idea and improves debuggability. The change is non-trivial so so this patch focusing

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-15 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: lld/COFF/LTO.cpp:229 +StringRef ltoObjName; +if (bitcodeFilePath == "ld-temp.o") { + ltoObjName = MaskRay wrote: > tejohnson wrote: > > zequanwu wrote: > > > tejohnson wrote: > > > > This case should always

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:846 + auto AddStream = [&](size_t Task, + Twine ModuleName) -> std::unique_ptr { int FD = -1; `Twine` should only be used as const refer

[Lldb-commits] [PATCH] D138276: TableGen: require tablegen cl::opts to be registered explicitly

2022-11-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. This is similar to `mlir::register*Options` and looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D138376: Use None consistently (NFC)

2022-11-19 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. `enum class NoneType { None = 1 };` is from 0cd22f9540c0591132ec991c51103cf800cf4e24 (2017) for MSVC workaround. I assume it is no longer needed.. Repository: rG LLVM Github Monore

[Lldb-commits] [PATCH] D137217: [LTO][COFF] Use bitcode file names in lto native object file names.

2022-11-22 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment. In D137217#3945366 , @glandium wrote: > Almost there, but not quite: > > [task 2022-11-22T23:55:36.341Z] > /builds/worker/fetches/llvm-project/llvm/tools/gold/gold-plugin.cpp:1106:6: > error: no matching function for call to o

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

2022-11-23 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:218 + explicit NameLookup(std::nullopt_t) : Data(nullptr, true) {} explicit NameLookup(std::nullptr_t) : Data(nullptr, false) {} NameLookup() : NameLookup(nullptr) {} --

[Lldb-commits] [PATCH] D138310: [NFC] Make headers self-contained.

2022-11-28 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: StephenFan. Comment at: lldb/source/Plugins/Instruction/RISCV/RISCVInstructions.h:15 -#include "EmulateInstructionRISCV.h" #include "llvm/

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

2022-12-06 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-06 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-06 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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.

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

2022-12-07 Thread Fangrui Song via Phabricator via lldb-commits
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

<    1   2   3