This is an archive of the discontinued LLVM Phabricator instance.

[NFC][DebugInfo] Create InstructionOrdering helper class (1/4)
ClosedPublic

Authored by Orlando on Aug 18 2020, 10:11 AM.

Details

Summary

Group the map and methods used to query instruction ordering for trimVarLocs
(D82129) into a class. This will make it easier to reuse the functionality upcoming
patches.

Note:
In D86151 I add an InstructionOrdering member to DebugHandlerBase. I then call initialize on it in debugHandlerBase::beginFunction and it is passed to trimVarLocs and validThroughout.

Diff Detail

Event Timeline

Orlando created this revision.Aug 18 2020, 10:11 AM
aprantl added inline comments.Aug 18 2020, 1:46 PM
llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
62

why isn't this a constructor? does it get initialized multiple times?

168

Hmm.. If we can make this a throwaway/RAII object I think I would prefer that

Orlando edited the summary of this revision. (Show Details)Aug 19 2020, 12:56 AM
Orlando added inline comments.
llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
62

Sorry I should have said something in the description about this.

In D86151 I add an InstructionOrdering member to DebugHandlerBase. I then call initialize in DebugHandlerBase::beginFunction, and the object is passed by const ref to trimVarLocs and validThroughout.

aprantl accepted this revision.Aug 19 2020, 9:32 AM
aprantl added inline comments.
llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
92

unnecessary {

This revision is now accepted and ready to land.Aug 19 2020, 9:32 AM
Orlando marked 2 inline comments as done.Aug 19 2020, 10:21 AM

Thanks for the review! I'll land the patch alongside the others in the stack when they're good to go.

llvm/lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
92

Following the recent llvm-dev discussion on braces I got the impression that the preference was to use braces if the body doesn't sit on a single line. But looking closer, it doesn't look like a consensus was reached and AFAICT CodingStandards.rst wasn't updated either.

I'll remove them before landing.