Hi Duncan,<div><br></div><div>Looks like Eli and Chandler beat me to it. Short answer: if we generated an ext + mul, the ext could be hoisted, preventing our BB-at-a-time isel from forming the neon instr. This patch only expands where it's safe.</div>
<div><br></div><div>Cheers,</div><div>Lang.<br><br><div class="gmail_quote">On Tue, May 1, 2012 at 1:28 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.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="gmail_quote"><div class="im">On Tue, May 1, 2012 at 1:07 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank">baldrick@free.fr</a>></span> wrote:<br>
</div><div class="im"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Lang,<br>
<div><br>
On 01/05/12 02:20, Lang Hames wrote:<br>
> Author: lhames<br>
> Date: Mon Apr 30 19:20:38 2012<br>
> New Revision: 155866<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=155866&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=155866&view=rev</a><br>
> Log:<br>
> Add support for llvm.arm.neon.vmull* intrinsics to InstCombine. Fixes<br>
> <rdar://problem/11291436>.<br>
><br>
> This is a second attempt at a fix for this, the first was r155468. Thanks<br>
> to Chandler, Bob and others for the feedback that helped me improve this.<br>
<br>
</div>are these intrinsics actually useful?  Couldn't you get the same effect using<br>
generic IR, eg: first extend <4 x i16> to <4 x i32> then do the multiplication<br>
in <4 x i32>; and have the ARM code generators recognize this pattern and create<br>
the appropriate vmull instruction?<br></blockquote><div><br></div></div><div>See the lengthy thread about this.</div><div><br></div><div>Short summary: it *should* work, and it's theoretically cleaner, but it doesn't today. Maybe in the future it will, and we can switch.</div>

<div><br></div><div>The problem is that the DAG and isel machinery runs BB-at-a-time, and the *ext instructions can get hoisted out of a basic block. Once that happens, the code generator can't match this back down to the widening instructions.</div>

<div><br></div><div>The end-game solution is whole-function isel. We get that, this problem vanishes.</div><div><br></div><div>I suggested one compromise of adding target-specific hooks to the DAG builder so that it could look across the BBs of the IR, and directly form the target-specific DAG node for the widening mul, but it doesn't seem likely to be worth doing just for this intrinsic....</div>
<div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Ciao, Duncan.<br>
<div><br>
><br>
> Added:<br>
>      llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll<br>
> Modified:<br>
>      llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
><br>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=155866&r1=155865&r2=155866&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=155866&r1=155865&r2=155866&view=diff</a><br>


> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Mon Apr 30 19:20:38 2012<br>
> @@ -694,6 +694,57 @@<br>
>       break;<br>
>     }<br>
><br>
> +  case Intrinsic::arm_neon_vmulls:<br>
> +  case Intrinsic::arm_neon_vmullu: {<br>
> +    Value *Arg0 = II->getArgOperand(0);<br>
> +    Value *Arg1 = II->getArgOperand(1);<br>
> +<br>
> +    // Handle mul by zero first:<br>
> +    if (isa<ConstantAggregateZero>(Arg0) || isa<ConstantAggregateZero>(Arg1)) {<br>
> +      return ReplaceInstUsesWith(CI, ConstantAggregateZero::get(II->getType()));<br>
> +    }<br>
> +<br>
</div>> +    // Check for constant LHS&  RHS - in this case we just simplify.<br>
<div><div>> +    bool Zext = (II->getIntrinsicID() == Intrinsic::arm_neon_vmullu);<br>
> +    VectorType *NewVT = cast<VectorType>(II->getType());<br>
> +    unsigned NewWidth = NewVT->getElementType()->getIntegerBitWidth();<br>
> +    if (ConstantDataVector *CV0 = dyn_cast<ConstantDataVector>(Arg0)) {<br>
> +      if (ConstantDataVector *CV1 = dyn_cast<ConstantDataVector>(Arg1)) {<br>
> +        VectorType* VT = cast<VectorType>(CV0->getType());<br>
> +        SmallVector<Constant*, 4>  NewElems;<br>
> +        for (unsigned i = 0; i<  VT->getNumElements(); ++i) {<br>
> +          APInt CV0E =<br>
> +            (cast<ConstantInt>(CV0->getAggregateElement(i)))->getValue();<br>
> +          CV0E = Zext ? CV0E.zext(NewWidth) : CV0E.sext(NewWidth);<br>
> +          APInt CV1E =<br>
> +            (cast<ConstantInt>(CV1->getAggregateElement(i)))->getValue();<br>
> +          CV1E = Zext ? CV1E.zext(NewWidth) : CV1E.sext(NewWidth);<br>
> +          NewElems.push_back(<br>
> +            ConstantInt::get(NewVT->getElementType(), CV0E * CV1E));<br>
> +        }<br>
> +        return ReplaceInstUsesWith(CI, ConstantVector::get(NewElems));<br>
> +      }<br>
> +<br>
> +      // Couldn't simplify - cannonicalize constant to the RHS.<br>
> +      std::swap(Arg0, Arg1);<br>
> +    }<br>
> +<br>
> +    // Handle mul by one:<br>
> +    if (ConstantDataVector *CV1 = dyn_cast<ConstantDataVector>(Arg1)) {<br>
> +      if (ConstantInt *Splat =<br>
> +            dyn_cast_or_null<ConstantInt>(CV1->getSplatValue())) {<br>
> +        if (Splat->isOne()) {<br>
> +          if (Zext)<br>
> +            return CastInst::CreateZExtOrBitCast(Arg0, II->getType());<br>
> +          // else<br>
> +          return CastInst::CreateSExtOrBitCast(Arg0, II->getType());<br>
> +        }<br>
> +      }<br>
> +    }<br>
> +<br>
> +    break;<br>
> +  }<br>
> +<br>
>     case Intrinsic::stackrestore: {<br>
>       // If the save is right next to the restore, remove the restore.  This can<br>
>       // happen when variable allocas are DCE'd.<br>
><br>
> Added: llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll?rev=155866&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll?rev=155866&view=auto</a><br>


> ==============================================================================<br>
> --- llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll (added)<br>
> +++ llvm/trunk/test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll Mon Apr 30 19:20:38 2012<br>
> @@ -0,0 +1,68 @@<br>
> +target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32-S32"<br>
> +target triple = "thumbv7-apple-ios0"<br>
> +<br>
> +; RUN: opt -S -instcombine<  %s | FileCheck %s<br>
> +<br>
> +define<4 x i32>  @mulByZero(<4 x i16>  %x) nounwind readnone ssp {<br>
> +entry:<br>
> +  %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  %x,<4 x i16>  zeroinitializer) nounwind<br>
> +  ret<4 x i32>  %a<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: ret<4 x i32>  zeroinitializer<br>
> +}<br>
> +<br>
> +define<4 x i32>  @mulByOne(<4 x i16>  %x) nounwind readnone ssp {<br>
> +entry:<br>
> +  %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  %x,<4 x i16>  <i16 1, i16 1, i16 1, i16 1>) nounwind<br>
> +  ret<4 x i32>  %a<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: %a = sext<4 x i16>  %x to<4 x i32><br>
> +; CHECK-NEXT: ret<4 x i32>  %a<br>
> +}<br>
> +<br>
> +define<4 x i32>  @constantMul() nounwind readnone ssp {<br>
> +entry:<br>
> +  %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  <i16 3, i16 3, i16 3, i16 3>,<4 x i16>  <i16 2, i16 2, i16 2, i16 2>) nounwind<br>
> +  ret<4 x i32>  %a<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: ret<4 x i32>  <i32 6, i32 6, i32 6, i32 6><br>
> +}<br>
> +<br>
> +define<4 x i32>  @constantMulS() nounwind readnone ssp {<br>
> +entry:<br>
> +  %b = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  <i16 -1, i16 -1, i16 -1, i16 -1>,<4 x i16>  <i16 1, i16 1, i16 1, i16 1>) nounwind<br>
> +  ret<4 x i32>  %b<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: ret<4 x i32>  <i32 -1, i32 -1, i32 -1, i32 -1><br>
> +}<br>
> +<br>
> +define<4 x i32>  @constantMulU() nounwind readnone ssp {<br>
> +entry:<br>
> +  %b = tail call<4 x i32>  @llvm.arm.neon.vmullu.v4i32(<4 x i16>  <i16 -1, i16 -1, i16 -1, i16 -1>,<4 x i16>  <i16 1, i16 1, i16 1, i16 1>) nounwind<br>
> +  ret<4 x i32>  %b<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: ret<4 x i32>  <i32 65535, i32 65535, i32 65535, i32 65535><br>
> +}<br>
> +<br>
> +define<4 x i32>  @complex1(<4 x i16>  %x) nounwind readnone ssp {<br>
> +entry:<br>
> +  %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  <i16 2, i16 2, i16 2, i16 2>,<4 x i16>  %x) nounwind<br>
> +  %b = add<4 x i32>  zeroinitializer, %a<br>
> +  ret<4 x i32>  %b<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  <i16 2, i16 2, i16 2, i16 2>,<4 x i16>  %x) nounwind<br>
> +; CHECK-NEXT: ret<4 x i32>  %a<br>
> +}<br>
> +<br>
> +define<4 x i32>  @complex2(<4 x i32>  %x) nounwind readnone ssp {<br>
> +entry:<br>
> +  %a = tail call<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>  <i16 3, i16 3, i16 3, i16 3>,<4 x i16>  <i16 2, i16 2, i16 2, i16 2>) nounwind<br>
> +  %b = add<4 x i32>  %x, %a<br>
> +  ret<4 x i32>  %b<br>
> +; CHECK: entry:<br>
> +; CHECK-NEXT: %b = add<4 x i32>  %x,<i32 6, i32 6, i32 6, i32 6><br>
> +; CHECK-NEXT: ret<4 x i32>  %b<br>
> +}<br>
> +<br>
> +declare<4 x i32>  @llvm.arm.neon.vmulls.v4i32(<4 x i16>,<4 x i16>) nounwind readnone<br>
> +declare<4 x i32>  @llvm.arm.neon.vmullu.v4i32(<4 x i16>,<4 x i16>) nounwind readnone<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>
_______________________________________________<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>
</div></div></blockquote></div></div></div><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></blockquote></div><br></div>