[PATCH] MIPS: Remove compact branch policy Kconfig entries

[PATCH] MIPS: Remove compact branch policy Kconfig entries

[Paul Burton]

Commit c1a0e9bc885d ("MIPS: Allow compact branch policy to be changed")
added Kconfig entries allowing for the compact branch policy used by the
compiler for MIPSr6 kernels to be specified. This can be useful for
debugging, particularly in systems where compact branches have recently
been introduced.

Unfortunately mainline gcc 5.x supports MIPSr6 but not the
-mcompact-branches compiler flag, leading to MIPSr6 kernels failing to
build with gcc 5.x with errors such as:

  mipsel-linux-gnu-gcc: error: unrecognized command line option '-mcompact-branches=optimal'
  make[2]: *** [kernel/bounds.s] Error 1

Fixing this by hiding the Kconfig entry behind another seems to be more
hassle than it's worth, as MIPSr6 & compact branches have been around
for a while now and if policy does need to be set for debug it can be
done easily enough with KCFLAGS. Therefore remove the compact branch
policy Kconfig entries & their handling in the Makefile.

This reverts commit c1a0e9bc885d ("MIPS: Allow compact branch policy to
be changed").

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+
Cc: linux-mips@linux-mips.org
Cc: Ralf Baechle <ralf@linux-mips.org>
---
Patches 13275[1] & 13276[2] seem to have gone nowhere, so this is a
simple alternative.

Ralf: if you're OK with this could we get it in ASAP, perhaps for v4.8?

[1] https://patchwork.linux-mips.org/patch/13275/
[2] https://patchwork.linux-mips.org/patch/13276/
---
 arch/mips/Kconfig.debug | 36 ------------------------------------
 arch/mips/Makefile      |  4 ----
 2 files changed, 40 deletions(-)

diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
index 94cb034..c6bfcb6 100644
--- a/arch/mips/Kconfig.debug
+++ b/arch/mips/Kconfig.debug
@@ -113,42 +113,6 @@ config SPINLOCK_TEST
 	help
 	  Add several files to the debugfs to test spinlock speed.
 
-if CPU_MIPSR6
-
-choice
-	prompt "Compact branch policy"
-	default MIPS_COMPACT_BRANCHES_OPTIMAL
-
-config MIPS_COMPACT_BRANCHES_NEVER
-	bool "Never (force delay slot branches)"
-	help
-	  Pass the -mcompact-branches=never flag to the compiler in order to
-	  force it to always emit branches with delay slots, and make no use
-	  of the compact branch instructions introduced by MIPSr6. This is
-	  useful if you suspect there may be an issue with compact branches in
-	  either the compiler or the CPU.
-
-config MIPS_COMPACT_BRANCHES_OPTIMAL
-	bool "Optimal (use where beneficial)"
-	help
-	  Pass the -mcompact-branches=optimal flag to the compiler in order for
-	  it to make use of compact branch instructions where it deems them
-	  beneficial, and use branches with delay slots elsewhere. This is the
-	  default compiler behaviour, and should be used unless you have a
-	  reason to choose otherwise.
-
-config MIPS_COMPACT_BRANCHES_ALWAYS
-	bool "Always (force compact branches)"
-	help
-	  Pass the -mcompact-branches=always flag to the compiler in order to
-	  force it to always emit compact branches, making no use of branch
-	  instructions with delay slots. This can result in more compact code
-	  which may be beneficial in some scenarios.
-
-endchoice
-
-endif # CPU_MIPSR6
-
 config SCACHE_DEBUGFS
 	bool "L2 cache debugfs entries"
 	depends on DEBUG_FS
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 4921b08..8a30b05 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -205,10 +205,6 @@ cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_VIRT
 toolchain-xpa				:= $(call cc-option-yn,$(mips-cflags) -mxpa)
 cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_XPA
 
-cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_NEVER)	+= -mcompact-branches=never
-cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_OPTIMAL)	+= -mcompact-branches=optimal
-cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_ALWAYS)	+= -mcompact-branches=always
-
 #
 # Firmware support
 #
-- 
2.9.3

Re: [PATCH] MIPS: Remove compact branch policy Kconfig entries

[Ralf Baechle]

On Mon, Sep 12, 2016 at 10:58:06AM +0100, Paul Burton wrote:

> Fixing this by hiding the Kconfig entry behind another seems to be more
> hassle than it's worth, as MIPSr6 & compact branches have been around
> for a while now and if policy does need to be set for debug it can be
> done easily enough with KCFLAGS. Therefore remove the compact branch
> policy Kconfig entries & their handling in the Makefile.

I've applied your patch - and given where we are wrt. to R6 I think this
simply and bulletproof solution is certainly the right thing.

But, have you considered probing for the option and only using it where
it actually is available with something like:

cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_NEVER)   += $(call cc-option,-mcompact-branches=never)
cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_OPTIMAL) += $(call cc-option,-mcompact-branches=optimal)
cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_ALWAYS)  += $(call cc-option,-mcompact-branches=always)

?

I'm also wondering how much we gain from -mcompact-branches?

  Ralf

Re: [PATCH] MIPS: Remove compact branch policy Kconfig entries

[Paul Burton]

On 13/09/16 13:43, Ralf Baechle wrote:
> On Mon, Sep 12, 2016 at 10:58:06AM +0100, Paul Burton wrote:
> 
>> Fixing this by hiding the Kconfig entry behind another seems to be more
>> hassle than it's worth, as MIPSr6 & compact branches have been around
>> for a while now and if policy does need to be set for debug it can be
>> done easily enough with KCFLAGS. Therefore remove the compact branch
>> policy Kconfig entries & their handling in the Makefile.
> 
> I've applied your patch - and given where we are wrt. to R6 I think this
> simply and bulletproof solution is certainly the right thing.
> 
> But, have you considered probing for the option and only using it where
> it actually is available with something like:
> 
> cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_NEVER)   += $(call cc-option,-mcompact-branches=never)
> cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_OPTIMAL) += $(call cc-option,-mcompact-branches=optimal)
> cflags-$(CONFIG_MIPS_COMPACT_BRANCHES_ALWAYS)  += $(call cc-option,-mcompact-branches=always)
> 
> ?

Hi Ralf,

I believe that suggestion came up in discussion about an older patch,
but I think that might violate the principle of least surprise. ie. it
would mean you could set the policy to never in Kconfig, build a kernel
& it could contain compact branches.

> I'm also wondering how much we gain from -mcompact-branches?
> 
>   Ralf

Well, the default policy is "optimal" so we shouldn't gain much if
anything from tweaking it via Kconfig. Different CPUs/pipelines have
different gains, and the compiler should do something sensible for the
target CPU when the default optimal policy is set. This was always about
debugging, and KCFLAGS seems good enough for that especially since there
shouldn't be much to debug anymore.

Thanks,
    Paul