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; } // Pushes a marker at the given point.
