Messages in this thread | | | Date | Fri, 15 Sep 2017 11:10:41 -0500 | From | Josh Poimboeuf <> | Subject | Re: [RFC PATCH 3/4] x86/asm: Make alternative macro interfaces more clear and consistent |
| |
On Thu, Sep 14, 2017 at 01:45:29PM -0500, Josh Poimboeuf wrote: > On Thu, Sep 14, 2017 at 11:28:30AM -0700, Linus Torvalds wrote: > > On Thu, Sep 14, 2017 at 10:33 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > >> > > >> a) uglifying the 15 or so relevant inline asm locations with ifdefs; or > > > > > > Actually I guess we could put the "sp" in a macro... I'll try it. > > > > Exactly. Do something like > > > > #ifdef CONFIG_FRAME_POINTER > > # define EXTRA_ASM_CLOBBERS "rsp" > > #else > > # define EXTRA_ASM_CLOBBERS > > #endif > > > > and then replace the nasty > > > > register void *__sp asm(_ASM_SP); > > .. > > "+r" (__sp) > > > > games with just that EXTRA_ASM_CLOBBERS thing at the end of the clobbers. > > > > Yes, you'd probably have to document that the alternative_call_2() > > thing doesn't take a "input" argument, but a input_and_clobbers, but > > all users do that anyway. > > > > I dunno. > > There's also alternative_call(), which doesn't yet have the '__rsp' > annotation, but it probably should. It has some callers which pass > clobbers and some which don't, so its conversion would be trickier. > > So my plan is to keep patch 3 of this series, which clarifies those > alternative macro interfaces, and also separates the inputs from the > clobbers. That'll make it really easy to add something like > EXTRA_ASM_CLOBBERS above. > > In fact I'll probably keep patches 1-3, because they're all > improvements. Then I'll replace the original patch 4 (ASM_CALL) with > the "sp" clobbers thing.
So I couldn't figure out how to make it any simpler than this:
#ifdef CONFIG_FRAME_POINTER # define ASM_CALL_CLOBBERS "sp" # define ASM_CALL_CLOBBERS_APPEND , ASM_CALL_CLOBBERS # define ASM_CALL_CLOBBERS_ARGS(args...) ASM_CALL_CLOBBERS, ## args #else # define ASM_CALL_CLOBBERS # define ASM_CALL_CLOBBERS_APPEND # define ASM_CALL_CLOBBERS_ARGS(args...) args #endif
ASM_CALL_CLOBBERS is the normal one:
asm volatile("call foo" : : : ASM_CALL_CLOBBERS);
ASM_CALL_CLOBBERS_APPEND is needed when combining the option with other clobbers options, like:
asm volatile("call foo" : : : "memory" ASM_CALL_CLOBBERS_APPEND);
ASM_CALL_CLOBBERS_ARGS is needed for the pesky alternative_call() macro so it can work with the variadic argument:
#define alternative_call(oldfunc, newfunc, feature, outputs, inputs, \ clobbers...) \ asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", \ feature), \ : outputs \ : [old] "i" (oldfunc), [new] "i" (newfunc) \ ARGS_APPEND(inputs) \ : ASM_CALL_CLOBBERS_ARGS(clobbers))
So I *was* about ready to post something like the above. But, of course, the kbuild robot found that the new version of my patches manages to crash GCC with a certain randconfig.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82221
So the saga continues. Now I've managed to break clang, sparse, and GCC with each consecutive iteration of these patches... I'm starting to lose my faith in compilers.
-- Josh
| |