[PATCH 19/19] RISC-V/gas: .attribute vs .insn

[Jiawei]

> "... before any instruction", as the diagnostic from s_riscv_attribute()
> says, presumably ought to include also insns resulting from .insn. Make a
> small helper function.
>
Treating .insn as starting assembly looks right to me.  If .attribute 
must be
set before any instruction, then instructions emitted through .insn should
follow the same rule as normal instructions.
>
> ---
> RFC: I can't quite figure why this constraint exists, so there may also be
>      the option of relaxing things (up to fully dropping start_assemble).
>
> As to other uses of .attribute, shouldn't redundant conflicting attribute
> values at least be warned about?
> For stack_align,
> - shouldn't the smallest non-zero value be used?
> - shouldn't non-power-of-2 values at least be warned about?
> For unaligned_access,
> - shouldn't values other than 0/1 be warned about?
>
Regarding the RFC question, I think keeping the current restriction is safer
for now.  The architecture and privileged attributes describe file-level
properties, while instruction parsing, ABI selection, mapping symbols and
other state may already have been affected once assembly has started.  So
relaxing or removing start_assemble would need a wider audit and should
probably be handled separately from this consistency fix.

For the other attributes, I agree that there is room for follow-up
improvement. Repeated .attribute directives with conflicting values could at
least deserve a warning, and known-value attributes such as unaligned_access
should probably reject or warn for values outside the defined range. A
non-power-of-two stack_align value also looks suspicious enough to diagnose.

I am less sure about automatically using the smallest non-zero stack_align
value, though. Since stack alignment affects ABI compatibility, the current
mismatch diagnostic seems safer unless the psABI explicitly defines a merge
rule for such cases. This may also be worth raising discussion in the 
psABI TG.

> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -4411,6 +4411,18 @@ riscv_ip_hardcode (char *str,
>    return NULL;
>  }
>
> +/* The architecture and privileged elf attributes should be set before
> +   assembling.  */
> +static bool
> +start_assembly(void)
>
One tiny formatting nit:

   start_assembly (void)
>
> +{
> +  start_assemble = true;
> +
> +  riscv_set_abi_by_arch ();
> +
> +  return riscv_set_default_priv_spec (NULL);
> +}
> +
>  void
>  md_assemble (char *str)
>  {
> @@ -4418,16 +4430,8 @@ md_assemble (char *str)
>    expressionS imm_expr;
>    bfd_reloc_code_real_type imm_reloc = BFD_RELOC_UNUSED;
>
> -  /* The architecture and privileged elf attributes should be set
> -     before assembling.  */
> -  if (!start_assemble)
> -    {
> -      start_assemble = true;
> -
> -      riscv_set_abi_by_arch ();
> -      if (!riscv_set_default_priv_spec (NULL))
> -       return;
> -    }
> +  if (!start_assemble && !start_assembly())
> +    return;
>
>    riscv_mapping_state (MAP_INSN, 0, false/* fr_align_code */);
>
> @@ -5636,6 +5640,9 @@ s_riscv_insn (int x ATTRIBUTE_UNUSED)
>    bfd_reloc_code_real_type imm_reloc = BFD_RELOC_UNUSED;
>    char save_c;
>
> +  if (!start_assemble && !start_assembly())
> +    return;
> +
>    while (!is_end_of_stmt (*input_line_pointer))
>      ++input_line_pointer;
>
>
Reviewed-by: Jiawei <jiawei@iscas.ac.cn>