On July 10, 2018 5:14:37 PM GMT+02:00, Qing Zhao <qing.z...@oracle.com> wrote:
>
>> On Jul 9, 2018, at 3:25 PM, Martin Sebor <mse...@gmail.com> wrote:
>>
>> check_access() calls warning_at() to issue warnings, and that
>> function only issues warnings if they are enabled, so the guard
>> isn't necessary to make it work this way.
>
>Okay I see.
>
>then, in the current code: (for routine expand_builtin_memcmp)
>
>/* Diagnose calls where the specified length exceeds the size of either
> object. */
> if (warn_stringop_overflow)
> {
> tree size = compute_objsize (arg1, 0);
> if (check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE, len,
> /*maxread=*/NULL_TREE, size, /*objsize=*/NULL_TREE))
> {
> size = compute_objsize (arg2, 0);
> check_access (exp, /*dst=*/NULL_TREE, /*src=*/NULL_TREE, len,
> /*maxread=*/NULL_TREE, size, /*objsize=*/NULL_TREE);
> }
> }
>
>Is the above condition on variable warn_stringop_overflow unnecessary?
>all the warnings inside check_access are controlled by
>OPT_Wstringop_overflow_.
Well, the condition certainly saves compile time.
>
>can I safely delete the above condition if (warn_stringop_overflow)?
>
>>
>>>> Beyond that, an enhancement to this optimization that might
>>>> be worth considering is inlining even non-constant calls
>>>> with array arguments whose size is no greater than the limit.
>>>> As in:
>>>>
>>>> extern char a[4], *b;
>>>>
>>>> int n = strcmp (a, b);
>>>>
>>>> Because strcmp arguments are required to be nul-terminated
>>>> strings, a's length above must be at most 3. This is analogous
>>>> to similar optimizations GCC performs, such as folding to zero
>>>> calls to strlen() with one-element arrays.
>>>
>>> Yes, I agree that this will be another good enhancement to the
>strcmp inlining.
>>>
>>> however, it’s not easy to be integrated with my current patch. The
>major issue is:
>>>
>>> The inlined code for the strcmp call without string constant will
>be different than the inlined code for the
>>> strcmp call with string constant, then:
>>>
>>> 1. the default value for the threshold that control the maximum
>length of the string length for inlining will
>>> be different than the one for the strcmp call with string constant,
>more experiments need to be run and a new parameter
>>> need to be added to control this;
>>> 2. the inlined transformed code will be different than the current
>one.
>>>
>>> based on the above, I’d like to open a new PR to record this new
>enhancement and finish it with a new patch later.
>>>
>>> what’s your opinion on this?
>>
>> I'm not sure I see the issues above as problems and I would expect
>> the non-constant optimization to naturally handle the constant case
>> as well. But if you prefer it that way, implementing the
>non-constant
>> optimization in a separate step sounds reasonable to me. It's your
>> call.
>
>the inlined code for call to strcmp with constant string will only have
>one load instruction for each byte, but for call to strcmp
>without constant string, there will be two load instructions for each
>byte. So, the run time performance impact will be different.
>we need separate default values of the maximum length of the string to
>enable the transformation.
>
>I will create a PR on this and add a new patch after this one.
>
>thanks.
>
>Qing
