Hi,
On Tue, Nov 05 2024, Jan Hubicka wrote:
>> Hi,
>>
>> I believe that the current function ipa_range_set_and_normalize lacks
>> a check that a base of an ADDR_EXPR lacks a test whether the base
>> really cannot be NULL, so this patch adds it. Moreover, I never liked
>> the name as I do not think it makes the value of ranges any more
>> normal but rather just special-cases non-zero ip_invariant pointers.
>> Therefore, I have given it a different name and moved it to a .cc
>> file, our LTO bootstrap should inline (and/or split) it if necessary
>> anyway.
>>
>> Bootstrapped and tested on x86_64-linux, the whole patch series has
>> additionally passed LTO and profiled-LTO bootstrap on the same platform
>> and a bootstrap and testsuite on ppc64-linux. Aarch64-linux bootstrap
>> and testing is in progress. OK for master is that passes too?
>>
>> Thanks,
>>
>> Martin
>>
>>
>> gcc/ChangeLog:
>>
>> 2024-11-04 Martin Jambor <mjam...@suse.cz>
>>
>> * ipa-prop.h (ipa_get_range_from_ip_invariant): Declare.
>> (ipa_range_set_and_normalize): Remove.
>> * ipa-prop.cc (ipa_get_range_from_ip_invariant): New function.
>> * ipa-cp.cc (ipa_vr_intersect_with_arith_jfunc): Use
>> ipa_get_range_from_ip_invariant instead of
>> ipa_range_set_and_normalize.
>> * ipa-fnsummary.cc (evaluate_conditions_for_known_args): Likewise.
>> +/* Given VAL that conforms to is_gimple_ip_invariant, produce a VRANGE that
>> + represents it as a range. */
>> +
>> +void
>> +ipa_get_range_from_ip_invariant (vrange &r, tree val)
>> +{
>> + if (TREE_CODE (val) == ADDR_EXPR)
>> + {
>> + bool strict_overflow = false;
>> + if (tree_single_nonzero_warnv_p (val, &strict_overflow))
> this function uses maybe_nonzero_address that in turn checks
> symtab_node::nonzero_address which uses flag_delete_null_pointer_checks
> to verify that we can assume that defined symbols are non-NULL.
> Now -fdelete-null-pointer-checks is defined in common.opt as
> optimization and thus is stored in TREE_OPTIMIZATION_NODE.
>
> The patch is definitely an improvement. But it seems to me that we need
> to add function argument to tree_single_nonzero_warnv_p and propagate it
> down to symtab_node::nonzero_address so it can query in correct function
> context. This can be done incrementally, so the patch is OK.
>
I was looking for the use of the flag but somehow did not see it and in
some less-than-ideal state of mind assumed it must therefore be a global
option. Below is a patch that takes that into account.
The patch has been pre-approved by Honza and which has passed bootstrap
and testing on x86_64 and together with other two patches also on
ppc64le-linux and an LTO bootstrap on x86_64-linux, so I am going to
push it to master in a few moments.
Thanks,
Martin
---------- 8< ----------
I believe that the current function ipa_range_set_and_normalize lacks
a check that a base of an ADDR_EXPR lacks a test whether the base
really cannot be NULL, so this patch adds it. Moreover, I never liked
the name as I do not think it makes the value of ranges any more
normal but rather just special-cases non-zero ip_invariant pointers.
Therefore, I have given it a different name and moved it to a .cc
file, our LTO bootstrap should inline (and/or split) it if necessary
anyway.
Because, as Honza correctly pointed out, deriving non-NULLness from a
pointer depends on flag_delete_null_pointer_checks which is an
optimization flag and thus depends on a given function, in this
version of the patch ipa_get_range_from_ip_invariant gets a
context_node parameter for that purpose. This then needs to be used
within symtab_node::nonzero_address which gets a special overload in
which the value of the flag can be provided as a parameter.
gcc/ChangeLog:
2024-12-11 Martin Jambor <mjam...@suse.cz>
* cgraph.h (symtab_node): Add a new overload of nonzero_address.
* symtab.cc (symtab_node::nonzero_address): Add a new overload whith a
parameter for delete_null_pointer_checks. Make the original overload
call the new one which has retains the actual implementation.
* ipa-prop.h (ipa_get_range_from_ip_invariant): Declare.
(ipa_range_set_and_normalize): Remove.
* ipa-prop.c (ipa_get_range_from_ip_invariant): New function.
(ipa_range_set_and_normalize): Remove.
* ipa-cp.cc (ipa_vr_intersect_with_arith_jfunc): Add a new parameter
context_node. Use ipa_get_range_from_ip_invariant instead of
ipa_range_set_and_normalize and pass to it the new parameter.
(ipa_value_range_from_jfunc): Pass cs->caller as the context_node to
ipa_vr_intersect_with_arith_jfunc.
(propagate_vr_across_jump_function): Likewise.
(ipa_get_range_from_ip_invariant): New function.
* ipa-fnsummary.cc (evaluate_conditions_for_known_args): Use
ipa_get_range_from_ip_invariant instead of ipa_range_set_and_normalize
---
gcc/cgraph.h | 4 ++++
gcc/ipa-cp.cc | 12 ++++++++----
gcc/ipa-fnsummary.cc | 4 ++--
gcc/ipa-prop.cc | 37 +++++++++++++++++++++++++++++++++++++
gcc/ipa-prop.h | 15 +--------------
gcc/symtab.cc | 21 +++++++++++++++------
6 files changed, 67 insertions(+), 26 deletions(-)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 50bae96de4c..9b4cb6383af 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -431,6 +431,10 @@ public:
/* Return true if ONE and TWO are part of the same COMDAT group. */
inline bool in_same_comdat_group_p (symtab_node *target);
+ /* Return true if symbol is known to be nonzero, assume that
+ flag_delete_null_pointer_checks is equal to delete_null_pointer_checks.
*/
+ bool nonzero_address (bool delete_null_pointer_checks);
+
/* Return true if symbol is known to be nonzero. */
bool nonzero_address ();
diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index a664bc03f62..5d7b3d25df5 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -1693,11 +1693,14 @@ ipa_vr_operation_and_type_effects (vrange &dst_vr,
/* Given a PASS_THROUGH jump function JFUNC that takes as its source SRC_VR of
SRC_TYPE and the result needs to be DST_TYPE, if any value range information
- can be deduced at all, intersect VR with it. */
+ can be deduced at all, intersect VR with it. CONTEXT_NODE is the call graph
+ node representing the function for which optimization flags should be
+ evaluated. */
static void
ipa_vr_intersect_with_arith_jfunc (vrange &vr,
ipa_jump_func *jfunc,
+ cgraph_node *context_node,
const value_range &src_vr,
tree src_type,
tree dst_type)
@@ -1720,7 +1723,7 @@ ipa_vr_intersect_with_arith_jfunc (vrange &vr,
if (!handler)
return;
value_range op_vr (TREE_TYPE (operand));
- ipa_range_set_and_normalize (op_vr, operand);
+ ipa_get_range_from_ip_invariant (op_vr, operand, context_node);
tree operation_type;
if (TREE_CODE_CLASS (operation) == tcc_comparison)
@@ -1776,7 +1779,8 @@ ipa_value_range_from_jfunc (vrange &vr,
value_range srcvr;
(*sum->m_vr)[idx].get_vrange (srcvr);
- ipa_vr_intersect_with_arith_jfunc (vr, jfunc, srcvr, src_type,
parm_type);
+ ipa_vr_intersect_with_arith_jfunc (vr, jfunc, cs->caller, srcvr,
src_type,
+ parm_type);
}
}
@@ -2562,7 +2566,7 @@ propagate_vr_across_jump_function (cgraph_edge *cs,
ipa_jump_func *jfunc,
if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR
|| !ipa_edge_within_scc (cs))
- ipa_vr_intersect_with_arith_jfunc (vr, jfunc,
+ ipa_vr_intersect_with_arith_jfunc (vr, jfunc, cs->caller,
src_lats->m_value_range.m_vr,
operand_type, param_type);
}
diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc
index 3f5e09960ef..6799ffe0b9a 100644
--- a/gcc/ipa-fnsummary.cc
+++ b/gcc/ipa-fnsummary.cc
@@ -518,7 +518,7 @@ evaluate_conditions_for_known_args (struct cgraph_node
*node,
value_range op0 (TREE_TYPE (op->val[0]));
range_op_handler handler (op->code);
- ipa_range_set_and_normalize (op0, op->val[0]);
+ ipa_get_range_from_ip_invariant (op0, op->val[0], node);
if (!handler
|| !res.supports_type_p (op->type)
@@ -537,7 +537,7 @@ evaluate_conditions_for_known_args (struct cgraph_node
*node,
value_range val_vr (TREE_TYPE (c->val));
range_op_handler handler (c->code);
- ipa_range_set_and_normalize (val_vr, c->val);
+ ipa_get_range_from_ip_invariant (val_vr, c->val, node);
if (!handler
|| !val_vr.supports_type_p (TREE_TYPE (c->val))
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index f0b915ba2be..0daa950985b 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -2312,6 +2312,43 @@ ipa_set_jfunc_vr (ipa_jump_func *jf, const ipa_vr &vr)
ipa_set_jfunc_vr (jf, tmp);
}
+/* Given VAL that conforms to is_gimple_ip_invariant, produce a VRANGE that
+ represents it as a range. CONTEXT_NODE is the call graph node representing
+ the function for which optimization flags should be evaluated. */
+
+void
+ipa_get_range_from_ip_invariant (vrange &r, tree val, cgraph_node
*context_node)
+{
+ if (TREE_CODE (val) == ADDR_EXPR)
+ {
+ symtab_node *symbol;
+ tree base = TREE_OPERAND (val, 0);
+ if (!DECL_P (base))
+ {
+ r.set_varying (TREE_TYPE (val));
+ return;
+ }
+ if (!decl_in_symtab_p (base))
+ {
+ r.set_nonzero (TREE_TYPE (val));
+ return;
+ }
+ if (!(symbol = symtab_node::get (base)))
+ {
+ r.set_varying (TREE_TYPE (val));
+ return;
+ }
+
+ bool delete_null_pointer_checks
+ = opt_for_fn (context_node->decl, flag_delete_null_pointer_checks);
+ if (symbol->nonzero_address (delete_null_pointer_checks))
+ r.set_nonzero (TREE_TYPE (val));
+ else
+ r.set_varying (TREE_TYPE (val));
+ }
+ else
+ r.set (val, val);
+}
/* If T is an SSA_NAME that is the result of a simple type conversion statement
from an integer type to another integer type which is known to be able to
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 1ff71aba830..d77340bfb6e 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -1258,7 +1258,7 @@ tree ipcp_get_aggregate_const (struct function *func,
tree parm, bool by_ref,
HOST_WIDE_INT bit_size);
bool unadjusted_ptr_and_unit_offset (tree op, tree *ret,
poly_int64 *offset_ret);
-
+void ipa_get_range_from_ip_invariant (vrange &r, tree val, cgraph_node *node);
void ipa_prop_cc_finalize (void);
/* From tree-sra.cc: */
@@ -1267,19 +1267,6 @@ tree build_ref_for_offset (location_t, tree, poly_int64,
bool, tree,
/* In ipa-cp.cc */
void ipa_cp_cc_finalize (void);
-
-/* Set R to the range of [VAL, VAL] while normalizing addresses to
- non-zero. */
-
-inline void
-ipa_range_set_and_normalize (vrange &r, tree val)
-{
- if (TREE_CODE (val) == ADDR_EXPR)
- r.set_nonzero (TREE_TYPE (val));
- else
- r.set (val, val);
-}
-
bool ipa_return_value_range (value_range &range, tree decl);
void ipa_record_return_value_range (value_range val);
bool ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2);
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index 5be463ff768..3804383a4e1 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -2207,10 +2207,11 @@ symtab_node::get_partitioning_class (void)
return SYMBOL_PARTITION;
}
-/* Return true when symbol is known to be non-zero. */
+/* Return true when symbol is known to be non-zero, assume that
+ flag_delete_null_pointer_checks is equal to delete_null_pointer_checks. */
bool
-symtab_node::nonzero_address ()
+symtab_node::nonzero_address (bool delete_null_pointer_checks)
{
/* Weakrefs may be NULL when their target is not defined. */
if (alias && weakref)
@@ -2234,7 +2235,7 @@ symtab_node::nonzero_address ()
if (target->resolution != LDPR_UNKNOWN
&& target->resolution != LDPR_UNDEF
&& !target->can_be_discarded_p ()
- && flag_delete_null_pointer_checks)
+ && delete_null_pointer_checks)
return true;
return false;
}
@@ -2251,7 +2252,7 @@ symtab_node::nonzero_address ()
When parsing, beware the cases when WEAK attribute is added later. */
if ((!DECL_WEAK (decl) || DECL_COMDAT (decl))
- && flag_delete_null_pointer_checks)
+ && delete_null_pointer_checks)
{
refuse_visibility_changes = true;
return true;
@@ -2262,7 +2263,7 @@ symtab_node::nonzero_address ()
Play safe for flag_delete_null_pointer_checks where weak definition may
be re-defined by NULL. */
if (definition && !DECL_EXTERNAL (decl)
- && (flag_delete_null_pointer_checks || !DECL_WEAK (decl)))
+ && (delete_null_pointer_checks || !DECL_WEAK (decl)))
{
if (!DECL_WEAK (decl))
refuse_visibility_changes = true;
@@ -2273,11 +2274,19 @@ symtab_node::nonzero_address ()
if (resolution != LDPR_UNKNOWN
&& resolution != LDPR_UNDEF
&& !can_be_discarded_p ()
- && flag_delete_null_pointer_checks)
+ && delete_null_pointer_checks)
return true;
return false;
}
+/* Return true when symbol is known to be non-zero. */
+
+bool
+symtab_node::nonzero_address ()
+{
+ return nonzero_address (flag_delete_null_pointer_checks);
+}
+
/* Return 0 if symbol is known to have different address than S2,
Return 1 if symbol is known to have same address as S2,
return -1 otherwise.
--
2.47.1
