Messages in this thread | | | From | Nadav Amit <> | Subject | Re: [PATCH 2/6] x86: bug: prevent gcc distortions | Date | Fri, 18 May 2018 14:30:28 +0000 |
| |
Ingo Molnar <mingo@kernel.org> wrote:
> > * Peter Zijlstra <peterz@infradead.org> wrote: > >> On Thu, May 17, 2018 at 09:13:58AM -0700, Nadav Amit wrote: >>> +asm(".macro __BUG_FLAGS ins:req file:req line:req flags:req size:req\n" >>> + "1:\t \\ins\n\t" >>> + ".pushsection __bug_table,\"aw\"\n" >>> + "2:\t "__BUG_REL(1b) "\t# bug_entry::bug_addr\n\t" >>> + __BUG_REL(\\file) "\t# bug_entry::file\n\t" >>> + ".word \\line" "\t# bug_entry::line\n\t" >>> + ".word \\flags" "\t# bug_entry::flags\n\t" >>> + ".org 2b+\\size\n\t" >>> + ".popsection\n\t" >>> + ".endm"); >>> + >>> +#define _BUG_FLAGS(ins, flags) \ >>> do { \ >>> + asm volatile("__BUG_FLAGS \"" ins "\" %c0 %c1 %c2 %c3" \ >>> + : : "i" (__FILE__), "i" (__LINE__), \ >>> + "i" (flags), \ >>> "i" (sizeof(struct bug_entry))); \ >>> } while (0) >> >> This is an awesome hack, but is there really nothing we can do to make >> it more readable? Esp, that global asm doing the macro definition is a >> pain to read. >> >> Also, can we pretty please used named operands in 'new' code? > > Yes, that's my main worry too about all these inlining changes: > the very, very marked reduction in the readability of assembly code. > > It's bad to an extent that I'm questioning the wisdom of pandering to a compiler > limitation to begin with? > > How about asking GCC for an attribute where we can specify the inlined size of an > asm() function? Even if we'll just approximate it due to some vagaries of actual > code generation related to how arguments interact with GCC, an explicit byte value > will do a heck of a better job of it than some sort of implied, vague 'number of > newlines' heuristics ...
If it were to become a GCC feature, I think it is best to be a builtin that says: consider the enclosed expression as “free”. The problem of poor inlining decisions is not specific to inline asm. As I mentioned in the RFC, when there are two code paths for constants and variables based on __builtin_constant_p(), you can get the “cost” of the constant path for variables.
It is not hard to add such a feature to GCC, but I don’t know how easy it is to get new features into the compiler.
| |