https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125501
--- Comment #12 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Richard Biener from comment #11)
> So we somehow have recorded a non-null global range for a5_16:
>
> void f11 (int64_t a0, void * a4, void * a5)
> {
> # PT = nonlocal null
> void * a4_14(D) = a4;
> # PT = nonlocal
> void * a5_16(D) = a5;
>
> which DOM2 records. The pass before (thread1) still has
>
> void f11 (int64_t a0, void * a4, void * a5)
> {
> # PT = nonlocal null
> void * a4_14(D) = a4;
> # PT = nonlocal null
> void * a5_16(D) = a5;
>
> disabling only DOM2 is enough to fix things. So the logic is likely
> that a4 != 0 (in the outer loop header) and the inner loop exit
> is if (!(a4 == a5)) __builtin_unreachable (); and thus a5 is not NULL,
> but only when a4 != 0, so not sure why we think we can set that
> as global range?
:1
/
:1
I have no idea how dom works internally, but it does use the path ranger. I
see:
Optimizing block #9
1>>> STMT 1 = a4_14(D) le_expr a5_16(D)
1>>> STMT 1 = a4_14(D) ge_expr a5_16(D)
1>>> STMT 1 = a4_14(D) eq_expr a5_16(D)
1>>> STMT 0 = a4_14(D) ne_expr a5_16(D)
671 range_on_edge (a5_16(D)) on edge 8->9
672 range_on_exit (a5_16(D)) from BB 8
673 range_of_expr(a5_16(D)) at stmt if (a4_14(D) == a5_16(D))
674 range_on_entry (a5_16(D)) to BB 8
675 range_of_stmt (a5_16(D)) at stmt GIMPLE_NOP
TRUE : (675) cached (a5_16(D)) [prange] void * VARYING
TRUE : (674) range_on_entry (a5_16(D)) [prange] void * VARYING
TRUE : (673) range_of_expr (a5_16(D)) [prange] void * VARYING
TRUE : (672) range_on_exit (a5_16(D)) [prange] void * VARYING
676 GORI outgoing_edge for a5_16(D) on edge 8->9
677 GORI compute op 2 (a5_16(D)) at if (a4_14(D) == a5_16(D))
GORI LHS = [irange] _Bool [1, 1], a4_14(D) = [prange] void * [1,
+INF]
GORI Computes a5_16(D) = [prange] void * [1, +INF] intersect Known
range : [prange] void * VARYING
GORI TRUE : (677) produces (a5_16(D)) [prange] void * [1, +INF]
GORI TRUE : (676) outgoing_edge (a5_16(D)) [prange] void * [1, +INF]
TRUE : (671) range_on_edge (a5_16(D)) [prange] void * [1, +INF]
Global Exported: a5_16(D) = [prange] void * [1, +INF]
It appears to think that because a5_16 is [1, +INF] on 8->9 and 8->10 leads to
unreachable, that a5_16 is globally [1, +INF]
THe code in dom_opt_dom_walker::set_global_ranges_from_unreachable_edges:
FOR_EACH_GORI_EXPORT_NAME (m_ranger->gori_ssa (), pred_e->src, name)
if (all_uses_feed_or_dominated_by_stmt (name, stmt)
// The condition must post-dominate the definition point.
&& (SSA_NAME_IS_DEFAULT_DEF (name)
|| (gimple_bb (SSA_NAME_DEF_STMT (name))
== pred_e->src)))
{
value_range r (TREE_TYPE (name));
if (m_ranger->range_on_edge (r, pred_e, name)
&& !r.varying_p ()
&& !r.undefined_p ())
{
set_range_info (name, r);
maybe_set_nonzero_bits (pred_e, name);
}
}
SO I believe it is true that a5_16 is in fact [1, +INF] for all uses within the
function.
I guess the problem is that in dom2 the order is
if (a4_14 == 0) return if (a4_14 == a5_16)
IN which case it is true that a5_16 is globally [1, +INF] for all uses.
I see in q.i.167t.loopinit everything still seems fine, but in
q.i.168t.unswitch :
<bb 2> [local count: 396743129]:
if (a4_14(D) == a5_16(D))
goto <bb 35>; [100.00%]
else
goto <bb 23>; [0.00%]
<bb 23> [count: 0]:
# v12_27 = PHI <v12_12(D)(2)>
if (a4_14(D) == 0B)
goto <bb 11>; [4.02%]
The order has been changed... This means the global value assumption is no
longer true because all uses of a5_16 are no longer dominated by a4_14 == 0
statement.
IS that actually a safe thing to do? hoist the equality expression above a
compare to 0 and exit?
So either this isn't safe, or it is unsafe for DOM to set a global value based
on all available ranges like it does.. which could be the case.. the
__builtin_unreachable code in VRP went through some ppin to determine when it
can and when it cant set those kinds of ranges. I notice VRP1 didnt set the
range in this case. Perhaps DOM needs to be tightened up, or just leave it to
ranger to do this with unreachables.
Looking at the comment in tree-vrp from whenh I reworked the removal of
builtin_unreachable:
.
// This routine is called to check builtin_unreachable calls during any
// time before final removal. The only way we can be sure it does not
// provide any additional information is to expect that we can update the
// global values of all exports from a block. This means the branch
// to the unreachable call must dominate all uses of those ssa-names, with
// the exception that there can be a single use in the block containing
// the branch. IF the name used in the branch is defined in the block, it may
// contain the name of something else that will be an export. And likewise
// that may also use another name that is an export etc.
//
// As long as there is only a single use, we can be sure that there are no
other
// side effects (like being passed to a call, or stored to a global, etc.
// This means we will miss cases where there are 2 or more uses that have
// no interveneing statements that may had side effects, but it catches most
// of the cases we care about, and prevents expensive in depth analysis.
//
// Ranger will still reflect the proper ranges at other places in these missed
// cases, we simply will not remove/set globals early.
SO I think thats the problem... Ranger errs on the side of conservatism, and
DOM is only checking that a5_16 is dominated and writes out the value, whereas
VRP1 looks att all the exports and will see that both a4_14 and a5_16 are
exports and that a4_14 is still globally VARYING, so it wont do either.
Too bad Aldy left and abandoned getting rid of DOM :-) I think he was close.
Maybe we can scavenge some of the builtin_unreachable code for DOM.. it
requires everything that is exported:
tree name;
FOR_EACH_GORI_EXPORT_AND_DEP_NAME (m_ranger.gori_ssa (), e->src, name, m_tmp)
{
if (!fully_replaceable (name, e->src))
return;
}
// Set the global value for each.
FOR_EACH_GORI_EXPORT_NAME (m_ranger.gori_ssa (), e->src, name)
{
value_range r (TREE_TYPE (name));
m_ranger.range_on_entry (r, e->dest, name);
// Nothing at this late stage we can do if the write fails.
if (!set_range_info (name, r))
continue;
}
I can look at it if a get a chance, but I think this is the culprit...
