RL78 UNUSED note setting bug fix in rl78_note_reg_set

Message ID 000601d38bcf$8790a0d0$96b1e270$@renesas.com
State New
Headers show
Series
  • RL78 UNUSED note setting bug fix in rl78_note_reg_set
Related show

Commit Message

Sebastian Perta Jan. 12, 2018, 6:02 p.m.
Hello,

The below patch fixes 31 regression failures (28 for C and 3 for C++, see
below) plus the problem discovered here
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00688.html by DJ.
Tested with the following command:
make -k check-gcc RUNTESTFLAGS=--target_board=rl78-sim

List of previous fails which get fixed by this patch:
FAIL: gcc.c-torture/execute/pr42512.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O1  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O3 -g  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -Os  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr53645-2.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O1  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O3 -g  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -Os  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr53645.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
FAIL: gcc.c-torture/execute/pr61306-2.c   -O1  execution test
FAIL: gcc.c-torture/execute/pr61306-2.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr61306-2.c   -O3 -g  execution test
FAIL: gcc.c-torture/execute/pr61306-2.c   -Os  execution test
FAIL: gcc.c-torture/execute/pr61306-2.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr65401.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: gcc.dg/compat/struct-by-value-13 c_compat_x_tst.o-c_compat_y_tst.o
link
FAIL: gcc.dg/builtin-arith-overflow-1.c execution test
FAIL: gcc.dg/builtin-bswap-4.c execution test
FAIL: gcc.dg/pr26719.c execution test
FAIL: c-c++-common/torture/builtin-arith-overflow-14.c   -O0  (test for
excess errors)
FAIL: c-c++-common/torture/builtin-arith-overflow-3.c   -O0  (test for
excess errors)
FAIL: gcc.dg/torture/pr57569.c   -Os  execution test
FAIL: tmpdir-g++.dg-struct-layout-1/t005 cp_compat_x_tst.o-cp_compat_y_tst.o
link
FAIL: c-c++-common/torture/builtin-arith-overflow-14.c   -O0  (test for
excess errors)
FAIL: c-c++-common/torture/builtin-arith-overflow-3.c   -O0  (test for
excess errors)

Please let me know if this is OK. Thank you!

Best Regards,
Sebastian

   for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)

Comments

Sebastian Perta Jan. 12, 2018, 6:41 p.m. | #1
Hi DJ,

>>Do you have checkin privs yet?
I have filled out the form. "Thanks for your request. It must be approved by the person you named as approver ...

>> This is ok aside from..
Sorry about this. I will keep this in mind in future.
I corrected the patch with your second suggestion.

Best Regards,
Sebastian

Index: ChangeLog
===================================================================
--- ChangeLog(revision 256590)
+++ ChangeLog(working copy)
@@ -1,3 +1,8 @@
+2018-01-12  Sebastian Perta  <sebastian.perta@renesas.com>
+
+* config/rl78/rl78.c (rl78_note_reg_set): fixed dead reg check
+for non-QImode registers
+
 2018-01-12  Vladimir Makarov  <vmakarov@redhat.com>

 PR rtl-optimization/80481
Index: config/rl78/rl78.c
===================================================================
--- config/rl78/rl78.c(revision 256590)
+++ config/rl78/rl78.c(working copy)
@@ -3792,7 +3792,7 @@
 rl78_note_reg_set (char *dead, rtx d, rtx insn)
 {
   int r, i;
-
+  bool is_dead;
   if (GET_CODE (d) == MEM)
     rl78_note_reg_uses (dead, XEXP (d, 0), insn);

@@ -3799,9 +3799,15 @@
   if (GET_CODE (d) != REG)
     return;

+ /* Do not mark the reg unused unless all QImode parts of it are dead.  */
   r = REGNO (d);
-  if (dead [r])
-    add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d), r));
+  is_dead = true;
+  for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)
+  if (!dead [r + i])
+  is_dead = false;
+  if(is_dead)
+add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d), r));
   if (dump_file)
     fprintf (dump_file, "note set reg %d size %d\n", r, GET_MODE_SIZE (GET_MODE (d)));
   for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)

> -----Original Message-----
> From: DJ Delorie [mailto:dj@redhat.com]
> Sent: 12 January 2018 18:12
> To: Sebastian Perta <Sebastian.Perta@renesas.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] RL78 UNUSED note setting bug fix in rl78_note_reg_set
>
>
> "Sebastian Perta" <sebastian.perta@renesas.com> writes:
> > Please let me know if this is OK. Thank you!
>
> Do you have checkin privs yet?
>
> This is ok aside from..
>
> > +  /* 'dead' keeps track of the QImode registers if r is of different size
> > +  we need to check the other subparts as well  */
>
> Missing period at the end of a sentence; should capitalize first word
> but it's a variable, which should be block caps anyway, and it reads
> better as two sentences:
>
> > +  /* DEAD keeps track of the QImode registers.  If R is of different size
> > +  we need to check the other subparts as well.  */
>
> Or rewrite to not mention variables?
>
> > + /* Do not mark the reg unused unless all QImode parts of it are dead.  */



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Sebastian Perta Jan. 19, 2018, 1:47 p.m. | #2
HI DJ,

>>Do you have checkin privs yet?
>> This is ok aside from.. ... + /* Do not mark the reg unused unless all
QImode parts of it are dead.  */
Can I checkin this patch? Thank you!

Best Regards,
Sebastian


> -----Original Message-----
> From: Sebastian Perta
> Sent: 12 January 2018 18:42
> To: 'DJ Delorie' <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] RL78 UNUSED note setting bug fix in rl78_note_reg_set
> 
> Hi DJ,
> 
> >>Do you have checkin privs yet?
> I have filled out the form. "Thanks for your request. It must be approved
by
> the person you named as approver ...
> 
> >> This is ok aside from..
> Sorry about this. I will keep this in mind in future.
> I corrected the patch with your second suggestion.
> 
> Best Regards,
> Sebastian
> 
> Index: ChangeLog
> ==========================================================
> =========
> --- ChangeLog	(revision 256590)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,8 @@
> +2018-01-12  Sebastian Perta  <sebastian.perta@renesas.com>
> +
> +	* config/rl78/rl78.c (rl78_note_reg_set): fixed dead reg check
> +	for non-QImode registers
> +
>  2018-01-12  Vladimir Makarov  <vmakarov@redhat.com>
> 
>  	PR rtl-optimization/80481
> Index: config/rl78/rl78.c
> ==========================================================
> =========
> --- config/rl78/rl78.c	(revision 256590)
> +++ config/rl78/rl78.c	(working copy)
> @@ -3792,7 +3792,7 @@
>  rl78_note_reg_set (char *dead, rtx d, rtx insn)
>  {
>    int r, i;
> -
> +  bool is_dead;
>    if (GET_CODE (d) == MEM)
>      rl78_note_reg_uses (dead, XEXP (d, 0), insn);
> 
> @@ -3799,9 +3799,15 @@
>    if (GET_CODE (d) != REG)
>      return;
> 
> + /* Do not mark the reg unused unless all QImode parts of it are dead.
*/
>    r = REGNO (d);
> -  if (dead [r])
> -    add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d), r));
> +  is_dead = true;
> +  for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)
> +	  if (!dead [r + i])
> +		  is_dead = false;
> +  if(is_dead)
> +	add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d),
> r));
>    if (dump_file)
>      fprintf (dump_file, "note set reg %d size %d\n", r, GET_MODE_SIZE
> (GET_MODE (d)));
>    for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)
> 
> > -----Original Message-----
> > From: DJ Delorie [mailto:dj@redhat.com]
> > Sent: 12 January 2018 18:12
> > To: Sebastian Perta <Sebastian.Perta@renesas.com>
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] RL78 UNUSED note setting bug fix in
> rl78_note_reg_set
> >
> >
> > "Sebastian Perta" <sebastian.perta@renesas.com> writes:
> > > Please let me know if this is OK. Thank you!
> >
> > Do you have checkin privs yet?
> >
> > This is ok aside from..
> >
> > > +  /* 'dead' keeps track of the QImode registers if r is of different
size
> > > +  we need to check the other subparts as well  */
> >
> > Missing period at the end of a sentence; should capitalize first word
> > but it's a variable, which should be block caps anyway, and it reads
> > better as two sentences:
> >
> > > +  /* DEAD keeps track of the QImode registers.  If R is of different
size
> > > +  we need to check the other subparts as well.  */
> >
> > Or rewrite to not mention variables?
> >
> > > + /* Do not mark the reg unused unless all QImode parts of it are
dead.
> */

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 256590)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2018-01-12  Sebastian Perta  <sebastian.perta@renesas.com>
+	
+	* config/rl78/rl78.c (rl78_note_reg_set): fixed dead reg check 
+	for non-QImode registers
+
 2018-01-12  Vladimir Makarov  <vmakarov@redhat.com>
 
 	PR rtl-optimization/80481
Index: config/rl78/rl78.c
===================================================================
--- config/rl78/rl78.c	(revision 256590)
+++ config/rl78/rl78.c	(working copy)
@@ -3792,7 +3792,7 @@ 
 rl78_note_reg_set (char *dead, rtx d, rtx insn)
 {
   int r, i;
-
+  bool is_dead;
   if (GET_CODE (d) == MEM)
     rl78_note_reg_uses (dead, XEXP (d, 0), insn);
 
@@ -3799,9 +3799,15 @@ 
   if (GET_CODE (d) != REG)
     return;
 
+  /* 'dead' keeps track of the QImode registers if r is of different size
+  we need to check the other subparts as well  */
   r = REGNO (d);
-  if (dead [r])
-    add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d), r));
+  is_dead = true;
+  for (i = 0; i < GET_MODE_SIZE (GET_MODE (d)); i ++)
+	  if (!dead [r + i])
+		  is_dead = false;
+  if(is_dead)
+	add_reg_note (insn, REG_UNUSED, gen_rtx_REG (GET_MODE (d), r));
   if (dump_file)
     fprintf (dump_file, "note set reg %d size %d\n", r, GET_MODE_SIZE
(GET_MODE (d)));