[PATCH] D127115: [RFC][DAGCombine] Make sure combined nodes are added back to the worklist in topological order.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vxor-vp.ll:166
+; RV32-NEXT:    vxor.vv v8, v8, v10, v0.t
+; RV32-NEXT:    ret
+;
----------------
RKSimon wrote:
> frasercrmck wrote:
> > RKSimon wrote:
> > > @craig.topper Might all of these vrgather regressions be due to a missing combine for splat cases?
> > Something like that. I see that we have the initial pattern:
> > ```
> >           t9: v2i64 = insert_vector_elt undef:v2i64, Constant:i64<-1>, Constant:i64<0>
> >         t10: v2i64 = vector_shuffle<0,0> t9, undef:v2i64
> > ```
> > 
> > on RV64 that's combined:
> > ```
> > Combining: t9: v2i64 = insert_vector_elt undef:v2i64, Constant:i64<-1>, Constant:i64<0>
> >  ... into: t14: v2i64 = BUILD_VECTOR Constant:i64<-1>, undef:i64
> > 
> > Combining: t14: v2i64 = BUILD_VECTOR Constant:i64<-1>, undef:i64
> > 
> > Combining: t10: v2i64 = vector_shuffle<0,0> t14, undef:v2i64
> >  ... into: t15: v2i64 = BUILD_VECTOR Constant:i64<-1>, Constant:i64<-1>
> > 
> >         t15: v2i64 = BUILD_VECTOR Constant:i64<-1>, Constant:i64<-1>
> >       t11: v2i64 = sub t5, t15
> > ```
> > 
> > On RV32 it's:
> > ```
> > Combining: t8: v2i64 = insert_vector_elt undef:v2i64, Constant:i64<-1>, Constant:i32<0>
> >  ... into: t15: v2i64 = BUILD_VECTOR Constant:i64<-1>, undef:i64
> > 
> > Combining: t15: v2i64 = BUILD_VECTOR Constant:i64<-1>, undef:i64
> >  ... into: t16: v2i64 = splat_vector Constant:i64<-1>
> > 
> > Combining: t16: v2i64 = splat_vector Constant:i64<-1>
> > 
> > Combining: t9: v2i64 = vector_shuffle<0,0> t16, undef:v2i64
> > 
> >           t16: v2i64 = splat_vector Constant:i64<-1>
> >         t9: v2i64 = vector_shuffle<0,0> t16, undef:v2i64
> >       t10: v2i64 = sub t5, t9
> > ```
> > 
> > I'm not able to say what //exactly// is going wrong, though. We're definitely missing combines for shuffles of splats, but do we really want to see `splat_vector` for fixed-length vectors here?
> Something you might consider is just use SelectionDAG::getSplatSourceVector / isSplatValue - that way you aren't having to handle specific splat patterns.
Perhaps yeah. I should've said, though, that none of this is happening in RISCV-specific combines. This seems to purely be `visitBUILD_VECTOR` doing the BV->splat combine, and `visitVECTOR_SHUFFLE` working on BV but not splats.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127115