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