This is an archive of the discontinued LLVM Phabricator instance.

[instrprof] Add an overload to accept raw_string_ostream.
ClosedPublic

Authored by snehasish on Jun 27 2023, 11:37 AM.

Details

Summary

Add an overload for InstrProfWriter::write so that users can emit the
buffer to a string. Also use this new overload for existing unit test
usecases.

Diff Detail

Event Timeline

snehasish created this revision.Jun 27 2023, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2023, 11:37 AM
snehasish requested review of this revision.Jun 27 2023, 11:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2023, 11:37 AM
snehasish retitled this revision from [instrprof] Add an overload for raw_fd_ostream. to [instrprof] Add an overload to accept raw_string_ostream..Jun 27 2023, 1:36 PM
snehasish updated this revision to Diff 535120.Jun 27 2023, 1:38 PM

Fix capitalization in comment.

The change looks fine to me but is this patch missing the referenced unit test changes?:

Also use this new overload for existing unit test usecases.

The change looks fine to me but is this patch missing the referenced unit test changes?:

Also use this new overload for existing unit test usecases.

Sorry, my wording wasn't clear. By changing L658 in InstrProfWriter.cpp to use the new overload (in writeBuffer), all the unit tests now implicitly call the new overload.

tejohnson accepted this revision.Jun 28 2023, 9:23 AM

LGTM but please change description per your explanation

This revision is now accepted and ready to land.Jun 28 2023, 9:23 AM
This revision was landed with ongoing or failed builds.Jun 28 2023, 9:37 AM
This revision was automatically updated to reflect the committed changes.

LGTM but please change description per your explanation

Hmm, I amended the commit message in my branch but after landing it I don't see it in git log. Let me try to fix up.

LGTM but please change description per your explanation

Hmm, I amended the commit message in my branch but after landing it I don't see it in git log. Let me try to fix up.

Fixing up now (after other commits have landed on top of this one) is too risky without a revert and restore. Let me know if you feel strongly about updating the message.

LGTM but please change description per your explanation

Hmm, I amended the commit message in my branch but after landing it I don't see it in git log. Let me try to fix up.

Fixing up now (after other commits have landed on top of this one) is too risky without a revert and restore. Let me know if you feel strongly about updating the message.

It's fine as is then, the explanation is here in the patch if anyone is curious.