> 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_.
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
