[llvm] [DA] Infer nsw on validated fixed-size delinearized subscripts (PR #201882)

[Madhur Amilkanthwar via llvm-commits]

madhur13490 wrote:

> Disclaimer: I'm not an expert on ScalarEvolution, so my understanding may be wrong.
> 
> First, I think this patch is _sound_. However, I think this issue runs deeper (which is one reason why I'm not trying to fix it myself). And I _think_ that adding such an ad-hoc nowrap flag inference on the client side is not a good approach; we should rely on the existing infrastructure (i.e., ScalarEvolution) as much as possible.
> 
> There are several private methods in ScalarEvolution that infer the nowrap flags, such as proveNoWrapViaConstantRanges, proveNoSignedWrapViaInduction, and so on. In this specific case, ScalarEvolution could prove `0 <=s {0,+,1}<%outer.header> <s 5`, so I'd guess that ScalarEvolution knows the possible range of `{0,+,1}<%outer.header>`. If that's the case, then in theory ScalarEvolution should be able to infer the nowrap flags for the addrec via proveNoWrapViaConstantRanges, but for some reason it doesn't. In fact, I tried forcing a call to this function after creating the addrec, and it was then able to infer both `nuw` and `nsw`. As another example, there is a "hack" to force ScalarEvolution to run some expensive inference: you can create a meaningless SCEV expression such as a sext/zext of the subscripts, and you'll see that `nsw` gets inferred for the subscripts.
> 
> I think this is a known limitation of ScalarEvolution. In particular, as described in [this article](https://www.npopov.com/2023/10/03/LLVM-Scalar-evolution.html), SCEV results are query-order dependent. That is, ScalarEvolution itself already has the capability to infer the nowrap flags in this case, but the inference is not triggered for some reason -- perhaps to mitigate compile-time or to prevent infinite recursion (I don't know the details, though).
> 
> Therefore, IMHO, the right way to address this issue is to somehow trigger the inference in ScalarEvolution that currently isn't being called, instead of adding a client-side workaround. That said, I'm not an expert on ScalarEvolution, so it would be great to have an expert's opinion on this. @nikic I'd appreciate it if you could share your thoughts on this issue. Thanks in advance!

I agree with almost of all of this. I have not spent much time on SCEV for this purpose so I will wait for @nikic to respond on further direction. 

https://github.com/llvm/llvm-project/pull/201882