[PATCH] D156349: [DAG] Support store merging of vector constant stores

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19406
           // Make sure correctly size type is the correct type.
           Val = DAG.getBitcast(MemVT, Val);
         }
----------------
Just a heads up. Found some problems with this downstream.

When it crashes it tries to merge these stores:

```
t218: v4i32 = BUILD_VECTOR Constant:i32<0>, Constant:i32<0>, Constant:i32<0>, Constant:i32<0>
t221: ch = store<(store (s64) into %ir.arrayinit.start, align 1, !tbaa <0x557ab8701858>), trunc to v4i16> t4, t218, t104, undef:i16
t219: v2i32 = BUILD_VECTOR Constant:i32<0>, Constant:i32<0>
t194: ch = store<(store (s64) into %fixed-stack.0 + 15, align 1)> t4, t219, t102, undef:i16
```
So the first store is a v4i32->v4i16 trunc store.

When we arrive on this line (19406) we got `MemVT`=v2i32 but `Val `is t218 from above (so that is a v4i32).
I think the correct thing would be to first truncate to v4i16 and then bitcast to v2i32 (given that we have a trunc store). And maybe the same bug exist for the `IsConstantSrc` false path as well.

I'll see if I can create an upstream reproducer for this. I only see it downstream in certain configs depending on which vector types that are legal, so not sure how easy it will be.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156349