On Mon, May 04, 2020 at 09:20:03AM +0200, Rasmus Villemoes wrote:
> > So there is something utterly terrible we can do to address both:
> >
> > void __static_call_nop(void)
> > {
> > }
> >
> > #define __static_cond_call(name) \
> > ({ \
> > void *func = READ_ONCE(STATIC_CALL_KEY(name).func); \
> > if (!func) \
> > func = &__static_call_nop; \
> > (typeof(STATIC_CALL_TRAMP(name))*)func; \
> > })
> >
> > #define static_cond_call(name) (void)__static_cond_call(name)
> >
> > This gets us into Undefined Behaviour territory, but it ought to work.
> >
> > It adds the READ_ONCE(), and it cures the argument evaluation issue.
>
> Indeed, that is horrible. And it "fixes" the argument evaluation by
> changing the !HAVE_STATIC_CALL case to match the HAVE_STATIC_CALL, not
> the other way around,
Correct; making it the other way is far more 'interesting'. It would
basically mean combining the static_branch and static_call, but that
would also make it less optimal for simple forwarding cases.
> which means that it is not a direct equivalent to the
>
> if (foo)
> foo(a, b, c)
>
> [which pattern of course has the READ_ONCE issue, but each individual
> existing site with that may be ok for various reasons].
>
> Is gcc smart enough to change the if (!func) to a jump across the
> function call (but still evaluting side effects in args), or is
> __static_call_nop actually emitted and called?
I was hoping it would be clever, but I just tried (find below) and it is
not -- although there's always hoping a newer version / clang might be
smarter.
It does indeed emit the nop function :/
> If the latter, then one
> might as well patch the write-side to do "WRITE_ONCE(foo, func ? :
> __static_call_nop)" and elide the test from __static_cond_call() - in
> fact, that just becomes a single READ_ONCE. [There's probably some
> annoying issue with making sure static initialization of foo points at
> __static_call_nop].
But that would not give a more clever compiler the ability to do the
'right' thing here..
> And that brings me to the other issue I raised - do you have a few
> examples of call sites that could use this, so we can see disassembly
> before/after?
Patch 17 has a few -- which is why I wrote the support in the first
place. Obviously those will never ever hit the !HAVE_STATIC_BRANCH case.
> I'm still concerned that, even if there are no
> side-effects in the arguments, you still force the compiler to
> spill/shuffle registers for call/restore unconditionally, whereas with a
> good'ol if(), all that work is guarded by the load+test.
https://godbolt.org/z/SDRG2q
---
#include <stddef.h>
#define READ_ONCE(var) (*((volatile typeof(var) *)&(var)))
#define WRITE_ONCE(var, val) (*((volatile typeof(var) *)&(var)) = (val))
struct static_call_key {
void *func;
};
#define DECLARE_STATIC_CALL(name, func) \
extern struct static_call_key name; \
extern typeof(func) __SCT__##name;
#define DEFINE_STATIC_COND_CALL(name, _func) \
DECLARE_STATIC_CALL(name, _func) \
struct static_call_key name = { \
.func = NULL, \
}
static void __static_call_nop(void)
{
}
#define __static_cond_call(name) \
({ \
void *func = READ_ONCE(name.func); \
if (!func) \
func = &__static_call_nop; \
(typeof(__SCT__##name)*)func; \
})
#define static_cond_call(name) (void)__static_cond_call(name)
static void inline static_call_update(struct static_call_key *call, void *func)
{
WRITE_ONCE(call->func, func);
}
volatile int _x;
void bar(int x)
{
_x = x;
}
DEFINE_STATIC_COND_CALL(foo, bar);
void ponies(int x)
{
static_cond_call(foo)(x);
}
