[PATCH] D16522: BlockGenerators: Replace getNewScalarValue with getNewValue

[Michael Kruse via llvm-commits]

Meinersbur added inline comments.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:1216
@@ -1262,3 +1215,3 @@
 
-    Val = getNewScalarValue(Val, R, Stmt, LTS, *LocalBBMap);
+    Val = getNewValue(Stmt, Val, *LocalBBMap, LTS, L);
     Builder.CreateStore(Val, Address);
----------------
I see the problem with getExit(), but I am not yet convinced it is wrong. If it's the backedge that means that we did not leave that loop, i.e. passing that loop to the SCEV expander is correct.

On the other side, getEntry() might be wrong as well. With -polly-allow-nonaffine-loops, there might be a loop in the non-affine subregion as well and the SCEV depend on that. If the entry is part of the loop, SCEV expander will generate an expression depending on its induction variable, but the point of evaluation is the edge to the exit block, which is not part of the contained loop.

ASCII-Art:
```
  _____
 /      Entry
 \      /       
  - loop       
       |               
  loop.after  
        |  \
        |   side
        |   /
        Exit
```
Let's say the loop Entry=>loop has a non-affine exit condition (such as "i*i <n" which AFAIK is supported by SCEV). With -polly-allow-nonaffine-loops, the non-affine subregion should be Entry=>Exit. Exit has a phi:

Exit:
  %phi = phi i32 [%i.inc, %loop.after], [0, %side]

%i.inc needs to be evaluated in %loop.after for the MK_PHI MemoryAccess, but if passing the Loop Entry=>loop to it, we expand to an expression that depends on the loop's iv.



http://reviews.llvm.org/D16522