Re: [Qemu-devel] Re: [PATCH][RFC] Run time TCGv size check for debugging

[Blue Swirl]

On 10/30/08, Fabrice Bellard <address@hidden> wrote:
> Paul Brook wrote:
>  > On Wednesday 29 October 2008, Fabrice Bellard wrote:
>  >> Blue Swirl wrote:
>  >>> Hi,
>  >>>
>  >>> When emulating a mixed 32/64 bit Qemu target CPUs it's easy to confuse
>  >>> the TCGv size, passing 32 bit TCGv to a function expecting a 64 bit
>  >>> one and vice versa. This patch adds a run time sanity check for TCGv
>  >>> sizes.
>  >>>
>  >>> Because a 32 bit Qemu host does not really use 64 bit TCGvs, the patch
>  >>> is only functional on a 64 bit host. Of course also a pure 32 bit Qemu
>  >>> target is not likely to suffer from TCGv size confusion.
>  >>>
>  >>> Some use cases are not covered yet. Comments?
>  >> Theses tests can be done at compile time by introducing the TCGv_i32 and
>  >> TCGv_i64 types. The same can be done with the helpers by using a few
>  >> macros to declare them.
>  >
>  > That would also require updating all the target code in translate.c to use
>  > these types.  In principle there's no reason why this couldn't be done, but
>  > it'd be a much more invasive change.
>
>
> If you define TCGv as the word size of the emulated CPU, it will
>  eliminates most of the changes.

This version introduces TCGv_i32 and TCGv_i64. TCGv_ptr and TCGv (TL
sized) are based on them.

For Sparc, the patch is very invasive (I just commented out the
helpers to avoid that part)  but I think i386 would need much smaller
changes.

With the patch, I found some bugs in Sparc translation. I'm not sure
what to do with helpers, there should be a way to declare the size of
the arguments somehow and then the calling should be easier than:
tcg_gen_helper_1_4_i64_tl_i32_i32_i32(helper_ld_asi, dst, addr, r_asi,
r_size, r_sign);

Otherwise, I think only some variant of the TCGV_LOW parts are worth
committing, they make the code slightly more easy to understand.

compile_time_tcgv_size_check.diff.bz2
Description: BZip2 compressed data

sparc_tcgv_size_fixes.diff
Description: plain/text