[AVR] Do not chain stores in call frame setup

[aykevl]

Summary

Previously, AVRTargetLowering::LowerCall attempted to keep stack stores in order with chains. Perhaps this worked in the past, but it does not work now: it appears that the SelectionDAG legalization phase removes these chains. Therefore, I've removed these chains entirely to match X86 (which, similar to AVR, also prefers to use push instructions over stack-relative stores to set up a call frame). With this change, all the stack stores are in a somewhat reasonable order.

See also: https://reviews.llvm.org/D78581.

---

I ran my batch of compiler-rt tests locally and they still pass with this patch.
This patch is really a preparation for a different patch that attempts to optimize/fix the bug I originally tried to fix in D78581 but didn't do correctly.

llvm/lib/Target/AVR/AVRISelLowering.cpp

if (VA.isMemLoc()) {
       break;
     }
 
     // Arguments that can be passed on registers must be kept in the RegsToPass
     // vector.
     RegsToPass.push_back(std::make_pair(VA.getLocReg(), Arg));
   }
 
-  // Second, stack arguments have to walked in reverse order by inserting
-  // chained stores, this ensures their order is not changed by the scheduler
-  // and that the push instruction sequence generated is correct, otherwise they
-  // can be freely intermixed.
+  // Second, stack arguments have to walked.
+  // Previously this code created chained stores but those chained stores appear
+  // to be unchained in the legalization phase. Therefore, do not attempt to
+  // chain them here. In fact, chaining them here somehow causes the first and
+  // second store to be reversed which is the exact opposite of the intended
+  // effect.
   if (HasStackArgs) {
-    for (AE = AI, AI = ArgLocs.size(); AI != AE; --AI) {
-      unsigned Loc = AI - 1;
-      CCValAssign &VA = ArgLocs[Loc];
-      SDValue Arg = OutVals[Loc];
+    SmallVector<SDValue, 8> MemOpChains;
+    for (; AI != AE; AI++) {
+      CCValAssign &VA = ArgLocs[AI];
+      SDValue Arg = OutVals[AI];
 
       assert(VA.isMemLoc());
 
       // SP points to one stack slot further so add one to adjust it.
       SDValue PtrOff = DAG.getNode(
           ISD::ADD, DL, getPointerTy(DAG.getDataLayout()),
           DAG.getRegister(AVR::SP, getPointerTy(DAG.getDataLayout())),
           DAG.getIntPtrConstant(VA.getLocMemOffset() + 1, DL));
 
-      Chain =
+      MemOpChains.push_back(
           DAG.getStore(Chain, DL, Arg, PtrOff,
-                       MachinePointerInfo::getStack(MF, VA.getLocMemOffset()));
+                       MachinePointerInfo::getStack(MF, VA.getLocMemOffset())));
     }
+
+    if (!MemOpChains.empty())
+      Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, MemOpChains);
   }
 
   // Build a sequence of copy-to-reg nodes chained together with token chain and
   // flag operands which copy the outgoing args into registers.  The InFlag in
   // necessary since all emited instructions must be stuck together.
   SDValue InFlag;
   for (auto Reg : RegsToPass) {
     Chain = DAG.getCopyToReg(Chain, DL, Reg.first, Reg.second, InFlag);

llvm/test/CodeGen/AVR/call.ll

 ; CHECK: ldi r23, 2
 ; CHECK: call foo16_1
     %result1 = call i16 @foo16_1(i16 513, i16 514)
     ret i16 %result1
 }
 
 define i16 @calli16_stack() {
 ; CHECK-LABEL: calli16_stack:
-; CHECK: ldi [[REG1:r[0-9]+]], 9
-; CHECK: ldi [[REG2:r[0-9]+]], 2 
-; CHECK: std Z+1, [[REG1]]
-; CHECK: std Z+2, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 10
 ; CHECK: ldi [[REG2:r[0-9]+]], 2
 ; CHECK: std Z+3, [[REG1]]
 ; CHECK: std Z+4, [[REG2]]
+; CHECK: ldi [[REG1:r[0-9]+]], 9
+; CHECK: ldi [[REG2:r[0-9]+]], 2
+; CHECK: std Z+1, [[REG1]]
+; CHECK: std Z+2, [[REG2]]
 ; CHECK: call foo16_2
     %result1 = call i16 @foo16_2(i16 512, i16 513, i16 514, i16 515, i16 516, i16 517, i16 518, i16 519, i16 520, i16 521, i16 522)
     ret i16 %result1
 }
 
 define i32 @calli32_reg() {
 ; CHECK-LABEL: calli32_reg:
 ; CHECK: ldi r22, 64
 ; CHECK: ldi r23, 66
 ; CHECK: ldi r24, 15
 ; CHECK: ldi r25, 2
 ; CHECK: ldi r18, 128
 ; CHECK: ldi r19, 132
 ; CHECK: ldi r20, 30
 ; CHECK: ldi r21, 2
 ; CHECK: call foo32_1
     %result1 = call i32 @foo32_1(i32 34554432, i32 35554432)
     ret i32 %result1
 }
 
 define i32 @calli32_stack() {
 ; CHECK-LABEL: calli32_stack:
-; CHECK: ldi [[REG1:r[0-9]+]], 64
-; CHECK: ldi [[REG2:r[0-9]+]], 66
-; CHECK: std Z+1, [[REG1]]
-; CHECK: std Z+2, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 15
 ; CHECK: ldi [[REG2:r[0-9]+]], 2
 ; CHECK: std Z+3, [[REG1]]
 ; CHECK: std Z+4, [[REG2]]
+; CHECK: ldi [[REG1:r[0-9]+]], 64
+; CHECK: ldi [[REG2:r[0-9]+]], 66
+; CHECK: std Z+1, [[REG1]]
+; CHECK: std Z+2, [[REG2]]
 ; CHECK: call foo32_2
     %result1 = call i32 @foo32_2(i32 1, i32 2, i32 3, i32 4, i32 34554432)
     ret i32 %result1
 }
 
 define i64 @calli64_reg() {
 ; CHECK-LABEL: calli64_reg:
 ; CHECK: ldi r18, 255
 ; CHECK: ldi r19, 255
 ; CHECK: ldi r20, 155
 ; CHECK: ldi r21, 88
 ; CHECK: ldi r22, 76
 ; CHECK: ldi r23, 73
 ; CHECK: ldi r24, 31
 ; CHECK: ldi r25, 242
 ; CHECK: call foo64_1
     %result1 = call i64 @foo64_1(i64 17446744073709551615)
     ret i64 %result1
 }
 
 define i64 @calli64_stack() {
 ; CHECK-LABEL: calli64_stack:
 
-; CHECK: ldi [[REG1:r[0-9]+]], 76
-; CHECK: ldi [[REG2:r[0-9]+]], 73
-; CHECK: std Z+5, [[REG1]]
-; CHECK: std Z+6, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 31
 ; CHECK: ldi [[REG2:r[0-9]+]], 242
 ; CHECK: std Z+7, [[REG1]]
 ; CHECK: std Z+8, [[REG2]]
+; CHECK: ldi [[REG1:r[0-9]+]], 76
+; CHECK: ldi [[REG2:r[0-9]+]], 73
+; CHECK: std Z+5, [[REG1]]
+; CHECK: std Z+6, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 155
 ; CHECK: ldi [[REG2:r[0-9]+]], 88
 ; CHECK: std Z+3, [[REG1]]
 ; CHECK: std Z+4, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 255
 ; CHECK: ldi [[REG2:r[0-9]+]], 255
 ; CHECK: std Z+1, [[REG1]]
 ; CHECK: std Z+2, [[REG2]]

llvm/test/CodeGen/AVR/dynalloca.ll

 ; CHECK: in r0, 63
 ; CHECK-NEXT: cli
 ; CHECK-NEXT: out 62, {{.*}}
 ; CHECK-NEXT: out 63, r0
 ; CHECK-NEXT: out 61, {{.*}}
 ; Store values on the stack
 ; CHECK: ldi r16, 0
 ; CHECK: ldi r17, 0
-; CHECK: std Z+5, r16
-; CHECK: std Z+6, r17
 ; CHECK: std Z+7, r16
 ; CHECK: std Z+8, r17
+; CHECK: std Z+5, r16
+; CHECK: std Z+6, r17
 ; CHECK: std Z+3, r16
 ; CHECK: std Z+4, r17
 ; CHECK: std Z+1, r16
 ; CHECK: std Z+2, r17
 ; CHECK: call
 ; Call frame restore
 ; CHECK-NEXT: in r30, 61
 ; CHECK-NEXT: in r31, 62

llvm/test/CodeGen/AVR/varargs.ll

; CHECK: ldd r25, [[REG]]+{{[0-9]+}}
   %1 = va_arg i8** %ap, i16
   call void @llvm.va_end(i8* %ap1)
   ret i16 %1
 }
 
 declare void @var1223(i16, ...)
 define void @varargcall() {
 ; CHECK-LABEL: varargcall:
-; CHECK: ldi [[REG1:r[0-9]+]], 189
-; CHECK: ldi [[REG2:r[0-9]+]], 205
-; CHECK: std Z+3, [[REG1]]
-; CHECK: std Z+4, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 191
 ; CHECK: ldi [[REG2:r[0-9]+]], 223
 ; CHECK: std Z+5, [[REG1]]
 ; CHECK: std Z+6, [[REG2]]
+; CHECK: ldi [[REG1:r[0-9]+]], 189
+; CHECK: ldi [[REG2:r[0-9]+]], 205
+; CHECK: std Z+3, [[REG1]]
+; CHECK: std Z+4, [[REG2]]
 ; CHECK: ldi [[REG1:r[0-9]+]], 205
 ; CHECK: ldi [[REG2:r[0-9]+]], 171
 ; CHECK: std Z+1, [[REG1]]
 ; CHECK: std Z+2, [[REG2]]
 ; CHECK: call
 ; CHECK: adiw r30, 6
   tail call void (i16, ...) @var1223(i16 -21555, i16 -12867, i16 -8257)
   ret void
 }