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

Michael Zolotukhin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 15 16:04:29 PST 2016

mzolotukhin accepted this revision.
mzolotukhin added a comment.
This revision is now accepted and ready to land.

Hi Dehao,

Thanks, the patch mostly looks good to me (see minor remarks inline). I also would like Chandler to take a look at this too, as we discussed this with him in the past - I've added him to reviewers.


Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:54
+             "unrolling will reduce the total runtime From X to Y, we will "
+             "boost the loop unroll threshold to DefaultThreshold*(X/Y)^2. "
+             "The boost factor should not exceed this parameter in order to "
The comment is still about quadratic formula:)

Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:748
+                ? UP.PercentMaxThresholdBoost
+                : std::min(100 * Cost->RolledDynamicCost / Cost->UnrolledCost,
+                           UP.PercentMaxThresholdBoost);
This computation potentially may overflow. We probably need some checks against it.

Comment at: test/Transforms/LoopUnroll/full-unroll-heuristics-dce.ll:1
-; RUN: opt < %s -S -loop-unroll -unroll-max-iteration-count-to-analyze=100 -unroll-dynamic-cost-savings-discount=1000 -unroll-threshold=10 -unroll-percent-dynamic-cost-saved-threshold=60 | FileCheck %s
+; RUN: opt < %s -S -loop-unroll -unroll-max-iteration-count-to-analyze=100 -unroll-threshold=12 -unroll-max-percent-threshold-boost=400 | FileCheck %s
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
Minor: if possible, please don't change `-unroll-threshold=12`. We're replacing `-dynamic-cost-savings-discount` and `-unroll-percent-dynamic-cost-saved-threshold`, so the changes should touch only them.

Comment at: test/Transforms/LoopUnroll/partial-unroll-const-bounds.ll:24
   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
-  %exitcond = icmp eq i64 %indvars.iv.next, 10
+  %exitcond = icmp eq i64 %indvars.iv.next, 20
   br i1 %exitcond, label %for.end, label %for.body
Can we avoid this change?  If no, the comment before the test needs to be updated.


More information about the llvm-commits mailing list