Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH v2] x86/asm: fix assembly constraints in bitops | Date | Fri, 5 Apr 2019 11:12:03 +0000 |
| |
From: Ingo Molnar > Sent: 05 April 2019 10:40 > > * 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. > > ... > > 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.
The missing memory clobber (change 1) can cause very difficult to debug bugs. Simple things like gcc deciding to inline a function can change the order of memory accesses. Having the wrong just isn't worth the trouble it can cause.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
| |