<div dir="ltr">Yeah. BEXTR was less co-operative unfortunately. We need to match:<div><br></div><div>and (lsr X, shift_amt), mask</div><div><br></div><div>to</div><div><br></div><div>BEXTR X, (shift_amt << 8 | CountTrailingOnes(mask)).</div>

<div><br></div><div>For that we'd need (at least) to extend SDNodeXForm to handle functions of more than one argument. From DAGISelMatcherGen.cpp:</div><div><br></div>// FIXME2: Could easily generalize this to support multiple inputs and outputs                                                                                                                                                                                                              <br>
// to the SDNodeXForm.  For now we just support one input and one output like                                                                                                                                                                                                               <br>

// the old instruction selector. <div><br></div><div>Do you have any intuition as to whether this would be worth the effort? Are there many other clients who could benefit from binary (or higher) forms? I imagine they're fairly rare.</div>
<div><br>
</div><div>- Lang.                                                                                                                                                                                                                            </div>

</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Apr 23, 2014 at 1:26 AM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div>Excellent. I wouldn't expect trouble getting tblgen to do these sorts of instructions. X86 does a lot of things in c++ code that these days doesn't need to be. I suspect it's due to a combination of so much of the backend predating a lot of tblgen improvements and exactly what you ran into here of rolling the pattern of existing code even when it's not the current practice. </div>
<span class="HOEnZb"><font color="#888888"><div><br></div><div>Jim</div></font></span><div><div class="h5"><div><br>On Apr 22, 2014, at 3:53 AM, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>> wrote:<br>
<br></div><blockquote type="cite"><div><div dir="ltr">That was less painful than I expected. Tablegenified in r206879.<div><br></div><div>I'll try the same for BEXTR tomorrow.</div><div><br></div><div>Thanks again for the review Ben.</div>
<div><br></div><div>
- Lang.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 22, 2014 at 5:56 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Ben,<div><br></div><div>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.</div>


<div><br></div><div>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. :)</div>


<div><br></div><div>Cheers,</div><div>Lang.</div><div><div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Apr 21, 2014 at 8:10 PM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br>



<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>
On 21.04.2014, at 10:18, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>> wrote:<br>
<br>
> Author: lhames<br>
> Date: Mon Apr 21 03:18:53 2014<br>
> New Revision: 206738<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206738&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206738&view=rev</a><br>
> Log:<br>
> [X86] ISEL (and X, <constant mask>) to BZHI when BMI2 is available.<br>
><br>
> Generating BZHI in the variable mask case, i.e. (and X, (sub (shl 1, N), 1)),<br>
> was already supported, but we were missing the constant-mask case. This patch<br>
> fixes that.<br>
<br>
</div>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.<br>
<br>
for example: unsigned y = x & 15;<br>
<br>
no bmi2:<br>
andl    $15, %edi               ## encoding: [0x83,0xe7,0x0f]<br>
<br>
bmi2:<br>
movb    $4, %al                 ## encoding: [0xb0,0x04]<br>
bzhil   %eax, %edi, %eax        ## encoding: [0xc4,0xe2,0x78,0xf5,0xc7]<br>
<br>
I fail to see the improvement here.<br>
<br>
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.<br>
<br>
- Ben<br>
<div><div><br>
><br>
> <<a>rdar://problem/15480077</a>><br>
><br>
><br>
> Modified:<br>
>    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
>    llvm/trunk/test/CodeGen/X86/bmi.ll<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=206738&r1=206737&r2=206738&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=206738&r1=206737&r2=206738&view=diff</a><br>




> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Apr 21 03:18:53 2014<br>
> @@ -18503,6 +18503,20 @@ static SDValue PerformAndCombine(SDNode<br>
>       }<br>
>     } // BEXTR<br>
><br>
> +    // Check for BZHI with contiguous mask: (and X, 0x0..0f..f)<br>
> +    // This should be checked after BEXTR - when X is a shift, a BEXTR is<br>
> +    // preferrable.<br>
> +    if (Subtarget->hasBMI2()) {<br>
> +      if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(N1)) {<br>
> +        uint64_t Mask = C->getZExtValue();<br>
> +        if (isMask_64(Mask)) {<br>
> +          unsigned LZ = CountTrailingOnes_64(Mask);<br>
> +          return DAG.getNode(X86ISD::BZHI, DL, VT, N0,<br>
> +                             DAG.getConstant(LZ, MVT::i8));<br>
> +        }<br>
> +      }<br>
> +    }<br>
> +<br>
>     return SDValue();<br>
>   }<br>
><br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/bmi.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bmi.ll?rev=206738&r1=206737&r2=206738&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/bmi.ll?rev=206738&r1=206737&r2=206738&view=diff</a><br>




> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/bmi.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/bmi.ll Mon Apr 21 03:18:53 2014<br>
> @@ -216,6 +216,24 @@ entry:<br>
> ; CHECK: bzhiq<br>
> }<br>
><br>
> +define i32 @bzhi32_constant_mask(i32 %x) #0 {<br>
> +entry:<br>
> +  %and = and i32 %x, 1073741823<br>
> +  ret i32 %and<br>
> +; CHECK-LABEL: bzhi32_constant_mask:<br>
> +; CHECK: movb    $30, %al<br>
> +; CHECK: bzhil   %eax, %edi, %eax<br>
> +}<br>
> +<br>
> +define i64 @bzhi64_constant_mask(i64 %x) #0 {<br>
> +entry:<br>
> +  %and = and i64 %x, 4611686018427387903<br>
> +  ret i64 %and<br>
> +; CHECK-LABEL: bzhi64_constant_mask:<br>
> +; CHECK: movb    $62, %al<br>
> +; CHECK: bzhiq   %rax, %rdi, %rax<br>
> +}<br>
> +<br>
> define i32 @blsi32(i32 %x) nounwind readnone {<br>
>   %tmp = sub i32 0, %x<br>
>   %tmp2 = and i32 %x, %tmp<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div></div></div>
</blockquote></div><br></div>
</div></blockquote></div></div></div>
</blockquote></div><br></div>