[PATCH] D108281: [GVN] Improve alias analysis for parallel accesses

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D108281#2952563 <https://reviews.llvm.org/D108281#2952563>, @alban.bridonneau wrote:

> In which case, we can remove the BB argument to the function created in this patch, and we can also ensure that both the QueryInst and the DepInst are from the same Basic Block, and and the same loop. Does that match what you had in mind?

I think any use of `llvm.loop.parallel_accesses` that does not take into account which loop it is attached to case be correct. A simple mitigation is to test whether it is parallel in all loops. Still, there is non-reducible control flow and intra-iteration dependencies, as in your test case.

> - With regards to using this metadata for better alias analysis, does that mean the particular case in this patch is alright with you?

It would help if the test would mention what it is trying to achieve. A pseudocode equivalent would also be helpful.

I think the test is incorrect. `%gepout` may alias in the same iteration with `%gep1` when `%out == %in`. From `llvm.loop.parallel_accesses` metadata one could at most infer that `%gep1` does not alias with `%gepout` **from other iterations** of the `omp.inner.for.body` loop. Even that is probably also not valid to infer. Strictly, the metadata only states that the memory accesses are not in the way of parallelizing/vectorizing the loop. It may also well be that the `store` is a trivial store (always writes the same value the memory already contains, e.g. because one of %0/%1 is always 0[*]) and hence aliasing still might have occured.

[X] It is controversial whether hardware has well-defined behavior for such writes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108281/new/

https://reviews.llvm.org/D108281