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

[Michael Liao]

On Mon, 2012-08-27 at 14:39 -0700, Evan Cheng wrote:
> Sorry, just got back.
> 
> One nitpick:
> 
> for (unsigned Slot = 0; Slot < Opnds.size(); ++Slot) {
> 
> You want something like this to avoid calling Opnds.size() in every iteration.
> for (unsigned Slot = 0, e = Opnds.size(); Slot < e; ++Slot) {

As the loop body will append elements into Opnds, this will quit the
loop earlier than expectation. To avoid per-iteration Opnds.size(), I
need extra 'e' updating once this small vector is updated. Here is the
revised patch.

Yours
- Michael

> 
> Otherwise LGTM.
> 
> Evan
> 
> On Aug 18, 2012, at 9:15 PM, Michael Liao <michael.liao at intel.com> wrote:
> 
> > 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
> > 
> > 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-X86-DAG-optimization-to-recognize-a-pttern-PTEST.patch
Type: text/x-patch
Size: 10000 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120827/b558dc14/attachment.bin>