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
691 stars 163 forks source link

Clarify that multiple R_RISCV_LO12_I/R_RISCV_LO12_S can share one single R_RISCV_HI20 #408

Closed MaskRay closed 10 months ago

MaskRay commented 10 months ago

This is most common on rv32 when loading a 8-byte aligned global variable, e.g.

double NaturallyAlignedScalar = 5.;
double accessNaturallyAlignedScalar() { return NaturallyAlignedScalar; }
//      lui     a1, %hi(NaturallyAlignedScalar)
//      lw      a0, %lo(NaturallyAlignedScalar)(a1)
//      lw      a1, %lo(NaturallyAlignedScalar+4)(a1)

double NaturallyAlignedArray[4] = { 3., 4., 5., 6. };
double accessNaturallyAlignedArray() {
  return NaturallyAlignedArray[0] + NaturallyAlignedArray[3];
}
//      lui     a2, %hi(NaturallyAlignedArray)
//      lw      a0, %lo(NaturallyAlignedArray)(a2)
//      lw      a1, %lo(NaturallyAlignedArray+4)(a2)
//      addi    a3, a2, %lo(NaturallyAlignedArray)
//      lw      a2, 24(a3)
//      lw      a3, 28(a3)

The HI20 values for the multiple fragments must be identical and all the relaxed global-pointer offsets must be in range.

kito-cheng commented 10 months ago

Generally LGTM, but I guess we may need to mention the alignment issue, the addends can't larger than alignment if the high part and low part using different addends, otherwise the high part may different.

e.g. symbol_address of foo is 0x17fc, which is an address align to 4 byte, not align to 8 byte, then it can't share to alignment between foo and foo+4, because (0x17fc + 0x800) >> 12 is 0x1 and (0x17fc + 4 + 0x800) >> 12 is 0x2.

rui314 commented 10 months ago

The examples in this change raise the question of whether the example code is actually valid, and to address that question, we need to mention the alignment issue and its resolution, as pointed out by @kito-cheng. This discussion seems a bit derailed from the primary purpose of the psABI. Maybe just saying that there may be more than one R_RISCV_LO12_I/R_RISCV_LO12_S for a R_RISCV_HI20 is enough?

MaskRay commented 10 months ago

The examples in this change raise the question of whether the example code is actually valid, and to address that question, we need to mention the alignment issue and its resolution, as pointed out by @kito-cheng. This discussion seems a bit derailed from the primary purpose of the psABI. Maybe just saying that there may be more than one R_RISCV_LO12_I/R_RISCV_LO12_S for a R_RISCV_HI20 is enough?

I have added "..., a condition met when the symbol is sufficiently aligned." Hopefully this is clear without derailing from the primary purpose.