Add ePIC relocations and rewrite rules #405

luismarques · 2023-11-12T15:46:43Z

Continuing to follow up on the request to decompose the ePIC PR into multiple PRs, this PR adds the ePIC relocations and rewrite rules.

luismarques · 2023-11-12T15:50:04Z

riscv-elf.adoc

+** Rewrites the addition instruction into a canonical `nop` (`addi x0, x0, 0`) or `c.nop` instruction (if the relocation is being applied to an `add` or `c.add` instruction, respectively), or
+** Deletes the `add` or `c.add` instruction.
+* For GP-relative addressing:
+** Nothing needs to be done.


In the future, I'm hoping we'll have a R_RISCV_GPREL_ADD relocation, for relaxation purposes. If that happens, we can replace this "Nothing needs to be done" with "Optionally adds a R_RISCV_GPREL_ADD relaxation relocation with the same symbol and addend, at the same offset."

Put this as forward-looking non-normative next?

Please see if the added note properly addresses your suggestion.

riscv-elf.adoc

kito-cheng

LGTM :)

kito-cheng · 2024-02-04T12:59:57Z

@rui314 @MaskRay any comments? generally I would like get at least one more LGTM from someone other than GNU folk, but I know it's kinda embedded stuffs which you may not interested on that, so it's fine if no comment, but I would like to make sure it's at least not looks insane to linker expert.

@Nelson1225 You and me has spend times on discussion this before, and we both like this, so I assume you are OK with this, but I guess it would be great to get your explicitly LGTM here again :)

jrtc27 · 2024-02-04T17:35:03Z

riscv-elf.adoc

+* If the symbol resides in a writable output section, then GP-relative addressing is used;
+* If the symbol does not reside in an output section or the section is not writable, then PC-relative addressing is used instead.
+
+NOTE: Implementations are permitted to apply alternative rules for choosing between GP-relative or PC-relative addressing.


Notes are non-normative, so can't override what's outside a note. If it's not required then the spec above needs to not dictate the rules.

I've put the old description as part of the non-normative note. Please check if that addresses your concern.

jrtc27 · 2024-02-04T17:36:46Z

riscv-elf.adoc

+** Rewrites the addition instruction into a canonical `nop` (`addi x0, x0, 0`) or `c.nop` instruction (if the relocation is being applied to an `add` or `c.add` instruction, respectively), or
+** Deletes the `add` or `c.add` instruction.
+* For GP-relative addressing:
+** Nothing needs to be done.


Put this as forward-looking non-normative next?

MaskRay · 2024-02-05T06:11:04Z

I am happy with the GP-relative relocation part (also used by the FDPIC ABI proposal), but I am afraid I am concerned of R_RISCV_EPIC_* relocations. Commented here #343 (comment)

Also CC @sorear

sorear

I don't have a good understanding of the use cases for embedded systems that need PIC but not shared libraries. Is there any freely source available example of something that would use this? If it's needed for a commercial RTOS, is there at least a public manual or datasheet?

@MaskRay Was your comment questioning the existence of the use case for ePIC without shared libraries, suggesting that ePIC should be a special case of FDPIC, or something else?

Assuming that the use case is valid, the general approach looks good. Relocation behavior needs to be described in the language of externally observable linker behavior, not implementation details like synthesized non-dynamic relocations. I think we also need an absolute option, at least for weak symbols but it may be useful more generally.

sorear · 2024-02-16T18:00:24Z

riscv-elf.adoc

+* If the symbol resides in a writable output section, then GP-relative addressing is used;
+* If the symbol does not reside in an output section or the section is not writable, then PC-relative addressing is used instead.
+
+The ePIC relocations are applied to a sequence of instructions that initially address a symbol relative to the GP. When PC-relative addressing should be used instead, the ePIC relocations rewrite the instructions to perform PC-relative addressing and add the appropriate PC-relative relocations. When GP-relative addressing should be used, the instruction rewrites do not occur and GP-relative relocations are added. For correctness, the rewrites must occur even when linker relaxations are disabled. Paired `R_RISCV_RELAX` relocations are preserved during the rewrite process, and will pair with relocations added as part of that rewrite.


Suggested change

The ePIC relocations are applied to a sequence of instructions that initially address a symbol relative to the GP. When PC-relative addressing should be used instead, the ePIC relocations rewrite the instructions to perform PC-relative addressing and add the appropriate PC-relative relocations. When GP-relative addressing should be used, the instruction rewrites do not occur and GP-relative relocations are added. For correctness, the rewrites must occur even when linker relaxations are disabled. Paired `R_RISCV_RELAX` relocations are preserved during the rewrite process, and will pair with relocations added as part of that rewrite.

The ePIC relocations are applied to a sequence of instructions that initially address a symbol relative to the GP. When PC-relative addressing should be used instead, the ePIC relocations rewrite the instructions to perform PC-relative addressing. When GP-relative addressing should be used, the instruction rewrites do not occur. For correctness, the rewrites must occur even when linker relaxations are disabled. Relaxation may occur if paired `R_RISCV_RELAX` relocations are present.

The behavior of relocations needs to be specified in terms of observable linker behavior. The output will not contain non-dynamic relocations (and ePIC does not support dynamic linking), so describe it purely in terms of instruction rewrites.

+1 for this change

sorear · 2024-02-16T18:02:56Z

riscv-elf.adoc

@@ -849,6 +859,48 @@ def align(addend):
  return ALIGN
 ----

+==== Embedded PIC rewrite rules
+
+The Embedded PIC (ePIC) relocations allow addressing a symbol relative to either the PC or the GP, as appropriate for that symbol.


We need to allow absolute addressing as a third option (for weak symbols, and also usable for symbols not in an output section).

sorear · 2024-02-16T18:06:26Z

riscv-elf.adoc

+* For PC-relative addressing:
+** Rewrites the `lui` instruction into an `auipc` instruction with the same operands, by overwriting the opcode field.
+** Adds a `R_RISCV_PCREL_HI20` relocation with the same symbol and addend, at the same offset.
+* For GP-relative addressing:
+** Adds a `R_RISCV_GPREL_HI20` relocation with the same symbol and addend, at the same offset.


Use observable behavior.

Relaxation may delete the HI20 entirely, unless that is documented elsewhere.

sorear · 2024-02-16T18:07:20Z

riscv-elf.adoc

+* For GP-relative addressing:
+** Nothing needs to be done.
+
+NOTE: If a `R_RISCV_GPREL_ADD` relaxation relocation is defined in the future, the GP-relative addressing rule could be updated to read: "Optionally adds a `R_RISCV_GPREL_ADD` relaxation relocation with the same symbol and addend, at the same offset."


Delete, not observable.

sorear · 2024-02-16T18:09:22Z

riscv-elf.adoc

+* For PC-relative addressing, it either:
+** Rewrites the addition instruction into a canonical `nop` (`addi x0, x0, 0`) or `c.nop` instruction (if the relocation is being applied to an `add` or `c.add` instruction, respectively), or
+** Deletes the `add` or `c.add` instruction.
+* For GP-relative addressing:
+** Nothing needs to be done.


Only allow the instruction to be deleted if there is a paired R_RISCV_RELAX.

Allow deletion if there is a paired R_RISCV_RELAX and the HI20 was deleted by relaxation.

sorear · 2024-02-16T18:13:15Z

riscv-elf.adoc

+                                            <| Rewrite
+.2+| 68      .2+| EPIC_LO12_S  .2+| Static  | _S-Type_          .2+| <<Embedded PIC rewrite rules,Embedded PIC rewrite>> for the low 12 bits of a 32-bit PC- or GP-relative offset,  `epic_low(address of %epic_high)`
+                                            <| Rewrite
+.2+| 69      .2+| EPIC_BASE_ADD .2+| Static  | _I-type_         .2+| <<Embedded PIC rewrite rules,Embedded PIC rewrite>> of `gp` addition, `%epic_base_add(symbol)`


I would prefer to have EPIC_BASE_ADD point at the %epic_high for consistency, and rename it EPIC_ADD.

We can get rid of EPIC_LO12_I/EPIC_LO12_S and use the overloaded PCREL_LO12_I/PCREL_LO12_S (which are also used for GOT references, so they're already somewhat of misnomers).

It might be worth getting rid of the GPREL* relocations and using EPIC* exclusively, unless GPREL* already made it into a ratified version.

sorear · 2024-02-16T18:17:52Z

riscv-elf.adoc

+* For PC-relative addressing, it either:
+** Adds a `R_RISCV_PCREL_LO12_I` or `R_RISCV_PCREL_LO12_S` relocation, as appropriate, with the same symbol and addend, at the same offset.
+* For GP-relative addressing:
+** Adds a `R_RISCV_GPREL_LO12_I` or `R_RISCV_GPREL_LO12_S` relocation, as appropriate, at the same offset. The symbol and addend of the new relocation are those of the corresponding `R_RISCV_EPIC_HI20` relocation.


Describe observable behavior - it relocates and relaxes as if there were an XXX relocation, it does not create an XXX relocation.

MaskRay · 2024-02-20T05:48:29Z

I don't have a good understanding of the use cases for embedded systems that need PIC but not shared libraries. Is there any freely source available example of something that would use this? If it's needed for a commercial RTOS, is there at least a public manual or datasheet?

@MaskRay Was your comment questioning the existence of the use case for ePIC without shared libraries, suggesting that ePIC should be a special case of FDPIC, or something else?

Assuming that the use case is valid, the general approach looks good. Relocation behavior needs to be described in the language of externally observable linker behavior, not implementation details like synthesized non-dynamic relocations. I think we also need an absolute option, at least for weak symbols but it may be useful more generally.

My comment is questioning the existence of the use case for ePIC without shared libraries. I think we should consider FDPIC, which works with MMU-less and MMU systems, supports dynamic libraries. After FDPIC support, we can figure out whether there is still ePIC need. (I kinda suspect no.)

luismarques commented Nov 12, 2023

View reviewed changes

kito-cheng requested review from rui314, MaskRay, jrtc27, kito-cheng and Nelson1225 November 13, 2023 06:34

kito-cheng reviewed Nov 13, 2023

View reviewed changes

riscv-elf.adoc Outdated Show resolved Hide resolved

luismarques force-pushed the epic-relocs branch from f8dd7ac to afaee85 Compare November 19, 2023 14:27

kito-cheng reviewed Dec 4, 2023

View reviewed changes

riscv-elf.adoc Show resolved Hide resolved

luismarques force-pushed the epic-relocs branch from afaee85 to b847181 Compare February 3, 2024 21:08

kito-cheng approved these changes Feb 4, 2024

View reviewed changes

jrtc27 reviewed Feb 4, 2024

View reviewed changes

luismarques force-pushed the epic-relocs branch from b847181 to 7ab01ee Compare February 4, 2024 19:17

Add ePIC relocations and rewrite rules

85e1c31

luismarques force-pushed the epic-relocs branch from 7ab01ee to 85e1c31 Compare February 4, 2024 19:23

sorear requested changes Feb 16, 2024

View reviewed changes

sorear mentioned this pull request Feb 18, 2024

Proposal for RISC-V Embedded PIC (ePIC) specification #343

Draft

kito-cheng mentioned this pull request Mar 1, 2024

Revert "Add GP-relative relocations" #427

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ePIC relocations and rewrite rules #405

Add ePIC relocations and rewrite rules #405

luismarques commented Nov 12, 2023

luismarques Nov 12, 2023 •

edited

Loading

jrtc27 Feb 4, 2024

luismarques Feb 4, 2024

kito-cheng left a comment

kito-cheng commented Feb 4, 2024

jrtc27 Feb 4, 2024

luismarques Feb 4, 2024

jrtc27 Feb 4, 2024

MaskRay commented Feb 5, 2024 •

edited

Loading

sorear left a comment

sorear Feb 16, 2024

kito-cheng Mar 1, 2024

sorear Feb 16, 2024

sorear Feb 16, 2024

sorear Feb 16, 2024

sorear Feb 16, 2024

sorear Feb 16, 2024 •

edited

Loading

sorear Feb 16, 2024

MaskRay commented Feb 20, 2024

Add ePIC relocations and rewrite rules #405

Are you sure you want to change the base?

Add ePIC relocations and rewrite rules #405

Conversation

luismarques commented Nov 12, 2023

luismarques Nov 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kito-cheng left a comment

Choose a reason for hiding this comment

kito-cheng commented Feb 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay commented Feb 5, 2024 • edited Loading

sorear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorear Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay commented Feb 20, 2024

luismarques Nov 12, 2023 •

edited

Loading

MaskRay commented Feb 5, 2024 •

edited

Loading

sorear Feb 16, 2024 •

edited

Loading