patch to fix PR82353

patch to fix PR82353

[Vladimir Makarov]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353

LRA did not update hard reg liveness on bb borders for hard regs which 
are part of insn patterns like CFLAGS reg.  It was ok for inheritance in 
EBB which creates only moves and they usually have no embedded hard regs 
in the patterns.  But LRA rematerialization needs this.  So this patch 
implements such hard reg liveness updates.

The patch was successfully bootstrapped and tested on x86-64.

Committed as rev. 253656.



[-- Attachment #2: pr82353.patch --]
[-- Type: text/x-patch, Size: 5948 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 253655)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@
+2017-10-11  Vladimir Makarov  <vmakarov@redhat.com>
+
+	PR sanitizer/82353
+	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
+	locations.
+	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
+	(make_hard_regno_born, make_hard_regno_dead): Update
+	bb_killed_pseudos and bb_gen_pseudos.
+
 2017-10-11  Nathan Sidwell  <nathan@acm.org>
 
 	* incpath.h (enum incpath_kind): Name enum, prefix values.
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 253655)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2017-10-11  Vladimir Makarov  <vmakarov@redhat.com>
+
+	PR sanitizer/82353
+	* gcc.target/i386/i386.exp (tests): Permit '.C' extension.
+	* gcc.target/i386/pr82353.C: New.
+
 2017-10-11  Uros Bizjak  <ubizjak@gmail.com>
 
 	* gcc.target/i386/387-ficom-1.c: New test.
Index: lra-lives.c
===================================================================
--- lra-lives.c	(revision 253253)
+++ lra-lives.c	(working copy)
@@ -220,6 +220,9 @@ lra_intersected_live_ranges_p (lra_live_
   return false;
 }
 
+/* The corresponding bitmaps of BB currently being processed.  */
+static bitmap bb_killed_pseudos, bb_gen_pseudos;
+
 /* The function processing birth of hard register REGNO.  It updates
    living hard regs, START_LIVING, and conflict hard regs for living
    pseudos.  Conflict hard regs for the pic pseudo is not updated if
@@ -243,6 +246,7 @@ make_hard_regno_born (int regno, bool ch
 	|| i != REGNO (pic_offset_table_rtx))
 #endif
       SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
+  bitmap_set_bit (bb_gen_pseudos, regno);
 }
 
 /* Process the death of hard register REGNO.  This updates
@@ -255,6 +259,8 @@ make_hard_regno_dead (int regno)
     return;
   sparseset_set_bit (start_dying, regno);
   CLEAR_HARD_REG_BIT (hard_regs_live, regno);
+  bitmap_clear_bit (bb_gen_pseudos, regno);
+  bitmap_set_bit (bb_killed_pseudos, regno);
 }
 
 /* Mark pseudo REGNO as living at program point POINT, update conflicting
@@ -299,9 +305,6 @@ mark_pseudo_dead (int regno, int point)
     }
 }
 
-/* The corresponding bitmaps of BB currently being processed.  */
-static bitmap bb_killed_pseudos, bb_gen_pseudos;
-
 /* Mark register REGNO (pseudo or hard register) in MODE as live at
    program point POINT.  Update BB_GEN_PSEUDOS.
    Return TRUE if the liveness tracking sets were modified, or FALSE
Index: lra.c
===================================================================
--- lra.c	(revision 253253)
+++ lra.c	(working copy)
@@ -820,7 +820,8 @@ collect_non_operand_hard_regs (rtx *x, l
   const char *fmt = GET_RTX_FORMAT (code);
 
   for (i = 0; i < data->insn_static_data->n_operands; i++)
-    if (x == data->operand_loc[i])
+    if (! data->insn_static_data->operand[i].is_operator
+	&& x == data->operand_loc[i])
       /* It is an operand loc. Stop here.  */
       return list;
   for (i = 0; i < data->insn_static_data->n_dups; i++)
Index: testsuite/gcc.target/i386/i386.exp
===================================================================
--- testsuite/gcc.target/i386/i386.exp	(revision 253253)
+++ testsuite/gcc.target/i386/i386.exp	(working copy)
@@ -445,7 +445,7 @@ if [runtest_file_p $runtests $srcdir/$su
 }
 
 # Everything else.
-set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]]
+set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]]
 set tests [prune $tests $srcdir/$subdir/vect-args.c]
 
 # Main loop.
Index: testsuite/gcc.target/i386/pr82353.C
===================================================================
--- testsuite/gcc.target/i386/pr82353.C	(nonexistent)
+++ testsuite/gcc.target/i386/pr82353.C	(working copy)
@@ -0,0 +1,60 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
+
+extern unsigned long tf_2_var_1, tf_2_var_21;
+extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
+extern unsigned char tf_2_var_16, tf_2_var_31;
+extern short tf_2_var_69;
+extern unsigned tf_2_var_233;
+struct tf_2_struct_1 {
+  short member_1_0 : 27;
+  long member_1_1 : 10;
+};
+struct a {
+  int member_2_0 : 5;
+};
+struct tf_2_struct_3 {
+  static tf_2_struct_1 member_3_0;
+};
+struct tf_2_struct_4 {
+  static unsigned member_4_0;
+  a member_4_1;
+};
+struct tf_2_struct_5 {
+  tf_2_struct_1 member_5_2;
+  tf_2_struct_4 member_5_4;
+};
+struct tf_2_struct_6 {
+  tf_2_struct_5 member_6_2;
+  short member_6_4;
+} extern tf_2_struct_obj_2;
+extern tf_2_struct_3 tf_2_struct_obj_8;
+tf_2_struct_1 a;
+tf_2_struct_5 b;
+tf_2_struct_1 tf_2_struct_3::member_3_0;
+unsigned tf_2_struct_4::member_4_0;
+void tf_2_init() {
+  a.member_1_1 = tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1 = 5;
+}
+void tf_2_foo() {
+  int c = tf_2_struct_obj_2.member_6_2.member_5_4.member_4_1.member_2_0 -
+          -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
+  tf_2_struct_obj_8.member_3_0.member_1_0 =
+      tf_2_var_24 >
+      tf_2_var_21 * a.member_1_0 * tf_2_var_2 - tf_2_var_5 % a.member_1_1;
+  if ((~(tf_2_var_31 * tf_2_var_6) &&
+       -~tf_2_struct_obj_2.member_6_4 * 90284000534361) %
+      ~tf_2_var_31 * tf_2_var_6)
+    b.member_5_2.member_1_0 << tf_2_var_16 << tf_2_var_1;
+  tf_2_var_233 = -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
+  int d(tf_2_struct_obj_2.member_6_4);
+  if (b.member_5_2.member_1_0)
+    b.member_5_2.member_1_1 = c;
+  bool e(~-~tf_2_struct_obj_2.member_6_4);
+  a.member_1_1 % e;
+  if (tf_2_var_5 / tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1)
+    b.member_5_4.member_4_0 = tf_2_var_21 * a.member_1_0 * tf_2_var_2;
+  tf_2_var_69 = tf_2_var_6;
+}
+
+/* { dg-final { scan-rtl-dump-not "Inserting rematerialization insn" "reload" } } */

Re: patch to fix PR82353

[Jakub Jelinek]

On Wed, Oct 11, 2017 at 03:39:21PM -0400, Vladimir Makarov wrote:
> The following patch fixes
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
> 
> LRA did not update hard reg liveness on bb borders for hard regs which are
> part of insn patterns like CFLAGS reg.  It was ok for inheritance in EBB
> which creates only moves and they usually have no embedded hard regs in the
> patterns.  But LRA rematerialization needs this.  So this patch implements
> such hard reg liveness updates.
> 
> The patch was successfully bootstrapped and tested on x86-64.
> 
> Committed as rev. 253656.

Thanks for the fix, however I believe we do not want C++ tests in
gcc.target/*/, those are historically handled in g++.dg/ (and if we wanted
to have them, we'd introduce g++.target/*/), furthermore the test requires
working -fsanitize=undefined and such tests belog into g++.dg/ubsan/
if they are in C++.  And finally, the test fails on i386, where we
rematerialize something else, so I think we should just limit it to lp64.

Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
change, ok for trunk?

2017-10-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/82353
	* gcc.target/i386/i386.exp (tests): Revert the '.C' extension change.
        * gcc.target/i386/pr82353.C: Moved to ...
	* g++.dg/ubsan/pr82353.C: ... here.  Restrict to i?86/x86_64 && lp64.

--- gcc/testsuite/gcc.target/i386/i386.exp.jj	2017-10-11 22:37:51.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/i386.exp	2017-10-11 22:59:47.455746874 +0200
@@ -445,7 +445,7 @@ if [runtest_file_p $runtests $srcdir/$su
 }
 
 # Everything else.
-set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]]
+set tests [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]]
 set tests [prune $tests $srcdir/$subdir/vect-args.c]
 
 # Main loop.
--- gcc/testsuite/gcc.target/i386/pr82353.C.jj	2017-10-11 22:37:51.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr82353.C	2017-10-11 23:01:37.618390302 +0200
@@ -1,60 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
-
-extern unsigned long tf_2_var_1, tf_2_var_21;
-extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
-extern unsigned char tf_2_var_16, tf_2_var_31;
-extern short tf_2_var_69;
-extern unsigned tf_2_var_233;
-struct tf_2_struct_1 {
-  short member_1_0 : 27;
-  long member_1_1 : 10;
-};
-struct a {
-  int member_2_0 : 5;
-};
-struct tf_2_struct_3 {
-  static tf_2_struct_1 member_3_0;
-};
-struct tf_2_struct_4 {
-  static unsigned member_4_0;
-  a member_4_1;
-};
-struct tf_2_struct_5 {
-  tf_2_struct_1 member_5_2;
-  tf_2_struct_4 member_5_4;
-};
-struct tf_2_struct_6 {
-  tf_2_struct_5 member_6_2;
-  short member_6_4;
-} extern tf_2_struct_obj_2;
-extern tf_2_struct_3 tf_2_struct_obj_8;
-tf_2_struct_1 a;
-tf_2_struct_5 b;
-tf_2_struct_1 tf_2_struct_3::member_3_0;
-unsigned tf_2_struct_4::member_4_0;
-void tf_2_init() {
-  a.member_1_1 = tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1 = 5;
-}
-void tf_2_foo() {
-  int c = tf_2_struct_obj_2.member_6_2.member_5_4.member_4_1.member_2_0 -
-          -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
-  tf_2_struct_obj_8.member_3_0.member_1_0 =
-      tf_2_var_24 >
-      tf_2_var_21 * a.member_1_0 * tf_2_var_2 - tf_2_var_5 % a.member_1_1;
-  if ((~(tf_2_var_31 * tf_2_var_6) &&
-       -~tf_2_struct_obj_2.member_6_4 * 90284000534361) %
-      ~tf_2_var_31 * tf_2_var_6)
-    b.member_5_2.member_1_0 << tf_2_var_16 << tf_2_var_1;
-  tf_2_var_233 = -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
-  int d(tf_2_struct_obj_2.member_6_4);
-  if (b.member_5_2.member_1_0)
-    b.member_5_2.member_1_1 = c;
-  bool e(~-~tf_2_struct_obj_2.member_6_4);
-  a.member_1_1 % e;
-  if (tf_2_var_5 / tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1)
-    b.member_5_4.member_4_0 = tf_2_var_21 * a.member_1_0 * tf_2_var_2;
-  tf_2_var_69 = tf_2_var_6;
-}
-
-/* { dg-final { scan-rtl-dump-not "Inserting rematerialization insn" "reload" } } */
--- gcc/testsuite/g++.dg/ubsan/pr82353.C.jj	2017-10-11 23:00:59.442860406 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr82353.C	2017-10-11 23:04:27.264301238 +0200
@@ -0,0 +1,60 @@
+/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && lp64 } } } */
+/* { dg-options "-O2 -std=c++11 -fsanitize=undefined -fno-sanitize-recover=undefined -w -fdump-rtl-reload" } */
+
+extern unsigned long tf_2_var_1, tf_2_var_21;
+extern bool tf_2_var_2, tf_2_var_24, tf_2_var_6, tf_2_var_5;
+extern unsigned char tf_2_var_16, tf_2_var_31;
+extern short tf_2_var_69;
+extern unsigned tf_2_var_233;
+struct tf_2_struct_1 {
+  short member_1_0 : 27;
+  long member_1_1 : 10;
+};
+struct a {
+  int member_2_0 : 5;
+};
+struct tf_2_struct_3 {
+  static tf_2_struct_1 member_3_0;
+};
+struct tf_2_struct_4 {
+  static unsigned member_4_0;
+  a member_4_1;
+};
+struct tf_2_struct_5 {
+  tf_2_struct_1 member_5_2;
+  tf_2_struct_4 member_5_4;
+};
+struct tf_2_struct_6 {
+  tf_2_struct_5 member_6_2;
+  short member_6_4;
+} extern tf_2_struct_obj_2;
+extern tf_2_struct_3 tf_2_struct_obj_8;
+tf_2_struct_1 a;
+tf_2_struct_5 b;
+tf_2_struct_1 tf_2_struct_3::member_3_0;
+unsigned tf_2_struct_4::member_4_0;
+void tf_2_init() {
+  a.member_1_1 = tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1 = 5;
+}
+void tf_2_foo() {
+  int c = tf_2_struct_obj_2.member_6_2.member_5_4.member_4_1.member_2_0 -
+          -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
+  tf_2_struct_obj_8.member_3_0.member_1_0 =
+      tf_2_var_24 >
+      tf_2_var_21 * a.member_1_0 * tf_2_var_2 - tf_2_var_5 % a.member_1_1;
+  if ((~(tf_2_var_31 * tf_2_var_6) &&
+       -~tf_2_struct_obj_2.member_6_4 * 90284000534361) %
+      ~tf_2_var_31 * tf_2_var_6)
+    b.member_5_2.member_1_0 << tf_2_var_16 << tf_2_var_1;
+  tf_2_var_233 = -~tf_2_struct_obj_2.member_6_4 * char(90284000534361);
+  int d(tf_2_struct_obj_2.member_6_4);
+  if (b.member_5_2.member_1_0)
+    b.member_5_2.member_1_1 = c;
+  bool e(~-~tf_2_struct_obj_2.member_6_4);
+  a.member_1_1 % e;
+  if (tf_2_var_5 / tf_2_struct_obj_2.member_6_2.member_5_2.member_1_1)
+    b.member_5_4.member_4_0 = tf_2_var_21 * a.member_1_0 * tf_2_var_2;
+  tf_2_var_69 = tf_2_var_6;
+}
+
+/* { dg-final { scan-rtl-dump-not "Inserting rematerialization insn" "reload" } } */


	Jakub

Re: patch to fix PR82353

[Vladimir Makarov]

On 10/11/2017 05:11 PM, Jakub Jelinek wrote:
> On Wed, Oct 11, 2017 at 03:39:21PM -0400, Vladimir Makarov wrote:
>> The following patch fixes
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
>>
>> LRA did not update hard reg liveness on bb borders for hard regs which are
>> part of insn patterns like CFLAGS reg.  It was ok for inheritance in EBB
>> which creates only moves and they usually have no embedded hard regs in the
>> patterns.  But LRA rematerialization needs this.  So this patch implements
>> such hard reg liveness updates.
>>
>> The patch was successfully bootstrapped and tested on x86-64.
>>
>> Committed as rev. 253656.
> Thanks for the fix, however I believe we do not want C++ tests in
> gcc.target/*/, those are historically handled in g++.dg/ (and if we wanted
> to have them, we'd introduce g++.target/*/), furthermore the test requires
> working -fsanitize=undefined and such tests belog into g++.dg/ubsan/
> if they are in C++.  And finally, the test fails on i386, where we
> rematerialize something else, so I think we should just limit it to lp64.
I just followed arch64 which has C++ tests in gcc.target/arch64.

I think we will have more and more target tests on C++ in the future, so 
probably having g++.target is a good idea.
> Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
> change, ok for trunk?
>
>
Sure, Jakub.

Re: patch to fix PR82353

[Jakub Jelinek]

Hi!

On Wed, Oct 11, 2017 at 06:41:05PM -0400, Vladimir Makarov wrote:
> > Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
> > change, ok for trunk?

BTW, I think it is quite fragile to scan for the reload messages, so I've
cooked up a runtime test that fails before your patch and succeeds with your
patch.  Tested on x86_64-linux with -m32/-m64 (both with your patch reverted
and without), ok for trunk?

2017-10-12  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/82353
	* g++.dg/ubsan/pr82353-2.C: New test.
	* g++.dg/ubsan/pr82353-2-aux.cc: New file.
	* g++.dg/ubsan/pr82353-2.h: New file.

--- gcc/testsuite/g++.dg/ubsan/pr82353-2.C.jj	2017-10-12 18:39:43.712341186 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr82353-2.C	2017-10-12 18:39:23.000000000 +0200
@@ -0,0 +1,20 @@
+// PR sanitizer/82353
+// { dg-do run }
+// { dg-options "-fsanitize=undefined -fno-sanitize-recover=undefined -std=c++11 -O2 -w" }
+// { dg-additional-sources "pr82353-2-aux.cc" }
+
+#include "pr82353-2.h"
+
+unsigned long f, g;
+bool h, k, j, i;
+unsigned char l, m;
+short n;
+unsigned o;
+F p;
+
+int
+main ()
+{
+  foo ();
+  bar ();
+}
--- gcc/testsuite/g++.dg/ubsan/pr82353-2-aux.cc.jj	2017-10-12 18:39:39.375393412 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr82353-2-aux.cc	2017-10-12 18:38:47.000000000 +0200
@@ -0,0 +1,32 @@
+// PR sanitizer/82353
+
+#include "pr82353-2.h"
+
+B a;
+E b;
+B C::c0;
+unsigned D::d0;
+
+void
+foo ()
+{
+  a.b1 = p.f2.e2.b1 = 5;
+}
+
+void
+bar ()
+{
+  int c = p.f2.e4.d1.a0 - -~p.f4 * 89;
+  q.c0.b0 = i > g * a.b0 * h - k % a.b1;
+  if ((~(m * j) && -~p.f4 * 90284000534361) % ~m * j)
+    b.e2.b0 << l << f;
+  o = -~p.f4 * 89;
+  int d = p.f4;
+  if (b.e2.b0)
+    b.e2.b1 = c;
+  bool e = ~-~p.f4;
+  a.b1 % e;
+  if (k / p.f2.e2.b1)
+    b.e4.d0 = g * a.b0 * h;
+  n = j;
+}
--- gcc/testsuite/g++.dg/ubsan/pr82353-2.h.jj	2017-10-12 18:39:46.671305554 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr82353-2.h	2017-10-12 18:32:04.000000000 +0200
@@ -0,0 +1,31 @@
+extern unsigned long f, g;
+extern bool h, i, j, k;
+extern unsigned char l, m;
+extern short n;
+extern unsigned o;
+struct B {
+  short b0 : 27;
+  long b1 : 10;
+};
+struct A {
+  int a0 : 5;
+};
+struct C {
+  static B c0;
+};
+struct D {
+  static unsigned d0;
+  A d1;
+};
+struct E {
+  B e2;
+  D e4;
+};
+struct F {
+  E f2;
+  short f4;
+};
+extern F p;
+extern C q;
+void foo ();
+void bar ();


	Jakub

Re: patch to fix PR82353

[Vladimir Makarov]

On 10/12/2017 12:49 PM, Jakub Jelinek wrote:
> Hi!
>
> On Wed, Oct 11, 2017 at 06:41:05PM -0400, Vladimir Makarov wrote:
>>> Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
>>> change, ok for trunk?
> BTW, I think it is quite fragile to scan for the reload messages, so I've
> cooked up a runtime test that fails before your patch and succeeds with your
> patch.  Tested on x86_64-linux with -m32/-m64 (both with your patch reverted
> and without), ok for trunk?
>
>
OK.

FYI, I am going to revert LRA part of the patch because it results in a 
failure bootstrap with go or ada.  I guess the new version of the patch 
will be ready tomorrow or on Monday.

Re: patch to fix PR82353

[Jakub Jelinek]

On Thu, Oct 12, 2017 at 01:05:21PM -0400, Vladimir Makarov wrote:
> 
> 
> On 10/12/2017 12:49 PM, Jakub Jelinek wrote:
> > Hi!
> > 
> > On Wed, Oct 11, 2017 at 06:41:05PM -0400, Vladimir Makarov wrote:
> > > > Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
> > > > change, ok for trunk?
> > BTW, I think it is quite fragile to scan for the reload messages, so I've
> > cooked up a runtime test that fails before your patch and succeeds with your
> > patch.  Tested on x86_64-linux with -m32/-m64 (both with your patch reverted
> > and without), ok for trunk?
> > 
> > 
> OK.

Thanks.

> FYI, I am going to revert LRA part of the patch because it results in a
> failure bootstrap with go or ada.  I guess the new version of the patch will
> be ready tomorrow or on Monday.

Yeah, I've just noticed that too (my bootstrap failed in Ada).  I'll defer
committing the patch after you commit your new fix then.

	Jakub

Re: patch to fix PR82353

[Jeff Law]

On 10/12/2017 10:49 AM, Jakub Jelinek wrote:
> Hi!
> 
> On Wed, Oct 11, 2017 at 06:41:05PM -0400, Vladimir Makarov wrote:
>>> Tested on x86_64-linux -m32/-m64, and verified with cc1plus before your
>>> change, ok for trunk?
> 
> BTW, I think it is quite fragile to scan for the reload messages, so I've
> cooked up a runtime test that fails before your patch and succeeds with your
> patch.  Tested on x86_64-linux with -m32/-m64 (both with your patch reverted
> and without), ok for trunk?
> 
> 2017-10-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/82353
> 	* g++.dg/ubsan/pr82353-2.C: New test.
> 	* g++.dg/ubsan/pr82353-2-aux.cc: New file.
> 	* g++.dg/ubsan/pr82353-2.h: New file.
OK.
Jeff

Re: patch to fix PR82353

[Uros Bizjak]

Hello!

> The following patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
>
> LRA did not update hard reg liveness on bb borders for hard regs which are part of insn patterns
> like CFLAGS reg. It was ok for inheritance in EBB which creates only moves and they usually
> have no embedded hard regs in the patterns. But LRA rematerialization needs this. So this patch
> implements such hard reg liveness updates.
>
> The patch was successfully bootstrapped and tested on x86-64.
>
> Committed as rev. 253656.

It looks that this patch caused libgo build bootstrap failure on
x86_64-linux-gnu:

../../../git/gcc/libgo/go/cmd/go/internal/load/pkg.go:377:1: error:
this is the insn:
(insn 1134 2835 2611 195 (parallel [
            (set (reg:DI 492)
                (const_int 0 [0]))
            (set (reg/f:DI 1008 [orig:490 <retval> ] [490])
                (plus:DI (ashift:DI (reg:DI 492)
                        (const_int 3 [0x3]))
                    (reg/f:DI 1008 [orig:490 <retval> ] [490])))
            (set (reg/f:DI 1009 [880])
                (plus:DI (ashift:DI (reg:DI 492)
                        (const_int 3 [0x3]))
                    (reg/f:DI 1009 [880])))
            (set (mem:BLK (reg/f:DI 1008 [orig:490 <retval> ] [490])
[22 *_372+0 S1048 A64])
                (mem/c:BLK (reg/f:DI 1009 [880]) [22 GOTMP.492+0 S1048 A128]))
            (use (reg:DI 492))
        ]) "../../../git/gcc/libgo/go/cmd/go/internal/load/pkg.go":679
930 {*rep_movdi_rex64}
     (expr_list:REG_UNUSED (reg/f:DI 1009 [880])
        (expr_list:REG_UNUSED (reg/f:DI 1008 [orig:490 <retval> ] [490])
            (expr_list:REG_UNUSED (reg:DI 492)
                (expr_list:REG_EH_REGION (const_int 1 [0x1])
                    (nil))))))
during RTL pass: reload
../../../git/gcc/libgo/go/cmd/go/internal/load/pkg.go:377:1: internal
compiler error: in assign_by_spills, at lra-assigns.c:1468
0x6b2709 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
        ../../git/gcc/gcc/rtl-error.c:108
0xbe87fa assign_by_spills
        ../../git/gcc/gcc/lra-assigns.c:1468
0xbe94a6 lra_assign()
        ../../git/gcc/gcc/lra-assigns.c:1662
0xbe48d4 lra(_IO_FILE*)
        ../../git/gcc/gcc/lra.c:2450
0xb9ec51 do_reload
        ../../git/gcc/gcc/ira.c:5440
0xb9ec51 execute
        ../../git/gcc/gcc/ira.c:5624
Please submit a full bug report,

Uros.

patch to fix PR82353

[Vladimir Makarov]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

This is another version of the patch to fix

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353

The patch was successfully bootstrapped on x86-64 with Go and Ada.

Committed as rev. 253796.



[-- Attachment #2: pr82353-2.patch --]
[-- Type: text/x-patch, Size: 2766 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 253795)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@
+2017-10-16  Vladimir Makarov  <vmakarov@redhat.com>
+
+	PR sanitizer/82353
+	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
+	locations.
+	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
+	(make_hard_regno_born, make_hard_regno_dead): Update
+	bb_killed_pseudos and bb_gen_pseudos for fixed regs.
+
 2017-10-16  Jeff Law  <law@redhat.com>
 
 	* tree-ssa-dse.c (live_bytes_read): Fix thinko.
Index: lra.c
===================================================================
--- lra.c	(revision 253685)
+++ lra.c	(working copy)
@@ -820,7 +820,8 @@ collect_non_operand_hard_regs (rtx *x, l
   const char *fmt = GET_RTX_FORMAT (code);
 
   for (i = 0; i < data->insn_static_data->n_operands; i++)
-    if (x == data->operand_loc[i])
+    if (! data->insn_static_data->operand[i].is_operator
+	&& x == data->operand_loc[i])
       /* It is an operand loc. Stop here.  */
       return list;
   for (i = 0; i < data->insn_static_data->n_dups; i++)
Index: lra-lives.c
===================================================================
--- lra-lives.c	(revision 253685)
+++ lra-lives.c	(working copy)
@@ -220,6 +220,9 @@ lra_intersected_live_ranges_p (lra_live_
   return false;
 }
 
+/* The corresponding bitmaps of BB currently being processed.  */
+static bitmap bb_killed_pseudos, bb_gen_pseudos;
+
 /* The function processing birth of hard register REGNO.  It updates
    living hard regs, START_LIVING, and conflict hard regs for living
    pseudos.  Conflict hard regs for the pic pseudo is not updated if
@@ -243,6 +246,8 @@ make_hard_regno_born (int regno, bool ch
 	|| i != REGNO (pic_offset_table_rtx))
 #endif
       SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
+  if (fixed_regs[regno])
+    bitmap_set_bit (bb_gen_pseudos, regno);
 }
 
 /* Process the death of hard register REGNO.  This updates
@@ -255,6 +260,11 @@ make_hard_regno_dead (int regno)
     return;
   sparseset_set_bit (start_dying, regno);
   CLEAR_HARD_REG_BIT (hard_regs_live, regno);
+  if (fixed_regs[regno])
+    {
+      bitmap_clear_bit (bb_gen_pseudos, regno);
+      bitmap_set_bit (bb_killed_pseudos, regno);
+    }
 }
 
 /* Mark pseudo REGNO as living at program point POINT, update conflicting
@@ -299,9 +309,6 @@ mark_pseudo_dead (int regno, int point)
     }
 }
 
-/* The corresponding bitmaps of BB currently being processed.  */
-static bitmap bb_killed_pseudos, bb_gen_pseudos;
-
 /* Mark register REGNO (pseudo or hard register) in MODE as live at
    program point POINT.  Update BB_GEN_PSEUDOS.
    Return TRUE if the liveness tracking sets were modified, or FALSE

Re: patch to fix PR82353

[Tom de Vries]

On 10/16/2017 10:38 PM, Vladimir Makarov wrote:
> This is another version of the patch to fix
> 
>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
> 
> The patch was successfully bootstrapped on x86-64 with Go and Ada.
> 
> Committed as rev. 253796.

Hi Vladimir,

AFAIU this bit of the patch makes sure that the flags register show up 
in the bb_livein of the bb in which it's used (and not defined before 
the use), but not in the bb_liveout of the predecessors of that bb.

I wonder if that's a compile-speed optimization, or an oversight.

[ I ran into a similar problem for target gcn here (
   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327 ) with the
   spill_class hook. I've posted a tentative fix in that PR, which
   piggybacks on this fix, but needed a few extra bits to make sure that
   inter-bb propagation was done:
- a bit at the end of process_bb_lives to detect the liveness change and
   then set live_change_p which make sure the propagation is run.
- a bit in lra_create_live_ranges_1 to unmask the registers we want to
   propagate in all_hard_regs_bitmap.
]

Thanks,
- Tom

> Index: lra-lives.c
> ===================================================================
> --- lra-lives.c	(revision 253685)
> +++ lra-lives.c	(working copy)
> @@ -220,6 +220,9 @@ lra_intersected_live_ranges_p (lra_live_
>     return false;
>   }
>   
> +/* The corresponding bitmaps of BB currently being processed.  */
> +static bitmap bb_killed_pseudos, bb_gen_pseudos;
> +
>   /* The function processing birth of hard register REGNO.  It updates
>      living hard regs, START_LIVING, and conflict hard regs for living
>      pseudos.  Conflict hard regs for the pic pseudo is not updated if
> @@ -243,6 +246,8 @@ make_hard_regno_born (int regno, bool ch
>   	|| i != REGNO (pic_offset_table_rtx))
>   #endif
>         SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
> +  if (fixed_regs[regno])
> +    bitmap_set_bit (bb_gen_pseudos, regno);
>   }
>   
>   /* Process the death of hard register REGNO.  This updates
> @@ -255,6 +260,11 @@ make_hard_regno_dead (int regno)
>       return;
>     sparseset_set_bit (start_dying, regno);
>     CLEAR_HARD_REG_BIT (hard_regs_live, regno);
> +  if (fixed_regs[regno])
> +    {
> +      bitmap_clear_bit (bb_gen_pseudos, regno);
> +      bitmap_set_bit (bb_killed_pseudos, regno);
> +    }
>   }
>   
>   /* Mark pseudo REGNO as living at program point POINT, update conflicting
> @@ -299,9 +309,6 @@ mark_pseudo_dead (int regno, int point)
>       }
>   }
>   
> -/* The corresponding bitmaps of BB currently being processed.  */
> -static bitmap bb_killed_pseudos, bb_gen_pseudos;
> -
>   /* Mark register REGNO (pseudo or hard register) in MODE as live at
>      program point POINT.  Update BB_GEN_PSEUDOS.
>      Return TRUE if the liveness tracking sets were modified, or FALSE
> 

Re: patch to fix PR82353

[Vladimir Makarov]

On 12/13/2017 07:34 AM, Tom de Vries wrote:
> On 10/16/2017 10:38 PM, Vladimir Makarov wrote:
>> This is another version of the patch to fix
>>
>>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
>>
>> The patch was successfully bootstrapped on x86-64 with Go and Ada.
>>
>> Committed as rev. 253796.
>
> Hi Vladimir,
>
> AFAIU this bit of the patch makes sure that the flags register show up 
> in the bb_livein of the bb in which it's used (and not defined before 
> the use), but not in the bb_liveout of the predecessors of that bb.
>
> I wonder if that's a compile-speed optimization, or an oversight.
>
Hi, Tom.  It was just a minimal fix.  I prefer minimal fixes for LRA 
because even for me it is hard to predict in many cases how the patch 
will affect all the targets.  Therefore many LRA patches have a few 
iterations before to be final.

I remember that I had some serious problems in the past when I tried to 
implement fixed hard reg liveness propagation in LRA.  It was long ago 
so we could try it again.  If you send patch you mentioned to gcc 
mailing list, I'll review and approve it.  But we need to be ready to 
revert it if some problems occur again.