diff mbox

Porting TCG to alpha platform

Message ID 4B6334B8.9040002@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Jan. 29, 2010, 7:19 p.m. UTC 
> +        } else if (cond == TCG_COND_EQ || cond == TCG_COND_NE) {
> +            tcg_out_mem_long(s, INSN_LDA, TMP_REG1, arg1, -arg2);
> +            opc = (cond == TCG_COND_EQ ? INSN_BEQ : INSN_BNE);

Bug here.  What was intended was to add "arg1 = TMP_REG1".
But since the constraints use "I" for uint8_t input constants,
we might as well remove this hunk entirely.

Also, let's future-proof this routine against changes to
the layout of the TCGCond enumeration.


r~
commit 9d787576342c193f13e2531953fc81442458de7e
Author: Richard Henderson <rth@twiddle.net>
Date:   Fri Jan 29 11:14:20 2010 -0800

    tcg-alpha: Fix EQ/NE with a constant.
    
    There was start of code to handle EQ and NE with arbitrary constants,
    but it wasn't completed.  Remove the half-done code and add a comment
    for future enhancements.
    
    Also, don't rely on the current layout of TCGCond; instead encode the
    need for inversion of the compare insn result by means of a low bit set
    in the cmp_opc table.  Reduce the element size of cmp_opc.

Comments

identifier scorpio Jan. 30, 2010, 2:45 a.m. UTC | #1 
BUG?

In tcg_out_qemu_ld

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));

since the when calling tcg_out_ld, the R1's value is (cpu_env + TLB_entry_offset), which was computed in tcg_out_tlb_cmp()

And the save problem in tcg_out_qemu_ld()

another BUG?

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.

I'm currently testing your code in my free time.

Dong Weiyu.



      ___________________________________________________________ 
  好玩贺卡等你发，邮箱贺卡全新上线！ 
http://card.mail.cn.yahoo.com/
diff mbox

Patch

diff --git a/tcg/alpha/tcg-target.c b/tcg/alpha/tcg-target.c
index 5b7dd25..18ab2c8 100644
--- a/tcg/alpha/tcg-target.c
+++ b/tcg/alpha/tcg-target.c
@@ -540,9 +540,11 @@  static void tcg_out_br(TCGContext *s, int opc, int ra, int label_index)
     tcg_out_fmt_br(s, opc, ra, value);
 }
 
-static void tcg_out_brcond(TCGContext *s, int cond, TCGArg arg1,
+static void tcg_out_brcond(TCGContext *s, TCGCond cond, TCGArg arg1,
                            TCGArg arg2, int const_arg2, int label_index)
 {
+    /* Note that unsigned comparisons are not present here, which means
+       that their entries will contain zeros.  */
     static const int br_opc[10] = {
         [TCG_COND_EQ] = INSN_BEQ,
         [TCG_COND_NE] = INSN_BNE,
@@ -552,38 +554,56 @@  static void tcg_out_brcond(TCGContext *s, int cond, TCGArg arg1,
         [TCG_COND_GT] = INSN_BGT
     };
 
-    static const uint64_t cmp_opc[10] = {
+    /* The low bit of these entries indicates that the result of 
+       the comparison must be inverted.  This bit should not be
+       output with the rest of the instruction.  */
+    static const int cmp_opc[10] = {
         [TCG_COND_EQ] = INSN_CMPEQ,
-        [TCG_COND_NE] = INSN_CMPEQ,
+        [TCG_COND_NE] = INSN_CMPEQ | 1,
         [TCG_COND_LT] = INSN_CMPLT,
-        [TCG_COND_GE] = INSN_CMPLT,
+        [TCG_COND_GE] = INSN_CMPLT | 1,
         [TCG_COND_LE] = INSN_CMPLE,
-        [TCG_COND_GT] = INSN_CMPLE,
+        [TCG_COND_GT] = INSN_CMPLE | 1,
         [TCG_COND_LTU] = INSN_CMPULT,
-        [TCG_COND_GEU] = INSN_CMPULT,
+        [TCG_COND_GEU] = INSN_CMPULT | 1,
         [TCG_COND_LEU] = INSN_CMPULE,
-        [TCG_COND_GTU] = INSN_CMPULE
+        [TCG_COND_GTU] = INSN_CMPULE | 1
     };
 
     int opc = 0;
 
-    if (const_arg2) {
-        if (arg2 == 0) {
-            opc = br_opc[cond];
-        } else if (cond == TCG_COND_EQ || cond == TCG_COND_NE) {
-            tcg_out_mem_long(s, INSN_LDA, TMP_REG1, arg1, -arg2);
-            opc = (cond == TCG_COND_EQ ? INSN_BEQ : INSN_BNE);
-        }
+    /* Possible improvements:
+       (1) Notice arg1 == $31 and !const_arg2.  In this case, swap the
+       two operands and swap the sense of the comparison to allow the
+       use of the direct branches.
+
+       (2) Allow arbitrary constants.  We can, on occasion, generate one
+       less instruction if we compute
+           TMP = ARG1 - CONST
+       instead of
+           TMP = ARG1 cmp TMP2
+       where TMP2 is the constant loaded into a register by generic code.
+       Note that for 64-bit operands this works only for EQ and NE.  For
+       32-bit operands, we would need to either limit this to signed
+       comparisons or properly zero-extend unsigned inputs.  The payoff
+       here isn't great though; much less than(1).  */
+
+    /* Notice signed comparisons vs zero.  These are handled by the
+       branch instructions directly.  */
+    if (const_arg2 && arg2 == 0) {
+        opc = br_opc[cond];
     }
 
+    /* Otherwise, generate a comparison into a temporary.  */
     if (opc == 0) {
-        opc = cmp_opc[cond];
+        opc = cmp_opc[cond] & ~1;
         if (const_arg2) {
             tcg_out_fmt_opi(s, opc, arg1, arg2, TMP_REG1);
         } else {
             tcg_out_fmt_opr(s, opc, arg1, arg2, TMP_REG1);
         }
-        opc = (cond & 1) ? INSN_BEQ : INSN_BNE;
+
+        opc = (cmp_opc[cond] & 1 ? INSN_BEQ : INSN_BNE);
         arg1 = TMP_REG1;
     }