[PATCH] D141931: [BOLT] Consider Code Fragments during regreassign

[Rafael Auler via Phabricator via llvm-commits]

rafauler added a comment.

Thanks! Mostly formatting nits:



================
Comment at: bolt/include/bolt/Core/BinaryFunction.h:376
   /// All fragments for a parent function.
-  SmallPtrSet<BinaryFunction *, 1> Fragments;
+  using FragmentsSetTy = SmallPtrSet<BinaryFunction *, 1>;
+  FragmentsSetTy Fragments;
----------------
Since you added a new getter for "getParentFraments", move this "using" to line 369 and change ParentFragments to use FrafmentsSetTy, just like Fragments.


================
Comment at: bolt/lib/Passes/RegReAssign.cpp:195-197
+  for (BinaryBasicBlock &BB : Function) {
+    countRegScore(BB);
+  }
----------------
nit: drop braces here:




================
Comment at: bolt/lib/Passes/RegReAssign.cpp:198-202
+  for (BinaryFunction *ChildFrag : Function.getFragments()) {
+    for (BinaryBasicBlock &BB : *ChildFrag) {
+      countRegScore(BB);
+    }
   }
----------------



================
Comment at: bolt/lib/Passes/RegReAssign.cpp:230
+  for (BinaryFunction *ChildFrag : Function.getFragments()) {
+    if (ChildFrag->getParentFragments()->size() > 1) return;
+    if (ChildFrag->empty()) {
----------------
clang-format


================
Comment at: bolt/lib/Passes/RegReAssign.cpp:231-233
+    if (ChildFrag->empty()) {
+      return;
+    }
----------------



================
Comment at: bolt/lib/Passes/RegReAssign.cpp:343
+  for (BinaryFunction *ChildFrag : Function.getFragments()) {
+    if (ChildFrag->getParentFragments()->size() > 1) return false;
+    if (ChildFrag->empty()) {
----------------
clang-format


================
Comment at: bolt/lib/Passes/RegReAssign.cpp:344-346
+    if (ChildFrag->empty()) {
+      return false;
+    }
----------------



================
Comment at: bolt/test/X86/reg-reassign-swap-cold.s:12
+# RUN: llvm-bolt %t.exe -o %t.out -data=%t.fdata --reg-reassign | FileCheck %s
+# RUN: %t.out
+
----------------
Because this tests runs the produced output, we need to move this test to the folder test/runtime/X86


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141931/new/

https://reviews.llvm.org/D141931