This is an archive of the discontinued LLVM Phabricator instance.

[RDA] getInstFromId: find instructions. NFC.
ClosedPublic

Authored by SjoerdMeijer on Feb 3 2020, 1:31 AM.

Details

Summary

To find the instruction in the block for a given ID, first a count and then a lookup was performed in the map, which is almost the same thing, thus doing double the work.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Feb 3 2020, 1:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2020, 1:31 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

This doesn't look safe to me. lookup will return a default constructed value if an existing object doesn't exist and I'm pretty sure this will be 0. This is also a perfectly reasonable number for InstId to be, therefore providing the wrong MI when an instruction has been inserted as the first instruction in the block, without RDA knowing about it.

Ah, yes, thanks for catching. We can just use find instead.

got rid of the lambda for readability.

samparker accepted this revision.Feb 6 2020, 5:38 AM

Ok.

llvm/lib/CodeGen/ReachingDefAnalysis.cpp
222

Could just return MI here without going through F.

This revision is now accepted and ready to land.Feb 6 2020, 5:38 AM
SjoerdMeijer marked an inline comment as done.Feb 6 2020, 5:42 AM
SjoerdMeijer added inline comments.
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
222

yep, cheers, will change that before committing.

This revision was automatically updated to reflect the committed changes.