Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform

[Richard Henderson <rth@twiddle.net>]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

> tcg_out_ld(s, TCG_TYPE_I64, r1, r1, offsetof(CPUTLBEntry, addend) -
> offsetof(CPUTLBEntry, addr_read));
>
> should be modified to
>
> tcg_out_ld(s, TCG_TYPE_I64, r1, r1, offsetof(CPUTLBEntry, addend));

Actually, it should be the entire offset from CPUState.  The ADDR_WRITE
offset before was entirely stuffed into the LD offset.

> At the end of tcg_out_tlb_cmp(), R0's value is (page# | low bit of
> VA), if the branch is taken, i.e., TLB miss, R0 will be passed as an
> argument to helper functions, is it currently holding the correct
> value? I think at this time R0 should equal to addr_reg.

You're absolutely right here.

Here's the kind of patch I think will fix it.


r~

[-- Attachment #2: commit-04e0dc2 --]
[-- Type: text/plain, Size: 4403 bytes --]

commit 04e0dc252203bfbc5b02c2d3d8757a005e304a39
Author: Richard Henderson <rth@twiddle.net>
Date:   Sun Jan 31 15:06:35 2010 -0800

    tcg-alpha: Fix problems in qemu_ld/st.
    
    Copy address to R0 in tcg_out_tlb_cmp, for use in helper path.
    Use entire TLB constant offset performing the ADDEND read.

diff --git a/tcg/alpha/tcg-target.c b/tcg/alpha/tcg-target.c
index 519edef..463b7ee 100644
--- a/tcg/alpha/tcg-target.c
+++ b/tcg/alpha/tcg-target.c
@@ -709,9 +709,10 @@ static void *qemu_st_helpers[4] = {
 static void tcg_out_tlb_cmp(TCGContext *s, int sizeop, int r0, int r1,
                             int addr_reg, int label1, long tlb_offset)
 {
+    int addrsizeop = TARGET_LONG_BITS == 32 ? 2 : 3;
     long val;
 
-    /* Mask the page, plus the low bits of the access, into R0.  Note
+    /* Mask the page, plus the low bits of the access, into TMP3.  Note
        that the low bits are added in order to catch unaligned accesses,
        as those bits won't be set in the TLB entry.  For 32-bit targets,
        force the high bits of the mask to be zero, as the high bits of
@@ -720,7 +721,7 @@ static void tcg_out_tlb_cmp(TCGContext *s, int sizeop, int r0, int r1,
     if (TARGET_LONG_BITS == 32) {
         val &= 0xffffffffu;
     }
-    tcg_out_andi(s, addr_reg, val, r0);
+    tcg_out_andi(s, addr_reg, val, TMP_REG3);
 
     /* Compute the index into the TLB into R1.  Again, note that the
        high bits of a 32-bit address must be cleared.  */
@@ -736,11 +737,14 @@ static void tcg_out_tlb_cmp(TCGContext *s, int sizeop, int r0, int r1,
     /* Load the word at (R1 + CPU_ENV + TLB_OFFSET).  Note that we
        arrange for a 32-bit load to be zero-extended.  */
     tcg_out_fmt_opr(s, INSN_ADDQ, r1, TCG_AREG0, r1);
-    tcg_out_ld_sz(s, TARGET_LONG_BITS == 32 ? 2 : 3,
-                  TMP_REG2, r1, tlb_offset);
+    tcg_out_ld_sz(s, addrsizeop, TMP_REG2, r1, tlb_offset);
 
-    /* Compare R0 with the value loaded from the TLB.  */
-    tcg_out_brcond(s, TCG_COND_NE, TMP_REG2, r0, 0, label1);
+    /* Copy the original address into R0.  This is needed on the
+       slow path through the helper function.  */
+    tcg_out_extend(s, addrsizeop, addr_reg, r0);
+
+    /* Compare TMP3 with the value loaded from the TLB.  */
+    tcg_out_brcond(s, TCG_COND_NE, TMP_REG2, TMP_REG3, 0, label1);
 }
 #endif /* SOFTMMU */
 
@@ -769,20 +773,12 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, int sizeop)
     /* TLB Hit.  Note that Alpha statically predicts forward branch as
        not taken, so arrange the fallthru as the common case.
 
-       ADDR_REG contains the guest address, and R1 contains the pointer
-       to TLB_ENTRY.ADDR_READ.  We need to load TLB_ENTRY.ADDEND and
-       add it to ADDR_REG to get the host address.  */
+       R0 contains the guest address, and R1 contains the pointer
+       to CPU_ENV plus the TLB entry offset.  */
 
     tcg_out_ld(s, TCG_TYPE_I64, r1, r1,
-               offsetof(CPUTLBEntry, addend)
-               - offsetof(CPUTLBEntry, addr_read));
-
-    if (TARGET_LONG_BITS == 32) {
-        tcg_out_extend(s, 2, addr_reg, r0);
-        addr_reg = r0;
-    }
-
-    tcg_out_fmt_opr(s, INSN_ADDQ, addr_reg, r1, r0);
+               offsetof(CPUState, tlb_table[mem_index][0].addend));
+    tcg_out_fmt_opr(s, INSN_ADDQ, r0, r1, r0);
     val = 0;
 #else
     r0 = addr_reg;
@@ -841,20 +837,12 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, int sizeop)
     /* TLB Hit.  Note that Alpha statically predicts forward branch as
        not taken, so arrange the fallthru as the common case.
 
-       ADDR_REG contains the guest address, and R1 contains the pointer
-       to TLB_ENTRY.ADDR_READ.  We need to load TLB_ENTRY.ADDEND and
-       add it to ADDR_REG to get the host address.  */
+       R0 contains the guest address, and R1 contains the pointer
+       to CPU_ENV plus the TLB entry offset.  */
 
     tcg_out_ld(s, TCG_TYPE_I64, r1, r1,
-               offsetof(CPUTLBEntry, addend)
-               - offsetof(CPUTLBEntry, addr_write));
-
-    if (TARGET_LONG_BITS == 32) {
-        tcg_out_extend(s, 2, addr_reg, r0);
-        addr_reg = r0;
-    }
-
-    tcg_out_fmt_opr(s, INSN_ADDQ, addr_reg, r1, r0);
+               offsetof(CPUState, tlb_table[mem_index][0].addend));
+    tcg_out_fmt_opr(s, INSN_ADDQ, r0, r1, r0);
     val = 0;
 #else
     r0 = addr_reg;