Re: [Qemu-arm] [PATCH 6/7] arm: Implement M profile exception return properly

[Peter Maydell]

On 10 April 2017 at 14:52, Philippe Mathieu-Daudé <address@hidden> wrote:
> On 04/10/2017 07:39 AM, Peter Maydell wrote:

>> +    /* Is the new PC value in the magic range indicating exception
>> return? */
>> +    tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], 0xff000000,
>> excret_label);
>
>
> Idea for a GSoC: branch prediction hints!
>
> tcg_gen_brcondi_i32(TCG_COND_GEU | TCG_COND_UNLIKELY, ...

Isn't the advice for modern CPUs to just trust the branch
predictor?

>> @@ -11092,9 +11137,10 @@ static void disas_thumb_insn(CPUARMState *env,
>> DisasContext *s)
>>                      tmp2 = tcg_temp_new_i32();
>>                      tcg_gen_movi_i32(tmp2, val);
>>                      store_reg(s, 14, tmp2);
>> +                    gen_bx(s, tmp);
>> +                } else {
>> +                    gen_bx_excret(s, tmp);
>
>
> This change was not easy to understand, can you add a one line comment?

Sure, but what did you have in mind? Maybe
    /* Only bx can be an exception-return, not blx */
?

>>                  }
>> -                /* already thumb, no need to check */
>> -                gen_bx(s, tmp);
>>                  break;
>>              }
>>              break;
>> @@ -11989,7 +12035,14 @@ void gen_intermediate_code(CPUARMState *env,
>> TranslationBlock *tb)
>>         instruction was a conditional branch or trap, and the PC has
>>         already been written.  */
>>      gen_set_condexec(dc);
>> -    if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
>> +    if (dc->is_jmp == DISAS_BX_EXCRET) {
>> +        /* Exception return branches need some special case code at the
>> +         * end of the TB, which is complex enough that it has to
>> +         * handle the single-step vs not and the condition-failed
>> +         * insn codepath itself.
>> +         */
>> +        gen_bx_excret_final_code(dc);
>> +    } else if (unlikely(cs->singlestep_enabled || dc->ss_active)) {
>>          /* Unconditional and "condition passed" instruction codepath. */
>>          switch (dc->is_jmp) {
>>          case DISAS_SWI:
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

thanks
-- PMM