[Lldb-commits] [PATCH] D124064: [NFC][trace] simplify the instruction dumper

wallace created this revision. wallace added a reviewer: jj10306. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. TraceInstructionDumper::DumpInstructions was becoming too big, so I'm refactoring it

[Lldb-commits] [PATCH] D124061: [LLDB][NativePDB] Fix the case when S_DEFRANGE_SUBFIELD_REGISTERs are out of order.

zequanwu created this revision. zequanwu added reviewers: rnk, labath. Herald added a project: All. zequanwu requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Previously, I was assuming that S_DEFRANGE_SUBFIELD_REGISTERs are always in the

[Lldb-commits] [PATCH] D124029: Add a mutex to the ThreadPlanStackMap to prevent it getting corrupted

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Jim and I discussed this offline last week. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124029/new/

[Lldb-commits] [lldb] c129220 - [lldb/gdb-remote] Fix -Wswitch after D116462

Author: Fangrui Song Date: 2022-04-19T18:01:06-07:00 New Revision: c129220eaa983d111c1432763db9da4526b405e3 URL: https://github.com/llvm/llvm-project/commit/c129220eaa983d111c1432763db9da4526b405e3 DIFF: https://github.com/llvm/llvm-project/commit/c129220eaa983d111c1432763db9da4526b405e3.diff

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik updated this revision to Diff 423725. shafik added a comment. -Removed override `RemoteAwarePlatformTest.cpp` and added NOLINT CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123340/new/ https://reviews.llvm.org/D123340 Files: lldb/.clang-tidy

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

shafik added a comment. In D123340#3438391 , @labath wrote: > The reason for the funny `/*override*/` thingy in the mock classes is that it > is impossible (in the gmock version that we use) to add the override keyword > to the methods overridden by

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

lattner accepted this revision. lattner added a comment. Nice, LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D124028: Add some docs on how to use container commands to the python reference doc

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/docs/use/python-reference.rst:675 + +:: + (lldb) help my-utilities For this to be formatted as code you need an extra

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

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

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

xbolva00 added a comment. Can you please add link to RFC to commit message? Otherwise looks good CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 ___ lldb-commits mailing list

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

nikic accepted this revision. nikic added inline comments. This revision is now accepted and ready to land. Comment at: llvm/docs/DeveloperPolicy.rst:195 +* Adding, removing, or regrouping a diagnostic. +* Fixing a bug (please link to the issue fixed in the bug database). +*

[Lldb-commits] [PATCH] D124029: Add a mutex to the ThreadPlanStackMap to prevent it getting corrupted

jingham created this revision. jingham added a reviewer: JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. We were seeing very occasional crashes that looked like corrupted ThreadPlans.

[Lldb-commits] [PATCH] D124028: Add some docs on how to use container commands to the python reference doc

jingham created this revision. jingham added a reviewer: JDevlieghere. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I added this feature a while back but didn't put an example in the python

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

aaron.ballman updated this revision to Diff 423683. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. - Made it more clear that adding a release note is discretionary rather than mandatory - Clarified that bug fix release notes

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

aaron.ballman added a comment. In D123957#3459557 , @lattner wrote: > This is awesome, I agree completely we should curate release notes better. > That said, I think this should make it more clear that there is a "difference > in kind" between

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

lattner added a comment. Also, when this lands, we should post on the forum about it. Every change to the developer policy warrants broader visibility than just a phab discussion IMO. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957

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

lattner added a comment. This is awesome, I agree completely we should curate release notes better. That said, I think this should make it more clear that there is a "difference in kind" between user-facing tools like clang/lldb etc and other libraries in LLVM. We don't want release note

[Lldb-commits] [PATCH] D123982: [trace][intel pt] Support events

wallace updated this revision to Diff 423654. wallace added a comment. address comments. Still pending response about the iff word. I've just created a stats objects for events with some util functions that make the code much cleaner. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D123984: [trace][intel pt] Add a memory usage test

wallace abandoned this revision. wallace added a comment. I think I can do better than this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123984/new/ https://reviews.llvm.org/D123984 ___ lldb-commits

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

philnik added inline comments. Comment at: libcxx/utils/gdb/libcxx/printers.py:192 class StdStringPrinter(object): """Print a std::string.""" dblaikie wrote: > philnik wrote: > > jgorbe wrote: > > > Mordante wrote: > > > > philnik wrote: > > > > >

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

dblaikie added inline comments. Comment at: libcxx/utils/gdb/libcxx/printers.py:192 class StdStringPrinter(object): """Print a std::string.""" philnik wrote: > jgorbe wrote: > > Mordante wrote: > > > philnik wrote: > > > > Mordante wrote: > > > > >

[Lldb-commits] [PATCH] D123984: [trace][intel pt] Add a memory usage test

wallace added a comment. > To catch changes that may increase the memory by a lot? exactly. We might introduce changes in the decoder that might produce erroneous data and increase the memory usage or number of instructions by too much. For example, all our current tests work with small number

[Lldb-commits] [PATCH] D123982: [trace][intel pt] Support events

wallace added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:1151 +// Events that might happen during a trace session. +FLAGS_ENUM(TraceEvents){ +// Tracing was paused. If instructions were executed after pausing jj10306 wrote: > What are

[Lldb-commits] [PATCH] D124000: [lldb] Add FixAnyAddress to ABI plugins

DavidSpickett added reviewers: omjavaid, JDevlieghere. DavidSpickett added a subscriber: JDevlieghere. DavidSpickett added a comment. This comes from https://reviews.llvm.org/D118794 on the subject of whether it's ok to just pick one of fixcode/fixdata and assume it'll work. Of all the uses of

[Lldb-commits] [PATCH] D124000: [lldb] Add FixAnyAddress to ABI plugins

DavidSpickett created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. FixAnyAddress is to be used when we don't know or don't care whether

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

aaron.ballman added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:195 +* Adding, removing, or regrouping a diagnostic. +* Fixing a bug (please link to the issue fixed in the bug database). +* Adding or removing an optimization. nikic wrote: > I

[Lldb-commits] [PATCH] D123984: [trace][intel pt] Add a memory usage test

jj10306 added a comment. What is the intention of this test? To catch changes that may increase the memory by a lot? I'm concerned this could be very flaky given that it's dependent on external factors (compiler output, context switches, etc). Now that we have this test, is there any value in

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

nikic requested changes to this revision. nikic added inline comments. This revision now requires changes to proceed. Comment at: llvm/docs/DeveloperPolicy.rst:195 +* Adding, removing, or regrouping a diagnostic. +* Fixing a bug (please link to the issue fixed in the bug

[Lldb-commits] [PATCH] D123982: [trace][intel pt] Support events

jj10306 added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:1151 +// Events that might happen during a trace session. +FLAGS_ENUM(TraceEvents){ +// Tracing was paused. If instructions were executed after pausing What are some other

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

hans added a comment. Looks great to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

aaron.ballman added a subscriber: xbolva00. aaron.ballman added a comment. In D123957#3458354 , @hans wrote: > If it's possible, maybe this could be worded in a way that strongly suggests > the release notes should ideally be updated in the same patch

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

aaron.ballman updated this revision to Diff 423594. aaron.ballman added a comment. Something went funny during a rebase, so this is the full update, not just changes since last time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123957/new/ https://reviews.llvm.org/D123957 Files:

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

aaron.ballman updated this revision to Diff 423593. aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. Fixed some grammar issues, added a bullet for fixing bugs, modified the bullet about diagnostics to include regrouping under a

[Lldb-commits] [lldb] 68e73ea - [lldb] Handle empty search string in "memory find"

Author: David Spickett Date: 2022-04-19T09:19:38Z New Revision: 68e73eaee632b29d36e8b24f62e77ef26084885d URL: https://github.com/llvm/llvm-project/commit/68e73eaee632b29d36e8b24f62e77ef26084885d DIFF: https://github.com/llvm/llvm-project/commit/68e73eaee632b29d36e8b24f62e77ef26084885d.diff

[Lldb-commits] [PATCH] D123793: [lldb] Handle empty search string in "memory find"

This revision was automatically updated to reflect the committed changes. Closed by commit rG68e73eaee632: [lldb] Handle empty search string in memory find (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123793/new/

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

jhenderson accepted this revision. jhenderson added a comment. LGTM, with suggested nits. Comment at: llvm/docs/DeveloperPolicy.rst:188 +notes, typically found in ``docs/ReleaseNotes.rst`` for the project. Changes to +a project that are user-facing or users may wish to know

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

hans added a comment. If it's possible, maybe this could be worded in a way that strongly suggests the release notes should ideally be updated in the same patch as the code change? I think we're generally good at doing this for tests. It would be great if we could treat release notes the same.

[Lldb-commits] [PATCH] D123984: [trace][intel pt] Add a memory usage test

wallace created this revision. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The existing tests that check for the memory usage of the decoded trace are using small binaries, and in this case the