Re: -Wtautological-bitwise-compare error in arm-tdep.c

[Luis Machado <luis.machado@linaro.org>]

On 5/26/20 6:45 AM, Alan Hayward wrote:
> 
> 
>> On 25 May 2020, at 14:51, Luis Machado <luis.machado@linaro.org> wrote:
>>
>> On 5/25/20 10:49 AM, Simon Marchi wrote:
>>> On 2020-05-25 9:08 a.m., Luis Machado wrote:
>>>> This fixes an instruction mask typo. We should be matching only
>>>> ldrd (immediate) and not any other of its variants. As is, it never matches
>>>> anything.
>>> And moreover, within the `ldrd (immediate)` instruction, it only matches the
>>> `Offset variant` variant, right?
>>
>> That's right. We don't want to handle anything that changes the SP here. And the post-indexed and pre-indexed variants do so.
>>
>>>>
>>>> With the patch, the instruction mask also allows matching of ldrd (literal),
>>>> but the check for SP discards this particular instruction pattern, as it has
>>>> a hardcoded PC register.
>>> I don't feel the most qualified to approve this patch.  Alan, could you please
>>> take a look?
>>> Simon
> 
> 
> The maths looks good now.
> 
> However, Binutils uses a slightly different mask, 0xff50:
> 
> 
>    {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2),
>      0xe9500000, 0xff500000,
>      "ldrd%c\t%12-15r, %8-11r, [%16-19r, #%23`-%0-7W]%21'!%L”},
> 
> 
> It does use 0xff70 for a different variation of ldrd:
> 
>    {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2),
>      0xe8600000, 0xff700000,
>      "strd%c\t%12-15r, %8-11r, [%16-19r], #%23`-%0-7W%L"},
>    {ARM_FEATURE_CORE_LOW (ARM_EXT_V6T2),
>      0xe8700000, 0xff700000,
>      "ldrd%c\t%12-15r, %8-11r, [%16-19r], #%23`-%0-7W%L”},
> 
> 
> That’s in binutils-gdb/opcodes/arm-dis.c.
> All that code was added at the same time in 2015.
> 
> 0xff50 is going to allow more matches than 0xff70.
> And given that the thing we care about is matching the opcode,
> then 0xff50 is safer.
> 
> Before I go off and start looking at instruction decodings,
> Luis - where did you get 0xff70 from?

 From the manual, a concatenation of 7 bits from the opcode plus 4 bits 
of op0 and 1 bit for Load/Store.

Mask 0xfe00 filters the opcode (1110100).
Mask 0x160 filters op0 (bits 0, 1 and 3 - 1011).
Mask 0x10 filters the load/store bit (load is 1).

We want to match the insn pattern 0xe950, which is ldrd (immediate).

Compared to 0xff70, with mask 0xff50 we would ignore op<0>. Ignoring 
op<0> would allow matching ldrd (immediate, pre-indexed) as well.