[PATCH] D144789: [DAG] Match select(icmp(x,y),sub(x,y),sub(y,x)) -> abd(x,y) patterns

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 12:08:02 PDT 2023


RKSimon added a comment.

ping - apart from the x86 multiuse cmp issues does anyone have anymore comments?



================
Comment at: llvm/test/CodeGen/X86/abds-vector-128.ll:1116
 ; SSE42-NEXT:    pcmpeqd %xmm0, %xmm0
 ; SSE42-NEXT:    pxor %xmm1, %xmm0
+; SSE42-NEXT:    paddq %xmm4, %xmm0
----------------
RKSimon wrote:
> RKSimon wrote:
> > goldstein.w.n wrote:
> > > the:
> > > ```
> > > pcmpgtq %xmm2, %xmm1
> > > pcmpeqd %xmm0, %xmm0
> > > pxor %xmm1, %xmm0
> > > ```
> > > 
> > > is just a duplicate of `pcmpgtq %xmm1, %xmm0`?
> > > 
> > > Likewise for the avx/avx2 tests.
> > > 
> > > Know whats going on there?
> > > 
> > > But guess at the end of the day, this can be fixed in `LowerABD` by looking for existing dag nodes and will be probably be easier to do after this patch.
> > > 
> > > Maybe add a TODO in x86 `LowerABD` to fixup missed optimizations. (Likewise we always `blendv` on `cmp` result, but should be doable on the `sub`, and for avx512 should be `vpternlogd` instead of `blendv`)
> > Yes, IIRC we have an existing problem with other problems with patterns using min/max pairs - we don't do enough to share SETCC nodes.
> We last looked at this for rG813459ed2b0b but I wonder if really we should be doing more generically with SETCC nodes before we get this far.
We're also being hit by the freeze nodes making it tricky to match setcc(x,y) and setcc(freeze(y),freeze(x)) - I think I'd prefer to add this to the list of existing issues we're having with duplicate equivalent compares.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144789



More information about the llvm-commits mailing list