[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-29 18:30:15 UTC ---
I'd say the arm backend just shouldn't ignore the IL restrictions and simply
apply a patch like in comment 8.  shared_const_p is just an exception for the
most frequently used CONST, we really shouldn't agree to share arbitrarily
complex CONST expressions, that will just leads to lots of bugs everywhere.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.2                       |4.5.3

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-16 13:03:25 UTC ---
GCC 4.5.2 is being released, adjusting target milestone.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #16 from Jeffrey A. Law <law at redhat dot com> 2011-01-17 17:41:05 UTC ---
I agree with Jakub -- let's keep the set of CONST expressions which can be
shared small, well defined and limited to those which make a significant
difference in memory usage, code maintainability, etc.

I really don't see much to be gained by allowing these more generic forms --
and they run the real risk of causing bugs.  Specifically, if we allow the
additional shared forms, then the middle end has to be prepared to deal with
fairly arbitrary CONST sharing created by the backends.  Getting this wrong
results in wrong-code, segfaults and all kinds of ugly and hard to diagnose
bugs.

I guess the simplest way to summarize how I look at this is we need a
compelling reason to expand the set of shared CONST nodes.

I'd like to see Andrey's patch go in along with some updated documentation on
what CONST nodes can be shared.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-26 13:21:46 UTC ---
Simplified testcase:

extern __thread int t;

void
foo (int a)
{
  t = a;
}

Looking at it in detail, not sure if the bug isn't earlier.
Seems the expander isn't extra careful about rtl sharing issues and relies on
subsequent unsharing of everything.  Except that unshare_all_rtl doesn't look
at constant pool expressions.

So, a fix could be something like:
--- expr.c.jj  2011-01-18 08:44:29.000000000 +0100
+++ expr.c      2011-01-26 13:29:34.644796679 +0100
@@ -3398,7 +3398,7 @@ emit_move_insn (rtx x, rtx y)
       && (set = single_set (last_insn)) != NULL_RTX
       && SET_DEST (set) == x
       && ! rtx_equal_p (y_cst, SET_SRC (set)))
-    set_unique_reg_note (last_insn, REG_EQUAL, y_cst);
+    set_unique_reg_note (last_insn, REG_EQUAL, copy_rtx (y_cst));

   return last_insn;
 }

or e.g.:
--- varasm.c.jj        2011-01-25 12:58:41.000000000 +0100
+++ varasm.c    2011-01-26 14:07:50.635389932 +0100
@@ -3518,7 +3518,7 @@ force_const_mem (enum machine_mode mode,
   pool->offset &= ~ ((align / BITS_PER_UNIT) - 1);

   desc->next = NULL;
-  desc->constant = tmp.constant;
+  desc->constant = copy_rtx (tmp.constant);
   desc->offset = pool->offset;
   desc->hash = hash;
   desc->mode = mode;

(the latter is IMHO better).  Both fix this ICE. Perhaps if
avoid_constant_pool_reference callers want to use the result in insn stream
they need to unshare it again (at least if they use it more than once), not
sure if avoid_constant_pool_reference itself shoudl do the copy_rtx or just if
callers should take care of it.  Jeff, what do you think about this?

I've tried if copy_rtx_if_shared couldn't be itself taught about unsharing the
constants, i.e. that we wouldn't unnecessarily unshare if it wasn't used
anywhere else, but it gets quite complicated because SYMBOL_REFs should be
shared, yet we want to recurse just once on their &SYMBOL_REF_CONSTANT
(x)->constant for CONSTANT_POOL_REFERENCE_P.  In copy_rtx_if_shared it can be
coded, but not sure what to do about mark_used_flags.  I think the varasm.c
patch above shouldn't be a big deal, most constants in the constant pool can be
actually shared and thus copy_rtx on them is a nop.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-26 15:34:38 UTC ---
Created attachment 23133
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23133
gcc46-pr42894.patch

The varasm.c alternative (untested).

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-26 20:04:55 UTC ---
Fixed up patch posted:
http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01916.html
(one } was missing in the testcase).

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-28 19:36:47 UTC ---
Author: jakub
Date: Fri Jan 28 19:36:44 2011
New Revision: 169370

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169370
Log:
    PR target/42894
    * varasm.c (force_const_mem): Store copy of x in desc->constant
    instead of x itself.
    * expr.c (emit_move_insn): Add a copy of y_cst instead of y_cst
    itself into REG_EQUAL note.

    * gcc.dg/tls/pr42894.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/tls/pr42894.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/varasm.c

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[richard.earnshaw at arm dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #21 from richard.earnshaw at arm dot com 2011-01-28 19:37:17 UTC ---
I shall be out of the office until Monday 28 February; I will not have email
access during most of that time, so I will read your message when I return. 
For managerial issues please contact Roger Teague.  For internal engineering
issues please contact Matthew Gretton-Dann.

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.5/4.6 Regression]        |[4.5 Regression] Invalid
                   |Invalid rtl sharing in      |rtl sharing in Thumb1.
                   |Thumb1.                     |

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-01-28 19:39:12 UTC ---
Should be fixed on the trunk now.

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[dnovillo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #23 from Diego Novillo <dnovillo at gcc dot gnu.org> 2011-02-02 18:00:37 UTC ---
Author: dnovillo
Date: Wed Feb  2 18:00:29 2011
New Revision: 169672

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169672
Log:
    PR target/42894
    * varasm.c (force_const_mem): Store copy of x in desc->constant
    instead of x itself.
    * expr.c (emit_move_insn): Add a copy of y_cst instead of y_cst
    itself into REG_EQUAL note.

    * gcc.dg/tls/pr42894.c: New test.

Added:
    branches/google/integration/gcc/testsuite/gcc.dg/tls/pr42894.c
Modified:
    branches/google/integration/gcc/ChangeLog
    branches/google/integration/gcc/expr.c
    branches/google/integration/gcc/testsuite/ChangeLog
    branches/google/integration/gcc/varasm.c

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.3                       |4.5.4

--- Comment #24 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-28 14:51:39 UTC ---
GCC 4.5.3 is being released, adjusting target milestone.

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ramana at gcc dot gnu.org

--- Comment #25 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2011-07-08 15:28:08 UTC ---
Jakub,

While this fixes it for 4.6 should this also be backported to the 4.5 branch ?

Ramana

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-07-08 15:35:01 UTC ---
It could, but I don't have time to do it myself.  So, if somebody goes ahead
and tests the backport, fine.

[Bug target/42894] [4.5 Regression] Invalid rtl sharing in Thumb1.

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.5.4                       |4.6.0

--- Comment #27 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-07-02 10:26:16 UTC ---
Fixed in 4.6.0, the 4.5 branch is being closed.

[Bug target/42894] [4.5/4.6 Regression] Invalid rtl sharing in Thumb1.

[rguenth at gcc dot gnu dot org]

------- Comment #13 from rguenth at gcc dot gnu dot org  2010-07-31 09:29 -------
GCC 4.5.1 is being released, adjusting target milestone.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.1                       |4.5.2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42894