[PATCH] D12062: Allow values to cause memory accesses

[Johannes Doerfert via llvm-commits]

jdoerfert added a comment.

In http://reviews.llvm.org/D12062#225288, @Meinersbur wrote:

> Can you please explain what "hacks" you are referring to?


The fact that a terminator is the cause of an access and the type of the branch condition as some kind of access type.


================
Comment at: lib/Analysis/ScopInfo.cpp:970
@@ -974,1 +969,3 @@
+    if (StoreMA->isScalar())
+      continue;
     if (StoreMA->isRead())
----------------
Meinersbur wrote:
> Why is this suddenly necessary?
Because getAccessInstruction in line 910 can become null now.

================
Comment at: lib/Analysis/TempScopInfo.cpp:148
@@ -147,3 @@
-    if (!OpI)
-      OpI = OpBB->getTerminator();
-
----------------
Meinersbur wrote:
> I don't see how removing this can work. The .phiops location needs to be written somewhere in the incoming block. If not the terminator, who is it?
This works because the codegen doesn't work the way you think it does. Except for non-affine regions (which are different as explained below) the codegen will not look at the original instruction for scalar write operations but simply insert them at the end of a basic block if they were part of the corresponding ScopStmt.

================
Comment at: lib/CodeGen/BlockGenerators.cpp:1141
@@ -1142,1 +1140,3 @@
+      if (PHIIdx < 0)
+        continue;
       ScalarAddr = getOrCreateAlloca(ScalarBase, PHIOpMap, ".phiops");
----------------
Meinersbur wrote:
> Why did PHIIdx < 0 become possible here, but not in the non-NonAffine case?
Because this is a non-affine region and we do not reschedule it. Here all scalar accesses are summarized in one ScopStmt and we have to figure out where to actually put them while iterating over the actual basic blocks.

But we can actually remove the PHIIdx stuff from the affine case now. Thanks, I'll update the patch.


http://reviews.llvm.org/D12062