Re: [PATCH] LoongArch: Remove unused header compiler.h

[Huacai Chen]

OK, please update the commit message and send V2, please.

On Thu, Jul 21, 2022 at 1:38 PM WANG Xuerui <kernel@xxxxxxxxxx> wrote:
>
> On 2022/7/21 11:22, Huacai Chen wrote:
> > Hi, Xuerui,
> >
> > On Thu, Jul 21, 2022 at 11:17 AM WANG Xuerui <kernel@xxxxxxxxxx> wrote:
> >> Hi YI Jun and Huacai,
> >>
> >> On 2022/7/21 10:52, Huacai Chen wrote:
> >>> Hi, Jun,
> >>>
> >>> On Thu, Jul 21, 2022 at 10:11 AM Jun Yi <yijun@xxxxxxxxxxx> wrote:
> >>>> Loongarch not used arch-specific compiler.h
> >>> I'm not sure whether compiler.h will be used in future. If it will be
> >>> used, I want to keep it as is. Xuerui, what do you think about it?
> >> I surveyed all the existing arch compiler.h in the tree:
> >>
> >> $ find ./arch -name compiler.h
> >> ./arch/alpha/include/asm/compiler.h
> >> ./arch/alpha/include/uapi/asm/compiler.h
> >> ./arch/arm/include/asm/compiler.h
> >> ./arch/arm64/include/asm/compiler.h
> >> ./arch/mips/include/asm/compiler.h
> >> ./arch/loongarch/include/asm/compiler.h
> >>
> >> Of all these occurrences:
> >>
> >> - alpha needs to ensure a certain insn is being emitted from time to
> >> time, with plain C constructs (or built-ins) on compiler versions with
> >> said support, falling back to inline asm otherwise;
> >> - arm and arm64 both need some inline assembly help (of different sort),
> >> with arm64 stuffing some pointer authentication helpers into this file too;
> >> - mips, which is obviously what the loongarch version is based on, needs
> >> (1) a kludge for older compilers to fix delay slot filling around
> >> __builtin_unreachable, (2) definitions for explicit arch level
> >> selection. There is also the historical GCC_OFF_SMALL_ASM() constraint
> >> definition that was rendered redundant by commit 4abaacc704729 ("MIPS:
> >> remove GCC < 4.9 support").
> >>
> >> For loongarch, the "ZC" constraint (I don't think it was a coincidence
> >> BTW) should be usable for all present and future hardware, so I do think
> >> the GCC_OFF_SMALL_ASM() here is redundant. We may want to remove the
> >> mips one too. And the arch level thing is not currently needed either,
> >> future revisions to the LoongArch ISA should be largely backwards
> >> compatible, so it could be a long time before such explicit selection of
> >> arch level is necessary, for exact control over emitted insn.
> >>
> >> So overall, I'm in favor of removing this header for now.
> > Have you considered the new relocation types will be added in the near
> > future? I think we need compiler.h at that time.
>
> I assume you mean the proposal being discussed at [1] [2] and [3].
>
> For new reloc types that affect module loading, asm/elf.h and
> kernel/module.c need modification to add awareness, but this doesn't
> involve compiler.h. The kernel image itself is not affected.
>
> There is also the case of building LoongArch kernel sources without
> support for the new reloc types, but on a newer compiler that emits the
> new-style reloc records by default. In this case, a switch reverting the
> compiler to the old-style relocs is needed in CFLAGS, but (1) not all
> essential support are merged for LoongArch so practically we don't need
> to care about non-kernel-ABI compatibility at this time, and (2) CFLAGS
> tweaks don't involve compiler.h either.
>
> [1]: https://sourceware.org/pipermail/binutils/2022-July/121849.html
> [2]: https://sourceware.org/pipermail/binutils/2022-July/121933.html
> [3]: https://github.com/loongson/LoongArch-Documentation/pull/57
>
>