[llvm-commits] [PATCH 2/2] FIx PR12312

[Michael Liao]

ping again. - Michael

On Tue, 2012-08-14 at 23:49 -0700, Michael Liao wrote:
> Here is the revised patch putting 128-bit vector checking earlier.
> 
> Yours
> - Michael
> 
> On Tue, 2012-08-14 at 13:53 -0700, Evan Cheng wrote:
> > +  if (VT.is128BitVector())
> > +    VecIn = DAG.getNode(ISD::BITCAST, DL, MVT::v2i64, VecIn);
> > +  else // FIXME: only 128-bit vector is supported so far.
> > +    return SDValue();
> > 
> > It seems like this check can happen earlier to avoid doing a bunch of work for no good reason.
> > 
> > Evan
> > 
> > On Aug 13, 2012, at 1:25 PM, Michael Liao <michael.liao at intel.com> wrote:
> > 
> > > Here is another revision of the 2nd part. The new change fixes an
> > > potential issue of stale iterator being used. The new loop BFS
> > > traversing OR'd tree is now over index instead of iterator as we may add
> > > elements in that loop.
> > > 
> > > Yours
> > > - Michael
> > > 
> > > On Sun, 2012-08-12 at 14:28 -0700, Michael Liao wrote:
> > >> On Fri, 2012-08-10 at 14:27 -0700, Evan Cheng wrote:
> > >>> +static SDValue FlaggedOrCombine(SDValue Or, X86::CondCode &CC, SelectionDAG &DAG,
> > >>> +                                const X86Subtarget *Subtarget) {
> > >>> 
> > >>> 
> > >>> The current naming convention calls for lower cased first letter.
> > >> 
> > >> The revised patch is attached. I checked the function naming in the 1st
> > >> part of patch as well.
> > >> 
> > >>> 
> > >>> 
> > >>> +  if (VecIn != DAG.getVectorShuffle(VT, DL, VecIn, VecIn, &Mask[0]))
> > >>> +    return SDValue();
> > >>> 
> > >>> What is this trying to do? Is it potentially creating a vectorshuffle node and then throwing it away? Please don't do that.
> > >>> 
> > >> 
> > >> It is checking whether the all vector elements are used in that OR'd
> > >> tree. I revised the code to use a bit mask for this purpose and simplify
> > >> both this checking and other parts.
> > >> 
> > >> Please review the revised patch attached.
> > >> 
> > >> Yours
> > >> - Michael
> > >> 
> > >>> Evan
> > >>> 
> > >>> On Aug 1, 2012, at 12:47 PM, Michael Liao <michael.liao at intel.com> wrote:
> > >>> 
> > >>>> Hi
> > >>>> 
> > >>>> This patch is to fix http://llvm.org/bugs/show_bug.cgi?id=12312, where a
> > >>>> special use of i128 needs efficient code generation with PTEST from
> > >>>> SSE4.1.
> > >>>> 
> > >>>> To fix this issue, 2 patches are developed. The second part is to
> > >>>> recognize that special use, casting from <4 x i32> to i128 followed by a
> > >>>> zero (or non-zero) checking. That sequence is lowered into OR
> > >>>> (X86ISD::OR at the root) tree with all leaves extracted from that vector
> > >>>> because i128 is mapped onto GPR pair. The attached patch adds a
> > >>>> X86-specific DAG optimization to recognize this OR'd tree and convert it
> > >>>> a PTEST if only the flag value is used from that X86ISD::OR root node.
> > >>>> 
> > >>>> Yours
> > >>>> - Michael
> > >>>> <0002-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch>_______________________________________________
> > >>>> llvm-commits mailing list
> > >>>> llvm-commits at cs.uiuc.edu
> > >>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >>> 
> > >> 
> > >> _______________________________________________
> > >> llvm-commits mailing list
> > >> llvm-commits at cs.uiuc.edu
> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > > 
> > > <0001-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch>
> > 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits