This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Use a DenseMap to coalesce MachineOperand locations
ClosedPublic

Authored by rnk on Sep 19 2017, 6:06 PM.

Details

Summary

The new code should be linear in the number of DBG_VALUEs, while the old
code was quadratic.

This is also hopefully a more direct expression of the problem, which
is to:

  1. Rewrite all virtual register operands to stack slots or physical registers
  2. Uniquely number those machine operands, assigning them location numbers
  3. Rewrite all uses of the old location numbers in the interval map to use the new location numbers

In r313400, I attempted to track which locations were spilled in a
parallel bitvector indexed by location number. My code was broken
because these location numbers are not stable during rewriting.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk created this revision.Sep 19 2017, 6:06 PM
aprantl accepted this revision.Sep 20 2017, 10:13 AM

Testcase is the same as in the original commit?

llvm/include/llvm/CodeGen/MachineOperand.h
785 ↗(On Diff #115944)

///

This revision is now accepted and ready to land.Sep 20 2017, 10:13 AM
hans accepted this revision.Sep 20 2017, 10:15 AM

I'm not familiar with the DBG_VALUE stuff, but this looks good as far as I can tell.

llvm/include/llvm/CodeGen/MachineOperand.h
787 ↗(On Diff #115944)

I wonder if this could be made future-proof in case another MO_ enumerator is ever added. Maybe add a MO_LAST = MO_Predicate to the enumeration?

llvm/lib/CodeGen/LiveDebugVariables.cpp
947 ↗(On Diff #115944)

nit: the change description said DenseMap :-)
Maybe comment somehow that the bool is just a dummy here and you're just using it as a set?

970 ↗(On Diff #115944)

Should this comment on the fact that coalescing can happen here if the new location is identical to something already in NewLocations?

rnk marked 2 inline comments as done.Sep 20 2017, 10:29 AM
rnk added inline comments.
llvm/include/llvm/CodeGen/MachineOperand.h
785 ↗(On Diff #115944)

Done, but I think it's private so it won't show up in Doxygen anyway.

787 ↗(On Diff #115944)

Good idea, done.

llvm/lib/CodeGen/LiveDebugVariables.cpp
970 ↗(On Diff #115944)

The coalescing really happens below when we update the IntervalMap. I'll elaborate there on what it is.

This revision was automatically updated to reflect the committed changes.
rnk marked an inline comment as done.