[Bug rtl-optimization/43804] New: [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[Bug rtl-optimization/43804] New: [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

$ gcc/xgcc -Bgcc/ -O2 -fpic -S memusage.i 
memusage.c: In function ‘me’:
memusage.c:311:1: error: insn does not satisfy its constraints:
(insn 342 136 137 10 memusage.c:253 (set (reg/f:SI 2 %d2 [137])
        (const:SI (unspec:SI [
                    (symbol_ref:SI ("start_sp") [flags 0x1a] <var_decl
0x40322ea0 start_sp>)
                    (const_int 2 [0x2])
                ] 6))) 36 {*movsi_m68k} (nil))
memusage.c:311:1: internal compiler error: in reload_cse_simplify_operands, at
postreload.c:396

Caused by:

commit e38def9ca7953bb5611d08ce8617249516ba5a99
Author: rth <rth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Sep 14 19:18:58 2009 +0000

    Squash commit of EH in gimple

    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@151696
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 75333c1..cc04831 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,343 @@
+2009-09-14  Richard Henderson  <rth@redhat.com>
+
+       * except.h: Update declarations.
+       (struct pointer_map_t): Forward declare.
+       (ERT_UNKNOWN, ERT_THROW, ERT_CATCH): Remove.


-- 
           Summary: [4.5/4.6 regression] ICE in reload_cse_simplify_operands
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: schwab at linux-m68k dot org
GCC target triplet: m68k-linux-*


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #1 from schwab at linux-m68k dot org  2010-04-19 20:13 -------
Created an attachment (id=20424)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20424&action=view)
Preprocessed testcase


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #2 from schwab at linux-m68k dot org  2010-04-19 20:14 -------
Created an attachment (id=20425)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20425&action=view)
Reduced testcase


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #3 from mkuvyrkov at gcc dot gnu dot org  2010-04-21 10:47 -------
There are two problems here:
1. The immediate problem is that "movsi_m68k" doesn't allow (const) to be moved
to data register (see last alternative if movsi_m68k).

2. For reason, which I have not yet investigated, no alternative in
"addsi_internal" match
(set (reg) (plus (reg %a5) (const (unspec))))

Andreas, do you know why movsi_m68k doesn't allow data register in the last
alternative?

(define_insn "*movsi_m68k"
  [(set (match_operand:SI 0 "nonimmediate_operand" "=g,d,a<")
        (match_operand:SI 1 "general_src_operand" "damSnT,n,i"))]


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #4 from schwab at linux-m68k dot org  2010-04-21 13:22 -------
Introduced in r27576.  Presumably the first alternative is supposed to catch
this case, through T+g, but T does not match if flag_pic, contrary to its
documentation (introduced in r120961).


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[maxim at codesourcery dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]



------- Comment #5 from mkuvyrkov at gcc dot gnu dot org  2010-04-21 19:43 -------
Subject: Re:  [4.5/4.6 regression] ICE in reload_cse_simplify_operands

On 4/21/10 5:23 PM, schwab at linux-m68k dot org wrote:
> ------- Comment #4 from schwab at linux-m68k dot org  2010-04-21 13:22 -------
> Introduced in r27576.  Presumably the first alternative is supposed to catch
> this case, through T+g, but T does not match if flag_pic, contrary to its
> documentation (introduced in r120961).

Documentation for "T" seems correct to me, assuming -mpcrel and -fpic 
are equivalent: "Operands that satisfy ’s’ (const, symbol_ref, ...) when 
-mpcrel is not in effect".

It is clear that constants with unknown at compile time values are not 
welcome in data registers, but I cannot guess why.  What can possibly be 
the difference between an arbitrary constant known at compile time (the 
"n" constraint) and a constant known at assembly / link time (the "i" 
constraint).


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #6 from schwab at linux-m68k dot org  2010-04-21 22:51 -------
(In reply to comment #5)
> assuming -mpcrel and -fpic are equivalent:

They aren't.  -mpcrel implies -fpic, but not the other way round.


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #7 from schwab at linux-m68k dot org  2010-04-21 23:40 -------
I think 'T' should accept tls references like 's' does.


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[maxim at codesourcery dot com]

------- Comment #8 from mkuvyrkov at gcc dot gnu dot org  2010-04-22 07:09 -------
Subject: Re:  [4.5/4.6 regression] ICE in reload_cse_simplify_operands

On 4/22/10 3:40 AM, schwab at linux-m68k dot org wrote:
> ------- Comment #7 from schwab at linux-m68k dot org  2010-04-21 23:40 -------
> I think 'T' should accept tls references like 's' does.

Do you mean the following?
==
(define_constraint "T"
   "Used for operands that satisfy 's' when -mpcrel is not in effect."
   (and (match_code "symbol_ref,label_ref,const")
        (match_test "!flag_pic || m68k_tls_reference_p (op, true)")))
==

Yes, that should fix this particular problem, but one could equally 
successfully argue that "T" then should then accept usual PIC GOT 
references too.

I just do not understand the background behind "T" constraint.  Sigh.

Maybe, it was introduced as an optimization to stop GCC from putting 
addresses (symbol_refs, etc) into data registers and then reloading them 
into address registers.  Most probably, current GCC register allocator 
will do a just that without this kind of manual tweaking on the backend 
side.


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #9 from schwab at linux-m68k dot org  2010-04-22 09:20 -------
Probably the author of T didn't realise that !-mpcrel in (!-mpcrel && 's') is
already implied.  It is already a no-op in the -mpcrel case.


-- 


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

[Bug rtl-optimization/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #10 from mkuvyrkov at gcc dot gnu dot org  2010-04-23 10:20 -------
The problem seems to be in Richard's patch
(http://article.gmane.org/gmane.comp.gcc.patches/130602) checked in r120961.

All and all, it seems that revision 120961 should be reverted to enable 'T'
constraint for (flag_pic && !TARGET_PCREL).

The 's' constraint is defined as
==
              case 's':
                if (CONST_INT_P (op)
                    || (GET_CODE (op) == CONST_DOUBLE
                        && GET_MODE (op) == VOIDmode))
                  break;
              case 'i':
                if (CONSTANT_P (op))
                  win = 1;
                break;
==

So, unless I'm missing something, the statement
"... 's' doesn't accept anything if flag_pic"
is just wrong.

FYI, the story behind 'T' constraint is described in comment in r27576:

+   In parallel with these new predicates, two new constraint letters
+   were defined: 'S' and 'T'.  'S' is the -mpcrel analog of 'm'.
+   'T' replaces 's' in the non-pcrel case.  It is a no-op in the pcrel case.
+   In the pcrel case 's' is only valid in combination with 'a' registers.
+   See addsi3, subsi3, cmpsi, and movsi patterns for a better understanding
+   of how these constraints are used.
+
...
+   All of the ugliness with predicates and constraints is due to the
+   simple fact that the m68k does not allow a pc-relative addressing
+   mode as a destination.  gcc does not distinguish between source and
+   destination addresses.  Hence, if we claim that pc-relative address
+   modes are valid, e.g. GO_IF_LEGITIMATE_ADDRESS accepts them, then we
+   end up with invalid code.  To get around this problem, we left
+   pc-relative modes as invalid addresses, and then added special
+   predicates and constraints to accept them.


-- 

mkuvyrkov at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rdsandiford at googlemail
                   |                            |dot com


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #11 from mkuvyrkov at gcc dot gnu dot org  2010-05-02 18:02 -------
Created an attachment (id=20536)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20536&action=view)
Revert previous patch


-- 


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[mkuvyrkov at gcc dot gnu dot org]

------- Comment #12 from mkuvyrkov at gcc dot gnu dot org  2010-05-02 18:03 -------
Ping.

Richard S.,
Andreas,

Any comment on the above analysis?

AFAICT, the "T" constraint should accept operands if "flag_pic &&
!TARGET_PCREL".


-- 


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rdsandiford at googlemail dot com]

------- Comment #13 from rdsandiford at googlemail dot com  2010-05-03 09:53 -------
Subject: Re:  [4.5/4.6 regression] ICE in reload_cse_simplify_operands

"mkuvyrkov at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:
> ------- Comment #10 from mkuvyrkov at gcc dot gnu dot org  2010-04-23 10:20 -------
> The problem seems to be in Richard's patch
> (http://article.gmane.org/gmane.comp.gcc.patches/130602) checked in r120961.
>
> All and all, it seems that revision 120961 should be reverted to enable 'T'
> constraint for (flag_pic && !TARGET_PCREL).
>
> The 's' constraint is defined as
> ==
>               case 's':
>                 if (CONST_INT_P (op)
>                     || (GET_CODE (op) == CONST_DOUBLE
>                         && GET_MODE (op) == VOIDmode))
>                   break;
>               case 'i':
>                 if (CONSTANT_P (op))
>                   win = 1;
>                 break;
> ==
>
> So, unless I'm missing something, the statement
> "... 's' doesn't accept anything if flag_pic"
> is just wrong.

I meant "flag_pic && !TARGET_PCREL", since only the !TARGET_PCREL case
matters for 'T'.  And that was right at the time that I wrote it.
The interesting definition of 's' isn't the one you quote, but the one
in reload:

              case 's':
                if (CONST_INT_P (operand)
                    || (GET_CODE (operand) == CONST_DOUBLE
                        && GET_MODE (operand) == VOIDmode))
                  break;
              case 'i':
                if (CONSTANT_P (operand)
                    && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (operand)))
                  win = 1;
                break;

That is, 's' operands have to satisfy LEGITIMATE_PIC_OPERAND_P when
generating PIC.  I'm not sure which version you were quoting, but if it
was constrain_operands, that's a special case.  constrain_operands can
rely on the predicates to check for legitimate PIC operands, so there's
no need to repeat the check there.

At the time I committed the patch, LEGITIMATE_PIC_OPERAND_P was
defined as follows:

#define LEGITIMATE_PIC_OPERAND_P(X)                             \
  (!symbolic_operand (X, VOIDmode)                              \
   || (TARGET_PCREL && REG_STRICT_P))

Thus no symbolic constant was a legitimate PIC operand for
flag_pic && !TARGET_PCREL.  Thus nothing satisfied 's' when
flag_pic && !TARGET_PCREL.

The 'T' constraint is defined as follows:

  satisifies(T) == satisifies(s) && !TARGET_PCREL

so it followed that nothing should match 'T' when flag_pic.

So the patch was correct at the time it was committed.  Please
understand that reverting it is the wrong thing to do.  It would
reintroduce the original bug: that constraints _must not_ match
something that the associated predicates do not.  Only the other
way is allowed: predicates can allow things that the constraints
don't, within certain limits.

For example, let's say you have an insn that matches:

(define_insn "subsi3"
  [(set (match_operand:SI 0 "nonimmediate_operand" "=mda,m,d,a")
        (minus:SI (match_operand:SI 1 "general_operand" "0,0,0,0")
                  (match_operand:SI 2 "general_src_operand"
"I,dT,mSrT,mSrs")))]
  ""
  "@
   subq%.l %2, %0
   sub%.l %2,%0
   sub%.l %2,%0
   sub%.l %2,%0"
  [(set_attr "type" "aluq_l,alu_l,alu_l,alu_l")
   (set_attr "opy" "2")])

And let's suppose that operand 2 is a register that is equal to:

  (symbol_ref "x")

If 'T' allows any CONST, SYMBOL_REF or LABEL_REF when flag_pic &&
!TARGET_PCREL (as in your suggested patch), reload could quite happily
establish that operand 2 is (symbol_ref "x"), see that it matches 'T',
and use it.  This will then lead to an unrecognisable insn, because
although the constant matches the 'T' constraint, it doesn't match
general_src_operand.

Instead, the bug is that the 'T' constraint wasn't updated by the TLS
support at the same time as LEGITIMATE_PIC_OPERAND_P was.  An easy thing
to miss, of course.  I think the correct patch is the one I'm about
to attach.

Richard


-- 


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rsandifo at gcc dot gnu dot org]

------- Comment #14 from rsandifo at gcc dot gnu dot org  2010-05-03 09:55 -------
Created an attachment (id=20541)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=20541&action=view)
proposed patch


-- 


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[schwab at linux-m68k dot org]

------- Comment #15 from schwab at linux-m68k dot org  2010-05-03 20:17 -------
The patch is ok, please check it in.


-- 


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rsandifo at gcc dot gnu dot org]

------- Comment #16 from rsandifo at gcc dot gnu dot org  2010-05-10 19:49 -------
Thanks Andreas.  I don't have access to m68k-elfy targets these days, so could
someone test it just to be sure?  I'll commit if everything goes OK.


-- 

rsandifo at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rsandifo at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-05-10 19:49:34
               date|                            |


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4


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

[Bug target/43804] [4.5/4.6 regression] ICE in reload_cse_simplify_operands

[rguenth at gcc dot gnu dot org]

------- Comment #17 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=43804