Page MenuHomePhabricator

[LiveDebugValues] Insert entry values after bundles
ClosedPublic

Authored by dstenb on Aug 28 2019, 8:57 AM.

Details

Summary

Change LiveDebugValues so that it inserts entry values after the bundle
which contains the clobbering instruction. Previously it would insert
the debug value after the bundle head using insertAfter(), breaking the
bundle.

Diff Detail

Repository
rL LLVM

Event Timeline

dstenb created this revision.Aug 28 2019, 8:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2019, 8:57 AM
dstenb marked an inline comment as done.Aug 28 2019, 9:00 AM
dstenb added inline comments.
llvm/lib/CodeGen/LiveDebugValues.cpp
1271–1272 ↗(On Diff #217663)

A downstream comment was that this perhaps should become a helper function, e.g. MachineBasicBlock::insertAfterBundled().

@dstenb This is desirable! Thanks a lot for working on this! We also found such case when trying to add support for MIPS and we wanted to shared it, but it is good to see you also have found it!

I think MachineBasicBlock::insertAfterBundled() would be good to have.

djtodoro added inline comments.Aug 28 2019, 9:10 AM
llvm/test/DebugInfo/MIR/Hexagon/live-debug-values-bundled-entry-values.mir
46 ↗(On Diff #217663)

version 10.0.0

57 ↗(On Diff #217663)

version 10.0.0

aprantl added inline comments.Aug 28 2019, 1:37 PM
llvm/lib/CodeGen/LiveDebugValues.cpp
1271–1272 ↗(On Diff #217663)

I think this would be much more readable indeed.

otherwise lgtm

From my point of view this is good to go. 'insertAfterBundle' could go as NFC I suppose.

llvm/lib/CodeGen/LiveDebugValues.cpp
1271–1272 ↗(On Diff #217663)

I would have expected from insertAfter to insert at the end of the bundle. Bundled instructions are usually inserted with MIBundlerBuilder. Comment for MachineBasicBlock::insert() indicates that MIBundleBuilder::insert is designed for prepending/appending to the bundle.

dstenb updated this revision to Diff 217875.Aug 29 2019, 7:32 AM

Add MachineBasicBlock::insertAfterBundle() helper function.

dstenb marked 4 inline comments as done.Aug 29 2019, 7:50 AM
dstenb added inline comments.
llvm/lib/CodeGen/LiveDebugValues.cpp
1271–1272 ↗(On Diff #217663)

Perhaps that is how insertAfter() really ought to behave. Still, I think this new helper function can be useful since the iterator can point to any instruction in the bundle.

vsk accepted this revision.Aug 29 2019, 10:59 AM

LGTM

This revision is now accepted and ready to land.Aug 29 2019, 10:59 AM
This revision was automatically updated to reflect the committed changes.