<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 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>
> <rdar://problem/15480077><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>