[PATCH] D13487: [Polly] Load/Store scalar accesses before/after the statement itself

[Michael Kruse via llvm-commits]

Meinersbur added inline comments.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:1176
@@ -1166,1 +1175,3 @@
+    // copied directly.
+    if (MA->isPHI() && !IsPHIWrite)
       continue;
----------------
jdoerfert wrote:
> This part and the one below seem tricky and I have to understand the change better to get why/what happens here.
> 
> The idea of a single value map seems reasonable though. Are these two changes coupled?
> This part and the one below seem tricky and I have to understand the change better to get why/what happens here.

These conditions should become unecessary when I do the work to remove redunant accesses earlier in the Polly pipeline. Until then, this is required to keep the unit tests happy. 


> The idea of a single value map seems reasonable though. Are these two changes coupled?

Yes. Before it would be possible scalar loads are created in a BB, and then a BB processed later would look it up and use it although the two BBs might be unrelated in the dominator tree. (Practically I think the generateScalarLoads would just overwrite the older entry in ValueMap, but I am not brave enough to try it). After this patch all scalar loads are inserted into the EntryBB, which is guarantee to dominate everything in the subregion.


http://reviews.llvm.org/D13487