[Qemu-devel] [PATCH v3 02/10] tcg: Use uintptr_t type for jmp_list_{next|first} fields of TB

[Sergey Fedorov <sergey.fedorov@linaro.org>]

From: Sergey Fedorov <serge.fdrv@gmail.com>

These fields do not contain pure pointers to a TranslationBlock
structure. So uintptr_t is the most appropriate type for them.
Also put an assert to assure that the two least significant bits of the
pointer are zero before assigning it to jmp_list_first.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 * Eliminated duplicate dereference of 'ptb' in tb_jmp_remove()

 include/exec/exec-all.h | 13 ++++++++-----
 translate-all.c         | 38 ++++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 38a149cc5e0c..b055716ed690 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -275,14 +275,16 @@ struct TranslationBlock {
      * jmp_list_first points to the first TB jumping to this one.
      * jmp_list_next is used to point to the next TB in a list.
      * Since each TB can have two jumps, it can participate in two lists.
-     * The two least significant bits of a pointer are used to choose which
-     * data field holds a pointer to the next TB:
+     * jmp_list_first and jmp_list_next are 4-byte aligned pointers to a
+     * TranslationBlock structure, but the two least significant bits of
+     * them are used to encode which data field of the pointed TB should
+     * be used to traverse the list further from that TB:
      * 0 => jmp_list_next[0], 1 => jmp_list_next[1], 2 => jmp_list_first.
      * In other words, 0/1 tells which jump is used in the pointed TB,
      * and 2 means that this is a pointer back to the target TB of this list.
      */
-    struct TranslationBlock *jmp_list_next[2];
-    struct TranslationBlock *jmp_list_first;
+    uintptr_t jmp_list_next[2];
+    uintptr_t jmp_list_first;
 };
 
 #include "qemu/thread.h"
@@ -401,7 +403,8 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
 
         /* add in TB jmp circular list */
         tb->jmp_list_next[n] = tb_next->jmp_list_first;
-        tb_next->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | n);
+        assert(((uintptr_t)tb & 3) == 0);
+        tb_next->jmp_list_first = (uintptr_t)tb | n;
     }
 }
 
diff --git a/translate-all.c b/translate-all.c
index 33ca06c663d4..ba71ff73f55f 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -927,17 +927,17 @@ static inline void tb_page_remove(TranslationBlock **ptb, TranslationBlock *tb)
 
 static inline void tb_jmp_remove(TranslationBlock *tb, int n)
 {
-    TranslationBlock *tb1, **ptb;
+    TranslationBlock *tb1;
+    uintptr_t *ptb, ntb;
     unsigned int n1;
 
     ptb = &tb->jmp_list_next[n];
-    tb1 = *ptb;
-    if (tb1) {
+    if (*ptb) {
         /* find tb(n) in circular list */
         for (;;) {
-            tb1 = *ptb;
-            n1 = (uintptr_t)tb1 & 3;
-            tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
+            ntb = *ptb;
+            n1 = ntb & 3;
+            tb1 = (TranslationBlock *)(ntb & ~3);
             if (n1 == n && tb1 == tb) {
                 break;
             }
@@ -950,7 +950,7 @@ static inline void tb_jmp_remove(TranslationBlock *tb, int n)
         /* now we can suppress tb(n) from the list */
         *ptb = tb->jmp_list_next[n];
 
-        tb->jmp_list_next[n] = NULL;
+        tb->jmp_list_next[n] = (uintptr_t)NULL;
     }
 }
 
@@ -969,7 +969,7 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr)
     PageDesc *p;
     unsigned int h, n1;
     tb_page_addr_t phys_pc;
-    TranslationBlock *tb1, *tb2;
+    uintptr_t tb1, tb2;
 
     /* remove the TB from the hash list */
     phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
@@ -1005,19 +1005,20 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr)
     /* suppress any remaining jumps to this TB */
     tb1 = tb->jmp_list_first;
     for (;;) {
-        n1 = (uintptr_t)tb1 & 3;
+        TranslationBlock *tmp_tb;
+        n1 = tb1 & 3;
         if (n1 == 2) {
             break;
         }
-        tb1 = (TranslationBlock *)((uintptr_t)tb1 & ~3);
-        tb2 = tb1->jmp_list_next[n1];
-        tb_reset_jump(tb1, n1);
-        tb1->jmp_list_next[n1] = NULL;
+        tmp_tb = (TranslationBlock *)(tb1 & ~3);
+        tb2 = tmp_tb->jmp_list_next[n1];
+        tb_reset_jump(tmp_tb, n1);
+        tmp_tb->jmp_list_next[n1] = (uintptr_t)NULL;
         tb1 = tb2;
     }
 
-    /* fail safe */
-    tb->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | 2);
+    assert(((uintptr_t)tb & 3) == 0);
+    tb->jmp_list_first = (uintptr_t)tb | 2; /* fail safe */
 
     tcg_ctx.tb_ctx.tb_phys_invalidate_count++;
 }
@@ -1491,9 +1492,10 @@ static void tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc,
         tb->page_addr[1] = -1;
     }
 
-    tb->jmp_list_first = (TranslationBlock *)((uintptr_t)tb | 2);
-    tb->jmp_list_next[0] = NULL;
-    tb->jmp_list_next[1] = NULL;
+    assert(((uintptr_t)tb & 3) == 0);
+    tb->jmp_list_first = (uintptr_t)tb | 2;
+    tb->jmp_list_next[0] = (uintptr_t)NULL;
+    tb->jmp_list_next[1] = (uintptr_t)NULL;
 
     /* init original jump addresses */
     if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) {
-- 
2.8.1