[PATCH] D20276: [MBP] Reduce code size by running tail merging in MBP

[Haicheng Wu via llvm-commits]

haicheng added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1350
@@ -1339,1 +1349,3 @@
+
+  return isBrUpdated;
 }
----------------
deadalnix wrote:
> This is a bit confusing as this can return false in cases where basic blocks are scrambled but no branch was updated. Not that it is incorrect, but it goes against conventions. Also are we sure that this is the only cases that can crate opportunity for the optimization to take place ? Have you tried to just run the optimization every time ? I'd be interested to know if there is any changes.
Thank you very much, Amaury.  I did some research about this.  In short, running the optimization every time can capture more cases and I think I will do it in my next version.  Below is the detail of my findings

  # Running the optimization every time captures five more cases in spec2000/2006.  The same reason behind these cases is that the CFG is changed after the last time we call BranchFolding.  These new opportunities exist before we start MBP.

  # You are right that it is possible to change the layout without changing any branch.  I managed to create such a case, but there is no new tail merging opportunity in this case.

  # On average across spec2006, 74% of MFs need to update at least one branch to change the layout.  The rest either  does not need to change the layout or can change the layout without modifying any branch.

  # I think updateTerminator() can capture all the changes of fallthrough MBBs which is the only thing interesting to me.  In D19955, if updateTerminator() finds its layout successor is inconsistent with the current branch, it always returns true.  So, if running tail merging every time is too much, my current approach can still find all the cases caused by the MBP (I definitely need to make some changes to comply with the convention).

I will look into your looping suggestion as next.




  














Repository:
  rL LLVM

http://reviews.llvm.org/D20276