[PATCH v3 2/2] RISC-V: Better support for long instructions (assembler)
Jan Beulich
jbeulich@suse.com
Fri Nov 25 09:56:26 GMT 2022
On 25.11.2022 10:18, Tsukasa OI wrote:
> On 2022/11/25 18:04, Jan Beulich wrote:
>> On 25.11.2022 09:39, Tsukasa OI wrote:
>>> On 2022/11/25 17:15, Jan Beulich wrote:
>>>> On 25.11.2022 03:17, Tsukasa OI wrote:
>>>>> --- a/gas/testsuite/gas/riscv/insn-na.d
>>>>> +++ b/gas/testsuite/gas/riscv/insn-na.d
>>>>> @@ -73,3 +73,11 @@ Disassembly of section .text:
>>>>> [^:]+:[ ]+007f 0000 0000 0000 0000[ ]+[._a-z].*
>>>>> [^:]+:[ ]+0000107f 00000000 00000000[ ]+[._a-z].*
>>>>> [^:]+:[ ]+607f 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000[ ]+[._a-z].*
>>>>> +[^:]+:[ ]+007f 0000 0000 0000 8000[ ]+\.byte[ ]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
>>>>> +[^:]+:[ ]+007f 0000 0000 0000 8000[ ]+\.byte[ ]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
>>>>> +[^:]+:[ ]+607f 89ab 4567 0123 3210 7654 ba98 fedc 0000 0000 0000[ ]+\.byte[ ]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>>>
>>>> I have to admit that I still don't see what good the ".byte ..." part of
>>>> the expectations does for the purpose of the test. In the cover letter
>>>> you say "They are not 4-byte aligned (10 and 22-bytes) and unlikely to
>>>> change any time soon." But changing that is exactly my plan (unless
>>>> objected to by the arch maintainers): Showing insn components as bytes
>>>> is imo reasonable for RISC-V at most when things aren't even 2-byte
>>>> aligned. IOW I'd see these to be "disassembled" to ".2byte ...",
>>>> matching the "raw opcode" output left to .<N>byte. In fact when raw
>>>> opcodes are output I question the need for any .<N>byte - it's fully
>>>> redundant>
>>>> Bottom line: As before I'd prefer if these parts were dropped (to limit
>>>> the churn on the files when changing the .<N>byte granularity), but I'm
>>>> not going to insist. Apart from this the change looks good to me.
>>>
>>> Okay, I have to admit that I misunderstood your intent.
>>>
>>> Quoting my PATCH v1 cover letter:
>>>
>>>> [Disassembler: Instruction is trimmed with 64-bits]
>>>>
>>>> In this section, we reuse the object file generated by the section above
>>>> (two 22-byte instructions).
>>>>
>>>> 0000000000000000 <.text>:
>>>> 0: 607f 0000 0000 0000 .byte 0x7f, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>>> 8: 0000 0000 0000 0000
>>>> 10: 0000 0000 0000
>>>> 16: 607f 33cc 55aa cdef .byte 0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>>> 1e: 89ab 4567 0123 3210 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> 26: 7654 ba98 fedc zeroed out after first 64-bits
>>>>
>>>> See ".byte" at the address 0x16. It's trimmed at 64-bits.
>>>> The resolution is simple. If we simply add a char* argument (containing all
>>>> instruction bits), we can easily resolve this.
>>>>
>>>> 0000000000000000 <.text>:
>>>> 0: 607f 0000 0000 0000 .byte 0x7f, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>>> 8: 0000 0000 0000 0000
>>>> 10: 0000 0000 0000
>>>> 16: 607f 33cc 55aa cdef .byte 0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe
>>>> 1e: 89ab 4567 0123 3210 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> 26: 7654 ba98 fedc all instruction bits are correct
>>>
>>> At least, I want to test whether disassembly of this part (after first
>>> 64-bits of an instruction) is fixed. That is exactly what's fixed in
>>> PATCH 1/2 and I want to make sure that this part is changed correctly.
>>
>> Which you already achieve by the raw opcode output
>>
>> 607f 89ab 4567 0123 3210 7654 ba98 fedc 0000 0000 0000
>>
>> The subsequent
>>
>> .byte[ ]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>
>> does not check anything the first part didn't already check.
>
> That's simply not true.
>
> Again, quoting from PATCH v1 cover letter (BEFORE THE PATCH
> [disassembler part]):
>
>> 16: 607f 33cc 55aa cdef .byte 0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>> 1e: 89ab 4567 0123 3210 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> 26: 7654 ba98 fedc zeroed out after first 64-bits
>
> Compare hexdump and printed bytes (in ".byte"). You can see that "1e:
> 89ab..." are different from corresponding ".byte" (0x00, 0x00...).
> They are completely separate and PATCH 1/2 only changes ".byte" output.
>
> If ".byte[ ]+0x7f, 0x60..." does not check anything the first part
> didn't already check, the dump would look like this:
>
>> 16: 607f 33cc 55aa cdef .byte 0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>> 1e: 0000 0000 0000 0000 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> 26: 0000 0000 0000 zeroed out after first 64-bits (but matches to hexdump)
>
> If the hexdump and ".byte" output always matches EVEN BEFORE THE FIX,
> that's what I can call "redundant". But in reality, they do not.
> That's why I want to leave a test to make sure that the issue is fixed
> (now hexdump and ".byte" output always matches).
Oh, apologies: Disassembly then was wrong _only_ for the .byte part, but
_not_ for the raw encoding? While indeed (going back) you said so in the
original cover letter, the description of patch 1 doesn't make this
clear. In that case, yes, I agree that the .byte part wants to be part
of the expectations (but patch 1's description also wants adjusting).
Jan
More information about the Binutils
mailing list