[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

MaskRay added a comment. In D101462#2733691 , @flip1995 wrote: > Not sure how the process from here on out is. I think it is important to > note, that I don't have push rights and someone else will have to land this > for me (I guess?). I'll keep this

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

MaskRay added a comment. In D101462#2733726 , @flip1995 wrote: >> I'll keep this open for a few days as it touches too many things. > > Sounds good  > >> but you'll need to provide your name and email > > ~~I used `arc diff`. The commits I made with

[Lldb-commits] [PATCH] D101462: [MC] Untangle MCContext and MCObjectFileInfo

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

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. LGTM. This looks like an improvement because it avoids a temporary `MCObjectFileInfo MOFI;` (which appeared to be initialized in two subsequent calls) in numerous places. Repository: rG

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added a comment. Can `createMCObjectFileInfo` return `MCObjectFileInfo` instead of `std::unique_ptr`? Comment at: clang/lib/Parse/ParseStmtAsm.cpp:590 + if (!MAI || !MII || !MOFI || !STI) { Diag(AsmLoc, diag::err_msasm_unable_to_create_target)

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added inline comments. Comment at: llvm/include/llvm/Support/TargetRegistry.h:26 #include "llvm/ADT/iterator_range.h" +#include "llvm/MC/MCObjectFileInfo.h" #include "llvm/Support/CodeGen.h" `include/llvm/Support/TargetRegistry.h now has cyclic

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

MaskRay added subscribers: compnerd, dblaikie. MaskRay added a comment. Because of `new MCObjectFileInfo`, we cannot use a forward declaration (incomplete class) to replace `#include "llvm/MC/MCObjectFileInfo.h"` in `TargetRegistry.h`. I thought about moving `TargetRegistry.{h,cpp}` from

[Lldb-commits] [PATCH] D101921: [MC] Make it possible for targets to define their own MCObjectFileInfo

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc2f819af73c5: [MC] Refactor MCObjectFileInfo initialization and allow targets to create… (authored by flip1995, committed by MaskRay). Repository:

[Lldb-commits] [PATCH] D104819: [ADT] Rename StringRef case insensitive methods for clarity

MaskRay added inline comments. Comment at: llvm/include/llvm/ADT/StringRef.h:192 -/// equals_lower - Check for string equality, ignoring case. +/// equals_insensitive - Check for string equality, ignoring case. LLVM_NODISCARD

[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

MaskRay added a comment. The refactoring part seems useful on its own. The controversial 4-byte alignment part should be split. Doesn't GNU as use 4 for most architectures as well? Why do you want to change it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

MaskRay added a comment. The `InitMCObjectFileInfo` refactoring is good, but I'll suggest `initMCObjectFileInfo(MCContext , bool PIC, bool LargeCodeModel)` (change the function name to `lowerCase`, and reorder MCContext before bool arguments). The refactoring adding `Triple` to

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

This revision was automatically updated to reflect the committed changes. Closed by commit rG50830e50031b: [lldb] Add -Wl,-rpath to make tests run with fresh built libc++ (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

MaskRay added a comment. This touches a lot of files. I am a bit worried that it would not be easy for a contributor to know OF_TextWithCRLF is needed to make SystemZ happy. > On SystemZ we need to open text files in text mode, Why can't it be served without CRLF translation? Repository:

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

MaskRay added a comment. In D99426#2653869 , @abhina.sreeskantharajan wrote: > In D99426#2653725 , @MaskRay wrote: > >> This touches a lot of files. I am a bit worried that it would not be easy >> for a

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

MaskRay added a comment. In D99426#2656217 , @rnk wrote: > In D99426#2653725 , @MaskRay wrote: > >> This touches a lot of files. I am a bit worried that it would not be easy >> for a contributor to know

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

MaskRay added a comment. /// The file should be opened in text mode on platforms like z/OS that make /// this distinction. OF_Text = 1, F_Text = 1, // For compatibility /// The file should use a carriage linefeed '\r\n'. /// Only makes a difference on windows. OF_CRLF = 2,

[Lldb-commits] [PATCH] D99426: [Windows] Add new OF_TextWithCRLF flag and use this flag instead of OF_Text

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. In D99426#2664883 , @abhina.sreeskantharajan wrote: > In D99426#2664841 , @MaskRay wrote: > >> /// The

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

MaskRay added a comment. Is the https://github.com/ClangBuiltLinux/linux/issues/1269 unreachable error fixed? Reproduce: git clone https://android.googlesource.com/kernel/common.git --branch android-4.19-stable cd common PATH=path/to/your/clang/bin:$PATH make -s -j 30 LLVM=1

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

MaskRay added a comment. LG. Can you attach an example triggering clang `InlineAsmDiagHandler2`? I want to check what the diagnostic looks like now. Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:22 def note_fe_inline_asm_here : Note<"instantiated into

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

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

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

MaskRay accepted this revision. MaskRay added inline comments. Comment at: llvm/include/llvm/MC/MCContext.h:67 class SourceMgr; + template class function_ref; With `std::function`, this can be dropped. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

MaskRay added a comment. I am supportive of getting rid of InlineAsmDiagnosticHandler, too. The updated AMDGPU tests suggeste that previously `MCContext::reportError` may be called with no `SrcMgr` or `InlineSrcMgr`, so the error is propagated to the temporary `SourceMgr()`. The

[Lldb-commits] [PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

MaskRay added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:477 + StringRef Message = D.getMessage(); + if (Message.startswith("error: ")) +Message = Message.substr(7); `StringRef::consume_front` I know you are moving code, but do you

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

MaskRay created this revision. MaskRay added reviewers: labath, rupprecht. Herald added a reviewer: shafik. MaskRay requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This fixes 6 check-lldb-shell failures in a `-DLLVM_USE_SANITIZER=Leaks`

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

MaskRay added a comment. In D100800#2699940 , @teemperor wrote: > Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot > for LLDB... > > I just have some minor comments but otherwise this LGTM. Agree.. The 45+ `check-lldb`

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

MaskRay added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229 if (!function_decl) { +char *buf = nullptr; llvm::StringRef name = attrs.name.GetStringRef(); teemperor wrote: > `name_buf` ?

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

MaskRay updated this revision to Diff 338664. MaskRay marked an inline comment as done. MaskRay added a comment. buf -> name_buf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100800/new/ https://reviews.llvm.org/D100800 Files:

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa2cd6d07691a: [lldb] Fix demangler leaks in the DWARF AST parser (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

MaskRay added a comment. The remaining failures are on Linux x86-64 in a `-DLLVM_USE_SANITIZER=Leaks` build (`Address` should achieve the same effect because on many targets asan enables LeakSanitizer) Failed Tests (45): lldb-shell :: Reproducer/Functionalities/TestDataFormatter.test

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcdae6d7711d6: [lldb] Fix one leak in reproducer (authored by MaskRay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

MaskRay added a comment. @JDevlieghere The Reproducer library seems to have several other leaks. Maybe you'd like to look at them? :) I am unfamiliar with the code... ==2013285==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 2 object(s) allocated from: #0

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

MaskRay updated this revision to Diff 338688. MaskRay marked an inline comment as done. MaskRay added a comment. remove a stale comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100806/new/ https://reviews.llvm.org/D100806 Files:

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

MaskRay created this revision. MaskRay added reviewers: JDevlieghere, rupprecht, shafik, teemperor. MaskRay requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Use a variable of static storage duration to reference an intentionally leaked

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

MaskRay marked an inline comment as done. MaskRay added inline comments. Comment at: lldb/tools/driver/Driver.cpp:856 if (const char *reproducer_path = SBReproducer::GetPath()) { -// Leaking the string on purpose. -std::string *finalize_cmd = new

[Lldb-commits] [PATCH] D100806: [lldb] Fix one leak in reproducer

MaskRay marked 2 inline comments as done. MaskRay added inline comments. Comment at: lldb/tools/driver/Driver.cpp:856 if (const char *reproducer_path = SBReproducer::GetPath()) { -// Leaking the string on purpose. -std::string *finalize_cmd = new

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

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.

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.

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) +

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

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

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

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.

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

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

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] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

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

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

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.

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

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

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

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;

[Lldb-commits] [PATCH] D106339: Add support to generate Sphinx DOCX documentation

MaskRay added a comment. I remember that we even converted some `*.md` to `*.rst` to remove the number of supported documentation formats, so I agree that adding support the even less used `*.docx` may not be the proper direction. Thanks for splitting this off. Repository: rG LLVM Github

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

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

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] [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

[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 `_ *

[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 .

[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

[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] 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] 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/

[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

[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

[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] 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] [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

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:

[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/

[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] 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

[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

[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: >

[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

[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

[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

[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

[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 > >

[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

[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:

[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] 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] 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: [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

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 =

[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

[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

[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

[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_") ||

[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=444707#toc

[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:

[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 , + int Level = DefaultCompression);

[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:

[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] 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

[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] 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] D136572: Harmonize cmake_policy() across standalone builds of all projects

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

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

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

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

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

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++

<    1   2   3   >