[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