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

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:252-253
+    ///                                    PercentMaxThresholdBoost)
+    /// E.g. if complete unrolling reduces the loop execution time by 50%
+    /// then we boost the threshold by the factor of 4x. If unrolling is not
+    /// expected to reduce the running time, then we do not increase the
----------------
Shouldn't it be 2x instead of 4x?


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:54
+             "unrolling will reduce the total runtime by X%, we will boost "
+             "the loop unroll threshold to (1/(1-X%))^2. The boost factor "
+             "should not exceed this parameter in order to prevent from "
----------------
I didn't realize that we use this formula - I thought we're using a linear function. Doesn't it contradict to this?
```
    /// BoostedThreshold = Threshold * min(RolledCost / UnrolledCost,
    ///                                    PercentMaxThresholdBoost)
```


================
Comment at: test/Transforms/LoopUnroll/partial-unroll-const-bounds.ll:1
-; RUN: opt < %s -S -unroll-threshold=20 -loop-unroll -unroll-allow-partial -unroll-runtime -unroll-allow-remainder -unroll-dynamic-cost-savings-discount=0 | FileCheck %s
+; RUN: opt < %s -S -unroll-threshold=20 -loop-unroll -unroll-allow-partial -unroll-runtime -unroll-allow-remainder -unroll-max-percent-threshold-boost=200 | FileCheck %s
 
----------------
I think `unroll-max-percent-threshold-boost` should be `100` to correspond to `unroll-dynamic-cost-savings-discount=0`. BTW, it looks a bit weird that value `100` for the boost means that we are actually not boosting anything.


================
Comment at: test/Transforms/LoopUnroll/unroll-heuristics-pgo.ll:1
-; RUN: opt < %s -S -loop-unroll -unroll-runtime -unroll-threshold=40 -unroll-dynamic-cost-savings-discount=0 | FileCheck %s
+; RUN: opt < %s -S -loop-unroll -unroll-runtime -unroll-threshold=40 -unroll-max-percent-threshold-boost=200 | FileCheck %s
 
----------------
Same here.


https://reviews.llvm.org/D26989