[Bug target/50762] New: ICE: in extract_insn, at recog.c:2137 (unrecognizable insn) with -mavx2 and __builtin_shuffle

[Bug target/50762] New: ICE: in extract_insn, at recog.c:2137 (unrecognizable insn) with -mavx2 and __builtin_shuffle

[zsojka at seznam dot cz]

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

             Bug #: 50762
           Summary: ICE: in extract_insn, at recog.c:2137 (unrecognizable
                    insn) with -mavx2 and __builtin_shuffle
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: zsojka@seznam.cz
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu


Created attachment 25526
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25526
reduced testcase

Compiler output:
$ gcc -mavx2 testcase.c 
testcase.c: In function 'foo':
testcase.c:11:1: error: unrecognizable insn:
(insn 17 16 18 3 (set (reg:V4DF 62 [ v.3 ])
        (unspec:V4DF [
                (reg:V8SI 65)
                (reg:V8SI 66)
                (reg:V4DF 62 [ v.3 ])
            ] UNSPEC_BLENDV)) testcase.c:10 -1
     (nil))
testcase.c:11:1: internal compiler error: in extract_insn, at recog.c:2137
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

Tested revisions:
r180072 - crash
4.6 - doesn't recognise -mavx2

[Bug target/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn) with -mavx2 and __builtin_shuffle

[regehr at cs dot utah.edu]

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

John Regehr <regehr at cs dot utah.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |regehr at cs dot utah.edu

--- Comment #1 from John Regehr <regehr at cs dot utah.edu> 2011-11-08 04:30:50 UTC ---
Same crash, but with vanilla compiler flags and only vanilla C code in the test
case:

[regehr@gamow tmp048]$ current-gcc -O small.c

small.c: In function 'func_59':
small.c:48:1: error: unrecognizable insn:
(insn 29 28 30 3 (set (reg:DI 0 ax [78])
        (zero_extend:DI (const_int 1 [0x1]))) small.c:35 -1
     (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
        (nil)))
small.c:48:1: internal compiler error: in extract_insn, at recog.c:2137
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


[regehr@gamow tmp048]$ current-gcc -v

Using built-in specs.
COLLECT_GCC=current-gcc
COLLECT_LTO_WRAPPER=/uusoc/exports/scratch/regehr/z/compiler-install/gcc-r181147-install/bin/../libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../configure --with-libelf=/usr/local --enable-lto
--prefix=/home/regehr/z/compiler-install/gcc-r181147-install
--program-prefix=r181147- --enable-languages=c,c++
Thread model: posix
gcc version 4.7.0 20111108 (experimental) (GCC) 


[regehr@gamow tmp048]$ cat small.c


int
func_2 (int si1, int si2)
{
  return 0;
}

short
func_9 (int si1, int si2)
{
  return 0;
}

int g_92;
int g_97;
int g_159;
int g_210;
int g_242;
int g_281;
int
func_59 (unsigned p_60)
{
  int l_61[][1][1] = {
  };
lbl_348:for (p_60 = 0; p_60 <= 1; p_60 += 1)
    {
      int l_65[][1] = {
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0x87D1
      };
      int l_243;
      for (;;)
    {
      l_243 = func_9 (0, 0) == l_65[4][5];
      if (l_243)
        break;
      return l_61[p_60 + 1][0][0];
    }
    }
  for (;;)
    {
      if (p_60)
    continue;
      for (;; g_281 = 0)
    if (l_61[0][0][0] = g_92)
      if (func_2 (l_61[0][g_159][0] | g_210, g_97))
        if (g_242)
          goto lbl_348;
    }
}

int
main ()
{
  return 0;
}

[Bug target/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011-11-08
         AssignedTo|unassigned at gcc dot       |ubizjak at gmail dot com
                   |gnu.org                     |
   Target Milestone|---                         |4.7.0
            Summary|ICE: in extract_insn, at    |ICE: in extract_insn, at
                   |recog.c:2137                |recog.c:2137
                   |(unrecognizable insn) with  |(unrecognizable insn)
                   |-mavx2 and                  |
                   |__builtin_shuffle           |
     Ever Confirmed|0                           |1

--- Comment #2 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-08 10:56:28 UTC ---
(In reply to comment #1)
> Same crash, but with vanilla compiler flags and only vanilla C code in the test case:

The crash is the same, but the problem is totally different. In fact, all
unrecognizable insns will crash at this spot.

While the original test is fixed in current mainline (and also covered by the
testsuite), the test from Comment #1 still fails.

I have updated the summary, but please do not hijack PRs.

Looking into it.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ra
             Status|ASSIGNED                    |NEW
                 CC|                            |bernds at gcc dot gnu.org,
                   |                            |uweigand at gcc dot gnu.org
          Component|target                      |rtl-optimization
         AssignedTo|ubizjak at gmail dot com    |unassigned at gcc dot
                   |                            |gnu.org

--- Comment #3 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-08 11:16:42 UTC ---
This is reload failure, while propagating constant 0:

(insn 4 15 52 2 (set (reg/v:SI 59 [ p_60 ])
        (const_int 0 [0])) tt.c:24 64 {*movsi_internal}
     (nil))

...

(insn 29 28 30 3 (set (reg:DI 78)
        (zero_extend:DI (plus:SI (reg/v:SI 59 [ p_60 ])
                (const_int 1 [0x1])))) tt.c:35 250 {*lea_4_zext}
     (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
        (nil)))

to:

tt.c: In function ‘func_59’:
tt.c:48:1: error: unrecognizable insn:
(insn 29 28 30 3 (set (reg:DI 0 ax [78])
        (zero_extend:DI (const_int 1 [0x1]))) tt.c:35 -1
     (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
        (nil)))

backtrace:

#4  0x0000000000824a31 in extract_insn (insn=0x2aaaae8f8f78) at
../../gcc-svn/trunk/gcc/recog.c:2137
#5  0x0000000000840632 in find_reloads (insn=0x2aaaae8f8f78, replace=0,
ind_levels=0, live_known=1, 
    reload_reg_p=0x14c63a0) at ../../gcc-svn/trunk/gcc/reload.c:2663
#6  0x0000000000843174 in find_reloads (insn=0x2aaaae8f8f78, replace=0,
ind_levels=0, live_known=1, 
    reload_reg_p=0x14c63a0) at ../../gcc-svn/trunk/gcc/reload.c:2815
#7  0x000000000084e543 in calculate_needs_all_insns (global=1) at
../../gcc-svn/trunk/gcc/reload1.c:1525
#8  0x0000000000851ed8 in reload (first=0x2aaaae9039c0, global=1) at
../../gcc-svn/trunk/gcc/reload1.c:945
#9  0x000000000079984d in ira () at ../../gcc-svn/trunk/gcc/ira.c:3726
#10 rest_of_handle_ira () at ../../gcc-svn/trunk/gcc/ira.c:3799

Unassigning myself, adding some CCs.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #4 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-08 16:00:36 UTC ---
It seems to me (part of) the problem is that the operand constraint is too
generic here:

(define_insn "*lea_4_zext"
  [(set (match_operand:DI 0 "register_operand" "=r")
        (zero_extend:DI
          (match_operand:SI 1 "lea_address_operand" "p")))]

"p" accepts any legitimate address operand, including const_int.  But those are
then not actually accepted by the overall pattern (due to the mode check).

When reloading within an address operand, reload verifies only the constraint;
it does not expect the case where a constraint accepts something that is later
rejected by some other check.

One fix might be to use a more specific constraint here that does not accept
const_int.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[regehr at cs dot utah.edu]

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

--- Comment #5 from John Regehr <regehr at cs dot utah.edu> 2011-11-08 16:13:38 UTC ---
(In reply to comment #2)
> I have updated the summary, but please do not hijack PRs.

I'd be happy to not hijack PRs but you will need to help.

My assumptions have been:

1. Sometimes, a new and simpler test case for a known ICE is useful.  For
example, in the past I have added new test cases to show that a bug for an
unpopular architecture with bizarre flags can also happen on x86 using -O.

2. It is incorrect to always open a new PR for a new test case for a known ICE,
because most of the time someone will simply have to flag it as a duplicate.

If one of these assumptions is incorrect, let me know.  If they are correct,
then I could use some guidance in deciding when to add to an existing PR and
when to create a new one.  Thanks.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #6 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-09 18:30:29 UTC ---
(In reply to comment #4)
> It seems to me (part of) the problem is that the operand constraint is too
> generic here:
> 
> (define_insn "*lea_4_zext"
>   [(set (match_operand:DI 0 "register_operand" "=r")
>         (zero_extend:DI
>           (match_operand:SI 1 "lea_address_operand" "p")))]
> 
> "p" accepts any legitimate address operand, including const_int.  But those are
> then not actually accepted by the overall pattern (due to the mode check).
> 
> When reloading within an address operand, reload verifies only the constraint;
> it does not expect the case where a constraint accepts something that is later
> rejected by some other check.
> 
> One fix might be to use a more specific constraint here that does not accept
> const_int.

Unfortunately this approach doesn't work :(

Using following patch:

--cut here--
Index: i386.md
===================================================================
--- i386.md    (revision 181207)
+++ i386.md    (working copy)
@@ -5560,7 +5560,7 @@
 (define_insn "*lea_4_zext"
   [(set (match_operand:DI 0 "register_operand" "=r")
     (zero_extend:DI
-      (match_operand:SI 1 "lea_address_operand" "p")))]
+      (match_operand:SI 1 "lea_address_operand" "j")))]
   "TARGET_64BIT"
   "lea{l}\t{%a1, %k0|%k0, %a1}"
   [(set_attr "type" "lea")
Index: constraints.md
===================================================================
--- constraints.md    (revision 181207)
+++ constraints.md    (working copy)
@@ -127,6 +127,11 @@
   (and (not (match_test "TARGET_X32"))
        (match_operand 0 "memory_operand")))

+(define_constraint "j"
+  "@internal ."
+  (and (not (match_code "const_int"))
+       (match_operand 0 "address_operand")))
+
 ;; Integer constant constraints.
 (define_constraint "I"
   "Integer constant in the range 0 @dots{} 31, for 32-bit shifts."
--cut here--

results in:

pr50762.c: In function ‘func_59’:
pr50762.c:48:1: error: unable to generate reloads for:
(insn 29 28 30 3 (set (reg:DI 0 ax [77])
        (zero_extend:DI (plus:SI (const_int 0 [0])
                (const_int 1 [0x1])))) pr50762.c:35 250 {*lea_4_zext}
     (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
        (nil)))
pr50762.c:48:1: internal compiler error: in find_reloads, at reload.c:3812
Please submit a full bug report,

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #7 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-09 18:38:12 UTC ---
Redefining "j" constraint as "define_address_constraint" results in:

pr50762.c:48:1: error: unrecognizable insn:
(insn 29 28 30 3 (set (reg:DI 0 ax [77])
        (zero_extend:DI (const_int 1 [0x1]))) pr50762.c:35 -1
     (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
        (nil)))
pr50762.c:48:1: internal compiler error: in extract_insn, at recog.c:2137
Please submit a full bug report,

So, _why_ reload insists on pushing zero_extended constant to the register? I'd
expect that (const_int 1) is pushed into the register.

And finally, (zero_extend:DI (const_int 1 [0x1])) equals to (const_int 1
[0x1]), so why this RTX isn't simplified on-the-fly?

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #8 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-09 18:52:16 UTC ---
(In reply to comment #7)
> Redefining "j" constraint as "define_address_constraint" results in:

Yes, it needs to be define_address_constraint.

> pr50762.c:48:1: error: unrecognizable insn:
> (insn 29 28 30 3 (set (reg:DI 0 ax [77])
>         (zero_extend:DI (const_int 1 [0x1]))) pr50762.c:35 -1
>      (expr_list:REG_DEAD (reg/v:SI 59 [ p_60 ])
>         (nil)))
> pr50762.c:48:1: internal compiler error: in extract_insn, at recog.c:2137
> Please submit a full bug report,

That's odd.

> So, _why_ reload insists on pushing zero_extended constant to the register? I'd
> expect that (const_int 1) is pushed into the register.
> 
> And finally, (zero_extend:DI (const_int 1 [0x1])) equals to (const_int 1
> [0x1]), so why this RTX isn't simplified on-the-fly?

The zero_extend is a fixed part of the insn pattern in question:

(define_insn "*lea_4_zext"
  [(set (match_operand:DI 0 "register_operand" "=r")
        (zero_extend:DI
          (match_operand:SI 1 "lea_address_operand" "p")))]

Reload only ever changes what is *inside* the operands.  It does not change the
fixed parts of the pattern (outside of operands).

What I would have expected to happen is for reload to load the (const_int 1)
into an SImode register, and then zero-extend that one ...

Not sure why that doesn't happen.  I'll have a look.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ubizjak at gmail dot com

--- Comment #9 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-09 20:48:31 UTC ---
(In reply to comment #8)

> The zero_extend is a fixed part of the insn pattern in question:
> 
> (define_insn "*lea_4_zext"
>   [(set (match_operand:DI 0 "register_operand" "=r")
>         (zero_extend:DI
>           (match_operand:SI 1 "lea_address_operand" "p")))]
> 
> Reload only ever changes what is *inside* the operands.  It does not change the
> fixed parts of the pattern (outside of operands).

If this is the case, then (const_int 1) is perfectly acceptable here. It is
accepted as a valid address operand (most of the magic happens in
x86_decompose_address, followed by ix86_legitimate_address_p), and would result
in somehing like

    leal    1(,1), %eax


0000000000000000 <.text>:
   0:    8d 04 25 01 00 00 00     lea    0x1,%eax

Not quite effective way to load immediate 1, but whatnot.

> Not sure why that doesn't happen.  I'll have a look.

Thanks!

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #10 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-10 10:10:04 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> 
> > The zero_extend is a fixed part of the insn pattern in question:
> > 
> > (define_insn "*lea_4_zext"
> >   [(set (match_operand:DI 0 "register_operand" "=r")
> >         (zero_extend:DI
> >           (match_operand:SI 1 "lea_address_operand" "p")))]
> > 
> > Reload only ever changes what is *inside* the operands.  It does not change the
> > fixed parts of the pattern (outside of operands).
> 
> If this is the case, then (const_int 1) is perfectly acceptable here. 

However, it is rejected by the lea_address_operand predicate check
due to its mode (VOIDmode != SImode).  This is a bit odd because most
standard predicates accept a CONST_INT no matter what mode is requested.

But in this case, lea_address_operand is defined as "normal" predicate
(using define_predicate), but does not fall back onto any other normal
predicate, only the "special" predicate address_operand.  Therefore,
genpreds thinks it needs to add the mode check by itself, and this
version is quite strict and does not special-case CONST_INTs.

(Note that "address_operand" is special in that it uses its mode not
to check the mode of the address, but the mode of the memory operand
that is to be referred to -- on some machines, this makes a difference.)

Maybe one way to fix this would be to define lea_address_operand
using define_special_predicate and then implement an appropriate
mode check that handles CONST_INT by hand ...

> > Not sure why that doesn't happen.  I'll have a look.

Hmm, there's this special code in find_reloads that attempts to
re-recognize patterns after structural changes to an address
were made:

          /* If we now have a simple operand where we used to have a
             PLUS or MULT, re-recognize and try again.  */
          if ((OBJECT_P (*recog_data.operand_loc[i])
               || GET_CODE (*recog_data.operand_loc[i]) == SUBREG)
              && (GET_CODE (recog_data.operand[i]) == MULT
                  || GET_CODE (recog_data.operand[i]) == PLUS))
            {
              INSN_CODE (insn) = -1;
              retval = find_reloads (insn, replace, ind_levels, live_known,
                                     reload_reg_p);
              return retval;
            }

It's a bit unfortunate that this just ICEs if the modified
address is no longer recognized.

But if the lea_address_operand predicate is fixed as outlined
above, the problem should go away since the insn would then
be recognized.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #11 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 13:10:19 UTC ---
(In reply to comment #10)

> It's a bit unfortunate that this just ICEs if the modified
> address is no longer recognized.
> 
> But if the lea_address_operand predicate is fixed as outlined
> above, the problem should go away since the insn would then
> be recognized.

Indeed, changing lea_address_operand predicate to define_special_predicate
allows compilation to pass reload. In fact, the predicate _was_ defined as
special predicate until it was changed by ... well, ... me in the unrelated
"cleanup" change [1] to fix PR 49780. So, with following patch:

Index: config/i386/predicates.md
===================================================================
--- config/i386/predicates.md   (revision 181252)
+++ config/i386/predicates.md   (working copy)
@@ -809,7 +809,7 @@

 ;; Return true if op if a valid address for LEA, and does not contain
 ;; a segment override.
-(define_predicate "lea_address_operand"
+(define_special_predicate "lea_address_operand"
   (match_operand 0 "address_operand")
 {
   struct ix86_address parts;

the compilation advances over reload pass to break with:

tt.c: In function ‘func_59’:
tt.c:48:1: internal compiler error: in simplify_const_unary_operation, at
simplify-rtx.c:1468
Please submit a full bug report,

[1] http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01555.html

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

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

--- Comment #12 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 13:16:54 UTC ---
This time we hit assert in simplify_rtx.c:

    case ZERO_EXTEND:
      /* When zero-extending a CONST_INT, we need to know its
             original mode.  */
-->      gcc_assert (op_mode != VOIDmode);
      if (op_width == HOST_BITS_PER_WIDE_INT)
        {
          /* If we were really extending the mode,
         we would have to distinguish between zero-extension
         and sign-extension.  */
          gcc_assert (width == op_width);
          val = arg0;
        }

But ... CONST_INTs do not have mode.

A couple of lines below, CONST_INTs can be sign_extended, but not zero_extended
here... Let's ask some people that know this part of the compiler.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #13 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 13:28:37 UTC ---
A shot-in-the-dark patch that fixes compilation:

Index: simplify-rtx.c
===================================================================
--- simplify-rtx.c      (revision 181252)
+++ simplify-rtx.c      (working copy)
@@ -1463,9 +1463,9 @@ simplify_const_unary_operation (enum rtx_code code
          break;

        case ZERO_EXTEND:
-         /* When zero-extending a CONST_INT, we need to know its
-             original mode.  */
-         gcc_assert (op_mode != VOIDmode);
+         if (op_mode == VOIDmode)
+           op_mode = mode;
+         op_width = GET_MODE_PRECISION (op_mode);
          if (op_width == HOST_BITS_PER_WIDE_INT)
            {
              /* If we were really extending the mode,

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[bernds at gcc dot gnu.org]

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

--- Comment #14 from Bernd Schmidt <bernds at gcc dot gnu.org> 2011-11-10 13:34:02 UTC ---
Is this with your patch from comment 6? You really can't have a CONST_INT
inside a zero_extend; the abort is justified.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #15 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 13:38:50 UTC ---
(In reply to comment #14)
> Is this with your patch from comment 6? You really can't have a CONST_INT
> inside a zero_extend; the abort is justified.

No, this is with the patch from comment #11. This patch in fact just reverts
the definition to define_special_predicate as was in the past.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #16 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-10 14:04:42 UTC ---
(In reply to comment #15)
> (In reply to comment #14)
> > Is this with your patch from comment 6? You really can't have a CONST_INT
> > inside a zero_extend; the abort is justified.
> 
> No, this is with the patch from comment #11. This patch in fact just reverts
> the definition to define_special_predicate as was in the past.

You probably need the patch from comment 6 (with the define_address_constraint
fix) *in addition* to the patch from comment 11: this way, the zero_extend
(const_int) will be accepted temporarily by the predicate, but immediately
afterwards reloaded into a register.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ebotcazou at gcc dot gnu.org]

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

--- Comment #17 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-11-10 15:06:40 UTC ---
> However, it is rejected by the lea_address_operand predicate check
> due to its mode (VOIDmode != SImode).  This is a bit odd because most
> standard predicates accept a CONST_INT no matter what mode is requested.

This isn't only odd, this goes against the documentation:

"Predicates written with `define_predicate' automatically include a
test that MODE is `VOIDmode', or OP has the same mode as MODE, or OP is
a `CONST_INT' or `CONST_DOUBLE'.  They do _not_ check specifically for
integer `CONST_DOUBLE', nor do they test that the value of either kind
of constant fits in the requested mode."

> Maybe one way to fix this would be to define lea_address_operand
> using define_special_predicate and then implement an appropriate
> mode check that handles CONST_INT by hand ...

Clearly the oddity you pointed out above is counter-intuitive.  It is present
in the original patch: http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00663.html

Maybe worth discussing on the main list in order to decide what needs to be
changed, the documentation or the implementation.

[Bug rtl-optimization/50762] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot       |ubizjak at gmail dot com
                   |gnu.org                     |

--- Comment #18 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 16:01:32 UTC ---
Created attachment 25786
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25786
Patch to fix the ICE.

As suggested by Ulrich, combined Comment #6 (with define_address_constraint
fix) and Comment #11 patch I am regtesting now. Fixes ICE for the testcase.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|ra                          |
          Component|rtl-optimization            |target
            Summary|ICE: in extract_insn, at    |[4.7 Regression] ICE: in
                   |recog.c:2137                |extract_insn, at
                   |(unrecognizable insn)       |recog.c:2137
                   |                            |(unrecognizable insn)

--- Comment #19 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 16:04:47 UTC ---
(In reply to comment #17)

> Clearly the oddity you pointed out above is counter-intuitive.  It is present
> in the original patch: http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00663.html
> 
> Maybe worth discussing on the main list in order to decide what needs to be
> changed, the documentation or the implementation.

I will submit the patch to gcc-patches@ ML in a couple of hours, and will wait
for the resolution of this discussion before committing it to SVN.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #20 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 18:10:33 UTC ---
(In reply to comment #17)
> > However, it is rejected by the lea_address_operand predicate check
> > due to its mode (VOIDmode != SImode).  This is a bit odd because most
> > standard predicates accept a CONST_INT no matter what mode is requested.
> 
> This isn't only odd, this goes against the documentation:
> 
> "Predicates written with `define_predicate' automatically include a
> test that MODE is `VOIDmode', or OP has the same mode as MODE, or OP is
> a `CONST_INT' or `CONST_DOUBLE'.  They do _not_ check specifically for
> integer `CONST_DOUBLE', nor do they test that the value of either kind
> of constant fits in the requested mode."
> 
> > Maybe one way to fix this would be to define lea_address_operand
> > using define_special_predicate and then implement an appropriate
> > mode check that handles CONST_INT by hand ...
> 
> Clearly the oddity you pointed out above is counter-intuitive.  It is present
> in the original patch: http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00663.html
> 
> Maybe worth discussing on the main list in order to decide what needs to be
> changed, the documentation or the implementation.

The documentation is wrong, so following patch removes all the blurb about
handling of constants.

Index: doc/md.texi
===================================================================
--- doc/md.texi    (revision 181258)
+++ doc/md.texi    (working copy)
@@ -1001,16 +1001,7 @@

 Predicates written with @code{define_predicate} automatically include
 a test that @var{mode} is @code{VOIDmode}, or @var{op} has the same
-mode as @var{mode}, or @var{op} is a @code{CONST_INT} or
-@code{CONST_DOUBLE}.  They do @emph{not} check specifically for
-integer @code{CONST_DOUBLE}, nor do they test that the value of either
-kind of constant fits in the requested mode.  This is because
-target-specific predicates that take constants usually have to do more
-stringent value checks anyway.  If you need the exact same treatment
-of @code{CONST_INT} or @code{CONST_DOUBLE} that the generic predicates
-provide, use a @code{MATCH_OPERAND} subexpression to call
-@code{const_int_operand}, @code{const_double_operand}, or
-@code{immediate_operand}.
+mode as @var{mode}.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #21 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-10 18:22:50 UTC ---
(In reply to comment #20)
> 
> The documentation is wrong, so following patch removes all the blurb about
> handling of constants.
> 
> Index: doc/md.texi
> ===================================================================
> --- doc/md.texi    (revision 181258)
> +++ doc/md.texi    (working copy)
> @@ -1001,16 +1001,7 @@
> 
>  Predicates written with @code{define_predicate} automatically include
>  a test that @var{mode} is @code{VOIDmode}, or @var{op} has the same
> -mode as @var{mode}, or @var{op} is a @code{CONST_INT} or
> -@code{CONST_DOUBLE}.  They do @emph{not} check specifically for
> -integer @code{CONST_DOUBLE}, nor do they test that the value of either
> -kind of constant fits in the requested mode.  This is because
> -target-specific predicates that take constants usually have to do more
> -stringent value checks anyway.  If you need the exact same treatment
> -of @code{CONST_INT} or @code{CONST_DOUBLE} that the generic predicates
> -provide, use a @code{MATCH_OPERAND} subexpression to call
> -@code{const_int_operand}, @code{const_double_operand}, or
> -@code{immediate_operand}.
> +mode as @var{mode}.

But this is not quite true either.  genpreds will *omit* generation of
the explicit test (mode == VOIDmode || mode == GET_MODE (op)) if the
predicate has as a sub-test a call to one of the standard predicates,
arguing that "the standard predicate already did the test, so we don't
have to repeat it".  But the test performed by the standard predicates
*does* comply with the more elaborate rule spelled out in the docs ...

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #22 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 19:05:29 UTC ---
(In reply to comment #21)

> But this is not quite true either.  genpreds will *omit* generation of
> the explicit test (mode == VOIDmode || mode == GET_MODE (op)) if the
> predicate has as a sub-test a call to one of the standard predicates,
> arguing that "the standard predicate already did the test, so we don't
> have to repeat it".  But the test performed by the standard predicates
> *does* comply with the more elaborate rule spelled out in the docs ...

Yeah, this is also what I thought at the first sight. But please don't forget
that additional c-code test effectively creates

(and (match_operand (...)
     (match_test (call to c-code)))

and this construct will _always_ force generation of mode checks.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2011-11/msg01450.htm
                   |                            |l

--- Comment #23 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-10 19:31:17 UTC ---
Patch at [1].

[1] http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01450.html

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uweigand at gcc dot gnu.org]

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

--- Comment #24 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-11-11 13:04:02 UTC ---
(In reply to comment #22)

> Yeah, this is also what I thought at the first sight. But please don't forget
> that additional c-code test effectively creates
> 
> (and (match_operand (...)
>      (match_test (call to c-code)))
> 
> and this construct will _always_ force generation of mode checks.

No, it actually does not; see e.g. in the x86-64 insn-preds.c we have

(define_predicate "long_memory_operand"
  (and (match_operand 0 "memory_operand")
       (match_test "memory_address_length (op)")))

implemented as:

int
long_memory_operand (rtx op, enum machine_mode mode ATTRIBUTE_UNUSED)
{
  return (memory_operand (op, mode)) && (
#line 994 "../../gcc-head/gcc/config/i386/predicates.md"
(memory_address_length (op)));
}

This is because the genpred routines have code to follow boolean
operators and reason correctly that if match_operand performs the
mode test, and some other test is joined via "and", the test is
still not necessary.

In fact, except for the address_operand case, the mode checks seem
to be correct in their actual effect: we either call a standard operator
and omit the mode check, or else we have the genpred-generated mode check,
but in conjunction with some match_code test that excludes CONST_INT 
and CONST_DOUBLE anyway.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

--- Comment #25 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-11 13:17:41 UTC ---
(In reply to comment #24)

> In fact, except for the address_operand case, the mode checks seem
> to be correct in their actual effect: we either call a standard operator
> and omit the mode check, or else we have the genpred-generated mode check,
> but in conjunction with some match_code test that excludes CONST_INT 
> and CONST_DOUBLE anyway.

address_operand is declared as special in std_preds[] table in gensupport.c, so
it doesn't check mode checks. genpreds figures this out and adds mode checks at
the end of non-special predicate that uses address_operand.

All in all, if we want to omit mode checks, we have to declare
lea_address_operand as a special predicate.

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[uros at gcc dot gnu.org]

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

--- Comment #26 from uros at gcc dot gnu.org 2011-11-11 14:56:03 UTC ---
Author: uros
Date: Fri Nov 11 14:56:00 2011
New Revision: 181285

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181285
Log:
    PR target/50762
    * config/i386/constraints.md (j): New address constraint.
    * config/i386/predicates.md (lea_address_operand): Redefine as
    special predicate.
    * config/i386/i386.md (*lea_3_zext): Use "j" constraint for operand 1.
    (*lea_4_zext): Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/constraints.md
    trunk/gcc/config/i386/i386.md
    trunk/gcc/config/i386/predicates.md

[Bug target/50762] [4.7 Regression] ICE: in extract_insn, at recog.c:2137 (unrecognizable insn)

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

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

--- Comment #27 from Uros Bizjak <ubizjak at gmail dot com> 2011-11-11 15:02:14 UTC ---
Fixed.