[PATCH] D26989: Use continuous boosting factor for complete unroll.

Michael Zolotukhin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 1 17:54:41 PST 2016


mzolotukhin added a comment.

Hi Dehao,

Sorry for the delay, I missed the patch for some reason.

A couple of initial remarks:

1. Could you split unrelated changes into a separate (probably, NFC) patch? For instance, changing `int` to `unsigned`.
2. Did you check compile-time impact of this change? LLVM-testsuite exposed a couple of problems with the original version, so I wonder if it's ok with the new one.

Thanks,
Michael



================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:245-249
+    /// If complete unrolling will reduce the cost of the loop, we will boost
+    /// the Threshold by a certain percent to allow more aggressive complete
+    /// unrolling. This value provides the maximum boost percentage that we
+    /// can apply to Threshold (The value should be no less than 100).
+    unsigned PercentMaxThresholdBoost;
----------------
What is the formula for the threshold boost? Could you include it into the comment?


================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:570
                                    PhiVal, ConstantOp);
-  NewCI->setDebugLoc(FirstInst->getDebugLoc());
   return NewCI;
----------------
Why do we need to remove it?


https://reviews.llvm.org/D26989





More information about the llvm-commits mailing list