[AArch64] Insert/Extract of bitcast patterns

[dmgreen]

Summary

This adds some quick tablegen patterns for vector_insert(bitcast(..)) and bitcast(vector_extract(..)), allowing us to avoid a round-trip through GPRs.

[samtebbs]

Nice change

[alexfh]

Looks like some of our code gets miscompiled on aarch64 after this change. I can't yet share an isolated test, but this is the difference in the assembly:

@@ -109,11 +109,10 @@
        ldr     x10, [sp, #48]
        stp     x8, x9, [sp, #64]
        blr     x10
-       scvtf   d0, w0
        mov     w8, #1
-       fmov    x9, d0
+       scvtf   d1, w0
        dup     v0.2d, x8
-       mov     v0.d[0], x9
+       fmov    d0, d1
 .LBB1_5:
        adrp    x1, .L.str.3
        add     x1, x1, :lo12:.L.str.3

The code misbehaves only on arm with -O2 and higher (works fine on arm with -O1 and on x86-64 with any optimization level) and I don't see any undefined behavior in it.

[dmgreen]

Yeah that certainly looks wrong. The fmov will be clearing the result of the dup. I suspect that INSERT_SUBREG without a IMPLICIT_DEF operand is what is going wrong.

I'll revert the patch.

[dmgreen]

I've only removed the zero-element patterns in rG401481daac90. I believe those were the ones going wrong. Did that fix the issues that you are seeing? If not we can revert the rest whilst we investigate.

[alexfh]

In D134022#3818424, @dmgreen wrote:

I've only removed the zero-element patterns in rG401481daac90. I believe those were the ones going wrong. Did that fix the issues that you are seeing? If not we can revert the rest whilst we investigate.

Thank you Dave! rG401481daac90 fixes this particular problem. I'm now testing it completely to ensure it doesn't break anything else.

[dmgreen]

Thanks for the confirmation. Let us know.

[alexfh]

In D134022#3822808, @dmgreen wrote:

Thanks for the confirmation. Let us know.

No new issues discovered after this patch. Thanks again for the quick resolution!

llvm/lib/Target/AArch64/AArch64InstrInfo.td

def : Pat<(VT64 (vector_insert V64:$src,
                 dsub)>;
 }
 
 defm : Neon_INS_elt_pattern<v8f16, v4f16, f16, INSvi16lane>;
 defm : Neon_INS_elt_pattern<v8bf16, v4bf16, bf16, INSvi16lane>;
 defm : Neon_INS_elt_pattern<v4f32, v2f32, f32, INSvi32lane>;
 defm : Neon_INS_elt_pattern<v2f64, v1f64, f64, INSvi64lane>;
 
+// Insert from bitcast
+// vector_insert(bitcast(f32 src), n, lane) -> INSvi32lane(src, lane, INSERT_SUBREG(-, n), 0)
+def : Pat<(v4i32 (vector_insert v4i32:$src, (i32 (bitconvert (f32 FPR32:$Sn))), imm:$Immd)),
+          (INSvi32lane V128:$src, imm:$Immd, (INSERT_SUBREG (IMPLICIT_DEF), FPR32:$Sn, ssub), 0)>;
+def : Pat<(v4i32 (vector_insert v4i32:$src, (i32 (bitconvert (f32 FPR32:$Sn))), 0)),
+          (INSERT_SUBREG v4i32:$src, FPR32:$Sn, ssub)>;
+def : Pat<(v2i64 (vector_insert v2i64:$src, (i64 (bitconvert (f64 FPR64:$Sn))), imm:$Immd)),
+          (INSvi64lane V128:$src, imm:$Immd, (INSERT_SUBREG (IMPLICIT_DEF), FPR64:$Sn, dsub), 0)>;
+def : Pat<(v2i64 (vector_insert v2i64:$src, (i64 (bitconvert (f64 FPR64:$Sn))), 0)),
+          (INSERT_SUBREG v2i64:$src, FPR64:$Sn, dsub)>;
+
+// bitcast of an extract
+// f32 bitcast(vector_extract(v4i32 src, lane)) -> EXTRACT_SUBREG(INSvi32lane(-, 0, src, lane))
+def : Pat<(f32 (bitconvert (i32 (vector_extract v4i32:$src, imm:$Immd)))),
+          (EXTRACT_SUBREG (INSvi32lane (IMPLICIT_DEF), 0, V128:$src, imm:$Immd), ssub)>;
+def : Pat<(f32 (bitconvert (i32 (vector_extract v4i32:$src, 0)))),
+          (EXTRACT_SUBREG V128:$src, ssub)>;
+def : Pat<(f64 (bitconvert (i64 (vector_extract v2i64:$src, imm:$Immd)))),
+          (EXTRACT_SUBREG (INSvi64lane (IMPLICIT_DEF), 0, V128:$src, imm:$Immd), dsub)>;
+def : Pat<(f64 (bitconvert (i64 (vector_extract v2i64:$src, 0)))),
+          (EXTRACT_SUBREG V128:$src, dsub)>;
 
 // Floating point vector extractions are codegen'd as either a sequence of
 // subregister extractions, or a MOV (aka DUP here) if
 // the lane number is anything other than zero.
 def : Pat<(vector_extract (v2f64 V128:$Rn), 0),
           (f64 (EXTRACT_SUBREG V128:$Rn, dsub))>;
 def : Pat<(vector_extract (v4f32 V128:$Rn), 0),
           (f32 (EXTRACT_SUBREG V128:$Rn, ssub))>;

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

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -verify-machineinstrs -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s
 
 define <4 x i32> @test_vins_v4i32(<4 x i32> %a, float %b) {
 ; CHECK-LABEL: test_vins_v4i32:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov w8, s1
-; CHECK-NEXT:    mov v0.s[3], w8
+; CHECK-NEXT:    // kill: def $s1 killed $s1 def $q1
+; CHECK-NEXT:    mov v0.s[3], v1.s[0]
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast float %b to i32
   %d = insertelement <4 x i32> %a, i32 %c, i32 3
   ret <4 x i32> %d
 }
 
 define <4 x i32> @test_vins_v4i32_0(<4 x i32> %a, float %b) {
 ; CHECK-LABEL: test_vins_v4i32_0:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov w8, s1
-; CHECK-NEXT:    mov v0.s[0], w8
+; CHECK-NEXT:    fmov s0, s1
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast float %b to i32
   %d = insertelement <4 x i32> %a, i32 %c, i32 0
   ret <4 x i32> %d
 }
 
 define <2 x i32> @test_vins_v2i32(<2 x i32> %a, float %b) {
 ; CHECK-LABEL: test_vins_v2i32:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov w8, s1
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 def $q0
-; CHECK-NEXT:    mov v0.s[1], w8
+; CHECK-NEXT:    // kill: def $s1 killed $s1 def $q1
+; CHECK-NEXT:    mov v0.s[1], v1.s[0]
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast float %b to i32
   %d = insertelement <2 x i32> %a, i32 %c, i32 1
   ret <2 x i32> %d
 }
 
 define <2 x i32> @test_vins_v2i32_0(<2 x i32> %a, float %b) {
 ; CHECK-LABEL: test_vins_v2i32_0:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov w8, s1
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 def $q0
-; CHECK-NEXT:    mov v0.s[0], w8
+; CHECK-NEXT:    fmov s0, s1
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast float %b to i32
   %d = insertelement <2 x i32> %a, i32 %c, i32 0
   ret <2 x i32> %d
 }
 
 define <2 x i64> @test_vins_v2i64(<2 x i64> %a, double %b) {
 ; CHECK-LABEL: test_vins_v2i64:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov x8, d1
-; CHECK-NEXT:    mov v0.d[1], x8
+; CHECK-NEXT:    // kill: def $d1 killed $d1 def $q1
+; CHECK-NEXT:    mov v0.d[1], v1.d[0]
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast double %b to i64
   %d = insertelement <2 x i64> %a, i64 %c, i32 1
   ret <2 x i64> %d
 }
 
 define <2 x i64> @test_vins_v2i64_0(<2 x i64> %a, double %b) {
 ; CHECK-LABEL: test_vins_v2i64_0:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    fmov x8, d1
-; CHECK-NEXT:    mov v0.d[0], x8
+; CHECK-NEXT:    fmov d0, d1
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast double %b to i64
   %d = insertelement <2 x i64> %a, i64 %c, i32 0
   ret <2 x i64> %d
 }
 
 define <1 x i64> @test_vins_v1i64(<1 x i64> %a, double %b) {
 ; CHECK-LABEL: test_vins_v1i64:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    fmov d0, d1
 ; CHECK-NEXT:    ret
 entry:
   %c = bitcast double %b to i64
   %d = insertelement <1 x i64> %a, i64 %c, i32 0
   ret <1 x i64> %d
 }
 
 
 define float @test_vext_v4i32(<4 x i32> %a) {
 ; CHECK-LABEL: test_vext_v4i32:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    mov w8, v0.s[3]
-; CHECK-NEXT:    fmov s0, w8
+; CHECK-NEXT:    mov v0.s[0], v0.s[3]
+; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %b = extractelement <4 x i32> %a, i32 3
   %c = bitcast i32 %b to float
   ret float %c
 }
 
 define float @test_vext_v4i32_0(<4 x i32> %a) {
 ; CHECK-LABEL: test_vext_v4i32_0:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %b = extractelement <4 x i32> %a, i32 0
   %c = bitcast i32 %b to float
   ret float %c
 }
 
 define float @test_vext_v2i32(<2 x i32> %a) {
 ; CHECK-LABEL: test_vext_v2i32:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 def $q0
-; CHECK-NEXT:    mov w8, v0.s[1]
-; CHECK-NEXT:    fmov s0, w8
+; CHECK-NEXT:    mov v0.s[0], v0.s[1]
+; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %b = extractelement <2 x i32> %a, i32 1
   %c = bitcast i32 %b to float
   ret float %c
 }
 
 define float @test_vext_v2i32_0(<2 x i32> %a) {
 ; CHECK-LABEL: test_vext_v2i32_0:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    // kill: def $d0 killed $d0 def $q0
 ; CHECK-NEXT:    // kill: def $s0 killed $s0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %b = extractelement <2 x i32> %a, i32 0
   %c = bitcast i32 %b to float
   ret float %c
 }
 
 define double @test_vext_v2i64(<2 x i64> %a) {
 ; CHECK-LABEL: test_vext_v2i64:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    mov x8, v0.d[1]
-; CHECK-NEXT:    fmov d0, x8
+; CHECK-NEXT:    mov v0.d[0], v0.d[1]
+; CHECK-NEXT:    // kill: def $d0 killed $d0 killed $q0
 ; CHECK-NEXT:    ret
 entry:
   %b = extractelement <2 x i64> %a, i32 1
   %c = bitcast i64 %b to double
   ret double %c
 }
 
 define double @test_vext_v2i64_0(<2 x i64> %a) {