Re: [Qemu-devel] [PATCH v2 2/5] target/alpha: Use tcg_gen_lookup_and_goto_ptr

[Philippe Mathieu-Daudé]

Peter cleaned that and improved the doc on ARM:
https://patchwork.kernel.org/patch/9690993/
(commit b636649f5a2e108413dd171edaf320f781f57942)

$ git grep -- singlestep_enabled target|wc -l
113
still confusing...

On Thu, Jun 15, 2017 at 5:55 PM, Richard Henderson <address@hidden> wrote:
> On 06/15/2017 01:48 AM, Alex Bennée wrote:
>>
>>
>> Richard Henderson <address@hidden> writes:
>>
>>> Signed-off-by: Richard Henderson <address@hidden>
>>> ---
>>>   target/alpha/translate.c | 27 ++++++++++++++++++++++-----
>>>   1 file changed, 22 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
>>> index 7c45ae3..a48e451 100644
>>> --- a/target/alpha/translate.c
>>> +++ b/target/alpha/translate.c
>>> @@ -84,6 +84,7 @@ typedef enum {
>>>          the PC (for whatever reason), so there's no need to do it again
>>> on
>>>          exiting the TB.  */
>>>       EXIT_PC_UPDATED,
>>> +    EXIT_PC_UPDATED_NOCHAIN,
>>>
>>>       /* We are exiting the TB, but have neither emitted a goto_tb, nor
>>>          updated the PC for the next instruction to be executed.  */
>>> @@ -458,11 +459,17 @@ static bool in_superpage(DisasContext *ctx, int64_t
>>> addr)
>>>   #endif
>>>   }
>>>
>>> +static bool use_exit_tb(DisasContext *ctx)
>>> +{
>>> +    return ((ctx->tb->cflags & CF_LAST_IO)
>>> +            || ctx->singlestep_enabled
>>> +            || singlestep);
>>> +}
>>
>>
>> minor nit: why start testing this global? At the least we should
>> probably seed ctx->singlestep_enabled when we set up for translation.
>>
>>> +
>>>   static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
>>>   {
>>>       /* Suppress goto_tb in the case of single-steping and IO.  */
>>> -    if ((ctx->tb->cflags & CF_LAST_IO)
>>> -        || ctx->singlestep_enabled || singlestep) {
>
>
> I didn't start testing this global.  It's already there.
>
> Further, despite the name similarity these are very different conditions.
>
> For ctx->singlestep_enabled we emit a debug exception at the end of every
> instruction.
>
> For singlestep, we must execute only one insn in the TB and further we must
> return to the main loop after the TB.  Both are required for -singlestep -d
> cpu to log all that's being requested.
>
> One cannot combine the two conditions.
>
>
> r~
>