[PATCH] [MachineLICM] Sink instructions only if they are unlikely to be executed

Daniel Jasper djasper at google.com
Thu Mar 19 14:44:17 PDT 2015


================
Comment at: lib/CodeGen/MachineLICM.cpp:799
@@ -792,1 +798,3 @@
 
+  const BranchProbability ColdProb(1, 5);
+  const BlockFrequency HeaderFreq = MBFI->getBlockFreq(CurLoop->getHeader());
----------------
qcolombet wrote:
> Could you add a comment on the choice of the magic constant?
> It’s the same used in the loop vectorizer IIRC, but having some hints why we choose that is a good thing.
> 
> Would it may send to make the denominator parametrizable so that it is easy to check different threshold?
Done.

================
Comment at: lib/CodeGen/MachineLICM.cpp:836
@@ +835,3 @@
+    // factor of the a cost model.
+    if (HeaderFreq * ColdProb < MBFI->getBlockFreq(B))
+      continue;
----------------
qcolombet wrote:
> Shouldn’t we have a threshold in the other direction as well, i.e., Freq(B) < Freq(Preheader) * <some threshold>?
Could you elaborate what that might accomplish?

Also the frequency of the *pre*header doesn't really relate strongly to the frequencies within the loop AFAICT.

================
Comment at: test/CodeGen/X86/sink-cheap-instructions.ll:23
@@ -22,2 +22,3 @@
   %.0 = phi i8* [ %input, %0 ], [ %7, %.backedge.backedge ]
+  tail call void @_Z6assignPj(i32* %6)
   %7 = getelementptr inbounds i8, i8* %.0, i64 1
----------------
qcolombet wrote:
> What is the purpose of this change?
The purpose is to ensure that the getelementptr for %6 is not pulled into the loop as it is used in every iteration. I realize that I have forgotten to actually test that. Fixed.

http://reviews.llvm.org/D8451

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list