This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[RFA:] Fix PR target/6838 CRIS, request to revise 3.1 check-in policy
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: mark at codesourcery dot com, gcc-patches at gcc dot gnu dot org
- Date: Wed, 29 May 2002 17:33:33 +0200
- Subject: [RFA:] Fix PR target/6838 CRIS, request to revise 3.1 check-in policy
Please revise check-in policy for 3.1,
<URL:http://gcc.gnu.org/ml/gcc/2002-05/msg01212.html>. I
believe that port-local bug-fixes for new ports should be
fixable by a person with approval rights without release manager
approval.
Reason: I don't think it's reasonable to take up release manager
resources for patches like the following, which is not a
regression since there hasn't been any FSF release of gcc with
CRIS support, and the fix is indeed local to the port. The
policy above IIUC still requires that the patch goes through the
release manager.
So please add to "Anyone who can normally..." words like these:
"or fixes bugs in ports or functionality new to this release,
where the fix is local to the port or code for the new
functionality".
On to the substance:
Here's a fix for target/6838 and the code in the report tweaked
to make an executable test-case (checked with the author for use
in the testsuite); tested of course on cris-elf. The patch for
the trunk will have additional tweaks. All but one of the
changes in cris.md are to comments.
Ok for branch?
gcc/testsuite:
* gcc.c-torture/execute/20020529-1.c: New test.
gcc:
PR target/6838
* config/cris/cris.md: Fix typos and thinkos.
(splitter for mov_sideqi_mem, mov_sidehi_mem, mov_sidesi_mem):
Remove spurious mode specifier on operand 2.
*** /dev/null Tue Jan 1 05:00:00 1980
--- gcc.c-torture/execute/20020529-1.c Wed May 29 02:30:03 2002
***************
*** 0 ****
--- 1,78 ----
+ /* PR target/6838 from cato@df.lth.se.
+ cris-elf got an ICE with -O2: the insn matching
+ (insn 49 48 52 (parallel[
+ (set (mem/s:HI (plus:SI (reg/v/f:SI 0 r0 [24])
+ (const_int 8 [0x8])) [5 <variable>.c+0 S2 A8])
+ (reg:HI 2 r2 [27]))
+ (set (reg/f:SI 2 r2 [31])
+ (plus:SI (reg/v/f:SI 0 r0 [24])
+ (const_int 8 [0x8])))
+ ] ) 24 {*mov_sidehi_mem} (nil)
+ (nil))
+ forced a splitter through the output pattern "#", but there was no
+ matching splitter. */
+
+ struct xx
+ {
+ int a;
+ struct xx *b;
+ short c;
+ };
+
+ int f1 (struct xx *);
+ void f2 (void);
+
+ int
+ foo (struct xx *p, int b, int c, int d)
+ {
+ int a;
+
+ for (;;)
+ {
+ a = f1(p);
+ if (a)
+ return (0);
+ if (b)
+ continue;
+ p->c = d;
+ if (p->a)
+ f2 ();
+ if (c)
+ f2 ();
+ d = p->c;
+ switch (a)
+ {
+ case 1:
+ if (p->b)
+ f2 ();
+ if (c)
+ f2 ();
+ default:
+ break;
+ }
+ }
+ return d;
+ }
+
+ int main (void)
+ {
+ struct xx s = {0, &s, 23};
+ if (foo (&s, 0, 0, 0) != 0 || s.a != 0 || s.b != &s || s.c != 0)
+ abort ();
+ exit (0);
+ }
+
+ int
+ f1 (struct xx *p)
+ {
+ static int beenhere = 0;
+ if (beenhere++ > 1)
+ abort ();
+ return beenhere > 1;
+ }
+
+ void
+ f2 (void)
+ {
+ abort ();
+ }
Index: cris.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/cris/cris.md,v
retrieving revision 1.2
diff -p -c -r1.2 cris.md
*** cris.md 4 Nov 2001 02:51:23 -0000 1.2
--- cris.md 29 May 2002 14:29:51 -0000
***************
*** 1,5 ****
;; GCC machine description for CRIS cpu cores.
! ;; Copyright (C) 1998, 1999, 2000, 2001 Free Software Foundation, Inc.
;; Contributed by Axis Communications.
;; This file is part of GCC.
--- 1,5 ----
;; GCC machine description for CRIS cpu cores.
! ;; Copyright (C) 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
;; Contributed by Axis Communications.
;; This file is part of GCC.
***************
*** 385,391 ****
prefer to split up constants early, like this. The testcase in
gcc.c-torture/execute/961213-1.c shows that CSE2 gets confused by the
resulting subreg sets when using the construct from mcore (as of FSF
! CVS, version -r 1.5), and it believe that the high part (the last one
emitted) is the final value. This construct from romp seems more
robust, especially considering the head comments from
emit_no_conflict_block. */
--- 385,391 ----
prefer to split up constants early, like this. The testcase in
gcc.c-torture/execute/961213-1.c shows that CSE2 gets confused by the
resulting subreg sets when using the construct from mcore (as of FSF
! CVS, version -r 1.5), and it believes that the high part (the last one
emitted) is the final value. This construct from romp seems more
robust, especially considering the head comments from
emit_no_conflict_block. */
***************
*** 561,577 ****
;; Other way around; move to memory.
! ;; For all side-effect patterns, it seems to be the case that the
! ;; predicate isn't consulted after combine. For sake of stability, we
! ;; recognize and split the cases where dangerous register combinations are
! ;; spotted: where a register is set in the side-effect, and used in the
! ;; main insn. We don't handle the case where the set in the main insn
! ;; overlaps the set in the side-effect; that would be too big a bug to
! ;; paper over. We handle just the case where the set in the side-effect
! ;; overlaps the input operand of the main insn (i.e. just moves to memory).
;;
! ;; move.s rx,[ry=rx+rw.S]
;; FIXME: These could have anonymous mode for operand 3.
;; QImode
--- 561,582 ----
;; Other way around; move to memory.
! ;; Note that the condition (which for side-effect patterns is usually a
! ;; call to cris_side_effect_mode_ok), isn't consulted for register
! ;; allocation preferences -- constraints is the method for that. The
! ;; drawback is that we can't exclude register allocation to cause
! ;; "move.s rw,[rx=ry+rz.S]" when rw==rx without also excluding rx==ry or
! ;; rx==rz if we use an earlyclobber modifier for the constraint for rx.
! ;; Instead of that, we recognize and split the cases where dangerous
! ;; register combinations are spotted: where a register is set in the
! ;; side-effect, and used in the main insn. We don't handle the case where
! ;; the set in the main insn overlaps the set in the side-effect; that case
! ;; must be handled in gcc. We handle just the case where the set in the
! ;; side-effect overlaps the input operand of the main insn (i.e. just
! ;; moves to memory).
;;
! ;; move.s rz,[ry=rx+rw.S]
;; FIXME: These could have anonymous mode for operand 3.
;; QImode
***************
*** 628,637 ****
#
move.%s3 %3,[%4=%2+%0%T1]")
! ;; Split for the case above where the predicate isn't honored; only the
! ;; constraint, and we end up with the set in the side-effect gets the same
! ;; register as the input register. Arguably a GCC bug, but we'll spot it
! ;; rarely enough that we need to catch it ourselves to be safe.
(define_split
[(parallel
--- 633,642 ----
#
move.%s3 %3,[%4=%2+%0%T1]")
! ;; Split for the case above where we're out of luck with register
! ;; allocation (again, the condition isn't checked for that), and we end up
! ;; with the set in the side-effect getting the same register as the input
! ;; register.
(define_split
[(parallel
***************
*** 737,751 ****
}")
;; Like the biap case, a split where the set in the side-effect gets the
! ;; same register as the input register to the main insn due to gcc not
! ;; always checking the predicate.
(define_split
[(parallel
[(set (mem (plus:SI
(match_operand:SI 0 "cris_bdap_operand" "")
(match_operand:SI 1 "cris_bdap_operand" "")))
! (match_operand:SI 2 "register_operand" ""))
(set (match_operand:SI 3 "register_operand" "")
(plus:SI (match_dup 0) (match_dup 1)))])]
"reload_completed && reg_overlap_mentioned_p (operands[3], operands[2])"
--- 742,756 ----
}")
;; Like the biap case, a split where the set in the side-effect gets the
! ;; same register as the input register to the main insn, since the
! ;; condition isn't checked at register allocation.
(define_split
[(parallel
[(set (mem (plus:SI
(match_operand:SI 0 "cris_bdap_operand" "")
(match_operand:SI 1 "cris_bdap_operand" "")))
! (match_operand 2 "register_operand" ""))
(set (match_operand:SI 3 "register_operand" "")
(plus:SI (match_dup 0) (match_dup 1)))])]
"reload_completed && reg_overlap_mentioned_p (operands[3], operands[2])"
***************
*** 4272,4278 ****
(set (match_dup 5) (match_dup 2))]
"operands[5] = gen_rtx_MEM (GET_MODE (operands[2]), operands[3]);")
! ;; clear.d ry,[rx=rx+rz.S2]
(define_split
[(parallel
--- 4277,4283 ----
(set (match_dup 5) (match_dup 2))]
"operands[5] = gen_rtx_MEM (GET_MODE (operands[2]), operands[3]);")
! ;; clear.d [rx=rx+rz.S2]
(define_split
[(parallel
***************
*** 4292,4298 ****
(set (mem:SI (match_dup 3)) (const_int 0))]
"")
! ;; clear.w ry,[rx=rx+rz.S2]
(define_split
[(parallel
--- 4297,4303 ----
(set (mem:SI (match_dup 3)) (const_int 0))]
"")
! ;; clear.w [rx=rx+rz.S2]
(define_split
[(parallel
***************
*** 4312,4318 ****
(set (mem:HI (match_dup 3)) (const_int 0))]
"")
! ;; clear.b ry,[rx=rx+rz.S2]
(define_split
[(parallel
--- 4317,4323 ----
(set (mem:HI (match_dup 3)) (const_int 0))]
"")
! ;; clear.b [rx=rx+rz.S2]
(define_split
[(parallel
***************
*** 4332,4338 ****
(set (mem:QI (match_dup 3)) (const_int 0))]
"")
! ;; clear.d ry,[rx=rx+i]
(define_split
[(parallel
--- 4337,4343 ----
(set (mem:QI (match_dup 3)) (const_int 0))]
"")
! ;; clear.d [rx=rx+i]
(define_split
[(parallel
***************
*** 4349,4355 ****
(set (mem:SI (match_dup 2)) (const_int 0))]
"")
! ;; clear.w ry,[rx=rx+i]
(define_split
[(parallel
--- 4354,4360 ----
(set (mem:SI (match_dup 2)) (const_int 0))]
"")
! ;; clear.w [rx=rx+i]
(define_split
[(parallel
***************
*** 4366,4372 ****
(set (mem:HI (match_dup 2)) (const_int 0))]
"")
! ;; clear.b ry,[rx=rx+i]
(define_split
[(parallel
--- 4371,4377 ----
(set (mem:HI (match_dup 2)) (const_int 0))]
"")
! ;; clear.b [rx=rx+i]
(define_split
[(parallel
brgds, H-P