[llvm] [DA] Infer nsw on validated fixed-size delinearized subscripts (PR #201882)
Ryotaro Kasuga via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 6 11:18:43 PDT 2026
kasuga-fj 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, we can 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!
https://github.com/llvm/llvm-project/pull/201882
More information about the llvm-commits
mailing list