[AArch64] Fix bug in store of vector 0 DAGCombine.

[gberry]

Summary

Avoid using XZR/WZR directly as operands to split stores of zero
vectors. Doing so can lead to the XZR/WZR being used by an instruction
that doesn't allow it (e.g. add).

Fixes bug 34674.

[efriedma]

lib/Target/AArch64/AArch64ISelLowering.cpp

9463 ↗

(On Diff #116223)

I think you can use the entry node as the chain operand, rather than the store's chain?

test/CodeGen/AArch64/fastcc.ll

28 ↗

(On Diff #116223)

This is... interesting. But looks correct.

[gberry]

Thanks Eli.

lib/Target/AArch64/AArch64ISelLowering.cpp

9463 ↗

(On Diff #116223)

Right, I was thinking of it as the chain for the split stores, but the copy from xzr can happen whenever.

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

static SDValue replaceZeroVectorStore(SelectionDAG &DAG, StoreSDNode &St) {
   }
 
   for (int I = 0; I < NumVecElts; ++I) {
     SDValue EltVal = StVal.getOperand(I);
     if (!isNullConstant(EltVal) && !isNullFPConstant(EltVal))
       return SDValue();
   }
 
-  // Use WZR/XZR here to prevent DAGCombiner::MergeConsecutiveStores from
-  // undoing this transformation.
-  SDValue SplatVal = VT.getVectorElementType().getSizeInBits() == 32
-                         ? DAG.getRegister(AArch64::WZR, MVT::i32)
-                         : DAG.getRegister(AArch64::XZR, MVT::i64);
+  // Use a CopyFromReg WZR/XZR here to prevent
+  // DAGCombiner::MergeConsecutiveStores from undoing this transformation.
+  SDLoc DL(&St);
+  unsigned ZeroReg;
+  EVT ZeroVT;
+  if (VT.getVectorElementType().getSizeInBits() == 32) {
+    ZeroReg = AArch64::WZR;
+    ZeroVT = MVT::i32;
+  } else {
+    ZeroReg = AArch64::XZR;
+    ZeroVT = MVT::i64;
+  }
+  SDValue SplatVal =
+      DAG.getCopyFromReg(DAG.getEntryNode(), DL, ZeroReg, ZeroVT);
   return splitStoreSplat(DAG, St, SplatVal, NumVecElts);
 }
 
 /// Replace a splat of a scalar to a vector store by scalar stores of the scalar
 /// value. The load store optimizer pass will merge them to store pair stores.
 /// This has better performance than a splat of the scalar followed by a split
 /// vector store. Even if the stores are not merged it is four stores vs a dup,
 /// followed by an ext.b and two stores.

llvm/trunk/test/CodeGen/AArch64/arm64-memset-inline.ll

 ; RUN: llc < %s -mtriple=arm64-eabi | FileCheck %s
 
 define void @t1(i8* nocapture %c) nounwind optsize {
 entry:
 ; CHECK-LABEL: t1:
 ; CHECK: str wzr, [x0, #8]
 ; CHECK: str xzr, [x0]
   call void @llvm.memset.p0i8.i64(i8* %c, i8 0, i64 12, i32 8, i1 false)
   ret void
 }
 
 define void @t2() nounwind ssp {
 entry:
 ; CHECK-LABEL: t2:
+; CHECK: stp xzr, xzr, [sp, #16]
 ; CHECK: strh wzr, [sp, #32]
-; CHECK: stp xzr, xzr, [sp, #8]
-; CHECK: str xzr, [sp, #24]
+; CHECK: str xzr, [sp, #8]
   %buf = alloca [26 x i8], align 1
   %0 = getelementptr inbounds [26 x i8], [26 x i8]* %buf, i32 0, i32 0
   call void @llvm.memset.p0i8.i32(i8* %0, i8 0, i32 26, i32 1, i1 false)
   call void @something(i8* %0) nounwind
   ret void
 }
 
 declare void @something(i8*) nounwind
 declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind
 declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1) nounwind

llvm/trunk/test/CodeGen/AArch64/fastcc.ll

 ; CHECK-TAIL-NEXT: stp x29, x30, [sp, #32]
 ; CHECK-TAIL-NEXT: add x29, sp, #32
 ; CHECK-TAIL: str w{{[0-9]+}}, [sp]
 
 
   call fastcc void @func_stack8([8 x i32] undef, i32 42)
 ; CHECK:  bl func_stack8
 ; CHECK-NOT: sub sp, sp,
+; CHECK-NOT: [sp, #{{[-0-9]+}}]!
+; CHECK-NOT: [sp], #{{[-0-9]+}}
 
 ; CHECK-TAIL: bl func_stack8
-; CHECK-TAIL: sub sp, sp, #16
+; CHECK-TAIL: stp xzr, xzr, [sp, #-16]!
 
 
   call fastcc void @func_stack32([8 x i32] undef, i128 0, i128 9)
 ; CHECK: bl func_stack32
 ; CHECK-NOT: sub sp, sp,
 
 
 ; CHECK-TAIL: bl func_stack32

 ; CHECK-TAIL: stp x29, x30, [sp, #32]
 ; CHECK-TAIL: add x29, sp, #32
 ; CHECK-TAIL: str w{{[0-9]+}}, [sp]
 
 
   call fastcc void @func_stack8([8 x i32] undef, i32 42)
 ; CHECK:  bl func_stack8
 ; CHECK-NOT: sub sp, sp,
+; CHECK-NOT: [sp, #{{[-0-9]+}}]!
+; CHECK-NOT: [sp], #{{[-0-9]+}}
 
 
 ; CHECK-TAIL: bl func_stack8
-; CHECK-TAIL: sub sp, sp, #16
+; CHECK-TAIL: stp xzr, xzr, [sp, #-16]!
 
 
   call fastcc void @func_stack32([8 x i32] undef, i128 0, i128 9)
 ; CHECK: bl func_stack32
 ; CHECK-NOT: sub sp, sp,
 
 
 ; CHECK-TAIL: bl func_stack32

 
 ; CHECK-TAIL-LABEL: func_stack32:
 ; CHECK-TAIL: add x29, sp, #32
 
 
   call fastcc void @func_stack8([8 x i32] undef, i32 42)
 ; CHECK:  bl func_stack8
 ; CHECK-NOT: sub sp, sp,
+; CHECK-NOT: [sp, #{{[-0-9]+}}]!
+; CHECK-NOT: [sp], #{{[-0-9]+}}
 
 ; CHECK-TAIL: bl func_stack8
-; CHECK-TAIL: sub sp, sp, #16
+; CHECK-TAIL: stp xzr, xzr, [sp, #-16]!
 
 
   call fastcc void @func_stack32([8 x i32] undef, i128 0, i128 9)
 ; CHECK: bl func_stack32
 ; CHECK-NOT: sub sp, sp,
 
 
 ; CHECK-TAIL: bl func_stack32

llvm/trunk/test/CodeGen/AArch64/ldst-opt.ll

entry:
   store i64 0, i64* %p5
   %p6 = getelementptr i64, i64* %p, i64 6
   store i64 0, i64* %p6
   %p7 = getelementptr i64, i64* %p, i64 7
   store i64 0, i64* %p7
   ret void
 }
 
-
+; Check for bug 34674 where invalid add of xzr was being generated.
+; CHECK-LABEL: bug34674:
+; CHECK: // %entry
+; CHECK-NEXT: mov [[ZREG:x[0-9]+]], xzr
+; CHECK-DAG: stp [[ZREG]], [[ZREG]], [x0]
+; CHECK-DAG: add x{{[0-9]+}}, [[ZREG]], #1
+define i64 @bug34674(<2 x i64>* %p) {
+entry:
+  store <2 x i64> zeroinitializer, <2 x i64>* %p
+  %p2 = bitcast <2 x i64>* %p to i64*
+  %ld = load i64, i64* %p2
+  %add = add i64 %ld, 1
+  ret i64 %add
+}