Re: [Qemu-arm] [PATCH v2 18/68] target/arm: Convert the rest of A32 Miscelaneous instructions

[Richard Henderson]

On 8/27/19 3:32 AM, Peter Maydell wrote:
>> +static bool trans_HVC(DisasContext *s, arg_HVC *a)
>> +{
>> +    if (!ENABLE_ARCH_7 || IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
>> +        return false;
>> +    }
>> +    gen_hvc(s, a->imm);
>> +    return true;
>> +}
> 
> I was wondering about for these trans_ functions the
> difference between returning 'false' and calling
> unallocated_encoding() and then returning 'true'.
> If I understand the decodetree right this will only
> make a difference when the pattern is inside a {} group.

Correct.

> So for instance here we have
> 
> {
>   [...]
>   {
>     HVC          1111 0111 1110 ....  1000 .... .... ....     \
>                  &i imm=%imm16_16_0
>     CPS          1111 0011 1010 1111 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \
>                  &cps
>     UDF          1111 0111 1111 ----  1010 ---- ---- ----
>   }
>   B_cond_thumb   1111 0. cond:4 ...... 10.0 ............      &ci imm=%imm21
> }
> 
> which means that if the HVC returns 'false' we'll end up
> trying the insn as a B_cond_thumb.

Correct.

> In this case the
> trans function for the B_cond_thumb pattern will correctly
> return false itself for a->cond >= 0xe, so it makes no
> difference. But maybe it would be more robust for a pattern
> like HVC to be self-contained so it doesn't fall through
> for cases that really do belong to it but happen to be
> required to UNDEF (like IS_USER() == true) ?

I agree this should be the rule.

E.g. for this IS_USER case, we have successfully decoded HVC and so should not
return false.  The fact that HVC should raise SIGILL if IS_USER should not be
confused with decoding HVC.

Other constraints, such as rd != 15 or imod != 0, should continue to return
false so that a (potential) grouped insn can match.

> OTOH I suppose you could say that when you're writing patterns
> like the B_cond_thumb one you know you've underdecoded and must
> catch all the theoretical overlaps by writing checks in the trans
> function, so as long as you do that correctly you're fine.

Yes.


r~