qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: BALATON Zoltan <balaton@eik.bme.hu>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: lvivier@redhat.com, alistair23@gmail.com, qemu-devel@nongnu.org
Subject: Re: [PATCH 0/5] tcg: Dynamically allocate temporaries
Date: Wed, 20 Jan 2021 00:06:16 +0100 (CET)	[thread overview]
Message-ID: <7595e6e-bc3d-d626-656b-e7ba3bfd8b90@eik.bme.hu> (raw)
In-Reply-To: <20210119183428.556706-1-richard.henderson@linaro.org>

On Tue, 19 Jan 2021, Richard Henderson wrote:
> My recent change for caching tcg constants has, in a number of cases,
> overflowed the statically allocated array of temporaries.  Change to
> dynamic allocation.

This seems to work for me so

Tested-by: BALATON Zoltan <balaton@eik.bme.hu>

but have you done any performance tests to check that this actually 
improves emulation speed? To mee it seems slower. Booting AmigaOS on 
sam460ex with c0dd6654f207 (just before your TCG series) takes:

real	0m33.829s
user	0m34.432s
sys	0m0.296s

but on HEAD with this series:

real	0m44.381s
user	0m46.058s
sys	0m0.532s

This is noticable decrease in speed also without measuring it. With just 
increasing the TCG_MAX_TEMPS to 2048 on 7c79721606be without this series I 
get:

real	0m42.681s
user	0m44.208s
sys	0m0.435s

So the performance regression is somewhere in the original series not in 
this fix up series.

> I'll note that nothing in check-acceptance triggers this overflow.
> Anyone care to add some more test cases there?

The proposed test for the upcoming pegasos2 machine may also catch this 
(when that will be merged, its dependencies are still under review) or the 
sam460ex test that currently only checks the firmware could be enhanced to 
try to boot AROS if somebody wants to do that. The drawback is that it 
needs an external iso whereas the current test doesn't need any additional 
images but it did not catch problems with IRQ and neither this problem 
with TCG temps. This problem was also found with riscv and mips I think 
but don't know if those would be easier to test.

Regards,
BALATON Zoltan


  parent reply	other threads:[~2021-01-19 23:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19 18:34 [PATCH 0/5] tcg: Dynamically allocate temporaries Richard Henderson
2021-01-19 18:34 ` [PATCH 1/5] tcg: Add an index to TCGTemp Richard Henderson
2021-01-19 18:34 ` [PATCH 2/5] tcg: Introduce and use tcg_temp Richard Henderson
2021-01-19 18:34 ` [PATCH 3/5] tcg: Make TCGTempSet expandable Richard Henderson
2021-01-19 18:34 ` [PATCH 4/5] tcg: Adjust tcgv_*_temp/temp_tcgv_* Richard Henderson
2021-01-19 18:34 ` [PATCH 5/5] tcg: Dynamically allocate temporaries Richard Henderson
2021-01-19 23:06 ` BALATON Zoltan [this message]
2021-01-19 23:33   ` [PATCH 0/5] " Philippe Mathieu-Daudé
2021-01-20  9:03     ` BALATON Zoltan
2021-01-21 20:09   ` BALATON Zoltan
2021-01-21 20:17     ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7595e6e-bc3d-d626-656b-e7ba3bfd8b90@eik.bme.hu \
    --to=balaton@eik.bme.hu \
    --cc=alistair23@gmail.com \
    --cc=lvivier@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).