On 10/28/25 15:00, Robin Dapp wrote:
On 10/27/25 10:25, Robin Dapp wrote:
Hmm, code does not match comment?  I think you want r.nonzero_p ().
But that also means that at (*) we cannot simply leave 'r' to be the
range of the unshifted src.

+       assumptions = boolean_true_node;
+    }
+
+  /* If that didn't work use simplify_using_initial_conditions.  */
+  if (assumptions == boolean_false_node)
Yeah, I originally used the path-based ranger and had an inverse condition.
Then I removed it and swapped the condition, realizing that things still work.
But they don't actually... as only a path-based approach has enough information
but the boundary conditions here are constricted enough that my test case still
passes (similar to v1) because nonzero_p is also the wrong check...

I re-introduced the path-based approach now, using a helper function, and
re-tested everything on x86, rv64gcv_zvl512b, and power10 in the attached v3.

Regards

You should not need  path ranger to do this, a normal ranger should be
enough.

+/* Helper for number_of_iterations_cltz that uses path-based ranger to
+   determine if SRC, shifted left (when LEFT_SHIFT is true) or right
+   by NUM_IGNORED_BITS, is guaranteed to be != 0 inside LOOP.
+   Return true if so or false otherwise.  */
+
+static bool
+shifted_range_nonzero_p (loop_p loop, tree src,
+                        bool left_shift, int num_ignored_bits)
+{
+  int_range_max r (TREE_TYPE (src));
+  gcc_assert (num_ignored_bits >= 0);
+
+  auto_vec<basic_block, 2> path;
+  path.safe_push (loop->header);
+  path.safe_push (loop_preheader_edge (loop)->src);
+
+  bool range_nonzero = false;
+  gimple_ranger ranger;
+  path_range_query query (ranger, path);
+
+  if (query.range_of_expr (r, src)
THis will only pick up a global  value for src.  If I look at where ou
are calling from,

     tree src = gimple_phi_arg_def (phi, loop_preheader_edge (loop)->dest_idx);

I presume what you are looking for is the value of src coming in on that
edge?

in which case you should only have to do

if (get_range_query (cfun)->range_on_edge (r, loop_preheader_edge
(loop), src))

to get the range.. which  is what you were doing in the original patch.

In the latest version of the patch that doesn't use the path ranger,  is
there an active ranger?  (ie, the enable_ranger () call) .  If not, all
you will get is global values again.

Whats the current version of the patch, using the get_range_query()->
range_on_edge ()      ?
The loop is roughly:

   if (bits)
    {
     if (!(bits & 1)
     {
       do
        {
          bits >>= 1;
          cnt += 1;
        } while (!(bits & 1));

and we need to prove that bits >> 1 != 0 in order to know the loop terminates.

   get_range_query (cfun)->range_on_edge (r, loop_preheader_edge (loop), src)

I added a chunk to print the range being returned.    With your path ranger, R is [irange] BITMAP_WORD [1, +INF]  , which I assuem si the value you are looking for?

(1)

So I tried taking your path ranger patch, and directly used the ranger you  created for the path ranger, and avoided the path ranger completely:

diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index c955d7b31e9..d1ac8980e1f 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -2342,7 +2342,7 @@ shifted_range_nonzero_p (loop_p loop, tree src,
   gimple_ranger ranger;
   path_range_query query (ranger, path);

-  if (query.range_of_expr (r, src)
+  if (ranger.range_on_edge (r, loop_preheader_edge (loop), src)
       && !r.varying_p ()
       && !r.undefined_p ())
     {


And I get the same range  as the path ranger (on x86_64): [irange] BITMAP_WORD [1, +INF]


ie

(2) It ceases working if I switch to get_range_query(cfun), probably because there is no active ranger.

diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index c955d7b31e9..7bbd521d897 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -2339,10 +2339,8 @@ shifted_range_nonzero_p (loop_p loop, tree src,
   path.safe_push (loop_preheader_edge (loop)->src);

   bool range_nonzero = false;
-  gimple_ranger ranger;
-  path_range_query query (ranger, path);

-  if (query.range_of_expr (r, src)
+  if (get_range_query (cfun)->range_on_edge (r, loop_preheader_edge (loop), src)
       && !r.varying_p ()
       && !r.undefined_p ())
     {

This version (2) gives me VARYING.


(3)

If I add the enable/disable ranger calls in the function, which creates an active ranger again,  it Im back to getting [+1, +INF] for a range:

diff --git a/gcc/tree-ssa-loop-niter.cc b/gcc/tree-ssa-loop-niter.cc
index c955d7b31e9..645ae271d1d 100644
--- a/gcc/tree-ssa-loop-niter.cc
+++ b/gcc/tree-ssa-loop-niter.cc
@@ -2339,10 +2339,9 @@ shifted_range_nonzero_p (loop_p loop, tree src,
   path.safe_push (loop_preheader_edge (loop)->src);

   bool range_nonzero = false;
-  gimple_ranger ranger;
-  path_range_query query (ranger, path);

-  if (query.range_of_expr (r, src)
+  enable_ranger(cfun);
+  if (get_range_query (cfun)->range_on_edge (r, loop_preheader_edge (loop), src)
       && !r.varying_p ()
       && !r.undefined_p ())
     {
@@ -2370,7 +2369,7 @@ fprintf (stderr, "\n");
       if (!range_includes_zero_p (r))
        range_nonzero = true;
     }
-
+disable_ranger(cfun);
   return range_nonzero;
 }

Maybe we should just enable_ranger in the pass?  But I don't think you need a path ranger to get that info.


returned a varying range but the "path-based" approach seemed to work.
I was hoping that a path-based approach would combine the conditions
bits != 0 && (bits & 1) == 0, and that was what I appeared to be getting.

You're saying that the way I used it is not path-based?  I didn't explicitly
call enable_ranger () but adding the preheader and loop header path as in v3
still got me different results than just the range-on-edge query.


Or am I missing something else?

Andrew

Reply via email to