https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98464
--- Comment #10 from Kewen Lin <linkw at gcc dot gnu.org> --- (In reply to rguent...@suse.de from comment #9) > On Mon, 4 Jan 2021, linkw at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98464 > > > > --- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> --- > > (In reply to Richard Biener from comment #5) > > > But this > > > > > > sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), > > > use); > > > > > > should make it more conservative (compared to the more desirable use of > > > gimple_bb (stmt) aka 'b'). > > > > > > The issue is really that dominated_by_p_w_unex is not "transitive" with > > > respect to an intermediate immediate dominator chain. > > > > > > We can make that more consistent at least by doing sth like your patch but > > > only in vn_valueize_wrapper. > > > > OK, just noticed that wrapper is only for simplify_replace_tree. Thanks for > > fixing. But I still don't get why it's buggy for rpo_vn_valueize. > > Because rpo_vn_valueize argument can be a value handle SSA which does > not need to be a definition on the path to the stmt we are simplifying. > rpo_vn_valueize is called from (too) many contexts and I'm not 100% > convinced we're only feeding it names available at the location we are > processing. > Thanks for the explanation! I'll keep in mind of this kind of possibility. > Note I think we should use 'b' and not SSA_NAME_DEF_STMT of the use but > then this code is as-is since the rewrite and I don't feel tracking down > fallout of changing this back now (but I'll try to remember for stage1, > as well as somehow better dealing with dominated_by_w_unex) Yeah, I totally agree that improving dominated_by_w_unex is better. The existing checking in dominated_by_w_unex is limited, I did one hack on it by recursing one more argument depth and special handling on backedge before I went with the direction with SSA_NAME_DEF_STMT, but thought it can enlarge FRE applied scope and also impact compilation time definitely, looks not a good direction especially now is stage3.
