[AArch64] Mark known zero for high 16-bits of uaddlv intrinsic output with v8i8

[jaykang10]

Summary

If llvm.aarch64.neon.uaddlv intrinsic has v8i8 type input, the it returns 16-bits value.
clang generates llvm.aarch64.neon.uaddlv.i32.v8i8 and trunc to i16 for vaddlv_u8 neon intrinsic. It causes additional and 0xffff instruction from attached example as below.

foo:                                    // @foo
        uaddlv  h0, v0.8b
        fmov    w8, s0
        and     w0, w8, #0xffff
        ret

If we mark know zero for high 16-bits of uaddlv intrinsic output with v8i8, we can avoid the additional and 0xfff.

foo:                                    // @foo
        uaddlv  h0, v0.8b
        fmov    w0, s0
        ret

[dmgreen]

LGTM, thanks.

[efriedma]

Can we also handle v16i8?

Is it worth trying to use a more precise bound, instead of just the approximate power of two? The largest number of bits that can actually be set for a v8i8 is 11 (the number of bits set in 8*255). If we're using a more precise bound like this, we could also handle other forms of uaddlv.

[jaykang10]

Thanks for comments.

Can we also handle v16i8?

Yep, let me add the type.

Is it worth trying to use a more precise bound, instead of just the approximate power of two? The largest number of bits that can actually be set for a v8i8 is 11 (the number of bits set in 8*255). If we're using a more precise bound like this, we could also handle other forms of uaddlv.

Yep, I think it would be good to make more bits zero. Let me update the bound.

[jaykang10]

Following @efriedma's comment, changed the bound to 11 and supported v16i8 type.

[jaykang10]

@efriedma I have updated the patch. Please check it.

[jaykang10]

Fixed wrong bound for v16i8.

[efriedma]

It looks like you uploaded a diff on top of the first patch instead of the whole patch? Please make sure you commit the right thing.

LGTM with that fixed

[jaykang10]

Sorry, I pushed the first patch last week without mentioning this review accidentally...
Let me push this patch on the top of the first one.
Thanks for comments.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

case ISD::INTRINSIC_W_CHAIN: {
     break;
   }
   case ISD::INTRINSIC_WO_CHAIN:
   case ISD::INTRINSIC_VOID: {
     unsigned IntNo = cast<ConstantSDNode>(Op.getOperand(0))->getZExtValue();
     switch (IntNo) {
     default:
       break;
+    case Intrinsic::aarch64_neon_uaddlv: {
+      MVT VT = Op.getOperand(1).getValueType().getSimpleVT();
+      unsigned BitWidth = Known.getBitWidth();
+      if (VT == MVT::v8i8) {
+        assert(BitWidth >= 16 && "Unexpected width!");
+        APInt Mask = APInt::getHighBitsSet(BitWidth, BitWidth - 16);
+        Known.Zero |= Mask;
+      }
+      break;
+    }
     case Intrinsic::aarch64_neon_umaxv:
     case Intrinsic::aarch64_neon_uminv: {
       // Figure out the datatype of the vector operand. The UMINV instruction
       // will zero extend the result, so we can mark as known zero all the
       // bits larger than the element datatype. 32-bit or larget doesn't need
       // this as those are legal types and will be handled by isel directly.
       MVT VT = Op.getOperand(1).getValueType().getSimpleVT();
       unsigned BitWidth = Known.getBitWidth();

llvm/test/CodeGen/AArch64/neon-addlv.ll

 ; CHECK-NEXT:    saddlv s0, v0.4h
 ; CHECK-NEXT:    fmov w0, s0
 ; CHECK-NEXT:    ret
   %tmp1 = load <4 x i16>, ptr %A
   %tmp3 = call <2 x i32> @llvm.aarch64.neon.saddlp.v2i32.v4i16(<4 x i16> %tmp1)
   %tmp5 = call i32 @llvm.vector.reduce.add.v2i32(<2 x i32> %tmp3)
   ret i32 %tmp5
 }
+
+declare i32 @llvm.aarch64.neon.uaddlv.i32.v8i8(<8 x i8>) nounwind readnone
+
+define i32 @uaddlv_known_bits(<8 x i8> %a) {
+; CHECK-LABEL: uaddlv_known_bits:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    uaddlv h0, v0.8b
+; CHECK-NEXT:    fmov w0, s0
+; CHECK-NEXT:    ret
+  %tmp1 = tail call i32 @llvm.aarch64.neon.uaddlv.i32.v8i8(<8 x i8> %a)
+  %tmp2 = and i32 %tmp1, 65535
+  ret i32 %tmp2
+}