- User Since
- Mar 19 2018, 2:57 AM (79 w, 1 h)
Mon, Sep 16
I don't think this is good.
Fri, Sep 13
Thu, Sep 12
Wed, Sep 11
Address feedback, twiddle various things.
Tue, Sep 10
(Ninja-edit: this moves transfer-insertion to before flushPendingLocs. flushPendingLocs may manipulate VarLocs that refer to not-yet-inserted DBG_VALUEs as their sources).
Fri, Sep 6
Thu, Sep 5
Expand a comment, explaining why we terminate overwritten spill-locations during LiveDebugValues, instead of leaving it until later.
Wed, Sep 4
Enjoyable leaps in variable-coverage / bytes-coverage on the far right of these graphs:
Delete a vector that isn't used by anything.
Nix an un-necessary else.
Tue, Sep 3
LGTM, thanks for digging into this!
Mon, Sep 2
Looking good; a couple of extra nits, then it's ready to go.
Update a comment, we don't terminate variables, instead we close locations.
Revise some comments & assertions.
This update simplifies & flattens the copy-forwarding logic as suggested. Rather than trying to select the condition where copy-forwarding is valid, instead continue around the loop whenever a precondition isn't met.
A case of mistaken identity meant I linked to this review in commit r370648, the real review for that commit is D58450, sorry for that extra noise.
Committed in https://reviews.llvm.org/rL370648
Fri, Aug 30
Thu, Aug 29
Wed, Aug 28
Tue, Aug 27
This update removes the "initialisation" phase that was happening in LiveDebugValues, and doing so fixes the original problem I was trying to solve, horray.
Mon, Aug 26
Aug 23 2019
Remove an unnecessary change.
Aug 22 2019
Aug 21 2019
Aug 20 2019
Update: I noticed that in moving transferTerminatorInst, I'd changed the "process" method to always return false, so I've removed its return code.
Stepping back a bit: Is the result of SROA multiple smaller allocas?
If SROA is only creating new allocas, then describing them with dbg.declares should do no harm, since a later call to LowerDbgDeclare would truncate their range by inserting new dbg.values at every load. But if SROA is inserting the load, we do need to make sure that the loaded value is tracked by a dbg.value (potentially in addition to tracking the alloca with a dbg.declare). So I guess my question is, where are loads for the SROAed allocas generated?
Indeed, this'll need a MIR test for the BranchFolding pass to check for future regressions, and to demonstrate that this patch fixes the bug report. There are some MIR test examples in the llvm/test/CodeGen/MIR/X86 directory.
Aug 19 2019
Adjust comment wordings
Looking good, and this produces the ~3% increase in variable locations in clang-3.4 builds like the previous revisions did too. It looks like the two xfailed tests are testing for the behaviour you're explicitly disabling: it's probably better to just delete them, as this is a deliberate decision to change that behaviour.
/Really/ avoid nondeterminism this time.
Aug 16 2019
Sorry for the long delay --
Thanks for all the reviews!
Aug 15 2019
Rebase onto the crash-fixing patch in D66284. This patch now removes the crash-fixing workaround that salvages fragments from spilt expressions, in favour of a more generalised method to access the original unspilt expression. I've edited the details of that into the patch summary at the top.