On 4/25/23 22:34, Jeff Law wrote:
On 4/24/23 07:51, Andrew MacLeod wrote:
Its not a real cache.. its merely a statement shortcut in dependency
analysis to avoid re-parsing statements every time we look at them
for dependency analysis
It is not suppose to be used for anything other than dependency
checking. ie, if an SSA_NAME changes, we can check if it matches
either of the 2 "cached" names on this DEF, and if so, we know this
name is stale. we are never actually suppose to use the dependency
cached values to drive anything, merely respond to the question if
either matches a given name. So it doesnt matter if the name here
has been freed
OK. I'll take your word for it. Note that a free'd SSA_NAME may have
an empty TREE_TYPE or an unexpected TREE_CHAIN field IIRC. So you have
to be a bit careful if you're going to allow them.
We never re-use SSA names from within the pass releasing it. But if
the ranger cache
persists across passes this could be a problem. See
This particular valueswould never persist beyond a current pass.. its
just the dependency chains and they would get rebuilt every time
because the IL has changed.
Good. THat would limit the concerns significantly. I don't think we
recycle names within a pass anymore (we used to within DOM due to the
way threading worked eons ago, but we no longer take things out of SSA
form to handle the CFG/SSA graph updates. One could even argue we
don't need to maintain the freelist and recycle names anymore.
Jeff
well, no worries. taken care of thusly for the future. Its a hair
slower, but nothing outrageous
Bootstrapped on x86_64-pc-linux-gnu with no regressions. Pushed.
Andrew
From a530eb642032da7ad4d30de51131421631055f72 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacl...@redhat.com>
Date: Tue, 25 Apr 2023 15:33:52 -0400
Subject: [PATCH 1/5] Don't save ssa-name pointer in dependency cache.
If the direct dependence fields point directly to an ssa-name,
its possible that an optimization frees an ssa-name, and the value
pointed to may now be in the free list. Simply maintain the ssa
version number instead.
PR tree-optimization/109417
* gimple-range-gori.cc (range_def_chain::register_dependency):
Save the ssa version number, not the pointer.
(gori_compute::may_recompute_p): No need to check if a dependency
is in the free list.
* gimple-range-gori.h (class range_def_chain): Change ssa1 and ssa2
fields to be unsigned int instead of trees.
(ange_def_chain::depend1): Adjust.
(ange_def_chain::depend2): Adjust.
* gimple-range.h: Include "ssa.h" to inline ssa_name().
---
gcc/gimple-range-gori.cc | 8 ++++----
gcc/gimple-range-gori.h | 14 ++++++++++----
gcc/gimple-range.h | 1 +
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index d77e1f51ac2..5bba77c7b7b 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -182,9 +182,9 @@ range_def_chain::register_dependency (tree name, tree dep, basic_block bb)
// Set the direct dependency cache entries.
if (!src.ssa1)
- src.ssa1 = dep;
- else if (!src.ssa2 && src.ssa1 != dep)
- src.ssa2 = dep;
+ src.ssa1 = SSA_NAME_VERSION (dep);
+ else if (!src.ssa2 && src.ssa1 != SSA_NAME_VERSION (dep))
+ src.ssa2 = SSA_NAME_VERSION (dep);
// Don't calculate imports or export/dep chains if BB is not provided.
// This is usually the case for when the temporal cache wants the direct
@@ -1316,7 +1316,7 @@ gori_compute::may_recompute_p (tree name, basic_block bb, int depth)
// If the first dependency is not set, there is no recomputation.
// Dependencies reflect original IL, not current state. Check if the
// SSA_NAME is still valid as well.
- if (!dep1 || SSA_NAME_IN_FREE_LIST (dep1))
+ if (!dep1)
return false;
// Don't recalculate PHIs or statements with side_effects.
diff --git a/gcc/gimple-range-gori.h b/gcc/gimple-range-gori.h
index 3ea4b45595b..526edc24b53 100644
--- a/gcc/gimple-range-gori.h
+++ b/gcc/gimple-range-gori.h
@@ -46,8 +46,8 @@ protected:
bitmap_obstack m_bitmaps;
private:
struct rdc {
- tree ssa1; // First direct dependency
- tree ssa2; // Second direct dependency
+ unsigned int ssa1; // First direct dependency
+ unsigned int ssa2; // Second direct dependency
bitmap bm; // All dependencies
bitmap m_import;
};
@@ -66,7 +66,10 @@ range_def_chain::depend1 (tree name) const
unsigned v = SSA_NAME_VERSION (name);
if (v >= m_def_chain.length ())
return NULL_TREE;
- return m_def_chain[v].ssa1;
+ unsigned v1 = m_def_chain[v].ssa1;
+ if (!v1)
+ return NULL_TREE;
+ return ssa_name (v1);
}
// Return the second direct dependency for NAME, if there is one.
@@ -77,7 +80,10 @@ range_def_chain::depend2 (tree name) const
unsigned v = SSA_NAME_VERSION (name);
if (v >= m_def_chain.length ())
return NULL_TREE;
- return m_def_chain[v].ssa2;
+ unsigned v2 = m_def_chain[v].ssa2;
+ if (!v2)
+ return NULL_TREE;
+ return ssa_name (v2);
}
// GORI_MAP is used to accumulate what SSA names in a block can
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index 7ed4d3870b8..b8ddca59d2d 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_GIMPLE_RANGE_H
#define GCC_GIMPLE_RANGE_H
+#include "ssa.h"
#include "range.h"
#include "value-query.h"
#include "gimple-range-op.h"
--
2.39.2
