diff mbox

[0/6] Conversion of gimple types to C++ inheritance (v3)

Message ID 1383601413.5282.62.camel@surprise
State New
Headers show

Commit Message

David Malcolm Nov. 4, 2013, 9:43 p.m. UTC
On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
> On 11/01/2013 06:58 PM, David Malcolm wrote:
> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
> >>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
> >>>>>    static inline void
> >>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
> >>>>>    {
> >>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
> >> The checking you are removing here.
> >>
> >>> What checking?  There ought to be no checking at all in this
> >>> example...  gimple_build_call_vec returns a gimple_call, and
> >>> gimple_call_set_lhs()  doesn't have to check anything because it
> >>> only accepts gimple_call's.. so there is no checking other than the
> >>> usual "does my parameter match" that the compiler has to do...
> >> and want to replace it by checking of the types at compile time.
> >> The problem is that it uglifies the source too much, and, when you
> >> actually don't have a gimple_call but supposedly a base class of it,
> >> I expect you'd do as_a which is not only further uglification, but has
> >> runtime cost also for --enable-checking=release.
> > I can have a look next week at every call to gimple_call_set_lhs in the
> > tree, and see to what extent we know at compile-time that the initial
> > arg is indeed a call (of the ones I quickly grepped just now, most are
> > from gimple_build_call and friends, but one was from a gimple_copy).
> >
> > FWIW I did some performance testing of the is_a/as_a code in the earlier
> > version of the patch, and it didn't have a noticable runtime cost
> > compared to the GIMPLE_CHECK in the existing code:
> > Size of compiler executable:
> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
> > Compile times:
> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
> I actually really dislike as_a<> and is_a<>, and  think code needs to be 
> restructured rather than use them, other than possibly at the very 
> bottom level when we're allocating memory or something like that, or 
> some kind of emergency :-)...   If we require frequent uses of those, 
> I'd be against it, I find them quite ugly.
> 
> Like I said in the other reply, no rush, I don't think any of this 
> follow up is appropriate this late in stage 1.  It would be more of an 
> "interest" examination right now.. at least in my opinion...  I suspect 
> thinks like gimple_assign are more complex cases, but without looking 
> its hard to tell for sure.

I tried converting gimple_call_set_lhs to accept a gimple_call, rather
than a gimple, and excitingly, it was easiest to also convert
cgraph_edge's call_stmt to also be a gimple_call, rather than just a
gimple.

Am attaching a patch (on top of the patch series being discussed) which
adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
no use of is_a).

I'm also attaching a followup patch which eliminates gimple_call_set_lhs
in favor of a method of gimple_statement_call.

Comments

David Malcolm Nov. 4, 2013, 10:03 p.m. UTC | #1
On Mon, 2013-11-04 at 16:43 -0500, David Malcolm wrote:
> On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
> > On 11/01/2013 06:58 PM, David Malcolm wrote:
> > > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
> > >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
> > >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
> > >>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
> > >>>>>    static inline void
> > >>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
> > >>>>>    {
> > >>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
> > >> The checking you are removing here.
> > >>
> > >>> What checking?  There ought to be no checking at all in this
> > >>> example...  gimple_build_call_vec returns a gimple_call, and
> > >>> gimple_call_set_lhs()  doesn't have to check anything because it
> > >>> only accepts gimple_call's.. so there is no checking other than the
> > >>> usual "does my parameter match" that the compiler has to do...
> > >> and want to replace it by checking of the types at compile time.
> > >> The problem is that it uglifies the source too much, and, when you
> > >> actually don't have a gimple_call but supposedly a base class of it,
> > >> I expect you'd do as_a which is not only further uglification, but has
> > >> runtime cost also for --enable-checking=release.
> > > I can have a look next week at every call to gimple_call_set_lhs in the
> > > tree, and see to what extent we know at compile-time that the initial
> > > arg is indeed a call (of the ones I quickly grepped just now, most are
> > > from gimple_build_call and friends, but one was from a gimple_copy).
> > >
> > > FWIW I did some performance testing of the is_a/as_a code in the earlier
> > > version of the patch, and it didn't have a noticable runtime cost
> > > compared to the GIMPLE_CHECK in the existing code:
> > > Size of compiler executable:
> > > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
> > > Compile times:
> > > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
> > I actually really dislike as_a<> and is_a<>, and  think code needs to be 
> > restructured rather than use them, other than possibly at the very 
> > bottom level when we're allocating memory or something like that, or 
> > some kind of emergency :-)...   If we require frequent uses of those, 
> > I'd be against it, I find them quite ugly.
> > 
> > Like I said in the other reply, no rush, I don't think any of this 
> > follow up is appropriate this late in stage 1.  

I wasn't aware that there was a ramp in conservatism within stage1 - I
thought that we had a "large (tested) changes are OK" attitude
throughout all of stage1, and that the switch to conservatism only began
at the transition to stage3.  (and have been frantically attempting to
get my big changes in before November 21st - should I be rethinking
this?)

> It would be more of an 
> > "interest" examination right now.. at least in my opinion...  I suspect 
> > thinks like gimple_assign are more complex cases, but without looking 
> > its hard to tell for sure.
> 
> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> than a gimple, and excitingly, it was easiest to also convert
> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> gimple.
> 
> Am attaching a patch (on top of the patch series being discussed) which
> adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
> no use of is_a).
> 
> I'm also attaching a followup patch which eliminates gimple_call_set_lhs
> in favor of a method of gimple_statement_call.

(I posted the two patches in my prioer email more as a talking point
than for detailed review, sorry if that wasn't clear).

FWIW, my preferred strategy for 4.9 is a compromise: to do the 1st patch
but not the 2nd: i.e. to make the existing gimple_foo_bar accessor
functions be typesafe at compile-time, but without converting them to
C++ methods - I think this gives us compile-time type-safety, whilst
minimizing code-churn and minimizing the more "C++ish" aspects that may
alarm those less happy with C++ in the codebase.

I believe it would be easiest to do them one subclass at-a-time e.g.
convert all gimple_call_* functions to accepting a gimple_call, then
another subclass etc - assuming that this approach is blessed by the
core reviewers.

Dave
Jakub Jelinek Nov. 4, 2013, 10:25 p.m. UTC | #2
On Mon, Nov 04, 2013 at 04:43:33PM -0500, David Malcolm wrote:
> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> than a gimple, and excitingly, it was easiest to also convert
> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> gimple.
> 
> Am attaching a patch (on top of the patch series being discussed) which
> adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
> no use of is_a).

But that was just for gimple_call_set_lhs, which indeed usually is done just
for newly created calls, not for inspecting preexisting IL.  If you do it
for say gimple_call_arg, gimple_call_fndecl and/or even gimple_call_lhs, I'm
afraid suddenly it would be hundreds of ugly dyn_casts/as_a and similar mess
everywhere.  And, calls are still far less common gimple statements than
gimple assign.

	Jakub
Andrew MacLeod Nov. 4, 2013, 10:28 p.m. UTC | #3
On 11/04/2013 05:03 PM, David Malcolm wrote:
> On Mon, 2013-11-04 at 16:43 -0500, David Malcolm wrote:
>> On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
>>> On 11/01/2013 06:58 PM, David Malcolm wrote:
>>>> On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
>>>>> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
>>>>>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
>>>>>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
>>>>>>>>     static inline void
>>>>>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
>>>>>>>>     {
>>>>>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
>>>>> The checking you are removing here.
>>>>>
>>>>>> What checking?  There ought to be no checking at all in this
>>>>>> example...  gimple_build_call_vec returns a gimple_call, and
>>>>>> gimple_call_set_lhs()  doesn't have to check anything because it
>>>>>> only accepts gimple_call's.. so there is no checking other than the
>>>>>> usual "does my parameter match" that the compiler has to do...
>>>>> and want to replace it by checking of the types at compile time.
>>>>> The problem is that it uglifies the source too much, and, when you
>>>>> actually don't have a gimple_call but supposedly a base class of it,
>>>>> I expect you'd do as_a which is not only further uglification, but has
>>>>> runtime cost also for --enable-checking=release.
>>>> I can have a look next week at every call to gimple_call_set_lhs in the
>>>> tree, and see to what extent we know at compile-time that the initial
>>>> arg is indeed a call (of the ones I quickly grepped just now, most are
>>>> from gimple_build_call and friends, but one was from a gimple_copy).
>>>>
>>>> FWIW I did some performance testing of the is_a/as_a code in the earlier
>>>> version of the patch, and it didn't have a noticable runtime cost
>>>> compared to the GIMPLE_CHECK in the existing code:
>>>> Size of compiler executable:
>>>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
>>>> Compile times:
>>>> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
>>> I actually really dislike as_a<> and is_a<>, and  think code needs to be
>>> restructured rather than use them, other than possibly at the very
>>> bottom level when we're allocating memory or something like that, or
>>> some kind of emergency :-)...   If we require frequent uses of those,
>>> I'd be against it, I find them quite ugly.
>>>
>>> Like I said in the other reply, no rush, I don't think any of this
>>> follow up is appropriate this late in stage 1.
> I wasn't aware that there was a ramp in conservatism within stage1 - I
> thought that we had a "large (tested) changes are OK" attitude
> throughout all of stage1, and that the switch to conservatism only began
> at the transition to stage3.  (and have been frantically attempting to
> get my big changes in before November 21st - should I be rethinking
> this?)
You read a lot into things :-)

No, what I mean its too late to get a full change in, and partial 
changes don't seem worthwhile to me.. ie, I don't think there is much 
point in partially converting one or two gimple stmt kinds and leaving 
the others unconverted.  so converting gimple_call_stmt and some of its 
access methods doesnt seem worthwhile to me right now when we could do 
the entire thing during the next stage1 for instance.   There isnt 
enough benefit.    Especially since as the work progresses you may 
discover improvements that make you go back and unwind some of the 
changes you would now commit.   I believe this is a large enough body of 
work that needs some serious implementation before any of it would be 
appropriate for mainline.

That said, the patch which enables this is more self contained, so 
wouldn't be subject to that. Its a matter of whether it has enough merit 
of its own to go in.   Having the first patch in mainline would actually 
allow someone to experiment more easily during the "off season" if they 
wanted to, but wouldn't be mandatory since they could apply it to their 
own branch to work on.

Andrew
Andrew MacLeod Nov. 4, 2013, 10:31 p.m. UTC | #4
On 11/04/2013 05:03 PM, David Malcolm wrote:

> I believe it would be easiest to do them one subclass at-a-time e.g.
> convert all gimple_call_* functions to accepting a gimple_call, then
> another subclass etc - assuming that this approach is blessed by the
> core reviewers.
>
I think the strategy for reworking the gimple stmts needs quite a bit of 
experimenting to determine what would have to be done and what it would 
look like, and to get buy in from more people that the end result is 
indeed acceptable.  There are complex situation, and I dont see how we 
can avoid virtual functions in some cases (at least not easily without 
resorting to ugly as_a/is_a/dyn_cast functions). So I thinkit needs 
research.

Andrew
Andrew MacLeod Nov. 4, 2013, 10:48 p.m. UTC | #5
On 11/04/2013 05:25 PM, Jakub Jelinek wrote:
> On Mon, Nov 04, 2013 at 04:43:33PM -0500, David Malcolm wrote:
>> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
>> than a gimple, and excitingly, it was easiest to also convert
>> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
>> gimple.
>>
>> Am attaching a patch (on top of the patch series being discussed) which
>> adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
>> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
>> no use of is_a).
> But that was just for gimple_call_set_lhs, which indeed usually is done just
> for newly created calls, not for inspecting preexisting IL.  If you do it
> for say gimple_call_arg, gimple_call_fndecl and/or even gimple_call_lhs, I'm
> afraid suddenly it would be hundreds of ugly dyn_casts/as_a and similar mess
> everywhere.  And, calls are still far less common gimple statements than
> gimple assign.
>
> 	
Indeed, we need to look at a much larger swath of code before even 
thinking of committing anything.

Andrew
Richard Biener Nov. 5, 2013, 11:47 a.m. UTC | #6
On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
>> On 11/01/2013 06:58 PM, David Malcolm wrote:
>> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
>> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
>> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
>> >>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
>> >>>>>    static inline void
>> >>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
>> >>>>>    {
>> >>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
>> >> The checking you are removing here.
>> >>
>> >>> What checking?  There ought to be no checking at all in this
>> >>> example...  gimple_build_call_vec returns a gimple_call, and
>> >>> gimple_call_set_lhs()  doesn't have to check anything because it
>> >>> only accepts gimple_call's.. so there is no checking other than the
>> >>> usual "does my parameter match" that the compiler has to do...
>> >> and want to replace it by checking of the types at compile time.
>> >> The problem is that it uglifies the source too much, and, when you
>> >> actually don't have a gimple_call but supposedly a base class of it,
>> >> I expect you'd do as_a which is not only further uglification, but has
>> >> runtime cost also for --enable-checking=release.
>> > I can have a look next week at every call to gimple_call_set_lhs in the
>> > tree, and see to what extent we know at compile-time that the initial
>> > arg is indeed a call (of the ones I quickly grepped just now, most are
>> > from gimple_build_call and friends, but one was from a gimple_copy).
>> >
>> > FWIW I did some performance testing of the is_a/as_a code in the earlier
>> > version of the patch, and it didn't have a noticable runtime cost
>> > compared to the GIMPLE_CHECK in the existing code:
>> > Size of compiler executable:
>> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
>> > Compile times:
>> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
>> I actually really dislike as_a<> and is_a<>, and  think code needs to be
>> restructured rather than use them, other than possibly at the very
>> bottom level when we're allocating memory or something like that, or
>> some kind of emergency :-)...   If we require frequent uses of those,
>> I'd be against it, I find them quite ugly.
>>
>> Like I said in the other reply, no rush, I don't think any of this
>> follow up is appropriate this late in stage 1.  It would be more of an
>> "interest" examination right now.. at least in my opinion...  I suspect
>> thinks like gimple_assign are more complex cases, but without looking
>> its hard to tell for sure.
>
> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> than a gimple, and excitingly, it was easiest to also convert
> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> gimple.

Does that work (using gimple_call * objects) for our garbage collector?
That is, does it know it is looking at a 'gimple'?  It doesn't matter for this
case as all stmts are reachable from struct function as sequence of 'gimple',
but in general?

Richard.

> Am attaching a patch (on top of the patch series being discussed) which
> adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
> no use of is_a).
>
> I'm also attaching a followup patch which eliminates gimple_call_set_lhs
> in favor of a method of gimple_statement_call.
>
David Malcolm Nov. 5, 2013, 12:30 p.m. UTC | #7
On Tue, 2013-11-05 at 12:47 +0100, Richard Biener wrote:
> On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
> >> On 11/01/2013 06:58 PM, David Malcolm wrote:
> >> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
> >> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
> >> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
> >> >>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
> >> >>>>>    static inline void
> >> >>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
> >> >>>>>    {
> >> >>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
> >> >> The checking you are removing here.
> >> >>
> >> >>> What checking?  There ought to be no checking at all in this
> >> >>> example...  gimple_build_call_vec returns a gimple_call, and
> >> >>> gimple_call_set_lhs()  doesn't have to check anything because it
> >> >>> only accepts gimple_call's.. so there is no checking other than the
> >> >>> usual "does my parameter match" that the compiler has to do...
> >> >> and want to replace it by checking of the types at compile time.
> >> >> The problem is that it uglifies the source too much, and, when you
> >> >> actually don't have a gimple_call but supposedly a base class of it,
> >> >> I expect you'd do as_a which is not only further uglification, but has
> >> >> runtime cost also for --enable-checking=release.
> >> > I can have a look next week at every call to gimple_call_set_lhs in the
> >> > tree, and see to what extent we know at compile-time that the initial
> >> > arg is indeed a call (of the ones I quickly grepped just now, most are
> >> > from gimple_build_call and friends, but one was from a gimple_copy).
> >> >
> >> > FWIW I did some performance testing of the is_a/as_a code in the earlier
> >> > version of the patch, and it didn't have a noticable runtime cost
> >> > compared to the GIMPLE_CHECK in the existing code:
> >> > Size of compiler executable:
> >> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
> >> > Compile times:
> >> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
> >> I actually really dislike as_a<> and is_a<>, and  think code needs to be
> >> restructured rather than use them, other than possibly at the very
> >> bottom level when we're allocating memory or something like that, or
> >> some kind of emergency :-)...   If we require frequent uses of those,
> >> I'd be against it, I find them quite ugly.
> >>
> >> Like I said in the other reply, no rush, I don't think any of this
> >> follow up is appropriate this late in stage 1.  It would be more of an
> >> "interest" examination right now.. at least in my opinion...  I suspect
> >> thinks like gimple_assign are more complex cases, but without looking
> >> its hard to tell for sure.
> >
> > I tried converting gimple_call_set_lhs to accept a gimple_call, rather
> > than a gimple, and excitingly, it was easiest to also convert
> > cgraph_edge's call_stmt to also be a gimple_call, rather than just a
> > gimple.
> 
> Does that work (using gimple_call * objects) for our garbage collector?
> That is, does it know it is looking at a 'gimple'?  It doesn't matter for this
> case as all stmts are reachable from struct function as sequence of 'gimple',
> but in general?
Yes (as of r204146, I believe).

For example, the patch converts cgraph_edge's call_stmt to be a
"gimple_call", rather than just a "gimple", but gengtype handles this by
emitting a call to the base class visitor for call_stmt i.e.
gimple_statement_base:

void
gt_ggc_mx_cgraph_edge (void *x_p)
{
  struct cgraph_edge * x = (struct cgraph_edge *)x_p;
  [...snip chain_next/prev handling...]
  [...snip other fields...]
      gt_ggc_m_21gimple_statement_base ((*x).call_stmt);
  [..etc..]
}
Richard Biener Nov. 5, 2013, 12:32 p.m. UTC | #8
On Tue, Nov 5, 2013 at 1:30 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2013-11-05 at 12:47 +0100, Richard Biener wrote:
>> On Mon, Nov 4, 2013 at 10:43 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>> > On Mon, 2013-11-04 at 08:19 -0500, Andrew MacLeod wrote:
>> >> On 11/01/2013 06:58 PM, David Malcolm wrote:
>> >> > On Fri, 2013-11-01 at 22:57 +0100, Jakub Jelinek wrote:
>> >> >> On Fri, Nov 01, 2013 at 05:47:14PM -0400, Andrew MacLeod wrote:
>> >> >>> On 11/01/2013 05:41 PM, Jakub Jelinek wrote:
>> >> >>>> On Fri, Nov 01, 2013 at 05:36:34PM -0400, Andrew MacLeod wrote:
>> >> >>>>>    static inline void
>> >> >>>>> ! gimple_call_set_lhs (gimple gs, tree lhs)
>> >> >>>>>    {
>> >> >>>>> -   GIMPLE_CHECK (gs, GIMPLE_CALL);
>> >> >> The checking you are removing here.
>> >> >>
>> >> >>> What checking?  There ought to be no checking at all in this
>> >> >>> example...  gimple_build_call_vec returns a gimple_call, and
>> >> >>> gimple_call_set_lhs()  doesn't have to check anything because it
>> >> >>> only accepts gimple_call's.. so there is no checking other than the
>> >> >>> usual "does my parameter match" that the compiler has to do...
>> >> >> and want to replace it by checking of the types at compile time.
>> >> >> The problem is that it uglifies the source too much, and, when you
>> >> >> actually don't have a gimple_call but supposedly a base class of it,
>> >> >> I expect you'd do as_a which is not only further uglification, but has
>> >> >> runtime cost also for --enable-checking=release.
>> >> > I can have a look next week at every call to gimple_call_set_lhs in the
>> >> > tree, and see to what extent we know at compile-time that the initial
>> >> > arg is indeed a call (of the ones I quickly grepped just now, most are
>> >> > from gimple_build_call and friends, but one was from a gimple_copy).
>> >> >
>> >> > FWIW I did some performance testing of the is_a/as_a code in the earlier
>> >> > version of the patch, and it didn't have a noticable runtime cost
>> >> > compared to the GIMPLE_CHECK in the existing code:
>> >> > Size of compiler executable:
>> >> > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01920.html
>> >> > Compile times:
>> >> > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00171.html
>> >> I actually really dislike as_a<> and is_a<>, and  think code needs to be
>> >> restructured rather than use them, other than possibly at the very
>> >> bottom level when we're allocating memory or something like that, or
>> >> some kind of emergency :-)...   If we require frequent uses of those,
>> >> I'd be against it, I find them quite ugly.
>> >>
>> >> Like I said in the other reply, no rush, I don't think any of this
>> >> follow up is appropriate this late in stage 1.  It would be more of an
>> >> "interest" examination right now.. at least in my opinion...  I suspect
>> >> thinks like gimple_assign are more complex cases, but without looking
>> >> its hard to tell for sure.
>> >
>> > I tried converting gimple_call_set_lhs to accept a gimple_call, rather
>> > than a gimple, and excitingly, it was easiest to also convert
>> > cgraph_edge's call_stmt to also be a gimple_call, rather than just a
>> > gimple.
>>
>> Does that work (using gimple_call * objects) for our garbage collector?
>> That is, does it know it is looking at a 'gimple'?  It doesn't matter for this
>> case as all stmts are reachable from struct function as sequence of 'gimple',
>> but in general?
> Yes (as of r204146, I believe).
>
> For example, the patch converts cgraph_edge's call_stmt to be a
> "gimple_call", rather than just a "gimple", but gengtype handles this by
> emitting a call to the base class visitor for call_stmt i.e.
> gimple_statement_base:
>
> void
> gt_ggc_mx_cgraph_edge (void *x_p)
> {
>   struct cgraph_edge * x = (struct cgraph_edge *)x_p;
>   [...snip chain_next/prev handling...]
>   [...snip other fields...]
>       gt_ggc_m_21gimple_statement_base ((*x).call_stmt);
>   [..etc..]
> }

Ah, nice to know.

Thanks,
Richard.

>
Jeff Law Nov. 5, 2013, 9 p.m. UTC | #9
On 11/04/13 15:25, Jakub Jelinek wrote:
> On Mon, Nov 04, 2013 at 04:43:33PM -0500, David Malcolm wrote:
>> I tried converting gimple_call_set_lhs to accept a gimple_call, rather
>> than a gimple, and excitingly, it was easiest to also convert
>> cgraph_edge's call_stmt to also be a gimple_call, rather than just a
>> gimple.
>>
>> Am attaching a patch (on top of the patch series being discussed) which
>> adds this compile-time typesafety; bootstrap is in-progress.   IMHO very
>> little use of is-a.h was needed (5 instances of as_a, and 3 of dyn_cast;
>> no use of is_a).
>
> But that was just for gimple_call_set_lhs, which indeed usually is done just
> for newly created calls, not for inspecting preexisting IL.  If you do it
> for say gimple_call_arg, gimple_call_fndecl and/or even gimple_call_lhs, I'm
> afraid suddenly it would be hundreds of ugly dyn_casts/as_a and similar mess
> everywhere.  And, calls are still far less common gimple statements than
> gimple assign.
Understood.  I think one of the questions we need to answer is when a 
conversion of this nature is done (towards static typechecking), how 
often will we know the static type vs how often we're going to have to 
play these is_a/as_a games.

I'd claim that often if we're stuck with needing is_a/as_a, then we've 
got some refactoring to do.

Based on discussions Andrew, David & myself had today, my gut says let's 
try to go foward with the basic conversion based on its merits as they 
stand today, but without taking the step towards static typing at this time.

Jeff
Jeff Law Nov. 5, 2013, 9:09 p.m. UTC | #10
On 11/04/13 15:28, Andrew MacLeod wrote:
>
> That said, the patch which enables this is more self contained, so
> wouldn't be subject to that. Its a matter of whether it has enough merit
> of its own to go in.   Having the first patch in mainline would actually
> allow someone to experiment more easily during the "off season" if they
> wanted to, but wouldn't be mandatory since they could apply it to their
> own branch to work on.
Let's go with this as our "plan".

Jeff
diff mbox

Patch

commit 9a89f418eceb2d11d6eff60cb8c76c11f7cdf77c
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Nov 4 16:01:42 2013 -0500

    FIXME: conversion of gimple_call_set_lhs to a method

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index a9ab226..a12e581 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1587,7 +1587,7 @@  expand_thunk (struct cgraph_node *node, bool output_asm_thunks)
       gimple_call_set_from_thunk (call, true);
       if (restmp)
 	{
-          gimple_call_set_lhs (call, restmp);
+          call->set_lhs (restmp);
 	  gcc_assert (useless_type_conversion_p (TREE_TYPE (restmp),
 						 TREE_TYPE (TREE_TYPE (alias))));
 	}
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b76479c..cd44f3d 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29459,7 +29459,7 @@  add_condition_to_bb (tree function_decl, tree version_decl,
       predicate_decl = TREE_PURPOSE (predicate_chain);
       predicate_arg = TREE_VALUE (predicate_chain);
       call_cond_stmt = gimple_build_call (predicate_decl, 1, predicate_arg);
-      gimple_call_set_lhs (call_cond_stmt, cond_var);
+      call_cond_stmt->set_lhs (cond_var);
 
       gimple_set_block (call_cond_stmt, DECL_INITIAL (function_decl));
       gimple_set_bb (call_cond_stmt, new_bb);
diff --git a/gcc/gimple-low.c b/gcc/gimple-low.c
index b24961b..f7c399e 100644
--- a/gcc/gimple-low.c
+++ b/gcc/gimple-low.c
@@ -158,7 +158,7 @@  lower_function_body (void)
       arg = build_addr (disp_label, current_function_decl);
       t = builtin_decl_implicit (BUILT_IN_SETJMP_DISPATCHER);
       call = gimple_build_call (t, 1, arg);
-      gimple_call_set_lhs (call, disp_var);
+      call->set_lhs (disp_var);
       gsi_insert_after (&i, call, GSI_CONTINUE_LINKING);
 
       /* Build 'goto DISP_VAR;' and insert.  */
diff --git a/gcc/gimple.c b/gcc/gimple.c
index f31c935..a128582 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2252,7 +2252,7 @@  gimple_set_lhs (gimple stmt, tree lhs)
   if (code == GIMPLE_ASSIGN)
     gimple_assign_set_lhs (stmt, lhs);
   else if (code == GIMPLE_CALL)
-    gimple_call_set_lhs (static_cast<gimple_call> (stmt), lhs);
+    ((gimple_call)stmt)->set_lhs (lhs);
   else
     gcc_unreachable ();
 }
@@ -3083,7 +3083,7 @@  gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
     new_stmt = gimple_build_call_vec (gimple_call_fn (stmt), vargs);
   vargs.release ();
   if (gimple_call_lhs (stmt))
-    gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt));
+    new_stmt->set_lhs (gimple_call_lhs (stmt));
 
   gimple_set_vuse (new_stmt, gimple_vuse (stmt));
   gimple_set_vdef (new_stmt, gimple_vdef (stmt));
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 1180d3e..c9df1d9 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -292,6 +292,13 @@  struct GTY((tag("GSS_WITH_MEM_OPS")))
 struct GTY((tag("GSS_CALL")))
   gimple_statement_call : public gimple_statement_with_memory_ops_base
 {
+public:
+
+  /* Set LHS to be the LHS operand of this call statement.  */
+  inline void set_lhs (tree lhs);
+
+public:
+
   /* [ WORD 1-9 ] : base class */
 
   /* [ WORD 10-13 ]  */
@@ -2550,12 +2557,12 @@  gimple_call_lhs_ptr (const_gimple gs)
 
 /* Set LHS to be the LHS operand of call statement GS.  */
 
-static inline void
-gimple_call_set_lhs (gimple_call gs, tree lhs)
+inline void
+gimple_statement_call::set_lhs (tree lhs)
 {
-  gimple_set_op (gs, 0, lhs);
+  gimple_set_op (this, 0, lhs);
   if (lhs && TREE_CODE (lhs) == SSA_NAME)
-    SSA_NAME_DEF_STMT (lhs) = gs;
+    SSA_NAME_DEF_STMT (lhs) = this;
 }
 
 
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index e2c81b9..4cfc2e9 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1166,7 +1166,7 @@  build_stack_save_restore (gimple_call *save, gimple_call *restore)
 
   *save = gimple_build_call (builtin_decl_implicit (BUILT_IN_STACK_SAVE), 0);
   tmp_var = create_tmp_var (ptr_type_node, "saved_stack");
-  gimple_call_set_lhs (*save, tmp_var);
+  (*save)->set_lhs (tmp_var);
 
   *restore
     = gimple_build_call (builtin_decl_implicit (BUILT_IN_STACK_RESTORE),
@@ -3464,7 +3464,7 @@  gimplify_modify_expr_to_memcpy (tree *expr_p, tree size, bool want_value,
     {
       /* tmp = memcpy() */
       t = create_tmp_var (TREE_TYPE (to_ptr), NULL);
-      gimple_call_set_lhs (gs, t);
+      gs->set_lhs (t);
       gimplify_seq_add_stmt (seq_p, gs);
 
       *expr_p = build_simple_mem_ref (t);
@@ -3511,7 +3511,7 @@  gimplify_modify_expr_to_memset (tree *expr_p, tree size, bool want_value,
     {
       /* tmp = memset() */
       t = create_tmp_var (TREE_TYPE (to_ptr), NULL);
-      gimple_call_set_lhs (gs, t);
+      gs->set_lhs (t);
       gimplify_seq_add_stmt (seq_p, gs);
 
       *expr_p = build1 (INDIRECT_REF, TREE_TYPE (to), t);
@@ -4881,7 +4881,7 @@  gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
       gimple_call_set_fntype (assign, TREE_TYPE (fnptrtype));
       notice_special_calls (assign);
       if (!gimple_call_noreturn_p (assign))
-	gimple_call_set_lhs (call, *to_p);
+	call->set_lhs (*to_p);
     }
   else
     {
@@ -7792,7 +7792,7 @@  gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    gimplify_arg (&cond, pre_p, EXPR_LOCATION (*expr_p));
 	    gimple_call call = gimple_build_call_internal (IFN_ANNOTATE, 2,
 							   cond, id);
-	    gimple_call_set_lhs (call, tmp);
+	    call->set_lhs (tmp);
 	    gimplify_seq_add_stmt (pre_p, call);
 	    *expr_p = tmp;
 	    ret = GS_ALL_DONE;
@@ -8995,7 +8995,7 @@  gimplify_function_tree (tree fndecl)
       x = builtin_decl_implicit (BUILT_IN_RETURN_ADDRESS);
       call = gimple_build_call (x, 1, integer_zero_node);
       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
-      gimple_call_set_lhs (call, tmp_var);
+      call->set_lhs (tmp_var);
       gimplify_seq_add_stmt (&cleanup, call);
       x = builtin_decl_implicit (BUILT_IN_PROFILE_FUNC_EXIT);
       call = gimple_build_call (x, 2,
@@ -9007,7 +9007,7 @@  gimplify_function_tree (tree fndecl)
       x = builtin_decl_implicit (BUILT_IN_RETURN_ADDRESS);
       call = gimple_build_call (x, 1, integer_zero_node);
       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
-      gimple_call_set_lhs (call, tmp_var);
+      call->set_lhs (tmp_var);
       gimplify_seq_add_stmt (&body, call);
       x = builtin_decl_implicit (BUILT_IN_PROFILE_FUNC_ENTER);
       call = gimple_build_call (x, 2,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 642153d..5e8b068 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3713,7 +3713,7 @@  ipa_modify_call_arguments (struct cgraph_edge *cs, gimple stmt,
   new_stmt = gimple_build_call_vec (callee_decl, vargs);
   vargs.release ();
   if (gimple_call_lhs (stmt))
-    gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt));
+    new_stmt->set_lhs (gimple_call_lhs (stmt));
 
   gimple_set_block (new_stmt, gimple_block (stmt));
   if (gimple_has_location (stmt))
diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 915533c..b7f8381 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -1414,7 +1414,7 @@  split_function (struct split_point *split_point)
 		}
 	      if (DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
 		{
-		  gimple_call_set_lhs (call, build_simple_mem_ref (retval));
+		  call->set_lhs (build_simple_mem_ref (retval));
 		  gsi_insert_after (&gsi, call, GSI_NEW_STMT);
 		}
 	      else
@@ -1432,7 +1432,7 @@  split_function (struct split_point *split_point)
 		      gsi_insert_after (&gsi, cpy, GSI_NEW_STMT);
 		      retval = tem;
 		    }
-		  gimple_call_set_lhs (call, retval);
+		  call->set_lhs (retval);
 		  update_stmt (call);
 		}
 	    }
@@ -1468,9 +1468,9 @@  split_function (struct split_point *split_point)
 		    retval = make_ssa_name (retval, call);
 		}
 	      if (DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
-	        gimple_call_set_lhs (call, build_simple_mem_ref (retval));
+	        call->set_lhs (build_simple_mem_ref (retval));
 	      else
-	        gimple_call_set_lhs (call, retval);
+	        call->set_lhs (retval);
 	    }
           gsi_insert_after (&gsi, call, GSI_NEW_STMT);
 	  ret = gimple_build_return (retval);
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index dcececc..3b7882d 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2672,7 +2672,7 @@  build_omp_barrier (tree lhs)
 					   : BUILT_IN_GOMP_BARRIER);
   gimple_call g = gimple_build_call (fndecl, 0);
   if (lhs)
-    gimple_call_set_lhs (g, lhs);
+    g->set_lhs (lhs);
   return g;
 }
 
@@ -3113,7 +3113,7 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
 		  stmt = gimple_build_call (atmp, 1, x);
 		  tmp = create_tmp_var_raw (ptr_type_node, NULL);
 		  gimple_add_tmp_var (tmp);
-		  gimple_call_set_lhs (stmt, tmp);
+		  stmt->set_lhs (tmp);
 
 		  gimple_seq_add_stmt (ilist, stmt);
 
@@ -3521,7 +3521,7 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
       TREE_NO_WARNING (uid) = 1;
       gimple_call call
 	= gimple_build_call_internal (IFN_GOMP_SIMD_LANE, 1, uid);
-      gimple_call_set_lhs (call, lane);
+      call->set_lhs (lane);
       gimple_stmt_iterator gsi = gsi_start_1 (gimple_omp_body_ptr (ctx->stmt));
       gsi_insert_before_without_update (&gsi, call, GSI_SAME_STMT);
       c = build_omp_clause (UNKNOWN_LOCATION, OMP_CLAUSE__SIMDUID_);
@@ -3538,7 +3538,7 @@  lower_rec_input_clauses (tree clauses, gimple_seq *ilist, gimple_seq *dlist,
 	  {
 	    tree vf = create_tmp_var (unsigned_type_node, NULL);
 	    call = gimple_build_call_internal (IFN_GOMP_SIMD_VF, 1, uid);
-	    gimple_call_set_lhs (call, vf);
+	    call->set_lhs (vf);
 	    gimple_seq *seq = i == 0 ? ilist : dlist;
 	    gimple_seq_add_stmt (seq, call);
 	    tree t = build_int_cst (unsigned_type_node, 0);
@@ -3698,7 +3698,7 @@  lower_lastprivate_clauses (tree clauses, tree predicate, gimple_seq *stmt_list,
 			= gimple_build_call_internal (IFN_GOMP_SIMD_LAST_LANE,
 						      2, simduid,
 						      TREE_OPERAND (val, 1));
-		      gimple_call_set_lhs (g, lastlane);
+		      g->set_lhs (lastlane);
 		      gimple_seq_add_stmt (stmt_list, g);
 		    }
 		  new_var = build4 (ARRAY_REF, TREE_TYPE (val),
@@ -5547,7 +5547,7 @@  expand_omp_for_generic (struct omp_region *region,
     t = builtin_decl_explicit (BUILT_IN_GOMP_LOOP_END);
   gimple_call call = gimple_build_call (t, 0);
   if (gimple_omp_return_lhs (gsi_stmt (gsi)))
-    gimple_call_set_lhs (call, gimple_omp_return_lhs (gsi_stmt (gsi)));
+    call->set_lhs (gimple_omp_return_lhs (gsi_stmt (gsi)));
   gsi_insert_after (&gsi, call, GSI_SAME_STMT);
   gsi_remove (&gsi, true);
 
@@ -6939,7 +6939,7 @@  expand_omp_sections (struct omp_region *region)
       u = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_NEXT);
       call = gimple_build_call (u, 0);
     }
-  gimple_call_set_lhs (call, vin);
+  call->set_lhs (vin);
   gsi_insert_after (&si, call, GSI_SAME_STMT);
   gsi_remove (&si, true);
 
@@ -7029,7 +7029,7 @@  expand_omp_sections (struct omp_region *region)
 
       bfn_decl = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_NEXT);
       call = gimple_build_call (bfn_decl, 0);
-      gimple_call_set_lhs (call, vnext);
+      call->set_lhs (vnext);
       gsi_insert_after (&si, call, GSI_SAME_STMT);
       gsi_remove (&si, true);
 
@@ -7046,7 +7046,7 @@  expand_omp_sections (struct omp_region *region)
     t = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_END);
   call = gimple_build_call (t, 0);
   if (gimple_omp_return_lhs (gsi_stmt (si)))
-    gimple_call_set_lhs (call, gimple_omp_return_lhs (gsi_stmt (si)));
+    call->set_lhs (gimple_omp_return_lhs (gsi_stmt (si)));
   gsi_insert_after (&si, call, GSI_SAME_STMT);
   gsi_remove (&si, true);
 
@@ -8422,7 +8422,7 @@  lower_omp_single_simple (gimple single_stmt, gimple_seq *pre_p)
   decl = builtin_decl_explicit (BUILT_IN_GOMP_SINGLE_START);
   lhs = create_tmp_var (TREE_TYPE (TREE_TYPE (decl)), NULL);
   call = gimple_build_call (decl, 0);
-  gimple_call_set_lhs (call, lhs);
+  call->set_lhs (lhs);
   gimple_seq_add_stmt (pre_p, call);
 
   cond = gimple_build_cond (EQ_EXPR, lhs,
@@ -10010,7 +10010,7 @@  lower_omp_1 (gimple_stmt_iterator *gsi_p, omp_context *ctx)
 		gimple_call_set_fndecl (stmt, fndecl);
 		gimple_call_set_fntype (stmt, TREE_TYPE (fndecl));
 	      }
-	    gimple_call_set_lhs (call, lhs);
+	    call->set_lhs (lhs);
 	    tree fallthru_label;
 	    fallthru_label = create_artificial_label (UNKNOWN_LOCATION);
 	    gimple g;
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 0358b26..b9e1be6 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -1618,7 +1618,7 @@  lower_transaction (gimple_stmt_iterator *gsi, struct walk_stmt_info *wi)
 	gimple_build_call (builtin_decl_explicit (BUILT_IN_EH_POINTER),
 			   1, integer_zero_node);
       ptr = create_tmp_var (ptr_type_node, NULL);
-      gimple_call_set_lhs (call, ptr);
+      call->set_lhs (ptr);
       gimple_seq_add_stmt (&e_seq, call);
 
       g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TM_COMMIT_EH),
@@ -2115,7 +2115,7 @@  build_tm_load (location_t loc, tree lhs, tree rhs, gimple_stmt_iterator *gsi)
   t = TREE_TYPE (TREE_TYPE (decl));
   if (useless_type_conversion_p (type, t))
     {
-      gimple_call_set_lhs (gcall, lhs);
+      gcall->set_lhs (lhs);
       gsi_insert_before (gsi, gcall, GSI_SAME_STMT);
     }
   else
@@ -2124,7 +2124,7 @@  build_tm_load (location_t loc, tree lhs, tree rhs, gimple_stmt_iterator *gsi)
       tree temp;
 
       temp = create_tmp_reg (t, NULL);
-      gimple_call_set_lhs (gcall, temp);
+      gcall->set_lhs (temp);
       gsi_insert_before (gsi, gcall, GSI_SAME_STMT);
 
       t = fold_build1 (VIEW_CONVERT_EXPR, type, temp);
@@ -2414,7 +2414,7 @@  expand_call_tm (struct tm_region *region,
 	      }
 	}
 
-      gimple_call_set_lhs (call_stmt, tmp);
+      call_stmt->set_lhs (tmp);
       update_stmt (stmt);
       stmt = gimple_build_assign (lhs, tmp);
       gimple_set_location (stmt, loc);
@@ -2701,7 +2701,7 @@  expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
       region->original_transaction_was_outer = true;
     tree t = build_int_cst (tm_state_type, flags);
     gimple_call call = gimple_build_call (tm_start, 1, t);
-    gimple_call_set_lhs (call, tm_state);
+    call->set_lhs (tm_state);
     gimple_set_location (call, gimple_location (region->transaction_stmt));
 
     // Replace the GIMPLE_TRANSACTION with the call to BUILT_IN_TM_START.
@@ -4993,7 +4993,7 @@  ipa_tm_insert_gettmclone_call (struct cgraph_node *node,
 
   g = gimple_build_call (gettm_fn, 1, old_fn);
   ret = make_ssa_name (ret, g);
-  gimple_call_set_lhs (g, ret);
+  g->set_lhs (ret);
 
   gsi_insert_before (gsi, g, GSI_SAME_STMT);
 
@@ -5029,7 +5029,7 @@  ipa_tm_insert_gettmclone_call (struct cgraph_node *node,
       tree temp;
 
       temp = create_tmp_reg (rettype, 0);
-      gimple_call_set_lhs (stmt, temp);
+      stmt->set_lhs (temp);
 
       g2 = gimple_build_assign (lhs,
 				fold_build1 (VIEW_CONVERT_EXPR,
diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index 3cd625c..db43ea8 100644
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -517,7 +517,7 @@  fixup_noreturn_call (gimple_call stmt)
   if (gimple_call_lhs (stmt))
     {
       tree op = gimple_call_lhs (stmt);
-      gimple_call_set_lhs (stmt, NULL_TREE);
+      stmt->set_lhs (NULL_TREE);
 
       /* We need to remove SSA name to avoid checking errors.
 	 All uses are dominated by the noreturn and thus will
diff --git a/gcc/tree-complex.c b/gcc/tree-complex.c
index aee1e26..325554f 100644
--- a/gcc/tree-complex.c
+++ b/gcc/tree-complex.c
@@ -957,7 +957,7 @@  expand_complex_libcall (gimple_stmt_iterator *gsi, tree ar, tree ai,
   fn = builtin_decl_explicit (bcode);
 
   stmt = gimple_build_call (fn, 4, ar, ai, br, bi);
-  gimple_call_set_lhs (stmt, lhs);
+  stmt->set_lhs (lhs);
   update_stmt (stmt);
   gsi_replace (gsi, stmt, false);
 
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 3347b7b..c83229f 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -3207,7 +3207,7 @@  lower_resx (basic_block bb, gimple stmt, struct pointer_map_t *mnt_map)
 	  gimple_call call = gimple_build_call (fn, 1, src_nr);
 	  var = create_tmp_var (ptr_type_node, NULL);
 	  var = make_ssa_name (var, call);
-	  gimple_call_set_lhs (call, var);
+	  call->set_lhs (var);
 	  gsi_insert_before (&gsi, call, GSI_SAME_STMT);
 
 	  fn = builtin_decl_implicit (BUILT_IN_UNWIND_RESUME);
@@ -3583,7 +3583,7 @@  lower_eh_dispatch (basic_block src, gimple stmt)
 						       region_nr));
 	    filter = create_tmp_var (TREE_TYPE (TREE_TYPE (fn)), NULL);
 	    filter = make_ssa_name (filter, call);
-	    gimple_call_set_lhs (call, filter);
+	    call->set_lhs (filter);
 	    gsi_insert_before (&gsi, call, GSI_SAME_STMT);
 
 	    /* Turn the default label into a default case.  */
@@ -3610,7 +3610,7 @@  lower_eh_dispatch (basic_block src, gimple stmt)
 						     region_nr));
 	filter = create_tmp_var (TREE_TYPE (TREE_TYPE (fn)), NULL);
 	filter = make_ssa_name (filter, call);
-	gimple_call_set_lhs (call, filter);
+	call->set_lhs (filter);
 	gsi_insert_before (&gsi, call, GSI_SAME_STMT);
 
 	r->u.allowed.label = NULL;
diff --git a/gcc/tree-emutls.c b/gcc/tree-emutls.c
index 7b17a68..177d0e9 100644
--- a/gcc/tree-emutls.c
+++ b/gcc/tree-emutls.c
@@ -438,7 +438,7 @@  gen_emutls_addr (tree decl, struct lower_emutls_data *d)
       gimple_set_location (x, d->loc);
 
       addr = make_ssa_name (addr, x);
-      gimple_call_set_lhs (x, addr);
+      x->set_lhs (addr);
 
       gimple_seq_add_stmt (&d->seq, x);
 
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 83a3e27..e4c80a7 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -1683,7 +1683,7 @@  copy_bb (copy_body_data *id, basic_block bb, int frequency_scale,
 	      gimple_call_set_va_arg_pack (new_call, false);
 	      gimple_set_location (new_call, gimple_location (stmt));
 	      gimple_set_block (new_call, gimple_block (stmt));
-	      gimple_call_set_lhs (new_call, gimple_call_lhs (stmt));
+	      new_call->set_lhs (gimple_call_lhs (stmt));
 
 	      gsi_replace (&copy_gsi, new_call, false);
 	      stmt = new_call;
diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
index 5f5dcef..0258553 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -406,7 +406,7 @@  init_tmp_var_with_call (struct nesting_info *info, gimple_stmt_iterator *gsi,
   tree t;
 
   t = create_tmp_var_for (info, gimple_call_return_type (call), NULL);
-  gimple_call_set_lhs (call, t);
+  call->set_lhs (t);
   if (! gsi_end_p (*gsi))
     gimple_set_location (call, gimple_location (gsi_stmt (*gsi)));
   gsi_insert_before (gsi, call, GSI_SAME_STMT);
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 35c97b6..3acb721 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -4476,7 +4476,7 @@  replace_removed_params_ssa_names (gimple stmt,
   if (is_gimple_assign (stmt))
     gimple_assign_set_lhs (stmt, name);
   else if (is_gimple_call (stmt))
-    gimple_call_set_lhs (static_cast<gimple_call> (stmt), name);
+    static_cast<gimple_call> (stmt)->set_lhs (name);
   else
     gimple_phi_set_result (stmt, name);
 
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 3527848..4451585 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -1241,7 +1241,7 @@  eliminate_unnecessary_stmts (void)
 		      fprintf (dump_file, "\n");
 		    }
 
-		  gimple_call_set_lhs (call_stmt, NULL_TREE);
+		  call_stmt->set_lhs (NULL_TREE);
 		  maybe_clean_or_replace_eh_stmt (stmt, stmt);
 		  update_stmt (stmt);
 		  release_ssa_name (name);
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index c0415d4..6ad45c9 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -1653,7 +1653,7 @@  simplify_builtin_call (gimple_stmt_iterator *gsi_p, tree callee2)
 		 memset call.  */
 	      gimple_call call_stmt1 = static_cast <gimple_call> (stmt1);
 	      if (lhs1 && DECL_FUNCTION_CODE (callee1) == BUILT_IN_MEMPCPY)
-		gimple_call_set_lhs (call_stmt1, NULL_TREE);
+		call_stmt1->set_lhs (NULL_TREE);
 	      gimple_call_set_arg (stmt1, 1, new_str_cst);
 	      gimple_call_set_arg (stmt1, 2,
 				   build_int_cst (TREE_TYPE (len1), src_len));
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index 6253c3d..2ffa38a 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -771,7 +771,7 @@  execute_cse_sincos_1 (tree name)
     return false;
   call_stmt = gimple_build_call (fndecl, 1, name);
   res = make_temp_ssa_name (TREE_TYPE (TREE_TYPE (fndecl)), call_stmt, "sincostmp");
-  gimple_call_set_lhs (call_stmt, res);
+  call_stmt->set_lhs (res);
 
   def_stmt = SSA_NAME_DEF_STMT (name);
   if (!SSA_NAME_IS_DEFAULT_DEF (name)
@@ -2022,7 +2022,7 @@  execute_optimize_bswap (void)
 	      gsi_insert_after (&gsi, convert_stmt, GSI_SAME_STMT);
 	    }
 
-	  gimple_call_set_lhs (call, bswap_tmp);
+	  call->set_lhs (bswap_tmp);
 
 	  if (dump_file)
 	    {
diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 4565de0..0c84581 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -700,7 +700,7 @@  static void
 finish_update_gimple_call (gimple_stmt_iterator *si_p, gimple_call new_stmt,
 			   gimple stmt)
 {
-  gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt));
+  new_stmt->set_lhs (gimple_call_lhs (stmt));
   move_ssa_defining_stmt_for_defs (new_stmt, stmt);
   gimple_set_vuse (new_stmt, gimple_vuse (stmt));
   gimple_set_vdef (new_stmt, gimple_vdef (stmt));
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 982c6d2..0e66f7c 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -4097,7 +4097,7 @@  attempt_builtin_powi (gimple stmt, vec<operand_entry_t> *ops)
 	      pow_stmt = gimple_build_call (powi_fndecl, 2, rf1->repr, 
 					    build_int_cst (integer_type_node,
 							   power));
-	      gimple_call_set_lhs (pow_stmt, iter_result);
+	      pow_stmt->set_lhs (iter_result);
 	      gimple_set_location (pow_stmt, gimple_location (stmt));
 	      gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
 
@@ -4198,7 +4198,7 @@  attempt_builtin_powi (gimple stmt, vec<operand_entry_t> *ops)
 	  pow_stmt = gimple_build_call (powi_fndecl, 2, rf1->repr, 
 					build_int_cst (integer_type_node,
 						       power));
-	  gimple_call_set_lhs (pow_stmt, iter_result);
+	  pow_stmt->set_lhs (iter_result);
 	  gimple_set_location (pow_stmt, gimple_location (stmt));
 	  gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
 	}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index ad18fc1..f47b20f 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -441,7 +441,7 @@  get_string_length (strinfo si)
 	  tem = unshare_expr (gimple_call_arg (stmt, 0));
 	  lenstmt_call = gimple_build_call (fn, 1, tem);
 	  lhs = make_ssa_name (TREE_TYPE (TREE_TYPE (fn)), lenstmt_call);
-	  gimple_call_set_lhs (lenstmt_call, lhs);
+	  lenstmt_call->set_lhs (lhs);
 	  gimple_set_vuse (lenstmt_call, gimple_vuse (stmt));
 	  gsi_insert_before (&gsi, lenstmt_call, GSI_SAME_STMT);
 	  tem = gimple_call_arg (stmt, 0);
@@ -474,7 +474,7 @@  get_string_length (strinfo si)
 	    }
 	  gimple_call_set_fndecl (stmt, fn);
 	  lhs = make_ssa_name (TREE_TYPE (TREE_TYPE (fn)), stmt);
-	  gimple_call_set_lhs (call_stmt, lhs);
+	  call_stmt->set_lhs (lhs);
 	  update_stmt (stmt);
 	  if (dump_file && (dump_flags & TDF_DETAILS) != 0)
 	    {
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 2ddaebe..7b8d20d 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -4468,7 +4468,7 @@  vect_setup_realignment (gimple stmt, gimple_stmt_iterator *gsi,
 	vect_create_destination_var (scalar_dest,
 				     gimple_call_return_type (call_stmt));
       new_temp = make_ssa_name (vec_dest, call_stmt);
-      gimple_call_set_lhs (call_stmt, new_temp);
+      call_stmt->set_lhs (new_temp);
 
       if (compute_in_loop)
 	gsi_insert_before (gsi, call_stmt, GSI_SAME_STMT);
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 2dc918f..b1a1e5f 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -806,7 +806,7 @@  vect_recog_pow_pattern (vec<gimple> *stmts, tree *type_in,
 	      != NULL_TREE)
 	    {
 	      var = vect_recog_temp_ssa_var (TREE_TYPE (base), stmt);
-	      gimple_call_set_lhs (stmt, var);
+	      stmt->set_lhs (var);
 	      return stmt;
 	    }
 	}
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 906a44d..b982046 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -1943,7 +1943,7 @@  vectorizable_call (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 		  gimple_call call = gimple_build_call_vec (fndecl, vargs);
 		  new_stmt = call;
 		  new_temp = make_ssa_name (vec_dest, new_stmt);
-		  gimple_call_set_lhs (call, new_temp);
+		  call->set_lhs (new_temp);
 		  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 		  SLP_TREE_VEC_STMTS (slp_node).quick_push (new_stmt);
 		}
@@ -1996,7 +1996,7 @@  vectorizable_call (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 	      gimple_call call = gimple_build_call_vec (fndecl, vargs);
 	      new_stmt = call;
 	      new_temp = make_ssa_name (vec_dest, new_stmt);
-	      gimple_call_set_lhs (call, new_temp);
+	      call->set_lhs (new_temp);
 	    }
 	  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 
@@ -2044,7 +2044,7 @@  vectorizable_call (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 		  gimple_call call = gimple_build_call_vec (fndecl, vargs);
 		  new_stmt = call;
 		  new_temp = make_ssa_name (vec_dest, new_stmt);
-		  gimple_call_set_lhs (call, new_temp);
+		  call->set_lhs (new_temp);
 		  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 		  SLP_TREE_VEC_STMTS (slp_node).quick_push (new_stmt);
 		}
@@ -2084,7 +2084,7 @@  vectorizable_call (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 	  gimple_call call = gimple_build_call_vec (fndecl, vargs);
 	  new_stmt = call;
 	  new_temp = make_ssa_name (vec_dest, new_stmt);
-	  gimple_call_set_lhs (call, new_temp);
+	  call->set_lhs (new_temp);
 	  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 
 	  if (j == 0)
@@ -2164,7 +2164,7 @@  vect_gen_widened_results_half (enum tree_code code,
 	call = gimple_build_call (decl, 1, vec_oprnd0);
       new_stmt = call;
       new_temp = make_ssa_name (vec_dest, new_stmt);
-      gimple_call_set_lhs (call, new_temp);
+      call->set_lhs (new_temp);
     }
   else
     {
@@ -2719,7 +2719,7 @@  vectorizable_conversion (gimple stmt, gimple_stmt_iterator *gsi,
 		  gimple_call call = gimple_build_call (decl1, 1, vop0);
 		  new_stmt = call;
 		  new_temp = make_ssa_name (vec_dest, new_stmt);
-		  gimple_call_set_lhs (call, new_temp);
+		  call->set_lhs (new_temp);
 		}
 	      else
 		{
@@ -2831,7 +2831,7 @@  vectorizable_conversion (gimple stmt, gimple_stmt_iterator *gsi,
 		      gimple_call call = gimple_build_call (decl1, 1, vop0);
 		      new_stmt = call;
 		      new_temp = make_ssa_name (vec_dest, new_stmt);
-		      gimple_call_set_lhs (call, new_temp);
+		      call->set_lhs (new_temp);
 		    }
 		  else
 		    {
@@ -2890,7 +2890,7 @@  vectorizable_conversion (gimple stmt, gimple_stmt_iterator *gsi,
 		    gimple_call call = gimple_build_call (decl1, 1, vop0);
 		    new_stmt = call;
 		    new_temp = make_ssa_name (vec_dest, new_stmt);
-		    gimple_call_set_lhs (call, new_temp);
+		    call->set_lhs (new_temp);
 		  }
 		else
 		  {
@@ -4251,7 +4251,7 @@  vectorizable_store (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 	  data_ref = create_array_ref (aggr_type, dataref_ptr, first_dr);
 	  gimple_call call = gimple_build_call_internal (IFN_STORE_LANES, 1, vec_array);
 	  new_stmt = call;
-	  gimple_call_set_lhs (call, data_ref);
+	  call->set_lhs (data_ref);
 	  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 	}
       else
@@ -4752,7 +4752,7 @@  vectorizable_load (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 			  == TYPE_VECTOR_SUBPARTS (rettype));
 	      var = vect_get_new_vect_var (rettype, vect_simple_var, NULL);
 	      op = make_ssa_name (var, new_stmt);
-	      gimple_call_set_lhs (call, op);
+	      call->set_lhs (op);
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
 	      var = make_ssa_name (vec_dest, NULL);
 	      op = build1 (VIEW_CONVERT_EXPR, vectype, op);
@@ -4763,7 +4763,7 @@  vectorizable_load (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 	  else
 	    {
 	      var = make_ssa_name (vec_dest, new_stmt);
-	      gimple_call_set_lhs (call, var);
+	      call->set_lhs (var);
 	    }
 
 	  vect_finish_stmt_generation (stmt, new_stmt, gsi);
@@ -5124,7 +5124,7 @@  vectorizable_load (gimple stmt, gimple_stmt_iterator *gsi, gimple *vec_stmt,
 	  data_ref = create_array_ref (aggr_type, dataref_ptr, first_dr);
 	  gimple_call call = gimple_build_call_internal (IFN_LOAD_LANES, 1, data_ref);
 	  new_stmt = call;
-	  gimple_call_set_lhs (call, vec_array);
+	  call->set_lhs (vec_array);
 	  vect_finish_stmt_generation (stmt, new_stmt, gsi);
 
 	  /* Extract each vector into an SSA_NAME.  */
diff --git a/gcc/tsan.c b/gcc/tsan.c
index de95625..8a8dccf 100644
--- a/gcc/tsan.c
+++ b/gcc/tsan.c
@@ -489,8 +489,7 @@  instrument_builtin_call (gimple_stmt_iterator *gsi)
 		    args[1] = var;
 		  }
 		gimple_call call_stmt = as_a<gimple_statement_call> (stmt);
-		gimple_call_set_lhs (call_stmt,
-				     make_ssa_name (TREE_TYPE (lhs), NULL));
+		call_stmt->set_lhs (make_ssa_name (TREE_TYPE (lhs), NULL));
 		/* BIT_NOT_EXPR stands for NAND.  */
 		if (tsan_atomic_table[i].code == BIT_NOT_EXPR)
 		  {
@@ -572,7 +571,7 @@  instrument_builtin_call (gimple_stmt_iterator *gsi)
 						  args[1],
 						  gimple_assign_lhs (g));
 		gimple_call call_stmt = as_a<gimple_statement_call> (stmt);
-		gimple_call_set_lhs (call_stmt, t);
+		call_stmt->set_lhs (t);
 		update_stmt (stmt);
 		gsi_insert_after (gsi, g, GSI_NEW_STMT);
 	      }
@@ -660,7 +659,7 @@  instrument_func_entry (void)
   builtin_decl = builtin_decl_implicit (BUILT_IN_RETURN_ADDRESS);
   g = gimple_build_call (builtin_decl, 1, integer_zero_node);
   ret_addr = make_ssa_name (ptr_type_node, NULL);
-  gimple_call_set_lhs (g, ret_addr);
+  g->set_lhs (ret_addr);
   gimple_set_location (g, cfun->function_start_locus);
   gsi_insert_before (&gsi, g, GSI_SAME_STMT);
 
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index 7ca9f4d..29d40a1 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -1332,7 +1332,7 @@  gimple_ic (gimple_call icall_stmt, struct cgraph_node *direct_call,
   gimple_call_set_fndecl (dcall_stmt, direct_call->decl);
   dflags = flags_from_decl_or_type (direct_call->decl);
   if ((dflags & ECF_NORETURN) != 0)
-    gimple_call_set_lhs (dcall_stmt, NULL_TREE);
+    dcall_stmt->set_lhs (NULL_TREE);
   gsi_insert_before (&gsi, dcall_stmt, GSI_SAME_STMT);
 
   /* Fix CFG. */
@@ -1397,11 +1397,9 @@  gimple_ic (gimple_call icall_stmt, struct cgraph_node *direct_call,
     {
       tree result = gimple_call_lhs (icall_stmt);
       gimple phi = create_phi_node (result, join_bb);
-      gimple_call_set_lhs (icall_stmt,
-			   duplicate_ssa_name (result, icall_stmt));
+      icall_stmt->set_lhs (duplicate_ssa_name (result, icall_stmt));
       add_phi_arg (phi, gimple_call_lhs (icall_stmt), e_ij, UNKNOWN_LOCATION);
-      gimple_call_set_lhs (dcall_stmt,
-			   duplicate_ssa_name (result, dcall_stmt));
+      dcall_stmt->set_lhs (duplicate_ssa_name (result, dcall_stmt));
       add_phi_arg (phi, gimple_call_lhs (dcall_stmt), e_dj, UNKNOWN_LOCATION);
     }
 
@@ -1637,11 +1635,9 @@  gimple_stringop_fixed_value (gimple_call vcall_stmt, tree icall_size, int prob,
     {
       tree result = gimple_call_lhs (vcall_stmt);
       gimple phi = create_phi_node (result, join_bb);
-      gimple_call_set_lhs (vcall_stmt,
-			   duplicate_ssa_name (result, vcall_stmt));
+      vcall_stmt->set_lhs (duplicate_ssa_name (result, vcall_stmt));
       add_phi_arg (phi, gimple_call_lhs (vcall_stmt), e_vj, UNKNOWN_LOCATION);
-      gimple_call_set_lhs (icall_stmt,
-			   duplicate_ssa_name (result, icall_stmt));
+      icall_stmt->set_lhs (duplicate_ssa_name (result, icall_stmt));
       add_phi_arg (phi, gimple_call_lhs (icall_stmt), e_ij, UNKNOWN_LOCATION);
     }
 
diff --git a/gcc/vtable-verify.c b/gcc/vtable-verify.c
index a276a8d..d743c3e 100644
--- a/gcc/vtable-verify.c
+++ b/gcc/vtable-verify.c
@@ -686,7 +686,7 @@  verify_bb_vtables (basic_block bb)
                      return value, and make the call_stmt use the
                      variable for that purpose.  */
                   tmp0 = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "VTV");
-                  gimple_call_set_lhs (call_stmt, tmp0);
+                  call_stmt->set_lhs (tmp0);
                   update_stmt (call_stmt);
 
                   /* Find the next stmt, after the vptr assignment