This is an archive of the discontinued LLVM Phabricator instance.

[InjectTLIMappings] Use StringRef instead of std::string for FN name.
ClosedPublic

Authored by nadav on Jul 14 2020, 11:17 AM.

Details

Summary

Remove thousands of std::string instances in a helper function that is frequently called. In the common case we don't need to construct an std::string instance when we can just pass the StringRef to TTI.

All of the clang-check tests pass. I discovered this with a Facebook-internal tool.

Diff Detail

Event Timeline

nadav created this revision.Jul 14 2020, 11:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2020, 11:17 AM
nadav updated this revision to Diff 277912.Jul 14 2020, 11:19 AM
fhahn accepted this revision.Jul 14 2020, 11:55 AM
fhahn added a reviewer: fpetrogalli.
fhahn added a subscriber: fhahn.

LGTM thanks. It might be good to adjust the commit message to be a bit more descriptive, e.g. something like [InjectTLIMappings] Use StringRef instead of std::string for FN name.

llvm/lib/Transforms/Utils/InjectTLIMappings.cpp
80

It might be worth spelling out StringRef here, as the return type is not completely obvious.

86

nit: unrelated whitespace change.

This revision is now accepted and ready to land.Jul 14 2020, 11:55 AM
nadav retitled this revision from Remove thousands of std::string instances in a helper function that is frequently called. to [InjectTLIMappings] Use StringRef instead of std::string for FN name..Jul 14 2020, 12:05 PM
nadav marked 2 inline comments as done.Jul 14 2020, 12:16 PM

Thank you. I will make the changes and try to commit the patch.

nadav updated this revision to Diff 277953.Jul 14 2020, 1:06 PM
fhahn closed this revision.Jul 20 2020, 1:49 AM

@nadav I think the change landed in rG8f0a8ed44e27 and the revision can be closed now, right?

It looks like you included the URL to the review in the commit message, but I think you need to put a something like Differential Revision: https://reviews.llvm.org/D83797 in the message for Phabricator to automatically close the revision.