[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar() |
Date: |
Fri, 13 Sep 2019 13:16:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 9/13/19 1:08 PM, Sven Schnelle wrote:
> Hi Philippe,
>
> On Fri, Sep 13, 2019 at 12:58:14PM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Sven,
>>
>> On 9/13/19 12:17 PM, Sven Schnelle wrote:
>>> nullify_over() calls brcond which destroys all temporaries.
>>>
>>> Signed-off-by: Sven Schnelle <address@hidden>
>>> ---
>>> target/hppa/translate.c | 10 ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
>>> index b12525d535..c1b2822f60 100644
>>> --- a/target/hppa/translate.c
>>> +++ b/target/hppa/translate.c
>>> @@ -3404,10 +3404,6 @@ static bool do_depw_sar(DisasContext *ctx, unsigned
>>> rt, unsigned c,
>>> TCGv_reg mask, tmp, shift, dest;
>>> unsigned msb = 1U << (len - 1);
>>>
>>> - if (c) {
>>> - nullify_over(ctx);
>>> - }
>>> -
>>> dest = dest_gpr(ctx, rt);
>>> shift = tcg_temp_new();
>>> tmp = tcg_temp_new();
>>> @@ -3440,11 +3436,17 @@ static bool do_depw_sar(DisasContext *ctx, unsigned
>>> rt, unsigned c,
>>>
>>> static bool trans_depw_sar(DisasContext *ctx, arg_depw_sar *a)
>>> {
>>> + if (a->c) {
>>> + nullify_over(ctx);
>>> + }
>>> return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_gpr(ctx,
>>> a->r));
>>> }
>>>
>>> static bool trans_depwi_sar(DisasContext *ctx, arg_depwi_sar *a)
>>> {
>>> + if (a->c) {
>>> + nullify_over(ctx);
>>> + }
>>
>> I don't see how this patch helps or change anything, isn't it the same?
>> You clean in the caller rather than the callee.
>
> The Problem is that load_gpr()/load_const() allocate a temporary, which
> gets destroyed in do_depw_sar() when nullify_over() is called. If we
> move the nullify_over() before doing the load_gpr()/load_const() this
> doesn't happen.
Ah! The 'val' argument... I missed that, thanks!
Maybe we can add a comment to make it clearer:
if (a->c) {
/*
* nullify here to not free the load_gpr() arg before
* calling depw_sar.
*/
nullify_over(ctx);
}
return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_gpr(ctx, a->r));
(also in the other function).
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>>> return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_const(ctx,
>>> a->i));
>>> }
>>>
>>>
>
> Regards
> Sven
>