On 07/10/2018 08:34 AM, Jeff Law wrote:
On 07/10/2018 08:25 AM, Richard Biener wrote:
On Tue, Jul 10, 2018 at 4:10 PM Martin Sebor <mse...@gmail.com> wrote:
On 07/10/2018 01:12 AM, Richard Biener wrote:
On Mon, Jul 9, 2018 at 11:26 PM Martin Sebor <mse...@gmail.com> wrote:
On 07/09/2018 08:36 AM, Aldy Hernandez wrote:
{ dg-do run }
{ do-options "-O2 -fno-tree-strlen" } */
^^^^ I don't think this is doing anything.
If you look at the test run you can see that -fno-tree-strlen is never
passed (I think you actually mean -fno-optimize-strlen for that
matter). Also, the builtins.exp harness runs your test for an
assortment of other flags, not just -O2.
I didn't know the harness ignores dg-options specified in these
tests. That's surprising and feels like a bug in the harness
not to complain about it. The purpose of the test is to verify
that the strnlen expansion in builtins.c does the right thing
and it deliberately tries to disable the earlier strlen
optimizations to make sure the expansion in builtins.c is fully
exercised. By not pointing out my mistake the harness effectively
let me commit a change without making sure it's thoroughly tested
(I tested it manually before committing the patch but things could
regress without us noticing). I'll look into fixing this somehow.
This test is failing on my range branch for -Og, because
expand_builtin_strnlen() needs range info:
+ wide_int min, max;
+ enum value_range_type rng = get_range_info (bound, &min, &max);
+ if (rng != VR_RANGE)
+ return NULL_RTX;
but interestingly enough, it seems to be calculated in the sprintf
pass as part of the DOM walk:
/* First record ranges generated by this statement. */
evrp_range_analyzer.record_ranges_from_stmt (stmt, false);
It feels wrong that the sprintf warning pass is generating range info
that you may later depend on at rtl expansion time (and for a totally
unrelated thing-- strlen expansion).
Any pass that records ranges for statements will have this
effect. The sprintf pass seems to be the first one to make
use of this utility (and it's not just a warning pass but also
an optimization pass) but it would be a shame to put it off
limits to warning-only passes only because it happens to set
ranges.
As you noted elsewhere warning options shouldn't change code-generation.
This means that ranges may not be set to the IL in case they are only
computed when a warning option is enabled.
I agree. That's also why I think it should be a basic service
rather than a side-effect of tree traversal, either one done
to implement a particular optimization, or one done by a warning.
The main reason for the work Jeff put in to extracting it out
of EVRP, if I recall correctly, was to expose more accurate
range information to warning passes. Would setting statement
ranges make sense as part of EVRP (or some other similar pass)?
If not, the only way to conform to the policy that I can think
of is to have warning-only passes that need it make
the traversal and call record_ranges regardless of whether or
not the warning itself is enabled. That would seem needlessly
inefficient, even if the code implementing the warning itself
were disabled.
Well, simply not set range-info on SSA names but use the
lattice values? Should be easy to key that to a flag.
Right. That was essentially my suggestion. For a client that uses EVRP
analysis to drive warnings, they do not mirror results into the global
range info we attach to SSA_NAMEs. An optimization pass which utilizes
EVRP can (of course) set the global range info.
THe sprintf bits are a bit tricky as it's a mix of warning and
optimization, but I think there's enough separation that we can arrange
to do the right thing.
Since I introduced EVRP into the sprintf bits, I'm happy to own fixing
this up. I just wanted to get general agreement on the approach.
I'm not sure I understand what about the sprintf pass needs
changing since that the warning is independent of the optimization.
The gate function makes the intent clear:
pass_sprintf_length::gate (function *)
{
/* Run the pass iff -Warn-format-overflow or -Warn-format-truncation
is specified and either not optimizing and the pass is being invoked
early, or when optimizing and the pass is being invoked during
optimization (i.e., "late"). */
return ((warn_format_overflow > 0
|| warn_format_trunc > 0
|| flag_printf_return_value)
&& (optimize > 0) == fold_return_value);
}
The only other use of the warn_format_overflow and
warn_format_trunc variables is to set the warn_level variable,
and that one is only used to affect the LIKELY counter which
is used for warnings only but not for optimization.
Am I missing something?
Martin
