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

Jan Beulich jbeulich@suse.com
Tue Jan 10 09:25:04 GMT 2023


On 09.01.2023 22:59, Maciej W. Rozycki wrote:
> On Mon, 9 Jan 2023, Palmer Dabbelt wrote:
> 
>>>>> 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.
>>>
>>> With 2.40 scheduled to be cut in less than a week, may I ask for an arch
>>> maintainer's view here?
>>
>> 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. Hence a solution is needed here which retains the
order, and which also respects that insn operands may be parsed
- more than once,
- with identical names having different meaning depending on context.
I realize what I'm doing is a hack, but I cannot think of anything better.

Jan


More information about the Binutils mailing list