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

Chris Li <[email protected]> Tue, 17 Mar 2026 00:41:33 -0700
Newsgroups org.kernel.vger.linux-sparse
Message-ID <CACePvbUQtojHiCmGo+R-64W_yrofxc_J5TDTQ8--WWs+4y0aEA@mail.gmail.com>
On Sun, Mar 15, 2026 at 11:53=E2=80=AFPM Al Viro <[email protected]> =
wrote:
>
> On Thu, Feb 26, 2026 at 07:29:45AM +0000, Al Viro wrote:
> > On Wed, Feb 25, 2026 at 10:18:51PM +0000, Al Viro wrote:
> >
> > > NOTE: substitute() is the second hottest loop in the entire thing; on=
ly
> > > tokenizer is hotter.  And gcc is too enthusiastic about the inlining
> > > around that function, ending up with bad register spills, along with
> > > a bunch of stalls.  Worse, decisions are sensitive to minor changes i=
n
> > > places textually far away, making it a real bitch to deal with.
> > > Makes for fun reordering the commits in local queue... ;-/
> >
> > FWIW, looking at that thing again, I wonder if we would be better off
> > with doing argument expansion on demand rather than doing it in
> > expand_arguments().  Should be doable with a bit of care - we'd need
> > to mark the TOKEN_..._ARG with several bits to decide whether we
> > want to duplicate or not, etc., but that's worth doing anyway -
> > better than playing with the counters.
> >
> > Note, BTW, that collapsing TOKEN_..._ARG together, with "kind of argume=
nt"
> > moved into bits stolen from ->argnum improves code generation - that
> > switch by token type is _hot_ and it reducing the number of cases
> > gives a measurable speedup.  Sure, we don't want heavy work at #define
> > time - most of the macros are never expanded at all, but AFAICS this
> > kind of processing can be dealt with while parsing the body, with no
> > extra passes needed, etc.
> >
> > I'm going down right now, will look into that tomorrow morning...
>
> That turned out to be trickier than I hoped, but I've got something that
> works.
>
> See git://git.kernel.org/pub/scm/linux/kernel/git/viro/sparse.git #va_opt
> (or individual patches in followups)

Nice, I applied the patch at the sparse-dev repo:
https://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git/

I will push to the stable repository and make a cut soon if no issues
are reported.

Chris

>
> __VA_OPT__ supported, AFAICS behaviour matches C23.
>         * expansion and stringifying of arguments is full-lazy now -
> done on demand and at most once.
>         * va-opt-replacement parsed at #define time, handled correctly
> by dump_macro() (i.e. -dM), comparisons when redefining and at expansion
> time.
>         * arglist mangling is gone, so's the argcount kludge.
>         * it's no slower than it used to be prior to that series.
>
> I have local followups (tentative fixes for whitespace handling in prepro=
cessor
> and optimizations in tokenizer), but let's deal with that one first.
>
> Shortlog:
> Al Viro (21):
>       split copy() into "need to copy" and "can move in place" cases
>       expand and simplify the call of dup_token() in copy()
>       more dup_token() optimizations
>       parsing #define: saner handling of argument count, part 1
>       simplify collect_arguments() and fix error handling there
>       try_arg(): don't use arglist for argument name lookups
>       make expand_has_...() responsible for expanding its argument
>       preparing to change argument number encoding for TOKEN_..._ARGUMENT
>       steal 2 bits from argnum for argument kind
>       on-demand argument expansion
>       kill create_arglist()
>       stop mangling arglist, get rid of TOKEN_ARG_COUNT
>       deal with ## on arguments separately
>       preparations for __VA_OPT__ support: reshuffle argument slot assign=
ments
>       pre-process.c: split try_arg()
>       __VA_OPT__: parsing
>       expansion-time va_opt handling
>       merge(): saner handling of ->noexpand
>       simplify the calling conventions of collect_arguments()
>       make expand_one_symbol() inline
>       substitute(): convert switch() into cascade of ifs
>
> Diffstat:
>  ident-list.h                                |   1 +
>  pre-process.c                               | 929 +++++++++++++++++-----=
------
>  symbol.h                                    |   1 +
>  token.h                                     |  32 +-
>  tokenize.c                                  |   4 -
>  validation/preprocessor/bad-args.c          |  18 +
>  validation/preprocessor/dump-macro.c        |  13 +
>  validation/preprocessor/has-attribute.c     |   3 +
>  validation/preprocessor/has-builtin.c       |   3 +
>  validation/preprocessor/va_opt.c            |  54 ++
>  validation/preprocessor/va_opt2.c           |  34 +
>  validation/preprocessor/va_opt_compare.c    |  28 +
>  validation/preprocessor/va_opt_parse.c      |  37 ++
>  validation/preprocessor/va_opt_whitespace.c |  14 +
>  14 files changed, 797 insertions(+), 374 deletions(-)
>  create mode 100644 validation/preprocessor/bad-args.c
>  create mode 100644 validation/preprocessor/dump-macro.c
>  create mode 100644 validation/preprocessor/va_opt.c
>  create mode 100644 validation/preprocessor/va_opt2.c
>  create mode 100644 validation/preprocessor/va_opt_compare.c
>  create mode 100644 validation/preprocessor/va_opt_parse.c
>  create mode 100644 validation/preprocessor/va_opt_whitespace.c
>
>
> PS: as for the interesting uses of __VA_OPT__, consider this:
> ; cat >test.c <<'EOF'
> // based on a fun trick from David Mazi=C3=A8res
> // see https://www.scs.stanford.edu/~dm/blog/va-opt.html for the entire s=
tory
> // No, it's not unbounded recursion - up to 256 (4^4) elements in __VA_AR=
GS__;
> // more with trivial modifications, just add more levels to EXPAND...
> #define PARENS ()
> #define EXPAND(...) EXPAND4(EXPAND4(EXPAND4(EXPAND4(__VA_ARGS__))))
> #define EXPAND4(...) EXPAND3(EXPAND3(EXPAND3(EXPAND3(__VA_ARGS__))))
> #define EXPAND3(...) EXPAND2(EXPAND2(EXPAND2(EXPAND2(__VA_ARGS__))))
> #define EXPAND2(...) EXPAND1(EXPAND1(EXPAND1(EXPAND1(__VA_ARGS__))))
> #define EXPAND1(...) __VA_ARGS__
> #define FOR_EACH_PAIR(macro, ...)                               \
>   __VA_OPT__(EXPAND(FOR_EACH_PAIR_HELPER(macro, __VA_ARGS__)))
> #define FOR_EACH_PAIR_HELPER(macro, a1, a2, ...)                \
>   macro(a1, a2)                                                 \
>   __VA_OPT__(FOR_EACH_PAIR_AGAIN PARENS (macro, __VA_ARGS__))
> #define FOR_EACH_PAIR_AGAIN() FOR_EACH_PAIR_HELPER
>
> FOR_EACH_PAIR(F, t1, id1, t2, id2, t3, id3, t4, id4, t5, id5, t6, id6)
> EOF
> ; cpp -E test.c
> # 0 "test.c"
> # 0 "<built-in>"
> # 0 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "<command-line>" 2
> # 1 "test.c"
> # 18 "test.c"
> F(t1, id1) F(t2, id2) F(t3, id3) F(t4, id4) F(t5, id5) F(t6, id6)
> ;
>
> and the same output from sparse, modulo the # ... lines - sparse -E doesn=
't
> produce those.  Our (fairly brittle) analogue is __MAP in linux/syscalls.=
h
> and if nothing else, unlike __MAP() this thing does not need the number
> of pairs passed as explicit argument.  Would be interesting to try unifyi=
ng
> SYSCALL0..SYSCALL6 into a single macro that would bloody well _count_ the
> arguments...
>