Hi, > > > > Hi, > > > > the attached Ada testcase compiled with -O2 -gnatn makes the compiler crash > > in > > vect_can_force_dr_alignment_p during SLP vectorization: > > > > if (decl_in_symtab_p (decl) > > && !symtab_node::get (decl)->can_increase_alignment_p ()) > > return false; > > > > because symtab_node::get (decl) returns a null node. The phenomenon occurs > > for a pair of twin symbols listed like so in .cgraph: > > > > Opt7_Pkg.T12b/17 (Opt7_Pkg.T12b) > > Type: variable definition analyzed > > Visibility: semantic_interposition external public artificial > > Aux: @0x44d45e0 > > References: > > Referring: opt7_pkg__enum_name_table/13 (addr) > > opt7_pkg__enum_name_table/13 > > (addr) > > Availability: not-ready > > Varpool flags: initialized read-only const-value-known > > > > Opt7_Pkg.T8b/16 (Opt7_Pkg.T8b) > > Type: variable definition analyzed > > Visibility: semantic_interposition external public artificial > > Aux: @0x7f9fda3fff00 > > References: > > Referring: opt7_pkg__enum_name_table/13 (addr) > > opt7_pkg__enum_name_table/13 > > (addr) > > Availability: not-ready > > Varpool flags: initialized read-only const-value-known > > > > with: > > > > opt7_pkg__enum_name_table/13 (Opt7_Pkg.Enum_Name_Table) > > Type: variable definition analyzed > > Visibility: semantic_interposition external public > > Aux: @0x44d45e0 > > References: Opt7_Pkg.T8b/16 (addr) Opt7_Pkg.T8b/16 (addr) Opt7_Pkg.T12b/17 > > (addr) Opt7_Pkg.T12b/17 (addr) > > Referring: opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) > > opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) > > opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) > > Availability: not-ready > > Varpool flags: initialized read-only const-value-known > > > > being the crux of the matter. > > > > What happens is that symtab_remove_unreachable_nodes leaves the last symbol > > in > > kind of a limbo state: in .remove_symbols, we have: > > > > opt7_pkg__enum_name_table/13 (Opt7_Pkg.Enum_Name_Table) > > Type: variable > > Body removed by symtab_remove_unreachable_nodes > > Visibility: externally_visible semantic_interposition external public > > References: > > Referring: opt7_pkg__image/2 (read) opt7_pkg__image/2 (read) > > Availability: not_available > > Varpool flags: initialized read-only const-value-known > > > > This means that the "body" (DECL_INITIAL) of the symbol has been disregarded > > during reachability analysis, causing the first two symbols to be discarded: > > > > Reclaiming variables: Opt7_Pkg.T12b/17 Opt7_Pkg.T8b/16 > > > > but the DECL_INITIAL is explicitly preserved for later constant folding, > > which > > makes it possible to retrofit the DECLs corresponding to the first two > > symbols > > in the GIMPLE IR and ultimately leads vect_can_force_dr_alignment_p to > > crash. > > > > > > The decision to disregard the "body" (DECL_INITIAL) of the symbol is made in > > the first process_references present in ipa.cc: > > > > if (node->definition && !node->in_other_partition > > && ((!DECL_EXTERNAL (node->decl) || node->alias) > > || (possible_inline_candidate_p (node) > > /* We use variable constructors during late compilation for > > constant folding. Keep references alive so partitioning > > knows about potential references. */ > > || (VAR_P (node->decl) > > && (flag_wpa > > || flag_incremental_link > > == INCREMENTAL_LINK_LTO) > > && dyn_cast <varpool_node *> (node) > > ->ctor_useable_for_folding_p ())))) > > > > because neither flag_wpa nor flag_incremental_link = INCREMENTAL_LINK_LTO is > > true, while the decision to ultimately preserve the DECL_INITIAL is made > > later > > in remove_unreachable_nodes: > > > > /* Keep body if it may be useful for constant folding. */ > > if ((flag_wpa || flag_incremental_link == INCREMENTAL_LINK_LTO) > > || ((init = ctor_for_folding (vnode->decl)) == error_mark_node)) > > vnode->remove_initializer (); > > else > > DECL_INITIAL (vnode->decl) = init; > > > > > > I think that the testcase shows that the "body" of > > ctor_useable_for_folding_p > > symbols must always be considered for reachability analysis (which could > > make > > the above test on ctor_for_folding useless). But implementing that > > introduces > > a regression for g++.dg/ipa/devirt-39.C, because the vtable is preserved and > > in turn forces the method to be preserved, hence the special case for > > vtables. > > > > The test also renames the first process_references function in ipa.cc to > > clear > > the confusion with the second function in the same file. > > > > Bootstrapped/regtested on x86-64/Linux, OK for the mainline? > > Ah, I've run into the same issue with IPA PTA recently, unfortunately Honza > seems unresponsive in bugzilla. IMO the patch looks OK, but let's give Honza > the chance to chime in here - esp. the DECL_VIRTUAL special-casing is > sth I'm not familiar with (wouldn't this apply to all COMDATs? but only > considering whole-groups?) Sorry for slow reaction - I am trying to catch up after end of the semester.
The original problem here was indeed the devirtualization. If C++ class is keyed we will produce virtual table as extern const variable. This table is useful for devirtualizatoin but at some time we need to be sure to not output all function bodies (which may be COMDAT) referenced by them (and only those which was actually used for devirtualization). Last time we can do it is the WPA stage, so idea is to keep all stuff reacable by external constructor alive until inlining and remove it then (devirtualization after inlining is not terribly useful transformation anyway). The code is indeed trying to keep constructors around for possible later folding whenever this is still doable. This does not introduce completely new problem. Some decls are not accessible from very begining of middle-end compilation. C++ is bit shady about what cen be used especially if -fdefault-visibility=hidden is used which may hide the symbols referneced by virtual table in DSO. So there is can_refer_decl_in_current_unit_p which tries to make C++ pakcages happy and also should cover the logic on when we can still access after the ctor has been oficicially moreved from vtable. So if you make everything referenced by external constructors reachable except for vtables I think we are moving the problem of outputting dead code to non-vtable case. I will need to check - does the retrofitting of decl into code go via can_refer_decl_in_current_unit_p? In most cases this actually tests that symbol table is present... Honza > > Thanks, > Richard. > > > > > > > 2025-05-29 Eric Botcazou <ebotca...@adacore.com> > > > > * ipa.cc (process_references): Rename into... > > (mark_references): ...this. Always mark referenced external > > variables as reachable if they are usable for folding, except > > for vtables. > > (symbol_table::remove_unreachable_nodes): Adjust to renaming. > > > > > > 2025-05-29 Eric Botcazou <ebotca...@adacore.com> > > > > * gnat.dg/specs/opt7.ads: New test. > > * gnat.dg/specs/opt7_pkg.ads: New helper. > > * gnat.dg/specs/opt7_pkg.adb: Likewise. > > > > -- > > Eric Botcazou
