[llvm] [DA] Add tests for nsw doesn't hold on entire iteration space (NFC) (PR #162281)

Ryotaro Kasuga via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 28 15:53:56 PST 2025


================

----------------
kasuga-fj wrote:

>  no code change is needed. (esp. when the change increases the code size from 24 lines to 36 lines).

No. We still need to change the code, since the monotonicity check is currently not performed. Saying that the code has become more complex just because it grew by only 12 lines doesn't make sense, Because the logic has changed substantially, Can you please look at the actual content? Even if you really want to discuss LoC, you should also take into account that I removed `absSCEVNoSignedOverflow` and `mulSCEVNoSignedOverflow`, which were only called from Strong SIV.

> I suspect the existing SCEV behavior will be a headache for loop opts in the future and also it is something that is possible to change

I think relying on the existing SCEV behavior (i.e., the nowrap property must hold all use of the SCEV) makes perfect sense, because

- If I understand correctly, LAA also relies on this. Maybe other passes do so as well. Changing this would have a large impact.
- SCEV doesn't have information such as code point. If we want to make SCEV contest-sensitive, probably it requires a larger redesign.
- There is #91961 to extend SCEV to context-sensitive. If you want to something like nowrap flags under certain contexts, you should take this kind of approach, which doesn't require changing existing SCEV behavior.

---

> > I'm not sure why you are so obsessed with your proofs. I say they are unnecessary. If you want to save your time, I'd recommend pausing it for now.
> 
> Stop this kind of comments. Let's focus on the technical reasoning about the issue at hand. Most of your comment above is irrelevant to the technical discussion.

Since you’ve mentioned several times that it's time-consuming (or similar), I said that with good intentions. But if you didn’t appreciate it, I’ll refrain from saying it again.

I do hope this isn’t too much to ask, but I would sincerely appreciate it if you could avoid pressing the "Request changes" button so casually. If possible, I would appreciate it if you could retract it even now. To be perfectly honest, I feel you pressed this button just because "I don't understand" or "I don't like this approach". Of course any opposing opinion is welcome, but I believe this button should have a more serious meaning and should be used in cases where there is a clear issue or when something absolutely must be fixed. For this particular case, it seems to me that you are requesting changes without fully understanding the various circumstances involved. At the very least, at the time you first pressed that button, it appeared that you had not actually read the DA code (I feel it, e.g., from the first sentence of https://github.com/llvm/llvm-project/pull/162281#issuecomment-3506590442). If that is the case, I don’t believe that this is an appropriate use of the button. I would be grateful if you could understand that I am spending time helping you grasp the the surrounding context, and that "Request changes" can create obstacles for the author and, potentially, for other reviewers as well. Again, some questions and/or opposing opinions are always welcome, however, I would sincerely appreciate it if you could refrain from using that button so lightly. Thank you very much for your understanding.


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


More information about the llvm-commits mailing list