- User Since
- Jul 8 2015, 10:26 AM (219 w, 4 d)
Fri, Sep 20
Yes, please go for it :)
Thanks for your feedback. I've added a flag to asm labels to disable global prefixing. I've tried to minimize the behavior change in this patch -- it seems to me that additional cleanups can happen in follow-ups.
Thu, Sep 19
Could you update 'lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp' in this patch? This should do it:
lgtm, I believe the original llvm-locstats can now land unmodified, which should provide test coverage.
Wed, Sep 18
FTR, I like this approach (introducing DW_AT_LLVM_constant). A side-benefit of not attaching AT_location to TAG_constant is that there's no need to update the dwarf location statistics code. So, +1/lgtm on my end -- would be great to have another +1 on this change though.
Tue, Sep 17
Splitting up the updateCallSiteInfo API is a great idea. Some minor comments inline.
Looks reasonable to me, I just suggest clarifying one comment. @aprantl any other outstanding concerns?
Thanks, this is quite helpful.
Mon, Sep 16
Fri, Sep 13
IIUC @jingham makes the argument here that DW_TAG_constant isn't appropriate for swift's 'let', and instead suggests attaching DW_AT_const_value to the DW_TAG_variable (which lldb seems to already understand). Wdyt of that as an alternative?
Thu, Sep 12
Wow, thanks for chasing this down.
Wed, Sep 11
Makes sense, thanks!
- Add a working example and some additional comments in Evaluate_DW_OP_entry_value.
Looks nice! One question: if DIFlagReservedBit4 is ever reused, will it break bitcode upgrades? Or is the idea that it won't ever be reused?
Lgtm, sorry for the delay.
Tue, Sep 10
- Fix return address lookup when the immediate parent frame is inlined.
- Tighten the test so that it actually verifies that tail calls, inlining, etc. occur, instead of assuming :).
- Add/move various TODOs as pointed out by reviewers.
While tightening up the test case I think I found an issue with the way inlined frames are handled. I need to take a closer look.
- Addressed review feedback, split out unrelated changes, and improved test coverage.
Mon, Sep 9
- Split out llvm change.
- Add a test to validate that lldb skips inline frames when evaluating DW_OP_entry_value.
- Partially address review feedback.
Looks good overall, especially the testing.
I think you've identified a real, pernicious problem, but am not sure that this is the best approach for addressing it. One potential problem here is that slight variations in the error message can confuse the reducer. If e.g. some location information changes in between messages, the reduction will stop too early.
Thanks a lot!
- Clean up the test.
There were 1.5 occurrences in test-suite.
I'd prefer not to try running it over llvm stage-2/3 (i can, but it's gonna be slow..)
Though i guess you were talking about *performance* numbers?
Fri, Sep 6
Lgtm as well, thanks.
Thu, Sep 5
Thanks for working on this!
Still think this looks good. Have you tried running this on the llvm test suite, or some other interesting corpus? Would be curious to see any pre/post patch numbers.
Wed, Sep 4
Thanks, this is looking pretty good.
Tue, Sep 3
- Shorten failure mode names.
Looks mostly good to me!
Thanks, this looks good to me. Perhaps you can get more feedback by dropping the WIP label. @aprantl, any thoughts?
- Allow users to pick the failure mode for profile merging.
@davidxl Are you recommending that the old behavior (fail the merge if any profiles are malformed) should remain the default? I think that's reasonable (it raises the alarm about toolchain bugs), just wanted to double-check.
Fri, Aug 30
FWIW, this looks good to me, but it would be great to get another +1 as I'm not familiar with this pass.
Need to add an option to preserve the old error reporting behavior. I'll get back to this on Tuesday.
Thanks for your feedback. I will address review comments and continue investigating on Tuesday.
Thu, Aug 29
- Don't record a read failure when a soft error is encountered within mergeRecordsFromWriter.
- Fix a comment.
- Bring back validation of NumCounters.
This mistakenly drops validation for NumCounters...
Thanks, I think this is a great starting point. LGTM.
- Address coding style issue in run.py.