Re: [tip:locking/core 6/10] arch/x86/include/asm/rmwcc.h:23:17: error: jump into statement expression

[Will Deacon]

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