[PATCH] D36929: [GPGPU] Correctly initialize array order and fixed_element information

[Siddharth via Phabricator via llvm-commits]

bollu requested changes to this revision.
bollu added a subscriber: philip.pfaffe.
bollu added a comment.
This revision now requires changes to proceed.

It's cool that `invariant-load-hoisting-with-failing-scop.ll` now succeeds, but that's not the purpose of the test case :)

The test case was supposed to check that when we set `BuildSuccessful = 0` when we fail the "scalar store check" Link to check here <https://github.com/llvm-mirror/polly/blob/7ac8d9b8d3687ba9d677bb3957407b15ff4a0914/lib/CodeGen/PPCGCodeGeneration.cpp#L2180>, we try to set the RTC to 0.

We used to do this incorrectly by making assumptions on the CFG that invariant load hoisting would wreck.

That is, invariant load hoisting would edit the CFG in such a way that us "looking up" the runtime check in the CFG would error out.

The correct way to update this test case is to make the test case more complex so we have an actual store to a scalar.



================
Comment at: lib/CodeGen/PPCGCodeGeneration.cpp:2804
       Access->n_index = Acc->getScopArrayInfo()->getNumberOfDimensions();
+      Access->fixed_element =
+          Acc->isLatestScalarKind() ? isl_bool_true : isl_bool_false;
----------------
Can you add a `TODO`, notifying that we can have `fixed_element` for pointers/arrays as well, and not just scalars? Eg. Only having `A[0]` accesses.

This would help with stuff that @philip.pfaffe was benchmarking (theoretically).


================
Comment at: lib/External/ppcg/gpu.c:256
+	return order;
 	int i;
 
----------------
Consider adding an
```lang=c
assert(0 && "should not reach here, polly should early-exit");
```
after the `return`?


https://reviews.llvm.org/D36929