Re: [PATCH 1/5] emit-rtl.c: more typesafety

2018-06-13 Thread David Malcolm 
On Tue, 2018-06-12 at 15:23 -0600, Jeff Law wrote:
> On 06/12/2018 03:16 PM, David Malcolm wrote:
> > On Tue, 2018-06-12 at 14:50 -0600, Jeff Law wrote:
> > 
> > Ultimately these boil down to:
> > 
> > gcc_checking_assert (INSN_P (rt)
> >  || NOTE_P (rt)
> >  || JUMP_TABLE_DATA_P (rt)
> >  || BARRIER_P (rt)
> >  || LABEL_P (rt));
> > 
> > (with an "if (rt)" for the safe_a_a variant), and hence they're
> > runtime
> > checks (and only enabled when #if CHECKING_P).
> 
> Ack.
> 
> > 
> > > Obviously your testing ought to be sufficient for the
> > > former.  Mine will help with the latter since it'll build arc,
> > > bfin
> > > and
> > > c6x elf targets through newlib.  For sh it'll build a mixture of
> > > -elf
> > > things through newlib and it'll bootstrap sh4.
> > 
> > Is this testing something that you've already triggered, or does it
> > happen upon commit to trunk?
> 
> THe jobs poll the various relevant git servers (gcc, binutils,
> newlib,
> glibc, linux kernel) once per day .  When a change is detected in any
> of
> the repos a full build is started.
> 
> 
> > 
> > (The sh changes are the only aspect of this kit that I'm keen for
> > review/testing of, as it needed extra casts; I *think* everywhere I
> > added them are places where rtx are already being unconditionally
> > treated as instruction nodes, but your eyes/testing would be
> > helpful).
> 
> They looked reasonable to me.  Thankfully sh is one that we can
> actually
> bootstrap GCC within a chroot using qemu, so it'll get fairly
> extensive
> testing.   It takes ~8hrs, but that's not too bad :-)

Thanks.  For reference, I've committed the combination of the patches
to trunk as r261547.

Dave

(I also smoketested the build of arc, since I noticed some changes
there since my earlier testing).

Re: [PATCH 1/5] emit-rtl.c: more typesafety

2018-06-12 Thread Jeff Law 
On 06/12/2018 03:16 PM, David Malcolm wrote:
> On Tue, 2018-06-12 at 14:50 -0600, Jeff Law wrote:
> 
> Ultimately these boil down to:
> 
> gcc_checking_assert (INSN_P (rt)
>  || NOTE_P (rt)
>  || JUMP_TABLE_DATA_P (rt)
>  || BARRIER_P (rt)
>  || LABEL_P (rt));
> 
> (with an "if (rt)" for the safe_a_a variant), and hence they're runtime
> checks (and only enabled when #if CHECKING_P).
Ack.

> 
>> Obviously your testing ought to be sufficient for the
>> former.  Mine will help with the latter since it'll build arc, bfin
>> and
>> c6x elf targets through newlib.  For sh it'll build a mixture of -elf
>> things through newlib and it'll bootstrap sh4.
> 
> Is this testing something that you've already triggered, or does it
> happen upon commit to trunk?
THe jobs poll the various relevant git servers (gcc, binutils, newlib,
glibc, linux kernel) once per day .  When a change is detected in any of
the repos a full build is started.


> 
> (The sh changes are the only aspect of this kit that I'm keen for
> review/testing of, as it needed extra casts; I *think* everywhere I
> added them are places where rtx are already being unconditionally
> treated as instruction nodes, but your eyes/testing would be helpful).
They looked reasonable to me.  Thankfully sh is one that we can actually
bootstrap GCC within a chroot using qemu, so it'll get fairly extensive
testing.   It takes ~8hrs, but that's not too bad :-)

jeff

Re: [PATCH 1/5] emit-rtl.c: more typesafety

2018-06-12 Thread David Malcolm 
On Tue, 2018-06-12 at 14:50 -0600, Jeff Law wrote:
> On 06/12/2018 12:54 PM, David Malcolm wrote:
> > This patch converts various rtx to rtx_insn * (or rtx_code_label
> > *).
> > It also convert the various "_loc" params from int to location_t
> > 
> > gcc/ChangeLog:
> > * emit-rtl.c (next_real_insn): Strengthen param from "rtx"
> > to "rtx_insn *".
> > (add_insn_after): Likewise for first two params.
> > (add_insn_before): Likewise.
> > (remove_insn): Likewise for param.
> > (emit_pattern_before_noloc): Likewise for second and third
> > params.
> > (emit_jump_insn_before_noloc): Convert NULL_RTX to NULL.
> > (emit_call_insn_before_noloc): Likewise.
> > (emit_debug_insn_before_noloc): Strengthen "before" param from
> > "rtx"
> > to "rtx_insn *".
> > (emit_barrier_before): Likewise.
> > (emit_label_before): Strengthen "label" param from "rtx" to
> > "rtx_code_label *".  Strengthen "before" param from "rtx" to
> > "rtx_insn *".
> > (emit_insn_after_1): Strengthen "after" param from "rtx" to
> > "rtx_insn *".
> > (emit_pattern_after_noloc): Likewise.
> > (emit_insn_after_noloc): Likewise.
> > (emit_jump_insn_after_noloc): Likewise.
> > (emit_call_insn_after_noloc): Likewise.
> > (emit_debug_insn_after_noloc): Likewise.
> > (emit_barrier_after): Likewise.
> > (emit_label_after): Likewise for both params.
> > (emit_pattern_after_setloc): Likewise for "after"
> > param.  Convert
> > "loc" param from "int" to "location_t".
> > (emit_insn_after_setloc): Likewise.
> > (emit_jump_insn_after_setloc): Likewise.
> > (emit_call_insn_after_setloc): Likewise.
> > (emit_debug_insn_after_setloc): Likewise.
> > (emit_pattern_before_setloc): Likewise for "before"
> > param.  Convert
> > "loc" param from "int" to "location_t".
> > (emit_pattern_before): Convert NULL_RTX to NULL.
> > (emit_insn_before_setloc): Convert "loc" param from "int" to
> > "location_t".
> > (emit_jump_insn_before_setloc): Likewise.
> > (emit_call_insn_before_setloc): Likewise.
> > (emit_debug_insn_before_setloc): Strengthen "before" param from
> > rtx to
> > rtx_insn *.  Convert "loc" param from "int" to "location_t".
> > * rtl.h (emit_insn_before_setloc, emit_jump_insn_before_setloc,
> > emit_call_insn_before_setloc, emit_debug_insn_before_setloc):
> > Convert 3rd param from "int" to "location_t".
> > (emit_barrier_before, emit_barrier_after, next_real_insn):
> > Strengthen param from rtx to rtx_insn *.
> > (emit_label_before): Strengthen 1st param from "rtx" to
> > "rtx_code_label *".  Strengthen 2nd param from "rtx" to
> > "rtx_insn *".
> > (emit_insn_after_noloc, emit_jump_insn_after_noloc,
> > emit_call_insn_after_noloc, emit_debug_insn_after_noloc):
> > Strengthen 2nd param from "rtx" to "rtx_insn *".
> > (emit_insn_after_setloc, emit_jump_insn_after_setloc)
> > emit_call_insn_after_setloc, emit_debug_insn_after_setloc):
> > Likewise. Convert 3rd param from "int" to "location_t".
> > (emit_label_after): Strengthen 1st param from "rtx" to
> > "rtx_code_label *".
> > (next_real_insn, remove_insn): Strengthen param from "rtx" to
> > "rtx_insn *".
> > (add_insn_before, add_insn_after): Strengthen 1st and 2nd
> > params
> > from "rtx" to "rtx_insn *".
> 
> OK.  I think that covers the entire set.
> 
> I can't immediately recall if the as-a and safe-as-a are compile or
> runtime checks.  

Ultimately these boil down to:

gcc_checking_assert (INSN_P (rt)
 || NOTE_P (rt)
 || JUMP_TABLE_DATA_P (rt)
 || BARRIER_P (rt)
 || LABEL_P (rt));

(with an "if (rt)" for the safe_a_a variant), and hence they're runtime
checks (and only enabled when #if CHECKING_P).

> Obviously your testing ought to be sufficient for the
> former.  Mine will help with the latter since it'll build arc, bfin
> and
> c6x elf targets through newlib.  For sh it'll build a mixture of -elf
> things through newlib and it'll bootstrap sh4.

Is this testing something that you've already triggered, or does it
happen upon commit to trunk?

(The sh changes are the only aspect of this kit that I'm keen for
review/testing of, as it needed extra casts; I *think* everywhere I
added them are places where rtx are already being unconditionally
treated as instruction nodes, but your eyes/testing would be helpful).

Thanks
Dave

Re: [PATCH 1/5] emit-rtl.c: more typesafety

2018-06-12 Thread Jeff Law 
On 06/12/2018 12:54 PM, David Malcolm wrote:
> This patch converts various rtx to rtx_insn * (or rtx_code_label *).
> It also convert the various "_loc" params from int to location_t
> 
> gcc/ChangeLog:
>   * emit-rtl.c (next_real_insn): Strengthen param from "rtx"
>   to "rtx_insn *".
>   (add_insn_after): Likewise for first two params.
>   (add_insn_before): Likewise.
>   (remove_insn): Likewise for param.
>   (emit_pattern_before_noloc): Likewise for second and third params.
>   (emit_jump_insn_before_noloc): Convert NULL_RTX to NULL.
>   (emit_call_insn_before_noloc): Likewise.
>   (emit_debug_insn_before_noloc): Strengthen "before" param from "rtx"
>   to "rtx_insn *".
>   (emit_barrier_before): Likewise.
>   (emit_label_before): Strengthen "label" param from "rtx" to
>   "rtx_code_label *".  Strengthen "before" param from "rtx" to
>   "rtx_insn *".
>   (emit_insn_after_1): Strengthen "after" param from "rtx" to
>   "rtx_insn *".
>   (emit_pattern_after_noloc): Likewise.
>   (emit_insn_after_noloc): Likewise.
>   (emit_jump_insn_after_noloc): Likewise.
>   (emit_call_insn_after_noloc): Likewise.
>   (emit_debug_insn_after_noloc): Likewise.
>   (emit_barrier_after): Likewise.
>   (emit_label_after): Likewise for both params.
>   (emit_pattern_after_setloc): Likewise for "after" param.  Convert
>   "loc" param from "int" to "location_t".
>   (emit_insn_after_setloc): Likewise.
>   (emit_jump_insn_after_setloc): Likewise.
>   (emit_call_insn_after_setloc): Likewise.
>   (emit_debug_insn_after_setloc): Likewise.
>   (emit_pattern_before_setloc): Likewise for "before" param.  Convert
>   "loc" param from "int" to "location_t".
>   (emit_pattern_before): Convert NULL_RTX to NULL.
>   (emit_insn_before_setloc): Convert "loc" param from "int" to
>   "location_t".
>   (emit_jump_insn_before_setloc): Likewise.
>   (emit_call_insn_before_setloc): Likewise.
>   (emit_debug_insn_before_setloc): Strengthen "before" param from rtx to
>   rtx_insn *.  Convert "loc" param from "int" to "location_t".
>   * rtl.h (emit_insn_before_setloc, emit_jump_insn_before_setloc,
>   emit_call_insn_before_setloc, emit_debug_insn_before_setloc):
>   Convert 3rd param from "int" to "location_t".
>   (emit_barrier_before, emit_barrier_after, next_real_insn):
>   Strengthen param from rtx to rtx_insn *.
>   (emit_label_before): Strengthen 1st param from "rtx" to
>   "rtx_code_label *".  Strengthen 2nd param from "rtx" to
>   "rtx_insn *".
>   (emit_insn_after_noloc, emit_jump_insn_after_noloc,
>   emit_call_insn_after_noloc, emit_debug_insn_after_noloc):
>   Strengthen 2nd param from "rtx" to "rtx_insn *".
>   (emit_insn_after_setloc, emit_jump_insn_after_setloc)
>   emit_call_insn_after_setloc, emit_debug_insn_after_setloc):
>   Likewise. Convert 3rd param from "int" to "location_t".
>   (emit_label_after): Strengthen 1st param from "rtx" to
>   "rtx_code_label *".
>   (next_real_insn, remove_insn): Strengthen param from "rtx" to
>   "rtx_insn *".
>   (add_insn_before, add_insn_after): Strengthen 1st and 2nd params
>   from "rtx" to "rtx_insn *".
OK.  I think that covers the entire set.

I can't immediately recall if the as-a and safe-as-a are compile or
runtime checks.  Obviously your testing ought to be sufficient for the
former.  Mine will help with the latter since it'll build arc, bfin and
c6x elf targets through newlib.  For sh it'll build a mixture of -elf
things through newlib and it'll bootstrap sh4.

jeff

[PATCH 1/5] emit-rtl.c: more typesafety

2018-06-12 Thread David Malcolm 
This patch converts various rtx to rtx_insn * (or rtx_code_label *).
It also convert the various "_loc" params from int to location_t

gcc/ChangeLog:
* emit-rtl.c (next_real_insn): Strengthen param from "rtx"
to "rtx_insn *".
(add_insn_after): Likewise for first two params.
(add_insn_before): Likewise.
(remove_insn): Likewise for param.
(emit_pattern_before_noloc): Likewise for second and third params.
(emit_jump_insn_before_noloc): Convert NULL_RTX to NULL.
(emit_call_insn_before_noloc): Likewise.
(emit_debug_insn_before_noloc): Strengthen "before" param from "rtx"
to "rtx_insn *".
(emit_barrier_before): Likewise.
(emit_label_before): Strengthen "label" param from "rtx" to
"rtx_code_label *".  Strengthen "before" param from "rtx" to
"rtx_insn *".
(emit_insn_after_1): Strengthen "after" param from "rtx" to
"rtx_insn *".
(emit_pattern_after_noloc): Likewise.
(emit_insn_after_noloc): Likewise.
(emit_jump_insn_after_noloc): Likewise.
(emit_call_insn_after_noloc): Likewise.
(emit_debug_insn_after_noloc): Likewise.
(emit_barrier_after): Likewise.
(emit_label_after): Likewise for both params.
(emit_pattern_after_setloc): Likewise for "after" param.  Convert
"loc" param from "int" to "location_t".
(emit_insn_after_setloc): Likewise.
(emit_jump_insn_after_setloc): Likewise.
(emit_call_insn_after_setloc): Likewise.
(emit_debug_insn_after_setloc): Likewise.
(emit_pattern_before_setloc): Likewise for "before" param.  Convert
"loc" param from "int" to "location_t".
(emit_pattern_before): Convert NULL_RTX to NULL.
(emit_insn_before_setloc): Convert "loc" param from "int" to
"location_t".
(emit_jump_insn_before_setloc): Likewise.
(emit_call_insn_before_setloc): Likewise.
(emit_debug_insn_before_setloc): Strengthen "before" param from rtx to
rtx_insn *.  Convert "loc" param from "int" to "location_t".
* rtl.h (emit_insn_before_setloc, emit_jump_insn_before_setloc,
emit_call_insn_before_setloc, emit_debug_insn_before_setloc):
Convert 3rd param from "int" to "location_t".
(emit_barrier_before, emit_barrier_after, next_real_insn):
Strengthen param from rtx to rtx_insn *.
(emit_label_before): Strengthen 1st param from "rtx" to
"rtx_code_label *".  Strengthen 2nd param from "rtx" to
"rtx_insn *".
(emit_insn_after_noloc, emit_jump_insn_after_noloc,
emit_call_insn_after_noloc, emit_debug_insn_after_noloc):
Strengthen 2nd param from "rtx" to "rtx_insn *".
(emit_insn_after_setloc, emit_jump_insn_after_setloc)
emit_call_insn_after_setloc, emit_debug_insn_after_setloc):
Likewise. Convert 3rd param from "int" to "location_t".
(emit_label_after): Strengthen 1st param from "rtx" to
"rtx_code_label *".
(next_real_insn, remove_insn): Strengthen param from "rtx" to
"rtx_insn *".
(add_insn_before, add_insn_after): Strengthen 1st and 2nd params
from "rtx" to "rtx_insn *".
---
 gcc/emit-rtl.c | 84 ++
 gcc/rtl.h  | 41 ++--
 2 files changed, 58 insertions(+), 67 deletions(-)

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 613030f..a327ff2 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3602,10 +3602,8 @@ prev_nonnote_nondebug_insn_bb (rtx_insn *insn)
SEQUENCEs.  */
 
 rtx_insn *
-next_real_insn (rtx uncast_insn)
+next_real_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = NEXT_INSN (insn);
@@ -4230,10 +4228,8 @@ add_insn_before_nobb (rtx_insn *insn, rtx_insn *before)
they know how to update a SEQUENCE. */
 
 void
-add_insn_after (rtx uncast_insn, rtx uncast_after, basic_block bb)
+add_insn_after (rtx_insn *insn, rtx_insn *after, basic_block bb)
 {
-  rtx_insn *insn = as_a  (uncast_insn);
-  rtx_insn *after = as_a  (uncast_after);
   add_insn_after_nobb (insn, after);
   if (!BARRIER_P (after)
   && !BARRIER_P (insn)
@@ -4260,10 +4256,8 @@ add_insn_after (rtx uncast_insn, rtx uncast_after, 
basic_block bb)
they know how to update a SEQUENCE. */
 
 void
-add_insn_before (rtx uncast_insn, rtx uncast_before, basic_block bb)
+add_insn_before (rtx_insn *insn, rtx_insn *before, basic_block bb)
 {
-  rtx_insn *insn = as_a  (uncast_insn);
-  rtx_insn *before = as_a  (uncast_before);
   add_insn_before_nobb (insn, before);
 
   if (!bb
@@ -4313,9 +4307,8 @@ set_insn_deleted (rtx insn)
To really delete an insn and related DF information, use delete_insn.  */
 
 void
-remove_insn (rtx uncast_insn)
+remove_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = as_a  (uncast_insn);
   rtx_insn *next = NEXT_INSN (insn);
   rtx_insn *prev =