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

[Jan Beulich]

On 10.01.2023 23:58, Maciej W. Rozycki wrote:
> On Tue, 10 Jan 2023, Jan Beulich wrote:
> 
>>>> Thanks for fixing this.  I don't have any issues with what's there, but looks
>>>> like I'm also getting some failures (glibc/multilib errno related stuff).  I'm
>>>> trying to bisect those so I can't really get a proper test up now, I'll try to
>>>> do so ASAP as it's really late to have stuff broken.
>>>
>>>  I wonder why the RISC-V port needs such a hack while the MIPS one 
>>> doesn't.
>>
>> Perhaps I misunderstood your earlier reply then. There you said you deal with
>> this by carefully ordering the opcode table. Here restoring the original order
>> would only be a temporary workaround, as it would re-introduce the
>> disassembler issue that was fixed by altering the order: Alias entries need to
>> come ahead of "real" ones, or else respective alias mnemonics would never be
>> emitted by the disassembler.
> 
>  Correct.  But why does it require such an intervention on the GAS side?
> 
>  AFAIK similarly to the MIPS port and like the disassembler GAS tries to 
> match instructions in the order they appear in the opcode table, except 
> that the disassembler matches by the opcode/mask, but GAS matches by the 
> mnemonic/operands.  It is expected that a single-operand alias appears 
> first so that the disassembler matches it first (unless `-M no-aliases'), 
> but it is also expected not to match a two-operand assembly instruction, 
> so that GAS proceeds to the next candidate opcode table entry.
> 
>  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.

As to "just" - the issue could in principle happen elsewhere as well
(beyond what my_getSmallExpression() accounts for at present), but luckily
(so far) it doesn't. Hence I thought it would be best to keep things
isolated to 'a'.

As suggested in a post-commit-message remark, another workaround might be
to count actual and expected operands first, and skip right to the next
table entry when the two numbers don't match. I didn't go this route
because I have no insight into possible future plans as to operands which
might themselves contain commas (see e.g. Arm or x86 AT&T syntax for
examples of such), at which point counting actuals would become more
complicated than just counting commas.

Jan

[1] As stated elsewhere, I think this is wrong, as I expect it breaks
certain use cases of, in particular, equates. But correcting it would
require finding another solution to the symbol table insertion issue.