[llvm] [DA] Rewrite the formula in the Strong SIV test (PR #179665)

[Ryotaro Kasuga via llvm-commits]

kasuga-fj wrote:

To move forward: I strongly disagree with your approach, and will probably never support it. I feel like I've said this several times already, but the reasons are as follows:

- Your approach (and current implementation) has a lot of edge-cases. Adding validations for them one by one may make the code correct at the moment, but such an implementation is very fragile and can easily break in the future. This comes from my experience reviewing and developing DA. People may not read the comments or fully understand the edge-cases, so even if a condition like `SE->isKnownNonNegative(BTC)` is unintentionally removed, it would not be surprising at all.
- There is already agreement on fixing DA at the expense of its functionality (see https://discourse.llvm.org/t/rfc-a-new-pass-as-an-alternative-to-dependenceanalysis/88403). In this context, a robust, reliable, easy-to-maintain, and simpler implementation is more important than extensibility. This is also the rationality behind delaying the introduction monotonicity. A complex algorithm like the one you are proposing should be discussed only after DA is fixed, stabilized (in other words, after it's enabled in the default pipeline).
- I don't expect the default behavior of SCEV to change, i.e., it will be guaranteed that nowrap flags are valid at every iteration. Thus your concern doesn't make much sense to me.

In addition, I feel that you still have not provided an adequate explanation of your concern. You haven't clarified the semantics of "nowrap flags of **addrec** under certain conditions". As I described in some previous comment, it would be very complex even harmful for DA. Moreover, even if such a feature were to be implemented, I don't think it would justify blocking this PR. As you also said, the feature is opt-in, and clients can choose which one to use. That is, it's possible using `const SCEV *` in this part while adopting `SCEVUse` in other parts of the StrongSIV test. I believe using `SCEVUse` in this part of the StrongSIV test would not be profitable.
Furthermore, even under your approach, using such context-sensitive SCEV expressions may be unsound -- these flags are not limited to addrecs. Given an addrec `{c,+,a}`, such context-sensitive flags can appear in `c` and `a` as well. This can affect the result of, for example,  `isKnownPredicate` queries. I'm not sure what it means, but in any case, whichever approach is taken, using such context-sensitive SCEV correctly would be very complex and far from trivial.

---

Let me ask again: are you still blocking this PR? If others agree with me, will you still block this PR?

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