[PATCH] [MachineLICM] Sink instructions only if they are unlikely to be executed
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());
> 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?
Comment at: lib/CodeGen/MachineLICM.cpp:836
@@ +835,3 @@
+ // factor of the a cost model.
+ if (HeaderFreq * ColdProb < MBFI->getBlockFreq(B))
> 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
> 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.
More information about the llvm-commits