[debug-early] rearrange some checks in gen_subprogram_die
diff mbox

Message ID 5429AAD6.7070708@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Sept. 29, 2014, 6:54 p.m. UTC
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.

Comments

Richard Biener Sept. 30, 2014, 10:23 a.m. UTC | #1
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
Aldy Hernandez Sept. 30, 2014, 3:06 p.m. UTC | #2
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
Richard Biener Oct. 1, 2014, 2:04 p.m. UTC | #3
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
Aldy Hernandez Oct. 1, 2014, 3:24 p.m. UTC | #4
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.

Patch
diff mbox

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.  */