[PATCH] D76132: [LoopUnrollAndJam] Changed safety checks to consider more than 2-levels loop nest.

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/test/Transforms/LoopUnrollAndJam/dependencies.ll:1
-; RUN: opt -basicaa -loop-unroll-and-jam -allow-unroll-and-jam -unroll-and-jam-count=4 < %s -S | FileCheck %s
-; RUN: opt -aa-pipeline=basic-aa -passes='unroll-and-jam' -allow-unroll-and-jam -unroll-and-jam-count=4 < %s -S | FileCheck %s
+; RUN: opt -da-disable-delinearization-checks -basicaa -loop-unroll-and-jam -allow-unroll-and-jam -unroll-and-jam-count=4 < %s -S | FileCheck %s
+; RUN: opt -da-disable-delinearization-checks -aa-pipeline=basic-aa -passes='unroll-and-jam' -allow-unroll-and-jam -unroll-and-jam-count=4 < %s -S | FileCheck %s
----------------
fhahn wrote:
> Whitney wrote:
> > dmgreen wrote:
> > > Why is this now using da-disable-delinearization-checks, and why have some of these existing tests been changed to use constant size arrays?
> > `-da-disable-delinearization-checks` is added to more accurately delinearization of fixed-size multi-dimensional arrays. See 
> > https://reviews.llvm.org/D72178 more detail explaination. 
> > 
> > > why have some of these existing tests been changed to use constant size arrays
> > 
> > They were originally testing single dimensional arrays, which may not be ideal for testing sub-sub portion of code. 
> > -da-disable-delinearization-checks is added to more accurately delinearization of fixed-size multi-dimensional arrays. 
> 
> Well, it returns more optimistic results at the loss of soundness IIRC. I think we should definitely keep test coverage without -da-disable-delinearization-checks. This is the common case, we should definitely handle correctly. Ideally we would have multi-dimensional tests that *do not* need -da-disable-delinearization-checks. IIRC constant loop bounds might help with that. 
> 
> IMO tests that really require -da-disable-delinearization-checks should be additional, maybe in a separate file.
> Ideally we would have multi-dimensional tests that *do not* need -da-disable-delinearization-checks. 
It's very difficult to come up with multi-dimensional tests that result in accurate dependence vectors. That's one of the main reasons why this option was added, to be able to let us test/exercise code paths that would otherwise not be taken due to overpessimistic dependence.

> IIRC constant loop bounds might help with that.
The delinearization validity checks compare the subscripts against parameteric terms in the subscript that are believed to be the size of a given dimension. If the arrays have dynamic sizes then the constant loop bound won't help because the subscripts still contain parameteric terms. If the arrays have fixed sizes, then we don't even try to delinearize them unless ` -da-disable-delinearization-checks` is enabled.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76132