On 3/29/25 20:54, Jeff Law wrote:
On 3/4/25 2:10 AM, Jakub Jelinek wrote:
Hi!
On Fri, Nov 22, 2024 at 07:25:23PM -0500, Andrew MacLeod wrote:
I will shortly be submitting , and presumable committing, this
patch as
part of a series to improve VRP time for 117467..
So it may be in place by the time you need it
On 11/18/24 09:31, Andrew MacLeod wrote:
Attached is a pre-approved patch which adds a range_query to the
inferred range mechanism.
The only change you will need to make is to replace "get_range_query
(cfun)->" with "q->" which is passed in.
This regstraps on x86 without your patch, and I got as far as a
bootstrap with your patches..
Sorry for the delay. Only recently the remaining patches have been
committed, so I'm getting back to this.
The current state of the trunk is that it handles constant arg2 (if
it is
zero or non-zero) and punts on everything else.
I've changed get_range_query (cfun)-> to q-> but unfortunately the test
FAILs with it, while it improves the if (n >= 42) case where there is a
SSA_NAME for n - 10, it doesn't improve the if (n) case, so it feels
like
it is querying just the global range rather than the local one from the
statement, because on the foo (b, n); statement which is guarded by
if (n)
n should be provably [1, SIZE_MAX].
The patch still bootstraps/regtests on x86_64-linux and i686-linux
and improves at least something, so I guess I could just comment out
part
of the testcase. Any thoughts why this happens though? And is that
something
that can be improved for GCC 15 or should wait for GCC 16?
2025-03-04 Jakub Jelinek <ja...@redhat.com>
Andrew MacLeod <amacl...@redhat.com>
PR c/117023
* gimple-range-infer.cc (gimple_infer_range::gimple_infer_range):
For nonnull_if_nonzero attribute check also arg2 range if it doesn't
include zero and in that case call add_nonzero too.
* gcc.dg/tree-ssa/pr78154-2.c: New test.
Feels like it should defer to gcc-16. But I'll defer (haha) to
Jakub's judgment on whether or not fixing this up for gcc-15 is critical.
jeff
OK, I took a look. Yes, VRP folding was still only using global ranges.
When VRP folds a statement it also adds any inferred ranges to the
oracle, via a call to register_inferred_ranges:
bool fold_stmt (gimple_stmt_iterator *gsi) override
{
bool ret = m_simplifier.simplify (gsi);
if (!ret)
ret = m_ranger->fold_stmt (gsi, follow_single_use_edges);
m_ranger->register_inferred_ranges (gsi_stmt
(*gsi)); <<<<---- Here.
return ret;
}
That is passed on by ranger to ranger's cache. Ranger is a range-query
object, but the cache is also implemented as a *read-only*
range-query.. it simply populates the cache as needed looking up
values. So when ranger passes it on:
void
gimple_ranger::register_inferred_ranges (gimple *s)
{
// First, export the LHS if it is a new global range.
tree lhs = gimple_get_lhs (s);
if (lhs)
{
value_range tmp (TREE_TYPE (lhs));
if (range_of_stmt (tmp, s, lhs) && !tmp.varying_p ())
set_range_info (lhs, tmp);
}
m_cache.apply_inferred_ranges (s); <<<<----- Passed on here.
}
The cache looks at the statement, and actually registers any inferred
ranges with the inferred range oracle.
void
ranger_cache::apply_inferred_ranges (gimple *s)
{
bool update = true;
basic_block bb = gimple_bb (s);
gimple_infer_range infer(s); <<<<<----- DOH! No range query provided.
if (infer.num () == 0)
return;
// Register ranges with the infer oracle.
<...>
so. When VRP does the folding, we aren't picking up any context. The
fix is for the cache to pass itself in.. This will result in a read-only
lookup which avoids any potential cycles, and as we are doing a DOM
walk, should resolve the issues. Patch is attached. It fixes your
testcase.
I am running it through a bootstrap/regression cycle (hasnt finished
yet). In theory it should be safe. It is only your condition which
actually uses the range query at the moment. The op1_range and
op2_range calls at the bottom of the function invoke it on constants
only I think, so they shouldn't be impacted.
If this works for you, I would suggest simply adding it to your patch as
that is the only thing really needing it at the moment.
Andrew
From 7116177599a3bb907d21fe642a4bdcb401e1263b Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
Date: Mon, 31 Mar 2025 11:18:22 -0400
Subject: [PATCH 2/2] Use the current cache when creating inferred ranges.
Infer range processing was adjusted to allow a query to be specified,
but during VRP folding, ranger w3as not providing a query. This results
in contextual ranges being missed. Pass the cache in as the query
which provide a read-only query of the current state.
* gimple-range-cache.cc (ranger_cache::apply_inferred_ranges): Pass
'this' as the range-query to the inferred range constructor.
---
gcc/gimple-range-cache.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index 818b801468a..ecf03319cd4 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -1861,7 +1861,7 @@ ranger_cache::apply_inferred_ranges (gimple *s)
bool update = true;
basic_block bb = gimple_bb (s);
- gimple_infer_range infer(s);
+ gimple_infer_range infer(s, this);
if (infer.num () == 0)
return;
--
2.45.0
