From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | xlc 12.1 miscompiles 32-bit ginCompareItemPointers() |
Date: | 2015-07-19 19:50:33 |
Message-ID: | 20150719195033.GB1300191@tornado.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
In a 32-bit build, xlc 12.1 for AIX miscompiles three inline expansions of
ginCompareItemPointers(), all in ginget.c (line numbers per commit 9aa6634):
739 Assert(!ItemPointerIsValid(&entry->curItem) ||
740 ginCompareItemPointers(&entry->curItem, &advancePast) <= 0);
847 } while (ginCompareItemPointers(&entry->curItem, &advancePast) <= 0);
915 if (ginCompareItemPointers(&key->curItem, &advancePast) > 0)
For one of the arguments, instead of computing
hi << 32 | lo << 16 | posid
it computes
(lo << 16) << 32 | lo << 16 | posid
PostgreSQL 9.4, which introduced the inline ginCompareItemPointers(), is the
oldest version affected. The problem remained invisible until my recent
commit 43d89a2; the quiet inline "configure" test had been failing, leaving
PG_USE_INLINE unset.
I tried some workarounds. Introducing intermediate variables or moving parts
of the calculation down into other inline functions did not help. The code
compiles fine when not inlined. Changing "hi << 32" to "hi << 33" worked (we
need just 48 of the 64 bits), but it presumably reduces performance on ABIs
where the current bit shifts boil down to a no-op.
I propose to expand the gin_private.h "#ifdef PG_USE_INLINE" test to exclude
xlc 32-bit configurations. The last 32-bit AIX kernel exited support on
2012-04-30. There's little reason to prefer 32-bit PostgreSQL under a 64-bit
kernel, so that configuration can do without the latest GIN optimization. One
alternative would be to distill a configure-time test detecting the bug, so
fixed xlc versions reacquire the optimization. Another alternative is to
change the bit layout within the uint64; for example, we could use the
most-significant 48 bits instead of the least-significant 48 bits. I dislike
the idea of a niche configuration driving that choice.
Thanks,
nm
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2015-07-19 20:07:37 | Re: xlc 12.1 miscompiles 32-bit ginCompareItemPointers() |
Previous Message | Josh Berkus | 2015-07-19 19:46:00 | Re: Support for N synchronous standby servers - take 2 |