Skip to contentSkip to conversation

sparc32 use stq_* for 64bit stores

PATCH In review Robert Reif 2 participants 5 messages Oct 14, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sparc32 use stq_* for 64bit stores

Use stq_* for 64 bit stores.

This fixes one bug where T1 was used twice rather than T1 and T2.

Should the address be 64 bit alligned? i.e. T0 & ~7 rather than T0 & ~3?

Should these unaligned address cause traps?

Index: target-sparc/op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v retrieving revision 1.44

op_helper.c +5 -10
diff -p -u -r1.44 op_helper.c
@@ -515,8 +515,7 @@ void helper_st_asi(int asi, int size)
             stl_user(T0 & ~3, T1);
             break;
         case 8:
-            stl_user(T0 & ~3, T1);
-            stl_user((T0 + 4) & ~3, T2);
+            stq_user(T0 & ~3, ((uint64_t)T1 << 32) | T2);
             break;
         }
         break;
@@ -533,8 +532,7 @@ void helper_st_asi(int asi, int size)
             stl_kernel(T0 & ~3, T1);
             break;
         case 8:
-            stl_kernel(T0 & ~3, T1);
-            stl_kernel((T0 + 4) & ~3, T2);
+            stq_kernel(T0 & ~3, ((uint64_t)T1 << 32) | T2);
             break;
         }
         break;
@@ -591,8 +589,7 @@ void helper_st_asi(int asi, int size)
                 stl_phys(T0 & ~3, T1);
                 break;
             case 8:
-                stl_phys(T0 & ~3, T1);
-                stl_phys((T0 + 4) & ~3, T2);
+                stq_phys(T0 & ~3, ((uint64_t)T1 << 32) | T2);
                 break;
             }
         }
@@ -615,10 +612,8 @@ void helper_st_asi(int asi, int size)
                            | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
                 break;
             case 8:
-                stl_phys((target_phys_addr_t)(T0 & ~3)
-                           | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
-                stl_phys((target_phys_addr_t)((T0 + 4) & ~3)
-                           | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
+                stq_phys((target_phys_addr_t)(T0 & ~3)
+                           | ((target_phys_addr_t)(asi & 0xf) << 32), ((uint64_t)T1 << 32) | T2);
                 break;
             }
         }
#1 lore raw
Quoted 1 line · Robert Reif

Use stq_* for 64 bit stores.

This could be less optimal for 32 bit hosts, but hopefully the compiler knows its business.

Quoted 1 line

This fixes one bug where T1 was used twice rather than T1 and T2.

Great!

Quoted 3 lines

Should the address be 64 bit alligned? i.e. T0 & ~7 rather than T0 & ~3?

Should these unaligned address cause traps?

Yes, but the checks are already generated from translate.c (gen_op_check_align_T0_7).

#2 lore raw
Quoted 13 lines · Blue Swirl
Quoted 5 lines · Robert Reif

Should the address be 64 bit alligned? i.e. T0 & ~7 rather than T0 & ~3?

Should these unaligned address cause traps?

Yes, but the checks are already generated from translate.c (gen_op_check_align_T0_7).

De we to align then again?

#3 lore raw
Quoted 11 lines · Robert Reif
Quoted 9 lines · Blue Swirl
Quoted 5 lines · Robert Reif

Should the address be 64 bit alligned? i.e. T0 & ~7 rather than T0 & ~3?

Should these unaligned address cause traps?

Quoted 5 lines
Quoted 4 lines

Yes, but the checks are already generated from translate.c (gen_op_check_align_T0_7).

De we to align then again?

The checks aren't fully enabled yet.

#4 lore raw
In reply to Robert Reif
Quoted 1 line · Robert Reif

Use stq_* for 64 bit stores.

I changed also uses of 64 bit loads to ldq. But it looks like this makes OpenBIOS trigger alignment traps, this is the same reason why the alignment checks aren't fully enabled. So I can't commit this yet except for the buggy store fix.

op_helper.c renamed from op_helper.c.orig +21 -21
Index: target-sparc/op_helper.c
===================================================================
@@ -170,6 +170,7 @@
 void helper_ld_asi(int asi, int size, int sign)
 {
     uint32_t ret = 0;
+    uint64_t tmp;
 #ifdef DEBUG_MXCC
     uint32_t last_T0 = T0;
 #endif
@@ -244,8 +245,9 @@
             ret = ldl_code(T0 & ~3);
             break;
         case 8:
-            ret = ldl_code(T0 & ~3);
-            T0 = ldl_code((T0 + 4) & ~3);
+            tmp = ldq_code(T0 & ~7);
+            ret = tmp >> 32;
+            T0 = tmp & 0xffffffff;
             break;
         }
         break;
@@ -262,8 +264,9 @@
             ret = ldl_user(T0 & ~3);
             break;
         case 8:
-            ret = ldl_user(T0 & ~3);
-            T0 = ldl_user((T0 + 4) & ~3);
+            tmp = ldq_user(T0 & ~7);
+            ret = tmp >> 32;
+            T0 = tmp & 0xffffffff;
             break;
         }
         break;
@@ -280,8 +283,9 @@
             ret = ldl_kernel(T0 & ~3);
             break;
         case 8:
-            ret = ldl_kernel(T0 & ~3);
-            T0 = ldl_kernel((T0 + 4) & ~3);
+            tmp = ldq_kernel(T0 & ~7);
+            ret = tmp >> 32;
+            T0 = tmp & 0xffffffff;
             break;
         }
         break;
@@ -303,8 +307,9 @@
             ret = ldl_phys(T0 & ~3);
             break;
         case 8:
-            ret = ldl_phys(T0 & ~3);
-            T0 = ldl_phys((T0 + 4) & ~3);
+            tmp = ldq_phys(T0 & ~7);
+            ret = tmp >> 32;
+            T0 = tmp & 0xffffffff;
             break;
         }
         break;
@@ -325,10 +330,10 @@
                            | ((target_phys_addr_t)(asi & 0xf) << 32));
             break;
         case 8:
-            ret = ldl_phys((target_phys_addr_t)(T0 & ~3)
-                           | ((target_phys_addr_t)(asi & 0xf) << 32));
-            T0 = ldl_phys((target_phys_addr_t)((T0 + 4) & ~3)
+            tmp = ldq_phys((target_phys_addr_t)(T0 & ~7)
                            | ((target_phys_addr_t)(asi & 0xf) << 32));
+            ret = tmp >> 32;
+            T0 = tmp & 0xffffffff;
             break;
         }
         break;
@@ -515,8 +520,7 @@
             stl_user(T0 & ~3, T1);
             break;
         case 8:
-            stl_user(T0 & ~3, T1);
-            stl_user((T0 + 4) & ~3, T2);
+            stq_user(T0 & ~7, ((uint64_t)T1 << 32) | T2);
             break;
         }
         break;
@@ -533,8 +537,7 @@
             stl_kernel(T0 & ~3, T1);
             break;
         case 8:
-            stl_kernel(T0 & ~3, T1);
-            stl_kernel((T0 + 4) & ~3, T2);
+            stq_kernel(T0 & ~7, ((uint64_t)T1 << 32) | T2);
             break;
         }
         break;
@@ -591,8 +594,7 @@
                 stl_phys(T0 & ~3, T1);
                 break;
             case 8:
-                stl_phys(T0 & ~3, T1);
-                stl_phys((T0 + 4) & ~3, T2);
+                stq_phys(T0 & ~7, ((uint64_t)T1 << 32) | T2);
                 break;
             }
         }
@@ -615,10 +617,8 @@
                            | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
                 break;
             case 8:
-                stl_phys((target_phys_addr_t)(T0 & ~3)
-                           | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
-                stl_phys((target_phys_addr_t)((T0 + 4) & ~3)
-                           | ((target_phys_addr_t)(asi & 0xf) << 32), T1);
+                stq_phys((target_phys_addr_t)(T0 & ~7)
+                           | ((target_phys_addr_t)(asi & 0xf) << 32), ((uint64_t)T1 << 32) | T2);
                 break;
             }
         }
op_mem.h renamed from op_mem.h.orig +6 -4
Index: target-sparc/op_mem.h
===================================================================
@@ -36,8 +36,7 @@
 
 void OPPROTO glue(op_std, MEMSUFFIX)(void)
 {
-    glue(stl, MEMSUFFIX)(ADDR(T0), T1);
-    glue(stl, MEMSUFFIX)((ADDR(T0 + 4)), T2);
+    glue(stq, MEMSUFFIX)(ADDR(T0), (T1 << 32) | (T2 & 0xffffffff));
 }
 
 void OPPROTO glue(op_ldstub, MEMSUFFIX)(void)
@@ -55,8 +54,11 @@
 
 void OPPROTO glue(op_ldd, MEMSUFFIX)(void)
 {
-    T1 = glue(ldl, MEMSUFFIX)(ADDR(T0));
-    T0 = glue(ldl, MEMSUFFIX)((ADDR(T0 + 4)));
+    target_ulong tmp;
+
+    tmp = glue(ldq, MEMSUFFIX)(ADDR(T0));
+    T1 = tmp >> 32;
+    T0 = tmp & 0xffffffff;
 }
 
 /***                         Floating-point store                          ***/