From 86ac6b9beec1c142867b53350006c02e20e4a7ef Mon Sep 17 00:00:00 2001
From: Michael Matz <address@hidden>
Date: Sun, 15 Apr 2012 05:12:43 +0200
Subject: [PATCH 3/9] x86_64: Fix indirection in struct paramaters

The first loop setting up struct arguments must not remove
elements from the vstack (via vtop--), as gen_reg needs them to
potentially evict some argument still held in registers to stack.

Swapping the arg in question to top (and back to its place) also
simplifies the vstore call itself, as not funny save/restore
or some "non-existing" stack elements need to be done.

Generally for a stack a vop-- operation conceptually clobbers
that element, so further references to it aren't allowed anymore.
---
 tests/tcctest.c |   29 +++++++++++++++++------------
 x86_64-gen.c    |   39 +++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/tests/tcctest.c b/tests/tcctest.c
index afb0825..fcd9822 100644
--- a/tests/tcctest.c
+++ b/tests/tcctest.c
@@ -1122,25 +1122,30 @@ struct structa1 struct_assign_test2(struct structa1 s1, int t)
 
 void struct_assign_test(void)
 {
-    struct structa1 lsta1, lsta2;
+    struct S {
+      struct structa1 lsta1, lsta2;
+      int i;
+    } s, *ps;
     
+    ps = &s;
+    ps->i = 4;
 #if 0
     printf("struct_assign_test:\n");
 
-    lsta1.f1 = 1;
-    lsta1.f2 = 2;
-    printf("%d %d\n", lsta1.f1, lsta1.f2);
-    lsta2 = lsta1;
-    printf("%d %d\n", lsta2.f1, lsta2.f2);
+    s.lsta1.f1 = 1;
+    s.lsta1.f2 = 2;
+    printf("%d %d\n", s.lsta1.f1, s.lsta1.f2);
+    s.lsta2 = s.lsta1;
+    printf("%d %d\n", s.lsta2.f1, s.lsta2.f2);
 #else
-    lsta2.f1 = 1;
-    lsta2.f2 = 2;
+    s.lsta2.f1 = 1;
+    s.lsta2.f2 = 2;
 #endif
-    struct_assign_test1(lsta2, 3, 4.5);
+    struct_assign_test1(ps->lsta2, 3, 4.5);
     
-    printf("before call: %d %d\n", lsta2.f1, lsta2.f2);
-    lsta2 = struct_assign_test2(lsta2, 4);
-    printf("after call: %d %d\n", lsta2.f1, lsta2.f2);
+    printf("before call: %d %d\n", s.lsta2.f1, s.lsta2.f2);
+    ps->lsta2 = struct_assign_test2(ps->lsta2, ps->i);
+    printf("after call: %d %d\n", ps->lsta2.f1, ps->lsta2.f2);
 
     static struct {
         void (*elem)();
diff --git a/x86_64-gen.c b/x86_64-gen.c
index 0b0e69e..49cfdd1 100644
--- a/x86_64-gen.c
+++ b/x86_64-gen.c
@@ -820,7 +820,6 @@ static const uint8_t arg_regs[REGN] = {
 void gfunc_call(int nb_args)
 {
     int size, align, r, args_size, i;
-    SValue *orig_vtop;
     int nb_reg_args = 0;
     int nb_sse_args = 0;
     int sse_reg, gen_reg;
@@ -845,7 +844,6 @@ void gfunc_call(int nb_args)
     /* for struct arguments, we need to call memcpy and the function
        call breaks register passing arguments we are preparing.
        So, we process arguments which will be passed by stack first. */
-    orig_vtop = vtop;
     gen_reg = nb_reg_args;
     sse_reg = nb_sse_args;
 
@@ -861,6 +859,14 @@ void gfunc_call(int nb_args)
     }
 
     for(i = 0; i < nb_args; i++) {
+	/* Swap argument to top, it will possibly be changed here,
+	   and might use more temps.  All arguments must remain on the
+	   stack, so that get_reg can correctly evict some of them onto
+	   stack.  We could use also use a vrott(nb_args) at the end
+	   of this loop, but this seems faster.  */
+        SValue tmp = vtop[0];
+	vtop[0] = vtop[-i];
+	vtop[-i] = tmp;
         if ((vtop->type.t & VT_BTYPE) == VT_STRUCT) {
             size = type_size(&vtop->type, &align);
             /* align to stack align size */
@@ -872,18 +878,9 @@ void gfunc_call(int nb_args)
             r = get_reg(RC_INT);
             orex(1, r, 0, 0x89); /* mov %rsp, r */
             o(0xe0 + REG_VALUE(r));
-            {
-                /* following code breaks vtop[1], vtop[2], and vtop[3] */
-                SValue tmp1 = vtop[1];
-                SValue tmp2 = vtop[2];
-                SValue tmp3 = vtop[3];
-                vset(&vtop->type, r | VT_LVAL, 0);
-                vswap();
-                vstore();
-                vtop[1] = tmp1;
-                vtop[2] = tmp2;
-                vtop[3] = tmp3;
-            }
+	    vset(&vtop->type, r | VT_LVAL, 0);
+	    vswap();
+	    vstore();
             args_size += size;
         } else if ((vtop->type.t & VT_BTYPE) == VT_LDOUBLE) {
             gv(RC_ST0);
@@ -913,10 +910,14 @@ void gfunc_call(int nb_args)
                 args_size += 8;
             }
         }
-        vtop--;
+
+	/* And swap the argument back to it's original position.  */
+        tmp = vtop[0];
+	vtop[0] = vtop[-i];
+	vtop[-i] = tmp;
     }
-    vtop = orig_vtop;
 
+    /* XXX This should be superfluous.  */
     save_regs(0); /* save used temporary registers */
 
     /* then, we prepare register passing arguments.
@@ -953,6 +954,12 @@ void gfunc_call(int nb_args)
         vtop--;
     }
 
+    /* We shouldn't have many operands on the stack anymore, but the
+       call address itself is still there, and it might be in %eax
+       (or edx/ecx) currently, which the below writes would clobber.
+       So evict all remaining operands here.  */
+    save_regs(0);
+
     /* Copy R10 and R11 into RDX and RCX, respectively */
     if (nb_reg_args > 2) {
         o(0xd2894c); /* mov %r10, %rdx */
-- 
1.7.3.4