Re: [Qemu-arm] [Qemu-devel] [PATCH v2 35/68] target/arm: Convert CPS (privileged)

[Richard Henderson]

On 8/25/19 9:20 AM, Peter Maydell wrote:
> On Mon, 19 Aug 2019 at 22:38, Richard Henderson
> <address@hidden> wrote:
>>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  target/arm/translate.c       | 87 +++++++++++++++---------------------
>>  target/arm/a32-uncond.decode |  3 ++
>>  target/arm/t32.decode        |  3 ++
>>  3 files changed, 42 insertions(+), 51 deletions(-)
>> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
>> index 18c268e712..354ad77fe6 100644
>> --- a/target/arm/t32.decode
>> +++ b/target/arm/t32.decode
>> @@ -44,6 +44,7 @@
>>  &bfi             !extern rd rn lsb msb
>>  &sat             !extern rd rn satimm imm sh
>>  &pkh             !extern rd rn rm imm tb
>> +&cps             !extern mode imod M A I F
>>
>>  # Data-processing (register)
>>
>> @@ -340,6 +341,8 @@ CLZ              1111 1010 1011 ---- 1111 .... 1000 .... 
>>      @rdm
>>      SMC          1111 0111 1111 imm:4 1000 0000 0000 0000     &i
>>      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
> 
> In T32 the CPS insn overlaps with the hint space (hint insns have
> bits [10:8] all-zeroes, whereas all the valid CPS insns have either
> M set or one of the imod bits set) -- why doesn't it need to be
> in the same insn group as the hints? If we're going to put it
> separated in the .decode file from the insns it overlaps with
> I guess a comment to that effect would help so it doesn't get
> inadvertently reordered with them.

It is grouped.  It's not immediately visible in the patch because there are a
*lot* of insns that overlap with the hints and 3 lines of context are
insufficient to see that.

But the grouping is semi-visible in the indentation here.

> CPS shouldn't exist at all for M-profile, but the legacy decoder
> got this wrong too, so we should put that on the todo list for
> fixing later (along, maybe, with UNDEFing on some of the
> unpredictable combinations of M/imod/etc for A profile?).

Fixing m-profile is just as easy as as commenting.
I'll leave a TODO for the unpredictable combinations.


r~