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

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

[Joel Stanley]

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@jms.id.au>
---
I considered putting these behind a cc-option test. If someone has an
opinion on that I'm happy to change.

 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

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

[Joel Stanley]

On Fri, 2 Nov 2018 at 15:09, Joel Stanley <joel@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@jms.id.au>
> ---
> I considered putting these behind a cc-option test. If someone has an
> opinion on that I'm happy to change.
>
>  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)

Whoops. I'll wait for any other comments before re-posting with this fixed.

> +endif
>  CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
>
>  ifdef CONFIG_PPC_BOOK3S_64
> --
> 2.19.1
>

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

[Nick Desaulniers]

On Thu, Nov 1, 2018 at 9:39 PM Joel Stanley <joel@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@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

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

[Michael Ellerman]

Joel Stanley <joel@jms.id.au> writes:

> 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@jms.id.au>
> ---
> I considered putting these behind a cc-option test. If someone has an
> opinion on that I'm happy to change.
>
>  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

I think using cc-option with a comment would be nicer, you could avoid
the ifdef and it would also future-proof it against the option either
appearing in a future clang or disappearing in a future gcc.

eg ~=:

# Clang doesn't need or support -ffixed-r2
CFLAGS-$(CONFIG_PPC32)	+= $(cc-option,-ffixed-r2)


cheers

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

[Joel Stanley]

On Mon, 5 Nov 2018 at 14:18, Michael Ellerman <mpe@ellerman.id.au> wrote:
> > I considered putting these behind a cc-option test. If someone has an
> > opinion on that I'm happy to change.

> > -CFLAGS-$(CONFIG_PPC32)       := -ffixed-r2 $(MULTIPLEWORD)
> > +ifndef CONFIG_CC_IS_CLANG
> > +#CFLAGS-$(CONFIG_PPC32)      := -ffixed-r2 $(MULTIPLEWORD)
> > +endif
>
> I think using cc-option with a comment would be nicer, you could avoid
> the ifdef and it would also future-proof it against the option either
> appearing in a future clang or disappearing in a future gcc.

Thanks Nick and Michael. I agree, I'll re-spin using cc-option.

Cheers,

Joel

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

[Segher Boessenkool]

On Mon, Nov 05, 2018 at 02:48:49PM +1100, Michael Ellerman wrote:
> Joel Stanley <joel@jms.id.au> writes:
> > When building for ppc32 with clang these flags are unsupported:
> >
> >   -ffixed-r2 and -mmultiple

> > -CFLAGS-$(CONFIG_PPC32)	:= -ffixed-r2 $(MULTIPLEWORD)
> > +ifndef CONFIG_CC_IS_CLANG
> > +#CFLAGS-$(CONFIG_PPC32)	:= -ffixed-r2 $(MULTIPLEWORD)
> > +endif
> 
> I think using cc-option with a comment would be nicer, you could avoid
> the ifdef and it would also future-proof it against the option either
> appearing in a future clang or disappearing in a future gcc.

These options will never be removed, lol.  -ffixed-<reg> is ancient and
generic, and -mmultiple is useful so not in any danger, and besides, we
would leave the command line option even if the feature is gone.

> eg ~=:
> 
> # Clang doesn't need or support -ffixed-r2
> CFLAGS-$(CONFIG_PPC32)	+= $(cc-option,-ffixed-r2)

This option changes the ABI.  The compiler had better handle it!  If it
doesn't, the only safe cause of action is to fail loudly.

If LLVM always makes r2 fixed, it is better to special-case it here.
(Even better would be to fix the compiler, of course).


Segher