Re: [committed] Minor cleanup/prep in DOM
On 9/30/22 18:07, H.J. Lu wrote: On Fri, Sep 30, 2022 at 4:06 PM Jeff Law wrote: It's a bit weird that free_dom_edge_info leaves a dangling pointer in e->aux. Not sure what I was thinking. There's two callers. One wipes e->aux immediately after the call, the other attaches a newly created object immediately after the call. So we can wipe e->aux within the call and simplify one of the two call sites. This is preparatory work for a minor optimization where we want to detect another class of edge equivalences in DOM (until something better is available) and either attach them an existing edge_info structure or create a new one if one doesn't currently exist for a given edge. Bootstrapped and regression tested on x86_64. Installing on the trunk. I got [ ... ] Pushed wrong version. Sorry about that. Fixed by attached patch which just moves the test down to its proper position. Jeff commit aa360fbf68b11e54017e8fa5b1bdb87ce7c19188 Author: Jeff Law Date: Fri Sep 30 20:46:04 2022 -0400 Install correct patch version. gcc/ * tree-ssa-dom.cc (record_edge_info): Install correct version of patch. diff --git a/gcc/tree-ssa-dom.cc b/gcc/tree-ssa-dom.cc index 8d8312ca350..e6b8dace5e9 100644 --- a/gcc/tree-ssa-dom.cc +++ b/gcc/tree-ssa-dom.cc @@ -684,11 +684,6 @@ record_edge_info (basic_block bb) !gsi_end_p (gsi); gsi_next (&gsi)) { - /* If the other alternative is the same as the result, -then this is a degenerate and can be ignored. */ - if (dst == PHI_ARG_DEF (phi, !alternative)) - continue; - /* Now get the EDGE_INFO class so we can append it to our list. We want the successor edge where the destination is not the source of @@ -697,6 +692,11 @@ record_edge_info (basic_block bb) tree src = PHI_ARG_DEF (phi, alternative); tree dst = PHI_RESULT (phi); + /* If the other alternative is the same as the result, +then this is a degenerate and can be ignored. */ + if (dst == PHI_ARG_DEF (phi, !alternative)) + continue; + if (EDGE_SUCC (bb, 0)->dest != EDGE_PRED (bb, !alternative)->src) edge_info = (class edge_info *)EDGE_SUCC (bb, 0)->aux;
Re: [committed] Minor cleanup/prep in DOM
On Fri, Sep 30, 2022 at 4:06 PM Jeff Law wrote: > > > It's a bit weird that free_dom_edge_info leaves a dangling pointer in > e->aux. Not sure what I was thinking. > > > There's two callers. One wipes e->aux immediately after the call, the > other attaches a newly created object immediately after the call. So we > can wipe e->aux within the call and simplify one of the two call sites. > > This is preparatory work for a minor optimization where we want to > detect another class of edge equivalences in DOM (until something better > is available) and either attach them an existing edge_info structure or > create a new one if one doesn't currently exist for a given edge. > > Bootstrapped and regression tested on x86_64. Installing on the trunk. > I got /export/gnu/import/git/sources/gcc/gcc/tree-ssa-dom.cc: In function ‘void record_edge_info(basic_block)’: /export/gnu/import/git/sources/gcc/gcc/tree-ssa-dom.cc:689:27: error: ‘dst’ was not declared in this scope; did you mean ‘dse’? 689 | if (dst == PHI_ARG_DEF (phi, !alternative)) | ^~~ | dse In file included from /export/gnu/import/git/sources/gcc/gcc/gimple-ssa.h:24, from /export/gnu/import/git/sources/gcc/gcc/ssa.h:27, from /export/gnu/import/git/sources/gcc/gcc/tree-ssa-dom.cc:28: /export/gnu/import/git/sources/gcc/gcc/tree-ssa-dom.cc:689:47: error: ‘phi’ was not declared in this scope; did you mean ‘gphi’? 689 | if (dst == PHI_ARG_DEF (phi, !alternative)) | ^~~ /export/gnu/import/git/sources/gcc/gcc/tree-ssa-operands.h:82:54: note: in definition of macro ‘PHI_ARG_DEF’ 82 | #define PHI_ARG_DEF(PHI, I) gimple_phi_arg_def ((PHI), (I)) | ^~~ make: *** [Makefile:1146: tree-ssa-dom.o] Error 1 -- H.J.
[committed] Minor cleanup/prep in DOM
It's a bit weird that free_dom_edge_info leaves a dangling pointer in e->aux. Not sure what I was thinking. There's two callers. One wipes e->aux immediately after the call, the other attaches a newly created object immediately after the call. So we can wipe e->aux within the call and simplify one of the two call sites. This is preparatory work for a minor optimization where we want to detect another class of edge equivalences in DOM (until something better is available) and either attach them an existing edge_info structure or create a new one if one doesn't currently exist for a given edge. Bootstrapped and regression tested on x86_64. Installing on the trunk. Jeff commit fbd95c027edcc169cc3b40806375fbabc08500e0 Author: Jeff Law Date: Fri Sep 30 18:59:24 2022 -0400 Minor cleanup/prep in DOM It's a bit weird that free_dom_edge_info leaves a dangling pointer in e->aux. Not sure what I was thinking. There's two callers. One wipes e->aux immediately after the call, the other attaches a newly created object immediately after the call. So we can wipe e->aux within the call and simplify one of the two call sites. This is preparatory work for a minor optimization where we want to detect another class of edge equivalences in DOM (until something better is available) and either attach them an existing edge_info structure or create a new one if one doesn't currently exist for a given edge. gcc/ * tree-ssa-dom.cc (free_dom_edge_info): Clear e->aux too. (free_all_edge_infos): Do not clear e->aux here. diff --git a/gcc/tree-ssa-dom.cc b/gcc/tree-ssa-dom.cc index 84bef798f52..fa43dbe6c44 100644 --- a/gcc/tree-ssa-dom.cc +++ b/gcc/tree-ssa-dom.cc @@ -393,7 +393,8 @@ edge_info::record_simple_equiv (tree lhs, tree rhs) simple_equivalences.safe_push (equiv_pair (lhs, rhs)); } -/* Free the edge_info data attached to E, if it exists. */ +/* Free the edge_info data attached to E, if it exists and + clear e->aux. */ void free_dom_edge_info (edge e) @@ -402,6 +403,7 @@ free_dom_edge_info (edge e) if (edge_info) delete edge_info; + e->aux = NULL; } /* Free all EDGE_INFO structures associated with edges in the CFG. @@ -420,10 +422,7 @@ free_all_edge_infos (void) FOR_EACH_BB_FN (bb, cfun) { FOR_EACH_EDGE (e, ei, bb->preds) -{ - free_dom_edge_info (e); - e->aux = NULL; - } + free_dom_edge_info (e); } }