- User Since
- Jul 8 2015, 10:26 AM (167 w, 1 d)
Teach SBThread::StepOut and SBThread::ReturnFromFrame to behave as-if artificial frames were not present.
I've added SB API support (SBFrame::IsArtificial), a SB API test, fleshed out the remaining tests, and rebased. PTAL, thanks!
This caused a backwards-compatibility issue which made llvm-dis crash when loading old bitcode (see https://reviews.llvm.org/rL342631#inline-2294). I've committed a fix in r342678 to get our bots back to a healthy state. I'm open to any post-commit review -- while I don't anticipate there being an issue, if there is one I'd rather iterate on a passing build.
Wed, Sep 19
Tue, Sep 18
LGTM. As this is somewhat fundamental change another +1 would be nice.
Thanks, LGTM. @aprantl?
Mon, Sep 17
Fri, Sep 14
Sorry for the delay, I was busy with other work. I think I've addressed the review feedback. PTAL.
As Matthias points out, a spill/reload can't unambiguously be associated with a specific instruction.
Thanks, this is looking pretty close. A few comments inline.
LGTM, possibly with an added assert. The test looks great!
Thu, Sep 13
Please document the filter behavior (see docs/UsersManual.rst).
Thanks, LGTM as well.
Please clang-format your diffs.
Wed, Sep 12
I think what %device_rm is doing is reasonably clear. I'd rather have this available than rely on the device correctly deleting files as needed.
It would help to have a regression test for this (somewhere in test/Transforms/GCOVProfiling). Something that exercises emitProfileArcs with 2 CU's would work.
Tue, Sep 11
This needed a small follow-up fix: see r341985, http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/12824/.
This LGTM. Davide?
I think it'd be useful to test the driver output specifically. The kind of testing lldb-test facilitates might not be a good fit here (too low-level).
Thanks, LGTM as well.
Mon, Sep 10
Fri, Sep 7
Tue, Aug 28
@JDevlieghere thanks for the feedback, and no worries!
Address the latest round of review feedback.
Thanks, LGTM. If you don't have commit access yet , I'd be happy to commit this on your behalf.
Mon, Aug 27
Looks great, thanks!
- Partially address some of @rsmith's feedback. Instead of using the capture default location, I used the beginning location of the capture list. This results in smoother single-stepping when those two locations are on different lines.
Address some review feedback.
Fri, Aug 24
- Per Adrian and Paul's feedback, remove the experimental "gating" cl::opt because the size overhead of call site info is low.
Thanks for the feedback!
@CarlosAlbertoEnciso thanks for investigating this and testing out a new way to collect debug values. Given the complexity of replacing the current algorithm, and that it's not strictly related to fixing MachineCSE, I'd recommend keeping the current algorithm (i.e not using MachineRegisterInfo for now). That will allow us to land a fix in tree soon and hopefully iterate on it.
Thu, Aug 23
LGTM. This is NFC, it seems. There's a FIXME in Preprocessor.h about the lifetime of SelectorTable eventually not being tied to Preprocessor, but this is correct for now.
Wed, Aug 22
Recently I addressed a dbg.value use-before-def bug in CodeGenPrepare (r340370). What I learned forced me to reconsider adding this verifier check.
Aug 21 2018
This is looking really good. I think it's great that the test exercises more than just MachineCSE in isolation. Just two more minor comments (inline) --
Aug 20 2018
(LGTM with the second comment addressed.)
Thanks so much for doing this!
- Here is a GIF that might help illustrate the bug: http://net.vedantk.com/static/llvm/lambda-implicit-capture-bug.gif
- Update test/SemaCXX/uninitialized.cpp to highlight the behavior change which comes from using getBeginOrDeclLoc().
+ Jan and Volodymyr. This seemed to be in good shape the last time I looked at it. Not having touched libclang for a while I don't think I can give an official lgtm.
Aug 17 2018
Thanks for doing this!
Aug 16 2018
Not having worked in this area I can't give an official lgtm, but this seems reasonable to me.
Aug 15 2018
Aug 14 2018
Aug 13 2018
Thanks, looks good with nitpicks.
Aug 10 2018
Rebase, and update the patch to use DW_AT_call_return_pc information.
Thanks for your feedback @dblaikie! I've addressed some of it and responded to the rest inline.
Aug 9 2018
Dsymutil will need to learn to resolve addresses in DW_AT_call_return_pc. Currently, I think it's skipping over them, which means that lldb can't use them.
- Add verifier & bitcode tests, rebase.
- Add complete DWARF generation support for call site entries (i.e, one call site entry per call, DW_AT_call_tail_call emission, and DW_AT_call_return_pc emission).
Aug 8 2018
Aug 7 2018
Aug 6 2018
- Make Allocation move-only. This should prevent a std::vector copy in ::Malloc.