Re: [PATCH] hppa: fix __O_SYNC to match the kernel

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/01/2015 02:55 PM, John David Anglin wrote:
> In fesetenv.c, the post increment of bufptr was retained in the first asm but the constraint for it
> does not indicate that bufptr is modified.  As a result, GCC miscompiled fesetenv.  We get better
> code by not modifying bufptr as GCC doesn't have to reload bufptr.

It uses "+r" which means the operand, the register, is read and modified.

The fact that we get better code, or work around a gcc bug, is a good reason
to make the change, but I don't see what's wrong with the original code.

> The main bug in feholdexcept is the second set of bufptr.  This existed to the restore the exception
> registers in reverse order.  This statement should have been removed when the code was changed
> to only update the status and first exception registers.  The offset used in the statement is also off
> by a factor two, so it probably never worked correctly.  With the current patch, the code loads zero to
> the status and exception register.  As a result, the T bit is not set properly.

This doesn't sound right either.

fstd,ma %%fr0,8(%1)

Results in a store to bufptr, with bubptr-=8 *after* the store.

fldd,mb -8(%0),%%fr0

Results in a bufptr-=8, followed by a load from bufptr to fr0.

Thus while the operations are left-over from previous iterations of
the code where we did save/load all of the registers, the above code
is idempotent with respect to your changes.

The one problem is that bufptr is not marked as changed in the *second*
assembly contstraint which just lists bufptr as input (which is wrong)
and an optimization might reorder things such that this breaks.

Cheers,
Carlos.