On Mon, Oct 22, 2018 at 12:08:30PM +0200, Peter Zijlstra wrote:
> ARGH; so, this:
>
> #define __GEN_RMWcc(fullop, _var, cc, clobbers, ...) \
> ({ \
> bool c = false; \
> asm_volatile_goto (fullop "; j" #cc " %l[cc_label]" \
> : : [var] "m" (_var), ## __VA_ARGS__ \
> : clobbers : cc_label); \
> if (0) { \
> cc_label: c = true; \
> } \
> c; \
> })
>
> static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock
> *lock)
> {
> u32 val = 0;
>
> if (GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", lock->val.counter, c,
> "I", _Q_PENDING_OFFSET))
> val |= _Q_PENDING_VAL;
>
> val |= atomic_read(&lock->val) & ~_Q_PENDING_MASK;
>
> return val;
> }
>
> fails to compile when combined with this:
>
> #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
Rosteeeedt!
> #define __trace_if(cond) \
> if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
> ({ \
> int ______r; \
> static struct ftrace_branch_data \
> __attribute__((__aligned__(4))) \
> __attribute__((section("_ftrace_branch"))) \
> ______f = { \
> .func = __func__, \
> .file = __FILE__, \
> .line = __LINE__, \
> }; \
> ______r = !!(cond); \
> ______f.miss_hit[______r]++; \
> ______r; \
> }))
>
> Because that moves the __GEN_RMWcc into a statement expression and GCC
> apparently doesn't like labels inside statement expressions.
>
> If we avoid if() and rewrite queued_fetch_set_pending_acquire() like so:
>
> static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock
> *lock)
> {
> bool pending;
> u32 val;
>
> pending = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", lock->val.counter, c,
> "I", _Q_PENDING_OFFSET);
>
> val = pending * _Q_PENDING_VAL;
> val |= atomic_read(&lock->val) & ~_Q_PENDING_MASK;
>
> return val;
> }
>
> then it compiles again; but urgh.
>
> Anybody see a better solution?
No, that looks about right to me, but please throw in a comment so that we
don't "fix" this in the future.
Will
