[PATCH] D141778: [DAGCombiner][X86] `mergeConsecutiveStores()`: support merging splat-stores of the same value

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:674-675
         return StoreSource::Load;
       default:
-        return StoreSource::Unknown;
+        return StoreSource::Splat;
       }
----------------
pengfei wrote:
> Is this too strong to assume all the other cases are `Splat`? If this is the fact, why adding it in `StoreSource` rather than replacing `Unknown`?
What do you mean by "too strong"?
Originally we'd just return `StoreSource::Unknown` and bailout
without looking at other stores. If the other stores *don't* store
the same value, then we'll bailout just the same.

There is probably some generalization -- just because we've matched
a more fine-grained `StoreSource`, doesn't mean that the stores
*aren't* splatting the same value -- but that is not a required part
of this enhancement.

As for `Unknown`, i don't know, but it seems not useful to just drop it,
since it can be used to signal invalid state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141778