Re: [PATCH] gas/RISC-V: adjust assembler for opcode table re-ordering

[Andrew Waterman <andrew@sifive.com>]

On Thu, Jan 12, 2023 at 12:26 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.01.2023 02:28, Maciej W. Rozycki wrote:
> > On Wed, 11 Jan 2023, Jan Beulich wrote:
> >
> >>>  And it does appear to happen, because correct machine code is produced
> >>> regardless of your hack, except for the spurious symbol produced.  So is
> >>> it not the case that simply the state (interal relocations recorded) is
> >>> not correctly reset on an unsuccessful operand match?  Why does it have to
> >>> be special-cased just for the `a' operand type?
> >>
> >> The parsing of an 'a' type operand involves expression(), a side effect of
> >> which is to insert a symbol table entry for symbols not otherwise
> >> recognized (and note how my_getSmallExpression() addresses the same issue
> >> by filtering out GPR names first [1]). Yes, in a way this is an
> >> "insufficient undoing" issue, just that undoing of that symbol table
> >> insertion would be quite hard and/or fragile (from all I can tell). And
> >> this is where the dual meaning of symbol names comes into play: This looks
> >> to be intentional, and hence we can't make use of md_parse_name() to
> >> suppress the symbol table insertion in the first place for symbols which
> >> (in other contexts) identify registers.
> >
> >  Thank you for looking into it.  Indeed it looks to me like a problem with
> > `expression' (or `expr' really) and the way the RISC-V assembly dialect
> > defines register references (unlike the MIPS one which uses a `$' prefix).
> >
> >  At a glance it seems to me that the correct approach would be to define a
> > "dry run" mode for `expr' and use it in the RISC-V backend to validate an
> > operand in the first invocation without causing any side effects, and then
> > only once all the operands have been processed and an opcode table entry
> > accepted `expr' would be called to finalise the expression.
> >
> >  I realise it's something you may not be willing to commit to, as it's
> > likely a larger task than a random tweak to the RISC-V backend, but I
> > think it's the way we ought to do it rather than piling up workarounds.
>
> I might actually try to do something along those lines, but only once it was
> clarified (by the arch maintainers) that the present behavior of identifiers
> meaning different things depending on context is actually intentional.

I haven't been following this discussion until now, but if I
understand the question correctly, then yes, it is intentional.  Were
we to travel back in time, we would have defined a different assembly
syntax that sidestepped this complexity.  But it is now part of an API
that is in widespread use, so we are stuck with it.

> There not being a prefix to indicate registers isn't unprecedented, after
> all - at least x86 (Intel syntax, or more generally "noprefix" mode), ia64,
> and Arm permit the same. The former two take the identifier as a register
> regardless of which insn this is an operand of (creating another problem
> when you really mean a symbol of that name, with varying approaches to
> dealing with). Arm instead makes sure that different mnemonics are used
> (b vs bx for Arm32, b vs br for Arm64) and hence ambiguities cannot arise.
>
> Jan