Re: [Qemu-devel] [PATCH v4 0/7] tcg/ppc: Add vector opcodes

[Mark Cave-Ayland]

On 26/06/2019 20:38, Mark Cave-Ayland wrote:

>> But I thought that _CALL_AIX was only defined for ppc64 elf version 1.  I
>> thought that ppc32 used _CALL_SYSV instead.  Certainly that's what is used
>> elsewhere...
> 
> No, that didn't work either. I've confirmed using #ifdef _CALL_AIX #error 
> ERROR
> #endif that _CALL_AIX is *NOT* defined and _CALL_SYSV *is* defined.
> 
> I've also tried removing TCG_REG_R2 from tcg_target_reg_alloc_order[] and
> tcg_regset_set_reg() for TCG_REG_R2 from tcg_target_init() and I'm still 
> generating
> bad code that writes to r2(!).
> 
> Since I can't find any other mentions of TCG_REG_TMP1 and TCG_REG_R2 that 
> isn't
> inside an #ifdef _CALL_AIX ... #endif section I'm starting to get stuck. Is 
> there any
> chance that the R_PPC_ADDR32 change could be causing this at all?

So after a lot more digging: the issue can be seen in tcg_out_ld() and 
tcg_out_st()
for the vector registers. Taking tcg_out_ld() as an example:

    case TCG_TYPE_V128:
        tcg_debug_assert(ret >= 32);
        assert((offset & 15) == 0);
        tcg_out_mem_long(s, 0, LVX, ret & 31, base, offset);
        break;

For the TCG_TYPE_V128 case we have ret = TCG_REG_V2 but (ret & 31) masks off 
the top
bit which converts this to TCG_REG_R2 and that's why tcg_out_mem_long() starts 
using
r2 to calculate offsets.

Assuming that rt is the temporary register used to calculate the address then 
the
patch below tentatively appears to get things working again by passing in
TCG_REG_TMP1 instead, but ultimately I still see a crash much later when trying 
to
boot MacOS 9:

diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 61732c1f45..dd823447cc 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -1139,7 +1139,7 @@ static void tcg_out_ld(TCGContext *s, TCGType type, 
TCGReg ret,
     case TCG_TYPE_V64:
         tcg_debug_assert(ret >= 32);
         assert((offset & 7) == 0);
-        tcg_out_mem_long(s, 0, LVX, ret & 31, base, offset & -16);
+        tcg_out_mem_long(s, 0, LVX, TCG_REG_TMP1, base, offset & -16);
         if (offset & 8) {
             tcg_out_vsldoi(s, ret, ret, ret, 8);
         }
@@ -1147,7 +1147,7 @@ static void tcg_out_ld(TCGContext *s, TCGType type, 
TCGReg ret,
     case TCG_TYPE_V128:
         tcg_debug_assert(ret >= 32);
         assert((offset & 15) == 0);
-        tcg_out_mem_long(s, 0, LVX, ret & 31, base, offset);
+        tcg_out_mem_long(s, 0, LVX, TCG_REG_TMP1, base, offset);
         break;
     default:
         g_assert_not_reached();
@@ -1186,12 +1186,13 @@ static void tcg_out_st(TCGContext *s, TCGType type, 
TCGReg arg,
             tcg_out_vsldoi(s, TCG_VEC_TMP1, arg, arg, 8);
             arg = TCG_VEC_TMP1;
         }
-        tcg_out_mem_long(s, 0, STVEWX, arg & 31, base, offset);
-        tcg_out_mem_long(s, 0, STVEWX, arg & 31, base, offset + 4);
+        tcg_out_mem_long(s, 0, STVEWX, TCG_REG_TMP1, base, offset);
+        tcg_out_mem_long(s, 0, STVEWX, TCG_REG_TMP1, base, offset + 4);
         break;
     case TCG_TYPE_V128:
         tcg_debug_assert(arg >= 32);
-        tcg_out_mem_long(s, 0, STVX, arg & 31, base, offset);
+        assert((offset & 15) == 0);
+        tcg_out_mem_long(s, 0, STVX, TCG_REG_TMP1, base, offset);
         break;
     default:
         g_assert_not_reached();

Richard: even though it's still not perfect, does this look like it's fixing the
right problem? Presumably the reason this didn't break on your Power 9 box is 
because
the 64-bit ABI doesn't mark r2 as reserved?


ATB,

Mark.