Message ID | DB3PR08MB008978DA72CE0E1B93BB477483A60@DB3PR08MB0089.eurprd08.prod.outlook.com |
---|---|
State | Superseded |
Headers |
Received: (qmail 76704 invoked by alias); 25 Feb 2016 13:04:27 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 76686 invoked by uid 89); 25 Feb 2016 13:04:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-spam-relays-external:sk:mail-he, H*RU:sk:mail-he, nul X-HELO: eu-smtp-delivery-143.mimecast.com From: Wilco Dijkstra <Wilco.Dijkstra@arm.com> To: 'GNU C Library' <libc-alpha@sourceware.org> CC: nd <nd@arm.com> Subject: [PATCH] Use strlen when searching for a nul char Date: Thu, 25 Feb 2016 13:04:12 +0000 Message-ID: <DB3PR08MB008978DA72CE0E1B93BB477483A60@DB3PR08MB0089.eurprd08.prod.outlook.com> x-microsoft-exchange-diagnostics: 1; DB3PR08MB0090; 5:7OU+07lUpyfZS3E3t+eA23iGWWt6+sQYQUi8wVlac1a5fsJVlMGASbus86q6ujXE2Wkj3y9ulflo5WaYDebwzgCcAN7/6AaM4qYoOJ/8TYC58vzUjaRGz1x2auCG0yFLI/cmymPF1VIzno2Uw/YYHA==; 24:JyudSi/uFkp4WkaOZcO8DB8yia5CJJK1HS2h2yA7Ni4cRjXvYt/ZmVu/8cJminH7eNrxbcpGt1pn5xK7FWltBKABcRlN+NQrkUNH188PcQg=; 20:4JxBbmiJLvr5d+PJB+aUwC6AK/zpV88rQug94MxGCVf/SKb4LDhdZuWzYXsJ8ZKEtgFNUd+gvkO00GGUx8RIvA95hrPIwpcJ3Ogud5DE6uG6KWeDwvcutQZiAfFFAqd5nodN/NNkBwZHMqxIp1SHh2hvMK/wclLKoxe60iAdh2Q= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB3PR08MB0090; x-ms-office365-filtering-correlation-id: 2360ea6c-44f7-44f4-ee24-08d33de42832 nodisclaimer: True x-microsoft-antispam-prvs: <DB3PR08MB0090F931DAC2977934A338F083A60@DB3PR08MB0090.eurprd08.prod.outlook.com> x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001); SRVR:DB3PR08MB0090; BCL:0; PCL:0; RULEID:; SRVR:DB3PR08MB0090; x-forefront-prvs: 08635C03D4 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(54534003)(377424004)(229853001)(92566002)(74316001)(2900100001)(6116002)(19580405001)(77096005)(19580395003)(33656002)(40100003)(450100001)(122556002)(5003600100002)(10400500002)(1096002)(11100500001)(189998001)(110136002)(106116001)(50986999)(54356999)(87936001)(102836003)(3846002)(1220700001)(4326007)(5004730100002)(2906002)(586003)(86362001)(76576001)(5002640100001)(66066001)(5001960100002)(5008740100001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR08MB0090; H:DB3PR08MB0089.eurprd08.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Feb 2016 13:04:12.4229 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR08MB0090 X-MC-Unique: 5KfUqFVqS2-jZ6oSf-xVRQ-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable |
Commit Message
Wilco Dijkstra
Feb. 25, 2016, 1:04 p.m. UTC
Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding the end of a string, however it is not the most efficient way of doing so. Strlen is a simpler operation which is significantly faster on larger inputs (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). ChangeLog: 2016-02-25 Wilco Dijkstra <wdijkstr@arm.com> * string/bits/string2.h (strchr): Remove define. (__rawmemchr): Add new define. (rawmemchr): Likewise. --
Comments
On 25 Feb 2016 13:04, Wilco Dijkstra wrote: > Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is > a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. > Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding > the end of a string, however it is not the most efficient way of doing so. > Strlen is a simpler operation which is significantly faster on larger inputs > (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). will there be a change in GCC to also detect rawmemchr(s,'\0') ? even then, since this optimization isn't showing up until GCC7, shouldn't we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into strlen before falling back ? -mike
On 19-04-2016 14:57, Mike Frysinger wrote: > On 25 Feb 2016 13:04, Wilco Dijkstra wrote: >> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is >> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. >> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding >> the end of a string, however it is not the most efficient way of doing so. >> Strlen is a simpler operation which is significantly faster on larger inputs >> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > > will there be a change in GCC to also detect rawmemchr(s,'\0') ? > > even then, since this optimization isn't showing up until GCC7, shouldn't > we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > strlen before falling back ? > -mike > Personally I am not very found on the string2.h header and its intrinsic logic, which contains optimization logic that might not be true depending of the architecture string optimization. Also for the specific optimization does we really need to keep pushing for these specific inline implementations? I would prefer a much simple string2.h header than a convoluted one we have today.
On 19 Apr 2016 17:27, Adhemerval Zanella wrote: > On 19-04-2016 14:57, Mike Frysinger wrote: > > On 25 Feb 2016 13:04, Wilco Dijkstra wrote: > >> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is > >> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. > >> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding > >> the end of a string, however it is not the most efficient way of doing so. > >> Strlen is a simpler operation which is significantly faster on larger inputs > >> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > > > > will there be a change in GCC to also detect rawmemchr(s,'\0') ? > > > > even then, since this optimization isn't showing up until GCC7, shouldn't > > we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > > strlen before falling back ? > > Personally I am not very found on the string2.h header and its intrinsic logic, > which contains optimization logic that might not be true depending of the > architecture string optimization. > > Also for the specific optimization does we really need to keep pushing for > these specific inline implementations? I would prefer a much simple string2.h > header than a convoluted one we have today. i don't have a real opinion on keeping it or just tossing the whole thing out. but if we keep it, i think we should be adding the bits that make sense (like my question above) rather than half-assing it. -mike
On 19-04-2016 17:51, Mike Frysinger wrote: > On 19 Apr 2016 17:27, Adhemerval Zanella wrote: >> On 19-04-2016 14:57, Mike Frysinger wrote: >>> On 25 Feb 2016 13:04, Wilco Dijkstra wrote: >>>> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is >>>> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. >>>> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding >>>> the end of a string, however it is not the most efficient way of doing so. >>>> Strlen is a simpler operation which is significantly faster on larger inputs >>>> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). >>> >>> will there be a change in GCC to also detect rawmemchr(s,'\0') ? >>> >>> even then, since this optimization isn't showing up until GCC7, shouldn't >>> we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into >>> strlen before falling back ? >> >> Personally I am not very found on the string2.h header and its intrinsic logic, >> which contains optimization logic that might not be true depending of the >> architecture string optimization. >> >> Also for the specific optimization does we really need to keep pushing for >> these specific inline implementations? I would prefer a much simple string2.h >> header than a convoluted one we have today. > > i don't have a real opinion on keeping it or just tossing the whole > thing out. but if we keep it, i think we should be adding the bits > that make sense (like my question above) rather than half-assing it. > -mike > My idea is to cleanup the header bit a bit until we could just removei it. That's why I would prefer to not add any more optimization bits on it.
On 19 Apr 2016 18:01, Adhemerval Zanella wrote: > On 19-04-2016 17:51, Mike Frysinger wrote: > > On 19 Apr 2016 17:27, Adhemerval Zanella wrote: > >> On 19-04-2016 14:57, Mike Frysinger wrote: > >>> On 25 Feb 2016 13:04, Wilco Dijkstra wrote: > >>>> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is > >>>> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. > >>>> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding > >>>> the end of a string, however it is not the most efficient way of doing so. > >>>> Strlen is a simpler operation which is significantly faster on larger inputs > >>>> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > >>> > >>> will there be a change in GCC to also detect rawmemchr(s,'\0') ? > >>> > >>> even then, since this optimization isn't showing up until GCC7, shouldn't > >>> we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > >>> strlen before falling back ? > >> > >> Personally I am not very found on the string2.h header and its intrinsic logic, > >> which contains optimization logic that might not be true depending of the > >> architecture string optimization. > >> > >> Also for the specific optimization does we really need to keep pushing for > >> these specific inline implementations? I would prefer a much simple string2.h > >> header than a convoluted one we have today. > > > > i don't have a real opinion on keeping it or just tossing the whole > > thing out. but if we keep it, i think we should be adding the bits > > that make sense (like my question above) rather than half-assing it. > > My idea is to cleanup the header bit a bit until we could just removei > it. That's why I would prefer to not add any more optimization bits > on it. if everyone agrees on that direction, then slowly culling it WFM -mike
Mike Frysinger wrote: > On 25 Feb 2016 13:04, Wilco Dijkstra wrote: >> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is >> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. >> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding >> the end of a string, however it is not the most efficient way of doing so. >> Strlen is a simpler operation which is significantly faster on larger inputs >> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > > will there be a change in GCC to also detect rawmemchr(s,'\0') ? I wasn't planning to add it - GCC doesn't currently have it as a builtin so support would need to be added first. It's unclear how often rawmemchr is used elsewhere and what percentage is for finding the end of a string. > even then, since this optimization isn't showing up until GCC7, shouldn't > we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > strlen before falling back ? If it is common to use an old GCC to build a new GLIBC for a distro then adding strchr->strlen with a PREREQ would be useful. But if one typically first builds GCC7 and then GLIBC with that then it would not matter. An alternative would be fixup uses in GLIBC so it doesn't rely on header or GCC optimizations to do the obvious. Wilco
On 19 Apr 2016 22:03, Wilco Dijkstra wrote: > Mike Frysinger wrote: > > On 25 Feb 2016 13:04, Wilco Dijkstra wrote: > >> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is > >> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. > >> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding > >> the end of a string, however it is not the most efficient way of doing so. > >> Strlen is a simpler operation which is significantly faster on larger inputs > >> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > > > > will there be a change in GCC to also detect rawmemchr(s,'\0') ? > > I wasn't planning to add it - GCC doesn't currently have it as a builtin so support > would need to be added first. It's unclear how often rawmemchr is used > elsewhere and what percentage is for finding the end of a string. > > > even then, since this optimization isn't showing up until GCC7, shouldn't > > we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > > strlen before falling back ? > > If it is common to use an old GCC to build a new GLIBC for a distro then adding > strchr->strlen with a PREREQ would be useful. But if one typically first builds GCC7 > and then GLIBC with that then it would not matter. the issue isn't what version of gcc is used to build glibc. this is an installed header, and we made guarantees that the installed headers work with much older versions of gcc at runtime. those guarantees don't hard extend to pure-optimizations, but typically we wait much longer for those versions to cycle out of common use. dropping code that requires gcc7 (which doesn't even exist yet) doesn't fall into that bucket. -mike
On 19 Apr 2016 18:01, Adhemerval Zanella wrote: > On 19-04-2016 17:51, Mike Frysinger wrote: > > On 19 Apr 2016 17:27, Adhemerval Zanella wrote: > >> On 19-04-2016 14:57, Mike Frysinger wrote: > >>> On 25 Feb 2016 13:04, Wilco Dijkstra wrote: > >>>> Remove the strchr (s, '\0') to rawmemchr optimization as using rawmemchr is > >>>> a bad idea - I have a patch to add strchr (s, '\0') -> strlen to GCC7. > >>>> Like strchr (s, '\0'), rawmemchr (s, '\0') appears a common idiom for finding > >>>> the end of a string, however it is not the most efficient way of doing so. > >>>> Strlen is a simpler operation which is significantly faster on larger inputs > >>>> (eg. on x86 strlen is 50% faster than rawmemchr on strings of 1KB). > >>> > >>> will there be a change in GCC to also detect rawmemchr(s,'\0') ? > >>> > >>> even then, since this optimization isn't showing up until GCC7, shouldn't > >>> we keep some logic here ? i.e. transform strchr/rawmemchr(s, '\0') into > >>> strlen before falling back ? > >> > >> Personally I am not very found on the string2.h header and its intrinsic logic, > >> which contains optimization logic that might not be true depending of the > >> architecture string optimization. > >> > >> Also for the specific optimization does we really need to keep pushing for > >> these specific inline implementations? I would prefer a much simple string2.h > >> header than a convoluted one we have today. > > > > i don't have a real opinion on keeping it or just tossing the whole > > thing out. but if we keep it, i think we should be adding the bits > > that make sense (like my question above) rather than half-assing it. > > My idea is to cleanup the header bit a bit until we could just removei > it. That's why I would prefer to not add any more optimization bits > on it. The latest string.h is already a lot smaller, and it should be feasible to get rid of it this release. However it means removing some optimizations as not all are useful enough to move to string.h (and/or be added to GCC): The strdup case with a string literal is likely so rare and the possible speedup by inlining memcpy so small (compared to the overhead of malloc) that removing it can't make any difference. The __strsep_*/__strok_* inlines are unlikely beneficial if we ensure small match strings are special cased in the strsep/strtok code (which is already much faster with the improved strpbrk and strspn). The strncmp inline doesn't appear generally useful - do people really accidentally write strncmp (s, "abc", 2)??? The strcmp one could be useful but it's better done inside GCC based on target preferences and whether it is an equality comparison. That leaves mostly defines like this: #ifndef _HAVE_STRING_ARCH_strncpy # define strncpy(dest, src, n) __builtin_strncpy (dest, src, n) #endif I believe these have no benefit so can be removed completely. If we can agree on this then string2.h can be removed completely! Wilco
diff --git a/string/bits/string2.h b/string/bits/string2.h index bebd158c5ff0f7bd7d9e4a4c3e120cd45b6e2143..f34fedb170352eaca0ed784ca6e76d7bbbfaefc2 100644 --- a/string/bits/string2.h +++ b/string/bits/string2.h @@ -62,14 +62,15 @@ #endif -#ifndef _HAVE_STRING_ARCH_strchr +#ifndef _HAVE_STRING_ARCH_rawmemchr extern void *__rawmemchr (const void *__s, int __c); -# if __GNUC_PREREQ (3, 2) -# define strchr(s, c) \ - (__extension__ (__builtin_constant_p (c) && !__builtin_constant_p (s) \ - && (c) == '\0' \ - ? (char *) __rawmemchr (s, c) \ - : __builtin_strchr (s, c))) +# define __rawmemchr(s, c) \ + (__extension__ ({ char *__s = (char *)(s); \ + __builtin_constant_p (c) && (c) == '\0' \ + ? (void *)(__s + strlen (__s)) \ + : __rawmemchr (__s, (c));})) +# ifdef __USE_GNU +# define rawmemchr(s,c) __rawmemchr ((s), (c)) # endif #endif