[PATCH] D49248: [TargetLowering] Add support for non-uniform vectors to BuildUDIV

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3567
+    if (VT.isVector()) {
+      Q = DAG.getVectorShuffle(VT, dl, Q, NPQ, ShuffleMask);
+      Created->push_back(Q.getNode());
----------------
efriedma wrote:
> RKSimon wrote:
> > RKSimon wrote:
> > > RKSimon wrote:
> > > > efriedma wrote:
> > > > > I'm pretty sure this isn't the most efficient way to handle this case.  Would it be more efficient to use a variable-amount shift to compute NPQ, rather than shuffle afterwards?  Or is it possible to fudge the magic number to avoid the blend?
> > > > > 
> > > > > Please make sure you have test coverage for both UseNPQ=true and UseNPQ=false.
> > > > We do have full coverage for scalars and uniform vectors only - I'll try to add all the necessary non-uniform cases as well.
> > > Tests now ensure we have vector cases with all/none/some NPQ path
> > > I'm pretty sure this isn't the most efficient way to handle this case. Would it be more efficient to use a variable amount shift to compute NPQ, rather than shuffle afterwards? Or is it possible to fudge the magic number to avoid the blend?
> > 
> > @efriedma  I haven't found anything that works any better then the current shuffle - the additional SUB and ADD seem to get in the way - do you have a codegen example of what you have in mind?
> > 
> By "variable amount shift", I was thinking of something like VPSUB+VPSRLVD+VPADD: if the shift amount is zero, it's a no-op.
> 
> The other thing I was thinking is that it might be possible to transform the magic number for an element so that SelNPQ is true for all the elements by shifting magics.a left?  Not sure if that actually works.
ADD( Q, SRL( SUB( N0, Q ), 0 ) ) --> N0

what I think you have in mind is if we can use MULHU/UMUL_LOHI, instead of SRL, so this becomes

ADD( Q, MULHU( SUB( N0, Q ), C ) ) --> Q (C == 0 for non-NPQ path)

Does that sound about right? DAGCombiner::visitMULHU might need some improvements as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D49248