[PATCH] D25095: [Polly] Ignore removed memory access. NFC.

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

In https://reviews.llvm.org/D25095#557608, @grosser wrote:

> I am worried about a situation where we have sth like:
>
> %val = load %A
>  %div = div %val, 0
>  store %div
>
> If we drop the div, we might trigger undefined behavior.


Did you mean "drop the load"? This patch does not influence non-access instruction handling.

> To my understanding this can never happen, as you will only drop direct copy sequences, right? I am probably too careful here, but if you could quickly check (assert) that the only use of %val is in a store instruction, my worries would disappear.

The correctness of removing a memory access is a concern of the the code that removes them. At codegen-time it is very difficult or even impossible to retroactively check whether a removal was correct. There is no exhaustive list of reasons why it could be removed. The detection mechanism you mentioned is neither sufficient nor mandatory even for just DeLICM. It is invalid if the store, which could also be an implicit store, is not also removed. Uses can also be in other statements that get their llvm::Value from a scalar load, but have no effect on the removal of such a load.

Also consider that not generating a store is just as harmful for the generated code as replacing a load by an `Undef`. The missing store would be even harder to find.

> Also, if you commit this separately, which certainly would not hurt, a JSON test case would be amazing.

The json importer currently does not support removing `MemoryAccess`es. Implementing that will require much more code and certainly also its own review.


https://reviews.llvm.org/D25095