| Message ID | 5429AAD6.7070708@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 29, 2014 at 8:54 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > I'm rearranging some code in Michael's original patch to minimize the > difference with mainline. > > It seems that the check for DECL_STRUCT_FUNCTION (decl)->gimple_df, was > merely a check to see if we had already set the FDE bits for the decl in > question. Sounds more like a check whether the frontend is finished? > I've moved the check inside the original DECL_EXTERNAL check, > thus making it obvious what is being accomplished. > > I also got rid of mainline's gcc_checking_assert of fun. We're > dereferencing it immediately after. That should be enough to trigger an > ICE. > > Also I removed Michael's check for DECL_STRUCT_FUNCTION(decl), since > mainline drops into this codepath regardless, and has/had that > gcc_checking_assert anyhow. > > No regressions. > > Committed to branch. > > Aldy
On 09/30/14 03:23, Richard Biener wrote: > On Mon, Sep 29, 2014 at 8:54 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >> I'm rearranging some code in Michael's original patch to minimize the >> difference with mainline. >> >> It seems that the check for DECL_STRUCT_FUNCTION (decl)->gimple_df, was >> merely a check to see if we had already set the FDE bits for the decl in >> question. > > Sounds more like a check whether the frontend is finished? Is that the canonical way for checking the FE is finished? Seems kinda odd. I'd prefer to check for ->fde, since this is the actual reason the rest of dwarf generation will not work in this case. Either way, I'm not terribly attached to this particular part of the patch. If you'd rather me use ->gimple_df, I can use it. It just doesn't seem very readable. Aldy
On Tue, Sep 30, 2014 at 5:06 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > On 09/30/14 03:23, Richard Biener wrote: >> >> On Mon, Sep 29, 2014 at 8:54 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >>> >>> I'm rearranging some code in Michael's original patch to minimize the >>> difference with mainline. >>> >>> It seems that the check for DECL_STRUCT_FUNCTION (decl)->gimple_df, was >>> merely a check to see if we had already set the FDE bits for the decl in >>> question. >> >> >> Sounds more like a check whether the frontend is finished? > > > Is that the canonical way for checking the FE is finished? Seems kinda odd. > I'd prefer to check for ->fde, since this is the actual reason the rest of > dwarf generation will not work in this case. > > Either way, I'm not terribly attached to this particular part of the patch. > If you'd rather me use ->gimple_df, I can use it. It just doesn't seem very > readable. No, checking ->gimple_df would be odd indeed. The check seems to be coming from Michas patch-set? Richard. > Aldy
On 10/01/14 07:04, Richard Biener wrote: > No, checking ->gimple_df would be odd indeed. The check seems to be coming from > Michas patch-set? Correct.
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 41c4feb..c92101f 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -18441,9 +18441,7 @@ gen_subprogram_die (tree decl, dw_die_ref context_die) equate_decl_number_to_die (decl, subr_die); } - else if (!DECL_EXTERNAL (decl) - && (!DECL_STRUCT_FUNCTION (decl) - || DECL_STRUCT_FUNCTION (decl)->gimple_df)) + else if (!DECL_EXTERNAL (decl)) { HOST_WIDE_INT cfa_fb_offset; @@ -18452,7 +18450,9 @@ gen_subprogram_die (tree decl, dw_die_ref context_die) if (!old_die || !get_AT (old_die, DW_AT_inline)) equate_decl_number_to_die (decl, subr_die); - gcc_checking_assert (fun); + if (!fun->fde) + goto no_fde_continue; + if (!flag_reorder_blocks_and_partition) { dw_fde_ref fde = fun->fde; @@ -18608,11 +18608,8 @@ gen_subprogram_die (tree decl, dw_die_ref context_die) if (fun->static_chain_decl) add_AT_location_description (subr_die, DW_AT_static_link, loc_list_from_tree (fun->static_chain_decl, 2)); - } - else if (!DECL_EXTERNAL (decl)) - { - if (!old_die || !get_AT (old_die, DW_AT_inline)) - equate_decl_number_to_die (decl, subr_die); + no_fde_continue: + ; } /* Generate child dies for template paramaters. */
I'm rearranging some code in Michael's original patch to minimize the difference with mainline. It seems that the check for DECL_STRUCT_FUNCTION (decl)->gimple_df, was merely a check to see if we had already set the FDE bits for the decl in question. I've moved the check inside the original DECL_EXTERNAL check, thus making it obvious what is being accomplished. I also got rid of mainline's gcc_checking_assert of fun. We're dereferencing it immediately after. That should be enough to trigger an ICE. Also I removed Michael's check for DECL_STRUCT_FUNCTION(decl), since mainline drops into this codepath regardless, and has/had that gcc_checking_assert anyhow. No regressions. Committed to branch. Aldy commit a23ae1821d5c45b2c56ea5db6940ea8982f8fd69 Author: Aldy Hernandez <aldyh@redhat.com> Date: Mon Sep 29 11:48:41 2014 -0700 * dwarf2out.c (gen_subprogram_die): Do not check DECL_STRUCT_FUNCTION, thus leaving the check as mainline. Test for fun>-fde instead of fun->gimple_df.