Patchwork Cleanup last_location and update input_location in ipa_prop

login
register
mail settings
Submitter Dehao Chen
Date Nov. 6, 2012, 1:20 a.m.
Message ID <CAO2gOZWMDwx45yWG4YmyyzA=CbStU-YgqNjQq1bY-Lf8dje-yg@mail.gmail.com>
Download mbox | patch
Permalink /patch/197381/
State New
Headers show

Comments

Dehao Chen - Nov. 6, 2012, 1:20 a.m.
Hi,

This is a patch to do some obvious cleanup and setting correct
input_location in ipa_prop (because it invokes gimplification
routines).

Bootstrapped and passed gcc regression tests.

Is it okay for trunk?

Thanks,
Dehao

gcc/ChangeLog:
2010-11-05  Dehao Chen  <dehao@google.com>

        * ipa-prop.c (ipa_modify_call_arguments): Set input_location so that
        gimplification routines can have right location.
        * emit-rtl.c (last_location): Remove unused variable.
Dehao Chen - Nov. 8, 2012, 5:39 p.m.
ping...


On Mon, Nov 5, 2012 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>
> Hi,
>
> This is a patch to do some obvious cleanup and setting correct
> input_location in ipa_prop (because it invokes gimplification
> routines).
>
> Bootstrapped and passed gcc regression tests.
>
> Is it okay for trunk?
>
> Thanks,
> Dehao
>
> gcc/ChangeLog:
> 2010-11-05  Dehao Chen  <dehao@google.com>
>
>         * ipa-prop.c (ipa_modify_call_arguments): Set input_location so that
>         gimplification routines can have right location.
>         * emit-rtl.c (last_location): Remove unused variable.
Richard Guenther - Nov. 26, 2012, 3:28 p.m.
On Thu, Nov 8, 2012 at 6:39 PM, Dehao Chen <dehao@google.com> wrote:
> ping...

The emit-rtl.c hunk is ok.  I'm questioning the ipa-prop.c hunk though - what
looks at input_location (nothing outside of the frontend should, really).

Thanks,
Richard.

>
> On Mon, Nov 5, 2012 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>>
>> Hi,
>>
>> This is a patch to do some obvious cleanup and setting correct
>> input_location in ipa_prop (because it invokes gimplification
>> routines).
>>
>> Bootstrapped and passed gcc regression tests.
>>
>> Is it okay for trunk?
>>
>> Thanks,
>> Dehao
>>
>> gcc/ChangeLog:
>> 2010-11-05  Dehao Chen  <dehao@google.com>
>>
>>         * ipa-prop.c (ipa_modify_call_arguments): Set input_location so that
>>         gimplification routines can have right location.
>>         * emit-rtl.c (last_location): Remove unused variable.
Dehao Chen - Nov. 26, 2012, 3:54 p.m.
On Mon, Nov 26, 2012 at 7:28 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Thu, Nov 8, 2012 at 6:39 PM, Dehao Chen <dehao@google.com> wrote:
>> ping...
>
> The emit-rtl.c hunk is ok.  I'm questioning the ipa-prop.c hunk though - what
> looks at input_location (nothing outside of the frontend should, really).

ipa_modify_call_arguments invokes force_gimple_operand_gsi, which uses
frontend routines to gimplify expr and uses input_location.

Thanks,
Dehao

>
> Thanks,
> Richard.
>
>>
>> On Mon, Nov 5, 2012 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>>>
>>> Hi,
>>>
>>> This is a patch to do some obvious cleanup and setting correct
>>> input_location in ipa_prop (because it invokes gimplification
>>> routines).
>>>
>>> Bootstrapped and passed gcc regression tests.
>>>
>>> Is it okay for trunk?
>>>
>>> Thanks,
>>> Dehao
>>>
>>> gcc/ChangeLog:
>>> 2010-11-05  Dehao Chen  <dehao@google.com>
>>>
>>>         * ipa-prop.c (ipa_modify_call_arguments): Set input_location so that
>>>         gimplification routines can have right location.
>>>         * emit-rtl.c (last_location): Remove unused variable.
Richard Guenther - Nov. 26, 2012, 4:10 p.m.
On Mon, Nov 26, 2012 at 4:54 PM, Dehao Chen <dehao@google.com> wrote:
> On Mon, Nov 26, 2012 at 7:28 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Thu, Nov 8, 2012 at 6:39 PM, Dehao Chen <dehao@google.com> wrote:
>>> ping...
>>
>> The emit-rtl.c hunk is ok.  I'm questioning the ipa-prop.c hunk though - what
>> looks at input_location (nothing outside of the frontend should, really).
>
> ipa_modify_call_arguments invokes force_gimple_operand_gsi, which uses
> frontend routines to gimplify expr and uses input_location.

Can you be more specific?  That's the place that needs fixing - there are a lot
more force_gimple_operand callers.

Richard.

> Thanks,
> Dehao
>
>>
>> Thanks,
>> Richard.
>>
>>>
>>> On Mon, Nov 5, 2012 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This is a patch to do some obvious cleanup and setting correct
>>>> input_location in ipa_prop (because it invokes gimplification
>>>> routines).
>>>>
>>>> Bootstrapped and passed gcc regression tests.
>>>>
>>>> Is it okay for trunk?
>>>>
>>>> Thanks,
>>>> Dehao
>>>>
>>>> gcc/ChangeLog:
>>>> 2010-11-05  Dehao Chen  <dehao@google.com>
>>>>
>>>>         * ipa-prop.c (ipa_modify_call_arguments): Set input_location so that
>>>>         gimplification routines can have right location.
>>>>         * emit-rtl.c (last_location): Remove unused variable.

Patch

Index: gcc/ipa-prop.c
===================================================================
--- gcc/ipa-prop.c	(revision 193174)
+++ gcc/ipa-prop.c	(working copy)
@@ -2826,7 +2826,9 @@  ipa_modify_call_arguments (struct cgraph_edge *cs,
   gimple_stmt_iterator gsi;
   tree callee_decl;
   int i, len;
+  location_t saved_location = input_location;
 
+  input_location = gimple_location (stmt);
   len = VEC_length (ipa_parm_adjustment_t, adjustments);
   vargs = VEC_alloc (tree, heap, len);
   callee_decl = !cs ? gimple_call_fndecl (stmt) : cs->callee->symbol.decl;
@@ -3004,6 +3006,7 @@  ipa_modify_call_arguments (struct cgraph_edge *cs,
   if (cs)
     cgraph_set_call_stmt (cs, new_stmt);
   update_ssa (TODO_update_ssa);
+  input_location = saved_location;
   free_dominance_info (CDI_DOMINATORS);
 }
 
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 193174)
+++ gcc/emit-rtl.c	(working copy)
@@ -5937,7 +5937,7 @@  location_t epilogue_location;
 /* Hold current location information and last location information, so the
    datastructures are built lazily only when some instructions in given
    place are needed.  */
-static location_t curr_location, last_location;
+static location_t curr_location;
 
 /* Allocate insn location datastructure.  */
 void
@@ -5945,7 +5945,6 @@  insn_locations_init (void)
 {
   prologue_location = epilogue_location = 0;
   curr_location = UNKNOWN_LOCATION;
-  last_location = UNKNOWN_LOCATION;
 }
 
 /* At the end of emit stage, clear current location.  */