llvm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	 x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	 Nathan Chancellor <nathan@kernel.org>, Tom Rix <trix@redhat.com>,
	linux-kernel@vger.kernel.org,  llvm@lists.linux.dev,
	David Howells <dhowells@redhat.com>,
	Jan Beulich <JBeulich@suse.com>,
	 Christophe Jaillet <christophe.jaillet@wanadoo.fr>,
	Joe Perches <joe@perches.com>,
	 Josh Poimboeuf <jpoimboe@kernel.org>,
	Yury Norov <yury.norov@gmail.com>
Subject: Re: [PATCH v7 0/2] x86/asm/bitops: optimize ff{s,z} functions for constant expressions
Date: Wed, 7 Sep 2022 16:49:55 +0900	[thread overview]
Message-ID: <CAMZ6RqLH3MdcRdxDFFmy_gbepJ-tU-mqFxBg+jS8=niR4xu71A@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOd=PgVtCwjS=Z_iqqmWsQ=+HsnEnHX2LOjRPCYFoVhAZhA@mail.gmail.com>

On Wed. 7 Sep. 2022 at 16:04, Nick Desaulniers <ndesaulniers@google.com> wrote:
> On Tue, Sep 6, 2022 at 11:26 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Sun, Sep 4, 2022 at 5:38 PM Vincent Mailhol
> > <mailhol.vincent@wanadoo.fr> wrote:
> > >
> > > The compilers provide some builtin expression equivalent to the ffs(),
> > > __ffs() and ffz() functions of the kernel. The kernel uses optimized
> > > assembly which produces better code than the builtin
> > > functions. However, such assembly code can not be folded when used
> > > with constant expressions.
> >
> > Another tact which may help additional sources other than just the
> > Linux kernel; it seems that compilers should be able to fold this.

Initially, I thought that you were suggesting folding the asm code
(which doesn’t seem trivial at all).

> > Vincent, if you're interested in making such an optimization in LLVM,
> > we'd welcome the contribution, and I'd be happy to show you where to
> > make such changes within LLVM; please let me know off thread.
>
> Oh right, it already does.
> https://github.com/llvm/llvm-project/blob/ea953b9d9a65c202985a79f1f95da115829baef6/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp#L2635
> I see what's happening. Constant propagation sinks constants into a
> specialized version of ffs when there's only 1 callsite in a given
> translation unit (or multiple call sites with the same constant).
> Then dead argument elimination removes the argument, so libcall
> optimization thinks this isn't the ffs(int) you're looking for, and
> skips it.

Isn’t it a wise decision to skip it? How should the optimization be
able to decide that the redefined ffs() is equivalent to
__builtin_ffs()?

More generally, if I write my own foo() which shadows a
__builtin_foo() function, the two functions might do something totally
different and I would be pissed off if the compiler decided to
constant-fold my foo().

Dummy example:

===================
char *s;

/* ffs: fast forward string
 * @i: how many bytes to move forward
 *
 * Move forward the global s pointer by @i or strlen(s) (whoever is smaller).
 *
 * Return: how many bytes we move forward.
 */
int ffs(int i)
{
        int len = strlen(s);
        int forward = i < len ? i : len;

        s += forward;
        return forward;
}
===================

How would you instruct the compiler to constant-fold the kernel’s
ffs() but not fold above dummy ffs()?

> Nice.
> https://github.com/llvm/llvm-project/issues/57599
> I guess ffs() is usually forward declared in strings.h, so we don't
> have such a static inline definition available to constant
> prop/specialize in normal C code.

  reply	other threads:[~2022-09-07  7:50 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-11 16:03 [PATCH v2 0/2] x86/asm/bitops: optimize ff{s,z} functions for constant expressions Vincent Mailhol
2022-05-11 16:03 ` [PATCH v2 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-05-11 20:56   ` Christophe JAILLET
2022-05-11 23:30     ` Vincent MAILHOL
2022-05-11 21:35   ` Nick Desaulniers
2022-05-11 23:48     ` Vincent MAILHOL
2022-05-11 16:03 ` [PATCH v2 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-05-11 22:20   ` Nick Desaulniers
2022-05-11 23:23     ` Vincent MAILHOL
2022-05-12  0:03 ` [PATCH v3 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent Mailhol
2022-05-12  0:03   ` [PATCH v3 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-05-12  0:28     ` Nick Desaulniers
2022-05-12  1:18       ` Vincent MAILHOL
2022-05-12  0:03   ` [PATCH v3 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-05-12  0:19     ` Nick Desaulniers
2022-05-12  1:18 ` [PATCH v4 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent Mailhol
2022-05-12  1:18   ` [PATCH v4 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-05-12  3:02     ` Joe Perches
2022-05-12  4:29       ` Vincent MAILHOL
2022-05-12  1:18   ` [PATCH v4 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-05-23  9:22   ` [PATCH v4 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent MAILHOL
2022-06-25  7:26 ` [RESEND PATCH " Vincent Mailhol
2022-06-25  7:26   ` [RESEND PATCH v4 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-06-25  7:26   ` [RESEND PATCH v4 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-07-23 15:15 ` [RESEND PATCH v4 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent Mailhol
2022-07-23 15:15   ` [RESEND PATCH v4 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-08-11 14:59     ` Borislav Petkov
2022-08-12 11:55       ` Vincent MAILHOL
2022-07-23 15:15   ` [RESEND PATCH v4 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-07-29 11:24   ` [RESEND PATCH v4 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent MAILHOL
2022-07-29 12:22     ` Borislav Petkov
2022-07-29 13:50       ` Vincent MAILHOL
2022-08-12 11:44 ` [PATCH v5 " Vincent Mailhol
2022-08-12 11:44   ` [PATCH v5 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-08-12 11:44   ` [PATCH v5 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-08-23 16:23     ` Borislav Petkov
2022-08-23 17:12       ` Nick Desaulniers
2022-08-23 17:43         ` Borislav Petkov
2022-08-23 20:31           ` Vincent MAILHOL
2022-08-24  8:43             ` Borislav Petkov
2022-08-24 12:10               ` Vincent MAILHOL
2022-08-24 13:24                 ` Borislav Petkov
2022-08-26 21:32                   ` Vincent MAILHOL
2022-09-07  4:06                     ` Borislav Petkov
2022-09-07  5:35                       ` Vincent MAILHOL
2022-09-07  8:50                         ` Borislav Petkov
2022-08-31  7:57 ` [PATCH v6 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Vincent Mailhol
2022-08-31  7:57   ` [PATCH v6 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-08-31  7:57   ` [PATCH v6 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-08-31  8:51   ` [PATCH v6 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Yury Norov
2022-09-01  3:49     ` Yury Norov
2022-09-01 10:30       ` Vincent MAILHOL
2022-09-01 14:19         ` Yury Norov
2022-09-01 17:06           ` Nick Desaulniers
2022-09-02  5:34             ` Borislav Petkov
2022-09-02  0:41           ` Vincent MAILHOL
2022-09-02  1:19     ` Vincent MAILHOL
2022-09-05  0:37 ` [PATCH v7 " Vincent Mailhol
2022-09-05  0:37   ` [PATCH v7 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-09-05  0:37   ` [PATCH v7 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol
2022-09-06 18:26   ` [PATCH v7 0/2] x86/asm/bitops: optimize ff{s,z} functions for " Nick Desaulniers
2022-09-07  7:04     ` Nick Desaulniers
2022-09-07  7:49       ` Vincent MAILHOL [this message]
2022-09-07  9:09 ` [PATCH v8 " Vincent Mailhol
2022-09-07  9:09   ` [PATCH v8 1/2] x86/asm/bitops: ffs: use __builtin_ffs to evaluate " Vincent Mailhol
2022-09-07  9:09   ` [PATCH v8 2/2] x86/asm/bitops: __ffs,ffz: use __builtin_ctzl " Vincent Mailhol

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='CAMZ6RqLH3MdcRdxDFFmy_gbepJ-tU-mqFxBg+jS8=niR4xu71A@mail.gmail.com' \
    --to=mailhol.vincent@wanadoo.fr \
    --cc=JBeulich@suse.com \
    --cc=bp@alien8.de \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=dave.hansen@linux.intel.com \
    --cc=dhowells@redhat.com \
    --cc=hpa@zytor.com \
    --cc=joe@perches.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mingo@redhat.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=trix@redhat.com \
    --cc=x86@kernel.org \
    --cc=yury.norov@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).