From: Linus Torvalds <torvalds@linux-foundation.org>
To: Eric Dumazet <edumazet@google.com>
Cc: David Laight <David.Laight@aculab.com>,
Noah Goldstein <goldstein.w.n@gmail.com>,
kernel test robot <lkp@intel.com>,
"x86@kernel.org" <x86@kernel.org>,
"oe-kbuild-all@lists.linux.dev" <oe-kbuild-all@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"mingo@redhat.com" <mingo@redhat.com>,
"bp@alien8.de" <bp@alien8.de>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"hpa@zytor.com" <hpa@zytor.com>
Subject: Re: x86/csum: Remove unnecessary odd handling
Date: Sat, 6 Jan 2024 11:32:57 -0800 [thread overview]
Message-ID: <CAHk-=wgobnShg4c2yyMbk2p=U-wmnOmX_0=b3ZY_479Jjey2xw@mail.gmail.com> (raw)
In-Reply-To: <CANn89iKjUZjw-9ACNWrEd_H+o79Uwkw9NVuujQ3w=c2pGRFotg@mail.gmail.com>
On Sat, 6 Jan 2024 at 02:26, Eric Dumazet <edumazet@google.com> wrote:
>
> On a related note, at least with clang, I found that csum_ipv6_magic()
> is needlessly using temporary on-stack storage,
> showing a stall on Cascade Lake unless I am patching add32_with_carry() :
This is a known compiler bug in clang:
https://github.com/llvm/llvm-project/issues/20571
https://github.com/llvm/llvm-project/issues/30873
https://github.com/llvm/llvm-project/issues/34837
and while we could always just use "r" for constraints, it will
(a) possibly run out of registers in some situations
(b) pessimize gcc that does this right
Can you please push the clang people to not do the stupid thing they
do now, which is to say "oh, I can use registers _or_ memory, so I'll
always use memory".
Btw, it's actually much worse than that, and clang is just doing
incredibly broken things. Switching to "r" just hides the garbage.
Because sometimes the source really *is* memory, ie we have
net/ipv4/udp.c:
csum = csum_add(csum, frags->csum);
and then it's criminally stupid to load it into a register when it can
be just used directly.
But clang says "criminally stupid? *I* will show you stupid!" -
because what *clang* does with the above is this thing of beauty:
movl 136(%rax), %edi
movl %edi, 28(%rsp)
addl 28(%rsp), %ecx
adcl $0, %ecx
which has turned from "criminally stupid" to "it's art, I tell you -
you're not supposed to understand it".
Anyway, this is *literally* a clang bug. Complain to clang people for
being *extra* stupid - we told the compiler that it can use a register
or memory, and clang decided "I'll use memory", so then when we gave
it a memory location, it said "no, not *that* memory - I'll just
reload it on stack".
In contrast, gcc gets this right - and for that udp.c case it just generates
addl 136(%rax),%ecx # frags_67->D.58941.D.58869.D.58836.csum, a
adcl $0,%ecx # a
like it should.
And for csum_ipv6_magic, gcc generates this:
addl %edx,%eax # tmp112, a
adcl $0,%eax # a
IOW, the kernel is *right*, and this is purely clang being completely bogus.
I really don't want to penalize a good compiler because a bad one
can't get its act together.
Linus
next prev parent reply other threads:[~2024-01-06 19:33 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20230628020657.957880-1-goldstein.w.n@gmail.com>
2023-06-28 9:12 ` x86/csum: Remove unnecessary odd handling Borislav Petkov
2023-06-28 15:32 ` Noah Goldstein
2023-06-28 17:44 ` Linus Torvalds
2023-06-28 18:34 ` Noah Goldstein
2023-06-28 20:02 ` Linus Torvalds
2023-06-29 14:04 ` David Laight
2023-06-29 14:27 ` David Laight
2023-09-01 22:21 ` Noah Goldstein
2023-09-06 13:49 ` David Laight
2023-09-06 14:38 ` David Laight
2023-09-20 19:20 ` Noah Goldstein
2023-09-20 19:23 ` Noah Goldstein
2023-09-23 3:24 ` kernel test robot
2023-09-23 14:05 ` Noah Goldstein
2023-09-23 21:13 ` David Laight
2023-09-24 14:35 ` Noah Goldstein
2023-12-23 22:18 ` Noah Goldstein
2024-01-04 23:28 ` Noah Goldstein
2024-01-04 23:34 ` Dave Hansen
2024-01-04 23:36 ` Linus Torvalds
2024-01-05 0:33 ` Linus Torvalds
2024-01-05 10:41 ` David Laight
2024-01-05 16:12 ` David Laight
2024-01-05 18:05 ` Linus Torvalds
2024-01-05 23:52 ` David Laight
2024-01-06 0:18 ` Linus Torvalds
2024-01-06 10:26 ` Eric Dumazet
2024-01-06 19:32 ` Linus Torvalds [this message]
2024-01-07 12:11 ` David Laight
2024-01-06 22:08 ` David Laight
2024-01-07 1:09 ` H. Peter Anvin
2024-01-07 11:44 ` David Laight
2023-09-24 14:35 ` Noah Goldstein
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='CAHk-=wgobnShg4c2yyMbk2p=U-wmnOmX_0=b3ZY_479Jjey2xw@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=David.Laight@aculab.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=edumazet@google.com \
--cc=goldstein.w.n@gmail.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=mingo@redhat.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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).