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

[nhuhuan]

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.

[rafauler]

bolt/lib/Passes/BinaryPasses.cpp

190–191

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

[nhuhuan]

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

[nhuhuan]

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

[nhuhuan]

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

[nhuhuan]

Added test file, fix option switch.

[Amir]

bolt/lib/Passes/BinaryPasses.cpp

190–191

Does the new message look good?

[maksfb]

bolt/lib/Passes/BinaryPasses.cpp

190

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

[nhuhuan]

Removing trailing dot, capture-default in lambda.

[Amir]

Please address one minor nit. Otherwise LGTM

bolt/include/bolt/Passes/BinaryPasses.h

145

[nhuhuan]

Edit typo in comment in BinaryPasses header

[Amir]

bolt/test/cache+-deprecated.test

2

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

[nhuhuan]

Remove comments

bolt/include/bolt/Passes/BinaryPasses.h

enum LayoutType : char {
     LT_OPTIMIZE,
     /// LT_OPTIMIZE_BRANCH is an implementation of what is suggested in Pettis'
     /// paper (PLDI '90) about block reordering, trying to minimize branch
     /// mispredictions.
     LT_OPTIMIZE_BRANCH,
     /// LT_OPTIMIZE_CACHE piggybacks on the idea from Ispike paper (CGO '04)
     /// that suggests putting frequently executed chains first in the layout.
     LT_OPTIMIZE_CACHE,
+    // CACHE_PLUS and EXT_TSP are synonyms, emit warning of deprecation.
+    LT_OPTIMIZE_CACHE_PLUS,
     /// Block reordering guided by the extended TSP metric.
     LT_OPTIMIZE_EXT_TSP,
     /// Create clusters and use random order for them.
     LT_OPTIMIZE_SHUFFLE,
   };
 
 private:
   void modifyFunctionLayout(BinaryFunction &Function, LayoutType Type,

bolt/lib/Passes/BinaryPasses.cpp

cl::values(
                    "perform optimal layout based on profile"),
         clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_BRANCH,
                    "branch-predictor",
                    "perform optimal layout prioritizing branch "
                    "predictions"),
         clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_CACHE, "cache",
                    "perform optimal layout prioritizing I-cache "
                    "behavior"),
-        clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_EXT_TSP, "cache+",
+        clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_CACHE_PLUS, "cache+",
                    "perform layout optimizing I-cache behavior"),
         clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_EXT_TSP, "ext-tsp",
                    "perform layout optimizing I-cache behavior"),
         clEnumValN(bolt::ReorderBasicBlocks::LT_OPTIMIZE_SHUFFLE,
                    "cluster-shuffle", "perform random layout of clusters")),
-    cl::ZeroOrMore, cl::cat(BoltOptCategory));
+    cl::ZeroOrMore, cl::cat(BoltOptCategory),
+    cl::callback([](const bolt::ReorderBasicBlocks::LayoutType &option) {
+      if (option == bolt::ReorderBasicBlocks::LT_OPTIMIZE_CACHE_PLUS) {
+        WithColor::warning()
+            << "'-reorder-blocks=cache+' is deprecated, "
+            << "please use '-reorder-blocks=ext-tsp' instead\n";
+        ReorderBlocks = bolt::ReorderBasicBlocks::LT_OPTIMIZE_EXT_TSP;
+      }
+    }));
 
 static cl::opt<unsigned>
 ReportBadLayout("report-bad-layout",
   cl::desc("print top <uint> functions with suboptimal code layout on input"),
   cl::init(0),
   cl::ZeroOrMore,
   cl::Hidden,
   cl::cat(BoltOptCategory));

bolt/test/cache+-deprecated.test

+
+
+REQUIRES: system-linux
+
+RUN: %clangxx %p/Inputs/bolt_icf.cpp -g -Wl,-q -o %t.exe
+RUN: llvm-bolt %t.exe -reorder-blocks=cache+ -relocs -o %t 2>&1 | FileCheck %s
+
+CHECK: '-reorder-blocks=cache+' is deprecated, please use '-reorder-blocks=ext-tsp' instead