This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][Middle-end]3rd patch of PR78809
On 07/10/2018 09:14 AM, Qing Zhao wrote:
On Jul 9, 2018, at 3:25 PM, Martin Sebor <msebor@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)?
I think the check above is only there to avoid the overhead
of the two calls to compute_objsize and check_access. There
are a few more like it in other functions in the file and
they all should be safe to remove, but also safe to keep.
(Some of them might make it easy to inadvertently introduce
a dependency between the warning option and an optimization
so that's something to consider.)
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.
You're right, that's true for builtins.c where all we have to
work with is arrays with unknown contents and string literals.
The strlen pass, on the other hand, has access to the lengths
of even unknown strings. That suggests that an even better
place for the optimization might be the strlen pass where
the folding could happen earlier and at a higher level, which
might even obviate having to worry about the constant vs non-
constant handling.
I will create a PR on this and add a new patch after this one.
Sure, one step at a time makes sense. I don't think there is
any harm in having the optimization in two places: builtins.c
and strlen.
Martin
thanks.
Qing