This is an archive of the discontinued LLVM Phabricator instance.

[Debug-info][InstrRef] Avoid un-necessary ordering in debug value-substitution records
ClosedPublic

Authored by jmorse on Jun 28 2021, 7:09 AM.

Details

Summary

As suggested by @djtodoro in https://reviews.llvm.org/D88891#2843425 , there's a std::map in MachineFunction that doesn't need to be ordered through most of compilation, only at the end. It can be replaced with a vector that gets ordered when needed, which is what this patch does.

NFCI, and only affects the experimental instruction-referencing work that's currently hidden behind a flag.

Diff Detail

Event Timeline

jmorse created this revision.Jun 28 2021, 7:09 AM
jmorse requested review of this revision.Jun 28 2021, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2021, 7:09 AM

@jmorse Thanks!

llvm/include/llvm/CodeGen/MachineFunction.h
469

I guess this needs clang-format

479

I guess we need to update this comment as well.

483

Can we use SmallVector instead?

jmorse updated this revision to Diff 354900.Jun 28 2021, 7:47 AM

Use SmallVector over std::vector, apply some clang-formatting and comment adjusting.

djtodoro accepted this revision.Jun 29 2021, 7:12 AM
This revision is now accepted and ready to land.Jun 29 2021, 7:12 AM
This revision was landed with ongoing or failed builds.Jul 9 2021, 7:53 AM
This revision was automatically updated to reflect the committed changes.
jmorse added a comment.Jul 9 2021, 7:55 AM

Thanks for the review; I found at the last minute that the assertion I'd put in MachineFunction.h for detecting duplicate sources in substitutions wasn't going to work: it would also fire when doing lookups. Checking for duplicates needs to be done explicitly; and it's moderately expensive, so I've downgraded it to an EXPENSIVE_CHECK in InstrRefBasedLDV::initialSetup.