[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -0,0 +1,31 @@ +//===-- SBStatisticsOptions.h ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_API_SBSTATISTICSOPTIONS_H +#define LLDB_API_SBSTATISTICSOPTIONS_H + +#include "lldb/API/SBDefines.h" + +namespace lldb { + +class LLDB_API SBStatisticsOptions { clayborg wrote: Would be good to get some header documentation on this simple class and explain which function it is for. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -75,6 +75,9 @@ class CommandObjectStatsDump : public CommandObjectParsed { case 'a': m_all_targets = true; break; + case 's': +m_summary_only = true; clayborg wrote: See comment about storing a different ivar below, then this becomes: ``` m_stats_options.summary_only = true; ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -198,16 +198,24 @@ SBDebugger SBTarget::GetDebugger() const { } SBStructuredData SBTarget::GetStatistics() { + LLDB_INSTRUMENT_VA(this); + SBStatisticsOptions options; + options.SetSummaryOnly(false); + return GetStatistics(options); +} + +SBStructuredData SBTarget::GetStatistics(SBStatisticsOptions options) { LLDB_INSTRUMENT_VA(this); SBStructuredData data; TargetSP target_sp(GetSP()); if (!target_sp) return data; std::string json_str = - llvm::formatv("{0:2}", - DebuggerStats::ReportStatistics(target_sp->GetDebugger(), - target_sp.get())).str(); + llvm::formatv( + "{0:2}", DebuggerStats::ReportStatistics( + target_sp->GetDebugger(), target_sp.get(), options.GetStatisticsOptions())) clayborg wrote: ``` target_sp->GetDebugger(), target_sp.get(), options.ref())) ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -83,13 +86,21 @@ class CommandObjectStatsDump : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_all_targets = false; + m_summary_only = false; } llvm::ArrayRef GetDefinitions() override { return llvm::ArrayRef(g_statistics_dump_options); } +StatisticsOptions GetStatisticsOptions() { + StatisticsOptions options; + options.summary_only = m_summary_only; + return options; clayborg wrote: See my ivar comment below, then this becomes: ``` return m_stats_options; ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -0,0 +1,31 @@ +//===-- SBStatisticsOptions.h ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_API_SBSTATISTICSOPTIONS_H +#define LLDB_API_SBSTATISTICSOPTIONS_H + +#include "lldb/API/SBDefines.h" + +namespace lldb { + +class LLDB_API SBStatisticsOptions { +public: + SBStatisticsOptions(); + SBStatisticsOptions(const lldb::SBStatisticsOptions &rhs); + ~SBStatisticsOptions(); + + const SBStatisticsOptions &operator=(const lldb::SBStatisticsOptions &rhs); + + void SetSummaryOnly(bool b); + lldb_private::StatisticsOptions GetStatisticsOptions(); clayborg wrote: We don't want this function to be public, it should be protected, and not return an instance of `lldb_private::StatisticsOptions`, cause if we do this, then we need to include the header file that defines this data structure and it should be opaque. In a class like SBAddress, we do something like this: ``` class SBStatisticsOptions { ... protected: friend class SBTarget; const lldb_private::StatisticsOptions &ref() const; } ``` If this is a `lldb_private::StatisticsOptions &`, then we don't need to include the header file that defines this structure in this file since the size of a references is just a pointer size. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -0,0 +1,46 @@ +//===-- SBStatisticsOptions.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/API/SBStatisticsOptions.h" +#include "lldb/Target/Statistics.h" +#include "lldb/Utility/Instrumentation.h" + +#include "Utils.h" + +using namespace lldb; +using namespace lldb_private; + +SBStatisticsOptions::SBStatisticsOptions() +: m_opaque_up(new StatisticsOptions()) { + LLDB_INSTRUMENT_VA(this); +} + +SBStatisticsOptions::SBStatisticsOptions(const SBStatisticsOptions &rhs) { + LLDB_INSTRUMENT_VA(this, rhs); + + m_opaque_up = clone(rhs.m_opaque_up); +} + +SBStatisticsOptions::~SBStatisticsOptions() = default; + +const SBStatisticsOptions & +SBStatisticsOptions::operator=(const SBStatisticsOptions &rhs) { + LLDB_INSTRUMENT_VA(this, rhs); + + if (this != &rhs) +m_opaque_up = clone(rhs.m_opaque_up); + return *this; +} + +void SBStatisticsOptions::SetSummaryOnly(bool b) { + m_opaque_up->summary_only = b; +} + +lldb_private::StatisticsOptions SBStatisticsOptions::GetStatisticsOptions() { + return *m_opaque_up; +} clayborg wrote: ``` const lldb_private::StatisticsOptions *SBStatisticsOptions::ref() { return *m_opaque_up; } ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -83,13 +86,21 @@ class CommandObjectStatsDump : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_all_targets = false; + m_summary_only = false; } llvm::ArrayRef GetDefinitions() override { return llvm::ArrayRef(g_statistics_dump_options); } +StatisticsOptions GetStatisticsOptions() { + StatisticsOptions options; + options.summary_only = m_summary_only; + return options; +} + bool m_all_targets = false; +bool m_summary_only = false; clayborg wrote: Change this ivar from: ``` bool summary_only = false; ``` to: ``` lldb_private::StatisticsOptions m_stats_options; ``` And fill things into this struct when parsing the options. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -100,60 +101,91 @@ llvm::json::Value ConstStringStats::ToJSON() const { return obj; } -json::Value TargetStats::ToJSON(Target &target) { - CollectStats(target); +json::Value TargetStats::ToJSON(Target &target, bool summary_only) { clayborg wrote: This function seems to still take a "bool summary_only"? https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -198,16 +198,24 @@ SBDebugger SBTarget::GetDebugger() const { } SBStructuredData SBTarget::GetStatistics() { + LLDB_INSTRUMENT_VA(this); + SBStatisticsOptions options; + options.SetSummaryOnly(false); clayborg wrote: This defaults to false right? I would make sure the default `SBStatisticsOptions` constructor sets everything to good defaults and we can remove this line. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -130,10 +130,14 @@ struct ConstStringStats { ConstString::MemoryStats stats = ConstString::GetMemoryStats(); }; +struct StatisticsOptions { + bool summary_only = false; +}; + /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: - llvm::json::Value ToJSON(Target &target); + llvm::json::Value ToJSON(Target &target, bool summary_only = false); clayborg wrote: We should still pass in a "const StatisticsOptions options = StatisticsOptions()" instead of a boolean here. If we add more options in the future, we should avoid changing this API that way. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
https://github.com/jasonmolenda approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/80786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
chelcassanova wrote: I did, I'm adding them as separate patches. DiagnosticEventTest.cpp could probably go here since it's a simple change but ProgressEventTest doesn't exist upstream yet since I had to revert so I think that at least should be it's own PR> https://github.com/llvm/llvm-project/pull/80786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Use shared once_flag in DiagnosticEventTest (PR #80788)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) Changes Incorporates the changes from https://github.com/llvm/llvm-project/pull/80786 to use a once_flag from `TestUtilities` instead of a local flag in order to prevent hitting an assertion that the debugger was initialized again in another test. --- Full diff: https://github.com/llvm/llvm-project/pull/80788.diff 1 Files Affected: - (modified) lldb/unittests/Core/DiagnosticEventTest.cpp (+2-3) ``diff diff --git a/lldb/unittests/Core/DiagnosticEventTest.cpp b/lldb/unittests/Core/DiagnosticEventTest.cpp index bca8f3789955a..98ea7ca8d0a88 100644 --- a/lldb/unittests/Core/DiagnosticEventTest.cpp +++ b/lldb/unittests/Core/DiagnosticEventTest.cpp @@ -11,6 +11,7 @@ #include "Plugins/Platform/MacOSX/PlatformMacOSX.h" #include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" #include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" #include "lldb/Core/Debugger.h" #include "lldb/Core/DebuggerEvents.h" #include "lldb/Host/FileSystem.h" @@ -26,8 +27,6 @@ using namespace lldb_private::repro; static const constexpr std::chrono::seconds TIMEOUT(0); static const constexpr size_t DEBUGGERS = 3; -static std::once_flag debugger_initialize_flag; - namespace { class DiagnosticEventTest : public ::testing::Test { public: @@ -35,7 +34,7 @@ class DiagnosticEventTest : public ::testing::Test { FileSystem::Initialize(); HostInfo::Initialize(); PlatformMacOSX::Initialize(); -std::call_once(debugger_initialize_flag, +std::call_once(TestUtilities::debugger_initialize_flag, []() { Debugger::Initialize(nullptr); }); ArchSpec arch("x86_64-apple-macosx-"); Platform::SetHostPlatform( `` https://github.com/llvm/llvm-project/pull/80788 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Use shared once_flag in DiagnosticEventTest (PR #80788)
https://github.com/chelcassanova created https://github.com/llvm/llvm-project/pull/80788 Incorporates the changes from https://github.com/llvm/llvm-project/pull/80786 to use a once_flag from `TestUtilities` instead of a local flag in order to prevent hitting an assertion that the debugger was initialized again in another test. >From 728d726a8ff5fcb90772fd45c8d6b1e897c228c5 Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Mon, 5 Feb 2024 18:50:40 -0800 Subject: [PATCH] [lldb][unittest] Use shared once_flag in DiagnosticEventTest Incorporates the changes from https://github.com/llvm/llvm-project/pull/80786 to use a once_flag from `TestUtilities` instead of a local flag in order to prevent hitting an assertion that the debugger was initialized again in another test. --- lldb/unittests/Core/DiagnosticEventTest.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/unittests/Core/DiagnosticEventTest.cpp b/lldb/unittests/Core/DiagnosticEventTest.cpp index bca8f3789955a..98ea7ca8d0a88 100644 --- a/lldb/unittests/Core/DiagnosticEventTest.cpp +++ b/lldb/unittests/Core/DiagnosticEventTest.cpp @@ -11,6 +11,7 @@ #include "Plugins/Platform/MacOSX/PlatformMacOSX.h" #include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" #include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" #include "lldb/Core/Debugger.h" #include "lldb/Core/DebuggerEvents.h" #include "lldb/Host/FileSystem.h" @@ -26,8 +27,6 @@ using namespace lldb_private::repro; static const constexpr std::chrono::seconds TIMEOUT(0); static const constexpr size_t DEBUGGERS = 3; -static std::once_flag debugger_initialize_flag; - namespace { class DiagnosticEventTest : public ::testing::Test { public: @@ -35,7 +34,7 @@ class DiagnosticEventTest : public ::testing::Test { FileSystem::Initialize(); HostInfo::Initialize(); PlatformMacOSX::Initialize(); -std::call_once(debugger_initialize_flag, +std::call_once(TestUtilities::debugger_initialize_flag, []() { Debugger::Initialize(nullptr); }); ArchSpec arch("x86_64-apple-macosx-"); Platform::SetHostPlatform( ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -83,13 +86,15 @@ class CommandObjectStatsDump : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_all_targets = false; + m_summary_only = false; } llvm::ArrayRef GetDefinitions() override { return llvm::ArrayRef(g_statistics_dump_options); } bool m_all_targets = false; +bool m_summary_only = false; kusmour wrote: > We should probably also move `m_all_targets` into > `lldb_private::StatisticsOptions` I can do that but notice the `m_all_targets` was not used anywhere outside of the command object. The target is decided with the current execution context. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
jasonmolenda wrote: Did you mean to include changes to DiagnosticEventTest.cpp and ProgressReportTest.cpp to use this new global? https://github.com/llvm/llvm-project/pull/80786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 0d091206dd656c2a9d31d6088a4aa6f9c2cc7156 2fa99391c097def91369c164195ad2a4a1cd98ed -- lldb/unittests/TestingSupport/TestUtilities.cpp lldb/unittests/TestingSupport/TestUtilities.h `` View the diff from clang-format here. ``diff diff --git a/lldb/unittests/TestingSupport/TestUtilities.h b/lldb/unittests/TestingSupport/TestUtilities.h index 99e569bfa5..636be860dc 100644 --- a/lldb/unittests/TestingSupport/TestUtilities.h +++ b/lldb/unittests/TestingSupport/TestUtilities.h @@ -32,8 +32,8 @@ namespace lldb_private { std::string GetInputFilePath(const llvm::Twine &name); class TestUtilities { - public: -static std::once_flag debugger_initialize_flag; +public: + static std::once_flag debugger_initialize_flag; }; class TestFile { `` https://github.com/llvm/llvm-project/pull/80786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add FreeBSD kernel coredump matching check. (PR #80785)
https://github.com/aokblast updated https://github.com/llvm/llvm-project/pull/80785 >From 5f9eac19a45cd4b71afe48643fc0cf8b4b4ab6be Mon Sep 17 00:00:00 2001 From: aokblast Date: Tue, 6 Feb 2024 10:18:34 +0800 Subject: [PATCH 1/2] [LLDB] Fetch UUID from note section in coredump --- .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 29 +++ .../Plugins/ObjectFile/ELF/ObjectFileELF.h| 3 ++ 2 files changed, 32 insertions(+) diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp index 0d95a1c12bde3..3b047e3d5c759 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -795,6 +795,32 @@ bool ObjectFileELF::ParseHeader() { return m_header.Parse(m_data, &offset); } +UUID ObjectFileELF::GetUUIDByNoteSection() { + if (!ParseProgramHeaders()) +return UUID(); + for (const ELFProgramHeader &H : m_program_headers) { +if (H.p_type == llvm::ELF::PT_NOTE) { + const elf_off ph_offset = H.p_offset; + const size_t ph_size = H.p_filesz; + + DataExtractor segment_data; + if (segment_data.SetData(m_data, ph_offset, ph_size) != ph_size) { +// The ELF program header contained incorrect data, probably corefile +// is incomplete or corrupted. +break; + } + + UUID uuid; + ArchSpec arch; + + if (RefineModuleDetailsFromNote(segment_data, arch, uuid).Success()) +return uuid; +} + } + + return UUID(); +} + UUID ObjectFileELF::GetUUID() { // Need to parse the section list to get the UUIDs, so make sure that's been // done. @@ -809,6 +835,9 @@ UUID ObjectFileELF::GetUUID() { if (!ParseProgramHeaders()) return UUID(); + if ((m_uuid = GetUUIDByNoteSection()).IsValid()) +return m_uuid; + core_notes_crc = CalculateELFNotesSegmentsCRC32(m_program_headers, m_data); diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h index bc8e34981a9d8..6954bd96b753a 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h @@ -272,6 +272,9 @@ class ObjectFileELF : public lldb_private::ObjectFile { uint32_t &gnu_debuglink_crc, lldb_private::ArchSpec &arch_spec); + // Use Note Section to get uuid. + lldb_private::UUID GetUUIDByNoteSection(); + /// Scans the dynamic section and locates all dependent modules (shared /// libraries) populating m_filespec_up. This method will compute the /// dependent module list only once. Returns the number of dependent >From 659d3446fd975fcafe997d63983830e5c45b41bc Mon Sep 17 00:00:00 2001 From: aokblast Date: Tue, 6 Feb 2024 10:48:05 +0800 Subject: [PATCH 2/2] Add match check between binary and coredump of FreeBSD kernel --- .../DynamicLoaderFreeBSDKernel.cpp| 32 +++ .../DynamicLoaderFreeBSDKernel.h | 3 +- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp index e504e6cbf6921..1a7e811ac1233 100644 --- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp @@ -153,9 +153,10 @@ addr_t DynamicLoaderFreeBSDKernel::FindKernelAtLoadAddress( } // Read ELF header from memry and return +template bool DynamicLoaderFreeBSDKernel::ReadELFHeader(Process *process, lldb::addr_t addr, - llvm::ELF::Elf32_Ehdr &header, + Elf_Ehdr &header, bool *read_error) { Status error; if (read_error) @@ -200,8 +201,19 @@ lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress( if (header.e_type != llvm::ELF::ET_EXEC) return UUID(); - ModuleSP memory_module_sp = - process->ReadModuleFromMemory(FileSpec("temp_freebsd_kernel"), addr); + ArchSpec kernel_arch(llvm::ELF::convertEMachineToArchName(header.e_machine)); + + ModuleSP memory_module_sp = process->ReadModuleFromMemory( + FileSpec("temp_freebsd_kernel"), addr, header.e_shoff); + if (header.e_ident[llvm::ELF::EI_CLASS] == llvm::ELF::ELFCLASS64) { +llvm::ELF::Elf64_Ehdr header; +if (!ReadELFHeader(process, addr, header)) { + *read_error = true; + return UUID(); +} +memory_module_sp = process->ReadModuleFromMemory( +FileSpec("temp_freebsd_kernel"), addr, header.e_shoff); + } if (!memory_module_sp.get()) { *read_error = true; @@ -218,10 +230,16 @@ lldb_private::UUID
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) Changes I tried adding a new unit test to the core test suite (https://github.com/llvm/llvm-project/pull/79533) but it broke the test suite on AArch64 Linux due to hitting an assertion for calling `Debugger::Initialize` more than once. When the unit test suite is invoked as a standalone binary the test suite state is shared, and `Debugger::Initialize` gets called in `DiagnosticEventTest.cpp` before being called in `ProgressReportTest.cpp`. `DiagnosticEventTest.cpp` uses a call_once flag to initialize the debugger but it's local to that test. This commit adds a once_flag to `TestUtilities` so that `Debugger::Initialize` can be called once by the tests that use it. --- Full diff: https://github.com/llvm/llvm-project/pull/80786.diff 2 Files Affected: - (modified) lldb/unittests/TestingSupport/TestUtilities.cpp (+1) - (modified) lldb/unittests/TestingSupport/TestUtilities.h (+6-1) ``diff diff --git a/lldb/unittests/TestingSupport/TestUtilities.cpp b/lldb/unittests/TestingSupport/TestUtilities.cpp index 9e5523e487547..507241404c08d 100644 --- a/lldb/unittests/TestingSupport/TestUtilities.cpp +++ b/lldb/unittests/TestingSupport/TestUtilities.cpp @@ -19,6 +19,7 @@ using namespace lldb_private; extern const char *TestMainArgv0; +std::once_flag TestUtilities::debugger_initialize_flag; std::string lldb_private::GetInputFilePath(const llvm::Twine &name) { llvm::SmallString<128> result = llvm::sys::path::parent_path(TestMainArgv0); llvm::sys::fs::make_absolute(result); diff --git a/lldb/unittests/TestingSupport/TestUtilities.h b/lldb/unittests/TestingSupport/TestUtilities.h index 811c4c1521269..99e569bfa5250 100644 --- a/lldb/unittests/TestingSupport/TestUtilities.h +++ b/lldb/unittests/TestingSupport/TestUtilities.h @@ -31,6 +31,11 @@ namespace lldb_private { std::string GetInputFilePath(const llvm::Twine &name); +class TestUtilities { + public: +static std::once_flag debugger_initialize_flag; +}; + class TestFile { public: static llvm::Expected fromYaml(llvm::StringRef Yaml); @@ -51,6 +56,6 @@ class TestFile { std::string Buffer; }; -} +} // namespace lldb_private #endif `` https://github.com/llvm/llvm-project/pull/80786 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][unittest] Add call_once flag to initialize debugger (PR #80786)
https://github.com/chelcassanova created https://github.com/llvm/llvm-project/pull/80786 I tried adding a new unit test to the core test suite (https://github.com/llvm/llvm-project/pull/79533) but it broke the test suite on AArch64 Linux due to hitting an assertion for calling `Debugger::Initialize` more than once. When the unit test suite is invoked as a standalone binary the test suite state is shared, and `Debugger::Initialize` gets called in `DiagnosticEventTest.cpp` before being called in `ProgressReportTest.cpp`. `DiagnosticEventTest.cpp` uses a call_once flag to initialize the debugger but it's local to that test. This commit adds a once_flag to `TestUtilities` so that `Debugger::Initialize` can be called once by the tests that use it. >From 2fa99391c097def91369c164195ad2a4a1cd98ed Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Mon, 5 Feb 2024 18:41:14 -0800 Subject: [PATCH] [lldb][unittest] Add call_once flag to initialize debugger I tried adding a new unit test to the core test suite (https://github.com/llvm/llvm-project/pull/79533) but it broke the test suite on AArch64 Linux due to hitting an assertion for calling `Debugger::Initialize` more than once. When the unit test suite is invoked as a standalone binary the test suite state is shared, and `Debugger::Initialize` gets called in `DiagnosticEventTest.cpp` before being called in `ProgressReportTest.cpp`. `DiagnosticEventTest.cpp` uses a call_once flag to initialize the debugger but it's local to that test. This commit adds a once_flag to `TestUtilities` so that `Debugger::Initialize` can be called once by the tests that use it. --- lldb/unittests/TestingSupport/TestUtilities.cpp | 1 + lldb/unittests/TestingSupport/TestUtilities.h | 7 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lldb/unittests/TestingSupport/TestUtilities.cpp b/lldb/unittests/TestingSupport/TestUtilities.cpp index 9e5523e4875479..507241404c08d8 100644 --- a/lldb/unittests/TestingSupport/TestUtilities.cpp +++ b/lldb/unittests/TestingSupport/TestUtilities.cpp @@ -19,6 +19,7 @@ using namespace lldb_private; extern const char *TestMainArgv0; +std::once_flag TestUtilities::debugger_initialize_flag; std::string lldb_private::GetInputFilePath(const llvm::Twine &name) { llvm::SmallString<128> result = llvm::sys::path::parent_path(TestMainArgv0); llvm::sys::fs::make_absolute(result); diff --git a/lldb/unittests/TestingSupport/TestUtilities.h b/lldb/unittests/TestingSupport/TestUtilities.h index 811c4c1521269b..99e569bfa52507 100644 --- a/lldb/unittests/TestingSupport/TestUtilities.h +++ b/lldb/unittests/TestingSupport/TestUtilities.h @@ -31,6 +31,11 @@ namespace lldb_private { std::string GetInputFilePath(const llvm::Twine &name); +class TestUtilities { + public: +static std::once_flag debugger_initialize_flag; +}; + class TestFile { public: static llvm::Expected fromYaml(llvm::StringRef Yaml); @@ -51,6 +56,6 @@ class TestFile { std::string Buffer; }; -} +} // namespace lldb_private #endif ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add FreeBSD kernel coredump matching check. (PR #80785)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 78b4e7c5e349d8c101b50affbd260eb109748f8f e506f292dd07bcdd26a668d9fcf87a258864db95 -- lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp index 6a7618ee59..1a7e811ac1 100644 --- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp @@ -230,8 +230,8 @@ lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress( return UUID(); } - // Because the memory module is read from memory and in the memory, the type is eTypeExecutable - // so we have to assign the type manually + // Because the memory module is read from memory and in the memory, the type + // is eTypeExecutable so we have to assign the type manually memory_module_sp->GetObjectFile()->SetType(ObjectFile::eTypeCoreFile); if (memory_module_sp->GetUUID() != `` https://github.com/llvm/llvm-project/pull/80785 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/80767 >From e7af15ff3c1bdd42799b25f76145b8a19739eea5 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 5 Feb 2024 15:58:23 -0800 Subject: [PATCH 1/3] Add a new SBProcess:: GetCoreFile() API --- lldb/include/lldb/API/SBProcess.h | 7 +++ lldb/include/lldb/Target/PostMortemProcess.h | 9 lldb/include/lldb/Target/Process.h| 7 +++ lldb/include/lldb/Target/ProcessTrace.h | 3 ++- lldb/source/API/SBProcess.cpp | 11 ++ .../FreeBSDKernel/ProcessFreeBSDKernel.cpp| 21 +++ .../FreeBSDKernel/ProcessFreeBSDKernel.h | 3 ++- .../Process/elf-core/ProcessElfCore.cpp | 8 +++ .../Plugins/Process/elf-core/ProcessElfCore.h | 1 - .../Process/mach-core/ProcessMachCore.cpp | 6 +++--- .../Process/mach-core/ProcessMachCore.h | 1 - .../Process/minidump/ProcessMinidump.cpp | 2 +- .../Process/minidump/ProcessMinidump.h| 1 - lldb/source/Target/ProcessTrace.cpp | 7 --- .../postmortem/elf-core/TestLinuxCore.py | 11 ++ 15 files changed, 73 insertions(+), 25 deletions(-) diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 8c1c81418f83d..13994ed300853 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + /// Allocate memory within the process. /// /// This function will allocate memory in the process's address space. diff --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h index 7207fc99ef29a..9c9cd7fa599bc 100644 --- a/lldb/include/lldb/Target/PostMortemProcess.h +++ b/lldb/include/lldb/Target/PostMortemProcess.h @@ -24,7 +24,16 @@ class PostMortemProcess : public Process { using Process::Process; public: + PostMortemProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, +const FileSpec &core_file) + : Process(target_sp, listener_sp), m_core_file(core_file) {} + bool IsLiveDebugSession() const override { return false; } + + FileSpec GetCoreFile() const override { return m_core_file; } + +protected: + FileSpec m_core_file; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7048921d6034f..0ad626ffd3613 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1503,6 +1503,13 @@ class Process : public std::enable_shared_from_this, virtual bool IsLiveDebugSession() const { return true; }; + /// Provide a way to retrieve the core dump file that is loaded for debugging. + /// Only available if IsLiveDebugSession() returns true. + /// + /// \return + /// File path to the core file. + virtual FileSpec GetCoreFile() const { return {}; } + /// Before lldb detaches from a process, it warns the user that they are /// about to lose their debug session. In some cases, this warning doesn't /// need to be emitted -- for instance, with core file debugging where the diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h index 037dea232cc02..7a025100f6803 100644 --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -27,7 +27,8 @@ class ProcessTrace : public PostMortemProcess { static llvm::StringRef GetPluginDescriptionStatic(); - ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); + ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, + const FileSpec &core_file); ~ProcessTrace() override; diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 4864ea0e7d026..a9fe915324683 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(core_file); +} + lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions, lldb::SBError &sb_error) { LLDB_INSTRUMENT_VA(this, size, permissions, sb_error); diff --git a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp index 601f5df43dbba..997b590851103 100644 --- a/lldb/source/Plugins/P
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/80767 >From e7af15ff3c1bdd42799b25f76145b8a19739eea5 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 5 Feb 2024 15:58:23 -0800 Subject: [PATCH 1/2] Add a new SBProcess:: GetCoreFile() API --- lldb/include/lldb/API/SBProcess.h | 7 +++ lldb/include/lldb/Target/PostMortemProcess.h | 9 lldb/include/lldb/Target/Process.h| 7 +++ lldb/include/lldb/Target/ProcessTrace.h | 3 ++- lldb/source/API/SBProcess.cpp | 11 ++ .../FreeBSDKernel/ProcessFreeBSDKernel.cpp| 21 +++ .../FreeBSDKernel/ProcessFreeBSDKernel.h | 3 ++- .../Process/elf-core/ProcessElfCore.cpp | 8 +++ .../Plugins/Process/elf-core/ProcessElfCore.h | 1 - .../Process/mach-core/ProcessMachCore.cpp | 6 +++--- .../Process/mach-core/ProcessMachCore.h | 1 - .../Process/minidump/ProcessMinidump.cpp | 2 +- .../Process/minidump/ProcessMinidump.h| 1 - lldb/source/Target/ProcessTrace.cpp | 7 --- .../postmortem/elf-core/TestLinuxCore.py | 11 ++ 15 files changed, 73 insertions(+), 25 deletions(-) diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 8c1c81418f83d1..13994ed300853e 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + /// Allocate memory within the process. /// /// This function will allocate memory in the process's address space. diff --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h index 7207fc99ef29a4..9c9cd7fa599bc3 100644 --- a/lldb/include/lldb/Target/PostMortemProcess.h +++ b/lldb/include/lldb/Target/PostMortemProcess.h @@ -24,7 +24,16 @@ class PostMortemProcess : public Process { using Process::Process; public: + PostMortemProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, +const FileSpec &core_file) + : Process(target_sp, listener_sp), m_core_file(core_file) {} + bool IsLiveDebugSession() const override { return false; } + + FileSpec GetCoreFile() const override { return m_core_file; } + +protected: + FileSpec m_core_file; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7048921d6034fd..0ad626ffd3613c 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1503,6 +1503,13 @@ class Process : public std::enable_shared_from_this, virtual bool IsLiveDebugSession() const { return true; }; + /// Provide a way to retrieve the core dump file that is loaded for debugging. + /// Only available if IsLiveDebugSession() returns true. + /// + /// \return + /// File path to the core file. + virtual FileSpec GetCoreFile() const { return {}; } + /// Before lldb detaches from a process, it warns the user that they are /// about to lose their debug session. In some cases, this warning doesn't /// need to be emitted -- for instance, with core file debugging where the diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h index 037dea232cc024..7a025100f68032 100644 --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -27,7 +27,8 @@ class ProcessTrace : public PostMortemProcess { static llvm::StringRef GetPluginDescriptionStatic(); - ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); + ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, + const FileSpec &core_file); ~ProcessTrace() override; diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 4864ea0e7d0268..a9fe915324683e 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(core_file); +} + lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions, lldb::SBError &sb_error) { LLDB_INSTRUMENT_VA(this, size, permissions, sb_error); diff --git a/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp b/lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp index 601f5df43dbba4..997b5908511032 100644 --- a/lldb/sour
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
jeffreytan81 wrote: @bulbazord, the scenario is that user directly uses "lldb -c " to load core file, then the python script is invoked later to fetch metadata, resolve binaries/symbols etc... https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + jeffreytan81 wrote: @clayborg, I agree with what @jasonmolenda says here. If you look at the existing implementation, the core file passed in is not stored in Target object but inside the created process object. `SBTarget::GetCoreFile()` would require us redundantly store the core file path inside target. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(core_file); +} + lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions, jasonmolenda wrote: My thinking: this is like asking for the PID of alive process. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
jasonmolenda wrote: This seems like a useful addition to me, I've had some users of corefiles who have asked for an SBProcess::IsCore() type of method. (and others asking for ways to access metadata from the corefile itself via the Process, although I've never thought about how to do that, maybe some SBStructuredData thing because there are so many different forms of metadata that Process plugins might have available) https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(core_file); +} + lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions, jasonmolenda wrote: We pass the filepath to `SBTarget::LoadCore` but I don't think this is a property of the Target, it seems like a property of the Process to me. I suppose someone could create a Process with a core file, then close that Process, then create a new Process with a different core file, all in the same Target if the ArchSpecs were the same. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + jasonmolenda wrote: It seems correct to get this from the Process, because it's a property of the process. Before/after the Process exists, there is no core file FileSpec to return. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [clang] [compiler-rt] [libcxx] [clang-tools-extra] [mlir] [llvm] [openmp] [libc] [flang] [lldb] [Driver] Report invalid target triple versions for all environment types. (PR #7865
ZijunZhaoCCK wrote: > > > There's apparently also wasm32-wasi-preview2 and wasm32-wasi-pthread, > > > which I suppose are equally broken by this change. > > > > > > Yes, I think so. I think adding these environment types in wasi-libc repo > > could help fix those errors. > > If wasm can arbitrary environment types, it seems that we can opt out the > check for `isWasm()`. We define new environment types when they affect the > compiler behavior. If wasm arbitrary sets the environment, we should not > define a type for each one. Okay, I will add this check. https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][libc++] Adds system_clock data formatters. (PR #78609)
jasonmolenda wrote: Agreed, if I change this to ``` diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp index d0bdbe1fd4d..b37544f6dd3 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -1105,7 +1105,7 @@ bool lldb_private::formatters::LibcxxChronoSysSecondsSummaryProvider( const std::time_t seconds = ptr_sp->GetValueAsSigned(0); if (seconds < chrono_timestamp_min || seconds > chrono_timestamp_max) -stream.Printf("timestamp=%" PRIu64 " s", static_cast(seconds)); +stream.Printf("timestamp=%" PRId64 " s", static_cast(seconds)); else { std::array str; std::size_t size = @@ -1113,8 +1113,8 @@ bool lldb_private::formatters::LibcxxChronoSysSecondsSummaryProvider( if (size == 0) return false; -stream.Printf("date/time=%s timestamp=%" PRIu64 " s", str.data(), - static_cast(seconds)); +stream.Printf("date/time=%s timestamp=%" PRId64 " s", str.data(), + static_cast(seconds)); } return true; ``` the tests pass on Darwin. On Darwin `time_t` is `long`, a signed value - and given that this can represent negative times, that makes sense. The test is checking that PRIu64 prints a negative value but we're casting this time_t to a uint64_t, I don't understand why this would print as a negative value on Linux (and pass the tests). https://github.com/llvm/llvm-project/pull/78609 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -404,7 +404,7 @@ bool ProcessMachCore::LoadBinariesViaMetadata() { // LoadCoreFileImges may have set the dynamic loader, e.g. in // PlatformDarwinKernel::LoadPlatformBinaryAndSetup(). - // If we now have a dynamic loader, save its name so we don't + // If we now have a dynamic loader, save its name so we don't clayborg wrote: revert whitespace only change https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(core_file); +} + lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions, clayborg wrote: move to SBTarget for consistency at the public API layer since we load the core file in SBTarget. If others believe that this should stay in SBProcess, let me know as I can go with what makes most sense, but the target seems the right location as I am thinking about it right now. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -261,7 +261,7 @@ Status ProcessElfCore::DoLoadCore() { exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path, FileSpec::Style::native); if (exe_module_spec.GetFileSpec()) { -exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, +exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, clayborg wrote: revert whitespace only change https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + clayborg wrote: We load the core file in SBTarget. I would move this into SBTarget.h/.cpp https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. clayborg wrote: ``` /// Get the file specification for the core file that is currently being used /// for the process. If the process is not loaded from a core file, then an /// invalid file specification will be returned. ``` https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -865,7 +865,7 @@ llvm::Error ProcessElfCore::parseOpenBSDNotes(llvm::ArrayRef notes) { /// - NT_SIGINFO - Information about the signal that terminated the process /// - NT_AUXV - Process auxiliary vector /// - NT_FILE - Files mapped into memory -/// +/// clayborg wrote: revert whitespace only change https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -224,7 +224,7 @@ Status ProcessElfCore::DoLoadCore() { ArchSpec core_arch(m_core_module_sp->GetArchitecture()); target_arch.MergeFrom(core_arch); GetTarget().SetArchitecture(target_arch); - + clayborg wrote: revert whitespace only change https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
@@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. clayborg wrote: "The path to the core file for this target or an invalid file spec if the process isn't loaded from a core file. https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
bulbazord wrote: I'm not sure I understand. Why do you need to get the path to the core file through the SBAPI? Didn't you also load it through the SBAPI too? https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff a7bc9cb6ffa91ff0ebabc45c0c7263c7c2c3a4de e7af15ff3c1bdd42799b25f76145b8a19739eea5 -- lldb/include/lldb/API/SBProcess.h lldb/include/lldb/Target/PostMortemProcess.h lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/ProcessTrace.h lldb/source/API/SBProcess.cpp lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp lldb/source/Plugins/Process/elf-core/ProcessElfCore.h lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp lldb/source/Plugins/Process/mach-core/ProcessMachCore.h lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp lldb/source/Plugins/Process/minidump/ProcessMinidump.h lldb/source/Target/ProcessTrace.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index fbd0a4be40..8330e7bd7a 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -261,8 +261,8 @@ Status ProcessElfCore::DoLoadCore() { exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path, FileSpec::Style::native); if (exe_module_spec.GetFileSpec()) { -exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, - true /* notify */); +exe_module_sp = +GetTarget().GetOrCreateModule(exe_module_spec, true /* notify */); if (exe_module_sp) GetTarget().SetExecutableModule(exe_module_sp, eLoadDependentsNo); } `` https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: None (jeffreytan81) Changes We have a Python script that needs to locate coredump path during debugging so that we can retrieve certain metadata files associated with it. Currently, there is no API for this. This patch adds a new `SBProcess::GetCoreFile()` to retrieve target dump file spec used for dump debugging. Note: this is different from the main executable module spec. To achieve this, the patch hoists m_core_file into PostMortemProcess for sharing. --- Full diff: https://github.com/llvm/llvm-project/pull/80767.diff 15 Files Affected: - (modified) lldb/include/lldb/API/SBProcess.h (+7) - (modified) lldb/include/lldb/Target/PostMortemProcess.h (+9) - (modified) lldb/include/lldb/Target/Process.h (+7) - (modified) lldb/include/lldb/Target/ProcessTrace.h (+2-1) - (modified) lldb/source/API/SBProcess.cpp (+11) - (modified) lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp (+12-9) - (modified) lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.h (+2-1) - (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+4-4) - (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.h (-1) - (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+3-3) - (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.h (-1) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp (+1-1) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.h (-1) - (modified) lldb/source/Target/ProcessTrace.cpp (+4-3) - (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py (+11) ``diff diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 8c1c81418f83d..13994ed300853 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + /// Allocate memory within the process. /// /// This function will allocate memory in the process's address space. diff --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h index 7207fc99ef29a..9c9cd7fa599bc 100644 --- a/lldb/include/lldb/Target/PostMortemProcess.h +++ b/lldb/include/lldb/Target/PostMortemProcess.h @@ -24,7 +24,16 @@ class PostMortemProcess : public Process { using Process::Process; public: + PostMortemProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, +const FileSpec &core_file) + : Process(target_sp, listener_sp), m_core_file(core_file) {} + bool IsLiveDebugSession() const override { return false; } + + FileSpec GetCoreFile() const override { return m_core_file; } + +protected: + FileSpec m_core_file; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7048921d6034f..0ad626ffd3613 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1503,6 +1503,13 @@ class Process : public std::enable_shared_from_this, virtual bool IsLiveDebugSession() const { return true; }; + /// Provide a way to retrieve the core dump file that is loaded for debugging. + /// Only available if IsLiveDebugSession() returns true. + /// + /// \return + /// File path to the core file. + virtual FileSpec GetCoreFile() const { return {}; } + /// Before lldb detaches from a process, it warns the user that they are /// about to lose their debug session. In some cases, this warning doesn't /// need to be emitted -- for instance, with core file debugging where the diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h index 037dea232cc02..7a025100f6803 100644 --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -27,7 +27,8 @@ class ProcessTrace : public PostMortemProcess { static llvm::StringRef GetPluginDescriptionStatic(); - ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); + ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, + const FileSpec &core_file); ~ProcessTrace() override; diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 4864ea0e7d026..a9fe915324683 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/jeffreytan81 ready_for_review https://github.com/llvm/llvm-project/pull/80767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a new SBProcess:: GetCoreFile() API (PR #80767)
https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/80767 We have a Python script that needs to locate coredump path during debugging so that we can retrieve certain metadata files associated with it. Currently, there is no API for this. This patch adds a new `SBProcess::GetCoreFile()` to retrieve target dump file spec used for dump debugging. Note: this is different from the main executable module spec. To achieve this, the patch hoists m_core_file into PostMortemProcess for sharing. >From e7af15ff3c1bdd42799b25f76145b8a19739eea5 Mon Sep 17 00:00:00 2001 From: jeffreytan81 Date: Mon, 5 Feb 2024 15:58:23 -0800 Subject: [PATCH] Add a new SBProcess:: GetCoreFile() API --- lldb/include/lldb/API/SBProcess.h | 7 +++ lldb/include/lldb/Target/PostMortemProcess.h | 9 lldb/include/lldb/Target/Process.h| 7 +++ lldb/include/lldb/Target/ProcessTrace.h | 3 ++- lldb/source/API/SBProcess.cpp | 11 ++ .../FreeBSDKernel/ProcessFreeBSDKernel.cpp| 21 +++ .../FreeBSDKernel/ProcessFreeBSDKernel.h | 3 ++- .../Process/elf-core/ProcessElfCore.cpp | 8 +++ .../Plugins/Process/elf-core/ProcessElfCore.h | 1 - .../Process/mach-core/ProcessMachCore.cpp | 6 +++--- .../Process/mach-core/ProcessMachCore.h | 1 - .../Process/minidump/ProcessMinidump.cpp | 2 +- .../Process/minidump/ProcessMinidump.h| 1 - lldb/source/Target/ProcessTrace.cpp | 7 --- .../postmortem/elf-core/TestLinuxCore.py | 11 ++ 15 files changed, 73 insertions(+), 25 deletions(-) diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 8c1c81418f83d..13994ed300853 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -398,6 +398,13 @@ class LLDB_API SBProcess { /// valid. lldb::SBProcessInfo GetProcessInfo(); + /// Return target dump file during postmortem debugging. + /// An empty file will be returned for live debugging. + /// + /// \return + /// The target dump file spec. + lldb::SBFileSpec GetCoreFile(); + /// Allocate memory within the process. /// /// This function will allocate memory in the process's address space. diff --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h index 7207fc99ef29a..9c9cd7fa599bc 100644 --- a/lldb/include/lldb/Target/PostMortemProcess.h +++ b/lldb/include/lldb/Target/PostMortemProcess.h @@ -24,7 +24,16 @@ class PostMortemProcess : public Process { using Process::Process; public: + PostMortemProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, +const FileSpec &core_file) + : Process(target_sp, listener_sp), m_core_file(core_file) {} + bool IsLiveDebugSession() const override { return false; } + + FileSpec GetCoreFile() const override { return m_core_file; } + +protected: + FileSpec m_core_file; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7048921d6034f..0ad626ffd3613 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1503,6 +1503,13 @@ class Process : public std::enable_shared_from_this, virtual bool IsLiveDebugSession() const { return true; }; + /// Provide a way to retrieve the core dump file that is loaded for debugging. + /// Only available if IsLiveDebugSession() returns true. + /// + /// \return + /// File path to the core file. + virtual FileSpec GetCoreFile() const { return {}; } + /// Before lldb detaches from a process, it warns the user that they are /// about to lose their debug session. In some cases, this warning doesn't /// need to be emitted -- for instance, with core file debugging where the diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h index 037dea232cc02..7a025100f6803 100644 --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -27,7 +27,8 @@ class ProcessTrace : public PostMortemProcess { static llvm::StringRef GetPluginDescriptionStatic(); - ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp); + ProcessTrace(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, + const FileSpec &core_file); ~ProcessTrace() override; diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 4864ea0e7d026..a9fe915324683 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1244,6 +1244,17 @@ lldb::SBProcessInfo SBProcess::GetProcessInfo() { return sb_proc_info; } +lldb::SBFileSpec SBProcess::GetCoreFile() { + LLDB_INSTRUMENT_VA(this); + + ProcessSP process_sp(GetSP()); + FileSpec core_file; + if (process_sp) { +core_file = process_sp->GetCoreFile(); + } + return SBFileSpec(
[Lldb-commits] [libc] [clang-tools-extra] [openmp] [libcxx] [compiler-rt] [lldb] [mlir] [llvm] [lld] [flang] [clang] [Driver] Report invalid target triple versions for all environment types. (PR #7865
MaskRay wrote: > > There's apparently also wasm32-wasi-preview2 and wasm32-wasi-pthread, which > > I suppose are equally broken by this change. > > Yes, I think so. I think adding these environment types in wasi-libc repo > could help fix those errors. If wasm can arbitrary environment types, it seems that we can opt out the check for `isWasm()`. We define new environment types when they affect the compiler behavior. If wasm arbitrary sets the environment, we should not define a type for each one. https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -241,7 +241,7 @@ class DWARFUnit : public UserID { FileSpec GetFile(size_t file_idx); FileSpec::Style GetPathStyle(); - SymbolFileDWARFDwo *GetDwoSymbolFile(); + SymbolFileDWARFDwo *GetDwoSymbolFile(bool load_if_needed = true); bulbazord wrote: Instead of having a bool here, you could make this an enum with 2 values. Something like this: ``` enum LoadMode : bool { eDoNotForceLoad, eForceLoad }; ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -86,9 +86,13 @@ class LLDB_API SBTarget { /// Returns a dump of the collected statistics. /// + /// \param[in] summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// A SBStructuredData with the statistics collected. - lldb::SBStructuredData GetStatistics(); bulbazord wrote: This is an ABI-breaking change. You'll want to add a new method entirely. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -186,6 +186,10 @@ class SymbolFileDWARF : public SymbolFileCommon { GetMangledNamesForFunction(const std::string &scope_qualified_name, std::vector &mangled_names) override; + // Return total currently loaded debug info. + // For cases like .dwo files, the debug info = skeleton debug info + all dwo + // debug info where .dwo files might not be loaded yet. Calling this function + // will not force the loading of any .dwo files. bulbazord wrote: Could you make this into a proper doxygen comment? With `///` instead of `//` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/bulbazord edited https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/bulbazord requested changes to this pull request. Haven't looked over everything yet but this has an ABI-breaking change. Please revert the existing changes to SBTarget. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -83,13 +86,15 @@ class CommandObjectStatsDump : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_all_targets = false; + m_summary_only = false; } llvm::ArrayRef GetDefinitions() override { return llvm::ArrayRef(g_statistics_dump_options); } bool m_all_targets = false; clayborg wrote: This should get moved into `lldb_private::StatisticsOptions` as an option. Add accessor to `SBStatisticsOptions` so we can get/set this as well. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -2687,7 +2687,7 @@ uint64_t SymbolFileDWARF::GetDebugInfoSize() { if (cu == nullptr) continue; -SymbolFileDWARFDwo *dwo = cu->GetDwoSymbolFile(); +SymbolFileDWARFDwo *dwo = cu->GetDwoSymbolFile(false); clayborg wrote: This bool might need to be a bool in the "SymbolFile::GetDebugInfoSize()" virtual function. Users might want the know the rigtht answer (exactly how much total debug info they have if `load_if_needed == true` or just what is loaded currently `load_if_needed == false` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -326,7 +330,7 @@ class LLDB_API SBTarget { uint32_t GetAddressByteSize(); const char *GetTriple(); - + clayborg wrote: revert whitespace only changes https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -100,60 +101,91 @@ llvm::json::Value ConstStringStats::ToJSON() const { return obj; } -json::Value TargetStats::ToJSON(Target &target) { - CollectStats(target); +json::Value TargetStats::ToJSON(Target &target, bool summary_only) { clayborg wrote: Pass in "const lldb_private::StatisticsOptions &options" here insetead of bool https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -197,17 +197,18 @@ SBDebugger SBTarget::GetDebugger() const { return debugger; } -SBStructuredData SBTarget::GetStatistics() { +SBStructuredData SBTarget::GetStatistics(bool summary_only) { clayborg wrote: pass in const `SBStatisticsOptions options` instead of bool https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -4962,4 +4962,6 @@ std::recursive_mutex &Target::GetAPIMutex() { } /// Get metrics associated with this target in JSON format. -llvm::json::Value Target::ReportStatistics() { return m_stats.ToJSON(*this); } +llvm::json::Value Target::ReportStatistics(bool summary_only) { clayborg wrote: pass in "const lldb_private::StatisticsOptions &options" here instead of bool https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -83,13 +86,15 @@ class CommandObjectStatsDump : public CommandObjectParsed { void OptionParsingStarting(ExecutionContext *execution_context) override { m_all_targets = false; + m_summary_only = false; } llvm::ArrayRef GetDefinitions() override { return llvm::ArrayRef(g_statistics_dump_options); } bool m_all_targets = false; +bool m_summary_only = false; clayborg wrote: Change this to: ``` lldb_private::StatisticsOptions m_options; ``` We should probably also move `m_all_targets` into `lldb_private::StatisticsOptions`. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -86,9 +86,13 @@ class LLDB_API SBTarget { /// Returns a dump of the collected statistics. /// + /// \param[in] summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// A SBStructuredData with the statistics collected. - lldb::SBStructuredData GetStatistics(); + lldb::SBStructuredData GetStatistics(bool summary_only = false); clayborg wrote: This violates our public API policy where we can't take away or change a previously available public API function. So we must: - keep the old function around - add a new function with extra parameters If we are going to possibly add more options to GetStatistics in the future, we don't want to have to keep overloading this function with new values since we would need to keep adding overloads. The way we fix this in the API is to create a SBStatisticsOptions class that has accessors. This way we can keep adding more features in the future as it is ok to add new methods to any existing classes. So this code should look like: ``` /// Returns a dump of the collected statistics. /// /// \return /// A SBStructuredData with the statistics collected. lldb::SBStructuredData GetStatistics(); /// Returns a dump of the collected statistics. /// /// \param[in] options /// An objects object that contains all options for the statistics dumping. /// /// \return /// A SBStructuredData with the statistics collected. lldb::SBStructuredData GetStatistics(SBStatisticsOptions options); ``` Then we need to create a simple public API class for `SBStatisticsOptions`: ``` class SBStatisticsOptions { public: SBStatisticsOptions(); bool GetSummaryOnly(); void SetSummaryOnly(bool b); }; ``` And if later we might want to add another bool option, we can just add the new methods to this class. Lets say later we want to only emit target stats for a specific target, instead of all targets, we could add new functions to `SBStatisticsOptions`: ``` class SBStatisticsOptions { public: SBStatisticsOptions(); bool GetSummaryOnly(); void SetSummaryOnly(bool b); /// Limit statistics data for only 1 target instead of all targets void SetTarget(SBTarget target); }; ``` And we can still use our `lldb::SBStructuredData GetStatistics(SBStatisticsOptions options);` function because the signature didn't change! https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -1599,7 +1599,7 @@ class Target : public std::enable_shared_from_this, /// /// \return /// Returns a JSON value that contains all target metrics. - llvm::json::Value ReportStatistics(); + llvm::json::Value ReportStatistics(bool summary_only = false); clayborg wrote: pass in `const lldb_private::StatisticsOptions &options` instead of bool https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -133,7 +133,7 @@ struct ConstStringStats { /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: - llvm::json::Value ToJSON(Target &target); + llvm::json::Value ToJSON(Target &target, bool summary_only = false); clayborg wrote: We probably want a lldb_private::StatisticsOptions class that contains all options: ``` struct StatisticsOptions { bool summary_only = false; }; ``` Then when you create the SBStatisticsOptions class, it should contain a `std::unique_ptr m_opaque_up;` member, just like all other public API classes. And the methods on `SBStatisticsOptions` will modify the contained `lldb_private::StatisticsOptions` object appropriately. The reason we need the `SBStatisticsOptions` class to have a single member that is a unique pointer is so if we add extra things to `lldb_private::StatisticsOptions`, it won't change the size of the `SBStatisticsOptions` class since it just has a `unique_ptr` as its only member variable, which makes it suitable for use in the public API. We can't change the size of our objects if we want to maintain a stable public API. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -171,9 +171,14 @@ class DebuggerStats { /// The single target to emit statistics for if non NULL, otherwise dump /// statistics only for the specified target. /// + /// \param summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// Returns a JSON value that contains all target metrics. - static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target); + static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target, +bool summary_only = false); clayborg wrote: pass in const `lldb_private::StatisticsOptions &options` instead of bool https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [mlir] [clang] [libc] [flang] [clang-tools-extra] [lld] [openmp] [lldb] [llvm] [libcxx] [Driver] Report invalid target triple versions for all environment types. (PR #7865
glandium wrote: > Yes, I think so. I think adding these environment types in wasi-libc repo > could help fix those errors. WDYM? https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -2687,7 +2687,7 @@ uint64_t SymbolFileDWARF::GetDebugInfoSize() { if (cu == nullptr) continue; -SymbolFileDWARFDwo *dwo = cu->GetDwoSymbolFile(); +SymbolFileDWARFDwo *dwo = cu->GetDwoSymbolFile(false); jeffreytan81 wrote: Instead of hard-coding `false` here, I think it should the caller of `SymbolFileDWARF::GetDebugInfoSize` caller (statistics.cpp in this case) making this decision. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [flang] [llvm] [clang] [libc] [libcxx] [compiler-rt] [lldb] [clang-tools-extra] [openmp] [mlir] [Driver] Report invalid target triple versions for all environment types. (PR #7865
ZijunZhaoCCK wrote: > There's apparently also wasm32-wasi-preview2 and wasm32-wasi-pthread, which I > suppose are equally broken by this change. Yes, I think so. I think adding these environment types in wasi-libc repo could help fix those errors. https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability to define a Python based command that uses CommandObjectParsed (PR #70734)
jimingham wrote: At this point, I'd like to get this in so I can iterate on it in future PR's rather than churning this one. If folks have a bit to look over this, that would be great. Thanks! https://github.com/llvm/llvm-project/pull/70734 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add the ability to define a Python based command that uses CommandObjectParsed (PR #70734)
jimingham wrote: Okay, I made one more trivial change, I switched the `varname` field to `dest` so it matched what argparse had, and I renamed the `usage` entry to `help` so it matches the argparse usage as well. https://github.com/llvm/llvm-project/pull/70734 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add QSupported key to report watchpoint types supported (PR #80376)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/80376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/80745 >From 36c84ce56e9ea288d64833aa1f927a7f97fd904c Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Fri, 2 Feb 2024 15:42:01 -0800 Subject: [PATCH 1/3] Support statistics dump summary only mode Summary: Added a new --summary option to statistics dump command so that it is much light weight than the full version. With this change, statistics dump --summary can now be included in lldb command line telemetry without slowing down lldb exiting. --- lldb/include/lldb/API/SBTarget.h | 8 +- lldb/include/lldb/Target/Statistics.h | 9 +- lldb/include/lldb/Target/Target.h | 2 +- lldb/source/API/SBTarget.cpp | 9 +- lldb/source/Commands/CommandObjectStats.cpp | 8 +- lldb/source/Commands/Options.td | 3 + lldb/source/Target/Statistics.cpp | 194 +++--- lldb/source/Target/Target.cpp | 4 +- .../stats_api/TestStatisticsAPI.py| 15 ++ 9 files changed, 164 insertions(+), 88 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 83087623088c5b..7fd888cf7014e3 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -86,9 +86,13 @@ class LLDB_API SBTarget { /// Returns a dump of the collected statistics. /// + /// \param[in] summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// A SBStructuredData with the statistics collected. - lldb::SBStructuredData GetStatistics(); + lldb::SBStructuredData GetStatistics(bool summary_only = false); /// Return the platform object associated with the target. /// @@ -326,7 +330,7 @@ class LLDB_API SBTarget { uint32_t GetAddressByteSize(); const char *GetTriple(); - + const char *GetABIName(); const char *GetLabel() const; diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index f672786f58f84d..98658ba0cac317 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -133,7 +133,7 @@ struct ConstStringStats { /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: - llvm::json::Value ToJSON(Target &target); + llvm::json::Value ToJSON(Target &target, bool summary_only = false); void SetLaunchOrAttachTime(); void SetFirstPrivateStopTime(); @@ -171,9 +171,14 @@ class DebuggerStats { /// The single target to emit statistics for if non NULL, otherwise dump /// statistics only for the specified target. /// + /// \param summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// Returns a JSON value that contains all target metrics. - static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target); + static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target, +bool summary_only = false); protected: // Collecting stats can be set to true to collect stats that are expensive diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c37682e2a03859..4bf6c123dc1ddc 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1599,7 +1599,7 @@ class Target : public std::enable_shared_from_this, /// /// \return /// Returns a JSON value that contains all target metrics. - llvm::json::Value ReportStatistics(); + llvm::json::Value ReportStatistics(bool summary_only = false); TargetStats &GetStatistics() { return m_stats; } diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 8e616afbcb4e8d..615a00ceeaee16 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -197,7 +197,7 @@ SBDebugger SBTarget::GetDebugger() const { return debugger; } -SBStructuredData SBTarget::GetStatistics() { +SBStructuredData SBTarget::GetStatistics(bool summary_only) { LLDB_INSTRUMENT_VA(this); SBStructuredData data; @@ -205,9 +205,10 @@ SBStructuredData SBTarget::GetStatistics() { if (!target_sp) return data; std::string json_str = - llvm::formatv("{0:2}", - DebuggerStats::ReportStatistics(target_sp->GetDebugger(), - target_sp.get())).str(); + llvm::formatv( + "{0:2}", DebuggerStats::ReportStatistics( + target_sp->GetDebugger(), target_sp.get(), summary_only)) + .str(); data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str)); return data; } diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index 262de0bda144a6..781b90794dc377 100644 ---
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -74,7 +75,7 @@ json::Value ModuleStats::ToJSON() const { if (!symfile_modules.empty()) { json::Array symfile_ids; -for (const auto symfile_id: symfile_modules) +for (const auto symfile_id : symfile_modules) kusmour wrote: Yes I can revert that, probs a formatting on save that I didn't notice. https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [openmp] [clang] [lldb] [mlir] [llvm] [libcxx] [flang] [libc] [lld] [clang-tools-extra] [Driver] Report invalid target triple versions for all environment types. (PR #7865
glandium wrote: There's apparently also wasm32-wasi-preview2 and wasm32-wasi-pthread, which I suppose are equally broken by this change. https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang-tools-extra] [openmp] [compiler-rt] [llvm] [libcxx] [lldb] [lld] [clang] [flang] [libc] [mlir] [Driver] Report invalid target triple versions for all environment types. (PR #7865
glandium wrote: We stumbled upon this downstream because we have jobs building wasi-sdk with clang-trunk, and wasi-sdk builds some things with that target. It apparently comes from https://github.com/WebAssembly/wasi-libc/pull/381 https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [mlir] [lldb] [llvm] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
@@ -220,6 +220,31 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever +run and return the new loop inside the check. The loop body is moved pzread wrote: SG. Updated the comments https://github.com/llvm/llvm-project/pull/80331 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [mlir] [lldb] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
https://github.com/pzread updated https://github.com/llvm/llvm-project/pull/80331 >From 70f54b51bef87bde5e3f5ee067c0f2414d34e915 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Thu, 1 Feb 2024 19:57:26 + Subject: [PATCH 1/7] Add replaceWithZeroTripCheck to LoopLikeOpInterface --- .../mlir/Interfaces/LoopLikeInterface.td | 22 +++ 1 file changed, 22 insertions(+) diff --git a/mlir/include/mlir/Interfaces/LoopLikeInterface.td b/mlir/include/mlir/Interfaces/LoopLikeInterface.td index e2ac85a3f7725d..77409cb3a8274b 100644 --- a/mlir/include/mlir/Interfaces/LoopLikeInterface.td +++ b/mlir/include/mlir/Interfaces/LoopLikeInterface.td @@ -220,6 +220,28 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever +run and return the new loop inside the check. The loop body is moved +over to the new loop. Returns "failure" if the loop doesn't support +this transformation. + +After the transformation, the ops inserted to the parent region of the +loop are guaranteed to be run only if the loop body runs at least one +iteration. + +Note: Ops in the loop body might be rearranged because of loop rotating +to maintain the semantic. Terminators might be removed/added during this +transformation. + }], + /*retTy=*/"::mlir::FailureOr<::mlir::LoopLikeOpInterface>", + /*methodName=*/"replaceWithZeroTripCheck", + /*args=*/(ins "::mlir::RewriterBase &":$rewriter), + /*methodBody=*/"", + /*defaultImplementation=*/[{ +return ::mlir::failure(); + }] > ]; >From d6703ebbeb5ddc358929672b44994a9d05683523 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Fri, 2 Feb 2024 18:59:03 + Subject: [PATCH 2/7] Add tests --- mlir/unittests/Interfaces/CMakeLists.txt | 3 + .../Interfaces/LoopLikeInterfaceTest.cpp | 101 ++ 2 files changed, 104 insertions(+) create mode 100644 mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp diff --git a/mlir/unittests/Interfaces/CMakeLists.txt b/mlir/unittests/Interfaces/CMakeLists.txt index d192b2922d6b9d..cab9503cf295b1 100644 --- a/mlir/unittests/Interfaces/CMakeLists.txt +++ b/mlir/unittests/Interfaces/CMakeLists.txt @@ -3,6 +3,7 @@ add_mlir_unittest(MLIRInterfacesTests DataLayoutInterfacesTest.cpp InferIntRangeInterfaceTest.cpp InferTypeOpInterfaceTest.cpp + LoopLikeInterfaceTest.cpp ) target_link_libraries(MLIRInterfacesTests @@ -12,7 +13,9 @@ target_link_libraries(MLIRInterfacesTests MLIRDataLayoutInterfaces MLIRDLTIDialect MLIRFuncDialect + MLIRIR MLIRInferIntRangeInterface MLIRInferTypeOpInterface + MLIRLoopLikeInterface MLIRParser ) diff --git a/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp new file mode 100644 index 00..b0b7680fed68e7 --- /dev/null +++ b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp @@ -0,0 +1,101 @@ +//===- LoopLikeInterfaceTest.cpp - Unit tests for Loop Like Interfaces. ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "mlir/Interfaces/LoopLikeInterface.h" +#include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Dialect.h" +#include "mlir/IR/DialectImplementation.h" +#include "mlir/IR/OpDefinition.h" +#include "mlir/IR/OpImplementation.h" +#include "mlir/IR/PatternMatch.h" +#include "mlir/Parser/Parser.h" + +#include + +using namespace mlir; + +struct NoZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.no_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } +}; + +struct ImplZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.impl_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } + + FailureOr + replaceWithZeroTripCheck(RewriterBase &rewriter) { +return cast(this->getOperation()); + } +}; + +/// A dialect putting all the above together. +struct LoopTestDialect : Dialect { + explicit LoopTestDialect(MLIRContext *ctx) + : Dialect(getDialectNamespace(), ctx, TypeID::get()) { +addOperations(); + } + static StringRef getDialectNamespace() { return "looptest"; } +}; + +TEST(LoopLikeOpInterface, NoReplaceWithZeroTripCheck) { + const char *ir = R"MLIR( + "looptest.no_zero_trip_check_loop_op"() : () -> () + )ML
[Lldb-commits] [llvm] [mlir] [lldb] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
@@ -0,0 +1,101 @@ +//===- LoopLikeInterfaceTest.cpp - Unit tests for Loop Like Interfaces. ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "mlir/Interfaces/LoopLikeInterface.h" +#include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Dialect.h" +#include "mlir/IR/DialectImplementation.h" +#include "mlir/IR/OpDefinition.h" +#include "mlir/IR/OpImplementation.h" +#include "mlir/IR/PatternMatch.h" +#include "mlir/Parser/Parser.h" + +#include + +using namespace mlir; + +struct NoZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.no_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } +}; + +struct ImplZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.impl_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } + + FailureOr + replaceWithZeroTripCheck(RewriterBase &rewriter) { +return cast(this->getOperation()); + } +}; + +/// A dialect putting all the above together. +struct LoopTestDialect : Dialect { + explicit LoopTestDialect(MLIRContext *ctx) + : Dialect(getDialectNamespace(), ctx, TypeID::get()) { +addOperations(); + } + static StringRef getDialectNamespace() { return "looptest"; } +}; + +TEST(LoopLikeOpInterface, NoReplaceWithZeroTripCheck) { + const char *ir = R"MLIR( + "looptest.no_zero_trip_check_loop_op"() : () -> () + )MLIR"; + + DialectRegistry registry; + registry.insert(); + MLIRContext ctx(registry); + + OwningOpRef module = parseSourceString(ir, &ctx); + LoopLikeOpInterface testOp = + cast(module->getBody()->getOperations().front()); + + IRRewriter rewriter(&ctx); + FailureOr result = + testOp.replaceWithZeroTripCheck(rewriter); + + EXPECT_TRUE(failed(result)); +} + +TEST(LoopLikeOpInterface, ImplReplaceWithZeroTripCheck) { + const char *ir = R"MLIR( + "looptest.impl_zero_trip_check_loop_op"() : () -> () + )MLIR"; + + DialectRegistry registry; + registry.insert(); + MLIRContext ctx(registry); + + OwningOpRef module = parseSourceString(ir, &ctx); + LoopLikeOpInterface testOp = + cast(module->getBody()->getOperations().front()); + + IRRewriter rewriter(&ctx); + FailureOr result = + testOp.replaceWithZeroTripCheck(rewriter); + + EXPECT_TRUE(succeeded(result)); + EXPECT_EQ(*result, testOp); +} pzread wrote: Done. PTAL. I prefer to keep this change focusing on the interface definition so I only add a `scf.while` test with no transformation. The actual transformation and tests will be added in the follow-up change #80349 https://github.com/llvm/llvm-project/pull/80331 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [openmp] [clang] [libc] [mlir] [libcxx] [compiler-rt] [lld] [lldb] [llvm] [clang-tools-extra] [flang] [Driver] Report invalid target triple versions for all environment types. (PR #7865
ZijunZhaoCCK wrote: > This broke the wasi-threads target: `clang: error: version 'threads' in > target triple 'wasm32-unknown-wasi-threads' is invalid` Because `threads` is not in EnvironmentType list: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h#L231 or ObjectType list: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h#L284 . The format should be `arch-vendor-os-env`. If `threads` is a new environment type, please add it in the list. If `wasi-threads` is a special case, please let me know! And one more question, would you mind pointing me out the test case of `wasm32-unknown-wasi-threads` ? I don't see it. Thank you! https://github.com/llvm/llvm-project/pull/78655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [mlir] [lldb] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
https://github.com/pzread updated https://github.com/llvm/llvm-project/pull/80331 >From 70f54b51bef87bde5e3f5ee067c0f2414d34e915 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Thu, 1 Feb 2024 19:57:26 + Subject: [PATCH 1/6] Add replaceWithZeroTripCheck to LoopLikeOpInterface --- .../mlir/Interfaces/LoopLikeInterface.td | 22 +++ 1 file changed, 22 insertions(+) diff --git a/mlir/include/mlir/Interfaces/LoopLikeInterface.td b/mlir/include/mlir/Interfaces/LoopLikeInterface.td index e2ac85a3f7725..77409cb3a8274 100644 --- a/mlir/include/mlir/Interfaces/LoopLikeInterface.td +++ b/mlir/include/mlir/Interfaces/LoopLikeInterface.td @@ -220,6 +220,28 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever +run and return the new loop inside the check. The loop body is moved +over to the new loop. Returns "failure" if the loop doesn't support +this transformation. + +After the transformation, the ops inserted to the parent region of the +loop are guaranteed to be run only if the loop body runs at least one +iteration. + +Note: Ops in the loop body might be rearranged because of loop rotating +to maintain the semantic. Terminators might be removed/added during this +transformation. + }], + /*retTy=*/"::mlir::FailureOr<::mlir::LoopLikeOpInterface>", + /*methodName=*/"replaceWithZeroTripCheck", + /*args=*/(ins "::mlir::RewriterBase &":$rewriter), + /*methodBody=*/"", + /*defaultImplementation=*/[{ +return ::mlir::failure(); + }] > ]; >From d6703ebbeb5ddc358929672b44994a9d05683523 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Fri, 2 Feb 2024 18:59:03 + Subject: [PATCH 2/6] Add tests --- mlir/unittests/Interfaces/CMakeLists.txt | 3 + .../Interfaces/LoopLikeInterfaceTest.cpp | 101 ++ 2 files changed, 104 insertions(+) create mode 100644 mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp diff --git a/mlir/unittests/Interfaces/CMakeLists.txt b/mlir/unittests/Interfaces/CMakeLists.txt index d192b2922d6b9..cab9503cf295b 100644 --- a/mlir/unittests/Interfaces/CMakeLists.txt +++ b/mlir/unittests/Interfaces/CMakeLists.txt @@ -3,6 +3,7 @@ add_mlir_unittest(MLIRInterfacesTests DataLayoutInterfacesTest.cpp InferIntRangeInterfaceTest.cpp InferTypeOpInterfaceTest.cpp + LoopLikeInterfaceTest.cpp ) target_link_libraries(MLIRInterfacesTests @@ -12,7 +13,9 @@ target_link_libraries(MLIRInterfacesTests MLIRDataLayoutInterfaces MLIRDLTIDialect MLIRFuncDialect + MLIRIR MLIRInferIntRangeInterface MLIRInferTypeOpInterface + MLIRLoopLikeInterface MLIRParser ) diff --git a/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp new file mode 100644 index 0..b0b7680fed68e --- /dev/null +++ b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp @@ -0,0 +1,101 @@ +//===- LoopLikeInterfaceTest.cpp - Unit tests for Loop Like Interfaces. ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "mlir/Interfaces/LoopLikeInterface.h" +#include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Dialect.h" +#include "mlir/IR/DialectImplementation.h" +#include "mlir/IR/OpDefinition.h" +#include "mlir/IR/OpImplementation.h" +#include "mlir/IR/PatternMatch.h" +#include "mlir/Parser/Parser.h" + +#include + +using namespace mlir; + +struct NoZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.no_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } +}; + +struct ImplZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.impl_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } + + FailureOr + replaceWithZeroTripCheck(RewriterBase &rewriter) { +return cast(this->getOperation()); + } +}; + +/// A dialect putting all the above together. +struct LoopTestDialect : Dialect { + explicit LoopTestDialect(MLIRContext *ctx) + : Dialect(getDialectNamespace(), ctx, TypeID::get()) { +addOperations(); + } + static StringRef getDialectNamespace() { return "looptest"; } +}; + +TEST(LoopLikeOpInterface, NoReplaceWithZeroTripCheck) { + const char *ir = R"MLIR( + "looptest.no_zero_trip_check_loop_op"() : () -> () + )MLIR"; +
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -74,7 +75,7 @@ json::Value ModuleStats::ToJSON() const { if (!symfile_modules.empty()) { json::Array symfile_ids; -for (const auto symfile_id: symfile_modules) +for (const auto symfile_id : symfile_modules) hawkinsw wrote: Is this just a whitespace change? https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -100,60 +101,91 @@ llvm::json::Value ConstStringStats::ToJSON() const { return obj; } -json::Value TargetStats::ToJSON(Target &target) { - CollectStats(target); +json::Value TargetStats::ToJSON(Target &target, bool summary_only) { + json::Object target_metrics_json; + ProcessSP process_sp = target.GetProcessSP(); + if (!summary_only) { +CollectStats(target); - json::Array json_module_uuid_array; - for (auto module_identifier : m_module_identifiers) -json_module_uuid_array.emplace_back(module_identifier); +json::Array json_module_uuid_array; +for (auto module_identifier : m_module_identifiers) + json_module_uuid_array.emplace_back(module_identifier); - json::Object target_metrics_json{ - {m_expr_eval.name, m_expr_eval.ToJSON()}, - {m_frame_var.name, m_frame_var.ToJSON()}, - {"moduleIdentifiers", std::move(json_module_uuid_array)}}; +target_metrics_json.try_emplace(m_expr_eval.name, m_expr_eval.ToJSON()); +target_metrics_json.try_emplace(m_frame_var.name, m_frame_var.ToJSON()); +target_metrics_json.try_emplace("moduleIdentifiers", +std::move(json_module_uuid_array)); - if (m_launch_or_attach_time && m_first_private_stop_time) { -double elapsed_time = -elapsed(*m_launch_or_attach_time, *m_first_private_stop_time); -target_metrics_json.try_emplace("launchOrAttachTime", elapsed_time); - } - if (m_launch_or_attach_time && m_first_public_stop_time) { -double elapsed_time = -elapsed(*m_launch_or_attach_time, *m_first_public_stop_time); -target_metrics_json.try_emplace("firstStopTime", elapsed_time); +if (m_launch_or_attach_time && m_first_private_stop_time) { + double elapsed_time = + elapsed(*m_launch_or_attach_time, *m_first_private_stop_time); + target_metrics_json.try_emplace("launchOrAttachTime", elapsed_time); +} +if (m_launch_or_attach_time && m_first_public_stop_time) { + double elapsed_time = + elapsed(*m_launch_or_attach_time, *m_first_public_stop_time); + target_metrics_json.try_emplace("firstStopTime", elapsed_time); +} +target_metrics_json.try_emplace("targetCreateTime", +m_create_time.get().count()); + +json::Array breakpoints_array; +double totalBreakpointResolveTime = 0.0; +// Rport both the normal breakpoint list and the internal breakpoint list. hawkinsw wrote: ```suggestion // Report both the normal breakpoint list and the internal breakpoint list. ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -186,6 +186,10 @@ class SymbolFileDWARF : public SymbolFileCommon { GetMangledNamesForFunction(const std::string &scope_qualified_name, std::vector &mangled_names) override; + // Return total currently loaded debug info hawkinsw wrote: ```suggestion // Return total currently loaded debug info. ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -1412,4 +1412,7 @@ let Command = "trace schema" in { let Command = "statistics dump" in { def statistics_dump_all: Option<"all-targets", "a">, Group<1>, Desc<"Include statistics for all targets.">; + def statistics_dump_summary: Option<"summary", "s">, Group<1>, +Desc<"Dump only high level summary statistics." + "Exclude targets, modules, breakpoints etc.. details.">; hawkinsw wrote: ```suggestion "Exclude targets, modules, breakpoints etc... details.">; ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
@@ -1412,4 +1412,7 @@ let Command = "trace schema" in { let Command = "statistics dump" in { def statistics_dump_all: Option<"all-targets", "a">, Group<1>, Desc<"Include statistics for all targets.">; + def statistics_dump_summary: Option<"summary", "s">, Group<1>, +Desc<"Dump only high level summary statistics." hawkinsw wrote: ```suggestion Desc<"Dump only high-level summary statistics." ``` https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/hawkinsw commented: Thank you for doing this! I hope that these little nits are helpful! https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/hawkinsw edited https://github.com/llvm/llvm-project/pull/80745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/80745 >From 36c84ce56e9ea288d64833aa1f927a7f97fd904c Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Fri, 2 Feb 2024 15:42:01 -0800 Subject: [PATCH 1/2] Support statistics dump summary only mode Summary: Added a new --summary option to statistics dump command so that it is much light weight than the full version. With this change, statistics dump --summary can now be included in lldb command line telemetry without slowing down lldb exiting. --- lldb/include/lldb/API/SBTarget.h | 8 +- lldb/include/lldb/Target/Statistics.h | 9 +- lldb/include/lldb/Target/Target.h | 2 +- lldb/source/API/SBTarget.cpp | 9 +- lldb/source/Commands/CommandObjectStats.cpp | 8 +- lldb/source/Commands/Options.td | 3 + lldb/source/Target/Statistics.cpp | 194 +++--- lldb/source/Target/Target.cpp | 4 +- .../stats_api/TestStatisticsAPI.py| 15 ++ 9 files changed, 164 insertions(+), 88 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 83087623088c5b..7fd888cf7014e3 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -86,9 +86,13 @@ class LLDB_API SBTarget { /// Returns a dump of the collected statistics. /// + /// \param[in] summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// A SBStructuredData with the statistics collected. - lldb::SBStructuredData GetStatistics(); + lldb::SBStructuredData GetStatistics(bool summary_only = false); /// Return the platform object associated with the target. /// @@ -326,7 +330,7 @@ class LLDB_API SBTarget { uint32_t GetAddressByteSize(); const char *GetTriple(); - + const char *GetABIName(); const char *GetLabel() const; diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index f672786f58f84d..98658ba0cac317 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -133,7 +133,7 @@ struct ConstStringStats { /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: - llvm::json::Value ToJSON(Target &target); + llvm::json::Value ToJSON(Target &target, bool summary_only = false); void SetLaunchOrAttachTime(); void SetFirstPrivateStopTime(); @@ -171,9 +171,14 @@ class DebuggerStats { /// The single target to emit statistics for if non NULL, otherwise dump /// statistics only for the specified target. /// + /// \param summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// Returns a JSON value that contains all target metrics. - static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target); + static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target, +bool summary_only = false); protected: // Collecting stats can be set to true to collect stats that are expensive diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c37682e2a03859..4bf6c123dc1ddc 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1599,7 +1599,7 @@ class Target : public std::enable_shared_from_this, /// /// \return /// Returns a JSON value that contains all target metrics. - llvm::json::Value ReportStatistics(); + llvm::json::Value ReportStatistics(bool summary_only = false); TargetStats &GetStatistics() { return m_stats; } diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 8e616afbcb4e8d..615a00ceeaee16 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -197,7 +197,7 @@ SBDebugger SBTarget::GetDebugger() const { return debugger; } -SBStructuredData SBTarget::GetStatistics() { +SBStructuredData SBTarget::GetStatistics(bool summary_only) { LLDB_INSTRUMENT_VA(this); SBStructuredData data; @@ -205,9 +205,10 @@ SBStructuredData SBTarget::GetStatistics() { if (!target_sp) return data; std::string json_str = - llvm::formatv("{0:2}", - DebuggerStats::ReportStatistics(target_sp->GetDebugger(), - target_sp.get())).str(); + llvm::formatv( + "{0:2}", DebuggerStats::ReportStatistics( + target_sp->GetDebugger(), target_sp.get(), summary_only)) + .str(); data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str)); return data; } diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index 262de0bda144a6..781b90794dc377 100644 ---
[Lldb-commits] [lldb] [llvm] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 41ea02261224446dadb1b1561d70137699255518 c4b767909a9ffc2a3015dc9021e4c265da0d877d -- lldb/tools/lldb-dap/Breakpoint.cpp lldb/tools/lldb-dap/Breakpoint.h lldb/tools/lldb-dap/BreakpointBase.cpp lldb/tools/lldb-dap/BreakpointBase.h lldb/tools/lldb-dap/FunctionBreakpoint.cpp lldb/tools/lldb-dap/FunctionBreakpoint.h lldb/tools/lldb-dap/JSONUtils.cpp lldb/tools/lldb-dap/JSONUtils.h lldb/tools/lldb-dap/SourceBreakpoint.cpp lldb/tools/lldb-dap/SourceBreakpoint.h lldb/tools/lldb-dap/lldb-dap.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/tools/lldb-dap/Breakpoint.h b/lldb/tools/lldb-dap/Breakpoint.h index a668e29f3d..5600bc1792 100644 --- a/lldb/tools/lldb-dap/Breakpoint.h +++ b/lldb/tools/lldb-dap/Breakpoint.h @@ -19,7 +19,7 @@ struct Breakpoint : public BreakpointBase { Breakpoint() = default; Breakpoint(const llvm::json::Object &obj) : BreakpointBase(obj){}; - Breakpoint(lldb::SBBreakpoint bp): bp(bp) {} + Breakpoint(lldb::SBBreakpoint bp) : bp(bp) {} void SetCondition() override; void SetHitCondition() override; `` https://github.com/llvm/llvm-project/pull/80753 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [llvm] [lldb] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) Changes This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and `BreakpointBase` to have better separation and encapsulation so we are not directly operating on `SBBreakpoint`. I basically moved the `SBBreakpoint` and the methods that requires it from `BreakpointBase` to `Breakpoint`. This allows adding support for data watchpoint easier by sharing the logic inside `BreakpointBase`. --- Patch is 27.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80753.diff 13 Files Affected: - (added) lldb/tools/lldb-dap/Breakpoint.cpp (+182) - (added) lldb/tools/lldb-dap/Breakpoint.h (+34) - (modified) lldb/tools/lldb-dap/BreakpointBase.cpp (-113) - (modified) lldb/tools/lldb-dap/BreakpointBase.h (+6-6) - (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1) - (modified) lldb/tools/lldb-dap/FunctionBreakpoint.cpp (+2-10) - (modified) lldb/tools/lldb-dap/FunctionBreakpoint.h (+2-2) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3-43) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+3-2) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+2-10) - (modified) lldb/tools/lldb-dap/SourceBreakpoint.h (+3-3) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+9-8) - (modified) llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn (+1) ``diff diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp new file mode 100644 index 0..4ccf353b06ccc --- /dev/null +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -0,0 +1,182 @@ +//===-- Breakpoint.cpp --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "Breakpoint.h" +#include "DAP.h" +#include "JSONUtils.h" +#include "llvm/ADT/StringExtras.h" + +using namespace lldb_dap; + +void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); } + +void Breakpoint::SetHitCondition() { + uint64_t hitCount = 0; + if (llvm::to_integer(hitCondition, hitCount)) +bp.SetIgnoreCount(hitCount - 1); +} + +// logMessage will be divided into array of LogMessagePart as two kinds: +// 1. raw print text message, and +// 2. interpolated expression for evaluation which is inside matching curly +//braces. +// +// The function tries to parse logMessage into a list of LogMessageParts +// for easy later access in BreakpointHitCallback. +void Breakpoint::SetLogMessage() { + logMessageParts.clear(); + + // Contains unmatched open curly braces indices. + std::vector unmatched_curly_braces; + + // Contains all matched curly braces in logMessage. + // Loop invariant: matched_curly_braces_ranges are sorted by start index in + // ascending order without any overlap between them. + std::vector> matched_curly_braces_ranges; + + lldb::SBError error; + // Part1 - parse matched_curly_braces_ranges. + // locating all curly braced expression ranges in logMessage. + // The algorithm takes care of nested and imbalanced curly braces. + for (size_t i = 0; i < logMessage.size(); ++i) { +if (logMessage[i] == '{') { + unmatched_curly_braces.push_back(i); +} else if (logMessage[i] == '}') { + if (unmatched_curly_braces.empty()) +// Nothing to match. +continue; + + int last_unmatched_index = unmatched_curly_braces.back(); + unmatched_curly_braces.pop_back(); + + // Erase any matched ranges included in the new match. + while (!matched_curly_braces_ranges.empty()) { +assert(matched_curly_braces_ranges.back().first != + last_unmatched_index && + "How can a curley brace be matched twice?"); +if (matched_curly_braces_ranges.back().first < last_unmatched_index) + break; + +// This is a nested range let's earse it. +assert((size_t)matched_curly_braces_ranges.back().second < i); +matched_curly_braces_ranges.pop_back(); + } + + // Assert invariant. + assert(matched_curly_braces_ranges.empty() || + matched_curly_braces_ranges.back().first < last_unmatched_index); + matched_curly_braces_ranges.emplace_back(last_unmatched_index, i); +} + } + + // Part2 - parse raw text and expresions parts. + // All expression ranges have been parsed in matched_curly_braces_ranges. + // The code below uses matched_curly_braces_ranges to divide logMessage + // into raw text parts and expression parts. + int last_raw_text_start = 0; + for (const std::pair &curly_braces_range : + matched_curly_braces_ranges) { +// Raw text before open curly brace. +assert(curly_braces_range.first >= last_raw_text_start); +size_t raw_text_len = curly_braces_r
[Lldb-commits] [llvm] [lldb] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/80753 This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and `BreakpointBase` to have better separation and encapsulation so we are not directly operating on `SBBreakpoint`. I basically moved the `SBBreakpoint` and the methods that requires it from `BreakpointBase` to `Breakpoint`. This allows adding support for data watchpoint easier by sharing the logic inside `BreakpointBase`. >From c4b767909a9ffc2a3015dc9021e4c265da0d877d Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Mon, 5 Feb 2024 17:26:48 -0500 Subject: [PATCH] [lldb-dap][NFC] Add Breakpoint struct to share common logic. --- lldb/tools/lldb-dap/Breakpoint.cpp| 182 ++ lldb/tools/lldb-dap/Breakpoint.h | 34 lldb/tools/lldb-dap/BreakpointBase.cpp| 113 --- lldb/tools/lldb-dap/BreakpointBase.h | 12 +- lldb/tools/lldb-dap/CMakeLists.txt| 1 + lldb/tools/lldb-dap/FunctionBreakpoint.cpp| 12 +- lldb/tools/lldb-dap/FunctionBreakpoint.h | 4 +- lldb/tools/lldb-dap/JSONUtils.cpp | 46 + lldb/tools/lldb-dap/JSONUtils.h | 5 +- lldb/tools/lldb-dap/SourceBreakpoint.cpp | 12 +- lldb/tools/lldb-dap/SourceBreakpoint.h| 6 +- lldb/tools/lldb-dap/lldb-dap.cpp | 17 +- .../gn/secondary/lldb/tools/lldb-dap/BUILD.gn | 1 + 13 files changed, 248 insertions(+), 197 deletions(-) create mode 100644 lldb/tools/lldb-dap/Breakpoint.cpp create mode 100644 lldb/tools/lldb-dap/Breakpoint.h diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp new file mode 100644 index 00..4ccf353b06 --- /dev/null +++ b/lldb/tools/lldb-dap/Breakpoint.cpp @@ -0,0 +1,182 @@ +//===-- Breakpoint.cpp --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "Breakpoint.h" +#include "DAP.h" +#include "JSONUtils.h" +#include "llvm/ADT/StringExtras.h" + +using namespace lldb_dap; + +void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); } + +void Breakpoint::SetHitCondition() { + uint64_t hitCount = 0; + if (llvm::to_integer(hitCondition, hitCount)) +bp.SetIgnoreCount(hitCount - 1); +} + +// logMessage will be divided into array of LogMessagePart as two kinds: +// 1. raw print text message, and +// 2. interpolated expression for evaluation which is inside matching curly +//braces. +// +// The function tries to parse logMessage into a list of LogMessageParts +// for easy later access in BreakpointHitCallback. +void Breakpoint::SetLogMessage() { + logMessageParts.clear(); + + // Contains unmatched open curly braces indices. + std::vector unmatched_curly_braces; + + // Contains all matched curly braces in logMessage. + // Loop invariant: matched_curly_braces_ranges are sorted by start index in + // ascending order without any overlap between them. + std::vector> matched_curly_braces_ranges; + + lldb::SBError error; + // Part1 - parse matched_curly_braces_ranges. + // locating all curly braced expression ranges in logMessage. + // The algorithm takes care of nested and imbalanced curly braces. + for (size_t i = 0; i < logMessage.size(); ++i) { +if (logMessage[i] == '{') { + unmatched_curly_braces.push_back(i); +} else if (logMessage[i] == '}') { + if (unmatched_curly_braces.empty()) +// Nothing to match. +continue; + + int last_unmatched_index = unmatched_curly_braces.back(); + unmatched_curly_braces.pop_back(); + + // Erase any matched ranges included in the new match. + while (!matched_curly_braces_ranges.empty()) { +assert(matched_curly_braces_ranges.back().first != + last_unmatched_index && + "How can a curley brace be matched twice?"); +if (matched_curly_braces_ranges.back().first < last_unmatched_index) + break; + +// This is a nested range let's earse it. +assert((size_t)matched_curly_braces_ranges.back().second < i); +matched_curly_braces_ranges.pop_back(); + } + + // Assert invariant. + assert(matched_curly_braces_ranges.empty() || + matched_curly_braces_ranges.back().first < last_unmatched_index); + matched_curly_braces_ranges.emplace_back(last_unmatched_index, i); +} + } + + // Part2 - parse raw text and expresions parts. + // All expression ranges have been parsed in matched_curly_braces_ranges. + // The code below uses matched_curly_braces_ranges to divide logMessage + // into raw text parts and expression parts. + int last_raw_text_start = 0; + for (const std::pa
[Lldb-commits] [lldb] [lldb] Add QSupported key to report watchpoint types supported (PR #80376)
https://github.com/JDevlieghere approved this pull request. LGTM if @DavidSpickett and @bulbazord are happy. https://github.com/llvm/llvm-project/pull/80376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Support statistics dump summary only mode (PR #80745)
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/80745 >From 30d723ba9808c7a8109b11dd0f20b4d8808e9ad5 Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Fri, 2 Feb 2024 15:42:01 -0800 Subject: [PATCH 1/2] Support statistics dump summary only mode Summary: Added a new --summary option to statistics dump command so that it is much light weight than the full version. With this change, statistics dump --summary can now be included in lldb command line telemetry without slowing down lldb exiting. --- lldb/include/lldb/API/SBTarget.h | 8 +- lldb/include/lldb/Target/Statistics.h | 9 +- lldb/include/lldb/Target/Target.h | 2 +- lldb/source/API/SBTarget.cpp | 9 +- lldb/source/Commands/CommandObjectStats.cpp | 8 +- lldb/source/Commands/Options.td | 3 + lldb/source/Target/Statistics.cpp | 194 +++--- lldb/source/Target/Target.cpp | 4 +- .../stats_api/TestStatisticsAPI.py| 15 ++ 9 files changed, 164 insertions(+), 88 deletions(-) diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h index 83087623088c5b..7fd888cf7014e3 100644 --- a/lldb/include/lldb/API/SBTarget.h +++ b/lldb/include/lldb/API/SBTarget.h @@ -86,9 +86,13 @@ class LLDB_API SBTarget { /// Returns a dump of the collected statistics. /// + /// \param[in] summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// A SBStructuredData with the statistics collected. - lldb::SBStructuredData GetStatistics(); + lldb::SBStructuredData GetStatistics(bool summary_only = false); /// Return the platform object associated with the target. /// @@ -326,7 +330,7 @@ class LLDB_API SBTarget { uint32_t GetAddressByteSize(); const char *GetTriple(); - + const char *GetABIName(); const char *GetLabel() const; diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index f672786f58f84d..98658ba0cac317 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -133,7 +133,7 @@ struct ConstStringStats { /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: - llvm::json::Value ToJSON(Target &target); + llvm::json::Value ToJSON(Target &target, bool summary_only = false); void SetLaunchOrAttachTime(); void SetFirstPrivateStopTime(); @@ -171,9 +171,14 @@ class DebuggerStats { /// The single target to emit statistics for if non NULL, otherwise dump /// statistics only for the specified target. /// + /// \param summary_only + /// If true, only report high level summary statistics without + /// targets/modules/breakpoints etc.. details. + /// /// \return /// Returns a JSON value that contains all target metrics. - static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target); + static llvm::json::Value ReportStatistics(Debugger &debugger, Target *target, +bool summary_only = false); protected: // Collecting stats can be set to true to collect stats that are expensive diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index c37682e2a03859..4bf6c123dc1ddc 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1599,7 +1599,7 @@ class Target : public std::enable_shared_from_this, /// /// \return /// Returns a JSON value that contains all target metrics. - llvm::json::Value ReportStatistics(); + llvm::json::Value ReportStatistics(bool summary_only = false); TargetStats &GetStatistics() { return m_stats; } diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 8e616afbcb4e8d..615a00ceeaee16 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -197,7 +197,7 @@ SBDebugger SBTarget::GetDebugger() const { return debugger; } -SBStructuredData SBTarget::GetStatistics() { +SBStructuredData SBTarget::GetStatistics(bool summary_only) { LLDB_INSTRUMENT_VA(this); SBStructuredData data; @@ -205,9 +205,10 @@ SBStructuredData SBTarget::GetStatistics() { if (!target_sp) return data; std::string json_str = - llvm::formatv("{0:2}", - DebuggerStats::ReportStatistics(target_sp->GetDebugger(), - target_sp.get())).str(); + llvm::formatv( + "{0:2}", DebuggerStats::ReportStatistics( + target_sp->GetDebugger(), target_sp.get(), summary_only)) + .str(); data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str)); return data; } diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index 262de0bda144a6..781b90794dc377 100644 ---
[Lldb-commits] [lldb] [mlir] [llvm] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
@@ -220,6 +220,31 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever pzread wrote: I think whether the loop rotation is needed (or possible) is the implementation details of each type of loop. `scf.while` needs it because the before block is in the loop and can contain ops with side-effects. `scf.for` is simpler as the `lb < ub` should have no side-effect and lightweight, which can be run twice (for `scf.if` and the for loop) https://github.com/llvm/llvm-project/pull/80331 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [mlir] [llvm] [lldb] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
@@ -220,6 +220,31 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever +run and return the new loop inside the check. The loop body is moved +over to the new loop. Returns "failure" if the loop doesn't support +this transformation. + +After the transformation, the ops inserted to the parent region of the +loop are guaranteed to be run only if the loop body runs at least one +iteration. + +Note: Ops in the loop body might be rearranged because of loop rotating +to maintain the semantic. Terminators might be removed/added during this +transformation. Also callers are not required to check the side-effect +of loop condition, so the transformation needs to consider that to make +sure the loop behavior is unchanged when moving the condtion out of the pzread wrote: Fixed. https://github.com/llvm/llvm-project/pull/80331 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [mlir] [llvm] [lldb] [mlir] Introduce replaceWithZeroTripCheck in LoopLikeOpInterface (PR #80331)
https://github.com/pzread updated https://github.com/llvm/llvm-project/pull/80331 >From 70f54b51bef87bde5e3f5ee067c0f2414d34e915 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Thu, 1 Feb 2024 19:57:26 + Subject: [PATCH 1/4] Add replaceWithZeroTripCheck to LoopLikeOpInterface --- .../mlir/Interfaces/LoopLikeInterface.td | 22 +++ 1 file changed, 22 insertions(+) diff --git a/mlir/include/mlir/Interfaces/LoopLikeInterface.td b/mlir/include/mlir/Interfaces/LoopLikeInterface.td index e2ac85a3f7725d..77409cb3a8274b 100644 --- a/mlir/include/mlir/Interfaces/LoopLikeInterface.td +++ b/mlir/include/mlir/Interfaces/LoopLikeInterface.td @@ -220,6 +220,28 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> { /*defaultImplementation=*/[{ return ::mlir::failure(); }] +>, +InterfaceMethod<[{ +Add a zero-trip-check around the loop to check if the loop body is ever +run and return the new loop inside the check. The loop body is moved +over to the new loop. Returns "failure" if the loop doesn't support +this transformation. + +After the transformation, the ops inserted to the parent region of the +loop are guaranteed to be run only if the loop body runs at least one +iteration. + +Note: Ops in the loop body might be rearranged because of loop rotating +to maintain the semantic. Terminators might be removed/added during this +transformation. + }], + /*retTy=*/"::mlir::FailureOr<::mlir::LoopLikeOpInterface>", + /*methodName=*/"replaceWithZeroTripCheck", + /*args=*/(ins "::mlir::RewriterBase &":$rewriter), + /*methodBody=*/"", + /*defaultImplementation=*/[{ +return ::mlir::failure(); + }] > ]; >From d6703ebbeb5ddc358929672b44994a9d05683523 Mon Sep 17 00:00:00 2001 From: Jerry Wu Date: Fri, 2 Feb 2024 18:59:03 + Subject: [PATCH 2/4] Add tests --- mlir/unittests/Interfaces/CMakeLists.txt | 3 + .../Interfaces/LoopLikeInterfaceTest.cpp | 101 ++ 2 files changed, 104 insertions(+) create mode 100644 mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp diff --git a/mlir/unittests/Interfaces/CMakeLists.txt b/mlir/unittests/Interfaces/CMakeLists.txt index d192b2922d6b9d..cab9503cf295b1 100644 --- a/mlir/unittests/Interfaces/CMakeLists.txt +++ b/mlir/unittests/Interfaces/CMakeLists.txt @@ -3,6 +3,7 @@ add_mlir_unittest(MLIRInterfacesTests DataLayoutInterfacesTest.cpp InferIntRangeInterfaceTest.cpp InferTypeOpInterfaceTest.cpp + LoopLikeInterfaceTest.cpp ) target_link_libraries(MLIRInterfacesTests @@ -12,7 +13,9 @@ target_link_libraries(MLIRInterfacesTests MLIRDataLayoutInterfaces MLIRDLTIDialect MLIRFuncDialect + MLIRIR MLIRInferIntRangeInterface MLIRInferTypeOpInterface + MLIRLoopLikeInterface MLIRParser ) diff --git a/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp new file mode 100644 index 00..b0b7680fed68e7 --- /dev/null +++ b/mlir/unittests/Interfaces/LoopLikeInterfaceTest.cpp @@ -0,0 +1,101 @@ +//===- LoopLikeInterfaceTest.cpp - Unit tests for Loop Like Interfaces. ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "mlir/Interfaces/LoopLikeInterface.h" +#include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Dialect.h" +#include "mlir/IR/DialectImplementation.h" +#include "mlir/IR/OpDefinition.h" +#include "mlir/IR/OpImplementation.h" +#include "mlir/IR/PatternMatch.h" +#include "mlir/Parser/Parser.h" + +#include + +using namespace mlir; + +struct NoZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.no_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } +}; + +struct ImplZeroTripCheckLoopOp +: public Op { + using Op::Op; + + static ArrayRef getAttributeNames() { return {}; } + + static StringRef getOperationName() { +return "looptest.impl_zero_trip_check_loop_op"; + } + + SmallVector getLoopRegions() { return {}; } + + FailureOr + replaceWithZeroTripCheck(RewriterBase &rewriter) { +return cast(this->getOperation()); + } +}; + +/// A dialect putting all the above together. +struct LoopTestDialect : Dialect { + explicit LoopTestDialect(MLIRContext *ctx) + : Dialect(getDialectNamespace(), ctx, TypeID::get()) { +addOperations(); + } + static StringRef getDialectNamespace() { return "looptest"; } +}; + +TEST(LoopLikeOpInterface, NoReplaceWithZeroTripCheck) { + const char *ir = R"MLIR( + "looptest.no_zero_trip_check_loop_op"() : () -> () + )ML
[Lldb-commits] [libcxxabi] [lldb] [clang] [flang] [compiler-rt] [lld] [libc] [clang-tools-extra] [libcxx] [llvm] [SLP]Improve findReusedOrderedScalars and graph rotation. (PR #77529)
https://github.com/alexey-bataev updated https://github.com/llvm/llvm-project/pull/77529 >From 7440ee8ba235fd871af0999f66d5d6130456400b Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Tue, 9 Jan 2024 21:43:31 + Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.5 --- .../Transforms/Vectorize/SLPVectorizer.cpp| 476 ++ .../AArch64/extractelements-to-shuffle.ll | 16 +- .../AArch64/reorder-fmuladd-crash.ll | 7 +- .../SLPVectorizer/AArch64/tsc-s116.ll | 22 +- .../Transforms/SLPVectorizer/X86/pr35497.ll | 16 +- .../SLPVectorizer/X86/reduction-transpose.ll | 16 +- .../X86/reorder-clustered-node.ll | 11 +- .../X86/reorder-reused-masked-gather.ll | 7 +- .../SLPVectorizer/X86/reorder-vf-to-resize.ll | 2 +- .../X86/scatter-vectorize-reorder.ll | 17 +- .../X86/shrink_after_reorder2.ll | 11 +- 11 files changed, 445 insertions(+), 156 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp index 8e22b54f002d1c..4765cef290b9df 100644 --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp @@ -858,7 +858,7 @@ static void addMask(SmallVectorImpl &Mask, ArrayRef SubMask, /// values 3 and 7 respectively: /// before: 6 9 5 4 9 2 1 0 /// after: 6 3 5 4 7 2 1 0 -static void fixupOrderingIndices(SmallVectorImpl &Order) { +static void fixupOrderingIndices(MutableArrayRef Order) { const unsigned Sz = Order.size(); SmallBitVector UnusedIndices(Sz, /*t=*/true); SmallBitVector MaskedIndices(Sz); @@ -2418,7 +2418,8 @@ class BoUpSLP { std::optional isGatherShuffledSingleRegisterEntry( const TreeEntry *TE, ArrayRef VL, MutableArrayRef Mask, - SmallVectorImpl &Entries, unsigned Part); + SmallVectorImpl &Entries, unsigned Part, + bool ForOrder); /// Checks if the gathered \p VL can be represented as multi-register /// shuffle(s) of previous tree entries. @@ -2432,7 +2433,7 @@ class BoUpSLP { isGatherShuffledEntry( const TreeEntry *TE, ArrayRef VL, SmallVectorImpl &Mask, SmallVectorImpl> &Entries, - unsigned NumParts); + unsigned NumParts, bool ForOrder = false); /// \returns the scalarization cost for this list of values. Assuming that /// this subtree gets vectorized, we may need to extract the values from the @@ -3756,65 +3757,169 @@ static void reorderOrder(SmallVectorImpl &Order, ArrayRef Mask) { std::optional BoUpSLP::findReusedOrderedScalars(const BoUpSLP::TreeEntry &TE) { assert(TE.State == TreeEntry::NeedToGather && "Expected gather node only."); - unsigned NumScalars = TE.Scalars.size(); + // Try to find subvector extract/insert patterns and reorder only such + // patterns. + SmallVector GatheredScalars(TE.Scalars.begin(), TE.Scalars.end()); + Type *ScalarTy = GatheredScalars.front()->getType(); + int NumScalars = GatheredScalars.size(); + if (!isValidElementType(ScalarTy)) +return std::nullopt; + auto *VecTy = FixedVectorType::get(ScalarTy, NumScalars); + int NumParts = TTI->getNumberOfParts(VecTy); + if (NumParts == 0 || NumParts >= NumScalars) +NumParts = 1; + SmallVector ExtractMask; + SmallVector Mask; + SmallVector> Entries; + SmallVector> ExtractShuffles = + tryToGatherExtractElements(GatheredScalars, ExtractMask, NumParts); + SmallVector> GatherShuffles = + isGatherShuffledEntry(&TE, GatheredScalars, Mask, Entries, NumParts, +/*ForOrder=*/true); + // No shuffled operands - ignore. + if (GatherShuffles.empty() && ExtractShuffles.empty()) +return std::nullopt; OrdersType CurrentOrder(NumScalars, NumScalars); - SmallVector Positions; - SmallBitVector UsedPositions(NumScalars); - const TreeEntry *STE = nullptr; - // Try to find all gathered scalars that are gets vectorized in other - // vectorize node. Here we can have only one single tree vector node to - // correctly identify order of the gathered scalars. - for (unsigned I = 0; I < NumScalars; ++I) { -Value *V = TE.Scalars[I]; -if (!isa(V)) - continue; -if (const auto *LocalSTE = getTreeEntry(V)) { - if (!STE) -STE = LocalSTE; - else if (STE != LocalSTE) -// Take the order only from the single vector node. -return std::nullopt; - unsigned Lane = - std::distance(STE->Scalars.begin(), find(STE->Scalars, V)); - if (Lane >= NumScalars) -return std::nullopt; - if (CurrentOrder[Lane] != NumScalars) { -if (Lane != I) + if (GatherShuffles.size() == 1 && + *GatherShuffles.front() == TTI::SK_PermuteSingleSrc && + Entries.front().front()->isSame(TE.Scalars)) { +// Exclude nodes for strided geps from analysis, bett