[llvm] [DependenceAnalysis] Extending SIV to handle fusable loops (PR #128782)

[Alireza Torabian via llvm-commits]

1997alireza wrote:

> > The IR provided in the link appears to be incorrect. Specifically, k.0 should be replaced with k.1 in line 31. With this change, the current DA can already determine that there is no dependency by using the RDIV test. Therefore, my patch does not change the result in this case.
> 
> You are completely correct. I was stupid, sorry about that... I now understand that the RDIV test already handles loops whose nesting levels are greater than CommonLevels. So, while I still suspect it might be incorrect in some situations, that's not relevant to your patch. On the other hand, it turns out there are indeed cases where your patch changes the result. Consider the following case:
> 
> ```c
> for (int i = 0; i < 10; i++) {
>   for (int j = 1; j < 11; j++)
>     A[i][j+1][j-1] = 42;
>   for (int j = 0; j < 10; j++)
>     A[i][j][j] = 43;
> }
> ```
> 
> The current DA produces the following result (godbolt: https://godbolt.org/z/5d6xej3bv):
> 
> ```
> Printing analysis 'Dependence Analysis' for function 'f':
> Src:  store i32 42, ptr %idx.0, align 4 --> Dst:  store i32 42, ptr %idx.0, align 4
>   da analyze - none!
> Src:  store i32 42, ptr %idx.0, align 4 --> Dst:  store i32 43, ptr %idx.1, align 4
>   da analyze - output [0|<]!
> Src:  store i32 43, ptr %idx.1, align 4 --> Dst:  store i32 43, ptr %idx.1, align 4
>   da analyze - none!
> ```
> 
> I also ran this locally with your patch, which produced:
> 
> ```
> Printing analysis 'Dependence Analysis' for function 'f':
> Src:  store i32 42, ptr %idx.0, align 4 --> Dst:  store i32 42, ptr %idx.0, align 4
>   da analyze - none!
> Src:  store i32 42, ptr %idx.0, align 4 --> Dst:  store i32 43, ptr %idx.1, align 4
>   da analyze - none!
> Src:  store i32 43, ptr %idx.1, align 4 --> Dst:  store i32 43, ptr %idx.1, align 4
>   da analyze - none!
> ```
> 
> This time it should be correct, and I'm still not sure whether using this result is sound for callers other than LoopFuse.

Yes, my patch will modify certain cases such as this one. May I ask what your specific objection is? Do you believe that my patch produces incorrect results? If not, I do not see any issue with it. The patch enhances the results and can even detect additional cases where no dependency exists. The point you raise regarding changes to the results could equally apply to the RDIV test. For example, what if someone improved RDIV to address cases similar to the one above?

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