- User Since
- Mar 28 2019, 8:58 AM (42 w, 1 d)
Thu, Jan 16
Thanks @djtodoro for looking at this. Sorry for this really delayed response!
Mon, Jan 13
Dec 13 2019
Hi, sorry for the slow update.
Dec 11 2019
As far as I understand it I don't think it is safe to move CFI_INSTRUCTIONs
around like this. CFI_INSTRUCTIONs describe the effect of specific instructions.
We encode where this 'description' starts by the position of the CFI_INSTRUCTION
in the instruction stream. So, if you move the CFI_INSTRUCTION like this you
are changing the meaning.
Dec 6 2019
Thanks for taking a look @aprantl.
Dec 5 2019
Missed one of djtodoro's comments, sorry for the spam!
+ Address comments from @djtodoro.
Nov 27 2019
+ Update dexter test for @jmorse.
+ Small additional change to print().
+ Rephrase a small part of the summary.
Nov 25 2019
Thanks! Let's cycle back to this after the llvm-dev thread that @dblaikie started has been resolved.
Thanks for the review, committed in 2de23c8364b.
Nov 24 2019
Hi @alok. I haven't been following the DW_OP_implicit_pointer discussion too closely, so just some inline nitpick comments/questions from me.
Nov 22 2019
LGTM -- though having given only one LGTM before I'd feel more comfortable waiting for +1.
Nov 20 2019
Thanks all for the initial reviews.
Nov 15 2019
+ Added reviewers
+ Improve some comments
+ rename getter/setters as Adrian requested
+ remove some whitespace changes
Nov 13 2019
More Info about this patch: As far as LDV is concerned undefs and 'blockers' are semantically very similar. However, blockers are not emitted (as DBG_VALUES) because it is unnecessary: They are only added to UserValues with overlapping fragments for any given def, and a def implicitly terminates preceding defs with overlapping fragments .
Nov 12 2019
I'm abandoning this because I've created the WIP revision D70121 which handles fragment overlaps.
Oct 31 2019
Oct 29 2019
Hi @bjope thank you for pointing this out and providing a workaround. I'll test out your change and investigate further.
Oct 17 2019
I'm working in this area for pr41992 so I noticed a couple of things (inline comments). I am happy to just add the changes I'm suggesting into my own upcoming patch if you'd like?
Oct 16 2019
After timing some more self-hosts it looks like my original numbers which looked
too good to be true sadly are just that. The self-host asan build time reduction
seems correct (-3.5%) but the non-asan builds are much less impacted then first
thought (about -1%). These numbers are more in line with my original expectation.
Oct 11 2019
Addressed comments and fixed Summary (previously it referred to
LiveDebugValues.cpp instead of LiveDebugVariables.cpp).
Oct 10 2019
The numbers are much better than I expected but I haven't been able to prove
myself wrong after a bunch of testing. So, I've put this patch up for the
community's expert eyes.
Sep 20 2019
I don't feel confident enough in this area to give an official LGTM but If I understand it correctly the patch seems reasonable: move loop metadata from old to new latch when a loop header is split.
Sep 3 2019
Aug 22 2019
Aug 20 2019
Thanks for the review!
Aug 19 2019
Jun 19 2019
Thanks, I've now resubmitted this patch (1251cac62af5).
Jun 18 2019
LGTM. Thanks for doing this!
Jun 17 2019
Jun 12 2019
No problem at all - reverted with rGa94715639619. I'll take a look at this next week, thank you for the info.
Jun 11 2019
May 28 2019
The major change to this previously accepted patch is the modification of "bcmp-debugify-remarks.ll" and "inlined-argument.ll".
Minor changes include spelling corrections in comments and removing some superfluous debug data from the new tests.
May 20 2019
Resubmitted in rL361149.
Reverted with commit 95805bc425b because armv8 build bots are failing.
May 16 2019
Address suggestions and add some const correctness.
May 15 2019
May 14 2019
This update addresses the problems with the original patch that caused the built bot failures
(sorry about that!).
May 7 2019
May 2 2019
Hi, I added a few inline comments, mostly just nits on style.
Apr 30 2019
My take-away from the discussion was this: It is desirable to map the instructions to something in the loop (e.g., not line 0), unless doing so will provide confusing information to the mapping that PGO uses to optimize the relevant branches. Am I correct in saying that this latter issue is of minimal concern in this case?
Apr 29 2019
A summary of the discussion so far:
Apr 18 2019
Are the function attributes in the test necessary? Watching other reviews leads me to believe these are undesirable. Apart from this it LGTMy untrained eyes (I'm new to reviewing!).
Sorry, I missed a couple of failing tests. I've fixed them and updated the diff.
Apr 17 2019
Apr 12 2019
I've made the suggested changes.
Apr 11 2019
Apr 2 2019
Updated a comment with some more info as vsk suggested.
Apr 1 2019
The test has been updated to check for the expected instructions.
I've updated the test to include the source and commands to generate the IR it uses. Please note that I stripped out the call void @llvm.dbg.* lines from the test file by hand so any regenerated IR will look more cluttered.