[AArch64] Lower bitreverse in ISel

[Rin]

Summary

Adding lowering support for bitreverse.

Previously, lowering bitreverse would expand it into a series of other instructions. This patch makes it so this produces a single rbit instruction instead.

[dmgreen]

Sounds good.

llvm/lib/Target/AArch64/AArch64InstrInfo.td

4134

We can remove this. And Can we remove the definition on int_aarch64_neon_rbit now too?

llvm/test/CodeGen/AArch64/bitreverse.ll

33

Can we add tests for all these types:
v8i8 v16i8
v4i16 v8i16
v2i32 v4i32
v1i64 v2i64
Then use update_llc_test_checks to generate the check lines. Also i32 and i64 would be good to have in here.

Bonus points for pre-committing with the old codegen, to just show changes here.

[Rin]

Remove unnecessary comment and add more bitreverse tests

[dmgreen]

llvm/lib/IR/AutoUpgrade.cpp

556

Do we have a test for the autoupgrade, to show that @llvm.aarch64.neon.rbit turns into @llvm.bitreverse?

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

381

Can we move this to the other "hasNeon" section?

1019

Perhaps here. This looks quite neon.

[Rin]

Add AutoUpgrade test and move bitreverse lowering

[dmgreen]

Thanks. LGTM

llvm/test/CodeGen/AArch64/neon_rbit.ll

9 ↗

(On Diff #345465)

v8i8 and v8i16 are fine for this patch. They others were not valid aarch64.neon.rbit intrinsics anyway.

Also, it may be better to make this an opt test, I'm not sure. It's probably fine either way.

clang/test/CodeGen/aarch64-neon-misc.c

 // CHECK-LABEL: @test_vmvnq_p8(
 // CHECK:   [[NEG_I:%.*]] = xor <16 x i8> %a, <i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1, i8 -1>
 // CHECK:   ret <16 x i8> [[NEG_I]]
 poly8x16_t test_vmvnq_p8(poly8x16_t a) {
   return vmvnq_p8(a);
 }
 
 // CHECK-LABEL: @test_vrbit_s8(
-// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.rbit.v8i8(<8 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.bitreverse.v8i8(<8 x i8> %a)
 // CHECK:   ret <8 x i8> [[VRBIT_I]]
 int8x8_t test_vrbit_s8(int8x8_t a) {
   return vrbit_s8(a);
 }
 
 // CHECK-LABEL: @test_vrbitq_s8(
-// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.aarch64.neon.rbit.v16i8(<16 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.bitreverse.v16i8(<16 x i8> %a)
 // CHECK:   ret <16 x i8> [[VRBIT_I]]
 int8x16_t test_vrbitq_s8(int8x16_t a) {
   return vrbitq_s8(a);
 }
 
 // CHECK-LABEL: @test_vrbit_u8(
-// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.rbit.v8i8(<8 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.bitreverse.v8i8(<8 x i8> %a)
 // CHECK:   ret <8 x i8> [[VRBIT_I]]
 uint8x8_t test_vrbit_u8(uint8x8_t a) {
   return vrbit_u8(a);
 }
 
 // CHECK-LABEL: @test_vrbitq_u8(
-// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.aarch64.neon.rbit.v16i8(<16 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.bitreverse.v16i8(<16 x i8> %a)
 // CHECK:   ret <16 x i8> [[VRBIT_I]]
 uint8x16_t test_vrbitq_u8(uint8x16_t a) {
   return vrbitq_u8(a);
 }
 
 // CHECK-LABEL: @test_vrbit_p8(
-// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.aarch64.neon.rbit.v8i8(<8 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <8 x i8> @llvm.bitreverse.v8i8(<8 x i8> %a)
 // CHECK:   ret <8 x i8> [[VRBIT_I]]
 poly8x8_t test_vrbit_p8(poly8x8_t a) {
   return vrbit_p8(a);
 }
 
 // CHECK-LABEL: @test_vrbitq_p8(
-// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.aarch64.neon.rbit.v16i8(<16 x i8> %a)
+// CHECK:   [[VRBIT_I:%.*]] = call <16 x i8> @llvm.bitreverse.v16i8(<16 x i8> %a)
 // CHECK:   ret <16 x i8> [[VRBIT_I]]
 poly8x16_t test_vrbitq_p8(poly8x16_t a) {
   return vrbitq_p8(a);
 }
 
 // CHECK-LABEL: @test_vmovn_s16(
 // CHECK:   [[TMP0:%.*]] = bitcast <8 x i16> %a to <16 x i8>
 // CHECK:   [[VMOVN_I:%.*]] = trunc <8 x i16> %a to <8 x i8>

llvm/lib/IR/AutoUpgrade.cpp

if (Name.startswith("arm.rbit") || Name.startswith("aarch64.rbit")) {
                                         F->arg_begin()->getType());
       return true;
     }
     if (Name.startswith("aarch64.neon.frintn")) {
       NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::roundeven,
                                         F->arg_begin()->getType());
       return true;
     }
+    if (Name.startswith("aarch64.neon.rbit")) {
+      NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::bitreverse,
+                                        F->arg_begin()->getType());
+      return true;
+    }
     if (Name.startswith("arm.neon.vclz")) {
       Type* args[2] = {
         F->arg_begin()->getType(),
         Type::getInt1Ty(F->getContext())
       };
       // Can't use Intrinsic::getDeclaration here as it adds a ".i1" to
       // the end of the name. Change name from llvm.arm.neon.vclz.* to
       //  llvm.ctlz.*

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM,
   setOperationAction(ISD::STRICT_FSETCC, MVT::f16, Custom);
   setOperationAction(ISD::STRICT_FSETCC, MVT::f32, Custom);
   setOperationAction(ISD::STRICT_FSETCC, MVT::f64, Custom);
   setOperationAction(ISD::STRICT_FSETCCS, MVT::f16, Custom);
   setOperationAction(ISD::STRICT_FSETCCS, MVT::f32, Custom);
   setOperationAction(ISD::STRICT_FSETCCS, MVT::f64, Custom);
   setOperationAction(ISD::BITREVERSE, MVT::i32, Legal);
   setOperationAction(ISD::BITREVERSE, MVT::i64, Legal);
+  setOperationAction(ISD::BITREVERSE, MVT::v8i8, Legal);
+  setOperationAction(ISD::BITREVERSE, MVT::v16i8, Legal);
   setOperationAction(ISD::BRCOND, MVT::Other, Expand);
   setOperationAction(ISD::BR_CC, MVT::i32, Custom);
   setOperationAction(ISD::BR_CC, MVT::i64, Custom);
   setOperationAction(ISD::BR_CC, MVT::f16, Custom);
   setOperationAction(ISD::BR_CC, MVT::f32, Custom);
   setOperationAction(ISD::BR_CC, MVT::f64, Custom);
   setOperationAction(ISD::SELECT, MVT::i32, Custom);
   setOperationAction(ISD::SELECT, MVT::i64, Custom);
if (Subtarget->hasFullFP16()) {
       // to i32 first.
       setOperationPromotedToType(ISD::UINT_TO_FP, MVT::v4i16, MVT::v4i32);
       setOperationPromotedToType(ISD::SINT_TO_FP, MVT::v4i16, MVT::v4i32);
       setOperationPromotedToType(ISD::SINT_TO_FP, MVT::v8i16, MVT::v8i32);
       setOperationPromotedToType(ISD::UINT_TO_FP, MVT::v8i16, MVT::v8i32);
     }
 
     setOperationAction(ISD::CTLZ,       MVT::v1i64, Expand);
     setOperationAction(ISD::CTLZ,       MVT::v2i64, Expand);
 
     // AArch64 doesn't have MUL.2d:
     setOperationAction(ISD::MUL, MVT::v2i64, Expand);
     // Custom handling for some quad-vector types to detect MULL.
     setOperationAction(ISD::MUL, MVT::v8i16, Custom);
     setOperationAction(ISD::MUL, MVT::v4i32, Custom);
     setOperationAction(ISD::MUL, MVT::v2i64, Custom);
 

llvm/lib/Target/AArch64/AArch64InstrInfo.td

 
 def : Pat<(vnot (v4i16 V64:$Rn)),  (NOTv8i8  V64:$Rn)>;
 def : Pat<(vnot (v8i16 V128:$Rn)), (NOTv16i8 V128:$Rn)>;
 def : Pat<(vnot (v2i32 V64:$Rn)),  (NOTv8i8  V64:$Rn)>;
 def : Pat<(vnot (v4i32 V128:$Rn)), (NOTv16i8 V128:$Rn)>;
 def : Pat<(vnot (v1i64 V64:$Rn)),  (NOTv8i8  V64:$Rn)>;
 def : Pat<(vnot (v2i64 V128:$Rn)), (NOTv16i8 V128:$Rn)>;
 
-defm RBIT   : SIMDTwoVectorB<1, 0b01, 0b00101, "rbit", int_aarch64_neon_rbit>;
+//defm RBIT   : SIMDTwoVectorB<1, 0b01, 0b00101, "rbit", int_aarch64_neon_rbit>;
+defm RBIT   : SIMDTwoVectorB<1, 0b01, 0b00101, "rbit", bitreverse>;
 defm REV16  : SIMDTwoVectorB<0, 0b00, 0b00001, "rev16", AArch64rev16>;
 defm REV32  : SIMDTwoVectorBH<1, 0b00000, "rev32", AArch64rev32>;
 defm REV64  : SIMDTwoVectorBHS<0, 0b00000, "rev64", AArch64rev64>;
 defm SADALP : SIMDLongTwoVectorTied<0, 0b00110, "sadalp",
        BinOpFrag<(add node:$LHS, (int_aarch64_neon_saddlp node:$RHS))> >;
 defm SADDLP : SIMDLongTwoVector<0, 0b00010, "saddlp", int_aarch64_neon_saddlp>;
 defm SCVTF  : SIMDTwoVectorIntToFP<0, 0, 0b11101, "scvtf", sint_to_fp>;
 defm SHLL   : SIMDVectorLShiftLongBySizeBHS;

llvm/test/CodeGen/AArch64/arm64-vbitwise.ll

 ; RUN: llc < %s -mtriple=arm64-eabi -aarch64-neon-syntax=apple | FileCheck %s
 
 define <8 x i8> @rbit_8b(<8 x i8>* %A) nounwind {
 ;CHECK-LABEL: rbit_8b:
 ;CHECK: rbit.8b
   %tmp1 = load <8 x i8>, <8 x i8>* %A
-  %tmp3 = call <8 x i8> @llvm.aarch64.neon.rbit.v8i8(<8 x i8> %tmp1)
+  %tmp3 = call <8 x i8> @llvm.bitreverse.v8i8(<8 x i8> %tmp1)
   ret <8 x i8> %tmp3
 }
 
 define <16 x i8> @rbit_16b(<16 x i8>* %A) nounwind {
 ;CHECK-LABEL: rbit_16b:
 ;CHECK: rbit.16b
   %tmp1 = load <16 x i8>, <16 x i8>* %A
-  %tmp3 = call <16 x i8> @llvm.aarch64.neon.rbit.v16i8(<16 x i8> %tmp1)
+  %tmp3 = call <16 x i8> @llvm.bitreverse.v16i8(<16 x i8> %tmp1)
   ret <16 x i8> %tmp3
 }
 
-declare <8 x i8> @llvm.aarch64.neon.rbit.v8i8(<8 x i8>) nounwind readnone
-declare <16 x i8> @llvm.aarch64.neon.rbit.v16i8(<16 x i8>) nounwind readnone
+declare <8 x i8> @llvm.bitreverse.v8i8(<8 x i8>) nounwind readnone
+declare <16 x i8> @llvm.bitreverse.v16i8(<16 x i8>) nounwind readnone
 
 define <8 x i16> @sxtl8h(<8 x i8>* %A) nounwind {
 ;CHECK-LABEL: sxtl8h:
 ;CHECK: sshll.8h
   %tmp1 = load <8 x i8>, <8 x i8>* %A
   %tmp2 = sext <8 x i8> %tmp1 to <8 x i16>
   ret <8 x i16> %tmp2
 }

llvm/test/CodeGen/AArch64/bitreverse.ll

 ; CHECK-NEXT: lsr w0, [[REG]], #24
 ; CHECK-NEXT: ret
   %b = call i8 @llvm.bitreverse.i8(i8 %a)
   ret i8 %b
 }
 
 declare <8 x i8> @llvm.bitreverse.v8i8(<8 x i8>) readnone
 
 define <8 x i8> @g_vec(<8 x i8> %a) {
-; CHECK-DAG: movi [[M1:v.*]], #15
-; CHECK-DAG: movi [[M2:v.*]], #240
-; CHECK:     and  [[A1:v.*]], v0.8b, [[M1]]
-; CHECK:     and  [[A2:v.*]], v0.8b, [[M2]]
-; CHECK-DAG: shl  [[L4:v.*]], [[A1]], #4
-; CHECK-DAG: ushr [[R4:v.*]], [[A2]], #4
-; CHECK-DAG: orr  [[V4:v.*]], [[R4]], [[L4]]
-
-; CHECK-DAG: movi [[M3:v.*]], #51
-; CHECK-DAG: movi [[M4:v.*]], #204
-; CHECK:     and  [[A3:v.*]], [[V4]], [[M3]]
-; CHECK:     and  [[A4:v.*]], [[V4]], [[M4]]
-; CHECK-DAG: shl  [[L2:v.*]], [[A3]], #2
-; CHECK-DAG: ushr [[R2:v.*]], [[A4]], #2
-; CHECK-DAG: orr  [[V2:v.*]], [[R2]], [[L2]]
-
-; CHECK-DAG: movi [[M5:v.*]], #85
-; CHECK-DAG: movi [[M6:v.*]], #170
-; CHECK:     and  [[A5:v.*]], [[V2]], [[M5]]
-; CHECK:     and  [[A6:v.*]], [[V2]], [[M6]]
-; CHECK-DAG: shl  [[L1:v.*]], [[A5]], #1
-; CHECK-DAG: ushr [[R1:v.*]], [[A6]], #1
-; CHECK:     orr  [[V1:v.*]], [[R1]], [[L1]]
-
+;CHECK-LABEL: g_vec:
+;rbit v0.8b, v0.8b
 ; CHECK:     ret
   %b = call <8 x i8> @llvm.bitreverse.v8i8(<8 x i8> %a)
   ret <8 x i8> %b
 }