search

riscv-non-isa / riscv-elf-psabi-doc

A RISC-V ELF psABI Document
https://jira.riscv.org/browse/RVG-4
Creative Commons Attribution 4.0 International
663 stars 158 forks source link

Fix TLSDESC description #420

Closed MaskRay closed 5 months ago

MaskRay commented 5 months ago

The table specifies R_RISCV_TLSDESC_LOAD_LO12 and R_RISCV_TLSDESC_ADD_LO12, while some text uses the _I suffix. Remove the _I suffix to be compatible with LLVM and the proposed binutils patch.

MaskRay commented 5 months ago

@ishitatsuyuki @ilovepi

ishitatsuyuki commented 5 months ago

The _I change looks good to me.

I’m not sure if the tX change is necessary. Both a0 and tX are legal choices here (although tY is not guaranteed to be alive because LOAD is allowed to be reordered after ADD). I think a0 is better overall because the resulting instruction sequence is macro-fusable if both instructions are adjacent.

ilovepi commented 5 months ago

I think it makes sense that things are kept consistent with the initial template, so the use of tX seems appropriate to me.

If there's a big concern about that, then maybe we can add aa comment/note that using a0 would maximize the benefit in most cases, but is up to the implementation.

ishitatsuyuki commented 5 months ago

I think it makes sense that things are kept consistent with the initial template, so the use of tX seems appropriate to me.

I’m not sure if I see why this is more consistent. The destination register for the ADD instruction is a0, so I think it makes more sense for the relaxed instruction to target a0 too.

It’s fine to clarify that both a0/tX can be used, but if we want to pick one, I don’t see any reason so far why tX should be used instead.

MaskRay commented 5 months ago

I think it makes sense that things are kept consistent with the initial template, so the use of tX seems appropriate to me.

I’m not sure if I see why this is more consistent. The destination register for the ADD instruction is a0, so I think it makes more sense for the relaxed instruction to target a0 too.

It’s fine to clarify that both a0/tX can be used, but if we want to pick one, I don’t see any reason so far why tX should be used instead.

I was thinking whether we needed the flexibility but I guess the answer is probably no. From the perspective of implementations, a fixed register is easier, so I reverted the auipc wording change :)