Page MenuHomePhabricator

[llvm-readobj] Print function names with `--bb-addr-map`.
ClosedPublic

Authored by rahmanl on May 20 2021, 11:37 PM.

Details

Summary

This patch uses the getSymbolIndexForFunctionAddress helper function to print function names for BB address map entries.

Diff Detail

Event Timeline

rahmanl created this revision.May 20 2021, 11:37 PM
rahmanl updated this revision to Diff 347168.May 21 2021, 7:26 PM

Rename and refactoring.

rahmanl published this revision for review.May 21 2021, 7:30 PM
rahmanl edited the summary of this revision. (Show Details)
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 7:31 PM

The quadratic behavior leading to a 10 minute latency is too much. Binary search like you mentioned or just hash look up seems like it would drastically reduce the problem, could we bake it in the same change?

rahmanl planned changes to this revision.May 21 2021, 7:58 PM

The quadratic behavior leading to a 10 minute latency is too much. Binary search like you mentioned or just hash look up seems like it would drastically reduce the problem, could we bake it in the same change?

Sure. I'll revise.

Agree with tmsriram@ the quadratic behaviour needs to be addressed, though I think we should split it out as a separate change which can be committed first. That would make it easier for folks to review independently. Thanks!

rahmanl updated this revision to Diff 347554.May 24 2021, 7:48 PM
  • Reimplement symbol index lookup using a hash map. This reduces the 10 minute running time to only 3 seconds.
rahmanl updated this revision to Diff 347558.May 24 2021, 8:02 PM
  • Fix the map value type.
rahmanl planned changes to this revision.May 24 2021, 8:08 PM

Agree with tmsriram@ the quadratic behaviour needs to be addressed, though I think we should split it out as a separate change which can be committed first. That would make it easier for folks to review independently. Thanks!

Ah. Got it. You want to submit the optimization patch before the feature patch. Makes sense.

rahmanl updated this revision to Diff 348096.May 26 2021, 2:23 PM
rahmanl edited the summary of this revision. (Show Details)
jhenderson added inline comments.May 27 2021, 12:42 AM
llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
21

I think you need to check the warning in this case too?

llvm/tools/llvm-readobj/ELFDumper.cpp
6746

Elsewhere, we usually use <?> to indicate unknown names. Probably best to be consistent here.

rahmanl updated this revision to Diff 348429.May 27 2021, 6:54 PM
rahmanl marked 2 inline comments as done.

Check the warning.

rahmanl updated this revision to Diff 348432.May 27 2021, 7:35 PM

Remove unwanted duplicate variable definition.

jhenderson accepted this revision.May 28 2021, 12:25 AM

LGTM, thanks!

This revision is now accepted and ready to land.May 28 2021, 12:25 AM
This revision was automatically updated to reflect the committed changes.