Page MenuHomePhabricator

[DebugInfo] Make InstrRange into a class, NFC
ClosedPublic

Authored by dstenb on Mar 28 2019, 8:38 AM.

Details

Summary

Replace use of std::pair by creating a class for the debug value
instruction ranges instead. This is a preparatory refactoring for
improving handling of clobbered fragments.

In an upcoming commit the Begin pointer will become a PointerIntPair, so
it will be cleaner to have a getter for that.

Diff Detail

Repository
rL LLVM

Event Timeline

dstenb created this revision.Mar 28 2019, 8:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2019, 8:38 AM

LGTM with some small style nitpicks inline.

include/llvm/CodeGen/DbgEntityHistoryCalculator.h
33 ↗(On Diff #192643)

class Doxygen comment please

lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
50 ↗(On Diff #192643)

!Ranges.back().getEnd() or introduce a helper such as Ranges.back().isOpen()

80 ↗(On Diff #192643)

ditto

dstenb updated this revision to Diff 194314.Apr 9 2019, 6:45 AM
dstenb marked an inline comment as done.

Address review comments.

dstenb marked 2 inline comments as done.Apr 9 2019, 6:58 AM
dstenb added inline comments.
include/llvm/CodeGen/DbgEntityHistoryCalculator.h
33 ↗(On Diff #192643)

I took the comment that was listed above and doxygenized it. I made some adjustments, since the explanation about the range's end was not really correct for fragments.

(The class will change quite a bit in the follow-up patches, so the End explanation will not live for long, but I thought that it could still be good.)

dstenb updated this revision to Diff 194328.Apr 9 2019, 7:59 AM

Slight adjustment: Add an isEnded() query function rather than using getEnd(). An isEnded() function is added in the follow-up patch D59941, so we can avoid unnecessary diffs in that patch by adding such a function here also.

aprantl accepted this revision.Apr 9 2019, 10:05 AM
aprantl added inline comments.
include/llvm/CodeGen/DbgEntityHistoryCalculator.h
45 ↗(On Diff #194328)

I think the proper terminology for this may be isOpen / is Closed? https://en.wikipedia.org/wiki/Interval_(mathematics)

This revision is now accepted and ready to land.Apr 9 2019, 10:05 AM
dstenb marked an inline comment as done.Apr 10 2019, 1:30 AM
dstenb added inline comments.
include/llvm/CodeGen/DbgEntityHistoryCalculator.h
45 ↗(On Diff #194328)

Sorry, yes! I'll change it to "Closed" before landing this.

This revision was automatically updated to reflect the committed changes.