On 5/5/23 17:10, Martin Jambor wrote:
Hello,
On Wed, Apr 26 2023, Aldy Hernandez via Gcc-patches wrote:
gcc/ChangeLog:
* ipa-cp.cc (ipa_vr_operation_and_type_effects): Convert to ranger API.
(ipa_value_range_from_jfunc): Same.
(propagate_vr_across_jump_function): Same.
* ipa-fnsummary.cc (evaluate_conditions_for_known_args): Same.
* ipa-prop.cc (ipa_compute_jump_functions_for_edge): Same.
* vr-values.cc (bounds_of_var_in_loop): Same.
thanks for taking care of the value range uses in IPA.
---
gcc/ipa-cp.cc | 28 +++++++++++++++++++++------
gcc/ipa-fnsummary.cc | 45 ++++++++++++++++++++++++++++----------------
gcc/ipa-prop.cc | 5 ++---
gcc/vr-values.cc | 6 ++++--
4 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index 65c49558b58..6788883c40b 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -128,6 +128,7 @@ along with GCC; see the file COPYING3. If not see
#include "attribs.h"
#include "dbgcnt.h"
#include "symtab-clones.h"
+#include "gimple-range.h"
template <typename valtype> class ipcp_value;
@@ -1900,10 +1901,15 @@ ipa_vr_operation_and_type_effects (value_range *dst_vr,
enum tree_code operation,
tree dst_type, tree src_type)
{
- range_fold_unary_expr (dst_vr, operation, dst_type, src_vr, src_type);
- if (dst_vr->varying_p () || dst_vr->undefined_p ())
+ if (!irange::supports_p (dst_type) || !irange::supports_p (src_type))
return false;
- return true;
+
+ range_op_handler handler (operation, dst_type);
Would it be possible to document the range_op_handler class somewhat?
+ return (handler
+ && handler.fold_range (*dst_vr, dst_type,
+ *src_vr, value_range (dst_type))
+ && !dst_vr->varying_p ()
+ && !dst_vr->undefined_p ());
It looks important but the class is not documented at all. Although the
use of fold_range is probably hopefully mostly clear from its uses in
this patch, the meaning of the return value of this method and what
other methods do is less obvious.
For example, I am curious why (not in this patch, but in the code as it
is now in the repo), uses of fold_range seem to be always preceeded with
a check for supports_type_p, even though the type is then also fed into
fold_range itself. Does the return value of fold_range mean something
slightly different from "could not deduce anything?"
Oh, I see what you mean.
Take for instance this bit in ipa-cp:
if (!irange::supports_p (dst_type) || !irange::supports_p (src_type))
return false;
range_op_handler handler (operation, dst_type);
return (handler
&& handler.fold_range (*dst_vr, dst_type,
*src_vr, value_range (dst_type))
&& !dst_vr->varying_p ()
&& !dst_vr->undefined_p ());
range_op_handler::fold_range() takes a type agnostic vrange (from which
irange inherits). If you pass it an irange, but the type is say a
float, you'll get an ICE downstream.
Ranger itself is type agnostic and takes a vrange almost everywhere.
It's up to the user to make sure the the range type and the type of the
operation matches.
Eventually we should convert all those value_range arguments in IPA to
vrange and have it work in a type agnostic manner. I have patches for
this, but I still have to flush out all this preliminary stuff :).
Aldy
