[Lldb-commits] [lldb] [lldb] Add support for changing char in Scalar::SetValueFromCString (PR #67784)
kpdev wrote: > This seems like a somewhat limited way to poke a character into the value if > the string has more than one character already in it. > > If you are trying to do more fancy setting of the contents of an SBValue, > then it would be more straightforward to get the SBData for the value with > GetData, then you have access to the actual bytes in the data, and you can > poke in values wherever you want. I think that might be a better approach > than trying to get SetValueFromCString to handle changing single character > ValueObjects. The main purpose of all these patches is to be able to update string value during debug process in IDE (vscode for example). LLDB communication with IDE is not straight, it uses some external tools for this, e.g.: When we want to change value in the vscode, firstly vscode send request to lldb-mi through `Debug Adapter Protocol`, then lldb-mi asks lldb to change received value through `SetValueFromCString` api, so if we would like to avoid using this api - we need to add such support on the lldb-mi side. But it is not necessary, that IDE will communicate to the lldb-mi, it can send requests to any tool which supports DAP, and this tool will probably use `SetValueFromCString` api https://github.com/llvm/llvm-project/pull/67784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add support for updating string during debug process (PR #67782)
kpdev wrote: > What is it about this change that is defeating the ValueObject printer from > compressing this output onto one line? It looks like the contents that get > printed are the same, so there's something about switching from a Summary > provider to a child provider that's causing problems. We should fix that as > people are really picky about variable printing being as space efficient as > possible. > To clear up terminology... Strings had data formatters before AND after this > change. The difference is that you've switched from a "Summary Provider" data > formatter to a "Synthetic Child Provider" data formatter. > > It looks like you've made the printing of std::strings less space efficient. > That shouldn't be necessary, and isn't desirable. We should figure out why > that's happening and fix that before this change is going to not cause > complaints. As there is mentioned in a comment for `FormatManager::ShouldPrintAsOneLiner` ( https://github.com/llvm/llvm-project/blob/main/lldb/source/DataFormatters/FormatManager.cpp#L498 ): ``` // if we decided to define synthetic children for a type, we probably care // enough to show them, but avoid nesting children in children ``` So, there is a condition for that: ``` // but if we only have them to provide a value, keep going if (!synth_sp->MightHaveChildren() && synth_sp->DoesProvideSyntheticValue()) is_synth_val = true; else return false; ``` This patch adds StringSynthetic and this synthetic `MightHaveChildren()` is `true`, therefore `ShouldPrintAsOneLiner` returns `false`. And the printer will use `"()"` or `"{\n ... \n}"` according to whether we return `true` or `false` from this function. If we would like to avoid unnecessary output we probably may ask SyntheticFrontend directly if it would like to print expanded info or not, what do you think about it? https://github.com/llvm/llvm-project/pull/67782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add support for updating string during debug process (PR #67782)
kpdev wrote: > BTW, I have no problem with the general direction of this change. It makes a > lot more sense to ask a synthetic child provider to change a value - since it > does represent the value of the ValueObject - rather than the summary which > is just some free-form text. And being able to change characters in a string > seems a reasonable thing to do, so switching the std::string comprehension > from a Summary provider to a Synthetic Child Provider is the way to do that. > So that part if fine. > > But std::strings abound in code, and so if we're going to make this change I > don't think we can make that printing less space efficient, which this change > seems to have done. We should figure out why that's the case and fix for this > to be a really good change. So, is it Ok to use current `SetValueFromCString` api from ValueObject to ask synthetic provider to update the underlying string? You mentioned previously that we may add `SetSummaryFromCString` api - in fact currently this is what I am doing - changing the whole string through its summary (please check attached gif for example). But the problem with the new API is the same as for the changing characters through `SBValue::GetData` - IDE doesn't support it https://github.com/llvm/llvm-project/pull/67782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-vscode] Update installation instructions (PR #68234)
DavidSpickett wrote: Some general comments but otherwise I followed the instructions myself and it worked fine. I'll let @clayborg give the final ok, since I am new to this stuff. https://github.com/llvm/llvm-project/pull/68234 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Extract DW_AT_data_member_location calculation logic (PR #68231)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/68231 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3a35ca0 - [lldb][DWARFASTParserClang][NFCI] Extract DW_AT_data_member_location calculation logic (#68231)
Author: Michael Buch Date: 2023-10-05T10:49:42+01:00 New Revision: 3a35ca01fc55f27315d1652ec1dedff10e79918b URL: https://github.com/llvm/llvm-project/commit/3a35ca01fc55f27315d1652ec1dedff10e79918b DIFF: https://github.com/llvm/llvm-project/commit/3a35ca01fc55f27315d1652ec1dedff10e79918b.diff LOG: [lldb][DWARFASTParserClang][NFCI] Extract DW_AT_data_member_location calculation logic (#68231) Currently this non-trivial calculation is repeated multiple times, making it hard to reason about when the `byte_offset`/`member_byte_offset` is being set or not. This patch simply moves all those instances of the same calculation into a helper function. We return an optional to remain an NFC patch. Default initializing the offset would make sense but requires further analysis and can be done in a follow-up patch. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 37fb16d4e0351c9..005711d6f488c7f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -519,6 +519,33 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc, return UpdateSymbolContextScopeForType(sc, die, type_sp); } +static std::optional +ExtractDataMemberLocation(DWARFDIE const &die, DWARFFormValue const &form_value, + ModuleSP module_sp) { + // With DWARF 3 and later, if the value is an integer constant, + // this form value is the offset in bytes from the beginning of + // the containing entity. + if (!form_value.BlockData()) +return form_value.Unsigned(); + + Value initialValue(0); + Value memberOffset(0); + const DWARFDataExtractor &debug_info_data = die.GetData(); + uint32_t block_length = form_value.Unsigned(); + uint32_t block_offset = + form_value.BlockData() - debug_info_data.GetDataStart(); + if (!DWARFExpression::Evaluate( + nullptr, // ExecutionContext * + nullptr, // RegisterContext * + module_sp, DataExtractor(debug_info_data, block_offset, block_length), + die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr, memberOffset, + nullptr)) { +return {}; + } + + return memberOffset.ResolveValue(nullptr).UInt(); +} + lldb::TypeSP DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc, const DWARFDIE &die, @@ -1406,26 +1433,9 @@ void DWARFASTParserClang::ParseInheritance( encoding_form = form_value; break; case DW_AT_data_member_location: -if (form_value.BlockData()) { - Value initialValue(0); - Value memberOffset(0); - const DWARFDataExtractor &debug_info_data = die.GetData(); - uint32_t block_length = form_value.Unsigned(); - uint32_t block_offset = - form_value.BlockData() - debug_info_data.GetDataStart(); - if (DWARFExpression::Evaluate( - nullptr, nullptr, module_sp, - DataExtractor(debug_info_data, block_offset, block_length), - die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr, - memberOffset, nullptr)) { -member_byte_offset = memberOffset.ResolveValue(nullptr).UInt(); - } -} else { - // With DWARF 3 and later, if the value is an integer constant, - // this form value is the offset in bytes from the beginning of - // the containing entity. - member_byte_offset = form_value.Unsigned(); -} +if (auto maybe_offset = +ExtractDataMemberLocation(die, form_value, module_sp)) + member_byte_offset = *maybe_offset; break; case DW_AT_accessibility: @@ -2557,29 +2567,9 @@ VariantMember::VariantMember(DWARFDIE &die, lldb::ModuleSP module_sp) { break; case DW_AT_data_member_location: -if (form_value.BlockData()) { - Value initialValue(0); - Value memberOffset(0); - const DWARFDataExtractor &debug_info_data = die.GetData(); - uint32_t block_length = form_value.Unsigned(); - uint32_t block_offset = - form_value.BlockData() - debug_info_data.GetDataStart(); - if (DWARFExpression::Evaluate( - nullptr, // ExecutionContext * - nullptr, // RegisterContext * - module_sp, - DataExtractor(debug_info_data, block_offset, -block_length), - die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr, - memberOffset, nullptr)) { -
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
@@ -340,6 +340,44 @@ BasicBlock *llvm::splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch, return splitBB(Builder, CreateBranch, Old->getName() + Suffix); } +// This function creates a fake integer value and a fake use for the integer +// value. It returns the fake value created. This is useful in modeling the +// extra arguments to the outlined functions. +Value *createFakeIntVal(IRBuilder<> &Builder, +OpenMPIRBuilder::InsertPointTy OuterAllocaIP, +std::stack &ToBeDeleted, +OpenMPIRBuilder::InsertPointTy InnerAllocaIP, +const Twine &Name = "", bool AsPtr = true) { + Builder.restoreIP(OuterAllocaIP); + Instruction *FakeVal; + AllocaInst *FakeValAddr = + Builder.CreateAlloca(Builder.getInt32Ty(), nullptr, Name + ".addr"); + ToBeDeleted.push(FakeValAddr); + + if (AsPtr) +FakeVal = FakeValAddr; + else { kiranchandramohan wrote: Nit: braces to match else. Same for tid below. https://github.com/llvm/llvm-project/pull/67723 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
@@ -5748,6 +5758,7 @@ OpenMPIRBuilder::createTeams(const LocationDescription &Loc, BasicBlock *BodyBB = splitBB(Builder, /*CreateBranch=*/true, "teams.entry"); Builder.SetInsertPoint(BodyBB, BodyBB->begin()); } + InsertPointTy OuterAllocaIP(&OuterAllocaBB, OuterAllocaBB.begin()); kiranchandramohan wrote: Can this be defined close to its use? https://github.com/llvm/llvm-project/pull/67723 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
https://github.com/kiranchandramohan edited https://github.com/llvm/llvm-project/pull/67723 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
@@ -340,6 +340,44 @@ BasicBlock *llvm::splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch, return splitBB(Builder, CreateBranch, Old->getName() + Suffix); } +// This function creates a fake integer value and a fake use for the integer +// value. It returns the fake value created. This is useful in modeling the +// extra arguments to the outlined functions. +Value *createFakeIntVal(IRBuilder<> &Builder, +OpenMPIRBuilder::InsertPointTy OuterAllocaIP, +std::stack &ToBeDeleted, +OpenMPIRBuilder::InsertPointTy InnerAllocaIP, +const Twine &Name = "", bool AsPtr = true) { + Builder.restoreIP(OuterAllocaIP); + Instruction *FakeVal; + AllocaInst *FakeValAddr = + Builder.CreateAlloca(Builder.getInt32Ty(), nullptr, Name + ".addr"); + ToBeDeleted.push(FakeValAddr); + + if (AsPtr) +FakeVal = FakeValAddr; + else { +FakeVal = +Builder.CreateLoad(Builder.getInt32Ty(), FakeValAddr, Name + ".val"); +ToBeDeleted.push(FakeVal); kiranchandramohan wrote: Would this delete twice for the `AsPtr` case? https://github.com/llvm/llvm-project/pull/67723 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [AMDGPU] Add another SIFoldOperands instance after shrink (PR #67878)
jayfoad wrote: I've taken another look at this. The patch does not show any benefit from running another `SIFoldOperands` pass _after_ `SIShrinkInstructions` per se; you get exactly the same results (modulo a couple of add instructions that have their operands commuted differently) if you put the second `SIFoldOperands` run _before_ `SIShrinkInstructions` instead. In other words `SIFoldOperands` is not idempotent, and the reason for the that seems to be: > And the reason it only happens for some SUBREV instructions is even more > convoluted. It's because SIFoldOperands will sometimes shrink > V_SUB_CO_U32_e64 to V_SUBREV_CO_U32_e32 even it does not manage to fold > anything into it. This does seem wrong and is probably worth a closer look. This goes back to https://reviews.llvm.org/D51345. Notice how the code that was added to `updateOperand` does the shrinking but does not actually do any folding; it returns before we get to `Old.ChangeToImmediate`/`Old.substVirtReg`. A second run of `SIFoldOperands` will see the shrunk instruction and fold into it. https://github.com/llvm/llvm-project/pull/67878 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [InstCombine] Simplify the pattern `a ne/eq (zext/sext (a ne/eq c))` (PR #65852)
dtcxzyw wrote: Ping. https://github.com/llvm/llvm-project/pull/65852 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/68300 **Background** Prior to DWARFv4, there was no clear normative text on how to handle static data members. Non-normative text suggested we compilers should use `DW_AT_external` to mark static data members of structrues/unions. Clang does this consistently. However, GCC doesn't, e.g., when the structure/union is in an anonymous namespace (which is C++ standard conformant). Additionally, GCC never emits `DW_AT_data_member_location`s for union members (regardless of storage linkage and storage duration). Since DWARFv5 (issue 161118.1), static data members get emitted as `DW_TAG_variable`. LLDB used to differentiate between static and non-static members by checking the `DW_AT_external` flag and the absence of `DW_AT_data_member_location`. With D18008 LLDB started to pretend that union members always have a `0` `DW_AT_data_member_location` by default (because GCC never emits these locations). In D124409 LLDB stopped checking the `DW_AT_external` flag to account for the case where GCC doesn't emit the flag for types in anonymous namespaces; instead we only check for presence of `DW_AT_data_member_location`s. The combination of these changes then meant that LLDB would never correctly detect that a union has static data members. **Solution** Instead of unconditionally initializing the `member_byte_offset` to `0` specifically for union members, this patch proposes to check for both the absence of `DW_AT_data_member_location` and `DW_AT_declaration`, which consistently gets emitted for static data members on GCC and Clang. We initialize the `member_byte_offset` to `0` anyway if we determine it wasn't a static. So removing the special case for unions makes this code simpler to reason about. Long-term, we should just use DWARFv5's new representation for static data members. Fixes #68135 >From 30ef50b808a8458a60bbd3cdc52b866ee296b6ba Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 5 Oct 2023 12:13:12 +0100 Subject: [PATCH] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members **Background** Prior to DWARFv4, there was no clear normative text on how to handle static data members. Non-normative text suggested we compilers should use `DW_AT_external` to mark static data members of structrues/unions. Clang does this consistently. However, GCC doesn't, e.g., when the structure/union is in an anonymous namespace (which is C++ standard conformant). Additionally, GCC never emits `DW_AT_data_member_location`s for union members (regardless of storage linkage and storage duration). Since DWARFv5 (issue 161118.1), static data members get emitted as `DW_TAG_variable`. LLDB used to differentiate between static and non-static members by checking the `DW_AT_external` flag and the absence of `DW_AT_data_member_location`. With D18008 LLDB started to pretend that union members always have a `0` `DW_AT_data_member_location` by default (because GCC never emits these locations). In D124409 LLDB stopped checking the `DW_AT_external` flag to account for the case where GCC doesn't emit the flag for types in anonymous namespaces; instead we only check for presence of `DW_AT_data_member_location`s. The combination of these changes then meant that LLDB would never correctly detect that a union has static data members. **Solution** Instead of unconditionally initializing the `member_byte_offset` to `0` specifically for union members, this patch proposes to check for both the absence of `DW_AT_data_member_location` and `DW_AT_declaration`, which consistently gets emitted for static data members on GCC and Clang. We initialize the `member_byte_offset` to `0` anyway if we determine it wasn't a static. So removing the special case for unions makes this code simpler to reason about. Long-term, we should just use DWARFv5's new representation for static data members. Fixes #68135 --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 22 +++--- .../cpp/union-static-data-members/Makefile| 3 ++ .../TestCppUnionStaticMembers.py | 43 +++ .../cpp/union-static-data-members/main.cpp| 25 +++ 4 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/Makefile create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/main.cpp diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 37fb16d4e0351c9..ee35a7de80c1e18 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2482,8 +2482,9 @@ struct MemberAttributes { DWARFFormValue encoding_form; /// Indicates the byte offset of the word from the base addre
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
Michael137 wrote: Alternatively, we could start checking `DW_AT_external` again, at the cost of not supporting some GCC cases pre-DWARFv5 https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
llvmbot wrote: @llvm/pr-subscribers-lldb Changes **Background** Prior to DWARFv4, there was no clear normative text on how to handle static data members. Non-normative text suggested we compilers should use `DW_AT_external` to mark static data members of structrues/unions. Clang does this consistently. However, GCC doesn't, e.g., when the structure/union is in an anonymous namespace (which is C++ standard conformant). Additionally, GCC never emits `DW_AT_data_member_location`s for union members (regardless of storage linkage and storage duration). Since DWARFv5 (issue 161118.1), static data members get emitted as `DW_TAG_variable`. LLDB used to differentiate between static and non-static members by checking the `DW_AT_external` flag and the absence of `DW_AT_data_member_location`. With D18008 LLDB started to pretend that union members always have a `0` `DW_AT_data_member_location` by default (because GCC never emits these locations). In D124409 LLDB stopped checking the `DW_AT_external` flag to account for the case where GCC doesn't emit the flag for types in anonymous namespaces; instead we only check for presence of `DW_AT_data_member_location`s. The combination of these changes then meant that LLDB would never correctly detect that a union has static data members. **Solution** Instead of unconditionally initializing the `member_byte_offset` to `0` specifically for union members, this patch proposes to check for both the absence of `DW_AT_data_member_location` and `DW_AT_declaration`, which consistently gets emitted for static data members on GCC and Clang. We initialize the `member_byte_offset` to `0` anyway if we determine it wasn't a static. So removing the special case for unions makes this code simpler to reason about. Long-term, we should just use DWARFv5's new representation for static data members. Fixes #68135 --- Full diff: https://github.com/llvm/llvm-project/pull/68300.diff 4 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+16-6) - (added) lldb/test/API/lang/cpp/union-static-data-members/Makefile (+3) - (added) lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py (+43) - (added) lldb/test/API/lang/cpp/union-static-data-members/main.cpp (+25) ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 37fb16d4e0351c9..ee35a7de80c1e18 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2482,8 +2482,9 @@ struct MemberAttributes { DWARFFormValue encoding_form; /// Indicates the byte offset of the word from the base address of the /// structure. - uint32_t member_byte_offset; + uint32_t member_byte_offset = UINT32_MAX; bool is_artificial = false; + bool is_declaration = false; }; /// Parsed form of all attributes that are relevant for parsing Objective-C @@ -2656,8 +2657,6 @@ DiscriminantValue &VariantPart::discriminant() { return this->_discriminant; } MemberAttributes::MemberAttributes(const DWARFDIE &die, const DWARFDIE &parent_die, ModuleSP module_sp) { - member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX; - DWARFAttributes attributes = die.GetAttributes(); for (size_t i = 0; i < attributes.Size(); ++i) { const dw_attr_t attr = attributes.AttributeAtIndex(i); @@ -2717,6 +2716,9 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die, case DW_AT_artificial: is_artificial = form_value.Boolean(); break; + case DW_AT_declaration: +is_declaration = form_value.Boolean(); +break; default: break; } @@ -2923,10 +2925,18 @@ void DWARFASTParserClang::ParseSingleMember( if (class_is_objc_object_or_interface) attrs.accessibility = eAccessNone; - // Handle static members, which is any member that doesn't have a bit or a - // byte member offset. + // Handle static members, which are typically members without + // locations. However, GCC *never* emits DW_AT_data_member_location + // for static data members of unions. + // Non-normative text pre-DWARFv5 recommends marking static + // data members with an DW_AT_external flag. Clang emits this consistently + // whereas GCC emits it only for static data members if not part of an + // anonymous namespace. The flag that is consistently emitted for static + // data members is DW_AT_declaration, so we check it instead. + // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we can + // consistently detect them on both GCC and Clang without below heuristic. if (attrs.member_byte_offset == UINT32_MAX && - attrs.data_bit_offset == UINT64_MAX) { + attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
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 777a6e6f10b2b90496d248b7fa904fce834484be 30ef50b808a8458a60bbd3cdc52b866ee296b6ba -- lldb/test/API/lang/cpp/union-static-data-members/main.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index ee35a7de80c1..436632473816 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2933,8 +2933,8 @@ void DWARFASTParserClang::ParseSingleMember( // whereas GCC emits it only for static data members if not part of an // anonymous namespace. The flag that is consistently emitted for static // data members is DW_AT_declaration, so we check it instead. - // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we can - // consistently detect them on both GCC and Clang without below heuristic. + // FIXME: Since DWARFv5, static data members are marked DW_AT_variable so we + // can consistently detect them on both GCC and Clang without below heuristic. if (attrs.member_byte_offset == UINT32_MAX && attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) { Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference()); `` https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/68300 >From 30ef50b808a8458a60bbd3cdc52b866ee296b6ba Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 5 Oct 2023 12:13:12 +0100 Subject: [PATCH 1/2] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members **Background** Prior to DWARFv4, there was no clear normative text on how to handle static data members. Non-normative text suggested we compilers should use `DW_AT_external` to mark static data members of structrues/unions. Clang does this consistently. However, GCC doesn't, e.g., when the structure/union is in an anonymous namespace (which is C++ standard conformant). Additionally, GCC never emits `DW_AT_data_member_location`s for union members (regardless of storage linkage and storage duration). Since DWARFv5 (issue 161118.1), static data members get emitted as `DW_TAG_variable`. LLDB used to differentiate between static and non-static members by checking the `DW_AT_external` flag and the absence of `DW_AT_data_member_location`. With D18008 LLDB started to pretend that union members always have a `0` `DW_AT_data_member_location` by default (because GCC never emits these locations). In D124409 LLDB stopped checking the `DW_AT_external` flag to account for the case where GCC doesn't emit the flag for types in anonymous namespaces; instead we only check for presence of `DW_AT_data_member_location`s. The combination of these changes then meant that LLDB would never correctly detect that a union has static data members. **Solution** Instead of unconditionally initializing the `member_byte_offset` to `0` specifically for union members, this patch proposes to check for both the absence of `DW_AT_data_member_location` and `DW_AT_declaration`, which consistently gets emitted for static data members on GCC and Clang. We initialize the `member_byte_offset` to `0` anyway if we determine it wasn't a static. So removing the special case for unions makes this code simpler to reason about. Long-term, we should just use DWARFv5's new representation for static data members. Fixes #68135 --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 22 +++--- .../cpp/union-static-data-members/Makefile| 3 ++ .../TestCppUnionStaticMembers.py | 43 +++ .../cpp/union-static-data-members/main.cpp| 25 +++ 4 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/Makefile create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/TestCppUnionStaticMembers.py create mode 100644 lldb/test/API/lang/cpp/union-static-data-members/main.cpp diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 37fb16d4e0351c9..ee35a7de80c1e18 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2482,8 +2482,9 @@ struct MemberAttributes { DWARFFormValue encoding_form; /// Indicates the byte offset of the word from the base address of the /// structure. - uint32_t member_byte_offset; + uint32_t member_byte_offset = UINT32_MAX; bool is_artificial = false; + bool is_declaration = false; }; /// Parsed form of all attributes that are relevant for parsing Objective-C @@ -2656,8 +2657,6 @@ DiscriminantValue &VariantPart::discriminant() { return this->_discriminant; } MemberAttributes::MemberAttributes(const DWARFDIE &die, const DWARFDIE &parent_die, ModuleSP module_sp) { - member_byte_offset = (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX; - DWARFAttributes attributes = die.GetAttributes(); for (size_t i = 0; i < attributes.Size(); ++i) { const dw_attr_t attr = attributes.AttributeAtIndex(i); @@ -2717,6 +2716,9 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die, case DW_AT_artificial: is_artificial = form_value.Boolean(); break; + case DW_AT_declaration: +is_declaration = form_value.Boolean(); +break; default: break; } @@ -2923,10 +2925,18 @@ void DWARFASTParserClang::ParseSingleMember( if (class_is_objc_object_or_interface) attrs.accessibility = eAccessNone; - // Handle static members, which is any member that doesn't have a bit or a - // byte member offset. + // Handle static members, which are typically members without + // locations. However, GCC *never* emits DW_AT_data_member_location + // for static data members of unions. + // Non-normative text pre-DWARFv5 recommends marking static + // data members with an DW_AT_external flag. Clang emits this consistently + // whereas GCC emits it only for static data members if not part of an + // anonymous namespace. The flag that is consistently emitted for static + // data member
[Lldb-commits] [lldb] [Support] Add KnownBits::computeForSubBorrow (PR #67788)
https://github.com/christiankissig updated https://github.com/llvm/llvm-project/pull/67788 >From 5d86936c3a48c613460983c980271fcab8128b75 Mon Sep 17 00:00:00 2001 From: Christian Kissig Date: Tue, 26 Sep 2023 12:18:59 + Subject: [PATCH 1/5] [Support] Add KnownBits::computeForSubBorrow * Implements computeForSubBorrow as alias for computeforAddCarry. Borrow is expected to be 1-bit wide. * Adds exhaustive unit test. --- llvm/include/llvm/Support/KnownBits.h| 4 +++ llvm/lib/Support/KnownBits.cpp | 12 + llvm/unittests/Support/KnownBitsTest.cpp | 31 3 files changed, 47 insertions(+) diff --git a/llvm/include/llvm/Support/KnownBits.h b/llvm/include/llvm/Support/KnownBits.h index 8462aa11202d5d7..711ca8c12129a1b 100644 --- a/llvm/include/llvm/Support/KnownBits.h +++ b/llvm/include/llvm/Support/KnownBits.h @@ -332,6 +332,10 @@ struct KnownBits { static KnownBits computeForAddSub(bool Add, bool NSW, const KnownBits &LHS, KnownBits RHS); + /// Compute known bits results from subtracting RHS from LHS. + static KnownBits computeForSubBorrow(const KnownBits &LHS, KnownBits RHS, + const KnownBits &Borrow); + /// Compute knownbits resulting from llvm.sadd.sat(LHS, RHS) static KnownBits sadd_sat(const KnownBits &LHS, const KnownBits &RHS); diff --git a/llvm/lib/Support/KnownBits.cpp b/llvm/lib/Support/KnownBits.cpp index 097c22d33dd12ba..99ac50a34666fce 100644 --- a/llvm/lib/Support/KnownBits.cpp +++ b/llvm/lib/Support/KnownBits.cpp @@ -85,6 +85,18 @@ KnownBits KnownBits::computeForAddSub(bool Add, bool NSW, return KnownOut; } +KnownBits KnownBits::computeForSubBorrow(const KnownBits &LHS, KnownBits RHS, + const KnownBits &Borrow) { + assert(Borrow.getBitWidth() == 1 && "Borrow must be 1-bit"); + + // LHS - RHS = LHS + ~RHS + 1 + // Carry 1 - Borrow in ::computeForAddCarry + std::swap(RHS.Zero, RHS.One); + return ::computeForAddCarry(LHS, RHS, + /*CarryZero*/ Borrow.One.getBoolValue(), + /*CarryOne*/ Borrow.Zero.getBoolValue()); +} + KnownBits KnownBits::sextInReg(unsigned SrcBitWidth) const { unsigned BitWidth = getBitWidth(); assert(0 < SrcBitWidth && SrcBitWidth <= BitWidth && diff --git a/llvm/unittests/Support/KnownBitsTest.cpp b/llvm/unittests/Support/KnownBitsTest.cpp index 9d184beea3ba9e9..5597d69ab248d23 100644 --- a/llvm/unittests/Support/KnownBitsTest.cpp +++ b/llvm/unittests/Support/KnownBitsTest.cpp @@ -213,6 +213,37 @@ TEST(KnownBitsTest, AddSubExhaustive) { TestAddSubExhaustive(false); } +TEST(KnownBitsTest, SubBorrowExhaustive) { + unsigned Bits = 4; + ForeachKnownBits(Bits, [&](const KnownBits &Known1) { +ForeachKnownBits(Bits, [&](const KnownBits &Known2) { + ForeachKnownBits(1, [&](const KnownBits &KnownBorrow) { +// Explicitly compute known bits of the addition by trying all +// possibilities. +KnownBits Known(Bits); +Known.Zero.setAllBits(); +Known.One.setAllBits(); +ForeachNumInKnownBits(Known1, [&](const APInt &N1) { + ForeachNumInKnownBits(Known2, [&](const APInt &N2) { +ForeachNumInKnownBits(KnownBorrow, [&](const APInt &Borrow) { + APInt Sub = N1 - N2; + if (Borrow.getBoolValue()) +--Sub; + + Known.One &= Sub; + Known.Zero &= ~Sub; +}); + }); +}); + +KnownBits KnownComputed = +KnownBits::computeForSubBorrow(Known1, Known2, KnownBorrow); +EXPECT_EQ(Known, KnownComputed); + }); +}); + }); +} + TEST(KnownBitsTest, BinaryExhaustive) { testBinaryOpExhaustive( [](const KnownBits &Known1, const KnownBits &Known2) { >From f84c882cf429df238054d88ee07e41a08ae3fd6c Mon Sep 17 00:00:00 2001 From: Christian Kissig Date: Tue, 26 Sep 2023 18:02:49 + Subject: [PATCH 2/5] [CodeGen] Implement USUBC, USUBO_CARRY, and SSUBO_CARRY with KnownBits::computeForSubBorrow --- .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 12 ++ .../CodeGen/AArch64SelectionDAGTest.cpp | 24 +++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index cd21af770e1a4d9..ab3e9b4bdc67402 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -3732,14 +3732,18 @@ KnownBits SelectionDAG::computeKnownBits(SDValue Op, const APInt &DemandedElts, assert(Op.getResNo() == 0 && "We only compute knownbits for the difference here."); -// TODO: Compute influence of the carry operand. +// With UADDO_CARRY and SSUBO_CARRY a borrow bit may be added in. +KnownBits Borrow(1); if (Opcode == ISD::USUBO_CARRY || Opcode == ISD:
[Lldb-commits] [lldb] [lldb] Add support for updating string during debug process (PR #67782)
jimingham wrote: I don't know what the status of lldb-mi is, but lldb-vscode which does the same job (be a DAP server) is under active development. So "this adaptor doesn't use the better method to do X" shouldn't be a reason to not employ the better method. We should just fix the adaptors. I entertained the idea of "value setting summaries" but rejected it after some thought. Summaries are unstructured short developer notes about the value, they really aren't suitable as general stand-in's for the value. So that's really not an appropriate route for changing values. But the Synthetic Child Providers are a re-presentation of the value, so they are a suitable route to do value changing for more complex object types. I haven't read carefully through the ValueObject printer in a while, but we do compress the printing of "small" structures in some cases, so it should be possible to get it not to expand this output. I think people will not see it as a positive if these common objects start taking more space, so it would be good to investigate how to do that. https://github.com/llvm/llvm-project/pull/67782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [InstCombine] Simplify the pattern `a ne/eq (zext/sext (a ne/eq c))` (PR #65852)
goldsteinn wrote: Continue to LGTM... https://github.com/llvm/llvm-project/pull/65852 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [AMDGPU] Add another SIFoldOperands instance after shrink (PR #67878)
rampitec wrote: > I've taken another look at this. The patch does not show any benefit from > running another `SIFoldOperands` pass _after_ `SIShrinkInstructions` per se; > you get exactly the same results (modulo a couple of add instructions that > have their operands commuted differently) if you put the second > `SIFoldOperands` run _before_ `SIShrinkInstructions` instead. > > In other words `SIFoldOperands` is not idempotent, and the reason for the > that seems to be: > > > And the reason it only happens for some SUBREV instructions is even more > > convoluted. It's because SIFoldOperands will sometimes shrink > > V_SUB_CO_U32_e64 to V_SUBREV_CO_U32_e32 even it does not manage to fold > > anything into it. This does seem wrong and is probably worth a closer look. > > This goes back to https://reviews.llvm.org/D51345. Notice how the code that > was added to `updateOperand` does the shrinking but does not actually do any > folding; it returns before we get to > `Old.ChangeToImmediate`/`Old.substVirtReg`. A second run of `SIFoldOperands` > will see the shrunk instruction and fold into it. Yes, this is mostly old targets without no-carry add/sub and the impact is on these 2 instructions which needs to be shrunk before folding. Although fold operands' shrinking capabilities are really limited compared to the shrink pass. https://github.com/llvm/llvm-project/pull/67878 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [InstCombine] Simplify the pattern `a ne/eq (zext/sext (a ne/eq c))` (PR #65852)
https://github.com/nikic approved this pull request. Basically LGTM, but I think this is still missing negative tests for non-equality pred1/pred2? https://github.com/llvm/llvm-project/pull/65852 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
@@ -340,6 +340,44 @@ BasicBlock *llvm::splitBBWithSuffix(IRBuilderBase &Builder, bool CreateBranch, return splitBB(Builder, CreateBranch, Old->getName() + Suffix); } +// This function creates a fake integer value and a fake use for the integer +// value. It returns the fake value created. This is useful in modeling the +// extra arguments to the outlined functions. +Value *createFakeIntVal(IRBuilder<> &Builder, +OpenMPIRBuilder::InsertPointTy OuterAllocaIP, +std::stack &ToBeDeleted, +OpenMPIRBuilder::InsertPointTy InnerAllocaIP, +const Twine &Name = "", bool AsPtr = true) { + Builder.restoreIP(OuterAllocaIP); + Instruction *FakeVal; + AllocaInst *FakeValAddr = + Builder.CreateAlloca(Builder.getInt32Ty(), nullptr, Name + ".addr"); + ToBeDeleted.push(FakeValAddr); + + if (AsPtr) +FakeVal = FakeValAddr; + else { +FakeVal = +Builder.CreateLoad(Builder.getInt32Ty(), FakeValAddr, Name + ".val"); +ToBeDeleted.push(FakeVal); shraiysh wrote: The address is inserted to be deleted once at the beginning of this function. If AsPtr is true, control never comes here. If false, then the load instruction is added here. It wouldn't get deleted twice. https://github.com/llvm/llvm-project/pull/67723 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [OpenMPIRBuilder] Remove wrapper function in `createTask`, `createTeams` (PR #67723)
https://github.com/shraiysh updated https://github.com/llvm/llvm-project/pull/67723 >From 6aabc3c10ea2d587120b74966b7ce96f9b8167af Mon Sep 17 00:00:00 2001 From: Shraiysh Vaishay Date: Thu, 28 Sep 2023 13:35:07 -0500 Subject: [PATCH 1/6] [OpenMPIRBuilder] Remove wrapper function in `createTask` This patch removes the wrapper function in `OpenMPIRBuilder::createTask`. The outlined function is directly of the form that is expected by the runtime library calls. This also fixes the global thread ID argument, which should be used whenever `kmpc_global_thread_num()` is called inside the outlined function. --- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 106 -- .../Frontend/OpenMPIRBuilderTest.cpp | 56 + mlir/test/Target/LLVMIR/openmp-llvm.mlir | 51 +++-- 3 files changed, 99 insertions(+), 114 deletions(-) diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index 9c70d384e55db2b..54012b488c6b671 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -35,6 +35,7 @@ #include "llvm/IR/Function.h" #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/IRBuilder.h" +#include "llvm/IR/InstIterator.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/MDBuilder.h" #include "llvm/IR/Metadata.h" @@ -1496,6 +1497,14 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc, InsertPointTy AllocaIP, BodyGenCallbackTy BodyGenCB, bool Tied, Value *Final, Value *IfCondition, SmallVector Dependencies) { + // We create a temporary i32 value that will represent the global tid after + // outlining. + SmallVector ToBeDeleted; + Builder.restoreIP(AllocaIP); + AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr"); + LoadInst *TID = Builder.CreateLoad(Int32, TIDAddr, "tid.addr.use"); + ToBeDeleted.append({TID, TIDAddr}); + if (!updateToLocation(Loc)) return InsertPointTy(); @@ -1523,41 +1532,27 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc, BasicBlock *TaskAllocaBB = splitBB(Builder, /*CreateBranch=*/true, "task.alloca"); + // Fake use of TID + Builder.SetInsertPoint(TaskAllocaBB, TaskAllocaBB->begin()); + BinaryOperator *AddInst = + dyn_cast(Builder.CreateAdd(TID, Builder.getInt32(10))); + ToBeDeleted.push_back(AddInst); + OutlineInfo OI; OI.EntryBB = TaskAllocaBB; OI.OuterAllocaBB = AllocaIP.getBlock(); OI.ExitBB = TaskExitBB; - OI.PostOutlineCB = [this, Ident, Tied, Final, IfCondition, - Dependencies](Function &OutlinedFn) { -// The input IR here looks like the following- -// ``` -// func @current_fn() { -// outlined_fn(%args) -// } -// func @outlined_fn(%args) { ... } -// ``` -// -// This is changed to the following- -// -// ``` -// func @current_fn() { -// runtime_call(..., wrapper_fn, ...) -// } -// func @wrapper_fn(..., %args) { -// outlined_fn(%args) -// } -// func @outlined_fn(%args) { ... } -// ``` - -// The stale call instruction will be replaced with a new call instruction -// for runtime call with a wrapper function. + OI.ExcludeArgsFromAggregate = {TID}; + OI.PostOutlineCB = [this, Ident, Tied, Final, IfCondition, Dependencies, + TaskAllocaBB, ToBeDeleted](Function &OutlinedFn) { +// Replace the Stale CI by appropriate RTL function call. assert(OutlinedFn.getNumUses() == 1 && "there must be a single user for the outlined function"); CallInst *StaleCI = cast(OutlinedFn.user_back()); // HasShareds is true if any variables are captured in the outlined region, // false otherwise. -bool HasShareds = StaleCI->arg_size() > 0; +bool HasShareds = StaleCI->arg_size() > 1; Builder.SetInsertPoint(StaleCI); // Gather the arguments for emitting the runtime call for @@ -1595,7 +1590,7 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc, Value *SharedsSize = Builder.getInt64(0); if (HasShareds) { AllocaInst *ArgStructAlloca = - dyn_cast(StaleCI->getArgOperand(0)); + dyn_cast(StaleCI->getArgOperand(1)); assert(ArgStructAlloca && "Unable to find the alloca instruction corresponding to arguments " "for extracted function"); @@ -1606,31 +1601,17 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc, SharedsSize = Builder.getInt64(M.getDataLayout().getTypeStoreSize(ArgStructType)); } - -// Argument - task_entry (the wrapper function) -// If the outlined function has some captured variables (i.e. HasShareds is -// true), then the wrapper function will have an additional argument (the -// struct containing captured variables). Otherwise, no such argument will -// be present. -SmallVector
[Lldb-commits] [lldb] [lldb][NFCI] Remove use of ConstString from FilterRule in StructuredDataDarwinLog (PR #68347)
https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/68347 There are only ever 2 FilterRules and their operations are either "regex" or "match". This does not benefit from deduplication since the strings have static lifetime and we can just compare StringRefs pointing to them. This is also not on a fast path, so it doesn't really benefit from the pointer comparisons of ConstStrings. >From 2c8ae3c5d1ff8b841255a79f7f64f81dd2bf9df1 Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Thu, 5 Oct 2023 12:52:10 -0700 Subject: [PATCH] [lldb][NFCI] Remove use of ConstString from FilterRule in StructuredDataDarwinLog There are only ever 2 FilterRules and their operations are either "regex" or "match". This does not benefit from deduplication since the strings have static lifetime and we can just compare StringRefs pointing to them. This is also not on a fast path, so it doesn't really benefit from the pointer comparisons of ConstStrings. --- .../DarwinLog/StructuredDataDarwinLog.cpp | 32 ++- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp index 61e04900da342d2..f8a8df84ca37f29 100644 --- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -32,6 +32,8 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegularExpression.h" +#include "llvm/ADT/StringMap.h" + #define DARWIN_LOG_TYPE_VALUE "DarwinLog" using namespace lldb; @@ -183,21 +185,20 @@ class FilterRule { std::function; - static void RegisterOperation(ConstString operation, + static void RegisterOperation(llvm::StringRef operation, const OperationCreationFunc &creation_func) { GetCreationFuncMap().insert(std::make_pair(operation, creation_func)); } static FilterRuleSP CreateRule(bool match_accepts, size_t attribute, - ConstString operation, + llvm::StringRef operation, const std::string &op_arg, Status &error) { // Find the creation func for this type of filter rule. auto map = GetCreationFuncMap(); auto find_it = map.find(operation); if (find_it == map.end()) { - error.SetErrorStringWithFormat("unknown filter operation \"" - "%s\"", - operation.GetCString()); + error.SetErrorStringWithFormatv("unknown filter operation \"{0}\"", + operation); return FilterRuleSP(); } @@ -217,7 +218,7 @@ class FilterRule { dict_p->AddStringItem("attribute", s_filter_attributes[m_attribute_index]); // Indicate the type of the rule. -dict_p->AddStringItem("type", GetOperationType().GetCString()); +dict_p->AddStringItem("type", GetOperationType()); // Let the rule add its own specific details here. DoSerialization(*dict_p); @@ -227,10 +228,10 @@ class FilterRule { virtual void Dump(Stream &stream) const = 0; - ConstString GetOperationType() const { return m_operation; } + llvm::StringRef GetOperationType() const { return m_operation; } protected: - FilterRule(bool accept, size_t attribute_index, ConstString operation) + FilterRule(bool accept, size_t attribute_index, llvm::StringRef operation) : m_accept(accept), m_attribute_index(attribute_index), m_operation(operation) {} @@ -243,7 +244,7 @@ class FilterRule { } private: - using CreationFuncMap = std::map; + using CreationFuncMap = llvm::StringMap; static CreationFuncMap &GetCreationFuncMap() { static CreationFuncMap s_map; @@ -252,7 +253,8 @@ class FilterRule { const bool m_accept; const size_t m_attribute_index; - const ConstString m_operation; + // The lifetime of m_operation should be static. + const llvm::StringRef m_operation; }; using FilterRules = std::vector; @@ -296,8 +298,8 @@ class RegexFilterRule : public FilterRule { return FilterRuleSP(new RegexFilterRule(accept, attribute_index, op_arg)); } - static ConstString StaticGetOperation() { -static ConstString s_operation("regex"); + static llvm::StringRef StaticGetOperation() { +static constexpr llvm::StringLiteral s_operation("regex"); return s_operation; } @@ -341,8 +343,8 @@ class ExactMatchFilterRule : public FilterRule { new ExactMatchFilterRule(accept, attribute_index, op_arg)); } - static ConstString StaticGetOperation() { -static ConstString s_operation("match"); + static llvm::StringRef StaticGetOperation() { +static constexpr llvm::StringLiteral s_operation("match"); return s_operation; } @@ -701,7 +703,7 @@ class EnableOptions : public Options { // add filter
[Lldb-commits] [lldb] [lldb][NFCI] Remove use of ConstString from FilterRule in StructuredDataDarwinLog (PR #68347)
llvmbot wrote: @llvm/pr-subscribers-lldb Changes There are only ever 2 FilterRules and their operations are either "regex" or "match". This does not benefit from deduplication since the strings have static lifetime and we can just compare StringRefs pointing to them. This is also not on a fast path, so it doesn't really benefit from the pointer comparisons of ConstStrings. --- Full diff: https://github.com/llvm/llvm-project/pull/68347.diff 1 Files Affected: - (modified) lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp (+17-15) ``diff diff --git a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp index 61e04900da342d2..f8a8df84ca37f29 100644 --- a/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ b/lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -32,6 +32,8 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegularExpression.h" +#include "llvm/ADT/StringMap.h" + #define DARWIN_LOG_TYPE_VALUE "DarwinLog" using namespace lldb; @@ -183,21 +185,20 @@ class FilterRule { std::function; - static void RegisterOperation(ConstString operation, + static void RegisterOperation(llvm::StringRef operation, const OperationCreationFunc &creation_func) { GetCreationFuncMap().insert(std::make_pair(operation, creation_func)); } static FilterRuleSP CreateRule(bool match_accepts, size_t attribute, - ConstString operation, + llvm::StringRef operation, const std::string &op_arg, Status &error) { // Find the creation func for this type of filter rule. auto map = GetCreationFuncMap(); auto find_it = map.find(operation); if (find_it == map.end()) { - error.SetErrorStringWithFormat("unknown filter operation \"" - "%s\"", - operation.GetCString()); + error.SetErrorStringWithFormatv("unknown filter operation \"{0}\"", + operation); return FilterRuleSP(); } @@ -217,7 +218,7 @@ class FilterRule { dict_p->AddStringItem("attribute", s_filter_attributes[m_attribute_index]); // Indicate the type of the rule. -dict_p->AddStringItem("type", GetOperationType().GetCString()); +dict_p->AddStringItem("type", GetOperationType()); // Let the rule add its own specific details here. DoSerialization(*dict_p); @@ -227,10 +228,10 @@ class FilterRule { virtual void Dump(Stream &stream) const = 0; - ConstString GetOperationType() const { return m_operation; } + llvm::StringRef GetOperationType() const { return m_operation; } protected: - FilterRule(bool accept, size_t attribute_index, ConstString operation) + FilterRule(bool accept, size_t attribute_index, llvm::StringRef operation) : m_accept(accept), m_attribute_index(attribute_index), m_operation(operation) {} @@ -243,7 +244,7 @@ class FilterRule { } private: - using CreationFuncMap = std::map; + using CreationFuncMap = llvm::StringMap; static CreationFuncMap &GetCreationFuncMap() { static CreationFuncMap s_map; @@ -252,7 +253,8 @@ class FilterRule { const bool m_accept; const size_t m_attribute_index; - const ConstString m_operation; + // The lifetime of m_operation should be static. + const llvm::StringRef m_operation; }; using FilterRules = std::vector; @@ -296,8 +298,8 @@ class RegexFilterRule : public FilterRule { return FilterRuleSP(new RegexFilterRule(accept, attribute_index, op_arg)); } - static ConstString StaticGetOperation() { -static ConstString s_operation("regex"); + static llvm::StringRef StaticGetOperation() { +static constexpr llvm::StringLiteral s_operation("regex"); return s_operation; } @@ -341,8 +343,8 @@ class ExactMatchFilterRule : public FilterRule { new ExactMatchFilterRule(accept, attribute_index, op_arg)); } - static ConstString StaticGetOperation() { -static ConstString s_operation("match"); + static llvm::StringRef StaticGetOperation() { +static constexpr llvm::StringLiteral s_operation("match"); return s_operation; } @@ -701,7 +703,7 @@ class EnableOptions : public Options { // add filter spec auto rule_sp = FilterRule::CreateRule( -accept, attribute_index, ConstString(operation), +accept, attribute_index, operation, std::string(rule_text.substr(operation_end_pos + 1)), error); if (rule_sp && error.Success()) `` https://github.com/llvm/llvm-project/pull/68347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFCI] Remove use of ConstString from FilterRule in StructuredDataDarwinLog (PR #68347)
https://github.com/walter-erquinigo approved this pull request. https://github.com/llvm/llvm-project/pull/68347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Allow specifying a custom exports file (PR #68013)
https://github.com/bulbazord approved this pull request. The warnings look good to me, thanks for taking care of that. How does this look @jimingham? https://github.com/llvm/llvm-project/pull/68013 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang] Check DW_AT_declaration to determine static data members (PR #68300)
https://github.com/adrian-prantl approved this pull request. https://github.com/llvm/llvm-project/pull/68300 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Allow specifying a custom exports file (PR #68013)
jimingham wrote: LGTM https://github.com/llvm/llvm-project/pull/68013 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Allow specifying a custom exports file (PR #68013)
walter-erquinigo wrote: Thanks! https://github.com/llvm/llvm-project/pull/68013 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 87c6ff6 - [LLDB] Allow specifying a custom exports file (#68013)
Author: Walter Erquinigo Date: 2023-10-05T20:17:48-04:00 New Revision: 87c6ff6da82a1288ce5c80370d5d8cdd4c20220d URL: https://github.com/llvm/llvm-project/commit/87c6ff6da82a1288ce5c80370d5d8cdd4c20220d DIFF: https://github.com/llvm/llvm-project/commit/87c6ff6da82a1288ce5c80370d5d8cdd4c20220d.diff LOG: [LLDB] Allow specifying a custom exports file (#68013) LLDB has the cmake flag `LLDB_EXPORT_ALL_SYMBOLS` that exports the lldb, lldb_private namespaces, as well as other symbols like python and lua (see `lldb/source/API/liblldb-private.exports`). However, not all symbols in lldb fall into these categories and in order to get access to some symbols that live in plugin folders (like dwarf parsing symbols), it's useful to be able to specify a custom exports file giving more control to the developer using lldb as a library. This adds the new cmake flag `LLDB_EXPORT_ALL_SYMBOLS_EXPORTS_FILE` that is used when `LLDB_EXPORT_ALL_SYMBOLS` is enabled to specify that custom exports file. This is a follow up of https://github.com/llvm/llvm-project/pull/67851 Added: Modified: lldb/cmake/modules/LLDBConfig.cmake lldb/source/API/CMakeLists.txt Removed: diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 380016ce48015fa..ce5e666a6f5e1ac 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -123,7 +123,10 @@ if(APPLE AND CMAKE_GENERATOR STREQUAL Xcode) endif() set(LLDB_EXPORT_ALL_SYMBOLS 0 CACHE BOOL - "Causes lldb to export all symbols when building liblldb.") + "Causes lldb to export some private symbols when building liblldb. See lldb/source/API/liblldb-private.exports for the full list of symbols that get exported.") + +set(LLDB_EXPORT_ALL_SYMBOLS_EXPORTS_FILE "" CACHE PATH + "When `LLDB_EXPORT_ALL_SYMBOLS` is enabled, this specifies the exports file to use when building liblldb.") if ((NOT MSVC) OR MSVC12) add_definitions( -DHAVE_ROUND ) diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt index 7cfa3aaafdae188..a574a461d4920ae 100644 --- a/lldb/source/API/CMakeLists.txt +++ b/lldb/source/API/CMakeLists.txt @@ -177,11 +177,18 @@ if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows") # from working on some systems but limits the liblldb size. MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb namespace") add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb.exports) - else() -# Don't use an explicit export. Instead, tell the linker to -# export all symbols. + elseif (NOT LLDB_EXPORT_ALL_SYMBOLS_EXPORTS_FILE) +# Don't use an explicit export. Instead, tell the linker to export all symbols. MESSAGE("-- Symbols (liblldb): exporting all symbols from the lldb and lldb_private namespaces") +MESSAGE(WARNING "Private LLDB symbols frequently change and no API stability is guaranteed. " +"Only the SB API is guaranteed to be stable.") add_llvm_symbol_exports(liblldb ${CMAKE_CURRENT_SOURCE_DIR}/liblldb-private.exports) + else () +MESSAGE("-- Symbols (liblldb): exporting all symbols specified in the exports " +" file '${LLDB_EXPORT_ALL_SYMBOLS_EXPORTS_FILE}'") +MESSAGE(WARNING "Private LLDB symbols frequently change and no API stability is guaranteed. " +"Only the SB API is guaranteed to be stable.") +add_llvm_symbol_exports(liblldb "${LLDB_EXPORT_ALL_SYMBOLS_EXPORTS_FILE}") endif() set_target_properties(liblldb_exports PROPERTIES FOLDER "lldb misc") elseif (LLDB_EXPORT_ALL_SYMBOLS) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Allow specifying a custom exports file (PR #68013)
https://github.com/walter-erquinigo closed https://github.com/llvm/llvm-project/pull/68013 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [mlir][sparse] introduce MapRef, unify conversion/codegen for reader (PR #68360)
https://github.com/aartbik updated https://github.com/llvm/llvm-project/pull/68360 >From 6094912685a0cfa5c13e023e8ec97238a84fca2f Mon Sep 17 00:00:00 2001 From: Aart Bik Date: Thu, 5 Oct 2023 13:22:28 -0700 Subject: [PATCH 1/4] [mlir][sparse] introduce MapRef, unify conversion/codegen for reader This revision introduces a MapRef, which will support a future generalization beyond permutations (e.g. block sparsity). This revision also unifies the conversion/codegen paths for the sparse_tensor.new operation from file (eg. the readers). Note that more unification is planned as well as general affine dim2lvl and lvl2dim (all marked with TODOs). --- .../mlir/ExecutionEngine/SparseTensor/File.h | 156 ++-- .../ExecutionEngine/SparseTensor/MapRef.h | 96 ++ .../ExecutionEngine/SparseTensor/Storage.h| 108 +-- .../ExecutionEngine/SparseTensorRuntime.h | 8 - .../SparseTensor/Transforms/CodegenUtils.cpp | 89 + .../SparseTensor/Transforms/CodegenUtils.h| 18 ++ .../Transforms/SparseTensorCodegen.cpp| 73 ++-- .../Transforms/SparseTensorConversion.cpp | 111 ++- .../SparseTensor/CMakeLists.txt | 1 + .../ExecutionEngine/SparseTensor/MapRef.cpp | 52 ++ .../ExecutionEngine/SparseTensorRuntime.cpp | 60 +++--- mlir/test/Dialect/SparseTensor/codegen.mlir | 172 +- .../test/Dialect/SparseTensor/conversion.mlir | 18 +- 13 files changed, 475 insertions(+), 487 deletions(-) create mode 100644 mlir/include/mlir/ExecutionEngine/SparseTensor/MapRef.h create mode 100644 mlir/lib/ExecutionEngine/SparseTensor/MapRef.cpp diff --git a/mlir/include/mlir/ExecutionEngine/SparseTensor/File.h b/mlir/include/mlir/ExecutionEngine/SparseTensor/File.h index 78c1a0544e3a521..9157bfa7e773239 100644 --- a/mlir/include/mlir/ExecutionEngine/SparseTensor/File.h +++ b/mlir/include/mlir/ExecutionEngine/SparseTensor/File.h @@ -20,6 +20,7 @@ #ifndef MLIR_EXECUTIONENGINE_SPARSETENSOR_FILE_H #define MLIR_EXECUTIONENGINE_SPARSETENSOR_FILE_H +#include "mlir/ExecutionEngine/SparseTensor/MapRef.h" #include "mlir/ExecutionEngine/SparseTensor/Storage.h" #include @@ -75,6 +76,10 @@ inline V readValue(char **linePtr, bool isPattern) { } // namespace detail +//===--===// +// +// Reader class. +// //===--===// /// This class abstracts over the information stored in file headers, @@ -132,6 +137,7 @@ class SparseTensorReader final { /// Reads and parses the file's header. void readHeader(); + /// Returns the stored value kind. ValueKind getValueKind() const { return valueKind_; } /// Checks if a header has been successfully read. @@ -185,58 +191,37 @@ class SparseTensorReader final { /// valid after parsing the header. void assertMatchesShape(uint64_t rank, const uint64_t *shape) const; - /// Reads a sparse tensor element from the next line in the input file and - /// returns the value of the element. Stores the coordinates of the element - /// to the `dimCoords` array. - template - V readElement(uint64_t dimRank, uint64_t *dimCoords) { -assert(dimRank == getRank() && "rank mismatch"); -char *linePtr = readCoords(dimCoords); -return detail::readValue(&linePtr, isPattern()); - } - - /// Allocates a new COO object for `lvlSizes`, initializes it by reading - /// all the elements from the file and applying `dim2lvl` to their - /// dim-coordinates, and then closes the file. Templated on V only. - template - SparseTensorCOO *readCOO(uint64_t lvlRank, const uint64_t *lvlSizes, - const uint64_t *dim2lvl); - /// Allocates a new sparse-tensor storage object with the given encoding, /// initializes it by reading all the elements from the file, and then /// closes the file. Templated on P, I, and V. template SparseTensorStorage * readSparseTensor(uint64_t lvlRank, const uint64_t *lvlSizes, - const DimLevelType *lvlTypes, const uint64_t *lvl2dim, - const uint64_t *dim2lvl) { -auto *lvlCOO = readCOO(lvlRank, lvlSizes, dim2lvl); + const DimLevelType *lvlTypes, const uint64_t *dim2lvl, + const uint64_t *lvl2dim) { +const uint64_t dimRank = getRank(); +MapRef map(dimRank, lvlRank, dim2lvl, lvl2dim); +auto *coo = readCOO(map, lvlSizes); auto *tensor = SparseTensorStorage::newFromCOO( -getRank(), getDimSizes(), lvlRank, lvlTypes, lvl2dim, *lvlCOO); -delete lvlCOO; +dimRank, getDimSizes(), lvlRank, lvlTypes, lvl2dim, *coo); +delete coo; return tensor; } /// Reads the COO tensor from the file, stores the coordinates and values to /// the given buffers, returns a boolean value to indicate whether the COO /// elements are sorted. - /// Precondition: the buffers sho
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/68378 Add the ability to list all processes through the SB API. rdar://116188959 >From 8611bebd2b6cd4f6de797240c1cb184af71f384d Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 5 Oct 2023 21:07:03 -0700 Subject: [PATCH] [lldb] Expose SBPlatform::GetAllProcesses to the SB API Add the ability to list all processes through the SB API. rdar://116188959 --- lldb/bindings/headers.swig| 1 + .../interface/SBProcessInfoListExtensions.i | 13 lldb/bindings/interfaces.swig | 2 + lldb/include/lldb/API/LLDB.h | 1 + lldb/include/lldb/API/SBDefines.h | 1 + lldb/include/lldb/API/SBPlatform.h| 4 + lldb/include/lldb/API/SBProcessInfo.h | 1 + lldb/include/lldb/API/SBProcessInfoList.h | 46 lldb/include/lldb/Target/Platform.h | 4 +- lldb/include/lldb/Utility/ProcessInfo.h | 20 + lldb/source/API/CMakeLists.txt| 1 + lldb/source/API/SBPlatform.cpp| 15 lldb/source/API/SBProcessInfoList.cpp | 73 +++ lldb/source/Target/Platform.cpp | 8 ++ .../TestPlatformListProcesses.py | 54 ++ 15 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 lldb/bindings/interface/SBProcessInfoListExtensions.i create mode 100644 lldb/include/lldb/API/SBProcessInfoList.h create mode 100644 lldb/source/API/SBProcessInfoList.cpp create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestPlatformListProcesses.py diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index d392ed43d8c0c9e..b1d88726f754354 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -46,6 +46,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/bindings/interface/SBProcessInfoListExtensions.i b/lldb/bindings/interface/SBProcessInfoListExtensions.i new file mode 100644 index 000..42999846ef6a52f --- /dev/null +++ b/lldb/bindings/interface/SBProcessInfoListExtensions.i @@ -0,0 +1,13 @@ +%extend lldb::SBProcessInfoList { +#ifdef SWIGPYTHON +%pythoncode%{ +def __len__(self): + '''Return the number of process info in a lldb.SBProcessInfoListExtensions object.''' + return self.GetSize() + +def __iter__(self): + '''Iterate over all the process info in a lldb.SBProcessInfoListExtensions object.''' + return lldb_iter(self, 'GetSize', 'GetProcessInfoAtIndex') +%} +#endif +} diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 306cfe683893271..373c2f6cf545cfb 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -122,6 +122,7 @@ %include "lldb/API/SBPlatform.h" %include "lldb/API/SBProcess.h" %include "lldb/API/SBProcessInfo.h" +%include "lldb/API/SBProcessInfoList.h" %include "lldb/API/SBQueue.h" %include "lldb/API/SBQueueItem.h" %include "lldb/API/SBReproducer.h" @@ -184,6 +185,7 @@ %include "./interface/SBModuleSpecExtensions.i" %include "./interface/SBModuleSpecListExtensions.i" %include "./interface/SBProcessExtensions.i" +%include "./interface/SBProcessInfoListExtensions.i" %include "./interface/SBQueueItemExtensions.i" %include "./interface/SBScriptObjectExtensions.i" %include "./interface/SBSectionExtensions.i" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index eacbbeafcf1cd86..f652d1bdb835b59 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -49,6 +49,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index ec5e940fdaf36fc..c6f01cc03f263c8 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -90,6 +90,7 @@ class LLDB_API SBPlatformConnectOptions; class LLDB_API SBPlatformShellCommand; class LLDB_API SBProcess; class LLDB_API SBProcessInfo; +class LLDB_API SBProcessInfoList; class LLDB_API SBQueue; class LLDB_API SBQueueItem; class LLDB_API SBReplayOptions; diff --git a/lldb/include/lldb/API/SBPlatform.h b/lldb/include/lldb/API/SBPlatform.h index e0acc7003a54bc3..d63d2ed1eaba627 100644 --- a/lldb/include/lldb/API/SBPlatform.h +++ b/lldb/include/lldb/API/SBPlatform.h @@ -11,11 +11,13 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBProcess.h" +#include "lldb/API/SBProcessInfoList.h" #include struct PlatformConnectOptions; struct PlatformShellComman
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
llvmbot wrote: @llvm/pr-subscribers-lldb Changes Add the ability to list all processes through the SB API. rdar://116188959 --- Full diff: https://github.com/llvm/llvm-project/pull/68378.diff 15 Files Affected: - (modified) lldb/bindings/headers.swig (+1) - (added) lldb/bindings/interface/SBProcessInfoListExtensions.i (+13) - (modified) lldb/bindings/interfaces.swig (+2) - (modified) lldb/include/lldb/API/LLDB.h (+1) - (modified) lldb/include/lldb/API/SBDefines.h (+1) - (modified) lldb/include/lldb/API/SBPlatform.h (+4) - (modified) lldb/include/lldb/API/SBProcessInfo.h (+1) - (added) lldb/include/lldb/API/SBProcessInfoList.h (+46) - (modified) lldb/include/lldb/Target/Platform.h (+3-1) - (modified) lldb/include/lldb/Utility/ProcessInfo.h (+20) - (modified) lldb/source/API/CMakeLists.txt (+1) - (modified) lldb/source/API/SBPlatform.cpp (+15) - (added) lldb/source/API/SBProcessInfoList.cpp (+73) - (modified) lldb/source/Target/Platform.cpp (+8) - (added) lldb/test/API/functionalities/gdb_remote_client/TestPlatformListProcesses.py (+54) ``diff diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index d392ed43d8c0c9e..b1d88726f754354 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -46,6 +46,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/bindings/interface/SBProcessInfoListExtensions.i b/lldb/bindings/interface/SBProcessInfoListExtensions.i new file mode 100644 index 000..42999846ef6a52f --- /dev/null +++ b/lldb/bindings/interface/SBProcessInfoListExtensions.i @@ -0,0 +1,13 @@ +%extend lldb::SBProcessInfoList { +#ifdef SWIGPYTHON +%pythoncode%{ +def __len__(self): + '''Return the number of process info in a lldb.SBProcessInfoListExtensions object.''' + return self.GetSize() + +def __iter__(self): + '''Iterate over all the process info in a lldb.SBProcessInfoListExtensions object.''' + return lldb_iter(self, 'GetSize', 'GetProcessInfoAtIndex') +%} +#endif +} diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 306cfe683893271..373c2f6cf545cfb 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -122,6 +122,7 @@ %include "lldb/API/SBPlatform.h" %include "lldb/API/SBProcess.h" %include "lldb/API/SBProcessInfo.h" +%include "lldb/API/SBProcessInfoList.h" %include "lldb/API/SBQueue.h" %include "lldb/API/SBQueueItem.h" %include "lldb/API/SBReproducer.h" @@ -184,6 +185,7 @@ %include "./interface/SBModuleSpecExtensions.i" %include "./interface/SBModuleSpecListExtensions.i" %include "./interface/SBProcessExtensions.i" +%include "./interface/SBProcessInfoListExtensions.i" %include "./interface/SBQueueItemExtensions.i" %include "./interface/SBScriptObjectExtensions.i" %include "./interface/SBSectionExtensions.i" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index eacbbeafcf1cd86..f652d1bdb835b59 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -49,6 +49,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index ec5e940fdaf36fc..c6f01cc03f263c8 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -90,6 +90,7 @@ class LLDB_API SBPlatformConnectOptions; class LLDB_API SBPlatformShellCommand; class LLDB_API SBProcess; class LLDB_API SBProcessInfo; +class LLDB_API SBProcessInfoList; class LLDB_API SBQueue; class LLDB_API SBQueueItem; class LLDB_API SBReplayOptions; diff --git a/lldb/include/lldb/API/SBPlatform.h b/lldb/include/lldb/API/SBPlatform.h index e0acc7003a54bc3..d63d2ed1eaba627 100644 --- a/lldb/include/lldb/API/SBPlatform.h +++ b/lldb/include/lldb/API/SBPlatform.h @@ -11,11 +11,13 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBProcess.h" +#include "lldb/API/SBProcessInfoList.h" #include struct PlatformConnectOptions; struct PlatformShellCommand; +class ProcessInstanceInfoMatch; namespace lldb { @@ -154,6 +156,8 @@ class LLDB_API SBPlatform { SBProcess Attach(SBAttachInfo &attach_info, const SBDebugger &debugger, SBTarget &target, SBError &error); + SBProcessInfoList GetAllProcesses(SBError &error); + SBError Kill(const lldb::pid_t pid); SBError diff --git a/lldb/include/lldb/API/SBProcessInfo.h b/lldb/include/lldb/API/SBProcessInfo.h index 36fae9e842a6136..aec5924e4704a49 100644 --- a/lldb/include/lldb/API/SBProcessInfo.h +++ b/lldb/include/lldb/API/SBProcessI
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -0,0 +1,73 @@ +//===-- SBProcessInfoList.cpp -===// medismailben wrote: same, missing some `---` https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
https://github.com/medismailben approved this pull request. LGTM with some comments https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -989,6 +989,14 @@ uint32_t Platform::FindProcesses(const ProcessInstanceInfoMatch &match_info, return match_count; } +ProcessInstanceInfoList Platform::GetAllProcesses() { + ProcessInstanceInfoList processes; + ProcessInstanceInfoMatch match; + assert(match.MatchAllProcesses()); medismailben wrote: why do we need this assert here ? https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -0,0 +1,73 @@ +//===-- SBProcessInfoList.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/SBProcessInfoList.h" +#include "Utils.h" medismailben wrote: sort ? https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -0,0 +1,46 @@ +//===-- SBProcessInfoList.h -*- C++ -*-===// medismailben wrote: This is missing from `` https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -0,0 +1,73 @@ +//===-- SBProcessInfoList.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/SBProcessInfoList.h" +#include "Utils.h" JDevlieghere wrote: This is sorted by clang-format, it wants the corresponding header first, then sorter alphabetically. I'll move utils down to avoid this weird looking situation. https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -989,6 +989,14 @@ uint32_t Platform::FindProcesses(const ProcessInstanceInfoMatch &match_info, return match_count; } +ProcessInstanceInfoList Platform::GetAllProcesses() { + ProcessInstanceInfoList processes; + ProcessInstanceInfoMatch match; + assert(match.MatchAllProcesses()); JDevlieghere wrote: This ensures that the default constructed `ProcessInstanceInfoMatch` actually matches all processes. This is the method that the implementation keys off of, so it seemed appropriate to have sanity check here. https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/68378 >From 450be6e0e3e7b9b13f7674fbade9c5ce3bce9d97 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 5 Oct 2023 21:07:03 -0700 Subject: [PATCH] [lldb] Expose SBPlatform::GetAllProcesses to the SB API Add the ability to list all processes through the SB API. rdar://116188959 --- lldb/bindings/headers.swig| 1 + .../interface/SBProcessInfoListExtensions.i | 13 lldb/bindings/interfaces.swig | 2 + lldb/include/lldb/API/LLDB.h | 1 + lldb/include/lldb/API/SBDefines.h | 1 + lldb/include/lldb/API/SBPlatform.h| 4 + lldb/include/lldb/API/SBProcessInfo.h | 1 + lldb/include/lldb/API/SBProcessInfoList.h | 46 lldb/include/lldb/Target/Platform.h | 4 +- lldb/include/lldb/Utility/ProcessInfo.h | 20 + lldb/source/API/CMakeLists.txt| 1 + lldb/source/API/SBPlatform.cpp| 15 lldb/source/API/SBProcessInfoList.cpp | 74 +++ lldb/source/Target/Platform.cpp | 8 ++ .../TestPlatformListProcesses.py | 54 ++ 15 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 lldb/bindings/interface/SBProcessInfoListExtensions.i create mode 100644 lldb/include/lldb/API/SBProcessInfoList.h create mode 100644 lldb/source/API/SBProcessInfoList.cpp create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestPlatformListProcesses.py diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index d392ed43d8c0c9e..b1d88726f754354 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -46,6 +46,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/bindings/interface/SBProcessInfoListExtensions.i b/lldb/bindings/interface/SBProcessInfoListExtensions.i new file mode 100644 index 000..42999846ef6a52f --- /dev/null +++ b/lldb/bindings/interface/SBProcessInfoListExtensions.i @@ -0,0 +1,13 @@ +%extend lldb::SBProcessInfoList { +#ifdef SWIGPYTHON +%pythoncode%{ +def __len__(self): + '''Return the number of process info in a lldb.SBProcessInfoListExtensions object.''' + return self.GetSize() + +def __iter__(self): + '''Iterate over all the process info in a lldb.SBProcessInfoListExtensions object.''' + return lldb_iter(self, 'GetSize', 'GetProcessInfoAtIndex') +%} +#endif +} diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 306cfe683893271..373c2f6cf545cfb 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -122,6 +122,7 @@ %include "lldb/API/SBPlatform.h" %include "lldb/API/SBProcess.h" %include "lldb/API/SBProcessInfo.h" +%include "lldb/API/SBProcessInfoList.h" %include "lldb/API/SBQueue.h" %include "lldb/API/SBQueueItem.h" %include "lldb/API/SBReproducer.h" @@ -184,6 +185,7 @@ %include "./interface/SBModuleSpecExtensions.i" %include "./interface/SBModuleSpecListExtensions.i" %include "./interface/SBProcessExtensions.i" +%include "./interface/SBProcessInfoListExtensions.i" %include "./interface/SBQueueItemExtensions.i" %include "./interface/SBScriptObjectExtensions.i" %include "./interface/SBSectionExtensions.i" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index eacbbeafcf1cd86..f652d1bdb835b59 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -49,6 +49,7 @@ #include "lldb/API/SBPlatform.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBProcessInfo.h" +#include "lldb/API/SBProcessInfoList.h" #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index ec5e940fdaf36fc..c6f01cc03f263c8 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -90,6 +90,7 @@ class LLDB_API SBPlatformConnectOptions; class LLDB_API SBPlatformShellCommand; class LLDB_API SBProcess; class LLDB_API SBProcessInfo; +class LLDB_API SBProcessInfoList; class LLDB_API SBQueue; class LLDB_API SBQueueItem; class LLDB_API SBReplayOptions; diff --git a/lldb/include/lldb/API/SBPlatform.h b/lldb/include/lldb/API/SBPlatform.h index e0acc7003a54bc3..d63d2ed1eaba627 100644 --- a/lldb/include/lldb/API/SBPlatform.h +++ b/lldb/include/lldb/API/SBPlatform.h @@ -11,11 +11,13 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBProcess.h" +#include "lldb/API/SBProcessInfoList.h" #include struct PlatformConnectOptions; struct PlatformShellCommand; +class ProcessInstanceInfoMatch; namespace lldb { @@ -154,6 +156,8 @@
[Lldb-commits] [lldb] [lldb] Expose SBPlatform::GetAllProcesses to the SB API (PR #68378)
@@ -989,6 +989,14 @@ uint32_t Platform::FindProcesses(const ProcessInstanceInfoMatch &match_info, return match_count; } +ProcessInstanceInfoList Platform::GetAllProcesses() { + ProcessInstanceInfoList processes; + ProcessInstanceInfoMatch match; + assert(match.MatchAllProcesses()); medismailben wrote: Sounds reasonable. https://github.com/llvm/llvm-project/pull/68378 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits