* [Qemu-devel] [PATCH 0/2] HPPA tcg fixes
@ 2019-09-13 10:17 Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 1/2] target/hppa: prevent trashing of temporary in trans_mtctl() Sven Schnelle
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sven Schnelle @ 2019-09-13 10:17 UTC (permalink / raw)
To: Richard Henderson; +Cc: Helge Deller, Sven Schnelle, qemu-devel
Hi Richard,
here are two patches for HPPA tcg. QEMU was crashing with a tcg assert
because dead temporaries where used. This could be observed at the end·
of a HP-UX 11.11 installation, or by running the STARBASE X11 demos in
HP-UX 10.20.
Thanks,
Sven
Sven Schnelle (2):
target/hppa: prevent trashing of temporary in trans_mtctl()
target/hppa: prevent trashing of temporary in do_depw_sar()
target/hppa/translate.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
--
2.23.0.rc1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] target/hppa: prevent trashing of temporary in trans_mtctl()
2019-09-13 10:17 [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Sven Schnelle
@ 2019-09-13 10:17 ` Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar() Sven Schnelle
2019-09-13 18:13 ` [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Richard Henderson
2 siblings, 0 replies; 7+ messages in thread
From: Sven Schnelle @ 2019-09-13 10:17 UTC (permalink / raw)
To: Richard Henderson
Cc: Helge Deller, Sven Schnelle, qemu-devel, Richard Henderson
nullify_over() calls brcond which destroys all temporaries.
Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
target/hppa/translate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 53e17d8963..b12525d535 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2214,10 +2214,11 @@ static bool trans_mtsp(DisasContext *ctx, arg_mtsp *a)
static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
{
unsigned ctl = a->t;
- TCGv_reg reg = load_gpr(ctx, a->r);
+ TCGv_reg reg;
TCGv_reg tmp;
if (ctl == CR_SAR) {
+ reg = load_gpr(ctx, a->r);
tmp = tcg_temp_new();
tcg_gen_andi_reg(tmp, reg, TARGET_REGISTER_BITS - 1);
save_or_nullify(ctx, cpu_sar, tmp);
@@ -2232,6 +2233,8 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
#ifndef CONFIG_USER_ONLY
nullify_over(ctx);
+ reg = load_gpr(ctx, a->r);
+
switch (ctl) {
case CR_IT:
gen_helper_write_interval_timer(cpu_env, reg);
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar()
2019-09-13 10:17 [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 1/2] target/hppa: prevent trashing of temporary in trans_mtctl() Sven Schnelle
@ 2019-09-13 10:17 ` Sven Schnelle
2019-09-13 10:58 ` Philippe Mathieu-Daudé
2019-09-13 18:13 ` [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Richard Henderson
2 siblings, 1 reply; 7+ messages in thread
From: Sven Schnelle @ 2019-09-13 10:17 UTC (permalink / raw)
To: Richard Henderson
Cc: Helge Deller, Sven Schnelle, qemu-devel, Richard Henderson
nullify_over() calls brcond which destroys all temporaries.
Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
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);
+ }
return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_const(ctx, a->i));
}
--
2.23.0.rc1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar()
2019-09-13 10:17 ` [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar() Sven Schnelle
@ 2019-09-13 10:58 ` Philippe Mathieu-Daudé
2019-09-13 11:08 ` Sven Schnelle
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 10:58 UTC (permalink / raw)
To: Sven Schnelle, Richard Henderson
Cc: Helge Deller, qemu-devel, Richard Henderson
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 <svens@stackframe.org>
> ---
> 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.
> return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_const(ctx, a->i));
> }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar()
2019-09-13 10:58 ` Philippe Mathieu-Daudé
@ 2019-09-13 11:08 ` Sven Schnelle
2019-09-13 11:16 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Sven Schnelle @ 2019-09-13 11:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Helge Deller, Richard Henderson, qemu-devel, Richard Henderson
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 <svens@stackframe.org>
> > ---
> > 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.
> > return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_const(ctx, a->i));
> > }
> >
> >
Regards
Sven
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar()
2019-09-13 11:08 ` Sven Schnelle
@ 2019-09-13 11:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 11:16 UTC (permalink / raw)
To: Sven Schnelle
Cc: Helge Deller, Richard Henderson, qemu-devel, Richard Henderson
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 <svens@stackframe.org>
>>> ---
>>> 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é <philmd@redhat.com>
>>> return do_depw_sar(ctx, a->t, a->c, a->nz, a->clen, load_const(ctx, a->i));
>>> }
>>>
>>>
>
> Regards
> Sven
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] HPPA tcg fixes
2019-09-13 10:17 [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 1/2] target/hppa: prevent trashing of temporary in trans_mtctl() Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar() Sven Schnelle
@ 2019-09-13 18:13 ` Richard Henderson
2 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2019-09-13 18:13 UTC (permalink / raw)
To: Sven Schnelle; +Cc: Helge Deller, qemu-devel
On 9/13/19 6:17 AM, Sven Schnelle wrote:
> Sven Schnelle (2):
> target/hppa: prevent trashing of temporary in trans_mtctl()
> target/hppa: prevent trashing of temporary in do_depw_sar()
>
> target/hppa/translate.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
Thanks, queued.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-13 18:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 10:17 [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 1/2] target/hppa: prevent trashing of temporary in trans_mtctl() Sven Schnelle
2019-09-13 10:17 ` [Qemu-devel] [PATCH 2/2] target/hppa: prevent trashing of temporary in do_depw_sar() Sven Schnelle
2019-09-13 10:58 ` Philippe Mathieu-Daudé
2019-09-13 11:08 ` Sven Schnelle
2019-09-13 11:16 ` Philippe Mathieu-Daudé
2019-09-13 18:13 ` [Qemu-devel] [PATCH 0/2] HPPA tcg fixes Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).