From: Dave Hansen <dave.hansen@intel.com>
To: David Binderman <dcb314@hotmail.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH] x86/mm: Simplify redundant overlap calculation
Date: Tue, 23 Jan 2024 09:00:45 -0800 [thread overview]
Message-ID: <784a6843-c5fb-46eb-a472-5d96101478a9@intel.com> (raw)
In-Reply-To: <AS8PR02MB10217E3448DF6D70285CA3D1D9C742@AS8PR02MB10217.eurprd02.prod.outlook.com>
On 1/23/24 08:54, David Binderman wrote:
>> Remove the second condition. It is exactly the same as the first.
> I don't think the first condition is sufficient. I suspect something like
>
> return (r2_start <= r1_start && r1_start <= r2_end) ||
> (r2_start <= r1_end && r1_end <= r2_end);
>
> Given the range [r2_start .. r2_end], then if r1_start or r1_end
> are in that range, you have overlap.
>
> Unless you know different.
First of all, I've gotten these bounds checks wrong in code more times
than I can count. I have zero trust that I'll get them right. :)
But the compiler seems to know different at least:
int overlaps1(unsigned long r1_start, unsigned long r1_end,
unsigned long r2_start, unsigned long r2_end)
{
return (r1_start <= r2_end && r1_end >= r2_start) ||
(r2_start <= r1_end && r2_end >= r1_start);
}
int overlaps2(unsigned long r1_start, unsigned long r1_end,
unsigned long r2_start, unsigned long r2_end)
{
return (r1_start <= r2_end && r1_end >= r2_start);
}
Results in:
0000000000001180 <overlaps1>:
1180: f3 0f 1e fa endbr64
1184: 48 39 cf cmp %rcx,%rdi
1187: 49 89 d0 mov %rdx,%r8
118a: 0f 96 c2 setbe %dl
118d: 31 c0 xor %eax,%eax
118f: 4c 39 c6 cmp %r8,%rsi
1192: 0f 93 c0 setae %al
1195: 21 d0 and %edx,%eax
1197: c3 ret
1198: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
119f: 00
00000000000011a0 <overlaps2>:
11a0: f3 0f 1e fa endbr64
11a4: 48 39 cf cmp %rcx,%rdi
11a7: 49 89 d0 mov %rdx,%r8
11aa: 0f 96 c2 setbe %dl
11ad: 31 c0 xor %eax,%eax
11af: 4c 39 c6 cmp %r8,%rsi
11b2: 0f 93 c0 setae %al
11b5: 21 d0 and %edx,%eax
11b7: c3 ret
11b8: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
11bf: 00
I also wrote a quick program to throw random numbers into both versions
and see if they differ. They never did, which they obviously can't if
they're the exact same instructions.
next prev parent reply other threads:[~2024-01-23 17:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-23 16:36 Dave Hansen
2024-01-23 16:47 ` Dave Hansen
2024-01-23 16:54 ` David Binderman
2024-01-23 17:00 ` Dave Hansen [this message]
2024-01-23 19:19 ` Sohil Mehta
2024-01-23 19:28 ` Sohil Mehta
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=784a6843-c5fb-46eb-a472-5d96101478a9@intel.com \
--to=dave.hansen@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=dcb314@hotmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--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
all inboxes | Powered by JetHome®