Re: [PATCH 0/5] i386: Optimize for Jump Conditional Code Erratum

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Dec 3, 2019 at 8:45 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2019-12-03, Jeff Law wrote:
> >On 12/3/19 1:19 PM, H.J. Lu wrote:
> >>
> >>> 4. Shall we default to -mbranches-within-32B-boundaries if the specified
> >>>    -march= or -mtune= may be affected by the erratum?
> >>
> >> No. It’s a performance mitigation for the microcode update not a
> >> functional fix. While it can mitigate the potential performance effect
> >> in most cases as we observed, it increases the code size and may harm
> >> the performance in some cases. It may also impact the performance of
> >> those architectures which are not affected by this JCC erratum.
> >>
> >> Software mitigation cannot be applied in some scenarios where
> >> application behavior is dependent on exact code size. In other words,
> >> the inserted padding (prefix, nop) may break the assumption of code
> >> size that the programmer has made.  We have observed such assumptions
> >> in the compilation of the Linux kernel.
>
> Padding instructions with prefixes instead of inserting multi-byte NOPs
> may be a generally-useful feature. This may be very useful if it can be
> applied at basic-block level, especially for loops. It will be very nice

This will require a new directive.

> if profile guided optimizations can insert some directives to guide the
> prefix placement in appropriate positions. This part should probably be
> made a bit more general so that it can be reused by performance
> improvement changes.

It can be extended to do it.  But I prefer to do it as a followup change after
it has been used for a while.
> >ISTM those cases (like the kernel startup code) could/should opt-opt,
> >possibly at the file level since IIRC it's just one assembly file where
> >the sizes of jumps are supposed to be fixed.
>
> Inserting prefixes at arbitrary positions can break assembly like:
>
> .if . - label == 4
>
> Such constructs are very rare. I've checked Linux kernel, there are
> indeed a few tricky constructs, only one instance is relevant, though:
>
>    // arch/arm/include/asm/assembler.h
>    .if . - 9997b == 2
>    // tools/testing/selftests/x86/sysret_rip.c (only this one is relevant)
>    .ifne . - test_page - 4096
>    // arch/powerpc/include/asm/head-64.h
>    .if (. - name > (start) + (size) - name##_start)
>
> >> Therefore we do not enable it by default. The user should evaluate its
> >> impact and make their own determination as to whether to enable the
> >> software mitigation  knowing that when this option is enabled, the
> >> performance impact may vary case-by-case.
> >The problem with not enabling it by default is a distro would have to
> >inject the flag into their builds.  It's not uncommon for injection
> >mechanisms to not work on packages like gcc, glibc, etc.
>
> The code size increase (3-4%) is large. In gcc, if an optimization can
> improve performance by a% at the cost of >a% code size increase, is it
> considered as a good trade-off for -O2? -O1? -Os?
>
> The performance decrease may not even be perceived for lots of software
> in a distribution. Opt-in may be a good first choice when we still lack
> statistics/feedback from users.
>
> If we have profile information, we can teach GCC to insert some
> directives at basic-block/function/file level to hint that jump
> instructions in some code sequences need more care.


-- 
H.J.