[llvm] [LoadStoreVectorizer] Allow redundant stores (PR #169946)

[Drew Kersnar via llvm-commits]

dakersnar wrote:

See the discussion in this change around a similar type of tradeoff: https://github.com/llvm/llvm-project/pull/145733, specifically https://github.com/llvm/llvm-project/pull/145733#discussion_r2178863311. We eventually decided to move the optimization _out_ of the vectorizer and _into_ InferAlignment, despite the compile time costs. Following that precedent, this change should also not be in the LSV.

I sorta had similar opinions for the Redundant Loads change, but there was more support for that, so it went through. I think we have to be careful about continuing down that path or the vectorizer will become bloated and way too complex.

Do you have numbers that support this extra DSE pass making a huge dent in compile time? You could always make the change internally in your downstream compiler since this is motivated by your specific compiler pipeline. I have another downstream alignment upgrading change in the LSV (using expensive scalar evolution as a last-ditch-attempt to upgrade alignment right before the chain gets discarded), and I plan on putting it up for discussion when I have time but fully expect we will reach the same conclusion of not including it in the open source LSV.

CC @Artem-B since this is turning into a bit of a meta discussion, and you've been involved in previous ones.

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