[PATCH] D73998: [DA] renaming the -da-disable-delinearization-checks option

Mon Feb 24 11:52:53 PST 2020

bmahjour added a comment.

In D73998#1879629 <https://reviews.llvm.org/D73998#1879629>, @Meinersbur wrote:

> As suggestion in D73995 <https://reviews.llvm.org/D73995>, subscripts derived from GEP might be valid with `inrange` modifiers or when we verify that the value range always falls into the dimension's range (e.g. with `ScalarEvolution::getRangeRef`).
>
> I am not convinced the new name is better because "assume inrange subscripts" reads to me like the subscripts are unambiguous (e.g. from the source code) can be reasoned about, while they are derived heuristically, at least in the delinerization case. Alternative suggestions: "-da-allow-unsafe-derived(-multidimensional)-subscripts", "-da-disable-derived(-multidimensional)-index-range-checks", "-da-assume-nonoverlapping-of-heuristically-derived-multidimensional-array-structures" (or combinations thereof).

I'm not particular about the name, but just don't fully understand the distinction being made here. Based on previous discussion on this (http://lists.llvm.org/pipermail/llvm-dev/2019-May/132372.html) the problem seems to be more related to the language specification than the delinearization algorithm itself. In other words, it appears that the only reason why we need these range checks is because (subject to some people's interpretation) C language allows array indexes to overlap. If that's true, then the fact that our subscripts are derived heuristically is irrelevant, because had the user guaranteed that the indexes don't overlap, we wouldn't need to check the range of our derived subscripts.

Are you suggesting that, even if the indexes are in-range at the source-code level, the delinearization can come up with subscripts that are out-of-range? If that's the case I'd be very interested in an example where that happens.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73998