Re: [RFC PATCH] pre-process: add __VA_OPT__ support

Al Viro <[email protected]> Thu, 19 Mar 2026 03:53:24 +0000
Newsgroups org.kernel.vger.linux-sparse
Message-ID <20260319035324.GG3836593@ZenIV>
On Mon, Mar 16, 2026 at 09:42:01AM -0700, Linus Torvalds wrote:
> I have tested that branch on a few trivial cases, and it looks good to me.
> 
> I did write a long rant about how I hate cpp tricks and wish we had a
> few simple extensions (__VA_COUNT__ would be the most simple one,
> because COUNT_ARGS() is disgusting), but it is what it is, and this
> makes things better. So I decided to just delete my rant.

Speaking of rants: gcc code generation in general and around bitfields.  
Example: in collect_arg() we have
		next->pos.stream = pos->stream;
		next->pos.line = pos->line;
		next->pos.pos = pos->pos;
		next->pos.newline = 0;
in a loop, pos is declared const struct position *.  Generates the
following horror:
        movzwl  2(%r12), %edx
        movl    (%r12), %ecx
        leaq    8(%rax), %rbp
        shrw    $6, %dx
        andl    $1048512, %ecx
        movzwl  %dx, %edx
        salq    $22, %rdx
        orq     %rcx, %rdx
        movl    4(%r12), %ecx
        andl    $2147483647, %ecx
        salq    $32, %rcx
        orq     %rcx, %rdx
        movq    (%rax), %rcx
        andq    %r14, %rcx
        orq     %rcx, %rdx
        movq    %rdx, (%rax)
r12 is 'pos', rax - 'next', r14 comes from
        movabsq $-9223372036852678593, %r14
in the beginning of the function (0x800000000020003f).  Note that
*everything* prior to the last 4 insns is equivalent to
	rbp = &(struct token *)rax->next
	rdx = *(u64 *)r12 & 0x7fffffffffc0xfffc0
written in a really convoluted way.
OK, so it doesn't figure out it could bloody well calculate that rdx
value once and store in some register (the same r12, for that matter).
Let's make it simple for the damn thing - pass struct position instead
of struct position *; what we get is
	movq    %r12, %rdx
	leaq    8(%rax), %rbp
	movabsq $9223372032559808512, %rcx
	andl    $1048512, %edx
	andq    %r12, %rcx
	orq     %r14, %rdx
	orq     %rcx, %rdx
	movabsq $-9223372036852678593, %rcx
	andq    (%rax), %rcx
	orq     %rcx, %rdx
	movq    %rdx, (%rax)
What the hell?  No, really - it's
	rdx = r12;
	rbp = &(struct token *)rax->next;
	rcx = 0x7fffffff00000000;
	rdx &= 0xfffc0;
	rcx &= r12;
	rdx |= rcx;
followed by
	rcx = 0x800000000020003f & *(u64 *)rax;
	rdx |= rcx;
	*(u64 *)rax = rdx;
Leaving aside the utility of repeating the same calculation on each
iteration of the loop, figuring out that 
	(0x7fffffff00000000 & r12) | (0xfffc0 & r12)
is equal to
	0x7fffffffffc0fffc0 & r12
ought to be within the abilities of the damn compiler - and it *is*
loading a 64bit constant into rcx as it is.  What's more, it's not
a preference to using 64bit constants with lower 32 bits clear -
another movabsq in the same chunk is not of that form.

Perhaps it's an explicit store of 0 to ->newline that does it?
Clearing pos.newline in the beginning and have 
		next->pos.newline = pos.newline;
instead of zeroing in the loop does not change anything (other than
worse register allocation).  Moving zeroing of pos.newline into the
caller finally gets that calculation out of loop... and messes with
the register allocation in the caller, which is inlined into expand(),
along with substitute(), do_argument() and quite a few other things.

Granted, collect_arg() is not particularly hot, but... ouch.
I really, really don't like the handling of bitfields ;-/