Skip to contentSkip to conversation

Avoid qemu SIGFPE for MIPS DIV

PATCH New Richard Sandiford 1 participant 1 message May 25, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Avoid qemu SIGFPE for MIPS DIV

Performing (-1 << 31) / -1 with a MIPS DIV instruction currently causes the emulator to exit with a SIGFPE. This wasn't a problem before TCGification because we used 64-bit division instead of 32-bit division. So I suppose there are two obvious fixes:
use 64-bit division once more, or add overflow checks in the same way as we do for DDIV. I've attached patches for both approaches below.

BTW, sorry for the flurry of patches. I tried to break things up as much as possible, but it makes the situation look far worse than it actually is...

Richard

target-mips/translate.c +31 -14
Index: qemu/target-mips/translate.c
===================================================================
@@ -1985,21 +1985,38 @@ static void gen_muldiv (DisasContext *ct
 
             tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, l1);
             {
-                TCGv r_tmp1 = new_tmp();
-                TCGv r_tmp2 = new_tmp();
-                TCGv r_tmp3 = new_tmp();
+                int l2 = gen_new_label();
 
-                tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
-                tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
-                tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
-                tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
-                tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
-                tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
-                gen_store_LO(cpu_T[0], 0);
-                gen_store_HI(cpu_T[1], 0);
-                dead_tmp(r_tmp1);
-                dead_tmp(r_tmp2);
-                dead_tmp(r_tmp3);
+                /* The handling of non-sign-extended values is unpredictable,
+                   but it still shouldn't trigger a SIGFPE in the emulator.  */
+                tcg_gen_ext32s_tl(cpu_T[0], cpu_T[0]);
+                tcg_gen_ext32s_tl(cpu_T[1], cpu_T[1]);
+                tcg_gen_brcondi_tl(TCG_COND_NE, cpu_T[0], -1 << 31, l2);
+                tcg_gen_brcondi_tl(TCG_COND_NE, cpu_T[1], -1, l2);
+                {
+                    tcg_gen_movi_tl(cpu_T[1], 0);
+                    gen_store_LO(cpu_T[0], 0);
+                    gen_store_HI(cpu_T[1], 0);
+                    tcg_gen_br(l1);
+                }
+                gen_set_label(l2);
+                {
+                    TCGv r_tmp1 = new_tmp();
+                    TCGv r_tmp2 = new_tmp();
+                    TCGv r_tmp3 = new_tmp();
+
+                    tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
+                    tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
+                    tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
+                    tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
+                    tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
+                    tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
+                    gen_store_LO(cpu_T[0], 0);
+                    gen_store_HI(cpu_T[1], 0);
+                    dead_tmp(r_tmp1);
+                    dead_tmp(r_tmp2);
+                    dead_tmp(r_tmp3);
+                }
             }
             gen_set_label(l1);
         }
target-mips/translate.c +10 -14
Index: qemu/target-mips/translate.c
===================================================================
@@ -1985,21 +1985,17 @@ static void gen_muldiv (DisasContext *ct
 
             tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_T[1], 0, l1);
             {
-                TCGv r_tmp1 = new_tmp();
-                TCGv r_tmp2 = new_tmp();
-                TCGv r_tmp3 = new_tmp();
+                TCGv r_tmp1 = tcg_temp_new(TCG_TYPE_I64);
+                TCGv r_tmp2 = tcg_temp_new(TCG_TYPE_I64);
 
-                tcg_gen_trunc_tl_i32(r_tmp1, cpu_T[0]);
-                tcg_gen_trunc_tl_i32(r_tmp2, cpu_T[1]);
-                tcg_gen_div_i32(r_tmp3, r_tmp1, r_tmp2);
-                tcg_gen_rem_i32(r_tmp1, r_tmp1, r_tmp2);
-                tcg_gen_ext_i32_tl(cpu_T[0], r_tmp3);
-                tcg_gen_ext_i32_tl(cpu_T[1], r_tmp1);
-                gen_store_LO(cpu_T[0], 0);
-                gen_store_HI(cpu_T[1], 0);
-                dead_tmp(r_tmp1);
-                dead_tmp(r_tmp2);
-                dead_tmp(r_tmp3);
+                tcg_gen_ext32s_tl(cpu_T[0], cpu_T[0]);
+                tcg_gen_ext32s_tl(cpu_T[1], cpu_T[1]);
+                tcg_gen_div_i64(r_tmp1, cpu_T[0], cpu_T[1]);
+                tcg_gen_rem_i64(r_tmp2, cpu_T[0], cpu_T[1]);
+                tcg_gen_ext32s_tl(r_tmp1, r_tmp1);
+                tcg_gen_ext32s_tl(r_tmp2, r_tmp2);
+                gen_store_LO(r_tmp1, 0);
+                gen_store_HI(r_tmp2, 0);
             }
             gen_set_label(l1);
         }