Re: [PATCH 1/5 v1] LoongArch: bfd: Add new reloc types.

[WANG Xuerui <i.swmail@xen0n.name>]

On 2022/7/18 19:33, Xi Ruoyao wrote:
> On Mon, 2022-07-18 at 18:06 +0800, WANG Xuerui wrote:
>> (Adding Ruoyao and MaskRay to CC, who might be interested in this
>> development as well, as it concerns the linker implementation.)
> I've already subscribed binutils@sourceware.org in order not to be
> passed by.
>
>> I think I've voiced my concerns over the naming of these ops multiple
>> times already; the primary comment ([1]; in English) was posted back in
>> May but no one in your team responded.
>>
>> Reproducing the content (and adjusting a little) here:
>>
>>
>> Overall in a good direction (and IMHO the direction everyone should have
>> taken in the first place), thanks!
>>
> Yes, we've been paying additional costs using those "stack based
> relocation".  I'd like to know why they were proposed in first place?
> (Not accusing anyone, just my curiosity: AFAIK no other targets ever
> used such a stack for relocation.)
As previously pointed out on this list, rx and rl78 both use stack-based 
relocs. (Both come from Renesas, so I do think it's delibrate design 
decision, but of course the original Renesas justification is most 
probably buried in history.) I explained this in 
https://github.com/loongson/binutils-gdb/issues/134 before.
>
> /* snip */
>
>
>> FYI, I did make a list of my suggested names for these reloc types
>> ("BFD_RELOC_LARCH_" abbreviated to "B_R_L_"):
>>
>> Original name           Suggested name
>> -------------           --------------
>> B_R_L_B16               B_R_L_PCREL_SK16 *1
>> B_R_L_B21               B_R_L_PCREL_SD5K16
>> B_R_L_B26               B_R_L_PCREL_SD10K16
>> B_R_L_ABS_LO12          B_R_L_ABS_0_SK12
>> B_R_L_ABS_HI20          B_R_L_ABS_12_SJ20
>> B_R_L_ABS64_LO20        B_R_L_ABS_32_SJ20
>> B_R_L_ABS64_HI12        B_R_L_ABS_52_SK12
>> B_R_L_PCALA_LO12        B_R_L_PCALA_0_SK12 *2
>> B_R_L_PCALA_HI20        B_R_L_PCALA_12_SJ20
>> B_R_L_PCALA64_LO20      B_R_L_PCALA_32_SJ20 *3
>> B_R_L_PCALA64_HI12      B_R_L_PCALA_52_SK12
>> B_R_L_GOT_PC_LO12       B_R_L_GOT_PCALA_0_SK12 *4
>> B_R_L_GOT_PC_HI20       B_R_L_GOT_PCALA_12_SJ20
>> B_R_L_GOT64_PC_LO20     B_R_L_GOT_PCALA_32_SJ20
>> B_R_L_GOT64_PC_HI12     B_R_L_GOT_PCALA_52_SK12
>> B_R_L_GOT64_LO12        B_R_L_GOT_ABS_0_SK12 *5
>> B_R_L_GOT64_HI20        B_R_L_GOT_ABS_12_SJ20
>> B_R_L_GOT64_LO20        B_R_L_GOT_ABS_32_SJ20
>> B_R_L_GOT64_HI12        B_R_L_GOT_ABS_52_SK12
>> B_R_L_TLS_LE_LO12       B_R_L_TLS_LE_ABS_0_SK12
>> B_R_L_TLS_LE_HI20       B_R_L_TLS_LE_ABS_12_SJ20
>> B_R_L_TLS_LE64_LO20     B_R_L_TLS_LE_ABS_32_SJ20
>> B_R_L_TLS_LE64_HI12     B_R_L_TLS_LE_ABS_52_SK12
>> B_R_L_TLS_IE_PC_LO12    B_R_L_TLS_IE_PCALA_0_SK12
>> B_R_L_TLS_IE_PC_HI20    B_R_L_TLS_IE_PCALA_12_SJ20
>> B_R_L_TLS_IE64_PC_LO20  B_R_L_TLS_IE_PCALA_32_SJ20
>> B_R_L_TLS_IE64_PC_HI12  B_R_L_TLS_IE_PCALA_52_SK12
>> B_R_L_TLS_IE64_LO12     B_R_L_TLS_IE_ABS_0_SK12
>> B_R_L_TLS_IE64_HI20     B_R_L_TLS_IE_ABS_12_SJ20
>> B_R_L_TLS_IE64_LO20     B_R_L_TLS_IE_ABS_32_SJ20
>> B_R_L_TLS_IE64_HI12     B_R_L_TLS_IE_ABS_52_SK12
>> B_R_L_TLS_LD_PC_HI20    B_R_L_TLS_LD_PCALA_12_SJ20 *6
>> B_R_L_TLS_LD64_HI20     B_R_L_TLS_LD_ABS_12_SJ20
>> B_R_L_TLS_GD_PC_HI20    B_R_L_TLS_GD_PCALA_12_SJ20
>> B_R_L_TLS_GD64_HI20     B_R_L_TLS_GD_ABS_12_SJ20
> It's overall better, but those "J, K" etc are cryptic IMHO.  And for
> "B_R_L_B16" I think "B_R_L_PCREL_SK16" fails to express that the offset
> should be shifted right by 2, so I'd keep B_R_L_B16 (and similarly,
> B_R_L_B21 and B_R_L_B26) like the PCALA case.
>
The slot names come from 
https://github.com/loongson/LoongArch-Documentation/pull/56 (to 
non-Chinese speakers: English translation still TODO, but hopefully at 
least the ABNF and the few tables should be intelligible even without 
translation).

As for the PCREL and implicit shifts, maybe a planned extension of the 
slot syntax could help. I plan to submit an Assembly Language Convention 
after the Instruction Format Convention gets reviewed and merged, and 
with this new convention such an extension is necessary anyway for 
accommodating the current assembly syntax as described by the ISA 
manual, able to express the <<2's and +1's for the PC-manipulating and 
ALSL insns.

For example, let's say the extension works like this: ordinary slot 
syntax, plus "p", plus an postprocess operator, with "s\d+" for "imm 
slot = assembly operand >> N", and "p\d+" for "imm slot = assembly 
operand + N". Then we could say "B_R_L_PCREL_SK16PS2" and have all the 
niceties. (In an ideal world, the LoongArch opcodes implementation 
should be modified to use this convention too, so for example it will be 
something like "DJSk16ps2" for JIRL, "JDSk16ps2" for BLT (notice the 
reversed operand order) and "DJKUa2pp1" for ALSL insns.