[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.
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,
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