S/390: Fix literal pool ICE regression

Ulrich Weigand uweigand@de.ibm.com
Tue Nov 9 21:08:00 GMT 2004


Hello,

the reorganization of the cmp*ccu/cct patterns has introduced a
regression: in some cases, reload generates insns that refer to
two different literal pool entries at the same time, which the
literal pool code cannot handle.

I had intended to prevent this by adding s390_pool_operand tests
to the relevant insn conditions.  Unfortunately this does not 
work as reload doesn't always re-evaluate the insn condition.

This patch fixes the problem by moving the test to the constraint,
defining a new family of 'B' constraints for the purpose.

Bootstrapped/regtested on s390-ibm-linux and s390x-ibm-linux,
committed to mainline.

Bye,
Ulrich


ChangeLog:

	* config/s390/s390-protos.h (s390_pool_operand): Remove.
	* config/s390/s390.c (s390_pool_operand): Likewise.
	(s390_extra_constraint_str): Handle 'B' constraints.
	* config/s390/s390.h (CONSTRAINT_LEN): Handle 'B' constraints.
	* config/s390/s390.md: Document 'B' constraints.
	("*cmpdi_cct", "*cmpsi_cct"): Use 'B' constraint instead of
	s390_pool_operand to prevent insns with two literal pool
	references.  Make pattern commutative.
	("*cmpdi_ccu", "*cmpsi_ccu", "*cmphi_ccu", "*cmpqi_ccu"): Use
	'B' constraint instead of s390_pool_operand.

testsuite/ChangeLog:

	* gcc.dg/20041109-1.c: New test.


Index: gcc/config/s390/s390-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390-protos.h,v
retrieving revision 1.65
diff -c -p -r1.65 s390-protos.h
*** gcc/config/s390/s390-protos.h	3 Nov 2004 21:39:45 -0000	1.65
--- gcc/config/s390/s390-protos.h	9 Nov 2004 15:16:46 -0000
*************** extern void s390_expand_logical_operator
*** 89,95 ****
  					  enum machine_mode, rtx *);
  extern bool s390_logical_operator_ok_p (rtx *);
  extern void s390_narrow_logical_operator (enum rtx_code, rtx *, rtx *);
- extern bool s390_pool_operand (rtx);
  extern void s390_split_access_reg (rtx, rtx *, rtx *);
  
  extern bool s390_output_addr_const_extra (FILE*, rtx);
--- 89,94 ----
Index: gcc/config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.197
diff -c -p -r1.197 s390.c
*** gcc/config/s390/s390.c	5 Nov 2004 15:27:30 -0000	1.197
--- gcc/config/s390/s390.c	9 Nov 2004 15:16:47 -0000
*************** s_operand (rtx op, enum machine_mode mod
*** 1501,1541 ****
    return 1;
  }
  
- /* Return true if OP is a memory operand pointing to the
-    literal pool, or an immediate operand.  */
- 
- bool
- s390_pool_operand (rtx op)
- {
-   struct s390_address addr;
- 
-   /* Just like memory_operand, allow (subreg (mem ...))
-      after reload.  */
-   if (reload_completed
-       && GET_CODE (op) == SUBREG
-       && GET_CODE (SUBREG_REG (op)) == MEM)
-     op = SUBREG_REG (op);
- 
-   switch (GET_CODE (op))
-     {
-     case CONST_INT:
-     case CONST_DOUBLE:
-       return true;
- 
-     case MEM:
-       if (!s390_decompose_address (XEXP (op, 0), &addr))
- 	return false;
-       if (addr.base && REG_P (addr.base) && REGNO (addr.base) == BASE_REGNUM)
- 	return true;
-       if (addr.indx && REG_P (addr.indx) && REGNO (addr.indx) == BASE_REGNUM)
- 	return true;
-       return false;
- 
-     default:
-       return false;
-     }
- }
- 
  /* Return true if OP a valid shift count operand.
     OP is the current operation.
     MODE is the current operation mode.  */
--- 1509,1514 ----
*************** s390_extra_constraint_str (rtx op, int c
*** 1627,1632 ****
--- 1600,1620 ----
        c = str[1];
      }
  
+   /* Check for non-literal-pool variants of memory constraints.  */
+   else if (c == 'B')
+     {
+       if (GET_CODE (op) != MEM)
+ 	return 0;
+       if (!s390_decompose_address (XEXP (op, 0), &addr))
+ 	return 0;
+       if (addr.base && REG_P (addr.base) && REGNO (addr.base) == BASE_REGNUM)
+ 	return 0;
+       if (addr.indx && REG_P (addr.indx) && REGNO (addr.indx) == BASE_REGNUM)
+ 	return 0;
+ 
+       c = str[1];
+     }
+ 
    switch (c)
      {
      case 'Q':
Index: gcc/config/s390/s390.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.h,v
retrieving revision 1.128
diff -c -p -r1.128 s390.h
*** gcc/config/s390/s390.h	3 Nov 2004 21:39:48 -0000	1.128
--- gcc/config/s390/s390.h	9 Nov 2004 15:16:47 -0000
*************** extern const enum reg_class regclass_map
*** 583,589 ****
  
  #define CONSTRAINT_LEN(C, STR)                                  	\
    ((C) == 'N' ? 5 : 							\
!    (C) == 'A' ? 2 : DEFAULT_CONSTRAINT_LEN ((C), (STR)))
  
  /* Stack layout and calling conventions.  */
  
--- 583,590 ----
  
  #define CONSTRAINT_LEN(C, STR)                                  	\
    ((C) == 'N' ? 5 : 							\
!    (C) == 'A' ? 2 :							\
!    (C) == 'B' ? 2 : DEFAULT_CONSTRAINT_LEN ((C), (STR)))
  
  /* Stack layout and calling conventions.  */
  
Index: gcc/config/s390/s390.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.md,v
retrieving revision 1.143
diff -c -p -r1.143 s390.md
*** gcc/config/s390/s390.md	7 Nov 2004 00:01:19 -0000	1.143
--- gcc/config/s390/s390.md	9 Nov 2004 15:16:48 -0000
***************
*** 50,55 ****
--- 50,58 ----
  ;;    T -- Memory reference with index register and long displacement.
  ;;    A -- Multiple letter constraint followed by Q, R, S, or T:
  ;;         Offsettable memory reference of type specified by second letter.
+ ;;    B -- Multiple letter constraint followed by Q, R, S, or T:
+ ;;         Memory reference of the type specified by second letter that
+ ;;         does *not* refer to a literal pool entry.
  ;;    U -- Pointer with short displacement.
  ;;    W -- Pointer with long displacement.
  ;;    Y -- Shift count operand.
***************
*** 525,557 ****
  
  (define_insn "*cmpdi_cct"
    [(set (reg 33)
!         (compare (match_operand:DI 0 "nonimmediate_operand" "d,d,d,m,Q")
!                  (match_operand:DI 1 "general_operand" "d,K,m,d,Q")))]
!   "s390_match_ccmode (insn, CCTmode) && TARGET_64BIT
!    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))"
    "@
     cgr\t%0,%1
     cghi\t%0,%c1
     cg\t%0,%1
-    cg\t%1,%0
     #"
!   [(set_attr "op_type" "RRE,RI,RXY,RXY,SS")])
  
  (define_insn "*cmpsi_cct"
    [(set (reg 33)
!         (compare (match_operand:SI 0 "nonimmediate_operand" "d,d,d,d,R,T,Q")
!                  (match_operand:SI 1 "general_operand" "d,K,R,T,d,d,Q")))]
!   "s390_match_ccmode (insn, CCTmode)
!    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))"
    "@
     cr\t%0,%1
     chi\t%0,%c1
     c\t%0,%1
     cy\t%0,%1
-    c\t%1,%0
-    cy\t%1,%0
     #"
!   [(set_attr "op_type" "RR,RI,RX,RXY,RX,RXY,SS")])
  
  
  ; Compare (signed) instructions
--- 528,555 ----
  
  (define_insn "*cmpdi_cct"
    [(set (reg 33)
!         (compare (match_operand:DI 0 "nonimmediate_operand" "%d,d,d,Q")
!                  (match_operand:DI 1 "general_operand" "d,K,m,BQ")))]
!   "s390_match_ccmode (insn, CCTmode) && TARGET_64BIT"
    "@
     cgr\t%0,%1
     cghi\t%0,%c1
     cg\t%0,%1
     #"
!   [(set_attr "op_type" "RRE,RI,RXY,SS")])
  
  (define_insn "*cmpsi_cct"
    [(set (reg 33)
!         (compare (match_operand:SI 0 "nonimmediate_operand" "%d,d,d,d,Q")
!                  (match_operand:SI 1 "general_operand" "d,K,R,T,BQ")))]
!   "s390_match_ccmode (insn, CCTmode)"
    "@
     cr\t%0,%1
     chi\t%0,%c1
     c\t%0,%1
     cy\t%0,%1
     #"
!   [(set_attr "op_type" "RR,RI,RX,RXY,SS")])
  
  
  ; Compare (signed) instructions
***************
*** 614,669 ****
  
  (define_insn "*cmpdi_ccu"
    [(set (reg 33)
!         (compare (match_operand:DI 0 "nonimmediate_operand" "d,d,Q")
!                  (match_operand:DI 1 "general_operand" "d,m,Q")))]
!   "s390_match_ccmode (insn, CCUmode) && TARGET_64BIT
!    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))"
    "@
     clgr\t%0,%1
     clg\t%0,%1
     #"
!   [(set_attr "op_type" "RRE,RXY,SS")])
  
  (define_insn "*cmpsi_ccu"
    [(set (reg 33)
!         (compare (match_operand:SI 0 "nonimmediate_operand" "d,d,d,Q")
!                  (match_operand:SI 1 "general_operand" "d,R,T,Q")))]
!   "s390_match_ccmode (insn, CCUmode)
!    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))"
    "@
     clr\t%0,%1
     cl\t%0,%1
     cly\t%0,%1
     #"
!   [(set_attr "op_type" "RR,RX,RXY,SS")])
  
  (define_insn "*cmphi_ccu"
    [(set (reg 33)
!         (compare (match_operand:HI 0 "nonimmediate_operand" "d,d,Q")
!                  (match_operand:HI 1 "general_operand" "Q,S,Q")))]
    "s390_match_ccmode (insn, CCUmode)
-    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))
     && !register_operand (operands[1], HImode)"
    "@
     clm\t%0,3,%S1
     clmy\t%0,3,%S1
     #"
!   [(set_attr "op_type" "RS,RSY,SS")])
  
  (define_insn "*cmpqi_ccu"
    [(set (reg 33)
!         (compare (match_operand:QI 0 "nonimmediate_operand" "d,d,Q,S,Q")
!                  (match_operand:QI 1 "general_operand" "Q,S,n,n,Q")))]
    "s390_match_ccmode (insn, CCUmode)
-    && (!s390_pool_operand (operands[0]) || !s390_pool_operand (operands[1]))
     && !register_operand (operands[1], QImode)"
    "@
     clm\t%0,1,%S1
     clmy\t%0,1,%S1
     cli\t%S0,%b1
     cliy\t%S0,%b1
     #"
!   [(set_attr "op_type" "RS,RSY,SI,SIY,SS")])
  
  
  ; Block compare (CLC) instruction patterns.
--- 612,667 ----
  
  (define_insn "*cmpdi_ccu"
    [(set (reg 33)
!         (compare (match_operand:DI 0 "nonimmediate_operand" "d,d,Q,BQ")
!                  (match_operand:DI 1 "general_operand" "d,m,BQ,Q")))]
!   "s390_match_ccmode (insn, CCUmode) && TARGET_64BIT"
    "@
     clgr\t%0,%1
     clg\t%0,%1
+    #
     #"
!   [(set_attr "op_type" "RRE,RXY,SS,SS")])
  
  (define_insn "*cmpsi_ccu"
    [(set (reg 33)
!         (compare (match_operand:SI 0 "nonimmediate_operand" "d,d,d,Q,BQ")
!                  (match_operand:SI 1 "general_operand" "d,R,T,BQ,Q")))]
!   "s390_match_ccmode (insn, CCUmode)"
    "@
     clr\t%0,%1
     cl\t%0,%1
     cly\t%0,%1
+    #
     #"
!   [(set_attr "op_type" "RR,RX,RXY,SS,SS")])
  
  (define_insn "*cmphi_ccu"
    [(set (reg 33)
!         (compare (match_operand:HI 0 "nonimmediate_operand" "d,d,Q,BQ")
!                  (match_operand:HI 1 "general_operand" "Q,S,BQ,Q")))]
    "s390_match_ccmode (insn, CCUmode)
     && !register_operand (operands[1], HImode)"
    "@
     clm\t%0,3,%S1
     clmy\t%0,3,%S1
+    #
     #"
!   [(set_attr "op_type" "RS,RSY,SS,SS")])
  
  (define_insn "*cmpqi_ccu"
    [(set (reg 33)
!         (compare (match_operand:QI 0 "nonimmediate_operand" "d,d,Q,S,Q,BQ")
!                  (match_operand:QI 1 "general_operand" "Q,S,n,n,BQ,Q")))]
    "s390_match_ccmode (insn, CCUmode)
     && !register_operand (operands[1], QImode)"
    "@
     clm\t%0,1,%S1
     clmy\t%0,1,%S1
     cli\t%S0,%b1
     cliy\t%S0,%b1
+    #
     #"
!   [(set_attr "op_type" "RS,RSY,SI,SIY,SS,SS")])
  
  
  ; Block compare (CLC) instruction patterns.
*** /dev/null	Mon Jul 12 10:01:50 2004
--- gcc/testsuite/gcc.dg/20041109-1.c	Tue Nov  9 17:52:18 2004
***************
*** 0 ****
--- 1,21 ----
+ /* This used to ICE due to a literal pool handling bug on s390x.  */
+ 
+ /* { dg-do compile { target s390*-*-* } } */
+ /* { dg-options "-O2 -fno-omit-frame-pointer" } */
+ 
+ static struct table { int x; } table[3];
+ 
+ int test (void)
+ {
+   struct table *t;
+ 
+   for (t = table; t < &table[3]; t++)
+     asm volatile ("" : : : "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "12");
+ 
+   for (t = table; t < &table[3]; t++)
+     if (t->x)
+       return 1;
+ 
+   return 0;
+ }
+ 
-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com



More information about the Gcc-patches mailing list