Page MenuHomePhabricator

Implement some micro-optimizations for Identifier. NFC
ClosedPublic

Authored by lattner on Apr 11 2020, 6:16 PM.

Details

Summary

Identifier doesn't maintain a length, so every time strref() is called,
it does a strlen. In the case of comparisons, this isn't necessary:
there is no need to scan a string to get its length, then rescan it to
do the comparison. Just done one comparison.

This also moves some assertions in Identifier::get as another
microoptimization for 'assertions enabled' modes.

Diff Detail

Event Timeline

lattner created this revision.Apr 11 2020, 6:16 PM
Herald added a project: Restricted Project. · View Herald Transcript
lattner updated this revision to Diff 256810.Apr 11 2020, 6:19 PM

Change to increase pedantic correctness.

rriddle accepted this revision.Apr 11 2020, 6:22 PM
rriddle added inline comments.
mlir/include/mlir/IR/Identifier.h
54

Can you add the justification here?

This revision is now accepted and ready to land.Apr 11 2020, 6:22 PM

I wonder if it would be better to just change Identifier to store a StringMapEntry* instead of the raw character buffer. That way strref should be O(1).

Harbormaster completed remote builds in B52824: Diff 256810.
lattner updated this revision to Diff 256818.Apr 11 2020, 9:47 PM

Add comments explaining 'why not memcmp?' to Identifier.h

lattner marked an inline comment as done.Apr 11 2020, 9:51 PM

I improved the comment in Identifier.h. It is a good idea to include a pointer to StringMapEntry. I'll take a look at that as a followup, thanks!

lattner closed this revision.Apr 11 2020, 9:51 PM