Messages in this thread | | | Date | Fri, 5 Apr 2019 11:39:31 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v2] x86/asm: fix assembly constraints in bitops |
| |
* Alexander Potapenko <glider@google.com> wrote:
> 1. Use memory clobber in bitops that touch arbitrary memory > > Certain bit operations that read/write bits take a base pointer and an > arbitrarily large offset to address the bit relative to that base. > Inline assembly constraints aren't expressive enough to tell the > compiler that the assembly directive is going to touch a specific memory > location of unknown size, therefore we have to use the "memory" clobber > to indicate that the assembly is going to access memory locations other > than those listed in the inputs/outputs. > To indicate that BTR/BTS instructions don't necessarily touch the first > sizeof(long) bytes of the argument, we also move the address to assembly > inputs. > > This particular change leads to size increase of 124 kernel functions in > a defconfig build. For some of them the diff is in NOP operations, other > end up re-reading values from memory and may potentially slow down the > execution. But without these clobbers the compiler is free to cache > the contents of the bitmaps and use them as if they weren't changed by > the inline assembly. > > 2. Use byte-sized arguments for operations touching single bytes. > > Passing a long value to ANDB/ORB/XORB instructions makes the compiler > treat sizeof(long) bytes as being clobbered, which isn't the case. This > may theoretically lead to worse code in the case of heavy optimization. > > Signed-off-by: Alexander Potapenko <glider@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Paul E. McKenney <paulmck@linux.ibm.com> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: James Y Knight <jyknight@google.com> > --- > v2: > -- renamed the patch > -- addressed comment by Peter Zijlstra: don't use "+m" for functions > returning void > -- fixed input types for operations touching single bytes > --- > arch/x86/include/asm/bitops.h | 41 +++++++++++++++-------------------- > 1 file changed, 18 insertions(+), 23 deletions(-)
I'm wondering what the primary motivation for the patch is:
- Does it fix an actual miscompilation, or only a theoretical miscompilation?
- If it fixes an existing miscompilation:
- Does it fix a miscompilation triggered by current/future versions of GCC? - Does it fix a miscompilation triggered by current/future versions of Clang?
- Also, is the miscompilation triggered by 'usual' kernel configs, or does it require exotics such as weird debug options or GCC plugins, etc?
I.e. a bit more context would be useful.
Thanks,
Ingo
| |