This is an archive of the discontinued LLVM Phabricator instance.

Update DBG_VALUE register operand during LiveInterval operations
ClosedPublic

Authored by yurydelendik on Jul 5 2018, 1:41 PM.

Details

Summary

Handling of DBG_VALUE in ConnectedVNInfoEqClasses::Distribute() was fixed in
PR16110. However DBG_VALUE register operands are not getting updated. This
patch properly resolves the value location.

Diff Detail

Repository
rL LLVM

Event Timeline

yurydelendik created this revision.Jul 5 2018, 1:41 PM
yurydelendik retitled this revision from Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass to [WebAssembly] Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass.Jul 5 2018, 2:16 PM
yurydelendik added a reviewer: MatzeB.

Makes sense, thanks for the patch.

Some comments:

  • Remove "WebAssembly" from the title as this should affect other targets too.
lib/CodeGen/LiveInterval.cpp
1318–1327 ↗(On Diff #154304)
  • This should probably use valueOut() instead of valueDefined().
  • Could you change the code to this instead to make the debug value special case more obvious:
const VNInfo *VNI;
if (MI->isDebugValue()) {
  // DBG_VALUE instructions don't have slot indexes, so get the index of the
  // instruction before them. The value is defined there too.
  SlotIndex Idx = LIS.getSlotIndexes()->getIndexBefore(*MI);
  VNI = LI.Query(Idx).valueOut();
} else {
  SlitIndex Idx = LIS.getInstructionIndex(*MI);
  LiveQueryResult LRQ = LI.Query(Idx);
  VNI = ValueIn ? LRQ.valueIn() : LRQ.valueDefined();
}
  • Use valueOut() instead of valueDefined()
  • Make the debug value special case more obvious
yurydelendik retitled this revision from [WebAssembly] Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass to Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass.Jul 16 2018, 9:12 AM
yurydelendik marked an inline comment as done.
vsk added a subscriber: vsk.Jul 16 2018, 12:59 PM

Thanks for the patch!

I don't have any substantive comments, but here are a few suggestions for reducing the test case further:

  • Remove unnecessary attributes.
  • Remove unnecessary lifetime markers.
  • Remove dbg.values which don't reference the variable "a" (i.e metadata !15).

Reducing the test case further: remove attributes, lifetime markers and non-"a" variable metadata.

yurydelendik added a project: debug-info.

@MatzeB or @vsk, can you recommend a reviewer for this patch?

@thegameg and @kparzysz, would either of you have some time to take a look?

MatzeB accepted this revision.Aug 20 2018, 10:24 AM

LGTM
Just change the title when committing since this is in generic code and will affect every target and not just "WebAssembly"

This revision is now accepted and ready to land.Aug 20 2018, 10:24 AM
yurydelendik retitled this revision from Update DBG_VALUE register operand during WebAssemblyOptimizeLiveIntervals pass to Update DBG_VALUE register operand during LiveInterval operations.Aug 20 2018, 5:15 PM
This revision was automatically updated to reflect the committed changes.