xlc 12.1 miscompiles 32-bit ginCompareItemPointers()

Поиск
Список
Период
Сортировка
От Noah Misch
Тема xlc 12.1 miscompiles 32-bit ginCompareItemPointers()
Дата
Msg-id 20150719195033.GB1300191@tornado.leadboat.com
обсуждение исходный текст
Ответы Re: xlc 12.1 miscompiles 32-bit ginCompareItemPointers()  (Andres Freund <andres@anarazel.de>)
Список 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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Josh Berkus
Дата:
Сообщение: Re: Support for N synchronous standby servers - take 2
Следующее
От: Andres Freund
Дата:
Сообщение: Re: xlc 12.1 miscompiles 32-bit ginCompareItemPointers()