Re: [PATCH 07/10] target/arm: Implement v8.1M low-overhead-loop instructions

[Richard Henderson]

On 10/12/20 12:56 PM, Peter Maydell wrote:
> On Mon, 12 Oct 2020 at 16:37, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> v8.1M's "low-overhead-loop" extension has three instructions
>> for looping:
>>  * DLS (start of a do-loop)
>>  * WLS (start of a while-loop)
>>  * LE (end of a loop)
>>
>> +static bool trans_WLS(DisasContext *s, arg_WLS *a)
>> +{
>> +    /* M-profile low-overhead while-loop start */
>> +    TCGv_i32 tmp;
>> +    TCGLabel *nextlabel;
>> +
>> +    if (!dc_isar_feature(aa32_lob, s)) {
>> +        return false;
>> +    }
>> +    if (a->rn == 13 || a->rn == 15) {
>> +        /* CONSTRAINED UNPREDICTABLE: we choose to UNDEF */
>> +        return false;
>> +    }
>> +
>> +    nextlabel = gen_new_label();
>> +    tcg_gen_brcondi_i32(TCG_COND_NE, cpu_R[a->rn], 0, nextlabel);
>> +    gen_jmp(s, read_pc(s) + a->imm);
>> +
>> +    gen_set_label(nextlabel);
>> +    tmp = load_reg(s, a->rn);
>> +    store_reg(s, 14, tmp);
>> +    gen_jmp(s, s->base.pc_next);
>> +    return true;
>> +}
> 
> This turns out not to work, because gen_jmp() always generates
> a goto-tb for tb exit 0, and we hit the assert() that exit 0
> was not used twice. Here's a fixup to fold into this patch:

Indeed.  I was going to suggest that here you should use arm_gen_condlabel()
like you did for LE.  Which I think would be still cleaner than your fixup 
patch.


r~