[all-commits] [llvm/llvm-project] 0789f2: [NFC][SCEV] Piping to pass TTI into SCEVExpander::...
Roman Lebedev via All-commits
all-commits at lists.llvm.org
Tue Feb 25 12:06:54 PST 2020
Branch: refs/heads/master
Home: https://github.com/llvm/llvm-project
Commit: 0789f280483e315d8bcb5e7005e04e7118983b21
https://github.com/llvm/llvm-project/commit/0789f280483e315d8bcb5e7005e04e7118983b21
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
M llvm/include/llvm/Transforms/Utils/LoopUtils.h
M llvm/include/llvm/Transforms/Utils/SimplifyIndVar.h
M llvm/include/llvm/Transforms/Utils/UnrollLoop.h
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/lib/Target/ARM/MVETailPredication.cpp
M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
M llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp
M llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp
M llvm/lib/Transforms/Utils/LoopUnroll.cpp
M llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
M llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
M llvm/lib/Transforms/Utils/LoopUtils.cpp
M llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
M llvm/unittests/Transforms/Utils/UnrollLoopTest.cpp
Log Message:
-----------
[NFC][SCEV] Piping to pass TTI into SCEVExpander::isHighCostExpansionHelper()
Summary:
Future patches will make use of TTI to perform cost-model-driven `SCEVExpander::isHighCostExpansionHelper()`
This is a fully NFC patch to make things reviewable.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, zzheng, javed.absar, dmgreen, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73704
Commit: b99c91a0872f4b7b7e482c40818bbdda4c967bd5
https://github.com/llvm/llvm-project/commit/b99c91a0872f4b7b7e482c40818bbdda4c967bd5
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
M llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
M llvm/lib/Transforms/Utils/LoopUtils.cpp
M llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
Log Message:
-----------
[NFC][SCEV] Piping to pass new SCEVCheapExpansionBudget option into SCEVExpander::isHighCostExpansionHelper()
Summary:
In future patches`SCEVExpander::isHighCostExpansionHelper()` will respect the budget allocated by performing TTI cost modelling.
This is a fully NFC patch to make things reviewable.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, zzheng, javed.absar, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73705
Commit: 1622f3e074cb72feadd6f9d32f21d2030d3bdc47
https://github.com/llvm/llvm-project/commit/1622f3e074cb72feadd6f9d32f21d2030d3bdc47
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[NFC][SCEV] SCEVExpander::isHighCostExpansionHelper(): check that we processed expression first
Summary:
As far as i can tell this is still NFC.
Initially in rL146438 it was added at the top of the function,
later rL238507 dethroned it, and rL244474 did it again.
I'm not sure if we have already checked the cost of this expansion, we should be doing that again.
Reviewers: reames, mkazantsev, wmi, sanjoy, atrick, igor-laevsky
Reviewed By: mkazantsev
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73706
Commit: 2d8275d72e1602a1869e6286b0ffbf9034ab102b
https://github.com/llvm/llvm-project/commit/2d8275d72e1602a1869e6286b0ffbf9034ab102b
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansion(): assert if TTI is not provided
Summary:
Currently, as per `check-llvm`, we never call `SCEVExpander::isHighCostExpansion()` with null TTI,
so this appears to be a safe restriction.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: javed.absar, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73712
Commit: f90973f48645d1d1799d7bdb81cd6873e3a8ab71
https://github.com/llvm/llvm-project/commit/f90973f48645d1d1799d7bdb81cd6873e3a8ab71
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): begin cost modelling - model cast cost
Summary:
This is not a NFC, although it does not change any of the existing tests.
I'm not really sure if we should have specific tests for the cost modelling itself.
This is the first patch that actually makes `SCEVExpander::isHighCostExpansionHelper()`
account for the cost of the SCEV expression, and consider the budget available,
by modelling cast expressions.
I believe the logic itself is "pretty obviously correct" - from budget,
we need to subtract the cost of the cast expression from inner type `Op->getType()`
to the `S->getType()` type, and recurse into the expression we are casting.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: xbolva00, hiraditya, javed.absar, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73716
Commit: b8793f0dabc974aec74ce09362d8790d77c6acba
https://github.com/llvm/llvm-project/commit/b8793f0dabc974aec74ce09362d8790d77c6acba
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model UDiv by power-of-two as LShr
Summary:
Like with casts, we need to subtract the cost of `lshr` instruction
from budget, and recurse into LHS operand.
Seems "pretty obviously correct" to me?
To be noted, there is a number of other shortcuts we //could// cost-model:
* `... + (-1 * ...)` -> `... - ...` <- likely very frequent case
* `x - (rem x, power-of-2)`, which is currently `(x udiv power-of-2) * power-of-2` -> `x & -log2(power-of-2)`
* `rem x, power-of-2`, which is currently `x - ((x udiv power-of-2) * power-of-2)` -> `x & log2(power-of-2)-1`
* `... * power-of-2` -> `... << log2(power-of-2)` <- likely not very beneficial
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73718
Commit: b8abdf9a176116271ed121ebd9c6b2194930e5e7
https://github.com/llvm/llvm-project/commit/b8abdf9a176116271ed121ebd9c6b2194930e5e7
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/test/Transforms/IndVarSimplify/exit_value_test2.ll
Log Message:
-----------
[NFC][IndVarSimplify] Adjust value names in IndVarSimplify/exit_value_test2.ll
%tmp prefix confuses auto-update scripts
Commit: cc29600b908ba3aefb41e53398922319841fdb37
https://github.com/llvm/llvm-project/commit/cc29600b908ba3aefb41e53398922319841fdb37
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/test/Transforms/IndVarSimplify/exit_value_test2.ll
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model plain UDiv
Summary:
If we don't believe this UDiv is actually a LShr in disguise, things are much worse.
First, we try to see if this UDiv actually originates from user code,
by looking for `S + 1`, and if found considering this UDiv to be free.
But otherwise, we always considered this UDiv to be high-cost.
However that is no longer the case with TTI-driven cost model:
our default budget is 4, which matches the default cost of UDiv,
so now we allow a single UDiv to not be counted as high-cost.
While that is the case, it is evident this is actually a regression
due to the fact that cost-modelling is incomplete - we did not account
for the `add`, `mul` costs yet. That is being addressed in D73728.
Cost-modelling for UDiv also seems pretty straight-forward:
subtract cost of the UDiv itself, and recurse into both the LHS and RHS.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73722
Commit: 756af2f88bda16a70df200283f833227f336822a
https://github.com/llvm/llvm-project/commit/756af2f88bda16a70df200283f833227f336822a
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/test/Transforms/IndVarSimplify/elim-extend.ll
M llvm/test/Transforms/IndVarSimplify/exit_value_test2.ll
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model add/mul
Summary:
While this resolves the regression from D73722 in `llvm/test/Transforms/IndVarSimplify/exit_value_test2.ll`,
this now regresses `llvm/test/Transforms/IndVarSimplify/elim-extend.ll` `@nestedIV` test,
we no longer can perform that expansion within default budget of `4`, but require budget of `6`.
That regression is being addressed by D73777.
The basic idea here is simple.
```
Op0, Op1, Op2 ...
| | |
\--+--/ |
| |
\---+---/
```
I.e. given N operands, we will have N-1 operations,
so we have to add cost of an add (mul) for **every** Op processed,
**except** the first one, plus we need to recurse into *every* Op.
I'm guessing there's already canonicalization that ensures we won't
have `1` operand in `scMulExpr`, and no `0` in `scAddExpr`/`scMulExpr`.
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73728
Commit: 0f3c9b54e60b384728c0c24518b8f2645719275e
https://github.com/llvm/llvm-project/commit/0f3c9b54e60b384728c0c24518b8f2645719275e
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model polynomial recurrence
Summary:
So, i wouldn't call this *obviously* correct,
but i think i got it right this time :)
Roughly, we have
```
Op0*x^0 + Op1*x^1 + Op2*x^2 ...
```
where `Op_{n} * x^{n}` is called term, and `n` the degree of term.
Due to the way they are stored internally in `SCEVAddRecExpr`,
i believe we can have `Op_{n}` to be `0`, so we should not charge for those.
I think it is most straight-forward to count the cost in 4 steps:
1. First, count it the same way we counted `scAddExpr`, but be sure to skip terms with zero constants.
Much like with `add` expr we will have one less addition than number of terms.
2. Each non-constant term (term degree >= 1) requires a multiplication between the `Op_{n}` and `x^{n}`.
But again, only charge for it if it is required - `Op_{n}` must not be 0 (no term) or 1 (no multiplication needed),
and obviously don't charge constant terms (`x^0 == 1`).
3. We must charge for all the `x^0`..`x^{poly_degree}` themselves.
Since `x^{poly_degree}` is `x * x * ... * x`, i.e. `poly_degree` `x`'es multiplied,
for final `poly_degree` term we again require `poly_degree-1` multiplications.
Note that all the `x^{0}`..`x^{poly_degree-1}` will be computed for the free along the way there.
4. And finally, the operands themselves.
Here, much like with add/mul exprs, we really don't look for preexisting instructions..
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, javed.absar, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73741
Commit: d6f47aeb5198b142072d8ce2bbf2fdd30d116db0
https://github.com/llvm/llvm-project/commit/d6f47aeb5198b142072d8ce2bbf2fdd30d116db0
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/test/Transforms/IndVarSimplify/elim-extend.ll
M llvm/test/Transforms/IndVarSimplify/eliminate-comparison.ll
M llvm/test/Transforms/IndVarSimplify/eliminate-trunc.ll
M llvm/test/Transforms/IndVarSimplify/full_widening.ll
M llvm/test/Transforms/IndVarSimplify/iv-widen.ll
M llvm/test/Transforms/IndVarSimplify/lftr-multi-exit.ll
M llvm/test/Transforms/IndVarSimplify/lftr-reuse.ll
M llvm/test/Transforms/IndVarSimplify/loop-invariant-conditions.ll
M llvm/test/Transforms/IndVarSimplify/widen-loop-comp.ll
M llvm/test/Transforms/LoopVectorize/X86/float-induction-x86.ll
Log Message:
-----------
[SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model min/max (PR44668)
Summary:
Previosly we simply always said that `SCEVMinMaxExpr` is too costly to expand.
But this isn't really true, it expands into just a comparison+swap pair.
And again much like with add/mul, there will be one less such pair
than the number of operands. And we need to count the cost of operands themselves.
This does change a number of testcases, and as far as i can tell,
all of these changes are improvements, in the sense that
we fixed up more latches to do the [in]equality comparison.
This concludes cost-modelling changes, no other SCEV expressions exist as of now.
This is a part of addressing [[ https://bugs.llvm.org/show_bug.cgi?id=44668 | PR44668 ]].
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, javed.absar, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73744
Commit: 44edc6fd2c63b7db43e13cc8caf1fee79bebdb5f
https://github.com/llvm/llvm-project/commit/44edc6fd2c63b7db43e13cc8caf1fee79bebdb5f
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/lib/Transforms/Utils/LoopUtils.cpp
A llvm/test/Transforms/IndVarSimplify/do-recompute-if-cheap.ll
R llvm/test/Transforms/IndVarSimplify/dont-recompute.ll
M llvm/test/Transforms/IndVarSimplify/elim-extend.ll
M llvm/test/Transforms/IndVarSimplify/lrev-existing-umin.ll
M llvm/test/Transforms/IndVarSimplify/pr28705.ll
M llvm/test/Transforms/IndVarSimplify/pr39673.ll
Log Message:
-----------
[SCEV] rewriteLoopExitValues(): even if have hard uses, still rewrite if cheap (PR44668)
Summary:
Replacing uses of IV outside of the loop is likely generally useful,
but `rewriteLoopExitValues()` is cautious, and if it isn't told to always
perform the replacement, and there are hard uses of IV in loop,
it doesn't replace.
In [[ https://bugs.llvm.org/show_bug.cgi?id=44668 | PR44668 ]],
that prevents `-indvars` from replacing uses of induction variable
after the loop, which might be one of the optimization failures
preventing that code from being vectorized.
Instead, now that the cost model is fixed, i believe we should be
a little bit more optimistic, and also perform replacement
if we believe it is within our budget.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=44668 | PR44668 ]].
Reviewers: reames, mkazantsev, asbirlea, fhahn, skatkov
Reviewed By: mkazantsev
Subscribers: nikic, hiraditya, zzheng, javed.absar, dmgreen, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73501
Commit: 400ceda425ab392096ff2800acad41a4322974d2
https://github.com/llvm/llvm-project/commit/400ceda425ab392096ff2800acad41a4322974d2
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date: 2020-02-25 (Tue, 25 Feb 2020)
Changed paths:
M llvm/include/llvm/Analysis/ScalarEvolutionExpander.h
M llvm/lib/Analysis/ScalarEvolutionExpander.cpp
M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
M llvm/test/Transforms/IndVarSimplify/elim-extend.ll
Log Message:
-----------
[SCEV][IndVars] Always provide insertion point to the SCEVExpander::isHighCostExpansion()
Summary: This addresses the `llvm/test/Transforms/IndVarSimplify/elim-extend.ll` `@nestedIV` regression from D73728
Reviewers: reames, mkazantsev, wmi, sanjoy
Reviewed By: mkazantsev
Subscribers: hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D73777
Compare: https://github.com/llvm/llvm-project/compare/e11f9fb45085...400ceda425ab
More information about the All-commits
mailing list