Message ID | 20180809174444.31705-5-paul.burton@mips.com |
---|---|
State | Not Applicable |
Headers | show |
Series | None | expand |
On Thu, Aug 9, 2018 at 7:45 PM Paul Burton <paul.burton@mips.com> wrote: > +/* > + * With GCC v4.5 onwards can use __builtin_unreachable to indicate to the > + * compiler that a particular code path will never be hit. This allows it to be > + * optimised out of the generated binary. > + * > + * Unfortunately GCC from at least v4.9.2 to current head of tree as of May > + * 2016 suffer from a bug that can lead to instructions from beyond an Has anything happened to address this in gcc in the meantime? Could you update this text to reflect whatever is in current gcc-9? Arnd
Hi Arnd, On Thu, Aug 09, 2018 at 08:12:27PM +0200, Arnd Bergmann wrote: > On Thu, Aug 9, 2018 at 7:45 PM Paul Burton <paul.burton@mips.com> wrote: > > > +/* > > + * With GCC v4.5 onwards can use __builtin_unreachable to indicate to the > > + * compiler that a particular code path will never be hit. This allows it to be > > + * optimised out of the generated binary. > > + * > > + * Unfortunately GCC from at least v4.9.2 to current head of tree as of May > > + * 2016 suffer from a bug that can lead to instructions from beyond an > > Has anything happened to address this in gcc in the meantime? > Could you update this text to reflect whatever is in current gcc-9? Good question. I can reproduce the problem using the test case from [1] using both GCC 6.4.0 & 7.3.0, but 8.1.0 generates wildly different code which looks good. Nothing relevant is listed in the release notes for GCC 8.x though, and I can't see anything obvious in gcc's commit logs. It doesn't looks like the fix Robert suggested went in. So I don't know whether current GCC's have resolved the problem or just get lucky enough not to hit it with the existing testcase. I've copied Matthew (GCC MIPS maintainer) in case he has any relevant information. Thanks, Paul [1] https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00360.html
diff --git a/arch/mips/include/asm/compiler.h b/arch/mips/include/asm/compiler.h index e081a265f422..1e9548faf9c7 100644 --- a/arch/mips/include/asm/compiler.h +++ b/arch/mips/include/asm/compiler.h @@ -8,6 +8,36 @@ #ifndef _ASM_COMPILER_H #define _ASM_COMPILER_H +/* + * With GCC v4.5 onwards can use __builtin_unreachable to indicate to the + * compiler that a particular code path will never be hit. This allows it to be + * optimised out of the generated binary. + * + * Unfortunately GCC from at least v4.9.2 to current head of tree as of May + * 2016 suffer from a bug that can lead to instructions from beyond an + * unreachable statement being incorrectly reordered into earlier delay slots + * if the unreachable statement is the only content of a case in a switch + * statement. This can lead to seemingly random behaviour, such as invalid + * memory accesses from incorrectly reordered loads or stores. See this + * potential GCC fix for details: + * + * https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00360.html + * + * GCC also handles stack allocation suboptimally when calling noreturn + * functions or calling __builtin_unreachable(): + * + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365 + * + * We work around both of these issues by placing a volatile asm statement, + * which GCC is prevented from reordering past, prior to __builtin_unreachable + * calls. + * + * The .insn statement is required to ensure that any branches to the + * statement, which sadly must be kept due to the asm statement, are known to + * be branches to code and satisfy linker requirements for microMIPS kernels. + */ +#define barrier_before_unreachable() asm volatile(".insn") + #if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4) #define GCC_IMM_ASM() "n" #define GCC_REG_ACCUM "$0"