diff mbox series

gas: bpf: new test for MOV with C-like numbers ll suffix

Message ID 20231030150212.21445-1-jose.marchesi@oracle.com
State New
Headers
Series gas: bpf: new test for MOV with C-like numbers ll suffix |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_check--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 warning Patch is already merged

Commit Message

Jose E. Marchesi Oct. 30, 2023, 3:02 p.m. UTC 
  The BPF pseudo-c syntax supports both MOV and LDDW instructions:

    mov:  r1 = EXPR
    lddw: r1 = EXPR ll

Note that the white space between EXPR and `ll' is necessary in order
to avoid ambiguity with the assembler's support for C-like numerical
suffixes.  This patch adds a new test to the GAS BPF testsuite to make
sure that instructions like:

    r1 = 666ll

are interpreted as `mov %r1,666', not as `lddw %r1,666'.

This matches clang's assembler behavior.

2023-10-30  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* testsuite/gas/bpf/alu-pseudoc.s: Add test to make sure C-like
	suffix `ll' is not interpreted as lddw syntax.
	* testsuite/gas/bpf/alu-pseudoc.d: Update expected results.
	* testsuite/gas/bpf/alu-be-pseudoc.d: Likewise.
---
 gas/ChangeLog                          | 7 +++++++
 gas/testsuite/gas/bpf/alu-be-pseudoc.d | 1 +
 gas/testsuite/gas/bpf/alu-pseudoc.d    | 1 +
 gas/testsuite/gas/bpf/alu-pseudoc.s    | 2 ++
 4 files changed, 11 insertions(+)

Comments

Jose E. Marchesi Oct. 30, 2023, 3:06 p.m. UTC | #1 
This is pushed.

> The BPF pseudo-c syntax supports both MOV and LDDW instructions:
>
>     mov:  r1 = EXPR
>     lddw: r1 = EXPR ll
>
> Note that the white space between EXPR and `ll' is necessary in order
> to avoid ambiguity with the assembler's support for C-like numerical
> suffixes.  This patch adds a new test to the GAS BPF testsuite to make
> sure that instructions like:
>
>     r1 = 666ll
>
> are interpreted as `mov %r1,666', not as `lddw %r1,666'.
>
> This matches clang's assembler behavior.
>
> 2023-10-30  Jose E. Marchesi  <jose.marchesi@oracle.com>
>
> 	* testsuite/gas/bpf/alu-pseudoc.s: Add test to make sure C-like
> 	suffix `ll' is not interpreted as lddw syntax.
> 	* testsuite/gas/bpf/alu-pseudoc.d: Update expected results.
> 	* testsuite/gas/bpf/alu-be-pseudoc.d: Likewise.
> ---
>  gas/ChangeLog                          | 7 +++++++
>  gas/testsuite/gas/bpf/alu-be-pseudoc.d | 1 +
>  gas/testsuite/gas/bpf/alu-pseudoc.d    | 1 +
>  gas/testsuite/gas/bpf/alu-pseudoc.s    | 2 ++
>  4 files changed, 11 insertions(+)
>
> diff --git a/gas/ChangeLog b/gas/ChangeLog
> index fef3c248196..230a4a73039 100644
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,10 @@
> +2023-10-30  Jose E. Marchesi  <jose.marchesi@oracle.com>
> +
> +	* testsuite/gas/bpf/alu-pseudoc.s: Add test to make sure C-like
> +	suffix `ll' is not interpreted as lddw syntax.
> +	* testsuite/gas/bpf/alu-pseudoc.d: Update expected results.
> +	* testsuite/gas/bpf/alu-be-pseudoc.d: Likewise.
> +
>  2023-10-02  Nick Clifton  <nickc@redhat.com>
>  
>  	PR 30861
> diff --git a/gas/testsuite/gas/bpf/alu-be-pseudoc.d b/gas/testsuite/gas/bpf/alu-be-pseudoc.d
> index 4c0f6ba9ca6..80d167c46ff 100644
> --- a/gas/testsuite/gas/bpf/alu-be-pseudoc.d
> +++ b/gas/testsuite/gas/bpf/alu-be-pseudoc.d
> @@ -69,3 +69,4 @@ Disassembly of section .text:
>   1d0:	d7 10 00 00 00 00 00 10 	r1 = bswap16 r1
>   1d8:	d7 20 00 00 00 00 00 20 	r2 = bswap32 r2
>   1e0:	d7 30 00 00 00 00 00 40 	r3 = bswap64 r3
> + 1e8:	b7 20 00 00 00 00 02 9a 	r2=0x29a
> diff --git a/gas/testsuite/gas/bpf/alu-pseudoc.d b/gas/testsuite/gas/bpf/alu-pseudoc.d
> index b5ab569563c..ea08c9a10da 100644
> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
> @@ -69,3 +69,4 @@ Disassembly of section .text:
>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
> \ No newline at end of file
> diff --git a/gas/testsuite/gas/bpf/alu-pseudoc.s b/gas/testsuite/gas/bpf/alu-pseudoc.s
> index 323a6522f22..3d60d4f7a77 100644
> --- a/gas/testsuite/gas/bpf/alu-pseudoc.s
> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.s
> @@ -61,3 +61,5 @@
>          r1 = bswap16 r1
>          r2 = bswap32 r2
>          r3 = bswap64 r3
> +        ;; The following is to be interpreted as a mov, not lddw.
> +        r2 = 666ll
Jan Beulich Oct. 30, 2023, 3:16 p.m. UTC | #2 
On 30.10.2023 16:02, Jose E. Marchesi wrote:
> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
> @@ -69,3 +69,4 @@ Disassembly of section .text:
>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
> \ No newline at end of file

Just as a remark - issues like this would be really nice to be taken
care of as files are touched. Imo much better than waiting for a
separate cleanup patch.

Jan
Jose E. Marchesi Oct. 30, 2023, 5:34 p.m. UTC | #3 
> On 30.10.2023 16:02, Jose E. Marchesi wrote:
>> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
>> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
>> @@ -69,3 +69,4 @@ Disassembly of section .text:
>>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
>> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
>> \ No newline at end of file
>
> Just as a remark - issues like this would be really nice to be taken
> care of as files are touched. Imo much better than waiting for a
> separate cleanup patch.

Your point being?
Jan Beulich Oct. 31, 2023, 7:30 a.m. UTC | #4 
On 30.10.2023 18:34, Jose E. Marchesi wrote:
> 
>> On 30.10.2023 16:02, Jose E. Marchesi wrote:
>>> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
>>> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
>>> @@ -69,3 +69,4 @@ Disassembly of section .text:
>>>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>>>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>>>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
>>> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
>>> \ No newline at end of file
>>
>> Just as a remark - issues like this would be really nice to be taken
>> care of as files are touched. Imo much better than waiting for a
>> separate cleanup patch.
> 
> Your point being?

"No newline at end of file" is something that shouldn't happen in any
source files. Therefore when changing adjacent code it would be nice
if the missing newline was inserted at the same time.

Jan
Jose E. Marchesi Oct. 31, 2023, 10:12 a.m. UTC | #5 
> On 30.10.2023 18:34, Jose E. Marchesi wrote:
>> 
>>> On 30.10.2023 16:02, Jose E. Marchesi wrote:
>>>> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
>>>> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
>>>> @@ -69,3 +69,4 @@ Disassembly of section .text:
>>>>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>>>>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>>>>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
>>>> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
>>>> \ No newline at end of file
>>>
>>> Just as a remark - issues like this would be really nice to be taken
>>> care of as files are touched. Imo much better than waiting for a
>>> separate cleanup patch.
>> 
>> Your point being?
>
> "No newline at end of file" is something that shouldn't happen in any
> source files. Therefore when changing adjacent code it would be nice
> if the missing newline was inserted at the same time.

Yes, agreed.  In all candour, in this case I didn't even notice it
before pushing (was in a rush) or i would have done it.

Would you say the same applies to typos and other trivial fixes?
Usually I refrain from fixing these that I happen to notice while
working on something else, unless they are very coupled with the change
(like a typo in a comment describing the code I am changing) even if it
is very tempting to do otherwise.  I guess it is a question of degree.
Jan Beulich Oct. 31, 2023, 10:25 a.m. UTC | #6 
On 31.10.2023 11:12, Jose E. Marchesi wrote:
> 
>> On 30.10.2023 18:34, Jose E. Marchesi wrote:
>>>
>>>> On 30.10.2023 16:02, Jose E. Marchesi wrote:
>>>>> --- a/gas/testsuite/gas/bpf/alu-pseudoc.d
>>>>> +++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
>>>>> @@ -69,3 +69,4 @@ Disassembly of section .text:
>>>>>   1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
>>>>>   1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
>>>>>   1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
>>>>> + 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
>>>>> \ No newline at end of file
>>>>
>>>> Just as a remark - issues like this would be really nice to be taken
>>>> care of as files are touched. Imo much better than waiting for a
>>>> separate cleanup patch.
>>>
>>> Your point being?
>>
>> "No newline at end of file" is something that shouldn't happen in any
>> source files. Therefore when changing adjacent code it would be nice
>> if the missing newline was inserted at the same time.
> 
> Yes, agreed.  In all candour, in this case I didn't even notice it
> before pushing (was in a rush) or i would have done it.
> 
> Would you say the same applies to typos and other trivial fixes?
> Usually I refrain from fixing these that I happen to notice while
> working on something else, unless they are very coupled with the change
> (like a typo in a comment describing the code I am changing) even if it
> is very tempting to do otherwise.  I guess it is a question of degree.

My rule of thumb is that anything that's touched anyway can also have
cosmetic corrections alongside the main intended adjustment(s).

Jan
diff mbox series

Patch

diff --git a/gas/ChangeLog b/gas/ChangeLog
index fef3c248196..230a4a73039 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,10 @@ 
+2023-10-30  Jose E. Marchesi  <jose.marchesi@oracle.com>
+
+	* testsuite/gas/bpf/alu-pseudoc.s: Add test to make sure C-like
+	suffix `ll' is not interpreted as lddw syntax.
+	* testsuite/gas/bpf/alu-pseudoc.d: Update expected results.
+	* testsuite/gas/bpf/alu-be-pseudoc.d: Likewise.
+
 2023-10-02  Nick Clifton  <nickc@redhat.com>
 
 	PR 30861
diff --git a/gas/testsuite/gas/bpf/alu-be-pseudoc.d b/gas/testsuite/gas/bpf/alu-be-pseudoc.d
index 4c0f6ba9ca6..80d167c46ff 100644
--- a/gas/testsuite/gas/bpf/alu-be-pseudoc.d
+++ b/gas/testsuite/gas/bpf/alu-be-pseudoc.d
@@ -69,3 +69,4 @@  Disassembly of section .text:
  1d0:	d7 10 00 00 00 00 00 10 	r1 = bswap16 r1
  1d8:	d7 20 00 00 00 00 00 20 	r2 = bswap32 r2
  1e0:	d7 30 00 00 00 00 00 40 	r3 = bswap64 r3
+ 1e8:	b7 20 00 00 00 00 02 9a 	r2=0x29a
diff --git a/gas/testsuite/gas/bpf/alu-pseudoc.d b/gas/testsuite/gas/bpf/alu-pseudoc.d
index b5ab569563c..ea08c9a10da 100644
--- a/gas/testsuite/gas/bpf/alu-pseudoc.d
+++ b/gas/testsuite/gas/bpf/alu-pseudoc.d
@@ -69,3 +69,4 @@  Disassembly of section .text:
  1d0:	d7 01 00 00 10 00 00 00 	r1 = bswap16 r1
  1d8:	d7 02 00 00 20 00 00 00 	r2 = bswap32 r2
  1e0:	d7 03 00 00 40 00 00 00 	r3 = bswap64 r3
+ 1e8:	b7 02 00 00 9a 02 00 00 	r2=0x29a
\ No newline at end of file
diff --git a/gas/testsuite/gas/bpf/alu-pseudoc.s b/gas/testsuite/gas/bpf/alu-pseudoc.s
index 323a6522f22..3d60d4f7a77 100644
--- a/gas/testsuite/gas/bpf/alu-pseudoc.s
+++ b/gas/testsuite/gas/bpf/alu-pseudoc.s
@@ -61,3 +61,5 @@ 
         r1 = bswap16 r1
         r2 = bswap32 r2
         r3 = bswap64 r3
+        ;; The following is to be interpreted as a mov, not lddw.
+        r2 = 666ll