Re: [PATCH v5 23/55] translator: add translator_ld{ub,sw,uw,l,q}

[Alex Bennée]

Peter Maydell <address@hidden> writes:

> On Mon, 14 Oct 2019 at 12:38, Alex Bennée <address@hidden> wrote:
>>
>> From: "Emilio G. Cota" <address@hidden>
>>
>> We don't bother with replicating the fast path (tlb_hit) of the old
>> cpu_ldst helpers as it has no measurable effect on performance. This
>> probably indicates we should consider flattening the whole set of
>> helpers but that is out of scope for this change.
>>
>> Suggested-by: Richard Henderson <address@hidden>
>> Signed-off-by: Emilio G. Cota <address@hidden>
>> [AJB: directly plumb into softmmu/user helpers]
>> Signed-off-by: Alex Bennée <address@hidden>
>>
>> diff --git a/tcg/tcg.h b/tcg/tcg.h
>> index a38659ea5b..302533b463 100644
>> --- a/tcg/tcg.h
>> +++ b/tcg/tcg.h
>> @@ -1317,6 +1317,7 @@ uint64_t helper_be_ldq_cmmu(CPUArchState *env, 
>> target_ulong addr,
>>  # define helper_ret_stl_mmu   helper_be_stl_mmu
>>  # define helper_ret_stq_mmu   helper_be_stq_mmu
>>  # define helper_ret_ldw_cmmu  helper_be_ldw_cmmu
>> +# define helper_ret_lduw_cmmu helper_be_ldw_cmmu
>>  # define helper_ret_ldl_cmmu  helper_be_ldl_cmmu
>>  # define helper_ret_ldq_cmmu  helper_be_ldq_cmmu
>>  #else
>> @@ -1330,6 +1331,7 @@ uint64_t helper_be_ldq_cmmu(CPUArchState *env, 
>> target_ulong addr,
>>  # define helper_ret_stl_mmu   helper_le_stl_mmu
>>  # define helper_ret_stq_mmu   helper_le_stq_mmu
>>  # define helper_ret_ldw_cmmu  helper_le_ldw_cmmu
>> +# define helper_ret_lduw_cmmu helper_le_ldw_cmmu
>>  # define helper_ret_ldl_cmmu  helper_le_ldl_cmmu
>>  # define helper_ret_ldq_cmmu  helper_le_ldq_cmmu
>>  #endif
>
> This looks odd. Why is it ok to define a 'lduw' helper
> as the 'ldw' cmmu helper ? One ought to be sign
> extending and the other not...

This was attempting to make things line up between the softmmu helpers
and the user-mode ld*_p helpers that we need to expand to. I'm not sure
a sign extending loader even makes sense for code load anyway.

>
> thanks
> -- PMM


--
Alex Bennée