diff mbox

[10/10] debug-early merge: compiler proper

Message ID 555CF12C.7080300@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez May 20, 2015, 8:40 p.m. UTC 
> How does this version, which has been committed to the debug-early
> branch, look?

One more thing Richi.  I merged trunk into the branch once again, and Go 
broke.  I tracked it down to a temporary that was being created late 
that IMO shouldn't even get debug info.

The fact that it gets created with create_tmp_var_name() in the first 
place is suspect.  The problem is actually the type, which doesn't even 
get passed through rest_of_type* or the debug_hooks->type_decl(). 
However, I see no reason to have these temporary variables even get fed 
to the debugger, so I'm marking them as DECL_IGNORED_P.

If you want I can repost the whole compiler proper patch, but this is a 
small enough change that y'all can just wave through.

I've committed the snippet below to the branch.  Everything else is as 
it was.

Branch retested on x86-64 Linux and has been merged with trunk.

commit 8824b5ecba26cef065e47b34609c72677c3c36fc
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Wed May 20 16:31:14 2015 -0400

     Set DECL_IGNORED_P on temporary arrays created in the switch
     conversion pass.

        fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,

Comments

Jan Hubicka May 20, 2015, 9:01 p.m. UTC | #1 
> 
> commit 8824b5ecba26cef065e47b34609c72677c3c36fc
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Wed May 20 16:31:14 2015 -0400
> 
>     Set DECL_IGNORED_P on temporary arrays created in the switch
>     conversion pass.
> 
> diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
> index 6b68a16..a4bcdba 100644
> --- a/gcc/tree-switch-conversion.c
> +++ b/gcc/tree-switch-conversion.c
> @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree
> arr_index_type,
>        DECL_ARTIFICIAL (decl) = 1;
>        TREE_CONSTANT (decl) = 1;
>        TREE_READONLY (decl) = 1;
> +      DECL_IGNORED_P (decl) = 1;
>        varpool_node::finalize_decl (decl);

This looks obvious enough to me.  Technically speaking the array type constructed
probalby should be TREE_ARTIFICAIL, but probably it does not matter.
If you grep for finalize_decl, there are several other calls:
asan.c:      varpool_node::finalize_decl (var);
asan.c:      varpool_node::finalize_decl (var);
cgraphbuild.c:      varpool_node::finalize_decl (decl);
cgraphunit.c:    - varpool_finalize_decl
cgraphunit.c:   varpool_node::finalize_decl (decl);
cgraphunit.c:varpool_node::finalize_decl (tree decl)
coverage.c:           varpool_node::finalize_decl (var);
coverage.c:  varpool_node::finalize_decl (var);
coverage.c:  varpool_node::finalize_decl (fn_info_ary);
coverage.c:  varpool_node::finalize_decl (gcov_info_var);
omp-low.c:        varpool_node::finalize_decl (t);
omp-low.c:        varpool_node::finalize_decl (t);
omp-low.c:        varpool_node::finalize_decl (decl);
omp-low.c:      varpool_node::finalize_decl (vars_decl);
omp-low.c:      varpool_node::finalize_decl (funcs_decl);
passes.c:           varpool_node::finalize_decl (decl);
tree-chkp.c:  varpool_node::finalize_decl (var);
tree-chkp.c:  varpool_node::finalize_decl (bnd_var);
tree-profile.c:  varpool_node::finalize_decl (ic_void_ptr_var);
tree-profile.c:  varpool_node::finalize_decl (ic_gcov_type_ptr_var);
tree-switch-conversion.c:      varpool_node::finalize_decl (decl);
ubsan.c:  varpool_node::finalize_decl (decl);
ubsan.c:  varpool_node::finalize_decl (var);
ubsan.c:      varpool_node::finalize_decl (array);
varasm.c:  varpool_node::finalize_decl (decl);
varpool.c:   Unlike finalize_decl function is intended to be used
varpool.c:  varpool_node::finalize_decl (decl);

I would say most of them needs similar treatment (I am not 100% sure about OMP
ones that may be user visible)

Honza
> 
>        fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
Aldy Hernandez May 20, 2015, 9:45 p.m. UTC | #2 
On 05/20/2015 05:01 PM, Jan Hubicka wrote:
>>
>> commit 8824b5ecba26cef065e47b34609c72677c3c36fc
>> Author: Aldy Hernandez <aldyh@redhat.com>
>> Date:   Wed May 20 16:31:14 2015 -0400
>>
>>      Set DECL_IGNORED_P on temporary arrays created in the switch
>>      conversion pass.
>>
>> diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
>> index 6b68a16..a4bcdba 100644
>> --- a/gcc/tree-switch-conversion.c
>> +++ b/gcc/tree-switch-conversion.c
>> @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree
>> arr_index_type,
>>         DECL_ARTIFICIAL (decl) = 1;
>>         TREE_CONSTANT (decl) = 1;
>>         TREE_READONLY (decl) = 1;
>> +      DECL_IGNORED_P (decl) = 1;
>>         varpool_node::finalize_decl (decl);
>
> This looks obvious enough to me.  Technically speaking the array type constructed
> probalby should be TREE_ARTIFICAIL, but probably it does not matter.

Yeah, that's what I thought.  I ignored the type because it won't make 
it to the debugging back end if we stop things at the DECL itself.

FWIW, Ada is filled with these temporaries and/or types that should 
really be ignored, and are currently causing grief.

> If you grep for finalize_decl, there are several other calls:
> asan.c:      varpool_node::finalize_decl (var);
> asan.c:      varpool_node::finalize_decl (var);
> cgraphbuild.c:      varpool_node::finalize_decl (decl);
> cgraphunit.c:    - varpool_finalize_decl
> cgraphunit.c:   varpool_node::finalize_decl (decl);
> cgraphunit.c:varpool_node::finalize_decl (tree decl)
> coverage.c:           varpool_node::finalize_decl (var);
> coverage.c:  varpool_node::finalize_decl (var);

Etc etc.

Hmmm, I bet mainline is generating dwarf for all this.  I don't feel 
comfortable touching all this (ok, I'm lazy), but it would seem like 
almost all of these calls would benefit from DECL_IGNORED_P.  Perhaps we 
could add an argument to finalize_decl() and do it in there.

Aldy

> coverage.c:  varpool_node::finalize_decl (fn_info_ary);
> coverage.c:  varpool_node::finalize_decl (gcov_info_var);
> omp-low.c:        varpool_node::finalize_decl (t);
> omp-low.c:        varpool_node::finalize_decl (t);
> omp-low.c:        varpool_node::finalize_decl (decl);
> omp-low.c:      varpool_node::finalize_decl (vars_decl);
> omp-low.c:      varpool_node::finalize_decl (funcs_decl);
> passes.c:           varpool_node::finalize_decl (decl);
> tree-chkp.c:  varpool_node::finalize_decl (var);
> tree-chkp.c:  varpool_node::finalize_decl (bnd_var);
> tree-profile.c:  varpool_node::finalize_decl (ic_void_ptr_var);
> tree-profile.c:  varpool_node::finalize_decl (ic_gcov_type_ptr_var);
> tree-switch-conversion.c:      varpool_node::finalize_decl (decl);
> ubsan.c:  varpool_node::finalize_decl (decl);
> ubsan.c:  varpool_node::finalize_decl (var);
> ubsan.c:      varpool_node::finalize_decl (array);
> varasm.c:  varpool_node::finalize_decl (decl);
> varpool.c:   Unlike finalize_decl function is intended to be used
> varpool.c:  varpool_node::finalize_decl (decl);
>
> I would say most of them needs similar treatment (I am not 100% sure about OMP
> ones that may be user visible)
>
> Honza
>>
>>         fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
Eric Botcazou May 22, 2015, 8:50 a.m. UTC | #3 
> FWIW, Ada is filled with these temporaries and/or types that should
> really be ignored, and are currently causing grief.

It's a little hard to believe that types created in a front-end should be 
marked ignored.  Either they are used by some objects and thus can be needed 
in the debug info, or they aren't and will be discarded by -feliminate-unused-
debug-types.  And those not present in the source should be marked artificial.
Richard Biener May 22, 2015, 11:26 a.m. UTC | #4 
On Wed, May 20, 2015 at 11:45 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 05/20/2015 05:01 PM, Jan Hubicka wrote:
>>>
>>>
>>> commit 8824b5ecba26cef065e47b34609c72677c3c36fc
>>> Author: Aldy Hernandez <aldyh@redhat.com>
>>> Date:   Wed May 20 16:31:14 2015 -0400
>>>
>>>      Set DECL_IGNORED_P on temporary arrays created in the switch
>>>      conversion pass.
>>>
>>> diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
>>> index 6b68a16..a4bcdba 100644
>>> --- a/gcc/tree-switch-conversion.c
>>> +++ b/gcc/tree-switch-conversion.c
>>> @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree
>>> arr_index_type,
>>>         DECL_ARTIFICIAL (decl) = 1;
>>>         TREE_CONSTANT (decl) = 1;
>>>         TREE_READONLY (decl) = 1;
>>> +      DECL_IGNORED_P (decl) = 1;
>>>         varpool_node::finalize_decl (decl);
>>
>>
>> This looks obvious enough to me.  Technically speaking the array type
>> constructed
>> probalby should be TREE_ARTIFICAIL, but probably it does not matter.

Fine to commit to trunk btw.

>
> Yeah, that's what I thought.  I ignored the type because it won't make it to
> the debugging back end if we stop things at the DECL itself.
>
> FWIW, Ada is filled with these temporaries and/or types that should really
> be ignored, and are currently causing grief.
>
>> If you grep for finalize_decl, there are several other calls:
>> asan.c:      varpool_node::finalize_decl (var);
>> asan.c:      varpool_node::finalize_decl (var);
>> cgraphbuild.c:      varpool_node::finalize_decl (decl);
>> cgraphunit.c:    - varpool_finalize_decl
>> cgraphunit.c:   varpool_node::finalize_decl (decl);
>> cgraphunit.c:varpool_node::finalize_decl (tree decl)
>> coverage.c:           varpool_node::finalize_decl (var);
>> coverage.c:  varpool_node::finalize_decl (var);
>
>
> Etc etc.
>
> Hmmm, I bet mainline is generating dwarf for all this.  I don't feel
> comfortable touching all this (ok, I'm lazy), but it would seem like almost
> all of these calls would benefit from DECL_IGNORED_P.  Perhaps we could add
> an argument to finalize_decl() and do it in there.

The only issue are in passes using build_decl directly.  I guess we'd want
a middle-end-ish "create new global static" similar to what we have for
locals (create_tmp_var).  Some of the callers above already set DECL_IGNORED_P
properly.

Richard.

> Aldy
>
>
>> coverage.c:  varpool_node::finalize_decl (fn_info_ary);
>> coverage.c:  varpool_node::finalize_decl (gcov_info_var);
>> omp-low.c:        varpool_node::finalize_decl (t);
>> omp-low.c:        varpool_node::finalize_decl (t);
>> omp-low.c:        varpool_node::finalize_decl (decl);
>> omp-low.c:      varpool_node::finalize_decl (vars_decl);
>> omp-low.c:      varpool_node::finalize_decl (funcs_decl);
>> passes.c:           varpool_node::finalize_decl (decl);
>> tree-chkp.c:  varpool_node::finalize_decl (var);
>> tree-chkp.c:  varpool_node::finalize_decl (bnd_var);
>> tree-profile.c:  varpool_node::finalize_decl (ic_void_ptr_var);
>> tree-profile.c:  varpool_node::finalize_decl (ic_gcov_type_ptr_var);
>> tree-switch-conversion.c:      varpool_node::finalize_decl (decl);
>> ubsan.c:  varpool_node::finalize_decl (decl);
>> ubsan.c:  varpool_node::finalize_decl (var);
>> ubsan.c:      varpool_node::finalize_decl (array);
>> varasm.c:  varpool_node::finalize_decl (decl);
>> varpool.c:   Unlike finalize_decl function is intended to be used
>> varpool.c:  varpool_node::finalize_decl (decl);
>>
>> I would say most of them needs similar treatment (I am not 100% sure about
>> OMP
>> ones that may be user visible)
>>
>> Honza
>>>
>>>
>>>         fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
>
>
Aldy Hernandez May 22, 2015, 1:32 p.m. UTC | #5 
On 05/22/2015 07:26 AM, Richard Biener wrote:
> On Wed, May 20, 2015 at 11:45 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
>> On 05/20/2015 05:01 PM, Jan Hubicka wrote:
>>>>
>>>>
>>>> commit 8824b5ecba26cef065e47b34609c72677c3c36fc
>>>> Author: Aldy Hernandez <aldyh@redhat.com>
>>>> Date:   Wed May 20 16:31:14 2015 -0400
>>>>
>>>>       Set DECL_IGNORED_P on temporary arrays created in the switch
>>>>       conversion pass.
>>>>
>>>> diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
>>>> index 6b68a16..a4bcdba 100644
>>>> --- a/gcc/tree-switch-conversion.c
>>>> +++ b/gcc/tree-switch-conversion.c
>>>> @@ -1097,6 +1097,7 @@ build_one_array (gswitch *swtch, int num, tree
>>>> arr_index_type,
>>>>          DECL_ARTIFICIAL (decl) = 1;
>>>>          TREE_CONSTANT (decl) = 1;
>>>>          TREE_READONLY (decl) = 1;
>>>> +      DECL_IGNORED_P (decl) = 1;
>>>>          varpool_node::finalize_decl (decl);
>>>
>>>
>>> This looks obvious enough to me.  Technically speaking the array type
>>> constructed
>>> probalby should be TREE_ARTIFICAIL, but probably it does not matter.
>
> Fine to commit to trunk btw.

Tested independently on trunk, and committed there.

Thanks.
diff mbox

Patch

diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index 6b68a16..a4bcdba 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1097,6 +1097,7 @@  build_one_array (gswitch *swtch, int num, tree 
arr_index_type,
        DECL_ARTIFICIAL (decl) = 1;
        TREE_CONSTANT (decl) = 1;
        TREE_READONLY (decl) = 1;
+      DECL_IGNORED_P (decl) = 1;
        varpool_node::finalize_decl (decl);