[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=whvS9x5NKtOqcUgJeTY7dfdAHcEALJT53cy3P7Hzfgr1g@mail.gmail.com>
Date: Fri, 7 Feb 2020 13:22:49 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Luc Van Oostenryck <luc.vanoostenryck@...il.com>
Cc: Martin KaFai Lau <kafai@...com>, bpf@...r.kernel.org,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
David Miller <davem@...emloft.net>, kernel-team@...com,
Linux-Sparse <linux-sparse@...r.kernel.org>,
Netdev <netdev@...r.kernel.org>,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH bpf] bpf: Improve bucket_log calculation logic
On Fri, Feb 7, 2020 at 12:41 PM Luc Van Oostenryck
<luc.vanoostenryck@...il.com> wrote:
>
> I never saw it so badly but it's not the first time I've bitten by
> the very early inlining. Independently of this, it would be handy to
> have an inliner at IR level, it shouldn't be very difficult but ...
> OTOH, it should really be straightforward would be to separate the
> current tree inliner from the type evaluation and instead run it just
> after expansion. The downsides would be:
> * the tree would need to be walked once more;
Actually, if we were to do the inlining _during_ expansion, we
wouldn't add a new phase.
> * this may make the expansion less useful but it could be run again
> after the inlining.
Same comment: how about doing it as part of the expansion phase?
This is where we handle the built-ins too, it would kind of make sense
to do inlining in expand_symbol_call(), I feel. An inline function is
a "builtin" that the user has defined, after all.
And if we do it in that phase, we'd automatically avoid it for
conditional expressions with a static conditional value, because
expansion does the obvious trivial simplification as it goes along,
and never expands the side that is trivially not seen.
Something like the attached completely broken patch. It builds but
doesn't work, because "inline_function()" is currently designed to
work during evaluation, not during expansion.
So the patch is complete garbage, but maybe could be the starting
point for something that works.
Linus
View attachment "broken.diff" of type "text/x-patch" (1117 bytes)
Powered by blists - more mailing lists