User Details
- User Since
- Dec 14 2017, 6:53 AM (161 w, 5 d)
Yesterday
(@labath Sorry for late response, I've been away from keyboard for some time.)
some comments inline, lgtm otherwise
Wed, Dec 30
Dec 15 2020
Ping. :)
Hi @xiangzhangllvm, please wait for @vsk to approve as well, since he is author of the original debugify IR patch and he is very involved in the area.
Thanks a lot for your patience and work.
Dec 11 2020
Dec 10 2020
LGTM, some nits inline. Thanks.
Dec 9 2020
lgtm, thanks!
Dec 7 2020
Thanks for addressing the comments.
lgtm, but please keep just one test case, since it is enough (as @dstenb suggested)
Dec 3 2020
Dec 2 2020
This makes sense to me. The call site info represents info about param value transferring, so if the value was undef, we have "nothing to transfer".
Nov 29 2020
Nov 27 2020
- Use the entry values for local vars as well
- Support old metadata
- Fix testcases
Nov 26 2020
(please leave some time (a few days) to other reviewers to take a look)
Nov 25 2020
Thanks for addressing the comments! We are almost there.
Nov 23 2020
Nice! (few nits inline)
Thanks!
Nov 20 2020
Nov 19 2020
Thanks!! So, this should be the most important one :) I would really like to see some measurements (e.g. llvm-locstats) before/after this patch.
Have you tried building llvm-project itself with this?
Nov 18 2020
Nov 17 2020
Thanks for doing this! (some initial comments inline)
Nov 13 2020
OK, IMPO we can go with removing it completely from the source.
Nov 12 2020
Nov 5 2020
Nov 3 2020
Oct 29 2020
lgtm, thanks.
Oct 28 2020
Super nit inline.
@jmorse Thanks. :)
LGTM; Please remove "Entry values" from the top-level comment TODO list.
Oct 27 2020
It was huge. Thanks.
Oct 26 2020
Oct 23 2020
Oct 22 2020
This looks reasonable to me.
Oct 19 2020
@StephenTozer Thanks for your comments. I'll address them and refactor the patch a bit, as soon as I catch some time to address some issues with the first patch from the stack.
Oct 16 2020
Oct 15 2020
Oct 13 2020
Oct 12 2020
Some nits included.
Oct 11 2020
Oct 9 2020
!var_p2, sorry for my mistake
.... llvm.dbg.value(%p1, !var_p1, !DIExpr(), %p2, !DIExpr()) <=== p1 is an argument, but it depends on different entry value
Yes, that is correct.
Oct 8 2020
Oct 6 2020
TODO: Handle Function Inlining
- Use VMContext instead of InsertBB->getParent()->getContext()
- Avoid null metadata within the new llvm.dbg.value()
Oct 5 2020
This needs to be tested more (so it is not for the final commit (yet)). I believe this triggers some asserts, but (at least) we are covering the cases we want.
- Use 2 extra arguments from llvm.dbg.value()
- Handle modified params
- Extend the llvm.dbg.value() with 2 operands by describing Entry Value
I am about to post new set of patches addressing this problem. We are back to the initial idea that refers to extending the llvm.dbg.value() to hold info about the entry Value. We need to do that, since the current approach (by relying on the method that uses the DILocalVariable*) couldn't detect variable modifications (in an efficient way). Another advantage we are getting with this new approach is that we can handle local variables as well, if it can be expressed in terms of an entry value such as:
void fn (int param) { ... int local = param + 2; ...
Oct 1 2020
- Return the names
Sep 30 2020
mostly lgtm, some initinal comments inline, thanks!
Sep 29 2020
Thanks a lot for such a nice explanation! I've got it (at least for this example)!
Sep 28 2020
Hi @jmorse, cool, nice improvement! It turns out that this value-based value tracking pass makes it simple as well as effective!
-Rebasing
-Rebasing
-Addressing the latest comments
-Rebasing
Sep 25 2020
@StephenTozer @dstenb Thanks for your comments! I'll try to change the approach (by putting some extra context), so we might end up changing this one (a bit).