[Bug target/60071] New: [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[Bug target/60071] New: [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[kkojima at gcc dot gnu.org]

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

            Bug ID: 60071
           Summary: [4.9 Regression] [SH] internal compiler error: in
                    final_scan_insn, at final.c:2963
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kkojima at gcc dot gnu.org
                CC: olegendo at gcc dot gnu.org
            Target: sh*-*-*

SH fails to build on trunk with

../../../ORIG/trunk/libgcc/fp-bit.c:1437:1: error: could not split insn
 }
 ^
(insn 119 38 83 (parallel [
            (set (reg:SI 0 r0 [orig:160 D.2909 ] [160])
                (minus:SI (const_int -2147483648 [0xffffffff80000000])
                    (reg:SI 147 t)))
            (clobber (reg:SI 147 t))
        ]) ../../../ORIG/trunk/libgcc/fp-bit.c:1429 404 {*mov_t_msb_neg}
     (expr_list:REG_DEAD (reg:SI 147 t)
        (expr_list:REG_UNUSED (reg:SI 147 t)
            (nil))))
../../../ORIG/trunk/libgcc/fp-bit.c:1437:1: internal compiler error: in
final_scan_insn, at final.c:2963
0x859edbf _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        ../../ORIG/trunk/gcc/rtl-error.c:109

during building libgcc after the revision

r207460 | jakub | 2014-02-04 21:14:52 +0900 (Tue, 04 Feb 2014) | 6 lines

        PR rtl-optimization/57915
        * recog.c (simplify_while_replacing): If all unary/binary/relational
        operation arguments are constant, attempt to simplify those.

Here is a reduced test case for -O1:

int
foo (int class, int sign)
{
  if (__builtin_expect (class == 4, 0))
    return (sign
            ? (-((int) ((~(unsigned)0) >> 1)))-1
            : ((int) ((~(unsigned)0) >> 1)));
}

It looks that now *mov_t_msb_neg define_insn_and_split is produced
in ira pass and has no chance to split.

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[rguenth at gcc dot gnu.org]

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

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

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

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'd say it is a backend bug.  Canonicalizing (minus:SI (const_int INT_MIN))
into (const_int INT_MIN) is IMNSHO very much desirable, it is just a target bug
that it matches an insn with it at the point where it no longer is able to cope
with it.  So, either it needs to use some predicate which will make such insn
invalid right after split1, or it needs to cope also with the case where during
splitting it can't create pseudos.

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note the case RTX_UNARY: handling in simplify_while_replacing wasn't strictly
necessary to fix the bug, so in theory that can be temporarily reverted, but
that doesn't change anything on this being a target bug.

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[olegendo at gcc dot gnu.org]

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

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I'll have a look.

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[kkojima at gcc dot gnu.org]

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

--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Oleg, any news on this?

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[olegendo at gcc dot gnu.org]

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

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Sorry, not yet, but it's on my desk/mind.  I'll try to come up with something
this week.  In the worst case we'll have to remove the problematic patterns, I
guess.

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-02-25
     Ever confirmed|0                           |1

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I've just tried this with -O2.  This is what happens...

After combine, there is the problematic insn:

(insn 41 40 34 3 (parallel [
            (set (reg:SI 161 [ <retval> ])
                (minus:SI (const_int -2147483648 [0xffffffff80000000])
                    (reg:SI 147 t)))
            (clobber (reg:SI 147 t))
        ]) sh_tmp.cpp:10 404 {*mov_t_msb_neg}
     (nil))


After split1 it is split correctly into constant load + negc and before the IRA
pass it looks like this:

(insn 45 40 46 4 (set (reg:SI 169)
        (const_int -2147483648 [0xffffffff80000000])) sh_tmp.cpp:10 257
{movsi_ie}
     (nil))
(insn 46 45 43 4 (parallel [
            (set (reg:SI 161 [ <retval> ])
                (minus:SI (neg:SI (reg:SI 169))
                    (reg:SI 147 t)))
            (clobber (reg:SI 147 t))
        ]) sh_tmp.cpp:10 205 {*negc}
     (nil))


However, the IRA pass moves the constant back and replaces the insn:

(insn 46 40 43 4 (parallel [
            (set (reg:SI 161 [ <retval> ])
                (minus:SI (const_int -2147483648 [0xffffffff80000000])
                    (reg:SI 147 t)))
            (clobber (reg:SI 147 t))
        ]) sh_tmp.cpp:10 404 {*mov_t_msb_neg}

which happens while can_create_pseudo_p () returns 'true', thus the insn
matches.  This is a bit counter productive.  The constant load was split out on
purpose so that it can be hoisted/combined easier (I was thinking of adding an
SH specific RTL pass to optimize constant loads and sharing.  On SH there are a
couple of cases where constants are emitted during combine and split1 which
won't be CSE'd).


The following seems to fix the problem (although not fully tested):

Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md    (revision 208155)
+++ gcc/config/sh/sh.md    (working copy)
@@ -11434,6 +11434,10 @@
 ;;    T = 1: 0x80000000 -> reg
 ;;    T = 0: 0x7FFFFFFF -> reg
 ;; This works because 0 - 0x80000000 = 0x80000000.
+;;
+;; This insn must not match again after it has been split into the constant
+;; load and negc.  This is accomplished by the special negc insn that
+;; has a use on the operand.
 (define_insn_and_split "*mov_t_msb_neg"
   [(set (match_operand:SI 0 "arith_reg_dest")
     (minus:SI (const_int -2147483648)  ;; 0x80000000
@@ -11444,12 +11448,23 @@
   "&& can_create_pseudo_p ()"
   [(set (match_dup 2) (const_int -2147483648))
    (parallel [(set (match_dup 0) (minus:SI (neg:SI (match_dup 2))
-                 (reg:SI T_REG)))
-          (clobber (reg:SI T_REG))])]
+                        (reg:SI T_REG)))
+          (clobber (reg:SI T_REG))
+          (use (match_dup 2))])]
 {
   operands[2] = gen_reg_rtx (SImode);
 })

+(define_insn "*mov_t_msb_neg_negc"
+  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
+    (minus:SI (neg:SI (match_operand:SI 1 "arith_reg_operand" "r"))
+          (match_operand:SI 2 "t_reg_operand")))
+   (clobber (reg:SI T_REG))
+   (use (match_dup 1))]
+  "TARGET_SH1"
+  "negc    %1,%0"
+  [(set_attr "type" "arith")])
+
 ;; These are essentially the same as above, but with the inverted T bit.
 ;; Combine recognizes the split patterns, but does not take them sometimes
 ;; if the T_REG clobber is specified.  Instead it tries to split out the

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[kkojima at gcc dot gnu.org]

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

--- Comment #7 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
A testresult with the patch in #6 on sh4-unknown-linux-gnu:
http://gcc.gnu.org/ml/gcc-testresults/2014-02/msg01866.html

[Bug target/60071] [4.9 Regression] [SH] internal compiler error: in final_scan_insn, at final.c:2963

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Fixed on trunk (4.9).