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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D70361#1757974 <https://reviews.llvm.org/D70361#1757974>, @bin.narwal wrote:

> > 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.


Part of why I think the code above is wrong is because whatever function created `AccessRelation` also defines/assumes `ArrayElemSize`. The code above re-defines the access relation in terms of bytes, but does not adjust `ArrayElemSize` (that is, `SAI->ElementType`). Effectively, making the array larger than it is.

The only reason this does not break big time is that the size of the outermost dimension (with `DimsAccess == 1` there is just one dimension) is rarely used. It is not needed for the address computation in CodeGen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70361