This is an archive of the discontinued LLVM Phabricator instance.

[Docs] Update llvm-test-suite PGO instructions to use LLVM IR PGO by default
ClosedPublic

Authored by mingmingl on Jun 12 2023, 12:38 PM.

Details

Summary
  • Currently, the default instructions will use Clang's frontend PGO feature.

Diff Detail

Event Timeline

mingmingl created this revision.Jun 12 2023, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 12:38 PM
Herald added subscribers: wlei, wenlei. · View Herald Transcript
mingmingl requested review of this revision.Jun 12 2023, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 12:38 PM
mingmingl edited the summary of this revision. (Show Details)Jun 12 2023, 12:38 PM
mingmingl added a reviewer: mtrofin.
mtrofin added inline comments.Jun 12 2023, 12:47 PM
llvm/docs/TestSuiteGuide.md
355

I would suggest either making TEST_SUITE_USE_IR_PGO=On default, or at minimum setting it to On in this example (e.g. line 340) and then flipping the comment on line 354.

mingmingl updated this revision to Diff 530664.Jun 12 2023, 2:11 PM
mingmingl retitled this revision from [Docs]Mention DTEST_SUITE_USE_IR_PGO O explicitly for LLLVM IR PGO in llvm-test-suite PGO testing to [Docs] Update llvm-test-suite PGO instructions to use LLVM IR PGO by default.
mingmingl edited the summary of this revision. (Show Details)
mingmingl marked an inline comment as done.
mingmingl added a reviewer: anemet.
mtrofin accepted this revision.Jun 20 2023, 11:28 AM

LGTM, but I'd wait to see if anyone has a strong dependency on the flag being the other way around.

This revision is now accepted and ready to land.Jun 20 2023, 11:28 AM
aeubanks added inline comments.
llvm/docs/TestSuiteGuide.md
355

+1, just change the default in llvm-test-suite and remove -DTEST_SUITE_USE_IR_PGO=ON from the docs

mingmingl added inline comments.Jun 20 2023, 1:49 PM
llvm/docs/TestSuiteGuide.md
355

Thanks for the input!

Given that front-end instrumentation (i.e., fprofile-instr-generate) is mentioned in user-manual (https://clang.llvm.org/docs/UsersManual.html#differences-between-sampling-and-instrumentation) and option-handling, I'm leaning towards keeping the default in case people rely on it in the daily testing workflow.

aeubanks added inline comments.Jun 20 2023, 3:10 PM
llvm/docs/TestSuiteGuide.md
355

I thought we were trying to move away from -fprofile-instr-generate for any sort of PGO and only use it for code coverage, but that requires documentation updating