[llvm] r206738 - [X86] ISEL (and X, <constant mask>) to BZHI when BMI2 is available.

[Lang Hames]

Hi Ben,

Thanks very much for the review. You're right about small masks. I've fixed
this in r206869: BZHI will only be generated for longer masks (>32 bits)
that would previously have required a movabsq. Short masks will be done
with AND as they had been previously.

As for the early conversion to BZHI nodes - I was following the pattern set
by the other BZHI selection code, which deals with variable masks. Looking
more closely, I *think* this should be doable in tablegen with the
introduction of some ImmLeafs and an SDNodeXForm. I plan to look in to this
next. CC'ing Craig, who's on the blame list for the other BZHI selection
code, and Jim, who knows tablegen better than me: Guys - if you know off
the top of your head that there's some reason this can't be done in
tablegen please give me a heads up, otherwise I'll let you know the outcome
of my experiments. :)

Cheers,
Lang.


On Mon, Apr 21, 2014 at 8:10 PM, Benjamin Kramer <benny.kra at gmail.com>wrote:

>
> On 21.04.2014, at 10:18, Lang Hames <lhames at gmail.com> wrote:
>
> > Author: lhames
> > Date: Mon Apr 21 03:18:53 2014
> > New Revision: 206738
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=206738&view=rev
> > Log:
> > [X86] ISEL (and X, <constant mask>) to BZHI when BMI2 is available.
> >
> > Generating BZHI in the variable mask case, i.e. (and X, (sub (shl 1, N),
> 1)),
> > was already supported, but we were missing the constant-mask case. This
> patch
> > fixes that.
>
> Is this a win for small masks? We always have to load an immediate into a
> register, plain ANDs can encode an immediate directly up to a certain bit
> width.
>
> for example: unsigned y = x & 15;
>
> no bmi2:
> andl    $15, %edi               ## encoding: [0x83,0xe7,0x0f]
>
> bmi2:
> movb    $4, %al                 ## encoding: [0xb0,0x04]
> bzhil   %eax, %edi, %eax        ## encoding: [0xc4,0xe2,0x78,0xf5,0xc7]
>
> I fail to see the improvement here.
>
> Another thing: Why isn't this implemented as a tblgen pattern instead of a
> dag combine? Inserting BZHI nodes early seems counter-productive to me.
>
> - Ben
>
> >
> > <rdar://problem/15480077>
> >
> >
> > Modified:
> >    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> >    llvm/trunk/test/CodeGen/X86/bmi.ll
> >
> > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=206738&r1=206737&r2=206738&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Apr 21 03:18:53
> 2014
> > @@ -18503,6 +18503,20 @@ static SDValue PerformAndCombine(SDNode
> >       }
> >     } // BEXTR
> >
> > +    // Check for BZHI with contiguous mask: (and X, 0x0..0f..f)
> > +    // This should be checked after BEXTR - when X is a shift, a BEXTR
> is
> > +    // preferrable.
> > +    if (Subtarget->hasBMI2()) {
> > +      if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(N1)) {
> > +        uint64_t Mask = C->getZExtValue();
> > +        if (isMask_64(Mask)) {
> > +          unsigned LZ = CountTrailingOnes_64(Mask);
> > +          return DAG.getNode(X86ISD::BZHI, DL, VT, N0,
> > +                             DAG.getConstant(LZ, MVT::i8));
> > +        }
> > +      }
> > +    }
> > +
> >     return SDValue();
> >   }
> >
> >
> > Modified: llvm/trunk/test/CodeGen/X86/bmi.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bmi.ll?rev=206738&r1=206737&r2=206738&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/bmi.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/bmi.ll Mon Apr 21 03:18:53 2014
> > @@ -216,6 +216,24 @@ entry:
> > ; CHECK: bzhiq
> > }
> >
> > +define i32 @bzhi32_constant_mask(i32 %x) #0 {
> > +entry:
> > +  %and = and i32 %x, 1073741823
> > +  ret i32 %and
> > +; CHECK-LABEL: bzhi32_constant_mask:
> > +; CHECK: movb    $30, %al
> > +; CHECK: bzhil   %eax, %edi, %eax
> > +}
> > +
> > +define i64 @bzhi64_constant_mask(i64 %x) #0 {
> > +entry:
> > +  %and = and i64 %x, 4611686018427387903
> > +  ret i64 %and
> > +; CHECK-LABEL: bzhi64_constant_mask:
> > +; CHECK: movb    $62, %al
> > +; CHECK: bzhiq   %rax, %rdi, %rax
> > +}
> > +
> > define i32 @blsi32(i32 %x) nounwind readnone {
> >   %tmp = sub i32 0, %x
> >   %tmp2 = and i32 %x, %tmp
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140422/4e66b3ce/attachment.html>