Bootstrapping pristine trunk with BOOT_CFLAGS='-O2 -g -fno-ipa-sra' leads to comparison failure because gcc/tree-ssa-loop-ivopts.o differs. Just switching IPA-SRA on (by a patch) when compiling that file resolves the problem. The failure has been introduced with revision 192848: 2012-10-26 Martin Jambor <mjambor@suse.cz> PR debug/54971 * tree-sra.c (struct access): New flag grp_to_be_debug_replaced. (dump_access): Dump the new flag. (analyze_access_subtree): Set the new flag when appropriate. (create_access_replacement): Handle debug replacements differently. (generate_subtree_copies): Handle the grp_to_be_debug_replaced flag. (init_subtree_with_zero): Likewise. (sra_modify_expr): Likewise. (load_assign_lhs_subreplacements): Likewise. (sra_modify_assign): Likewise. So I'll assign it to myself.
Sigh.
Is it a compare-debug failure?
Yes, compiling the pre-processed source with -c -O2 -fno-ipa-sra -fcompare-debug -fno-exceptions fails too. Time for some debug counters.
With the debug counters I have identified the first function where the difference in behavior of intra-SRA behavior with -g vs -g0 caused the comparison failure but could not spot any differences in non-debug statements. Adding -fno-inline makes the comparison failure go away (-fno-early-inlining does not help) so at this point I suppose it's tree-inline that generates different stuff when there are references to SSA names from debug statements.
Created attachment 29469 [details] Reduced testcase Testcase reduced by multidelta that fails with -c -O2 -fno-ipa-sra -fno-exceptions -fcompare-debug -no-inlining or -fdbg-cnt=tree_sra:7 makes the failure go away.
More reduced testcase for -O2 -fno-ipa-sra -fcompare-debug: struct comp_cost { int cost; unsigned complexity; }; struct cost_pair { struct iv_cand *cand; }; struct iv_use { unsigned n_map_members; cost_pair *cost_map; }; struct iv_cand { unsigned id; }; void bar (comp_cost, comp_cost) { } void foo (iv_use *use, iv_cand *cand) { unsigned i, s = cand->id & (use->n_map_members - 1); for (i = 0; i < s; i++) if (use->cost_map[i].cand) goto found; found: use->cost_map[i].cand = cand; comp_cost elim_cost, express_cost, bound_cost; bar (elim_cost, express_cost); }
Created attachment 29497 [details] Patch to fix at least part of the problem This patch fixes Jakub's testcase and the one produced by multidelta too. It adds a new parameter to get_access_replacement alogn the lines of the patch in comment #7 of PR 54971 and makes other provisions as discussed yesterday on IRC. The patch passes bootstrap and testsuite on x86_64-linux, but on its own it does not fix the original problem, i.e. bootstrap with IPA-SRA disabled still fails due to a miscomparison. So I'll defer submitting it until after I figure out what problems are still outstanding and whether we want to deal with them in a single patch or not.
Created attachment 29528 [details] A simpler patch to fix issue in comment 7 The patch from comment #8 had problems of its own, mainly it does not take care of the elaborate fancy names that create_access_replacement creates for the user-created aggregates. I think that the "SR" names that it leaves in the artificial aggregate replacements do not have a high enough value to justify another parameter of get_access_replacement and storing or late-creating fancy names and so on, so if we do not want to create replacements upfront, I'd actually prefer this much simpler patch. However, if we go down this route of creating replacements lazily, that will mean that replacement declarations may be created in different order in -g and -g0 runs (exactly in the cases when patch from comment #8 had to upgrade a debug-only replacement to a real one), which, if I understand it well, may lead to different ordering of decl UIDs which in turn may eventually lead to further code generation differences...
Created attachment 29529 [details] Another unrelated issue Irrespective of ho we decide to deal with the first issue, there is another problem that hinders bootstrap with IPA-SRA disabled of compilation of tree-ssa-loop-ivopts.c with -fno-ipa-sra and -fcompare-debug. I have reduced the issue with multidelta to the attached pre-processed form that fails with O2 -fno-ipa-sra -fcompare-debug and started failing with revision 195015: 2013-01-08 Martin Jambor <mjambor@suse.cz> PR debug/55579 * tree-sra.c (analyze_access_subtree): Return true also after potentially creating a debug-only replacement. As far I can see, dumps begin to diverge in the release_ssa early pass where, in the -g case, a control_var SSA names gets a version number 28 instead of 27 - that is also the problematic difference in the final dump. So far I have not figured out what is the one extra live SSA_NAME that lives in the debug case nor I have observed anything apparently wrong in the behavior of early SRA.
OK, patch http://gcc.gnu.org/ml/gcc-patches/2013-02/msg01233.html fixes the testcase from comment #7 and patch http://gcc.gnu.org/ml/gcc-patches/2013-02/msg01235.html should avoid some further (although apparently not that crucial) differences in SRA -g and -g0 output. Unfortunately, they do not fix the debug comparison failure from comment #9. The problematic difference is: --- testcase.gkd 2013-02-27 16:25:54.036565436 +0100 +++ testcase.gk.gkd 2013-02-27 16:25:54.185565083 +0100 @@ -140,3 +140,3 @@ (insn:TI# 0 0 5 (set (reg/f:DI 6 bp [orig:62 D.xxxx ] [62]) - (mem/f:DI (const_int 0 [0]) [ *control_var_27(D)+0 S8 A64])) testcase.ii:166# {*movdi_internal_rex64} + (mem/f:DI (const_int 0 [0]) [ *control_var_28(D)+0 S8 A64])) testcase.ii:166# {*movdi_internal_rex64} (nil)) and appears for the first time in release_ssa pass. It seems to me that (right after early SRA), two SSA names simply have a different order in -g0 and -g compilations: -SSA_NAME 63: elim_cost$complexity_63 - DEF: elim_cost$complexity_63 = PHI <elim_cost$complexity_50(D)(3), elim_cost$complexity_85(5), elim_cost$complexity_85(4)> +SSA_NAME 63: cost$cost_63 + DEF: cost$cost_63 = PHI <cost$cost_80(D)(17), cost$cost_79(18)> -SSA_NAME 71: cost$cost_71 - DEF: cost$cost_71 = PHI <cost$cost_80(D)(17), cost$cost_79(18)> +SSA_NAME 71: elim_cost$complexity_71 + DEF: elim_cost$complexity_71 = PHI <elim_cost$complexity_50(D)(3), elim_cost$complexity_85(5), elim_cost$complexity_85(4)> Also, if I just do not produce DEBUG statements with elim_cost$complexity on the RHS, the problem goes away. However, I do not anything wrong with the statements. Later, just before release_ssa, three SSA names have different order: -SSA_NAME 67: .MEM_67 - DEF: comp ={v} {CLOBBER}; +SSA_NAME 63: cost$cost_63 + DEF: cost$cost_63 = PHI <cost$cost_80(D)(12), elim_cost$cost_72(13)> -SSA_NAME 70: control_var_70(D) - DEF: GIMPLE_NOP +SSA_NAME 67: .MEM_67 + DEF: comp ={v} {CLOBBER}; -SSA_NAME 71: cost$cost_71 - DEF: cost$cost_71 = PHI <cost$cost_80(D)(12), elim_cost$cost_72(13)> +SSA_NAME 70: control_var_70(D) + DEF: GIMPLE_NOP ...and control_var manages to leak to the final dump, causing the failure. I'll return to this bug in a few days, but now I need a break from it, so I'm un-assigning myself.
Author: jamborm Date: Thu Feb 28 12:43:33 2013 New Revision: 196340 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196340 Log: 2013-02-28 Martin Jambor <mjambor@suse.cz> PR tree-optimization/56294 * tree-sra.c (analyze_access_subtree): Create replacement declarations. Adjust dumping. (get_access_replacement): Do not call create_access_replacement. Assert a replacement exists. (get_repl_default_def_ssa_name): Create the replacement declaration itself. testsuite/ * g++.dg/debug/pr56294.C: New test. Added: trunk/gcc/testsuite/g++.dg/debug/pr56294.C Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-sra.c
A guess is that you end up creating SSA names during code transform in different order - which can result from walking a hashtable to do things (which might be in different order when it doesn't have the same number of entries). Note that if you process debug stmts _at all_ (thus end up creating new SSA names because of them) then this will break as well.
No SSA_NAMEs should be created because of debug stmts. SSA_NAME_VERSION must be identical in between -fvar-tracking-assignments and -fno-var-tracking-assignments, DECL_UIDs might differ, but sorted by DECL_UID the decls must be same if you only consider decls seen with -fno-var-tracking-assignments.
(In reply to comment #13) > A guess is that you end up creating SSA names during code transform in > different > order - which can result from walking a hashtable to do things (which might > be in different order when it doesn't have the same number of entries). I have verified in the debugger that SRA does not create any new SSA names on its own, all of them are created by renaming after the pass finishes (i.e. by TODO_update_ssa). > Note that if you process debug stmts _at all_ (thus end up creating new SSA > names because of them) then this will break as well. I'm not sure what you mean. SRA does not process debug stmts but it now creates them. The re-namer is apparently clever enough not to create a PHI node and thus a new SSA name because of debug statements, yet it manages to create them in a different order, probably because it sees uses were there are none without debug statements. Can that be the case? Is creating such uses really a bug?
(In reply to comment #15) > (In reply to comment #13) > > A guess is that you end up creating SSA names during code transform in > > different > > order - which can result from walking a hashtable to do things (which might > > be in different order when it doesn't have the same number of entries). > > I have verified in the debugger that SRA does not create any new SSA > names on its own, all of them are created by renaming after the pass > finishes (i.e. by TODO_update_ssa). > > > Note that if you process debug stmts _at all_ (thus end up creating new SSA > > names because of them) then this will break as well. > > I'm not sure what you mean. SRA does not process debug stmts but it > now creates them. The re-namer is apparently clever enough not to > create a PHI node and thus a new SSA name because of debug statements, > yet it manages to create them in a different order, probably because > it sees uses were there are none without debug statements. Can that > be the case? Is creating such uses really a bug? Awww. ISTR SRA creates new decls that are renamed by the SSA renamer. SSA rewriting creates those SSA names when visiting definition sites (this is why it doesn't create PHI nodes for debug uses ... hopefully). Renaming is performed in DOM order, but DOM order has DOM children "unordered" - can you check if rewrite_update_enter_block visits basic-blocks in different order -g vs. -g0?
Which is the testcase that still fails? The attached and more reduced ones pass for me ...
(In reply to comment #17) > Which is the testcase that still fails? The attached and more reduced ones > pass for me ... The testcase from comment #10 (called "Another unrelated issue") still fails for me with g++ -O2 -fno-ipa-sra -fcompare-debug. (I use trunk from yesterday). (In reply to comment #16) > can you check if rewrite_update_enter_block visits basic-blocks > in different order -g vs. -g0? No, from the dumps it does not seem so (see below). The dump also shows that in the -g1 case, the renamer sees elim_cost$complexity much earlier with debug statements. However, even seeing them only a little bit earlier (BB 17 as opposed to 18) is enough to trigger the problem. Perhaps because there is a to-be SSA name definition at the beginning of BB 18? -g0: Updating SSA: Registering new PHI nodes in block #0 Registering new PHI nodes in block #2 Registering new PHI nodes in block #3 Registering new PHI nodes in block #4 Updating SSA information for statement elim_cost$cost = SR.30; Updating SSA information for statement elim_cost$complexity = SR.31; Updating SSA information for statement _22 = elim_cost$cost; Registering new PHI nodes in block #5 Updating SSA information for statement elim_cost$cost = _24; Registering new PHI nodes in block #6 Updating SSA information for statement cost$cost = elim_cost$cost; Updating SSA information for statement _74 = cost$cost; Registering new PHI nodes in block #7 Registering new PHI nodes in block #8 Registering new PHI nodes in block #9 Registering new PHI nodes in block #10 Registering new PHI nodes in block #11 Registering new PHI nodes in block #12 Registering new PHI nodes in block #13 Registering new PHI nodes in block #14 Registering new PHI nodes in block #15 Registering new PHI nodes in block #16 Updating SSA information for statement _50 = elim_cost$cost; Updating SSA information for statement elim_cost$cost = _51; Registering new PHI nodes in block #17 Registering new PHI nodes in block #18 Updating SSA information for statement cost$cost = elim_cost$cost; Updating SSA information for statement cost$complexity = elim_cost$complexity; Registering new PHI nodes in block #19 Updating SSA information for statement cost$cost = cost$cost; Updating SSA information for statement _78 = cost$cost; Registering new PHI nodes in block #20 Symbols to be put in SSA form { D.2850 D.2851 D.2854 D.2855 D.2856 D.2857 D.2858 D.2859 } Incremental SSA update started at block: 0 Number of blocks in CFG: 21 Number of blocks to update: 20 ( 95%) Affected blocks: 0 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 -g1: Updating SSA: Registering new PHI nodes in block #0 Registering new PHI nodes in block #2 Registering new PHI nodes in block #3 Registering new PHI nodes in block #4 Updating SSA information for statement elim_cost$cost = SR.30; Updating SSA information for statement elim_cost$complexity = SR.31; Updating SSA information for statement _22 = elim_cost$cost; Registering new PHI nodes in block #5 Updating SSA information for statement elim_cost$cost = _24; Registering new PHI nodes in block #6 Updating SSA information for statement cost$cost = elim_cost$cost; Updating SSA information for statement # DEBUG cost$complexity => elim_cost$complexity Updating SSA information for statement _74 = cost$cost; Registering new PHI nodes in block #7 Registering new PHI nodes in block #8 Registering new PHI nodes in block #9 Registering new PHI nodes in block #10 Registering new PHI nodes in block #11 Registering new PHI nodes in block #12 Registering new PHI nodes in block #13 Registering new PHI nodes in block #14 Registering new PHI nodes in block #15 Registering new PHI nodes in block #16 Updating SSA information for statement _50 = elim_cost$cost; Updating SSA information for statement elim_cost$cost = _51; Registering new PHI nodes in block #17 Updating SSA information for statement # DEBUG cost1$cost => elim_cost$cost Updating SSA information for statement # DEBUG cost1$complexity => elim_cost$complexity Registering new PHI nodes in block #18 Updating SSA information for statement cost$cost = elim_cost$cost; Updating SSA information for statement cost$complexity = elim_cost$complexity; Registering new PHI nodes in block #19 Updating SSA information for statement cost$cost = cost$cost; Updating SSA information for statement # DEBUG cost$complexity => cost$complexity Updating SSA information for statement _78 = cost$cost; Registering new PHI nodes in block #20 Symbols to be put in SSA form { D.2850 D.2851 D.2854 D.2855 D.2856 D.2857 D.2858 D.2864 } Incremental SSA update started at block: 0 Number of blocks in CFG: 21 Number of blocks to update: 20 ( 95%) Affected blocks: 0 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
Ok, with some added debug printing I see Updating SSA: creating PHI node in block #6 for elim_cost$cost creating PHI node in block #17 for elim_cost$cost -creating PHI node in block #19 for cost$cost creating PHI node in block #6 for elim_cost$complexity +creating PHI node in block #19 for cost$cost that's obviously a problem (creating a PHI node involves creating a new SSA name). That's because we insert new PHI nodes for to rename symbols via FOR_EACH_VEC_ELT (symbols_to_rename, i, sym) insert_updated_phi_nodes_for (sym, dfs, blocks_to_update, update_flags); but we need to visit that in UID order ... I have a patch.
Author: rguenth Date: Wed Mar 6 11:24:07 2013 New Revision: 196488 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196488 Log: 2013-03-06 Richard Biener <rguenther@suse.de> PR middle-end/56294 * tree-into-ssa.c (insert_phi_nodes_for): Add dumping. (insert_updated_phi_nodes_compare_uids): New function. (update_ssa): Sort symbols_to_rename after UID before traversing it to insert PHI nodes. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-into-ssa.c
Fixed.
Indeed, bootstrap with BOOT_CFLAGS='-O2 -g -fno-ipa-sra' now finally passes. Thanks!