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... >