This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

On 2015-03-08, at 3:52 PM, Carlos O'Donell wrote:

> 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.

Exactly.  The post and pre-increments could be used but one needs to ensure that the bufptr asm
arguments are marked as written.  I suspect instructions with post and pre-increment are slightly
slower but the difference may not be visible given the length of the pipeline.  We want to avoid
re-initializing bufptr.

Thus, I believe it's better to not modify bufptr in the asms in these two functions.

John David Anglin

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]