[Bug target/56470] New: [4.8 Regression] ICE output_operand: invalid shift operand

[Bug target/56470] New: [4.8 Regression] ICE output_operand: invalid shift operand

[jakub at gcc dot gnu.org]

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

             Bug #: 56470
           Summary: [4.8 Regression] ICE output_operand: invalid shift
                    operand
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jakub@gcc.gnu.org
            Target: arm


int w[5];

void
foo (int x, unsigned y)
{
  long long r = 0;
  long long s;
  int i;
  switch (x)
    {
    case 1:
      for (i = 0; i < 4; i++)
        {
          if (!y)
            s = (w[9] << (10 - 2 * i)) >> (32 - 5 * i + 5 * i);
          r |= (s & 0xffffffff) << (i * 5);
        }
      break;
    case 3:
      r = w[x] >> y;
    }
  w[x] = r;
}

ICEs at -O2 on arm-linux-gnuabi.  This reduced testcase ICEs also in 4.7, but
the unreduced one at https://bugzilla.redhat.com/show_bug.cgi?id=915830#c1
only ICEs in 4.8 and not in 4.7.
I wonder why arm backend uses constraints (N) to enfore the shift count in the
range, rather than e.g. masking the operand & 31 if CONST_INT before printing.
Shift counts >= 32 for SImode are of course undefined behavior, so it really
doesn't matter what gets emitted, but we shouldn't ICE on it.

[Bug target/56470] [4.8 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-03-04
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-04 07:01:07 UTC ---
Confirmed. 

I'm a bit wary of just truncating the value.  Shifts by 32 may be valid in the
ARM back-end in some circumstances where we're using the shift as part of
setting up the flags.

Fixing this fully would require getting rid of "shift_operator" and replacing
it with iterators.  But that's a pretty radical overhaul.  Long term that might
well be worthwhile, but not this close to a release.

[Bug target/56470] [4.8 Regression] ICE output_operand: invalid shift operand

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-04 08:17:56 UTC ---
In any case, this looks like a clear backend bug, as it allows const_int 32 in
the operand (through the use of M constraint), but then doesn't handle it at
all in arm_output_shift (it forces use of %S3 on it and that requires 0 .. 31
constant, not 0 .. 32).  And while the code has undefined behavior, if you
never invoke it, you should still be able to have it in a valid program.

[Bug target/56470] [4.8 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

--- Comment #3 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-05 07:01:42 UTC ---
Definitely a back-end bug.  I'm not disputing that.

My surprise is that this hasn't bitten us long before now, since the code has
been this way for well over ten years.

[Bug target/56470] [4.8 Regression] ICE output_operand: invalid shift operand

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-05 09:14:46 UTC ---
If the folder or GIMPLE passes see it is a shift by 32, they fold it away (with
or without a warning), so the fact that it is a constant larger than bitsize of
the shifted operand has to be seen only during RTL optimizations.

[Bug target/56470] [4.8 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #5 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-06 10:27:03 UTC ---
Working on a fix.

[Bug target/56470] [4.7/4.8 Regression] ICE output_operand: invalid shift operand

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code
           Priority|P3                          |P2
   Target Milestone|4.8.0                       |4.7.3
            Summary|[4.8 Regression] ICE        |[4.7/4.8 Regression] ICE
                   |output_operand: invalid     |output_operand: invalid
                   |shift operand               |shift operand

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> 2013-03-08 15:46:00 UTC ---
According to reporter also broken on the 4.7 branch.

[Bug target/56470] [4.7/4.8 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

--- Comment #7 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-11 11:48:45 UTC ---
Author: rearnsha
Date: Mon Mar 11 11:48:34 2013
New Revision: 196595

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196595
Log:
    PR target/56470
    * arm.c (shift_op): Validate RTL pattern on the fly.
    (arm_print_operand, case 'S'): Don't use shift_operator to validate
    the RTL.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c

[Bug target/56470] [4.7 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.7/4.8 Regression] ICE    |[4.7 Regression] ICE
                   |output_operand: invalid     |output_operand: invalid
                   |shift operand               |shift operand

--- Comment #8 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-11 14:38:14 UTC ---
Fixed on trunk.  Testing back-port to 4.7.

[Bug target/56470] [4.7 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

--- Comment #9 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-18 11:52:15 UTC ---
Author: rearnsha
Date: Mon Mar 18 11:52:08 2013
New Revision: 196780

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196780
Log:
    PR target/56470
    * arm.c (shift_op): Validate RTL pattern on the fly.
    (arm_print_operand, case 'S'): Don't use shift_operator to validate
    the RTL.

Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/arm/arm.c

[Bug target/56470] [4.7 Regression] ICE output_operand: invalid shift operand

[rearnsha at gcc dot gnu.org]

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

Richard Earnshaw <rearnsha at gcc dot gnu.org> changed:

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

--- Comment #10 from Richard Earnshaw <rearnsha at gcc dot gnu.org> 2013-03-18 11:54:13 UTC ---
Fixed