2022-07-28 12:30:28

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] LoongArch: Stop using undocumented assembler options

Hi, Ruoyao,

On Thu, Jul 28, 2022 at 8:03 PM Xi Ruoyao <[email protected]> wrote:
>
> Now we can handle GOT and GOT-based relocations properly, remove the
> undocumented `-Wa,-mla-{global,local}-with-{pcrel,abs}` assembler hacks.
I think "-Wa,-mla-{global,local}-with-{pcrel,abs}" may be regular
options rather than "hacks". If I'm right, the title and commit
message should be updated. And we can send patches to binutils to make
them "documented".

Huacai
>
> And, -fplt is the default of all supported compilers (GCC, and maybe
> Clang in the future), so it can be removed as well.
>
> Adjust assembly code to explicitly use "la.pcrel" where necessary.
>
> Signed-off-by: Xi Ruoyao <[email protected]>
> ---
> arch/loongarch/Makefile | 4 ----
> arch/loongarch/kernel/head.S | 10 +++++-----
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index 039dcc4fe1f3..800349ea9310 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -40,10 +40,6 @@ endif
>
> cflags-y += -G0 -pipe -msoft-float
> LDFLAGS_vmlinux += -G0 -static -n -nostdlib
> -KBUILD_AFLAGS_KERNEL += -Wa,-mla-global-with-pcrel
> -KBUILD_CFLAGS_KERNEL += -Wa,-mla-global-with-pcrel
> -KBUILD_AFLAGS_MODULE += -Wa,-mla-global-with-abs
> -KBUILD_CFLAGS_MODULE += -fplt -Wa,-mla-global-with-abs,-mla-local-with-abs
>
> cflags-y += -ffreestanding
> cflags-y += $(call cc-option, -mno-check-zero-division)
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index 74ea7bf6c8d6..193329ed6e8c 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -60,17 +60,17 @@ SYM_CODE_START(kernel_entry) # kernel entry point
> la.abs t0, 0f
> jirl zero, t0, 0
> 0:
> - la t0, __bss_start # clear .bss
> + la.pcrel t0, __bss_start # clear .bss
> st.d zero, t0, 0
> - la t1, __bss_stop - LONGSIZE
> + la.pcrel t1, __bss_stop - LONGSIZE
> 1:
> addi.d t0, t0, LONGSIZE
> st.d zero, t0, 0
> bne t0, t1, 1b
>
> - la t0, fw_arg0
> + la.pcrel t0, fw_arg0
> st.d a0, t0, 0 # firmware arguments
> - la t0, fw_arg1
> + la.pcrel t0, fw_arg1
> st.d a1, t0, 0
>
> /* KSave3 used for percpu base, initialized as 0 */
> @@ -78,7 +78,7 @@ SYM_CODE_START(kernel_entry) # kernel entry point
> /* GPR21 used for percpu base (runtime), initialized as 0 */
> or u0, zero, zero
>
> - la tp, init_thread_union
> + la.pcrel tp, init_thread_union
> /* Set the SP after an empty pt_regs. */
> PTR_LI sp, (_THREAD_SIZE - 32 - PT_SIZE)
> PTR_ADD sp, sp, tp
> --
> 2.37.0
>
>
>


2022-07-28 13:51:48

by Xi Ruoyao

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] LoongArch: Stop using undocumented assembler options

On Thu, 2022-07-28 at 20:25 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
>
> On Thu, Jul 28, 2022 at 8:03 PM Xi Ruoyao <[email protected]> wrote:
> >
> > Now we can handle GOT and GOT-based relocations properly, remove the
> > undocumented `-Wa,-mla-{global,local}-with-{pcrel,abs}` assembler
> > hacks.
> I think "-Wa,-mla-{global,local}-with-{pcrel,abs}" may be regular
> options rather than "hacks". If I'm right, the title and commit
> message should be updated. And we can send patches to binutils to make
> them "documented".

How about changing the message to:

GCC 13 no longer generates la.global and la.local in assembly, but
produces explicit PC-relative relocations to local symbols and GOT
entries for global symbols instead. As the result, -Wa,-mla-* are no
longer sufficient to control the code generation for symbol address
loading. As now we can handle GOT and GOT-based relocations
properly, remove those options to use GOT for global symbol address
consistently.

2022-07-29 02:51:02

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] LoongArch: Stop using undocumented assembler options

Hi, Ruoyao,

On Thu, Jul 28, 2022 at 9:10 PM Xi Ruoyao <[email protected]> wrote:
>
> On Thu, 2022-07-28 at 20:25 +0800, Huacai Chen wrote:
> > Hi, Ruoyao,
> >
> > On Thu, Jul 28, 2022 at 8:03 PM Xi Ruoyao <[email protected]> wrote:
> > >
> > > Now we can handle GOT and GOT-based relocations properly, remove the
> > > undocumented `-Wa,-mla-{global,local}-with-{pcrel,abs}` assembler
> > > hacks.
> > I think "-Wa,-mla-{global,local}-with-{pcrel,abs}" may be regular
> > options rather than "hacks". If I'm right, the title and commit
> > message should be updated. And we can send patches to binutils to make
> > them "documented".
>
> How about changing the message to:
>
> GCC 13 no longer generates la.global and la.local in assembly, but
> produces explicit PC-relative relocations to local symbols and GOT
> entries for global symbols instead. As the result, -Wa,-mla-* are no
> longer sufficient to control the code generation for symbol address
> loading. As now we can handle GOT and GOT-based relocations
> properly, remove those options to use GOT for global symbol address
> consistently.
GCC 13 can still generate la.global and la.local, so "GCC 13 no longer
generates la.global and la.local in assembly by default" may be
better. And "undocumented" in the title should be "non-preferred", I
think.

Huacai