Re: [Qemu-devel] [RFC PATCH 2/3] tcg: Add support for fence generation in x86 backend

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <rth@twiddle.net> writes:

> On 05/24/2016 10:18 AM, Pranith Kumar wrote:
>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>> ---
>>  tcg/i386/tcg-target.h     | 1 +
>>  tcg/i386/tcg-target.inc.c | 9 +++++++++
>>  tcg/tcg-opc.h             | 2 +-
>>  tcg/tcg.c                 | 1 +
>>  4 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
>> index 92be341..93ea42e 100644
>> --- a/tcg/i386/tcg-target.h
>> +++ b/tcg/i386/tcg-target.h
>> @@ -100,6 +100,7 @@ extern bool have_bmi1;
>>  #define TCG_TARGET_HAS_muls2_i32        1
>>  #define TCG_TARGET_HAS_muluh_i32        0
>>  #define TCG_TARGET_HAS_mulsh_i32        0
>> +#define TCG_TARGET_HAS_fence            1
>
> This has to be defined for all hosts.
>
> The default implementation should be a function call into tcg-runtime.c that
> calls smp_mb().

That would solves the problem of converting the various backends
piecemeal - although obviously we should move to all backends having
"native" support ASAP. However by introducing expensive substitute
functions we will slow down the translations as each front end is
expanded to translate the target barrier ops.

Should we make the emitting of the function call/TCGop conditional on
MTTCG being enabled? If we are running in round-robin mode there is no
need to issue any fence operations.

>
>> @@ -347,6 +347,7 @@ static inline int tcg_target_const_match(tcg_target_long val, TCGType type,
>>  #define OPC_SHRX        (0xf7 | P_EXT38 | P_SIMDF2)
>>  #define OPC_TESTL	(0x85)
>>  #define OPC_XCHG_ax_r32	(0x90)
>> +#define OPC_MFENCE      (0xAE | P_EXT)
>>
>>  #define OPC_GRP3_Ev	(0xf7)
>>  #define OPC_GRP5	(0xff)
>> @@ -686,6 +687,14 @@ static inline void tcg_out_pushi(TCGContext *s, tcg_target_long val)
>>      }
>>  }
>>
>> +static inline void tcg_out_fence(TCGContext *s)
>> +{
>> +    /* TODO: Figure out an appropriate place for the encoding */
>> +    tcg_out8(s, 0x0F);
>> +    tcg_out8(s, 0xAE);
>> +    tcg_out8(s, 0xF0);
>> +}
>
> Why define OPC_MFENCE if you're not going to use it?  Of course, it's not
> exactly a complete and useful definition, so maybe just delete OPC_MFENCE.
>
> Also, for 32-bit you need to check for sse2 before outputting this.  See also
> the existing cpuid checks in tcg_target_init and the fallback smp_mb definition
> for pre-gcc-4.4.
>
>
> r~


--
Alex Bennée