<div dir="ltr">Thanks for the comments.<div>Unfortunately I had the reverse select instruction. I'm committing the fix as soon as the tests finish running.</div><div><br></div><div>Daniel, David: It didn't trigger warnings on the default compile of clang for me, but you made me look again, so I added the parenthesis just in case. I also ran clang-format, which I had forgotten.</div>
<div><br></div><div>Itia, Andrea: Indeed, I was doing the inverse select mask. I'll fix it in the next commit.</div><div><br></div><div>Andrea: The patch was reviewed on-list but not on phab, it seems.</div><div><br></div>
<div>Thanks,</div><div><br></div><div> Filipe</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 27, 2014 at 9:22 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@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 class="HOEnZb"><div class="h5">On Tue, May 27, 2014 at 3:08 AM, Daniel Jasper <<a href="mailto:djasper@google.com">djasper@google.com</a>> wrote:<br>
><br>
><br>
><br>
> On Tue, May 27, 2014 at 5:42 AM, Filipe Cabecinhas <<a href="mailto:me@filcab.net">me@filcab.net</a>> wrote:<br>
>><br>
>> Author: filcab<br>
>> Date: Mon May 26 22:42:20 2014<br>
>> New Revision: 209643<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=209643&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=209643&view=rev</a><br>
>> Log:<br>
>> Convert some X86 blendv* intrinsics into IR.<br>
>><br>
>> Summary:<br>
>> Implemented an InstCombine transformation that takes a blendv* intrinsic<br>
>> call and translates it into an IR select, if the mask is constant.<br>
>><br>
>> This will eventually get lowered into blends with immediates if possible,<br>
>> or pblendvb (with an option to further optimize if we can transform the<br>
>> pblendvb into a blend+immediate instruction, depending on the selector).<br>
>> It will also enable optimizations by the IR passes, which give up on<br>
>> sight of the intrinsic.<br>
>><br>
>> Both the transformation and the lowering of its result to asm got shiny<br>
>> new tests.<br>
>><br>
>> The transformation is a bit convoluted because of blendvp[sd]'s<br>
>> definition:<br>
>><br>
>> Its mask is a floating point value! This forces us to convert it and get<br>
>> the highest bit. I suppose this happened because the mask has type<br>
>> __m128 in Intel's intrinsic and v4sf (for blendps) in gcc's builtin.<br>
>><br>
>> I will send an email to llvm-dev to discuss if we want to change this or<br>
>> not.<br>
>><br>
>> Reviewers: grosbach, delena, nadav<br>
>><br>
>> Differential Revision: <a href="http://reviews.llvm.org/D3859" target="_blank">http://reviews.llvm.org/D3859</a><br>
>><br>
>> Added:<br>
>> llvm/trunk/test/CodeGen/X86/avx2-blend.ll<br>
>> llvm/trunk/test/Transforms/InstCombine/blend_x86.ll<br>
>> Modified:<br>
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
>> llvm/trunk/test/CodeGen/X86/avx-blend.ll<br>
>> llvm/trunk/test/CodeGen/X86/sse41-blend.ll<br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=209643&r1=209642&r2=209643&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=209643&r1=209642&r2=209643&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)<br>
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Mon May 26<br>
>> 22:42:20 2014<br>
>> @@ -718,6 +718,41 @@ Instruction *InstCombiner::visitCallInst<br>
>> break;<br>
>> }<br>
>><br>
>> + case Intrinsic::x86_sse41_pblendvb:<br>
>> + case Intrinsic::x86_sse41_blendvps:<br>
>> + case Intrinsic::x86_sse41_blendvpd:<br>
>> + case Intrinsic::x86_avx_blendv_ps_256:<br>
>> + case Intrinsic::x86_avx_blendv_pd_256:<br>
>> + case Intrinsic::x86_avx2_pblendvb: {<br>
>> + // Convert blendv* to vector selects if the mask is constant.<br>
>> + // This optimization is convoluted because the intrinsic is defined<br>
>> as<br>
>> + // getting a vector of floats or doubles for the ps and pd versions.<br>
>> + // FIXME: That should be changed.<br>
>> + Value *Mask = II->getArgOperand(2);<br>
>> + if (auto C = dyn_cast<ConstantDataVector>(Mask)) {<br>
>> + auto Tyi1 = Builder->getInt1Ty();<br>
>> + auto SelectorType = cast<VectorType>(Mask->getType());<br>
>> + auto EltTy = SelectorType->getElementType();<br>
>> + unsigned Size = SelectorType->getNumElements();<br>
>> + unsigned BitWidth = EltTy->isFloatTy() ? 32 : (EltTy->isDoubleTy()<br>
>> ? 64 : EltTy->getIntegerBitWidth());<br>
>> + assert(BitWidth == 64 || BitWidth == 32 || BitWidth == 8 && "Wrong<br>
>> arguments for variable blend intrinsic");<br>
><br>
><br>
> This is assert is bad in that it triggers Clang's operator precedence<br>
<br>
</div></div>Are you sure this triggered /Clang's/ warning? I was pretty sure<br>
Clang's warning had a suppression for this case, since it doesn't<br>
actually change the semantics of the condition.<br>
<div class="HOEnZb"><div class="h5"><br>
> warning and works correctly more or less by accident (consider the<br>
> precedence of || and && - with the string implicitly evaluating to true).<br>
> Fixed in r209648.<br>
><br>
> Also, please adhere to LLVM coding standards (most importantly the 80 column<br>
> limit).<br>
><br>
>><br>
>> + SmallVector<Constant*, 32> Selectors;<br>
>> + for (unsigned I = 0; I < Size; ++I) {<br>
>> + // The intrinsics only read the top bit<br>
>> + uint64_t Selector;<br>
>> + if (BitWidth == 8)<br>
>> + Selector = C->getElementAsInteger(I);<br>
>> + else<br>
>> + Selector =<br>
>> C->getElementAsAPFloat(I).bitcastToAPInt().getZExtValue();<br>
>> + Selectors.push_back(ConstantInt::get(Tyi1, Selector >> (BitWidth<br>
>> - 1)));<br>
>> + }<br>
>> + auto NewSelector = ConstantVector::get(Selectors);<br>
>> + return SelectInst::Create(NewSelector, II->getArgOperand(0),<br>
>> II->getArgOperand(1), "blendv");<br>
>> + } else {<br>
>> + break;<br>
>> + }<br>
>> + }<br>
>> +<br>
>> case Intrinsic::x86_avx_vpermilvar_ps:<br>
>> case Intrinsic::x86_avx_vpermilvar_ps_256:<br>
>> case Intrinsic::x86_avx_vpermilvar_pd:<br>
>><br>
>> Modified: llvm/trunk/test/CodeGen/X86/avx-blend.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx-blend.ll?rev=209643&r1=209642&r2=209643&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx-blend.ll?rev=209643&r1=209642&r2=209643&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/X86/avx-blend.ll (original)<br>
>> +++ llvm/trunk/test/CodeGen/X86/avx-blend.ll Mon May 26 22:42:20 2014<br>
>> @@ -135,3 +135,26 @@ define <2 x double> @testb(<2 x double><br>
>> %min = select <2 x i1> %min_is_x, <2 x double> %x, <2 x double> %y<br>
>> ret <2 x double> %min<br>
>> }<br>
>> +<br>
>> +; If we can figure out a blend has a constant mask, we should emit the<br>
>> +; blend instruction with an immediate mask<br>
>> +define <4 x double> @constant_blendvpd_avx(<4 x double> %xy, <4 x double><br>
>> %ab) {<br>
>> +; CHECK-LABEL: constant_blendvpd_avx:<br>
>> +; CHECK-NOT: mov<br>
>> +; CHECK: vblendpd<br>
>> +; CHECK: ret<br>
>> + %1 = select <4 x i1> <i1 false, i1 false, i1 true, i1 false>, <4 x<br>
>> double> %xy, <4 x double> %ab<br>
>> + ret <4 x double> %1<br>
>> +}<br>
>> +<br>
>> +define <8 x float> @constant_blendvps_avx(<8 x float> %xyzw, <8 x float><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: constant_blendvps_avx:<br>
>> +; CHECK-NOT: mov<br>
>> +; CHECK: vblendps<br>
>> +; CHECK: ret<br>
>> + %1 = select <8 x i1> <i1 false, i1 false, i1 false, i1 true, i1 false,<br>
>> i1 false, i1 false, i1 true>, <8 x float> %xyzw, <8 x float> %abcd<br>
>> + ret <8 x float> %1<br>
>> +}<br>
>> +<br>
>> +declare <8 x float> @llvm.x86.avx.blendv.ps.256(<8 x float>, <8 x float>,<br>
>> <8 x float>)<br>
>> +declare <4 x double> @llvm.x86.avx.blendv.pd.256(<4 x double>, <4 x<br>
>> double>, <4 x double>)<br>
>><br>
>> Added: llvm/trunk/test/CodeGen/X86/avx2-blend.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx2-blend.ll?rev=209643&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/avx2-blend.ll?rev=209643&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/X86/avx2-blend.ll (added)<br>
>> +++ llvm/trunk/test/CodeGen/X86/avx2-blend.ll Mon May 26 22:42:20 2014<br>
>> @@ -0,0 +1,11 @@<br>
>> +; RUN: llc < %s -mtriple=x86_64-apple-darwin -mcpu=core-avx2 | FileCheck<br>
>> %s<br>
>> +<br>
>> +define <32 x i8> @constant_pblendvb_avx2(<32 x i8> %xyzw, <32 x i8><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: constant_pblendvb_avx2:<br>
>> +; CHECK: vmovdqa<br>
>> +; CHECK: vpblendvb<br>
>> + %1 = select <32 x i1> <i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false>, <32 x i8> %xyzw, <32 x i8> %abcd<br>
>> + ret <32 x i8> %1<br>
>> +}<br>
>> +<br>
>> +declare <32 x i8> @llvm.x86.avx2.pblendvb(<32 x i8>, <32 x i8>, <32 x<br>
>> i8>)<br>
>><br>
>> Modified: llvm/trunk/test/CodeGen/X86/sse41-blend.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sse41-blend.ll?rev=209643&r1=209642&r2=209643&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sse41-blend.ll?rev=209643&r1=209642&r2=209643&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/CodeGen/X86/sse41-blend.ll (original)<br>
>> +++ llvm/trunk/test/CodeGen/X86/sse41-blend.ll Mon May 26 22:42:20 2014<br>
>> @@ -88,3 +88,35 @@ entry:<br>
>> store double %extract214vector_func.i, double addrspace(1)* undef,<br>
>> align 8<br>
>> ret void<br>
>> }<br>
>> +<br>
>> +; If we can figure out a blend has a constant mask, we should emit the<br>
>> +; blend instruction with an immediate mask<br>
>> +define <2 x double> @constant_blendvpd(<2 x double> %xy, <2 x double><br>
>> %ab) {<br>
>> +; In this case, we emit a simple movss<br>
>> +; CHECK-LABEL: constant_blendvpd<br>
>> +; CHECK: movsd<br>
>> +; CHECK: ret<br>
>> + %1 = select <2 x i1> <i1 true, i1 false>, <2 x double> %xy, <2 x<br>
>> double> %ab<br>
>> + ret <2 x double> %1<br>
>> +}<br>
>> +<br>
>> +define <4 x float> @constant_blendvps(<4 x float> %xyzw, <4 x float><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: constant_blendvps<br>
>> +; CHECK-NOT: mov<br>
>> +; CHECK: blendps $7<br>
>> +; CHECK: ret<br>
>> + %1 = select <4 x i1> <i1 false, i1 false, i1 false, i1 true>, <4 x<br>
>> float> %xyzw, <4 x float> %abcd<br>
>> + ret <4 x float> %1<br>
>> +}<br>
>> +<br>
>> +define <16 x i8> @constant_pblendvb(<16 x i8> %xyzw, <16 x i8> %abcd) {<br>
>> +; CHECK-LABEL: constant_pblendvb:<br>
>> +; CHECK: movaps<br>
>> +; CHECK: pblendvb<br>
>> +; CHECK: ret<br>
>> + %1 = select <16 x i1> <i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1 true,<br>
>> i1 true, i1 true, i1 false>, <16 x i8> %xyzw, <16 x i8> %abcd<br>
>> + ret <16 x i8> %1<br>
>> +}<br>
>> +declare <16 x i8> @llvm.x86.sse41.pblendvb(<16 x i8>, <16 x i8>, <16 x<br>
>> i8>)<br>
>> +declare <4 x float> @llvm.x86.sse41.blendvps(<4 x float>, <4 x float>, <4<br>
>> x float>)<br>
>> +declare <2 x double> @llvm.x86.sse41.blendvpd(<2 x double>, <2 x double>,<br>
>> <2 x double>)<br>
>><br>
>> Added: llvm/trunk/test/Transforms/InstCombine/blend_x86.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/blend_x86.ll?rev=209643&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/blend_x86.ll?rev=209643&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Transforms/InstCombine/blend_x86.ll (added)<br>
>> +++ llvm/trunk/test/Transforms/InstCombine/blend_x86.ll Mon May 26<br>
>> 22:42:20 2014<br>
>> @@ -0,0 +1,56 @@<br>
>> +; RUN: opt < %s -instcombine -mtriple=x86_64-apple-macosx -mcpu=core-avx2<br>
>> -S | FileCheck %s<br>
>> +<br>
>> +define <2 x double> @constant_blendvpd(<2 x double> %xy, <2 x double><br>
>> %ab) {<br>
>> +; CHECK-LABEL: @constant_blendvpd<br>
>> +; CHECK: select <2 x i1> <i1 true, i1 false><br>
>> + %1 = tail call <2 x double> @llvm.x86.sse41.blendvpd(<2 x double> %xy,<br>
>> <2 x double> %ab, <2 x double> <double 0xFFFFFFFFE0000000, double<br>
>> 0.000000e+00>)<br>
>> + ret <2 x double> %1<br>
>> +}<br>
>> +<br>
>> +define <4 x float> @constant_blendvps(<4 x float> %xyzw, <4 x float><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: @constant_blendvps<br>
>> +; CHECK: select <4 x i1> <i1 false, i1 false, i1 false, i1 true><br>
>> + %1 = tail call <4 x float> @llvm.x86.sse41.blendvps(<4 x float> %xyzw,<br>
>> <4 x float> %abcd, <4 x float> <float 0.000000e+00, float 0.000000e+00,<br>
>> float 0.000000e+00, float 0xFFFFFFFFE0000000>)<br>
>> + ret <4 x float> %1<br>
>> +}<br>
>> +<br>
>> +define <16 x i8> @constant_pblendvb(<16 x i8> %xyzw, <16 x i8> %abcd) {<br>
>> +; CHECK-LABEL: @constant_pblendvb<br>
>> +; CHECK: select <16 x i1> <i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false><br>
>> + %1 = tail call <16 x i8> @llvm.x86.sse41.pblendvb(<16 x i8> %xyzw, <16<br>
>> x i8> %abcd, <16 x i8> <i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8<br>
>> 0, i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8 0>)<br>
>> + ret <16 x i8> %1<br>
>> +}<br>
>> +<br>
>> +define <4 x double> @constant_blendvpd_avx(<4 x double> %xy, <4 x double><br>
>> %ab) {<br>
>> +; CHECK-LABEL: @constant_blendvpd_avx<br>
>> +; CHECK: select <4 x i1> <i1 true, i1 false, i1 true, i1 false><br>
>> + %1 = tail call <4 x double> @llvm.x86.avx.blendv.pd.256(<4 x double><br>
>> %xy, <4 x double> %ab, <4 x double> <double 0xFFFFFFFFE0000000, double<br>
>> 0.000000e+00, double 0xFFFFFFFFE0000000, double 0.000000e+00>)<br>
>> + ret <4 x double> %1<br>
>> +}<br>
>> +<br>
>> +define <8 x float> @constant_blendvps_avx(<8 x float> %xyzw, <8 x float><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: @constant_blendvps_avx<br>
>> +; CHECK: select <8 x i1> <i1 false, i1 false, i1 false, i1 true, i1<br>
>> false, i1 false, i1 false, i1 true><br>
>> + %1 = tail call <8 x float> @llvm.x86.avx.blendv.ps.256(<8 x float><br>
>> %xyzw, <8 x float> %abcd, <8 x float> <float 0.000000e+00, float<br>
>> 0.000000e+00, float 0.000000e+00, float 0xFFFFFFFFE0000000, float<br>
>> 0.000000e+00, float 0.000000e+00, float 0.000000e+00, float<br>
>> 0xFFFFFFFFE0000000>)<br>
>> + ret <8 x float> %1<br>
>> +}<br>
>> +<br>
>> +define <32 x i8> @constant_pblendvb_avx2(<32 x i8> %xyzw, <32 x i8><br>
>> %abcd) {<br>
>> +; CHECK-LABEL: @constant_pblendvb_avx2<br>
>> +; CHECK: select <32 x i1> <i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false, i1 false, i1 false, i1 true, i1 false, i1<br>
>> true, i1 true, i1 true, i1 false><br>
>> + %1 = tail call <32 x i8> @llvm.x86.avx2.pblendvb(<32 x i8> %xyzw, <32 x<br>
>> i8> %abcd,<br>
>> + <32 x i8> <i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8<br>
>> 0,<br>
>> + i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8<br>
>> 0,<br>
>> + i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8<br>
>> 0,<br>
>> + i8 0, i8 0, i8 255, i8 0, i8 255, i8 255, i8 255, i8<br>
>> 0>)<br>
>> + ret <32 x i8> %1<br>
>> +}<br>
>> +<br>
>> +declare <16 x i8> @llvm.x86.sse41.pblendvb(<16 x i8>, <16 x i8>, <16 x<br>
>> i8>)<br>
>> +declare <4 x float> @llvm.x86.sse41.blendvps(<4 x float>, <4 x float>, <4<br>
>> x float>)<br>
>> +declare <2 x double> @llvm.x86.sse41.blendvpd(<2 x double>, <2 x double>,<br>
>> <2 x double>)<br>
>> +<br>
>> +declare <32 x i8> @llvm.x86.avx2.pblendvb(<32 x i8>, <32 x i8>, <32 x<br>
>> i8>)<br>
>> +declare <8 x float> @llvm.x86.avx.blendv.ps.256(<8 x float>, <8 x float>,<br>
>> <8 x float>)<br>
>> +declare <4 x double> @llvm.x86.avx.blendv.pd.256(<4 x double>, <4 x<br>
>> double>, <4 x double>)<br>
>> +<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">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>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">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>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">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>
</div></div></blockquote></div><br></div>