This is an archive of the discontinued LLVM Phabricator instance.

[BOLT][NFC] Warning for deprecated option '-reorder-blocks=cache+'
ClosedPublic

Authored by nhuhuan on May 31 2022, 2:51 PM.

Details

Summary

Emit warning when using deprecated option '-reorder-blocks=cache+'.
Auto switch to option '-reorder-blocks=ext-tsp'.

Test Plan:

ninja check-bolt

Added a new test cache+-deprecated.test.
Run and verify that the upstream tests are passed.

Diff Detail

Event Timeline

nhuhuan created this revision.May 31 2022, 2:51 PM
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: ayermolo. · View Herald Transcript
nhuhuan requested review of this revision.May 31 2022, 2:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2022, 2:51 PM
rafauler accepted this revision.May 31 2022, 3:24 PM
rafauler added inline comments.
bolt/lib/Passes/BinaryPasses.cpp
190–191

"option cache+ is deprecated, please use ext-tsp.\n"

This revision is now accepted and ready to land.May 31 2022, 3:24 PM
nhuhuan updated this revision to Diff 433223.May 31 2022, 4:22 PM

Update the warning message for option 'cache+'
Specify the flag options in the warning messages as it would be
exposed to the users.

nhuhuan updated this revision to Diff 433224.May 31 2022, 4:24 PM

Update the warning message for option 'cache+'
Specify the flag options in the warning messages as it would be
exposed to the users.

nhuhuan updated this revision to Diff 433225.May 31 2022, 4:26 PM

Update the warning message for option 'cache+'
Specify the flag options in the warning messages as it would be
exposed to the users.

nhuhuan updated this revision to Diff 433536.Jun 1 2022, 1:50 PM

Added test file, fix option switch.

Amir added inline comments.Jun 1 2022, 1:57 PM
bolt/lib/Passes/BinaryPasses.cpp
190–191

Does the new message look good?

maksfb accepted this revision.Jun 1 2022, 2:01 PM
maksfb added inline comments.
bolt/lib/Passes/BinaryPasses.cpp
190

You should drop the trailing period in the message according to LLVM guidelines. Otherwise LGTM.

nhuhuan updated this revision to Diff 433552.Jun 1 2022, 2:50 PM

Removing trailing dot, capture-default in lambda.

Amir added a comment.Jun 1 2022, 2:59 PM

Please address one minor nit. Otherwise LGTM

bolt/include/bolt/Passes/BinaryPasses.h
145
nhuhuan updated this revision to Diff 433567.Jun 1 2022, 3:08 PM

Edit typo in comment in BinaryPasses header

Amir added inline comments.Jun 1 2022, 10:54 PM
bolt/test/cache+-deprecated.test
1 ↗(On Diff #433567)

Final nit: please replace this comment, or drop it altogether.

nhuhuan updated this revision to Diff 433888.Jun 2 2022, 2:20 PM

Remove comments

nhuhuan marked 5 inline comments as done.Jun 2 2022, 2:21 PM
Amir accepted this revision.Jun 3 2022, 2:11 PM
This revision was automatically updated to reflect the committed changes.