Hi, This is the 3rd version of the patch for the last part of PR78809. the major change in this version is to address the following concerns raised by Martin:
> One of the basic design principles that I myself have > accidentally violated in the past is that warning options > should not impact the emitted object code. I don't think > your patch actually does introduce this dependency by having > the codegen depend on the result of check_access() -- I'm > pretty sure the function is designed to do the validation > irrespective of warning options and return based on > the result of the validation and not based on whether > a warning was issued. But the choice of the variable name, > no_overflow_warn, suggests that it does, in fact, have this > effect. So I would suggest to rename the variable and add > a test that verifies that this dependency does not exist. I have addressed this concern as following per our discussion: 1. in routine expand_builtin_memcmp, * delete the condition if (warn_stringop_overflow) before check_access; * change the name of the variable that holds the return value of check_access to no_overflow 2. in the testsuite, change the new testcase strcmpopt_6.c to inhibit inlining when check_access detects error (Not depend on whether the warning option is ON or not). the following is the new patch, tested on both X86 and aarch64, no regression. Okay for thunk? thanks. Qing gcc/ChangeLog: +2018-07-11 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-11 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
> On Jul 5, 2018, at 10:46 AM, Qing Zhao <qing.z...@oracle.com> wrote: > > Hi, > > I have sent two emails with the updated patches on 7/3: > > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00065.html > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00070.html > > however, these 2 emails were not successfully forwarded to the > gcc-patches@gcc.gnu.org mailing list. > > So, I am sending the same email again in this one, hopefully this time it can > go through. > Qing > > 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.
