[PATCH] D70361: [Polly][ScopInfo]Fix wrong map in updating AccessRelation of multi-element access

[bin via Phabricator via llvm-commits]

bin.narwal added a comment.

In D70361#1750237 <https://reviews.llvm.org/D70361#1750237>, @Meinersbur wrote:

> Adding @grosser as reviewer who wrote the code in question and already looked into the issue.
>
> I don't think `DimsArray > 1` is a reliable indicator whether `buildAccessMultiDim*` or any of the other access builder has been used. IMHO the issue is that sometimes we have an interpretation of "array of bytes", at other times it is "array of elements". "array of elements" isn't always possibly, for instance if any `memcpy`  occurs on the array. I though of switching to "array of bytes"-always but this yields non-injective access functions that parts of Polly bails out of (e.g. DeLICM is not prepared for this).
>
> When I looked into the issue, I identified the following code being part of the problem:
>
>   if (DimsAccess == 1) {
>     isl::val V = isl::val(Ctx, ArrayElemSize);
>     AccessRelation = AccessRelation.floordiv_val(V);
>   }
>   
>
> which assumes single dimensions only and is just wrong with more dimensions. It seems your patch just does the the same thing with more dimensions.


Hmm, subscripts (thus AccessRelations) for multi-dimensions are built in units of elment type, so it's right to handle only single dimension here?

I guess this assumption is from reading the code building memory access for single/multi dimension cases, but yes, I agree it's not as good as expected.

Anyway, leaving this to the code author.

Thanks,
bin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70361