kasuga-fj wrote:

> This is wrong, we fixed all outstanding bugs reported against DA at the time 
> when we enabled interchange in flang.

I’m referring to the bugs that existed at that time but were not reported. The 
issues that were raised do not represent all the defects.

> If we don't enable interchange by default then we don't get more bugs 
> reported against it because the pass is not getting used.

I don’t think enabling passes just to get more bug reports is a good strategy. 
At the very least, we should carefully check the existing code beforehand.

To be clear: all my bug reports are what I found by reading the code. I didn't 
use flang to find them.

> How many of the current 4 bugs you point to are related to flang?
> There is no mention about flang in any of the bug reports.

Are there any guarantees that Flang won’t generate the IR I mentioned in the 
issues? If so, that’s fine, but I don’t think any such guarantees exist.

> Instead of spreading FUD around, please be more precise and open bug reports 
> for the following:

At the very least I can’t help feeling anxious, since there seem to be so many 
issues that it’s really hard to imagine what might happen.

> Tell me more about it in a separate bug, and let's see how we can work 
> towards fixing it.

Here are some examples that immediately come to my mind (not limited to them):

- 
[This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1246-L1249)
 is wrong, `!isKnownNegative` doesn't mean it's positive.
- 
[This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1250)
 seems incorrect, since we don't check the monotonicity of each subscript.
- 
[This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L1284-L1286)
 is wrong, we don't check whether `X` is non-zero.
- 
[This](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L3116)
 is wrong. In general, the no-wrap flags aren't preserved.
- [In this 
function](https://github.com/llvm/llvm-project/blob/a53e73e6efb4cfe0440b7b35496c0c72b3ae6c4f/llvm/lib/Analysis/DependenceAnalysis.cpp#L2123),
 we must ensure the multiplication doesn't overflow.

But the most serious issue is that DA doesn’t handle overflow at all.

> > These issues aren't caught by assertions and can silently lead to incorrect 
> > transformations.
> 
> What are you speaking about?

I say that DA can miss dependency, and it can result in incorrect legality 
check in LoopInterchange. 

> Please provide data.

I don’t have any data, so I'm fine if the Flang community is okay with it. 
However, I strongly think these issues also affect Flang. In the end, there’s 
no data showing that these issues are unrelated to Flang.

https://github.com/llvm/llvm-project/pull/140182
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to