Re: [COMMITTED 2/3] gas: BPF pseudo-c syntax tests

[Jan Beulich <jbeulich@suse.com>]

On 26.04.2023 19:31, Jose E. Marchesi via Binutils wrote:
> --- a/gas/ChangeLog
> +++ b/gas/ChangeLog
> @@ -1,3 +1,23 @@
> +2023-04-20  Guillermo E. Martinez  <guillermo.e.martinez@oracle.com>
> +
> +	PR gas/29728
> +	* testsuite/gas/all/assign-bad-recursive.d: Skip test in bpf-*
> +	targets.
> +	* testsuite/gas/all/eqv-dot.d: Likewise.
> +	* testsuite/gas/all/gas.exp: Skip other assignment tests in bpf-*.

I view doing such as problematic. Looking at what patch 3 documents,
the uses of " = " are pretty limited, and ones not naming a register
on the lhs (or, for store forms, on the rhs) ought to be fine to
retain their meaning. Sadly there isn't an easy way to specify target-
specific flags, or else I'd be inclined to suggest that you have an
option to suppress recognition of the C-like syntax (which may be a
good idea anyway, as people might be using constructs like the ones
used in the testcases you now disable) and use it here and below.

In any event, ...

> --- a/gas/testsuite/gas/all/eqv-dot.d
> +++ b/gas/testsuite/gas/all/eqv-dot.d
> @@ -2,7 +2,7 @@
>  #name: eqv involving dot
>  # bfin doesn't support 'symbol = expression'
>  # tic30 and tic4x have 4 octets per byte, tic54x has 2 octets per byte
> -#notarget: bfin-*-* *c30-*-* *c4x-*-* *c54x-*-*
> +#notarget: bfin-*-* *c30-*-* *c4x-*-* *c54x-*-* *bpf-*-*

... at least in cases where there already are justifying comments, new
additions of exceptions shouldn't go uncommented.

> --- a/gas/testsuite/gas/bpf/alu-be.d
> +++ b/gas/testsuite/gas/bpf/alu-be.d
> @@ -1,5 +1,6 @@
>  #as: --EB
>  #source: alu.s
> +#source: alu-pseudoc.s
>  #objdump: -dr
>  #name: eBPF ALU64 instructions, big endian

I may of course be reading binutils-common.exp's run_dump_test wrong,
but is this having the intended effect of assembling each of the files
once and checking objdump output for each of them? It looks to me as
if only the assembling step would be performed for both, which I don't
think is what is wanted.

> --- a/gas/testsuite/gas/macros/macros.exp
> +++ b/gas/testsuite/gas/macros/macros.exp
> @@ -82,6 +82,7 @@ switch -glob $target_triplet {
>      rl78-*-* { }
>      rx-*-* { }
>      vax-*-* { }
> +    bpf-*-* { }
>      default { run_list_test dot "-alm" }
>  }

How is this test affected by your changes? It consists of only labels
and directives afaics, so insn syntax expectations shouldn't matter
at all.

Jan