[PATCH] D137791: [SDAG] bail out of mergeTruncStores() if there's an unknown store in the chain

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 29 12:02:54 PST 2022


spatel added a comment.

In D137791#3945116 <https://reviews.llvm.org/D137791#3945116>, @jonpa wrote:

>   N -> St1 -> St2 -> St3 -> Chain
>            /
>   St -> Ld
>
> Ld may be loading from one or more bytes overlapping St2 and/or St3. St could store to bytes in memory overlapping Ld, St2 and/or St3. Or am I missing something here?
>
> (I guess the confusing thing here to me is that the stores to be merged are chained after one another, while they are actually expected to be non-overlapping by the algorithm...)
>
> Personally I would feel safer if we did not allow any chains at all to get dropped - so we expect all the stores (except N) to only be chained before other stores in the group and nothing else. Ignoring and dropping chains seems best done as a separate optimization (like findBetterChain()), or at least deserves a comment when doing so, I'd say.

Yes, let's just check that each store has one use only (as the chain operand of a subsequent store). That ensures that no other interfering operation could occur in the interval between the first store and N. That's the smallest patch, and it does not cause any missed optimizations based on regression tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137791/new/

https://reviews.llvm.org/D137791



More information about the llvm-commits mailing list