On 01/02/2017 01:04 PM, Jakub Jelinek wrote:
Hi!
As mentioned in PR78901, the current -fprintf-return-value optimization
will remove (replace with constant) calls like:
int
foo (void)
{
return snprintf (0, 0, "%s%d", "abcde", 5);
}
but not
void
bar (void)
{
snprintf (0, 0, "%s%d", "abcde", 5);
}
which doesn't have any side-effect either (and there we even don't care that
much about whether the return value is constant or not, just that there
are no other side-effects like %n directive).
Looks good to me, thanks! Just a couple of suggestions:
@@ -2686,7 +2686,7 @@ get_destination_size (tree dest)
is known and exact. A result that isn't suitable for substitution may
have its range set to the range of return values, if that is known. */
-static void
+static bool
try_substitute_return_value (gimple_stmt_iterator *gsi,
const pass_sprintf_length::call_info &info,
const format_result &res)
Can you please update the comment above the function to explain
the return value like you did for
pass_sprintf_length::handle_gimple_call and...
@@ -3059,14 +3078,14 @@ pass_sprintf_length::execute (function *
basic_block bb;
FOR_EACH_BB_FN (bb, fun)
{
- for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si);
- gsi_next (&si))
+ for (gimple_stmt_iterator si = gsi_start_bb (bb); !gsi_end_p (si); )
{
/* Iterate over statements, looking for function calls. */
gimple *stmt = gsi_stmt (si);
- if (is_gimple_call (stmt))
- handle_gimple_call (&si);
+ if (is_gimple_call (stmt) && handle_gimple_call (&si))
+ continue;
...add a comment explaining that the gsi_next() call below is not
made when handle_gimple_call() removes the current statement
(effectively advancing to the next statement)? It wasn't entirely
obvious to me at first. (Another alternative might be to rename
handle_gimple_call to something more descriptive, like
maybe_optimize_sprintf_call or something like that.)
+ gsi_next (&si);
}
}
Martin
