On Fri, Feb 9, 2024 at 9:39 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Fri, 9 Feb 2024 at 11:01, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > We should also probably get rid of the existing "asm_volatile_goto()" > > macro name entirely. That name was always pretty horrific, in that it > > didn't even mark the asm as volatile even in the case where it did > > anything. > > > > So the name of that macro made little sense, and the new workaround > > should be only for asm goto with outputs. So I'd suggest jmaking a new > > macro with that name: > > > > #define asm_goto_output(x...) > > > > and on gcc use that old workaround, and on clang just make it be a > > plain "asm goto". > > So here's a suggested patch that does this. > > It's largely done with "git grep" and "sed -i", plus some manual > fixups for the (few) cases where we have outputs. > > It looks superficially sane to me, and it passed an allmodconfig build > with gcc, but I'm not going to claim that it is really tested. > > Sean? Does this work for the case you noticed? > > Basically this gets rid of the old "asm_volatile_goto()" entirely as > useless, but replaces it with "asm_goto_outputs()" for the places > where we have outputs. > > And then for gcc, it makes those cases > > (a) use "asm volatile goto" to fix the fact that some versions of gcc > will have missed the "volatile" > > (b) adds that extra empty asm as a second barrier after the "real" > asm goto statement > > That (b) is very much voodoo programming, but it matches the old magic > barrier thing that Jakub Jelinek suggested for the really *old* gcc > bug wrt plain (non-output) "asm goto". The underlying bug for _that_ > was fixed long ago: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670 > > We removed that for plain "asm goto" workaround a couple of years ago, > so "asm_volatile_goto()" has been a no-op since June 2022, but this > now resurrects that hack for the output case. > > I'm not loving it, but Sean seemed to confirm that it fixes the code > generation problem, so ... > > Adding Uros to the cc, since he is both involved with gcc and with the > previous asm goto workaround removal, so maybe he has other > suggestions. Uros, see > > https://lore.kernel.org/all/20240208220604.140859-1-seanjc@xxxxxxxxxx/ I'd suggest the original poster to file a bug report in the GCC bugzilla. This way, the bug can be properly analysed and eventually fixed. The detailed instructions are available at https://gcc.gnu.org/bugs/ > for background. > > Also adding Jakub since I'm re-using the hack he suggested for a > different - but similar - case. He may have strong opinions too, and > may object to that particular monkey-see-monkey-do voodoo programming. This big-hammer approach will effectively disable all optimizations around the asm, and should really be used only as a last resort. As learned from the PR58670 (linked above), these workarounds can stay in the kernel forever, and even when the compiler is fixed, there is little incentive to remove them - developers from the kernel world do not want to touch them, because "the workaround just works", and developers from the compiler world do not want to touch them either, because of unknown hidden consequences of removal. Please note, that the kernel is a heavy user of asm statements, and in GCC some forms of asm statements were developed specifically for kernel use, e.g. CC-output, and asm goto in all of its flavors. They have little or no use outside the kernel. If the kernel disables all optimizations around these statements, there actually remain no real-world testcases of how compiler optimizations interact with asm statements, and when new optimizations are introduced to the compiler, asm statemets are left behind (*). (*) When the GCC compiler nears its release, people start to test it on their code bases, and the kernel is one of the most important code bases as far as the compiler is concerned. We count on bugreports from these tests to fix possible fall-out from new optimizations, and the big-hammer workaround will just hide the possible problems. So, I'd suggest at least limit the workaround to known-bad compilers. Thanks, Uros.
