Hi, Jeff, thanks a lot for your review and comments.
I have addressed your comments,updated the patch, retested on both
aarch64 and x86.
The major changes in this version compared to the previous version are:
1. in routine expand_builtin_memcmp:
* move the inlining transformation AFTER the warning is issues for
-Wstringop-overflow;
* only apply inlining when there is No warning is issued.
2. in the testsuite, add a new testcase strcmpopt_6.c for this case.
3. update comments to:
* capitalize the first word.
* capitalize all the arguments.
NOTE, the routine expand_builtin_strcmp and expand_builtin_strncmp are not
changed.
the reason is: there is NO overflow checking for these two routines currently.
if we need overflow checking for these two routines, I think that a separate
patch is needed.
if this is needed, let me know, I can work on this separate patch for issuing
warning for strcmp/strncmp when
-Wstringop-overflow is specified.
The new patch is as following, please take a look at it.
thanks.
Qing
gcc/ChangeLog
+2018-07-02 Qing Zhao <qing.z...@oracle.com>
+
+ PR middle-end/78809
+ * builtins.c (expand_builtin_memcmp): Inline the calls first
+ when result_eq is false.
+ (expand_builtin_strcmp): Inline the calls first.
+ (expand_builtin_strncmp): Likewise.
+ (inline_string_cmp): New routine. Expand a string compare
+ call by using a sequence of char comparison.
+ (inline_expand_builtin_string_cmp): New routine. Inline expansion
+ a call to str(n)cmp/memcmp.
+ * doc/invoke.texi (--param builtin-string-cmp-inline-length): New
option.
+ * params.def (BUILTIN_STRING_CMP_INLINE_LENGTH): New.
+
gcc/testsuite/ChangeLog
+2018-07-02 Qing Zhao <qing.z...@oracle.com>
+
+ PR middle-end/78809
+ * gcc.dg/strcmpopt_5.c: New test.
+ * gcc.dg/strcmpopt_6.c: New test.
+
0001-3nd-Patch-for-PR78009.patch
Description: Binary data
> But leave them as lower case in code fragments like this. > > > So this generally looks pretty good. THe biggest technical concern is > making sure we're doing the right thing WRT issuing warnings. You can > tackle that problem by deferring inlining to a later point after > warnings have been issued or by verifying that your routines do not > inline in cases where warnings will be issued. It may be worth adding > testcases for these issues. > > There's a large number of comments that need capitalization fixes. > > Given there was no measured runtime performance impact, but slight > improvements on codesize for values <= 3, let's go ahead with that as > the default. > > Can you address the issues above and repost for final review? > > Thanks, > jeff
