[llvm] [Delinearization] Add function for fixed size array without relying on GEP (PR #145050)

Ryotaro Kasuga via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 8 01:00:27 PDT 2025


kasuga-fj wrote:

> * New function `delinearizeFixedSizeArray` will replace old function `tryDelinearizeFixedSizeImpl`: shall we therefore make the signature the same, i.e. should it at least return a bool upon success/failure of delinearization?

Yes, returning a bool makes perfect sense to me. I will update it.
 
> * Shall we rename `delinearizeFixedSizeArray` to `tryDelinearizeFixedSizeImplExperimental`; maybe that makes things more clear/explicit that one is going to replace the other?

I don't have a strong preference for the current name. However, I'd rather avoid using a word like `experimental` in the function name (similarly, I'm not a big fan of intrinsics such as `llvm.experimental.*` either). IMHO, it's preferable to attach that kind of information via an attribute or a comment instead. So my suggestion would be to mark the existing `tryDelinearizeFixedSizeImpl` as deprecated with `[[deprecated]]` attribute. WDYT?

> * Is there a way we can compare the the new and old delinearization? Or is it too early for that? For example, can we compare the return values where this is used, e.g.:
>   ```
>   bool TrySrcOld = tryDelinearizeFixedSizeImpl(SE, Src, SrcAccessFn, SrcSubscripts, SrcSizes);
>   bool TrySrcNew = tryDelinearizeFixedSizeImplExperimental(SE, Src, SrcAccessFn, SrcSubscripts, SrcSizes);
>   assert(TrySrcOld == TrySrcNew && "something");
>   ```

What are you trying to achieve? As things stand, there are still cases where the old function works but the new one doesn't, so I think adding an assert like that would be too early. If your goal is just to compare the capabilities of the two functions, I think that's possible, but I don’t intend to upstream that kind of code.


> * In [this](https://github.com/llvm/llvm-project/pull/144088#discussion_r2145496052) comment for another change you provided another example where DA is not doing the right thing. Is that case handled here too? Is it worth adding this here, or is it exactly the same as one of the other cases?

I owe you an apology regarding this. I realized the corner case I mentioned was slightly wrong. Sorry about that! (I had used a small script to generate the test case, but it was buggy. I’ve discarded that script immediately). I'll add more details in [the comment you mentioned](https://github.com/llvm/llvm-project/pull/144088#discussion_r2145496052).
Anyway, these cases still cause issues in DA, and I believe replacing `tryDelinearizeFixedSizeImpl` with the new function could resolve them. I'm not entirely sure whether it's worth adding a test case, but if so, it would be better to include it in a separate patch.

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


More information about the llvm-commits mailing list