Re: [PATCH][Middle-end][version 3]3rd patch of PR78809

[Jeff Law <law at redhat dot com>]

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.zhao@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.zhao@oracle.com>
> +
> +	PR middle-end/78809
> +	* gcc.dg/strcmpopt_5.c: New test.
> +	* gcc.dg/strcmpopt_6.c: New test.
OK
THanks

Jeff