On 07/11/2018 04:03 PM, Qing Zhao wrote: > 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. OK THanks
Jeff
