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

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

I could further investigate why why condition in generateScalarStores cannot be further simplified, but I don't think its worth bothering. http://reviews.llvm.org/D13762 should clean it all up.


================
Comment at: lib/CodeGen/BlockGenerators.cpp:1170
@@ -1127,1 +1169,3 @@
+               !DT.dominates(cast<Instruction>(Val)->getParent(), StmtExitBB))
+      continue;
 
----------------
Your proposed code fails Isl/CodeGen/phi_in_exit_early_lnt_failure_2.ll  with assertion:
```
Assertion `!verifyGeneratedFunction(S, *EnteringBB->getParent()) && "Verification of generated function failed"' failed.
```

================
Comment at: test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll:20-21
@@ -19,4 +19,4 @@
 ; CHECK-NEXT: %p_val2 = fadd float 1.000000e+00, 2.000000e+00
-; CHECK-NEXT: store float %p_val0, float* %merge.phiops
+; CHECK: store float %p_val0, float* %merge.phiops
 
 branch1:
----------------
```
  %val1.s2a.reload = load float, float* %val1.s2a
  %val2.s2a.reload = load float, float* %val2.s2a
```
appeared because I modified the check in generateScalarLoads that originally was ```isa<PHINode>(Inst)```. The new condition also let loads through that are also not supposed to be there in the first place. I reinstated the original condition although I am not convinced it is correct There could be a legitimate PHI in the subregion's entry block.

Just to remind, this is going to be cleaned up in D13762 anyway.


http://reviews.llvm.org/D13487