Page MenuHomePhabricator

jmorse (Jeremy Morse)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 19 2018, 2:57 AM (70 w, 1 h)

Recent Activity

Fri, Jul 19

jmorse added a comment to D64971: [SafeStack] Insert the deref after the offset.

This sounds similar to PR41675 -- sometimes the DWARF expression builder interprets a DW_OP_deref as an extra dereference, other times it interprets it as confirmation that the expression being built is a memory location.

Fri, Jul 19, 9:50 AM · Restricted Project
jmorse accepted D64645: DAG: Handle dbg_value for arguments split into multiple subregs.

LGTM, thanks for the patch!

Fri, Jul 19, 2:06 AM

Thu, Jul 18

jmorse added a comment to D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..

Probably, it is OK if dbg.value was not originally _supposed_ to be a dbg.declare but was converted in this place.

Thu, Jul 18, 5:55 AM · Restricted Project, debug-info
jmorse added a comment to D64645: DAG: Handle dbg_value for arguments split into multiple subregs.

Hmm, it looks like the extra DBG_VALUEs dropped in the msp430 test were legitimate locations for "b" -- if getUnderlyingArgRegs now fails but generates multiple {0,0} entries, the ArgRegsAndSizes.size() > 1 block on line 5487 generates no (well, zero sized) locations and returns true. Wheras before EmitFuncArgumentDbgValue returned false, and another code path found a location for "b".

Thu, Jul 18, 3:30 AM

Wed, Jul 17

jmorse added a comment to D64645: DAG: Handle dbg_value for arguments split into multiple subregs.

I got curious; the scenario I described happens in test/DebugInfo/MSP430/sdagsplit-1.ll, where one of the operands to a pair is a load.

Wed, Jul 17, 5:12 AM
jmorse added a comment to D64645: DAG: Handle dbg_value for arguments split into multiple subregs.

This looks good to me, and I'm sort of familiar enough with the debug bits to approve. I've one concern about the change to getUnderlyingArgRegs though: previously it would fail-safe and return 0/$noreg if it encountered a node it didn't recognise. However now, if there's anything unrecognised in the middle of a vector/pair, the element will be silently skipped. That could lead to splitMultiRegDbgValue producing DBG_VALUEs with the wrong fragment offset, as it's unaware of the skipped element.

Wed, Jul 17, 4:23 AM

Mon, Jul 15

jmorse added a comment to D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..

This is a great bug find -- I'd no idea SROA ran twice! Applying this patch gives a ~3% increase in variable locations covered on a clang-3.4 build.

Mon, Jul 15, 9:15 AM · Restricted Project, debug-info
jmorse added a comment to D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

! In D58453#1582991, @yurydelendik wrote:
Looks like I misunderstood meaning of *all*. I assumed that means DBG_VALUEs the refer the vreg def of the source instruction, excluding DBG_VALUEs that refer the same vreg def by the above instructions (in the same block). The test result above shows that collectDebugValues locates all DBG_VALUEs for the instruction even if it does not (re)define the value, but there are few instructions in the same block above that do. Is this the intended effect of this patch? If yes, we probably need to create more robust collectDebugValues-like method that will track lifetime of DBG_VALUEs.

Mon, Jul 15, 6:14 AM · Restricted Project

Fri, Jul 12

jmorse added a comment to D64630: [DebugInfo] Address performance regression with r364515.

Happily D58453 killing off a large amount of placeDbgValues activity significantly reduces DBG_VALUE grouping -- I don't have the numbers to hand, but I would say the density was almost an order of magnitude lower. The largest back in the benchmark I referred to was about ~120, and other large packs occurred much less frequently.

Fri, Jul 12, 1:01 PM · Restricted Project
jmorse updated the diff for D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

I've added some changes to the patch that makes collectDebugValues behave in the way I described, returning all debug users of a VReg, optionally limited to only the basic block of the source MachineInst. This should be useful for testing; and it's immediately caused the test/DebugInfo/WebAssembly/dbg-value-move-reg-stackify.mir [0] test to fail. The output is:

Fri, Jul 12, 9:33 AM · Restricted Project
jmorse created D64630: [DebugInfo] Address performance regression with r364515.
Fri, Jul 12, 6:17 AM · Restricted Project
jmorse updated the diff for D58386: [DebugInfo] Pre-RA MachineSink: sink DBG_VALUEs that don't immediately follow the sunk instruction too.

Refresh this patch:

  • Rebase it onto the current open stack
  • Remove a bunch of needless attributes and metadata
Fri, Jul 12, 2:50 AM · Restricted Project

Thu, Jul 11

jmorse updated the diff for D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies.

Split up subregister comparision into pairs, to make it clear we're comparing unsigneds.

Thu, Jul 11, 8:52 AM · Restricted Project
jmorse updated the diff for D58191: [DebugInfo] Make postra sinking of DBG_VALUEs safe in the presence of subregisters.

Facepalm: it looks like I mucked up the diff before uploading, the patch originally came with this change to MachineSink, not just the test. Oooff.

Thu, Jul 11, 8:36 AM · Restricted Project
jmorse added a comment to D58191: [DebugInfo] Make postra sinking of DBG_VALUEs safe in the presence of subregisters.
In D58191#1579194, @vsk wrote:

This test looks reasonable. Is D58238 the only MachineSink change you have queued up?

Thu, Jul 11, 8:27 AM · Restricted Project

Tue, Jul 9

jmorse committed rG9bebc65d7965: Revert r364515 and r364524 (authored by jmorse).
Revert r364515 and r364524
Tue, Jul 9, 2:39 AM
jmorse committed rL365448: Revert r364515 and r364524.
Revert r364515 and r364524
Tue, Jul 9, 2:37 AM

Tue, Jul 2

jmorse updated the diff for D58191: [DebugInfo] Make postra sinking of DBG_VALUEs safe in the presence of subregisters.

Archaeology-ping: this patch avoids super/sub-register sinking leading to DBG_VALUEs referring to the wrong location, would be good for LLVM-9.

Tue, Jul 2, 9:51 AM · Restricted Project

Mon, Jul 1

jmorse added a comment to D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

The change will be OK: we rely on collectDebugValues in WebAssembly to locate instruction's debug values. And we worry only about these atm.

Mon, Jul 1, 9:46 AM · Restricted Project
jmorse updated the diff for D58450: [DebugInfo][MachineCSE] Don't try to copy-propagate debuginfo for every COPY seen.

Rebase, update jump-insts in MIR test to latest version, address review comments (cheers).

Mon, Jul 1, 9:31 AM · Restricted Project
jmorse committed rGd2b6665e3394: [DebugInfo] Avoid adding too much indirection to pointer-valued variables (authored by jmorse).
[DebugInfo] Avoid adding too much indirection to pointer-valued variables
Mon, Jul 1, 2:40 AM
jmorse committed rL364736: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.
[DebugInfo] Avoid adding too much indirection to pointer-valued variables
Mon, Jul 1, 2:38 AM
jmorse closed D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.
Mon, Jul 1, 2:38 AM · Restricted Project

Fri, Jun 28

jmorse updated the diff for D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.

Vedant wrote:

This looks reasonable to me. Mind adding test coverage for simple tag_offset/fragment expressions?

Fri, Jun 28, 9:38 AM · Restricted Project

Thu, Jun 27

jmorse added a comment to D56265: [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block.

Ancient-ping on this, with the aim of it helping the placeDbgValues patch in. To re-summarise: this collects DBG_VALUEs to rewrite while walking through MachineCopyPropagation, rather than relying on the all-dbg-values-follow-the-defining-inst assumption.

Thu, Jun 27, 10:12 AM
jmorse committed rG3ca8f2b007c8: Add triple to a test I just added. (authored by jmorse).
Add triple to a test I just added.
Thu, Jun 27, 4:55 AM
jmorse committed rL364524: Add triple to a test I just added..
Add triple to a test I just added.
Thu, Jun 27, 4:55 AM
jmorse abandoned D61181: [WIP][DebugInfo] Avoid SelectionDAG un-necessarily debug-referring to dead VRegs.

Abandoning this WIP -- addressing the "values are in two places in SelectionDAG" problem isn't going to be so easy to solve.

Thu, Jun 27, 3:24 AM · Restricted Project
jmorse committed rGd528bcd96573: [DebugInfo] Avoid register coalesing unsoundly changing DBG_VALUE locations (authored by jmorse).
[DebugInfo] Avoid register coalesing unsoundly changing DBG_VALUE locations
Thu, Jun 27, 3:23 AM
jmorse committed rL364515: [DebugInfo] Avoid register coalesing unsoundly changing DBG_VALUE locations.
[DebugInfo] Avoid register coalesing unsoundly changing DBG_VALUE locations
Thu, Jun 27, 3:23 AM
jmorse closed D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.
Thu, Jun 27, 3:23 AM · Restricted Project
jmorse added a comment to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.

Bjorn wrote:

My feeling is that the cases where we now lose some debug info is quite rare (at least for our DSP-C/DSP target), and the benefit of making the register coalescers handling of DBG_VALUE more sound outweigh that problem.

Thu, Jun 27, 3:20 AM · Restricted Project

Wed, Jun 26

jmorse added a comment to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.

I'm reading this as: "this patch make debug info more accurate by rejecting invalid DBG_VALUEs earlier on, but it overshoots the target a bit and also rejects a few(?) false positives". I support this approach, as we all want to move LLVM into the "more accurate" direction, assuming that the false positives are fixable and the ratio of rejected incorrect DBG_VALUEs to false positives is favorable.

Wed, Jun 26, 2:48 AM · Restricted Project

Tue, Jun 25

jmorse added a comment to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.

Ping: we should hammer out this review, seeing how the branch date for llvm-9 has been announced. Could I suggest that everyone is happy with the *implementation* of this patch, but that it's not yet agreed that it needs merging?

Tue, Jun 25, 11:01 AM · Restricted Project

Jun 20 2019

jmorse updated the diff for D58453: [DebugInfo][CGP] Limit placeDbgValues movement of dbg.value intrinsics.

Freshen up patch to make it merge cleanly; check that DVI isn't null... not clear whether something has changed or if I uploaded a completely broken patch before.

Jun 20 2019, 10:03 AM · Restricted Project

Jun 18 2019

jmorse updated the diff for D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.

Account for DW_OP_LLVM_tag_offset when working out whether a DIExpression is "complex" or not.

Jun 18 2019, 8:30 AM · Restricted Project
jmorse added inline comments to D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.
Jun 18 2019, 7:39 AM · Restricted Project
jmorse added inline comments to D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.
Jun 18 2019, 7:35 AM · Restricted Project
jmorse updated the diff for D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.

Add an isComplex() method to DIExpression that detects when an expression could be interpreted as a memory location.

Jun 18 2019, 7:26 AM · Restricted Project
jmorse committed rGa1a4f5f12cc5: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are… (authored by jmorse).
[DebugInfo][Docs] Document that prologue/epilogue variable location changes are…
Jun 18 2019, 1:51 AM
jmorse committed rL363654: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are….
[DebugInfo][Docs] Document that prologue/epilogue variable location changes are…
Jun 18 2019, 1:50 AM
jmorse closed D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored.
Jun 18 2019, 1:50 AM · Restricted Project

Jun 17 2019

jmorse created D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables.
Jun 17 2019, 7:00 AM · Restricted Project
jmorse added a comment to D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored.

Semi-ping: while this is approved, the extra interest suggested I should delay committing in case there's more discussion. Without hearing anything else, I'll land this in the next couple of days.

Jun 17 2019, 1:40 AM · Restricted Project
jmorse added a comment to D61181: [WIP][DebugInfo] Avoid SelectionDAG un-necessarily debug-referring to dead VRegs.

Sort-of ping at @bjope on this: this is fundamentally a speculative patch as I can't dig into the regressions being seen in D56151, and I have no expectations on other peoples time; however we'll need to think about whether those regressions block the placeDbgValues chain-of-things going into llvm-9, which will branch sometime soon (next month?). Similar to what I wrote in D56151, we have a choice of poisons for llvm-9:

  • Leave placeDbgValues as it is,
  • Disable placeDbgValues but don't commit D56151, which may cause dead variable locations to be resurrected during register coalescing,
  • Disable placeDbgValues, commit D56151, triggering the regressions @bjope saw there, which may or may not be alleviated by this patch.
Jun 17 2019, 1:37 AM · Restricted Project
jmorse updated the diff for D61181: [WIP][DebugInfo] Avoid SelectionDAG un-necessarily debug-referring to dead VRegs.

Address feedback / review

Jun 17 2019, 1:02 AM · Restricted Project

Jun 13 2019

jmorse committed rGd2cd9c23b4ec: [NFC] Sink a function call into LiveDebugValues::process (authored by jmorse).
[NFC] Sink a function call into LiveDebugValues::process
Jun 13 2019, 6:13 AM
jmorse added a comment to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Follow up in https://reviews.llvm.org/rL363259

Jun 13 2019, 6:13 AM · Restricted Project
jmorse committed rL363259: [NFC] Sink a function call into LiveDebugValues::process.
[NFC] Sink a function call into LiveDebugValues::process
Jun 13 2019, 6:08 AM
jmorse added a comment to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Blast, missed the final nit, will patch that in a second.

Jun 13 2019, 5:53 AM · Restricted Project
jmorse committed rGbf2b2f08b02d: [DebugInfo] Honour variable fragments in LiveDebugValues (authored by jmorse).
[DebugInfo] Honour variable fragments in LiveDebugValues
Jun 13 2019, 5:50 AM
jmorse committed rL363256: [DebugInfo] Honour variable fragments in LiveDebugValues.
[DebugInfo] Honour variable fragments in LiveDebugValues
Jun 13 2019, 5:49 AM
jmorse closed D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.
Jun 13 2019, 5:48 AM · Restricted Project
jmorse committed rG181bf0cefb26: [DebugInfo] Use FrameDestroy to extend stack locations to end-of-function (authored by jmorse).
[DebugInfo] Use FrameDestroy to extend stack locations to end-of-function
Jun 13 2019, 3:02 AM
jmorse committed rL363245: [DebugInfo] Use FrameDestroy to extend stack locations to end-of-function.
[DebugInfo] Use FrameDestroy to extend stack locations to end-of-function
Jun 13 2019, 3:02 AM
jmorse closed D62314: [DebugInfo] Use FrameDestroy to extend stack locations to the end of a function.
Jun 13 2019, 3:02 AM · Restricted Project

Jun 12 2019

jmorse committed rGe2f94974dfde: [DebugInfo] Add a test that fell out of an earlier commit (authored by jmorse).
[DebugInfo] Add a test that fell out of an earlier commit
Jun 12 2019, 6:42 AM
jmorse committed rL363161: [DebugInfo] Add a test that fell out of an earlier commit.
[DebugInfo] Add a test that fell out of an earlier commit
Jun 12 2019, 6:39 AM
jmorse added inline comments to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.
Jun 12 2019, 4:33 AM · Restricted Project
jmorse updated the diff for D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Use getValueOr when extracting the value of a fragment with a default; assign some better variable names to container references in accumulateFragmentMap

Jun 12 2019, 4:31 AM · Restricted Project

Jun 11 2019

jmorse updated the diff for D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Switch instances of "frag" to "fragment" -- this only forces more wrapping on a few lines and is well worth it. I've also scattered a few more initializer-list replacements for make_pair, and added a fragmentsOverlap helper to DIExpression.

Jun 11 2019, 10:35 AM · Restricted Project
jmorse added a comment to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Please note that SROA is not the only producer of DW_OP_LLVM_fragments. The Swift frontend, for example, can produce fragments from the point where LLVM IR is generated long before LLVM's SROA is invoked. If a condition is not enforced by the Verifier it is not generally safe to rely on it because LLVM IR is generated by many producers, not just Clang.

Jun 11 2019, 9:24 AM · Restricted Project
jmorse updated the diff for D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Add a "default" fragment for DBG_VALUEs that don't have one, which covers all possible fragments. This causes non-fragment DBG_VALUEs to overlap all fragmented DBG_VALUEs.

Jun 11 2019, 9:21 AM · Restricted Project
jmorse added a comment to D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored.
In D63083#1536776, @rnk wrote:

I should add, it's really just the commit message that I find confusing. I think the documentation as written clearly documents LLVM's existing behavior.

Jun 11 2019, 4:35 AM · Restricted Project
jmorse updated the diff for D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored.

Whitespace change to ease reading of .rst, split summary into being an actual commit message

Jun 11 2019, 4:22 AM · Restricted Project

Jun 10 2019

jmorse set the repository for D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored to rL LLVM.
Jun 10 2019, 10:01 AM · Restricted Project
jmorse created D63083: [DebugInfo][Docs] Document that prologue/epilogue variable location changes are ignored.
Jun 10 2019, 10:01 AM · Restricted Project
jmorse committed rGbcff41729209: [DebugInfo] Terminate all location-lists at end of block (authored by jmorse).
[DebugInfo] Terminate all location-lists at end of block
Jun 10 2019, 8:23 AM
jmorse committed rL362951: [DebugInfo] Terminate all location-lists at end of block.
[DebugInfo] Terminate all location-lists at end of block
Jun 10 2019, 8:23 AM
jmorse closed D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
Jun 10 2019, 8:23 AM · Restricted Project

Jun 7 2019

jmorse added inline comments to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.
Jun 7 2019, 10:00 AM · Restricted Project
jmorse added inline comments to D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.
Jun 7 2019, 8:08 AM · Restricted Project

Jun 5 2019

jmorse abandoned D62385: [WIP][DebugInfo] LiveDebugValues should allow different variable fragments to have different locations.

Superceeded by D62904, I felt it cleaner to upload a new review req.

Jun 5 2019, 7:42 AM · Restricted Project
jmorse updated the diff for D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.

Ninja-edit: improve the comment at the top of the added regression test.

Jun 5 2019, 7:41 AM · Restricted Project
jmorse created D62904: [DebugInfo] Honour variable fragments in LiveDebugValues.
Jun 5 2019, 7:40 AM · Restricted Project

Jun 4 2019

jmorse accepted D60831: [DebugInfo@O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion.

LGTM -- this is the already-accepted patch with trivial change, the real difference is that a bug got fixed elsewhere.

Jun 4 2019, 2:46 AM · debug-info, Restricted Project

Jun 3 2019

jmorse added a comment to D60831: [DebugInfo@O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion.

Looking at the delta between the approved and ~final version, it looks good to me, seeing how the bug this tripped over was fixed in D61933 and nothing in the code has changed.

Jun 3 2019, 7:59 AM · debug-info, Restricted Project
jmorse added a comment to D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.

Ping on this -- AFAIUI people believe the change is good, and depending on FrameDestroy is deployed in a separate patch.

Jun 3 2019, 7:35 AM · Restricted Project

May 24 2019

jmorse added a comment to D62379: [DebugInfo] Stop undef fragments from closing non-overlapping fragments.

LGTM with a couple of questions, will leave it for further comments.

May 24 2019, 7:33 AM · Restricted Project, debug-info
jmorse created D62385: [WIP][DebugInfo] LiveDebugValues should allow different variable fragments to have different locations.
May 24 2019, 6:11 AM · Restricted Project
jmorse updated the diff for D62314: [DebugInfo] Use FrameDestroy to extend stack locations to the end of a function.

Move the change in a comment into this patch; remove an un-necessary test comment.

May 24 2019, 5:55 AM · Restricted Project
jmorse added inline comments to D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
May 24 2019, 5:49 AM · Restricted Project
jmorse updated the diff for D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.

Bjorn wrote:

Is the intention to change the behavior for prologues as well here? The description mentions that we do a different analysis for the epilogues. But with this patch we no longer ignore register clobbering for "FrameSetup" (collectChangingRegs used to ignore FrameSetup code).

Maybe that is a good thing (just not obvious when reading the description that this patch also treats prologues differently).

May 24 2019, 5:49 AM · Restricted Project

May 23 2019

jmorse created D62314: [DebugInfo] Use FrameDestroy to extend stack locations to the end of a function.
May 23 2019, 7:04 AM · Restricted Project
jmorse added a child revision for D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging: D62314: [DebugInfo] Use FrameDestroy to extend stack locations to the end of a function.
May 23 2019, 7:04 AM · Restricted Project
jmorse updated the diff for D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.

With this update I've separated the FrameDestroy portion from the rest of the patch. The result is that debug-loc-offset.mir and pr19307.mir are marked XFAIL, as the locations they check no longer extend through the frame-destroy'd part of the produced code. I'll upload the remaining part of the patch in a moment, that adds in use of the frame-destroy flag and un-xfails those tests.

May 23 2019, 6:59 AM · Restricted Project

May 22 2019

jmorse added a comment to D62196: [LiveDebugValues] Close range for previous variable's location when deducing new variable's location.

This looks good to me (I'll leave it for more comments).

May 22 2019, 8:47 AM · Restricted Project, debug-info

May 21 2019

jmorse added a comment to D61600: [DebugInfo] More precise variable range for stack locations.

I suppose that this is ready to land?

May 21 2019, 5:55 AM · Restricted Project, debug-info
jmorse added inline comments to D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
May 21 2019, 5:55 AM · Restricted Project
jmorse updated the diff for D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.

Apply review comments (change a vector size, shuffle the order of some blocks).

May 21 2019, 5:34 AM · Restricted Project

May 17 2019

jmorse added a comment to D61600: [DebugInfo] More precise variable range for stack locations.

Nikola mentioned in D61890 that the changes to fission-ranges.ll result in reduced variable location ranges, which is unfortunate. IMO, eyeballing the test, I think the input IR we have at the moment is wrong, and it was only the bug that this patch fixes that caused us to get the output right.

May 17 2019, 10:06 AM · Restricted Project, debug-info
jmorse added a comment to D61890: [LiveDebugValues] End variable's range with multiple locations at block entry.

The reason I offered this patch as a alternative to D61600 is that it degrades some ranges when it could leave them wider (test/DebugInfo/X86/fission-ranges.ll).

May 17 2019, 10:00 AM · debug-info
jmorse accepted D61600: [DebugInfo] More precise variable range for stack locations.

LGTM (I think I can approve?). The source IR needing no-frame-pointer-elim seems like enough reason to keep that attribute around.

May 17 2019, 9:10 AM · Restricted Project, debug-info

May 16 2019

jmorse retitled D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging from [WIP][DebugInfo] Don't always extend variable locations when the reg location is unchanging to [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
May 16 2019, 4:57 AM · Restricted Project
jmorse updated the diff for D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.

Update diff to more production-ised version, will edit summary and add reviewers shortly

May 16 2019, 4:15 AM · Restricted Project

May 15 2019

jmorse created D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
May 15 2019, 5:10 AM · Restricted Project
jmorse added a child revision for D61600: [DebugInfo] More precise variable range for stack locations: D61940: [DebugInfo] Don't always extend variable locations when the reg location is unchanging.
May 15 2019, 5:10 AM · Restricted Project, debug-info
jmorse added a comment to D61600: [DebugInfo] More precise variable range for stack locations.

Just to note: the patch this is based on (r359426) was reverted in r360301 due to compile time issues. I've got a patch incoming (tomorrow) that relies on this patch to fix it: they'll need to be un-reverted / committed in a certain order and possibly all together. (We can work through that when this is approved though).

May 15 2019, 4:09 AM · Restricted Project, debug-info
jmorse added a comment to D61890: [LiveDebugValues] End variable's range with multiple locations at block entry.

Hmmm, I get what the patch is doing, but I have the feeling that it might be the wrong place (or impossible) to fix in LiveDebugValues. Given the sample source in dbg-live-debug-values-end-range.mir, would I be right in thinking that this patch is to fix the ChangingRegs behaviour that David brought up in [0]?

May 15 2019, 4:05 AM · debug-info

May 10 2019

jmorse added inline comments to D59272: [DebugInfo] Select debug intrinsic line-numbers more carefully when promoting dbg.declare.
May 10 2019, 3:15 AM · Restricted Project