All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Eric Zhang <zxh@xh-zhang.com>
Cc: linux-sparse@vger.kernel.org, dan.carpenter@linaro.org,
	chriscli@google.com, ben.dooks@codethink.co.uk,
	rf@opensource.cirrus.com, torvalds@linux-foundation.org
Subject: Re: [RFC PATCH] pre-process: add __VA_OPT__ support
Date: Mon, 16 Mar 2026 06:56:22 +0000	[thread overview]
Message-ID: <20260316065622.GA607739@ZenIV> (raw)
In-Reply-To: <20260226072945.GA4104757@ZenIV>

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; only
> > 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 in
> > 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 argument"
> 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)

__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 preprocessor
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 assignments
      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ères
// see https://www.scs.stanford.edu/~dm/blog/va-opt.html for the entire story
// No, it's not unbounded recursion - up to 256 (4^4) elements in __VA_ARGS__;
// 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 unifying
SYSCALL0..SYSCALL6 into a single macro that would bloody well _count_ the
arguments...

  reply	other threads:[~2026-03-16  6:53 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1771930766.git.dan.carpenter@linaro.org>
2026-02-24 11:07 ` [PATCH] sparse: add support for __VA_OPT__ Dan Carpenter
2026-02-24 11:16   ` Ben Dooks
2026-02-24 11:56     ` Dan Carpenter
2026-02-24 12:42       ` Richard Fitzgerald
2026-02-24 13:15         ` Ben Dooks
2026-02-25  2:39   ` Chris Li
2026-02-25  3:36     ` Al Viro
2026-02-25  5:29       ` [RFC PATCH] pre-process: add __VA_OPT__ support Eric Zhang
2026-02-25  6:40         ` Al Viro
2026-02-25  7:27           ` Al Viro
2026-02-25  8:14             ` Eric Zhang
2026-02-25 22:18               ` Al Viro
2026-02-26  7:29                 ` Al Viro
2026-03-16  6:56                   ` Al Viro [this message]
2026-03-16  7:03                     ` [PATCH 01/21] split copy() into "need to copy" and "can move in place" cases Al Viro
2026-03-16  7:03                       ` [PATCH 02/21] expand and simplify the call of dup_token() in copy() Al Viro
2026-03-16  7:03                       ` [PATCH 03/21] more dup_token() optimizations Al Viro
2026-03-16  7:03                       ` [PATCH 04/21] parsing #define: saner handling of argument count, part 1 Al Viro
2026-03-16  7:03                       ` [PATCH 05/21] simplify collect_arguments() and fix error handling there Al Viro
2026-03-16  7:04                       ` [PATCH 06/21] try_arg(): don't use arglist for argument name lookups Al Viro
2026-03-16  7:04                       ` [PATCH 07/21] make expand_has_...() responsible for expanding its argument Al Viro
2026-03-16  7:04                       ` [PATCH 08/21] preparing to change argument number encoding for TOKEN_..._ARGUMENT Al Viro
2026-03-16  7:04                       ` [PATCH 09/21] steal 2 bits from argnum for argument kind Al Viro
2026-03-16  7:04                       ` [PATCH 10/21] on-demand argument expansion Al Viro
2026-03-16  7:04                       ` [PATCH 11/21] kill create_arglist() Al Viro
2026-03-16  7:04                       ` [PATCH 12/21] stop mangling arglist, get rid of TOKEN_ARG_COUNT Al Viro
2026-03-16  7:04                       ` [PATCH 13/21] deal with ## on arguments separately Al Viro
2026-03-16  7:04                       ` [PATCH 14/21] preparations for __VA_OPT__ support: reshuffle argument slot assignments Al Viro
2026-03-16  7:04                       ` [PATCH 15/21] pre-process.c: split try_arg() Al Viro
2026-03-16  7:04                       ` [PATCH 16/21] __VA_OPT__: parsing Al Viro
2026-05-04 11:56                         ` Dan Carpenter
2026-03-16  7:04                       ` [PATCH 17/21] expansion-time va_opt handling Al Viro
2026-03-16  7:04                       ` [PATCH 18/21] merge(): saner handling of ->noexpand Al Viro
2026-03-16  7:04                       ` [PATCH 19/21] simplify the calling conventions of collect_arguments() Al Viro
2026-03-16  7:04                       ` [PATCH 20/21] make expand_one_symbol() inline Al Viro
2026-03-16  7:04                       ` [PATCH 21/21] substitute(): convert switch() into cascade of ifs Al Viro
2026-03-16 16:42                     ` [RFC PATCH] pre-process: add __VA_OPT__ support Linus Torvalds
2026-03-19  3:53                       ` Al Viro
2026-03-19  4:07                         ` Linus Torvalds
2026-03-19  5:34                           ` Al Viro
2026-03-17  7:41                     ` Chris Li
2026-03-18  6:35                     ` Eric Zhang
2026-03-31  8:06                     ` Al Viro
2026-03-31  8:07                       ` [PATCH 1/6] nextchar(): get rid of special[] Al Viro
2026-03-31  8:07                         ` [PATCH 2/6] simplify the inlined side of nextchar() Al Viro
2026-03-31  8:07                         ` [PATCH 3/6] tokenize_stream(): don't bother with isspace() Al Viro
2026-03-31  8:07                         ` [PATCH 4/6] TOKEN_DIRECTIVE: recognize directive-introducing # in tokenizer Al Viro
2026-03-31  8:07                         ` [PATCH 5/6] saner collect_arg() code generation Al Viro
2026-03-31  8:07                         ` [PATCH 6/6] try to get whitespaces right Al Viro
2026-04-01 10:39                       ` [RFC PATCH] pre-process: add __VA_OPT__ support Al Viro
2026-04-01 16:18                         ` Linus Torvalds
2026-04-01 19:52                           ` Al Viro
2026-04-01 20:22                             ` Al Viro
2026-02-25  7:05       ` [PATCH] sparse: add support for __VA_OPT__ Chris Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260316065622.GA607739@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=ben.dooks@codethink.co.uk \
    --cc=chriscli@google.com \
    --cc=dan.carpenter@linaro.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=rf@opensource.cirrus.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zxh@xh-zhang.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.