[PATCH] RISC-V: Do fixup for local symbols while with "-mno-relax"
Palmer Dabbelt
palmer@dabbelt.com
Wed Dec 13 00:31:41 GMT 2023
On Tue, 12 Dec 2023 02:44:07 PST (-0800), nelson@rivosinc.com wrote:
> There is another stuff, do we need to limit linkers that only can link the
> objects with -mno-relax if this optimization applied? If so, then we
> probably need,
> 1. An assembler option for this optimization
> 2. Record the object is relaxable or not into the elf attribute? or
> readelf header? or ...
> 2.1 Maybe we can record the relaxation information with
> Tag_RISCV_x3_reg_usage?,
> https://sourceware.org/pipermail/binutils/2023-September/129500.html?
> 2.2 Once the object enables relaxation for some code, the object needs to
> be marked as "relaxable", even if it sets `.option norelax' later.
>
> On Tue, Dec 12, 2023 at 5:46 PM Nelson Chu <nelson@rivosinc.com> wrote:
>
>> The idea looks good to me. Passed regressions of riscv-gnu-toolchain, so
>> committed with some minor changes and indent fixes.
>>
>> There are some TODOs, but no rush to do for now,
>> 1. The implementation is based on the code from bfd/elfnn-riscv.c. We
>> probably can move the code to bfd/elfxx-riscv.c, so that can reduce
>> duplicate code, just like what we did for the architecture parser.
>> Before that, I renamed functions and variables from *reloc* to *fixup*, to
>> distinguish the code from bfd/elfnn-riscv.c, since they are still a little
>> bit different.
>> 2. Maybe not only pcrel_hi/lo12 relocation with local symbols can be resolved
>> at assembler time. Other pc-relative relocation, like branch, may also
>> be able to perform related optimizations.
Nelson and I were just talking about this. I'd been leaning towards
adding another option along the lines of "-mno-relax-abi", which would
explicitly mean that users can depend on no objects being relaxed. That
said, I'm not actually sure I can come up with a case where anything
breaks.
I was specifically worried about things like the compiler doing label
subtraction, but IIUC that can only happen within a single translation
unit so we're safe to take advantage of on R_RISCV_RELAX relocations.
There's also cases like misaligned globals, but we're already broken
there so I'm not sure it counts.
So maybe this safe?
>>
>> Thanks
>> Nelson
>>
>> On Wed, Nov 29, 2023 at 5:17 PM <lifang_xia@linux.alibaba.com> wrote:
>>
>>> From: Lifang Xia <lifang_xia@linux.alibaba.com>
>>>
>>> In the scenario of generating .ko files, the kernel does not relax the .ko
>>> files. However, due to the large amount of relax and local relocation
>>> information, this increases the size of the .ko files.
>>>
>>> In this patch, it will finish the fixup of the local relocations while
>>> with
>>> "-mno-relax" option. This can reduce the size of the relocation table.
>>>
>>> gas/
>>> * config/tc-riscv.c (struct riscv_pcrel_hi_reloc): New, reference
>>> from bfd/elfnn-riscv.c.
>>> (riscv_pcrel_hi_reloc_hash): Likewise.
>>> (riscv_pcrel_reloc_hash): Likewise.
>>> (riscv_pcrel_reloc_eq): Likewise.
>>> (riscv_record_pcrel_reolc): Likewise.
>>> (md_begin): Init pcrel_hi hash.
>>> (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_HI20>: Do fixup and
>>> record the pcrel_hi relocs, mark as done while with
>>> "-mno-relax".
>>> (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_LO12_I>:
>>> (md_apply_fix) <case BFD_RELOC_RISCV_PCREL_LO12_S>: Do fixup
>>> and mark as done while with "-mno-relax".
>>> (riscv_md_end): New, delete riscv_pcrel_hi_reloc_hash.
>>> * config/tc-riscv.h (md_end): Define md_end with riscv_md_end.
>>> gas/
>>> * testsuite/gas/riscv/fixup-local*: New tests.
>>> ---
>>> gas/config/tc-riscv.c | 111 +++++++++++++++++-
>>> gas/config/tc-riscv.h | 3 +
>>> gas/testsuite/gas/riscv/fixup-local-norelax.d | 23 ++++
>>> gas/testsuite/gas/riscv/fixup-local-relax.d | 41 +++++++
>>> gas/testsuite/gas/riscv/fixup-local.s | 13 ++
>>> 5 files changed, 190 insertions(+), 1 deletion(-)
>>> create mode 100644 gas/testsuite/gas/riscv/fixup-local-norelax.d
>>> create mode 100644 gas/testsuite/gas/riscv/fixup-local-relax.d
>>> create mode 100644 gas/testsuite/gas/riscv/fixup-local.s
>>>
>>> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>>> index 04738d5e00c..332103574fe 100644
>>> --- a/gas/config/tc-riscv.c
>>> +++ b/gas/config/tc-riscv.c
>>> @@ -1569,6 +1569,58 @@ init_opcode_hash (const struct riscv_opcode
>>> *opcodes,
>>> return hash;
>>> }
>>>
>>> +/* Record all PC-relative high-part relocs we've encountered to help us
>>> + later resolve the corresponding low-part relocs. */
>>> +struct riscv_pcrel_hi_reloc
>>> +{
>>> + bfd_vma address;
>>> + symbolS *symbol;
>>> + bfd_vma target;
>>> +};
>>> +
>>> +/* Handle of the pcrel_hi hash table. */
>>> +static htab_t riscv_pcrel_hi_reloc_hash;
>>> +
>>> +/* Get the key of a entry from the pcrel_hi hash table. */
>>> +
>>> +static hashval_t
>>> +riscv_pcrel_reloc_hash (const void *entry)
>>> +{
>>> + const struct riscv_pcrel_hi_reloc *e = entry;
>>> + return (hashval_t) (e->address);
>>> +}
>>> +
>>> +/* Compare the keys between two entries fo the pcrel_hi hash table. */
>>> +
>>> +static int
>>> +riscv_pcrel_reloc_eq (const void *entry1, const void *entry2)
>>> +{
>>> + const struct riscv_pcrel_hi_reloc *e1 = entry1, *e2 = entry2;
>>> + return e1->address == e2->address;
>>> +}
>>> +
>>> +/* Record the pcrel_hi relocs. */
>>> +
>>> +static bool
>>> +riscv_record_pcrel_reolc (htab_t p, bfd_vma address, symbolS *symbol,
>>> + bfd_vma target)
>>> +{
>>> + struct riscv_pcrel_hi_reloc entry = { address, symbol, target };
>>> + struct riscv_pcrel_hi_reloc **slot
>>> + = (struct riscv_pcrel_hi_reloc **)htab_find_slot (p, &entry,
>>> INSERT);
>>> + if (slot == NULL)
>>> + return false;
>>> +
>>> + *slot = (struct riscv_pcrel_hi_reloc *)xmalloc (
>>> + sizeof (struct riscv_pcrel_hi_reloc));
>>> + if (*slot != NULL)
>>> + {
>>> + **slot = entry;
>>> + return true;
>>> + }
>>> + return false;
>>> +}
>>> +
>>> /* This function is called once, at assembler startup time. It should
>>> set up
>>> all the tables, etc. that the MD part of the assembler will need. */
>>>
>>> @@ -1605,6 +1657,11 @@ md_begin (void)
>>> opcode_names_hash = str_htab_create ();
>>> init_opcode_names_hash ();
>>>
>>> + /* Create pcrel_hi hash table to resolve the relocation while with
>>> + -mno-relax. */
>>> + riscv_pcrel_hi_reloc_hash
>>> + = htab_create (1024, riscv_pcrel_reloc_hash, riscv_pcrel_reloc_eq,
>>> free);
>>> +
>>> /* Set the default alignment for the text section. */
>>> record_alignment (text_section, riscv_opts.rvc ? 1 : 2);
>>> }
>>> @@ -4213,9 +4270,52 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg
>>> ATTRIBUTE_UNUSED)
>>> break;
>>>
>>> case BFD_RELOC_RISCV_PCREL_HI20:
>>> + /* record the pcrel_hi relocs of the local symbols. And evaluate
>>> the hi20
>>> + of the lcoal symbols. Fill in a tentative value to improve
>>> objdump
>>> + readability for -mrelax, and set fx_done for -mno-relax. */
>>> + if (S_IS_LOCAL (fixP->fx_addsy) && fixP->fx_addsy
>>> + && S_GET_SEGMENT (fixP->fx_addsy) == seg)
>>> + {
>>> + bfd_vma target = S_GET_VALUE (fixP->fx_addsy) + *valP;
>>> + bfd_vma value = target - md_pcrel_from (fixP);
>>> +
>>> + /* Record PCREL_HI20. */
>>> + if (!riscv_record_pcrel_reolc (riscv_pcrel_hi_reloc_hash,
>>> + md_pcrel_from (fixP),
>>> fixP->fx_addsy,
>>> + target))
>>> + {
>>> + as_warn ("too many pcrel_hi");
>>> + }
>>> +
>>> + bfd_putl32 (bfd_getl32 (buf)
>>> + | ENCODE_UTYPE_IMM (RISCV_CONST_HIGH_PART
>>> (value)),
>>> + buf);
>>> + if (!riscv_opts.relax)
>>> + fixP->fx_done = 1;
>>> + }
>>> + relaxable = riscv_opts.relax;
>>> + break;
>>> case BFD_RELOC_RISCV_PCREL_LO12_S:
>>> case BFD_RELOC_RISCV_PCREL_LO12_I:
>>> - relaxable = riscv_opts.relax;
>>> + {
>>> + /* Resolve the low12 of the local symboles with the pcrel_hi
>>> relocs.
>>> + Fill in a tentative value to improve objdump readability for
>>> + -mrelax, and set fx_done for -mno-relax. */
>>> + bfd_vma location_pcrel_hi = S_GET_VALUE (fixP->fx_addsy) + *valP;
>>> + struct riscv_pcrel_hi_reloc search = { location_pcrel_hi, 0, 0 };
>>> + struct riscv_pcrel_hi_reloc *entry
>>> + = htab_find (riscv_pcrel_hi_reloc_hash, &search);
>>> + if (entry && entry->symbol && S_IS_LOCAL (entry->symbol)
>>> + && S_GET_SEGMENT (entry->symbol) == seg)
>>> + {
>>> + bfd_vma target = entry->target;
>>> + bfd_vma value = target - entry->address;
>>> + bfd_putl32 (bfd_getl32 (buf) | ENCODE_ITYPE_IMM (value),
>>> buf);
>>> + if (!riscv_opts.relax)
>>> + fixP->fx_done = 1;
>>> + }
>>> + relaxable = riscv_opts.relax;
>>> + }
>>> break;
>>>
>>> case BFD_RELOC_RISCV_ALIGN:
>>> @@ -4980,6 +5080,15 @@ riscv_md_finish (void)
>>> riscv_set_public_attributes ();
>>> if (riscv_opts.relax)
>>> bfd_map_over_sections (stdoutput, riscv_insert_uleb128_fixes, NULL);
>>> +
>>> +}
>>> +
>>> +/* Called just before the assembler exits. */
>>> +
>>> +void
>>> +riscv_md_end (void)
>>> +{
>>> + htab_delete (riscv_pcrel_hi_reloc_hash);
>>> }
>>>
>>> /* Adjust the symbol table. */
>>> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
>>> index 0c70c7d4739..9d2f05a4241 100644
>>> --- a/gas/config/tc-riscv.h
>>> +++ b/gas/config/tc-riscv.h
>>> @@ -80,6 +80,9 @@ extern int riscv_parse_long_option (const char *);
>>> extern void riscv_pre_output_hook (void);
>>> #define GAS_SORT_RELOCS 1
>>>
>>> +#define md_end riscv_md_end
>>> +extern void riscv_md_end (void);
>>> +
>>> /* Let the linker resolve all the relocs due to relaxation. */
>>> #define tc_fix_adjustable(fixp) 0
>>> #define md_allow_local_subtract(l,r,s) 0
>>> diff --git a/gas/testsuite/gas/riscv/fixup-local-norelax.d
>>> b/gas/testsuite/gas/riscv/fixup-local-norelax.d
>>> new file mode 100644
>>> index 00000000000..22132a32f91
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/riscv/fixup-local-norelax.d
>>> @@ -0,0 +1,23 @@
>>> +#as: -march=rv64i -mno-relax
>>> +#source: fixup-local.s
>>> +#objdump: -dr
>>> +
>>> +tmpdir/fixup-local.o: file format elf64-littleriscv
>>> +
>>> +
>>> +Disassembly of section .text:
>>> +
>>> +0+0000 <foo>:
>>> +[ ]+0:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+4:[ ]+00850513[ ]+addi a0,a0,8 # 8 <foo\+0x8>
>>> +[ ]+8:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+8: R_RISCV_PCREL_HI20 bar
>>> +[ ]+c:[ ]+00050513[ ]+mv a0,a0
>>> +[ ]+c: R_RISCV_PCREL_LO12_I .L0
>>> +[ ]+10:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+10: R_RISCV_PCREL_HI20 foo
>>> +[ ]+14:[ ]+00050513[ ]+mv a0,a0
>>> +[ ]+14: R_RISCV_PCREL_LO12_I .L0
>>> +[ ]+18:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+1c:[ ]+00852503[ ]+lw a0,8\(a0\) # 20 <foo\+0x20>
>>> +[ ]+20:[ ]+00008067[ ]+ret
>>> diff --git a/gas/testsuite/gas/riscv/fixup-local-relax.d
>>> b/gas/testsuite/gas/riscv/fixup-local-relax.d
>>> new file mode 100644
>>> index 00000000000..49c475c5dcb
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/riscv/fixup-local-relax.d
>>> @@ -0,0 +1,41 @@
>>> +#as: -march=rv64i
>>> +#source: fixup-local.s
>>> +#objdump: -dr
>>> +
>>> +tmpdir/fixup-local.o: file format elf64-littleriscv
>>> +
>>> +
>>> +Disassembly of section .text:
>>> +
>>> +0+0000 <foo>:
>>> +[ ]+0:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+0: R_RISCV_PCREL_HI20 .LL0
>>> +[ ]+0: R_RISCV_RELAX \*ABS\*
>>> +[ ]+4:[ ]+00850513[ ]+addi a0,a0,8 # 8 <.LL0>
>>> +[ ]+4: R_RISCV_PCREL_LO12_I .L0
>>> +[ ]+4: R_RISCV_RELAX \*ABS\*
>>> +
>>> +0000000000000008 <.LL0>:
>>> +[ ]+8:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+8: R_RISCV_PCREL_HI20 bar
>>> +[ ]+8: R_RISCV_RELAX \*ABS\*
>>> +[ ]+c:[ ]+00050513[ ]+mv a0,a0
>>> +[ ]+c: R_RISCV_PCREL_LO12_I .L0
>>> +[ ]+c: R_RISCV_RELAX \*ABS\*
>>> +[ ]+10:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+10: R_RISCV_PCREL_HI20 foo
>>> +[ ]+10: R_RISCV_RELAX \*ABS\*
>>> +[ ]+14:[ ]+00050513[ ]+mv a0,a0
>>> +[ ]+14: R_RISCV_PCREL_LO12_I .L0
>>> +[ ]+14: R_RISCV_RELAX \*ABS\*
>>> +
>>> +0000000000000018 <.LL1>:
>>> +[ ]+18:[ ]+00000517[ ]+auipc a0,0x0
>>> +[ ]+18: R_RISCV_PCREL_HI20 .LL2
>>> +[ ]+18: R_RISCV_RELAX \*ABS\*
>>> +[ ]+1c:[ ]+00852503[ ]+lw a0,8\(a0\) # 20 <.LL2>
>>> +[ ]+1c: R_RISCV_PCREL_LO12_I .LL1
>>> +[ ]+1c: R_RISCV_RELAX \*ABS\*
>>> +
>>> +0000000000000020 <.LL2>:
>>> +[ ]+20:[ ]+00008067[ ]+ret
>>> diff --git a/gas/testsuite/gas/riscv/fixup-local.s
>>> b/gas/testsuite/gas/riscv/fixup-local.s
>>> new file mode 100644
>>> index 00000000000..44b47311235
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/riscv/fixup-local.s
>>> @@ -0,0 +1,13 @@
>>> +.global foo
>>> +.global bar
>>> +foo:
>>> + la a0, .LL0
>>> +.LL0:
>>> + la a0, bar
>>> + la a0, foo
>>> +.LL1:
>>> + auipc a0, %pcrel_hi(.LL2)
>>> + lw a0, %pcrel_lo(.LL1)(a0)
>>> +
>>> +.LL2:
>>> + ret
>>> --
>>> 2.39.2 (Apple Git-143)
>>>
>>>
More information about the Binutils
mailing list