On Sun, 24 Oct 2004, Roger Sayle wrote:
> It's a long story...
And now even longer...
> However, David Anglin suggested an alternate strategy of subtracting
> max_reg from the stored quantity values themselves. Elaborating on
> that theme, the patch below reorganizes the encoding used for quantity
> numbers altogther. The original register values, previously in the
> range 0..max_reg-1, are now represented by negative values, and the
> constants being propagated are assigned sequential values >= 0. Then
> instead of representing registers as "regno - max_reg", requiring a
> memory reference to max_reg, I choose to assign them unique quantity
> values of "-regno - 1", or equivalently "-(regno+1)". On most CPUs,
> this can be done with two fast arithmetic operations, and avoids any
> memory reference to max_reg. As an added bonus, checking a quantity
> number for validity now tests for "qty >= 0" (instead of "qty >= max_reg")
> which should also provide a small speed improvement.
This has been a fantastic approach, however...
> 2004-10-24 Roger Sayle <ro...@eyesopen.com>
> John David Anglin <dave.ang...@nrc-cnrc.gc.ca>
>
> * cse.c: Change encoding of quantity numbers to avoid undefined
> pointer arithmetic on qty_table.
> (REGNO_QTY_VALID_P): A quantity is now valid if it isn't negative.
> (get_cse_reg_info): Initialize reg_qty to a unique negative value.
> (new_basic_block): Assign "real" quantity numbers from zero.
> (delete_reg_equiv): Do nothing if quantity is invalid. Reset the
> REG_QTY to its unique negative value.
> (merge_equiv_classes): Calculate need_rehash if quantity is valid.
> (cse_main): Don't include max_reg when determining max_qty.
> (cse_basic_block): Avoid subtracting a large offset from qty_table,
> which causes undefined C99 behaviour. Only allocate needed memory.
... as innocuous as the change may seem sadly it has missed a case, which
has hit me badly as I chose to make use of my old userland based on glibc
2.4 along with the LinuxThreads add-on and built with GCC 4.1.2, in an
emergency attempt to make my Alpha/Linux box ready for toolchain and
kernel verification right away.
The missed case causes a miscompilation of rwlock.c from LinuxThreads,
where the write unlock code from `__pthread_rwlock_unlock' is optimised
away in CSE entirely, leaving the lock in the taken state and causing a
lockup later on when an attempt is made to reacquire the lock. This is
fully reproducible as from commit 08a692679fb8 ("Undefined cse.c behaviour
causes 3.4 regression on HPUX"),
<https://gcc.gnu.org/ml/gcc-patches/2004-10/msg02027.html>, and does not
trigger beforehand.
A later CSE code rewrite made around GCC 4.3 with commit 932ad4d9b550
("Make CSE path following use the CFG"),
<https://gcc.gnu.org/ml/gcc-patches/2006-12/msg00431.html>, changed the
flow within the pass sufficiently for the bug not to trigger with the
original reproducer anymore and of course LinuxThreads have been long
decommissioned too, both of which may have contributed to the bug having
gone unnoticed for so long, but it's still there lurking in GCC 15 master
waiting to bite and barring a .c to .cc rename the same fix still cleanly
applies that I made for my GCC 4.1.2. And of course it is not specific to
the Alpha port, I just had the (mis)fortune to hit it there.
I have opened PR #115565 with the original reproducer, and posted a fix
too: <https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655287.html>.
I'm so glad to have it fixed now and my Alpha/Linux box has been up and
running since (with said fossil userland).
Maciej
