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

[Aurelien Jarno <aurelien@aurel32.net>]

On 2023-01-06 13:34, Jan Beulich via Binutils wrote:
> PR gas/29940
> 
> With the single-operand JAL entry now sitting ahead of the two-operand
> one, the parsing of a two-operand insn would first try to parse an 'a'-
> style operand, resulting in the insertion of bogus (and otherwise
> unused) undefined symbols in the symbol table, having register names.
> Since 'a' is used as 1st operand only with J and JAL, and since JAL is
> the only insn _also_ allowing for a register as 1st operand (and then
> there being a 2nd one), special case this parsing aspect right there.
> ---
> This, of course, is fragile, but I guess such workarounds are
> unavoidable with the chosen approach of (recurring) parsing, and with
> register names being special only in certain contexts.
> 
> A more generic approach, then possibly also helping performance, might
> be to count the number of operands first, and do full parsing only when
> the count matches that in the operand specifier string (at least when
> there are multiple insn forms).
> 
> The similar workaround in my_getSmallExpression() actually looks
> suspicious to me: I expect that it would get in the way of using equates
> "shadowing" names of GPRs.
> 
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -3266,6 +3266,17 @@ riscv_ip (char *str, struct riscv_cl_ins
>  	      continue;
>  
>  	    case 'a': /* 20-bit PC-relative offset.  */
> +	      /* Like in my_getSmallExpression() we need to avoid emitting
> +		 a stray undefined symbol if the 1st JAL entry doesn't match,
> +		 but the 2nd (with 2 operands) might.  */
> +	      if (oparg == insn->args)
> +		{
> +		  asargStart = asarg;
> +		  if (reg_lookup (&asarg, RCLASS_GPR, NULL)
> +		      && (*asarg == ',' || (ISSPACE (*asarg) && asarg[1] == ',')))
> +		    break;
> +		  asarg = asargStart;
> +		}
>  	    jump:
>  	      my_getExpression (imm_expr, asarg);
>  	      asarg = expr_end;

Thanks for the patch. I have tested it and confirmed it fix the problem
I reported.

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net