[PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[Paul Burton]

It turns out that some toolchains which support MIPS R6 don't support
the -mcompact-branches flag to specify compact branch behaviour. Default
to not providing the -mcompact-branch option to the compiler such that
we can build with such toolchains.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Fixes: c1a0e9bc885d ("MIPS: Allow compact branch policy to be changed")
Cc: stable <stable@vger.kernel.org> # v4.4+

---

 arch/mips/Kconfig.debug | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
index f0e314c..e91b3d1 100644
--- a/arch/mips/Kconfig.debug
+++ b/arch/mips/Kconfig.debug
@@ -117,7 +117,15 @@ if CPU_MIPSR6
 
 choice
 	prompt "Compact branch policy"
-	default MIPS_COMPACT_BRANCHES_OPTIMAL
+	default MIPS_COMPACT_BRANCHES_DEFAULT
+
+config MIPS_COMPACT_BRANCHES_DEFAULT
+	bool "Toolchain Default (don't specify)"
+	help
+	  Don't pass the -mcompact-branches flag to the compiler, allowing it
+	  to use its default (generally "optimal"). This is particularly
+	  useful for early R6-supporting toolchains which don't support the
+	  -mcompact-branches flag.
 
 config MIPS_COMPACT_BRANCHES_NEVER
 	bool "Never (force delay slot branches)"
-- 
2.8.0

Re: [PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[Maciej W. Rozycki]

On Fri, 22 Apr 2016, Paul Burton wrote:

> It turns out that some toolchains which support MIPS R6 don't support
> the -mcompact-branches flag to specify compact branch behaviour. Default
> to not providing the -mcompact-branch option to the compiler such that
> we can build with such toolchains.

 Good idea overall, one further suggestion below.

> diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
> index f0e314c..e91b3d1 100644
> --- a/arch/mips/Kconfig.debug
> +++ b/arch/mips/Kconfig.debug
> @@ -117,7 +117,15 @@ if CPU_MIPSR6
>  
>  choice
>  	prompt "Compact branch policy"
> -	default MIPS_COMPACT_BRANCHES_OPTIMAL
> +	default MIPS_COMPACT_BRANCHES_DEFAULT
> +
> +config MIPS_COMPACT_BRANCHES_DEFAULT
> +	bool "Toolchain Default (don't specify)"
> +	help
> +	  Don't pass the -mcompact-branches flag to the compiler, allowing it
> +	  to use its default (generally "optimal"). This is particularly
> +	  useful for early R6-supporting toolchains which don't support the
> +	  -mcompact-branches flag.
>  
>  config MIPS_COMPACT_BRANCHES_NEVER
>  	bool "Never (force delay slot branches)"

 How about making the option depend on DEBUG_KERNEL and maybe making it an 
umbrella setting to hide details from users who do not want to be 
bothered, i.e. something like:

config MIPS_COMPACT_BRANCHES_OVERRIDE
     bool "Override the toolchain default for compact branch policy"
     depends on DEBUG_KERNEL
     default n
[...]
if MIPS_COMPACT_BRANCHES_OVERRIDE
choice
    prompt "Compact branch policy"
    default MIPS_COMPACT_BRANCHES_OPTIMAL
[...]
endif # MIPS_COMPACT_BRANCHES_OVERRIDE

?

  Maciej

Re: [PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

On Fri, Apr 22, 2016 at 04:56:02PM +0100, Maciej W. Rozycki wrote:
> On Fri, 22 Apr 2016, Paul Burton wrote:
> 
> > It turns out that some toolchains which support MIPS R6 don't support
> > the -mcompact-branches flag to specify compact branch behaviour. Default
> > to not providing the -mcompact-branch option to the compiler such that
> > we can build with such toolchains.
> 
>  Good idea overall, one further suggestion below.
> 
> > diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
> > index f0e314c..e91b3d1 100644
> > --- a/arch/mips/Kconfig.debug
> > +++ b/arch/mips/Kconfig.debug
> > @@ -117,7 +117,15 @@ if CPU_MIPSR6
> >  
> >  choice
> >  	prompt "Compact branch policy"
> > -	default MIPS_COMPACT_BRANCHES_OPTIMAL
> > +	default MIPS_COMPACT_BRANCHES_DEFAULT
> > +
> > +config MIPS_COMPACT_BRANCHES_DEFAULT
> > +	bool "Toolchain Default (don't specify)"
> > +	help
> > +	  Don't pass the -mcompact-branches flag to the compiler, allowing it
> > +	  to use its default (generally "optimal"). This is particularly
> > +	  useful for early R6-supporting toolchains which don't support the
> > +	  -mcompact-branches flag.
> >  
> >  config MIPS_COMPACT_BRANCHES_NEVER
> >  	bool "Never (force delay slot branches)"
> 
>  How about making the option depend on DEBUG_KERNEL and maybe making it an 
> umbrella setting to hide details from users who do not want to be 
> bothered, i.e. something like:
> 
> config MIPS_COMPACT_BRANCHES_OVERRIDE
>      bool "Override the toolchain default for compact branch policy"
>      depends on DEBUG_KERNEL
>      default n

Although note that "default n" is redundant.

Cheers
James

> [...]
> if MIPS_COMPACT_BRANCHES_OVERRIDE
> choice
>     prompt "Compact branch policy"
>     default MIPS_COMPACT_BRANCHES_OPTIMAL
> [...]
> endif # MIPS_COMPACT_BRANCHES_OVERRIDE
> 
> ?
> 
>   Maciej
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[Ralf Baechle]

I was wondering if we should simply probe for the availability of the
GCC option and not use it, if using an older GCC, then change the
help text for the option accordingly.  This approach would allow
make randconfig or similar to work as expected with older compilers.

  Ralf

Re: [PATCH] MIPS: Allow R6 compact branch policy to be left unspecified

[Maciej W. Rozycki]

On Wed, 11 May 2016, Ralf Baechle wrote:

> I was wondering if we should simply probe for the availability of the
> GCC option and not use it, if using an older GCC, then change the
> help text for the option accordingly.  This approach would allow
> make randconfig or similar to work as expected with older compilers.

 Well, if the default is `optimal' anyway, then I think we can simply omit 
the option unless someone has requested an override.  In which case I 
think the compilation should fail if the option is not supported, under 
the principle of the least surprise -- if someone has requested a feature, 
then they ought to be informed that it is absent rather than silently 
fooled into thinking it has been enabled while in fact it has not.

 I agree probing for the presence of the option requested and then failing 
gracefully (e.g. "Toolchain feature FOO not available, please upgrade or 
reconfigure without BAR" or suchlike) is a better idea than just aborting 
midway through, and I think `randconfig' and similar validators should be 
prepared for it and handle gracefully as well (i.e. not a kernel bug).

 FWIW,

  Maciej