[PATCH] D143069: [DAGCombine] Allow DAGCombine to remove dead masked stores

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11369
+        !MST->getBasePtr().isUndef() &&
+        ((Mask == MST1->getMask() && MST->getMemoryVT().getStoreSize() ==
+                                         MST1->getMemoryVT().getStoreSize()) ||
----------------
dtemirbulatov wrote:
> sdesmalen wrote:
> > > fixed error with the same mask but different type size by forbid to delete the store
> > I'm not sure if this is the reason you made this change, but the test that previously removed the redundant store:
> > ```define void @dead_masked_store_same_mask_bigger_type(<vscale x 4 x i16> %val, <vscale x 4 x i32> %val1, ptr %a, <vscale x 4 x i1> %mask) {
> > ; CHECK-LABEL: dead_masked_store_same_mask_bigger_type:
> > ; CHECK:       // %bb.0:
> > ; CHECK-NEXT:    st1w { z1.s }, p0, [x0]
> > ; CHECK-NEXT:    ret
> >   call void @llvm.masked.store.nxv4i16(<vscale x 4 x i16> %val, ptr %a, i32 4, <vscale x 4 x i1> %mask)
> >   call void @llvm.masked.store.nxv4i32(<vscale x 4 x i32> %val1, ptr %a, i32 4, <vscale x 4 x i1> %mask)
> >   ret void
> > }
> > ```
> > Is now doing this:
> > ```define void @dead_masked_store_same_mask_bigger_type(<vscale x 4 x i16> %val, <vscale x 4 x i32> %val1, ptr %a, <vscale x 4 x i1> %mask) {
> > ; CHECK-LABEL: dead_masked_store_same_mask_bigger_type:
> > ; CHECK:       // %bb.0:
> > ; CHECK-NEXT:    st1h { z0.s }, p0, [x0]
> > ; CHECK-NEXT:    st1w { z1.s }, p0, [x0]
> > ; CHECK-NEXT:    ret
> >   call void @llvm.masked.store.nxv4i16(<vscale x 4 x i16> %val, ptr %a, i32 4, <vscale x 4 x i1> %mask)
> >   call void @llvm.masked.store.nxv4i32(<vscale x 4 x i32> %val1, ptr %a, i32 4, <vscale x 4 x i1> %mask)
> >   ret void
> > }```
> > 
> > The second store is overwriting **all** the i16 elements that were stored in the first llvm.masked.store, with i32 elements from the second llvm.masked.store, so that means the first store can be removed right?
> The mask is the same for both stores, but because we types are different starting then offsets of starting and ending affected elements different too. I think we have to keep both store in those cases.
The mask is the same for both stores, but because types are different starting then offsets of starting and ending affected elements different too. I think we have to keep both store in those cases.


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

https://reviews.llvm.org/D143069