Bug 35492 - ICE building kernel sk_stream_wait_connect output_operand: invalid operand for 'p' modifier
ICE building kernel sk_stream_wait_connect output_operand: invalid operand fo...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: target
4.3.1
: P3 normal
: 4.3.5
Assigned To: Hans-Peter Nilsson
: ice-on-valid-code
: 36615 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-07 02:20 UTC by Hans-Peter Nilsson
Modified: 2010-04-27 02:51 UTC (History)
2 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: cris-*-* and crisv32-*-*
Build:
Known to work: 4.3.2
Known to fail: 4.3.1
Last reconfirmed: 2008-03-07 03:07:59


Attachments
patch for 4.3 (1.70 KB, patch)
2008-07-08 10:08 UTC, Hans-Peter Nilsson
Details | Diff
patch for trunk/4.4 (13.89 KB, patch)
2008-07-08 10:26 UTC, Hans-Peter Nilsson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans-Peter Nilsson 2008-03-07 02:20:32 UTC
With trunk r132993 and gcc-4_3-branch r132992, the following testcase yields,
with "-march=v32 -O2":
(const_int -13 [0xfffffffffffffff3])
s2.c: In function 'sk_stream_wait_connect':
s2.c:26: internal compiler error: output_operand: invalid operand for 'p' modifier

void prepare_to_wait (void *, void *, int);
void finish_wait (void *, void *);
extern signed long schedule_timeout (signed long);
struct sock
{
  unsigned char skc_state;
  void *sk_sleep;
  int sk_err;
};

void
sk_stream_wait_connect (struct sock *sk, long *timeo_p)
{
  int done;
  int wait;
  do
    {
      if ((1 << sk->skc_state) & ~12)
        return;
      prepare_to_wait (sk->sk_sleep, &wait, 1);
      *(timeo_p) = schedule_timeout (0);
      done = !sk->sk_err;
      finish_wait (sk->sk_sleep, &wait);
    }
  while (!done);
}
Comment 1 Hans-Peter Nilsson 2008-03-07 03:07:59 UTC
Also with trunk r132670.  The ICE points at the somewhat-hairy last alternative of the *btst pattern for being at fault for not matching for the insn appearing in 177r.greg:

(insn:QI 12 11 13 3 s2.c:18 (set (cc0)
        (zero_extract:SI (const_int -13 [0xfffffffffffffff3])
            (const_int 1 [0x1])
            (reg:SI 15 acr [orig:29 <variable>.skc_state ] [29]))) 16 {*btst} (nil))

On closer inspection however, the condition indicates that the operands are invalid and gcc should have noticed that, when transforming the following valid insn into the above (from 176r.lreg):
(insn:QI 12 11 13 3 s2.c:18 (set (cc0)
        (zero_extract:SI (reg:SI 44)
            (const_int 1 [0x1])
            (reg:SI 29 [ <variable>.skc_state ]))) 16 {*btst} (expr_list:REG_DEAD (reg:SI 29 [ <variable>.skc_state ])
        (nil)))

So, something replaces reg 44 and doesn't check the validity of the replacement.
For 4.3, it might be safer to remove the last alternative of *btst, but for trunk we should fix the real bug, supposedly in reload. ;)
Changing component to middle-end.
Comment 2 Hans-Peter Nilsson 2008-03-13 16:54:41 UTC
Also repeatable with plain "-O1", at least for cris-*-*.
Comment 3 Hans-Peter Nilsson 2008-06-24 12:16:07 UTC
*** Bug 36615 has been marked as a duplicate of this bug. ***
Comment 4 Hans-Peter Nilsson 2008-07-08 10:08:21 UTC
Created attachment 15874 [details]
patch for 4.3

While I investigate just a little more, here's the regtested patch I'll commit to the 4.3 branch. It's safe in that it just removes the offending alternative, avoiding the problematic situation.
Comment 5 Hans-Peter Nilsson 2008-07-08 10:26:47 UTC
Created attachment 15875 [details]
patch for trunk/4.4

This is one part of a correction (regtested on trunk for cris-elf). By itself, it hides the bug by adding a constraint (redefining K to be a constraint prefix, renaming current uses to Kc, and introducing Kp) such that the constraints don't allow what the condition refuses.  Lots of editing, mostly because of renaming.

Thus, it doesn't matter that the condition isn't checked.  It's correct to do this: putting as much of the description as possible into the constraints.  Having that check in the condition was an artefact from when GCC didn't have multi-letter constraints.

But, reload must also be fixed to not do operand transformations that don't also check the insn condition.  I'm still working on that part.
Comment 6 Hans-Peter Nilsson 2008-07-14 00:12:56 UTC
After tests and consideration, I think we should just adjust the documentation; the pattern condition, where it checks operands, must not be more restrictive than the constraints.  As no code change is expected in the middle-end, I change to component target.  Target patch commits imminent.
Comment 7 Hans-Peter Nilsson 2008-07-14 00:23:13 UTC
Subject: Bug 35492

Author: hp
Date: Mon Jul 14 00:22:35 2008
New Revision: 137765

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137765
Log:
	PR target/35492.
	* config/cris/cris.h (CRIS_CONST_OK_FOR_LETTER_P): Renamed from
	CONST_OK_FOR_LETTER_P.  All port-local users changed.
	(CONST_OK_FOR_CONSTRAINT_P): Define; implement Kc as old K,
	implement Kp matching power-of-two.
	(CONSTRAINT_LEN): Define to match.
	* config/cris/cris.md: Replace all use of constraint K with Kc.
	("*btst*): Use Kp for operand 0 of last alternative.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/cris/cris.c
    trunk/gcc/config/cris/cris.h
    trunk/gcc/config/cris/cris.md

Comment 8 Hans-Peter Nilsson 2008-07-14 00:25:19 UTC
Subject: Bug 35492

Author: hp
Date: Mon Jul 14 00:24:35 2008
New Revision: 137766

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137766
Log:
	PR target/35492
	* gcc.c-torture/compile/pr35492.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr35492.c
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 9 Hans-Peter Nilsson 2008-07-14 00:26:37 UTC
Subject: Bug 35492

Author: hp
Date: Mon Jul 14 00:25:52 2008
New Revision: 137767

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137767
Log:
	PR target/35492
	* gcc.c-torture/compile/pr35492.c: New test.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.c-torture/compile/pr35492.c
Modified:
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 10 Hans-Peter Nilsson 2008-07-14 00:29:08 UTC
Subject: Bug 35492

Author: hp
Date: Mon Jul 14 00:28:27 2008
New Revision: 137768

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137768
Log:
	PR target/35492.
	* config/cris/cris.md ("*btst"): Remove last alternative
	and related part in the condition.

Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/cris/cris.md

Comment 11 Hans-Peter Nilsson 2010-04-27 02:51:00 UTC
This still open?  I guess I kept it open for the doc patch to go through.
It got shot down (IMHO incorrectly) but I can't be bothered to pursue.
Closing this; all codegen behavior has been fixed.

(It'd like to set milestone to 4.3.2 but that list has been pruned. :)