This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Single threaded stdio optimization
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: Siddhesh Poyarekar <siddhesh at gotplt dot org>, Joseph Myers <joseph at codesourcery dot com>
- Cc: nd at arm dot com, GNU C Library <libc-alpha at sourceware dot org>, "triegel at redhat dot com" <triegel at redhat dot com>
- Date: Thu, 29 Jun 2017 14:29:24 +0100
- Subject: Re: [PATCH v2] Single threaded stdio optimization
- Authentication-results: sourceware.org; auth=none
- Authentication-results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=arm.com;
- Nodisclaimer: True
- References: <594AA0A4.7010600@arm.com> <alpine.DEB.2.20.1706211651110.3924@digraph.polyomino.org.uk> <594B92ED.6060809@arm.com> <2dc4477c-c349-86b1-f8a5-95d69c397f24@gotplt.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 29/06/17 12:41, Siddhesh Poyarekar wrote:
>> @@ -32,6 +32,8 @@ _IO_ferror (_IO_FILE *fp)
>> {
>> int result;
>> CHECK_FILE (fp, EOF);
>> + if (!_IO_need_lock (fp))
>> + return _IO_ferror_unlocked (fp);
>> _IO_flockfile (fp);
>> result = _IO_ferror_unlocked (fp);
>
> The patch looks OK except for the duplication (and a missing comment
> below), which looks a bit clumsy. How about something like this instead:
>
> bool need_lock = _IO_need_lock (fp);
>
> if (need_lock)
> _IO_flockfile (fp);
> result = _IO_ferror_unlocked (fp);
> if (need_lock)
> _IO_funlockfile (fp);
>
> return result;
>
> You could probably make some kind of a macro out of this, I haven't
> looked that hard.
>
this does not do what we want:
single check + tailcall the unlocked version.
with your code gcc is more likely to do two
checks and register spills and an additional
return.
while my code does not force gcc to do the
right thing at least it shows the intent.