phiopt, v2: Adjust instead of reset phires range
Commit Message
On Thu, Dec 22, 2022 at 08:46:33PM +0100, Aldy Hernandez wrote:
> I haven't looked at your problem above, but have you tried using
> int_range_max (or even int_range<2>) instead of value_range above?
>
> value_range is deprecated and uses the legacy anti-range business,
> which has a really hard time representing complex ranges, as well as
> union/intersecting them.
You're right. With int_range_max it works as I expected.
And no, floating point isn't possible here.
If value_range is right now just the legacy single range or anti-range,
then it explains why it didn't work - while on the first testcase
we could have anti-range ~[0, 0], on the second case [-128, -1] U [1, 127]
is turned into simple legacy [-128, 127].
So, ok for trunk if this passes bootstrap/regtest?
2022-12-22 Jakub Jelinek <jakub@redhat.com>
Aldy Hernandez <aldyh@redhat.com>
* tree-ssa-phiopt.cc (value_replacement): Instead of resetting
phires range info, union it with oarg.
Jakub
Comments
LGTM
On Thu, Dec 22, 2022, 22:44 Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Dec 22, 2022 at 08:46:33PM +0100, Aldy Hernandez wrote:
> > I haven't looked at your problem above, but have you tried using
> > int_range_max (or even int_range<2>) instead of value_range above?
> >
> > value_range is deprecated and uses the legacy anti-range business,
> > which has a really hard time representing complex ranges, as well as
> > union/intersecting them.
>
> You're right. With int_range_max it works as I expected.
> And no, floating point isn't possible here.
> If value_range is right now just the legacy single range or anti-range,
> then it explains why it didn't work - while on the first testcase
> we could have anti-range ~[0, 0], on the second case [-128, -1] U [1, 127]
> is turned into simple legacy [-128, 127].
>
> So, ok for trunk if this passes bootstrap/regtest?
>
> 2022-12-22 Jakub Jelinek <jakub@redhat.com>
> Aldy Hernandez <aldyh@redhat.com>
>
> * tree-ssa-phiopt.cc (value_replacement): Instead of resetting
> phires range info, union it with oarg.
>
> --- gcc/tree-ssa-phiopt.cc.jj 2022-12-22 12:52:36.588469821 +0100
> +++ gcc/tree-ssa-phiopt.cc 2022-12-22 13:11:51.145060050 +0100
> @@ -1492,11 +1492,25 @@ value_replacement (basic_block cond_bb,
> break;
> }
> if (equal_p)
> - /* After the optimization PHI result can have value
> - which it couldn't have previously.
> - We could instead of resetting it union the range
> - info with oarg. */
> - reset_flow_sensitive_info (gimple_phi_result (phi));
> + {
> + tree phires = gimple_phi_result (phi);
> + if (SSA_NAME_RANGE_INFO (phires))
> + {
> + /* After the optimization PHI result can have value
> + which it couldn't have previously. */
> + int_range_max r;
> + if (get_global_range_query ()->range_of_expr (r,
> phires,
> + phi))
> + {
> + int_range<2> tmp (carg, carg);
> + r.union_ (tmp);
> + reset_flow_sensitive_info (phires);
> + set_range_info (phires, r);
> + }
> + else
> + reset_flow_sensitive_info (phires);
> + }
> + }
> if (equal_p && MAY_HAVE_DEBUG_BIND_STMTS)
> {
> imm_use_iterator imm_iter;
>
>
> Jakub
>
>
@@ -1492,11 +1492,25 @@ value_replacement (basic_block cond_bb,
break;
}
if (equal_p)
- /* After the optimization PHI result can have value
- which it couldn't have previously.
- We could instead of resetting it union the range
- info with oarg. */
- reset_flow_sensitive_info (gimple_phi_result (phi));
+ {
+ tree phires = gimple_phi_result (phi);
+ if (SSA_NAME_RANGE_INFO (phires))
+ {
+ /* After the optimization PHI result can have value
+ which it couldn't have previously. */
+ int_range_max r;
+ if (get_global_range_query ()->range_of_expr (r, phires,
+ phi))
+ {
+ int_range<2> tmp (carg, carg);
+ r.union_ (tmp);
+ reset_flow_sensitive_info (phires);
+ set_range_info (phires, r);
+ }
+ else
+ reset_flow_sensitive_info (phires);
+ }
+ }
if (equal_p && MAY_HAVE_DEBUG_BIND_STMTS)
{
imm_use_iterator imm_iter;