Re: xlc 12.1 miscompiles 32-bit ginCompareItemPointers()

On July 19, 2015 9:50:33 PM GMT+02:00, Noah Misch <noah(at)leadboat(dot)com> wrote:
>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.

I vote to simply error out in that case then. Trying to fix individual compiler bugs in an niche OS sounds like a bad idea.

Andres

---
Please excuse brevity and formatting - I am writing this on my mobile phone.