public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: John Naylor <[email protected]>
Subject: Re: suspicious valgrind reports about radixtree/tidstore on arm64
Date: Wed, 19 Jun 2024 19:51:52 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
I wrote:
> I hypothesize that the reason we're not seeing equivalent failures
> on x86_64 is one of
> 1. x86_64 valgrind is stupider than aarch64, and fails to track that
> the contents of the SIMD registers are only partially defined.
> 2. x86_64 valgrind is smarter than aarch64, and is able to see
> that the "mask off invalid entries" step removes all the
> potentially-uninitialized bits.
Hah: it's the second case. If I patch radixtree.h as attached,
then x86_64 valgrind complains about
==00:00:00:32.759 247596== Conditional jump or move depends on uninitialised value(s)
==00:00:00:32.759 247596== at 0x52F668: local_ts_node_16_search_eq (radixtree.h:1018)
showing that it knows that the result of vector8_highbit_mask is
only partly defined. Kind of odd though that aarch64 valgrind
is getting the hard part right and not the easy(?) part.
regards, tom lane
Attachments:
[text/x-diff] make-radixtree-failure-visible-on-x86_64.patch (900B, ../[email protected]/2-make-radixtree-failure-visible-on-x86_64.patch)
download | inline diff:
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index 338e1d741d..267ec6de03 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -1015,12 +1015,15 @@ RT_NODE_16_SEARCH_EQ(RT_NODE_16 * node, uint8 chunk)
/* convert comparison to a bitfield */
bitfield = vector8_highbit_mask(cmp1) | (vector8_highbit_mask(cmp2) << sizeof(Vector8));
- /* mask off invalid entries */
- bitfield &= ((UINT64CONST(1) << count) - 1);
-
- /* convert bitfield to index by counting trailing zeros */
if (bitfield)
- slot_simd = &node->children[pg_rightmost_one_pos32(bitfield)];
+ {
+ /* mask off invalid entries */
+ bitfield &= ((UINT64CONST(1) << count) - 1);
+
+ /* convert bitfield to index by counting trailing zeros */
+ if (bitfield)
+ slot_simd = &node->children[pg_rightmost_one_pos32(bitfield)];
+ }
Assert(slot_simd == slot);
return slot_simd;
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: suspicious valgrind reports about radixtree/tidstore on arm64
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox