[PATCH] powerpc/32: Avoid unsupported flags with clang

[Nick Desaulniers]

On Thu, Nov 1, 2018 at 9:39 PM Joel Stanley <joel at jms.id.au> wrote:
>
> When building for ppc32 with clang these flags are unsupported:
>
>   -ffixed-r2 and -mmultiple
>
> llvm's lib/Target/PowerPC/PPCRegisterInfo.cpp marks r2 as reserved on
> when building for SVR4ABI and !ppc64:
>
>   // The SVR4 ABI reserves r2 and r13
>   if (Subtarget.isSVR4ABI()) {
>     // We only reserve r2 if we need to use the TOC pointer. If we have no
>     // explicit uses of the TOC pointer (meaning we're a leaf function with
>     // no constant-pool loads, etc.) and we have no potential uses inside an
>     // inline asm block, then we can treat r2 has an ordinary callee-saved
>     // register.
>     const PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
>     if (!TM.isPPC64() || FuncInfo->usesTOCBasePtr() || MF.hasInlineAsm())
>       markSuperRegs(Reserved, PPC::R2);  // System-reserved register
>     markSuperRegs(Reserved, PPC::R13); // Small Data Area pointer register
>   }
>
> This means we can safely omit -ffixed-r2 when building for 32-bit
> targets.
>
> The -mmultiple/-mno-multiple flags are not supported by clang, so
> platforms that might support multiple miss out on using multiple word
> instructions.
>
> Clang 8 can then build a ppc44x_defconfig which boots in Qemu:
>
>   make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-  ppc44x_defconfig
>   make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
>
>   qemu-system-ppc -M bamboo \
>    -kernel arch/powerpc/boot/zImage \
>    -dtb arch/powerpc/boot/dts/bamboo.dtb \
>    -initrd ~/ppc32-440-rootfs.cpio \
>    -nographic -serial stdio -monitor pty -append "console=ttyS0"
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/261
> Signed-off-by: Joel Stanley <joel at jms.id.au>
> ---
> I considered putting these behind a cc-option test. If someone has an
> opinion on that I'm happy to change.

Hi Joel, thanks for the patch!  In regards to cc-option, I guess it
might come down to whether you consider this a missing feature in
Clang that can be implemented vs is not strictly necessary.  If it's a
missing feature, then when Clang does implement this, and you try to
compile this code with the fixed version of Clang, you still won't
make use of it until at least one addition patch to this code is
submitted (at which point, maybe cc-option is the better choice).

cc-option makes it so then if something is going to be implemented,
but is not strictly required today, then it will start working without
additional effort in the future.  If there's no plans to implement
this in Clang because it's redundant/implicit/set by another flag/etc,
then the CONFIG_CC_IS_CLANG check is perfectly fine.

If you do consider it a missing feature, please take the time to file
a bug with LLVM.

>
>  arch/powerpc/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 17be664dafa2..5fa678eee747 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -152,7 +152,9 @@ endif
>  CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
>  CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
>
> -CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
> +ifndef CONFIG_CC_IS_CLANG
> +#CFLAGS-$(CONFIG_PPC32)        := -ffixed-r2 $(MULTIPLEWORD)
> +endif
>  CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
>
>  ifdef CONFIG_PPC_BOOK3S_64
> --
> 2.19.1
>


-- 
Thanks,
~Nick Desaulniers