[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 Martin Liška changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #6 from Martin Liška --- Fixed.
[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 --- Comment #5 from Martin Liška --- Author: marxin Date: Tue Nov 13 15:06:54 2018 New Revision: 266074 URL: https://gcc.gnu.org/viewcvs?rev=266074=gcc=rev Log: Improve -fprofile-report. 2018-11-13 Martin Liska PR tree-optimization/87885 * cfghooks.c (account_profile_record): Rename to ... (profile_record_check_consistency): ... this. Calculate missing num_mismatched_freq_in. (profile_record_account_profile): New function that calculates time and size of a function. * cfghooks.h (struct profile_record): Remove all tuples. (struct cfg_hooks): Remove after_pass flag. (account_profile_record): Rename to ... (profile_record_check_consistency): ... this. (profile_record_account_profile): New. * cfgrtl.c (rtl_account_profile_record): Remove after_pass flag. * passes.c (check_profile_consistency): Do only checking. (account_profile): Calculate size and time of function only. (pass_manager::dump_profile_report): Reformat output. (execute_one_ipa_transform_pass): Call consistency check before clean upand call account_profile after a clean up is done. (execute_one_pass): Call check_profile_consistency and account_profile instead of using after_pass flag.. * tree-cfg.c (gimple_account_profile_record): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/cfghooks.c trunk/gcc/cfghooks.h trunk/gcc/cfgrtl.c trunk/gcc/passes.c trunk/gcc/tree-cfg.c
[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 Martin Liška changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Target Milestone|--- |9.0 --- Comment #4 from Martin Liška --- Let me finish that based on what was discussed on IRC: dead BBs can have bogus IL, for example propagators do not bother to update IL in such blocks but release SSA names that are otherwise propagated out but actual BB removal is left to CFG cleanup because that knows how to properly do that hmm, that is not very pretty but I guess we could just drop the middle value. one can modify the code to account only reachable BBs but that would give simliar values as doing accounting after cfgcleanup. so that would mean moving both calls after TODO? just completely dropping the first call and the first accounted number would be OK I guess if it can't be computed safely, that is. but the docs say theres two phases - so it isn't really two phases?! we can also elide the first call if TODO_cfg_cleanup but then there isn't any cleanup and the first call is pointless why does it use estimate_num_insns rather than just counting stmts? well, the statistics was originally intended to help judging if, say, re-running fre for third time is useful it's about profile after all? because you can do FDO build and then you get number of much the code has similified after every pass OK, but then that's the after-TODO value that is interesting estimate_num_insns is used to estimate runtime of the program it also accounts profile mismatches. The two runs was really done to have idea how much the pass itself mangled the profile and how much the profile was beaten during the cfg cleanups the pass invoked having only the second value is OK if calculating first meaningfully is a trouble. so we can look at the profile only in the first pass and the size/time in the second? yes, that would work, too. anyway, I leave the PR to somebody else ;) I can look into it next week. works for me I wanted to discuss a bit the pointers. So my understanding is that C++ has rvalue and normal referneces.
[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 --- Comment #3 from Jan Hubicka --- OK, I now recall. The intend was really to have three values - profile before pass was run (which you can see from stats of previous pass) - profile after pass was run - profile after cleanups This is somewhat useful because, say for CCP one can see how much code sped up just by removing some calculation and how much it was affected by subsequent unreacable code removal. If we can't calculate the middle value safely, we can just drop it. Honza
[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 --- Comment #2 from Jan Hubicka --- The patch makes sense to me. I am not sure why it was run after pass but before cleanups originally... Seems like a bug.
[Bug tree-optimization/87885] ICE in release_ssa_name_fn with -fprofile-report
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87885 Richard Biener changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2018-11-06 CC||hubicka at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Richard Biener --- Well, obviously the profile-consistency checking requires up-to-date IL but that cannot be ensured before TODO of a pass was run. So I'm not sure why it is run twice? if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) check_profile_consistency (pass->static_pass_number, 0, true); /* Run post-pass cleanup and verification. */ execute_todo (todo_after | pass->todo_flags_finish | TODO_verify_il); if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) check_profile_consistency (pass->static_pass_number, 1, true); docs say /* Do profile consistency book-keeping for the pass with static number INDEX. If SUBPASS is zero, we run _before_ the pass, and if SUBPASS is one, then we run _after_ the pass. RUN is true if the pass really runs, or FALSE but the first call above isn't before the pass, it is before the TODO of the pass?! That is, the following fixes the ICE and placement according to docs. Honza? diff --git a/gcc/passes.c b/gcc/passes.c index d838d909941..4d600eeb7b9 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -2163,15 +2163,15 @@ execute_one_ipa_transform_pass (struct cgraph_node *node, if (pass->tv_id != TV_NONE) timevar_push (pass->tv_id); + if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) +check_profile_consistency (pass->static_pass_number, 0, true); + /* Run pre-pass verification. */ execute_todo (ipa_pass->function_transform_todo_flags_start); /* Do it! */ todo_after = ipa_pass->function_transform (node); - if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) -check_profile_consistency (pass->static_pass_number, 0, true); - /* Run post-pass cleanup and verification. */ execute_todo (todo_after); verify_interpass_invariants (); @@ -2417,6 +2417,9 @@ execute_one_pass (opt_pass *pass) if (pass->tv_id != TV_NONE) timevar_push (pass->tv_id); + if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) +check_profile_consistency (pass->static_pass_number, 0, true); + /* Run pre-pass verification. */ execute_todo (pass->todo_flags_start); @@ -2461,9 +2464,6 @@ execute_one_pass (opt_pass *pass) do_per_function (update_properties_after_pass, pass); - if (profile_report && cfun && (cfun->curr_properties & PROP_cfg)) -check_profile_consistency (pass->static_pass_number, 0, true); - /* Run post-pass cleanup and verification. */ execute_todo (todo_after | pass->todo_flags_finish | TODO_verify_il); if (profile_report && cfun && (cfun->curr_properties & PROP_cfg))
