- User Since
- Dec 14 2017, 6:53 AM (109 w, 4 d)
Looks good to me.
This makes sense to me!
Thanks for doing this!
Thu, Jan 16
Nice, thanks! Please post the rest of the patches, when you are ready, and make a stack of the patches.
Wed, Jan 15
Tue, Jan 14
Mon, Jan 13
I guess this should be closed? :)
@aprantl Thanks for the comment, sorry for the late response, I was on the vacation.
Thanks for the comments! Sorry for the late response, I was on the vacation.
Please add a test case.
Sun, Jan 12
Thanks for this!
Wed, Dec 25
An example of the plot on a GDB binary built with LLVM Trunk using the entry values vs. no entry values.
An example of the plot on a GDB binary built with LLVM Trunk.
Tue, Dec 24
Mon, Dec 23
Dec 19 2019
Thanks for doing this! Looks good to me now!
Please make sure the tool name is llvm-locstats instead of llvm-covstat.
Something like the diff bellow would address the concerns.
This won't solve the problem you reported..
Yes that was my point. Should we add the AND LLVM_BUILD_TOOLS part in both of the llvm/CMakeLists.txt and llvm/utils/llvm-locstats/CMakeLists.txt?
Sorry for the confusion. May be the llvm/CMakeLists.txt is better place to address this?
Hm...actually...this is not good..
Dec 17 2019
Good change! It sound reasonable to me.
Looks good to me! Thanks!
Sorry, one more nit included.
Dec 16 2019
Thanks for addressing the comments.
Thanks for this! This looks good to me now!
Dec 13 2019
Dec 12 2019
Please address my comments and try using the DIExpression::appendExt(). Thanks.
We can use llvm.org/pr44275 to track improvements for this.
Please wait for someone else from the debug-info project to take a look as well.
Dec 11 2019
This one should be rebased on top of the D71366.
@krisb Thanks for this!
Dec 9 2019
@krisb Thanks for this again. First option seems better to me.
Dec 8 2019
LGTM as well! Thanks!
Dec 6 2019
Hm, the bucket name ‘>0-9%' does not look nice to me as well.
Dec 5 2019
LGTM as well, thanks!
Looks good! Thanks!
Dec 4 2019
Dec 3 2019
Reverted while investigating. I am not sure what happened, since the test passed on my machine. Thanks!
Thanks for the reviews!
Nov 28 2019
LGTM. Thanks for doing this!
I like the way of the implementation. Thanks!
I'm not sure if that's possible here. Everything seems quite interdependent, and the bug fix (functionality change) happens as a result of the structural change (also a functionality change).
I could split this into two or more dependant patches and keep 'related' things together. For example, put all the debug print() updates in a single patch. Not sure that this would this help though, what do you think?
There is a very small change/fix to DbgValueLocation which I included in this patch. I could take that out (and probably should) but it won't have a noticable impact on the diff size.
I see. The semantic of the change does not allow us to get it shorter, so I am OK with that. Thanks for considering that!
Nov 27 2019
@jmorse Thanks for your comment!