[llvm] r259675 - Revert r259662, which caused regressions on polly tests.

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 3 14:40:04 PST 2016


>> 2) Polly currently uses SCEV in a way that we need to invalidate this
>> instruction cache at some point as otherwise we reference
>> instructions
>> that are not reachable at the point of use. If you could add
>> aninvalidateNewCache() function (maybe use a better name), I could
>> work
>> out a patch for Polly, such that we can then commit both together.
>
> Can you please explain this more? What is Polly doing that would cause this to happen?
>
> Thanks again,
> Hal
>

Tobias, many thanks for helping to analyze and solve the regressions
caused by my patch.

I did some analysis based on Tobias's hint and found Polly will
generate a new function based on the old one. To generate an
instruction for the new function,  it builds SCEV for the old
instruction, apply some tranformation on the SCEV generated, then
expand the transformed SCEV and insert the expanded value into new
function.

Because SCEV expansion may reuse value cached in ExprValueMap, the
value in old function may be inserted into new function, which is
wrong.

In SCEVExpander::expand, there is a logic to check the cached value to
be used should dominate the insertion point. However, for the above
case, the check always passes. That is because the insertion point is
in a new function, which is unreachable from the old function. However
for unreachable node, DominatorTreeBase::dominates thinks it will be
dominated by any other node.

I have a simple workaround for the problem -- simply add a check that
the cached value to be used in expansion should be in the same
function as the insertion point instruction. With the workaround, the
testcase passes.

--- lib/Analysis/ScalarEvolutionExpander.cpp 2016-02-03 14:38:15.187044436 -0800
+++ lib/Analysis/ScalarEvolutionExpander.1.cpp 2016-02-03
14:37:14.302402574 -0800
@@ -1653,6 +1655,7 @@
       // Choose a Value from the set which dominates the insertPt.
       for (auto const &Ent : *Set) {
         if (Ent && isa<Instruction>(Ent) && S->getType() == Ent->getType() &&
+            cast<Instruction>(Ent)->getFunction() == InsertPt->getFunction() &&
             SE.DT.dominates(cast<Instruction>(Ent), InsertPt)) {
           V = Ent;
           break;


I am still looking at the problem 1).

Thanks,
Wei.


More information about the llvm-commits mailing list