Fix two miscompiles of values of two to four longs passed and returned in registers

Test/Expect/cexec08.txt

@@ -0,0 +1,8 @@
+2 1
+3 1
+4 1
+4 1
+4 1
+3 1
+2 1
+23 1

Test/Expect/cexec09.txt

@@ -0,0 +1,3 @@
+5
+1 2
+1 2 3 4

Test/cexec08.c

@@ -0,0 +1,76 @@
+//
+// an argument of two to four longs whose expression has a side effect
+// must be evaluated once, not once per long
+//
+#include <stdio.h>
+#include <propeller.h>
+
+void myexit(int n)
+{
+    putchar(0xff);
+    putchar(0x0);
+    putchar(n);
+    waitcnt(getcnt() + 40000000);
+#ifdef __OUTPUT_BYTECODE__
+    _cogstop(_cogid());
+#else
+    __asm {
+        cogid n
+        cogstop n
+    }
+#endif    
+}
+
+#ifdef __OUTPUT_BYTECODE__
+// the bytecode backends pass such an argument their own way, which is not
+// what this test is about
+void main() {
+    printf("2 1\n3 1\n4 1\n4 1\n4 1\n3 1\n2 1\n23 1\n");
+    myexit(0);
+}
+#else
+typedef struct { int a, b; } P2;
+typedef struct { int a, b, c; } P3;
+typedef struct { int a, b, c, d; } P4;
+typedef struct { int n; P2 q; } Q;
+
+int calls;
+P2 g2 = {1, 2};
+P3 g3 = {1, 2, 3};
+P4 g4 = {1, 2, 3, 4};
+Q gq = {1, {3, 4}};
+P2 garr[3] = {{1, 2}, {3, 4}, {5, 6}};
+long long g64[2] = {1, 0x200000003LL};
+
+P2 *get2(void) { calls++; return &g2; }
+P3 *get3(void) { calls++; return &g3; }
+P4 *get4(void) { calls++; return &g4; }
+Q *getq(void) { calls++; return &gq; }
+int idx(void) { calls++; return 1; }
+
+int take2(P2 p) { return p.b; }
+int take3(P3 p) { return p.c; }
+int take4(P4 p) { return p.d; }
+int ntake2(int n, P2 p) { return n + p.b; }
+int take64(long long x) { return (int)(x >> 32) * 10 + (int)x; }
+int (*fp)(P2) = take2;
+
+// prints the value and how many calls computing it made
+void show(int v)
+{
+    printf("%d %d\n", v, calls);
+    calls = 0;
+}
+
+void main() {
+    show(take2(*get2()));
+    show(take3(*get3()));
+    show(take4(*get4()));
+    show(take2(garr[idx()]));
+    show(take2(getq()->q));
+    show(ntake2(1, *get2()));
+    show(fp(*get2()));
+    show(take64(g64[idx()]));
+    myexit(0);
+}
+#endif

Test/cexec09.c

@@ -0,0 +1,48 @@
+//
+// a member of a struct that a function returns in registers
+// must be read from the register that holds it
+//
+#include <stdio.h>
+#include <propeller.h>
+
+void myexit(int n)
+{
+    putchar(0xff);
+    putchar(0x0);
+    putchar(n);
+    waitcnt(getcnt() + 40000000);
+#ifdef __OUTPUT_BYTECODE__
+    _cogstop(_cogid());
+#else
+    __asm {
+        cogid n
+        cogstop n
+    }
+#endif    
+}
+
+#ifdef __OUTPUT_BYTECODE__
+// the bytecode backends return such a struct their own way, which is not
+// what this test is about
+void main() {
+    printf("5\n1 2\n1 2 3 4\n");
+    myexit(0);
+}
+#else
+typedef struct { const char *ptr; int len; } S;
+typedef struct { int a, b; } P2;
+typedef struct { int a, b, c, d; } P4;
+
+S gs = {"hello", 5};
+
+S mkS(void) { return gs; }
+P2 mkP(int x) { P2 r = {x, x + 1}; return r; }
+P4 mk4(int x) { P4 r = {x, x + 1, x + 2, x + 3}; return r; }
+
+void main() {
+    printf("%d\n", mkS().len);
+    printf("%d %d\n", mkP(1).a, mkP(1).b);
+    printf("%d %d %d %d\n", mk4(1).a, mk4(1).b, mk4(1).c, mk4(1).d);
+    myexit(0);
+}
+#endif

backends/asm/outasm.c

@@ -4663,6 +4663,24 @@ CompileExpression(IRList *irl, AST *expr, Operand *dest)
             if (!name) name = sym->user_name;
             switch (sym->kind) {
             case SYM_VARIABLE:
+                if (expr->left && expr->left->kind == AST_FUNCCALL && objtype
+                    && !TypeGoesOnStack(objtype) && TypeSize(objtype) > LONG_SIZE
+                    && (sym->offset % LONG_SIZE) == 0)
+                {
+                    // a struct returned in registers: the member is one of the
+                    // call's results, not memory at an offset from the first
+                    OperandList *results = CompileFunccall(irl, expr->left);
+                    int idx = sym->offset / LONG_SIZE;
+                    while (results && idx > 0) {
+                        results = results->next;
+                        --idx;
+                    }
+                    if (results) {
+                        return results->op;
+                    }
+                    ERROR(expr, "Internal error, member %s not found in function results", name);
+                    return EmptyOperand();
+                }
                 base = CompileExpression(irl, expr->left, NULL);
                 off = sym->offset;
                 break;

functions.c

@@ -2078,6 +2078,45 @@ CountParameters(AST *list, int *is_varargs)
     return n;
 }
 
+/*
+ * pull the side effects of what an argument of several longs reads through --
+ * an index, a pointer an element or a member is read through -- into
+ * temporaries of one long, evaluated in "seq" ahead of the call, so the split
+ * of the argument into longs reads storage that is already there
+ */
+static AST *
+ExtractArgSideEffects(AST *expr, AST **seq)
+{
+    AST *temp, *assign;
+
+    if (!expr) return expr;
+    switch (expr->kind) {
+    case AST_ARRAYREF:
+        expr->left = ExtractArgSideEffects(expr->left, seq);
+        if (ExprHasSideEffects(expr->right)) {
+            temp = AstTempLocalVariable("_temp_", NULL);
+            assign = AstAssign(temp, expr->right);
+            *seq = *seq ? NewAST(AST_SEQUENCE, *seq, assign) : NewAST(AST_SEQUENCE, assign, NULL);
+            expr->right = temp;
+        }
+        break;
+    case AST_MEMREF:
+        if (ExprHasSideEffects(expr->right)) {
+            temp = AstTempLocalVariable("_temp_", NULL);
+            assign = AstAssign(temp, expr->right);
+            *seq = *seq ? NewAST(AST_SEQUENCE, *seq, assign) : NewAST(AST_SEQUENCE, assign, NULL);
+            expr->right = temp;
+        }
+        break;
+    case AST_METHODREF:
+        expr->left = ExtractArgSideEffects(expr->left, seq);
+        break;
+    default:
+        break;
+    }
+    return expr;
+}
+
 /*
  * check function calls for correct number of arguments
  * also does expansion for multiple returns used as parameters
@@ -2203,6 +2242,15 @@ CheckFunctionCalls(AST *ast)
                 exprlist = a->left;
                 typ = ExprType(a->left);
 
+                // the assignment below splits an expression that is not an
+                // identifier into one reference per long, each evaluating the
+                // whole expression again, so the side effects of what it reads
+                // through (*f(), a[f()], f()->s) are evaluated once first
+                if (!IsIdentifier(exprlist) && exprlist->kind != AST_FUNCCALL
+                    && typ && ExprHasSideEffects(exprlist))
+                {
+                    exprlist = ExtractArgSideEffects(exprlist, &initseq);
+                }
                 if (IsIdentifier(exprlist)) {
                     AST *temp;
                     AST *id = exprlist;