[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

tschuett added a comment. Could you offer higher abstractions? Show me the current SVME vector length? Show me the current SVME mode? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155269/new/ https://reviews.llvm.org/D155269 _

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

tschuett added a comment. I would never question giving low-level access to the registers. As you mentioned less experienced users could accidentally switch between the modes with knowing. Comment at: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_d

[Lldb-commits] [PATCH] D115211: [lldb] Make the LLDB version a first class citizen

tschuett added a comment. I believe the `Basic` library in Clang does not depend on other Clang libraries. It is a tail library. Do you envision a Basic, Utility, ... library that does not depend on Clang and provides basic utilities. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11521

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

tschuett added a comment. I found this: https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120836/new/ https://reviews.llvm.org/D120836

[Lldb-commits] [PATCH] D121443: [lldb] Add a getter for the process' system architecture

tschuett added inline comments. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp:236 +ArchSpec NativeProcessWindows::GetSystemArchitecture() override; +return HostInfo::GetArchitecture(); +} two spaces? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

tschuett added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h:27 +struct DWARFStrOffsetsInfo { + lldb::offset_t cu_offset; If you would use a class instead of the struct, you could hide the member variables. The class var

[Lldb-commits] [PATCH] D123226: [lldb] use one shared ThreadPool and task groups

tschuett added a comment. You cannot access/find the global thread pool? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123226/new/ https://reviews.llvm.org/D123226 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D123957: Update the developer policy to mention release notes

tschuett added a comment. LGTM and thanks. Feel free to ignore my comment. Maybe sort entries by importance. I have problems reading longish texts and documents. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 ___

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

tschuett added a comment. For AArch64 the default alignment is 0? I would have expected 128. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D143842: [lldb][test] Add check for Xcode binutils version to test-runner

tschuett added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:889 +toolchain. Please make sure the Xcode tools are before any +other tools in your path. Run `xcode -f strip` for the correct +toolchain path. Did you mean `xcrun -f strip

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added inline comments. Comment at: lldb/unittests/Target/RegisterFlagsTest.cpp:125 +} \ No newline at end of file Missing line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.l

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added inline comments. Comment at: lldb/include/lldb/Target/RegisterFlags.h:20 + public: +Field(const std::string &name, unsigned start, unsigned end, + const std::string &type) There are a lot of `const std::string& ` . Are you allowed to

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

tschuett added a comment. My only comment is: views look more modern than const-ref. The efficiency is not affected at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145566/new/ https://reviews.llvm.org/D145566 ___

[Lldb-commits] [PATCH] D149040: Refactor and generalize debugserver code for setting hardware watchpoints on AArch64

tschuett added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp:856 + // user_size == 16 -> aligned_size == 16 + aligned_size = 1ULL << (addr_bit_size - __builtin_clzll(aligned_size - 1)); + JDevlieghere wrote: > Beau

[Lldb-commits] [PATCH] D149987: ObjectFile: introduce a COFF object file plugin

tschuett added inline comments. Comment at: lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp:26 +namespace { +bool IsCOFFObjectFile(const DataBufferSP &data) { + return identify_magic(toStringRef(data->GetData())) == Could you turn this into a static inste

[Lldb-commits] [PATCH] D133623: Fix DW_OP_convert to resolve the CU relative offset correctly.

tschuett added inline comments. Comment at: lldb/source/Expression/DWARFExpression.cpp:2379 +// offset is compile unit relative so we need to fix it up. +die_offset += dwarf_cu->GetOffset(); // FIXME: the constness has annoying ripple effects. ---

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

tschuett added a comment. If I read your summary correctly, then `std::variant` would simplify your code. LLVM still uses `llvm::optional`. As there is no LLVM variant, I would go for `std::variant` . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

tschuett added a comment. Sorry. I hoped that variant would make your life easier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134041/new/ https://reviews.llvm.org/D134041 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

tschuett added inline comments. Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_loongarch64.cpp:43 +uint32_t RegisterInfoPOSIX_loongarch64::GetRegisterCount() const { + return 0; +} Stupid question: why is the register count 0? =

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

tschuett added a comment. How about an optional instead, if GetPlatformString is fallible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136719/new/ https://reviews.llvm.org/D136719 ___ lldb-commits mail

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

tschuett added a comment. In D136719#3885974 , @jasonmolenda wrote: > In D136719#3884836 , @tschuett > wrote: > >> How about an optional instead, if GetPlatformString is fallible. > > Ah, interesting idea. debu

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

tschuett added a comment. Looks great, but I would have expected `std::optional` and a `nullopt`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136719/new/ https://reviews.llvm.org/D136719 ___ lldb-comm

[Lldb-commits] [PATCH] D136719: change debugserver to return an empty platform name for unknown platforms, instead of crashing

tschuett added inline comments. Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:6264 major_version, minor_version, patch_version); -if (platform) { +if (platform.has_value()) { os_handled = true; ---

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

tschuett added inline comments. Comment at: lldb/include/lldb/Core/Module.h:817 + void ForEachTypeSystem(std::function const &callback); + Maybe `llvm::function_ref`? https://llvm.org/doxygen/classllvm_1_1function__ref_3_01Ret_07Params_8_8_8_08_4.html Reposi

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

tschuett added inline comments. Comment at: lldb/source/Target/Statistics.cpp:266 +auto stats = ts->ReportStatistics(); +if (stats.hasValue()) { + module_stat_obj->try_emplace("TypeSystemInfo", stats.getValue()); clayborg wrote: > Remove

[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

tschuett added inline comments. Comment at: lldb/include/lldb/Core/Module.h:818 + void ForEachTypeSystem(llvm::function_ref const &callback); + Nit: the `const &`is redundant. It is a function reference. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

tschuett added a comment. https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138376/new/ https://reviews.llvm.org/D138376 __

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe `void FormatDiagnostic(SmallVectorImpl &OutStr, DiagnosticMode mode)`instead of `void FormatDiagnostic(SmallVectorImpl &OutStr)`? To make the transition easer and future proof. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing: enum class DiagnosticMode { Legacy, UserOriented, Default = Legacy } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the output device (console/sarif) are orthogonal issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939 __

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. I do not ask you to do anything! I just noticed that you add a lot of `FormatXXXDiagnostic` functions. An alternativ design is to have one `FormatDiagnostic` function with a mode parameter. Then you can decide whether to print legacy or user-oriented reasons. If next

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

tschuett added a comment. auto_ptr is deprecated since C++11 and will be removed in C++17. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85396/new/ https://reviews.llvm.org/D85396 ___ lldb-commits mailin

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

tschuett added a comment. And you can write it as std::unique_ptr synthetic_children = std::make_unique(...); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85396/new/ https://reviews.llvm.org/D85396

[Lldb-commits] [PATCH] D82064: [ARM64] Add QEMU testing environment setup guide for SVE testing

tschuett added a comment. You can buy a PRIMEHPC FX700 from Fujitsu. It has a A64FX CPU with SVE. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82064/new/ https://reviews.llvm.org/D82064 ___ lldb-commits mailing list lldb-commits@lists.llvm.o

[Lldb-commits] [PATCH] D94489: [lldb][docs] Use sphinx instead of epydoc to generate LLDB's Python reference

tschuett added a comment. There is no syntax highlighting for Python in sphinx? The code in https://lldb.llvm.org/python_api/lldb.SBDebugger.html#lldb.SBDebugger is hard to read. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94489/new/ https://revi

[Lldb-commits] [PATCH] D94489: [lldb][docs] Use sphinx instead of epydoc to generate LLDB's Python reference

tschuett added a comment. In D94489#2503685 , @teemperor wrote: > In D94489#2503662 , @tschuett wrote: > >> There is no syntax highlighting for Python in sphinx? The code in >> https://lldb.llvm.org/python_api/lldb.

[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

tschuett added a comment. You could try: clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 ___ lldb-commits mailing list lldb-commits