[PATCH, bugfix] builtin expansion of strcmp for rs6000

Aaron Sawdey acsawdey@linux.vnet.ibm.com
Mon Jan 16 21:10:00 GMT 2017


Here is an updated version of this patch. 

Tulio noted that glibc's strncmp test was failing. This turned out to
be the use of signed HOST_WIDE_INT for handling strncmp length. The
glibc test calls strncmp with length 2^64-1, presumably to provoke
exactly this type of bug. Fixing the issue required changing
select_block_compare_mode() and expand_block_compare() as well.

The other change is if we must emit a runtime check for the 4k
crossing, then we might as well set base_align to 8 and emit the best
possible code.

OK for trunk if bootstrap/regtest passes on ppc64/ppc64le?

Thanks,
  Aaron

On Wed, 2017-01-11 at 11:26 -0600, Aaron Sawdey wrote:
> This expands on the previous patch. For strcmp and for strncmp with N
> larger than 64, the first 64 bytes of comparison is expanded inline
> and
> then a call to strcmp or strncmp is emitted to compare the remainder
> if
> the strings are found to be equal at that point. 
> 
> -mstring-compare-inline-limit=N determines how many block comparisons
> are emitted. With the default 8, and 64-bit code, you get 64 bytes. 
> 
> Performance testing on a power8 system shows that the code is
> anywhere
> from 2-8 times faster than RHEL7.2 glibc strcmp/strncmp depending on
> alignment and length.
> 
> In the process of adding this I discovered that the expansion being
> generated for strncmp had a bug in that it was not testing for a zero
> byte to terminate the comparison. As a result inputs like
> strncmp("AB\0CDEFGX", "AB\0CDEFGY", 9) would be compared not equal.
> The
> initial comparison of a doubleword would be equal so a second one
> would
> be fetched and compared, ignoring the zero byte that should have
> terminated comparison. The fix is to use a cmpb to check for zero
> bytes
> in the equality case before comparing the next chunk. I updated the
> strncmp-1.c test case to check for this, and also added a new 
> strcmp-1.c test case to check strcmp expansion. Also both now have a
> length 100 tests to check the transition from the inline comparison
> to
> the library call for the remainder.
> 
> ChangeLog
> 2017-01-11  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>
> 	* config/rs6000/rs6000-protos.h (expand_strn_compare): Add arg.
> 	* config/rs6000/rs6000.c (expand_strn_compare): Add ability to
> expand
> 	strcmp. Fix bug where comparison didn't stop with zero byte.
> 	* config/rs6000/rs6000.md (cmpstrnsi): Args to
> expand_strn_compare.
> 	(cmpstrsi): Add pattern.
> 
> gcc.dg/ChangeLog
> 2017-01-11  Aaron Sawdey  <acsawdey@linux.vnet.ibm.com>
> 	* gcc.dg/strcmp-1.c: New.
> 	* gcc.dg/strncmp-1.c: Add test for a bug that escaped.
> 
> 
> 
> 
-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
-------------- next part --------------
A non-text attachment was scrubbed...
Name: strcmp.20170116.patch
Type: text/x-patch
Size: 33845 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170116/578a06d7/attachment.bin>


More information about the Gcc-patches mailing list