On 6/9/21 12:50 PM, Aldy Hernandez wrote:
On 6/9/21 7:10 PM, Martin Sebor wrote:
On 6/7/21 12:29 PM, Aldy Hernandez via Gcc-patches wrote:
Mostly just a question of the type choices in the implementation
of the ssa_equiv_stack class: m_stack is an auto_vec while
m_replacements is a plain array. I'd expect both to be the same
(auto_vec). Is there a reason for this choice?
If not, I'd suggest to use auto_vec. That way the ssa_equiv_stack
class shouldn't need a dtor (auto_vec doesn't need to be explicitly
released before it's destroyed), though it should delete its copy
and assignment.
TBH, the ssa_equiv_stack was lifted from evrp_range_analyzer (there are
also 2 more copies of the same mechanism in tree-ssa-scopedtables.h).
The code there already used an auto_vec, so I left that. However, for a
mere array of trees, I thought it'd be simpler to use new/delete. Which
in retrospect was easy to get wrong, as can be seen by:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100984
Would something like the code below work? It should also fix the PR.
Thanks.
Aldy
diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
index 7e1cf51239a..808d47506be 100644
--- a/gcc/gimple-ssa-evrp.c
+++ b/gcc/gimple-ssa-evrp.c
@@ -61,19 +61,18 @@ public:
private:
auto_vec<std::pair <tree, tree>> m_stack;
- tree *m_replacements;
+ auto_vec<tree> m_replacements;
const std::pair <tree, tree> m_marker = std::make_pair (NULL, NULL);
};
ssa_equiv_stack::ssa_equiv_stack ()
{
- m_replacements = new tree[num_ssa_names] ();
+ m_replacements.safe_grow_cleared (num_ssa_names);
}
ssa_equiv_stack::~ssa_equiv_stack ()
{
m_stack.release ();
- delete m_replacements;
}
Yes, this is what I was suggesting, with the additional removal of
the dtor above (since release() is called from the auto_vec dtor).
(Once auto_vec is safe to copy and assign we'll also get copy and
assignment for nothing.)
Martin
PS Enhancing -Wmismatched-new-delete to detect the bug in PR 100984
is on my to do list.
