Re: [Qemu-devel] [PATCH 11/13] tcg: sync globals for pure helpers instead of saving them

[Richard Henderson]

On 09/27/2012 10:15 AM, Aurelien Jarno wrote:
> Note: this is a change of behavior, but the new one is still compliant
> with the documentation. Currently only cris, i386 and s390x have PURE
> only helpers. I have checked they are really PURE, and also did some 
> tests on cris and i386.
> 
> diff --git a/tcg/README b/tcg/README
> index 27846f1..d61daa1 100644
> --- a/tcg/README
> +++ b/tcg/README
> @@ -77,11 +77,16 @@ destroyed, but local temporaries and globals are 
> preserved.
>  Using the tcg_gen_helper_x_y it is possible to call any function
>  taking i32, i64 or pointer types. By default, before calling a helper,
>  all globals are stored at their canonical location and it is assumed
> -that the function can modify them. This can be overridden by the
> -TCG_CALL_CONST function modifier. By default, the helper is allowed to
> -modify the CPU state or raise an exception. This can be overridden by
> -the TCG_CALL_PURE function modifier, in which case the call to the
> -function is removed if the return value is not used.
> +that the function can modify them. The helper is allowed to modify
> +the CPU state or raise an exception.
> +
> +This can be overridden by the TCG_CALL_CONST and TCG_CALL_PURE function
> +modifiers. A PURE helper can read globals but cannot modify them nor the
> +CPU state and cannot raise an exception. It can be removed if the return
> +value is not used. For that the globals are synchronized with their canonical
> +location, but the associated registers are not freed nor reloaded afterwise.
> +A CONST helper is a PURE helper which in addition cannot read globals, they
> +are not synchronized nor stored. Note that CONST implies PURE.

If we're going to re-org these flags, lets please do it right.

In particular the terms "const" and "pure" were clearly stolen from gcc,
but then given different meanings (!).  This, to me at least, is incredibly
confusing.  I have to go back and re-read the docs every time I touch
one of the helper declarations.

There are really three flags we care about:

(1) Helper may read globals.  Sometimes indirectly via exception.
    Implies that globals must be synced, but may remain in REG/CONST state.

(2) Helper may write globals.  Implies globals must be synced, and
    returned to MEM state.

(3) Helper has no side effects at all.  Implies that it can be deleted if
    its outputs are dead.

For the sake of discussion, lets call these READG, WRITEG, NOSIDE.

Our current default is READG | WRITEG.

Our current definition of PURE is READG | NOSIDE.  Note that the gcc
definition of pure actually talks about main memory.

Our current definition of CONST is none of these bits.  Note that the
gcc definition of const is a superset of pure.

What we'd like for all fp helpers is READG only.  Something that we
cannot get at the moment.

There are several cases in which the helper needs to return more than
64 bits in which we use a "temp" allocated in the env struct.  Sparc
does this for some of its 128-bit arithmetic; I'm planning to do just
the same in my s390x rewrite.  For this, the helper neither reads nor
writes globals, but it has non-global-register side effects.  There
will generally be a post-helper load from env to get the "extra" bits.
While the "true" output of the helper may be dead, the side load from
env may not be.  So we want none of READG | WRITEG | NOSIDE.

If we do reorg, we can certainly add compatibility defines so that we
need not update all translators at once.


r~