diff mbox

[221/236] Add insn method to rtx_expr_list

Message ID 1407345815-14551-222-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Aug. 6, 2014, 5:23 p.m. UTC
gcc/
	* rtl.h (rtx_expr_list::insn): New method.
---
 gcc/rtl.h | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Richard Henderson Aug. 19, 2014, 8:41 p.m. UTC | #1
On 08/06/2014 10:23 AM, David Malcolm wrote:
> gcc/
> 	* rtl.h (rtx_expr_list::insn): New method.
> ---
>  gcc/rtl.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index d028be1..d5811c2 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -414,6 +414,10 @@ public:
>  
>    /* Get at the underlying rtx.  */
>    rtx element () const;
> +
> +  /* Get at the rtx, casting to rtx_insn *.  */
> +  rtx_insn *insn () const;
> +
>  };
>  
>  template <>
> @@ -1287,6 +1291,11 @@ inline rtx rtx_expr_list::element () const
>    return XEXP (this, 0);
>  }
>  
> +inline rtx_insn *rtx_expr_list::insn () const
> +{
> +  return as_a <rtx_insn *> (XEXP (this, 0));
> +}
> +

Even with the current code base we aren't *supposed* to be putting insns into
an EXPR_LIST -- that's what INSN_LIST is for.  Note the horribleness with which
anything doing this will have in the rtl dumps.

Can we please fix these uses instead of adding this accessor?


r~
Jeff Law Aug. 25, 2014, 2:22 p.m. UTC | #2
On 08/19/14 14:41, Richard Henderson wrote:
> On 08/06/2014 10:23 AM, David Malcolm wrote:
>> gcc/
>> 	* rtl.h (rtx_expr_list::insn): New method.
>> ---
>>   gcc/rtl.h | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/gcc/rtl.h b/gcc/rtl.h
>> index d028be1..d5811c2 100644
>> --- a/gcc/rtl.h
>> +++ b/gcc/rtl.h
>> @@ -414,6 +414,10 @@ public:
>>
>>     /* Get at the underlying rtx.  */
>>     rtx element () const;
>> +
>> +  /* Get at the rtx, casting to rtx_insn *.  */
>> +  rtx_insn *insn () const;
>> +
>>   };
>>
>>   template <>
>> @@ -1287,6 +1291,11 @@ inline rtx rtx_expr_list::element () const
>>     return XEXP (this, 0);
>>   }
>>
>> +inline rtx_insn *rtx_expr_list::insn () const
>> +{
>> +  return as_a <rtx_insn *> (XEXP (this, 0));
>> +}
>> +
>
> Even with the current code base we aren't *supposed* to be putting insns into
> an EXPR_LIST -- that's what INSN_LIST is for.  Note the horribleness with which
> anything doing this will have in the rtl dumps.
>
> Can we please fix these uses instead of adding this accessor?
I'd be OK with that as a follow-up.

jeff
David Malcolm Aug. 26, 2014, 4 p.m. UTC | #3
On Mon, 2014-08-25 at 08:22 -0600, Jeff Law wrote:
On 08/19/14 14:41, Richard Henderson wrote:
> > On 08/06/2014 10:23 AM, David Malcolm wrote:
> >> gcc/
> >>      * rtl.h (rtx_expr_list::insn): New method.
> >> ---
> >>   gcc/rtl.h | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >>
> >> diff --git a/gcc/rtl.h b/gcc/rtl.h
> >> index d028be1..d5811c2 100644
> >> --- a/gcc/rtl.h
> >> +++ b/gcc/rtl.h
> >> @@ -414,6 +414,10 @@ public:
> >>
> >>     /* Get at the underlying rtx.  */
> >>     rtx element () const;
> >> +
> >> +  /* Get at the rtx, casting to rtx_insn *.  */
> >> +  rtx_insn *insn () const;
> >> +
> >>   };
> >>
> >>   template <>
> >> @@ -1287,6 +1291,11 @@ inline rtx rtx_expr_list::element () const
> >>     return XEXP (this, 0);
> >>   }
> >>
> >> +inline rtx_insn *rtx_expr_list::insn () const
> >> +{
> >> +  return as_a <rtx_insn *> (XEXP (this, 0));
> >> +}
> >> +
> >
> > Even with the current code base we aren't *supposed* to be putting insns into
> > an EXPR_LIST -- that's what INSN_LIST is for.  Note the horribleness with which
> > anything doing this will have in the rtl dumps.
> >
> > Can we please fix these uses instead of adding this accessor?
> I'd be OK with that as a follow-up.
> 
> jeff

It turned out there were two places in the tree where I was using the
unloved rtx_expr_list::insn method:
  * nonlocal_goto_handler_labels
  * forced_labels.

These are both currently EXPR_LIST, and both of them are set up by the
patch series leading up to #221 to be rtx_expr_list.  However, given that
they contain CODE_LABELs and are handled as insns, presumably they should
be INSN_LIST, rather than EXPR_LIST.

So I had a go at cleaning this up.

The first two patches take the place of patch #221, by converting them
from EXPR_LIST to INSN_LIST, updating the relevant vars from
rtx_expr_list * to rtx_insn_list *.

The third patch is a rewrite of patch #222, using the new types.

I've successfully bootstrapped each progressively on top of trunk+the
relevant patches of the patch series (trunk is currently at #171, so
thats #172-#220).

OK for trunk?

David Malcolm (3):
  Convert nonlocal_goto_handler_labels from an EXPR_LIST to an INSN_LIST
  Convert forced_labels from an EXPR_LIST to an INSN_LIST
  Use rtx_insn in more places in dwarf2cfi.c

 gcc/builtins.c  |  2 +-
 gcc/cfgbuild.c  |  8 ++++----
 gcc/cfgrtl.c    |  6 +++---
 gcc/dwarf2cfi.c | 33 +++++++++++++++++----------------
 gcc/except.c    |  2 +-
 gcc/function.h  |  6 +++---
 gcc/jump.c      |  6 +++---
 gcc/reload1.c   | 12 ++++++------
 gcc/rtl.h       |  1 +
 gcc/rtlanal.c   | 29 +++++++++++++++++++++++++++++
 gcc/stmt.c      | 10 +++++-----
 11 files changed, 73 insertions(+), 42 deletions(-)
Richard Henderson Aug. 26, 2014, 4:25 p.m. UTC | #4
On 08/26/2014 09:00 AM, David Malcolm wrote:
> OK for trunk?
> 
> David Malcolm (3):
>   Convert nonlocal_goto_handler_labels from an EXPR_LIST to an INSN_LIST
>   Convert forced_labels from an EXPR_LIST to an INSN_LIST
>   Use rtx_insn in more places in dwarf2cfi.c

Ok to all.  Thanks.


r~
diff mbox

Patch

diff --git a/gcc/rtl.h b/gcc/rtl.h
index d028be1..d5811c2 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -414,6 +414,10 @@  public:
 
   /* Get at the underlying rtx.  */
   rtx element () const;
+
+  /* Get at the rtx, casting to rtx_insn *.  */
+  rtx_insn *insn () const;
+
 };
 
 template <>
@@ -1287,6 +1291,11 @@  inline rtx rtx_expr_list::element () const
   return XEXP (this, 0);
 }
 
+inline rtx_insn *rtx_expr_list::insn () const
+{
+  return as_a <rtx_insn *> (XEXP (this, 0));
+}
+
 /* Methods of rtx_insn_list.  */
 
 inline rtx_insn_list *rtx_insn_list::next () const