This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] hppa: fix __O_SYNC to match the kernel
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: John David Anglin <dave dot anglin at bell dot net>, Mike Frysinger <vapier at gentoo dot org>
- Cc: libc-alpha at sourceware dot org, carlos at systemhalted dot org
- Date: Sun, 08 Mar 2015 15:52:19 -0400
- Subject: Re: [PATCH] hppa: fix __O_SYNC to match the kernel
- Authentication-results: sourceware.org; auth=none
- References: <1424755185-27627-1-git-send-email-vapier at gentoo dot org> <BLU436-SMTP185F8B8082D09E2C4A6E5EF97160 at phx dot gbl> <20150227065339 dot GO29461 at vapier> <BLU436-SMTP175B7131A983C54990446FB97130 at phx dot gbl>
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.
Results in a store to bufptr, with bubptr-=8 *after* the store.
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.