[PATCH] D139682: [DAG] Stop replace masked compressstore with normal masked store

[Phoebe Wang via Phabricator via llvm-commits]

pengfei accepted this revision.
pengfei added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11034
   // truncating store.  We can do this even if this is already a masked
-  // truncstore.
+  // truncstore or a compress store.
   if ((Value.getOpcode() == ISD::TRUNCATE) && Value->hasOneUse() &&
----------------
xiangzhangllvm wrote:
> pengfei wrote:
> > Do you mean cannot be a compress store?
> Yes, right
Don't forget update the comments.


================
Comment at: llvm/test/CodeGen/X86/masked_compressstore_isel.ll:15-16
 ; CHECK-NEXT:    %3:vk16wm = VPMOVW2MZ128rr killed %2
-; CHECK-NEXT:    VPMOVDWZ256mrk $noreg, 1, $noreg, 0, $noreg, killed %3, %0 :: (store unknown-size into `i16* null`, align 16)
+; CHECK-NEXT:    %4:vr128x = VPMOVDWZ256rr %0
+; CHECK-NEXT:    VPCOMPRESSWZ128mrk $noreg, 1, $noreg, 0, $noreg, killed %3, killed %4 :: (store unknown-size into `i16* null`, align 16)
 ; CHECK-NEXT:    RET 0
----------------
xiangzhangllvm wrote:
> pengfei wrote:
> > xiangzhangllvm wrote:
> > > pengfei wrote:
> > > > Can we optimizate it to truncated `compressstore`?
> > > Do we have truncated compressstore?
> > > May be can implement by targets.
> > > But for target independent code we can not directly transfer to normal masted store.
> > I think the truncated xxx is just a code concept, it actually means turning the trunction from value to the mask.
> > So I think we should combine these 2 lines into `VPCOMPRESSWZ256mrk ...` just like mask store.
> Can we first let it in and then try do the optimization ?
> I wish to close a must fix bug first. thanks!
Sure. Better add a TODO in comments.


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

https://reviews.llvm.org/D139682