This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[3.0.2 PATCH] S/390 backend bugfixes


This patch backports several bugfixes to the 3.0 branch:

- Removes addsi3_cc and related insns to prevent incorrect 
  comparison code to be generated.  The mainline contains a
  more efficient fix, but this is a major change in condition
  code handling which I don't like to introduce to the branch
  at this point.

- The load address 'la' instructions performs a 31-bit addition,
  not 32-bit, therefore it must not be used to add non-addresses.
  This patch fixes some cases where the la instruction was used
  incorrectly.

- When using -mno-backchain, in some rare corner cases incorrect
  prolog code was generated.

- The operands 0 and 1 of cmpsi_cct do not commute (thanks to 
  Torbjorn Granlund for pointing this out).

In addition, two performance improvements are included, as they fix 
noticable performance regressions compared to our 2.95 compilers:

- Improve legitimize_address to make better use of the two-register
  (base + index + displacement) addressing mode.

- Improve scheduling by fixing incorrect function unit definitions
  and cost function.  Note that this is not the major scheduling
  rework from the mainline, but just a localized fix.

Bootstrapped and regtested on s390-ibm-linux and s390x-ibm-linux.
Installed on the branch.

Bye,
Ulrich

ChangeLog:

	* config/s390/s390.c (s390_adjust_cost): Fixed incorrect scheduling.
	* config/s390/s390.md (function units): Likewise.

	* config/s390/s390.md (addsi3_cc, addsi3_cconly, addsi3_cconly2,
	subsi3_cc, subsi3_cconly): Removed.

	* config/s390/s390.c (legitimate_la_operand_p): New.
	* config/s390/s390-protos.h (legitimate_la_operand_p): Add.
	* config/s390/s390.md (movsi): Convert load address patterns to
	arithmetic operations when necessary.
	(addaddr_picR, addaddr_picL, addaddr_picN): Removed.
	(do_la): Renamed to *do_la and use legitimate_la_operand_p.
	(*do_la_reg_0): Don't use before reload.

	* config/s390/s390.c (legitimize_address): Make more efficient
	use of two-register addressing mode.

	* config/s390/s390.c (s390_function_prologue): Fix incorrect prolog
	with -mno-backchain in some corner cases.

	* config/s390/s390.md (cmpsi_cct): Operands 0 and 1 do not commute.



Index: gcc/config/s390/s390-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390-protos.h,v
retrieving revision 1.1.6.1
diff -c -p -r1.1.6.1 s390-protos.h
*** s390-protos.h	2001/08/03 18:50:11	1.1.6.1
--- s390-protos.h	2001/09/29 22:42:57
*************** extern int bras_sym_operand PARAMS ((rtx
*** 39,44 ****
--- 39,45 ----
  extern int dead_p PARAMS ((rtx, rtx));
  extern void print_operand PARAMS ((FILE *, rtx, char));
  extern void print_operand_address PARAMS ((FILE *, rtx));
+ extern int legitimate_la_operand_p PARAMS ((rtx));
  extern int legitimate_pic_operand_p PARAMS ((rtx));
  extern int legitimate_constant_p PARAMS ((rtx));
  
Index: gcc/config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.1.6.2
diff -c -p -r1.1.6.2 s390.c
*** s390.c	2001/08/10 22:19:19	1.1.6.2
--- s390.c	2001/09/29 22:43:02
*************** legitimate_address_p (mode, addr, strict
*** 813,818 ****
--- 813,852 ----
    return s390_decompose_address (addr, NULL, strict);
  }
  
+ /* Return 1 if OP is a valid operand for the LA instruction.
+    In 31-bit, we need to prove that the result is used as an
+    address, as LA performs only a 31-bit addition.  */
+ 
+ int
+ legitimate_la_operand_p (op)
+      register rtx op;
+ {
+   struct s390_address addr;
+   if (!s390_decompose_address (op, &addr, FALSE))
+     return FALSE;
+ 
+   if (TARGET_64BIT)
+     return TRUE;
+ 
+   /* Use of the base or stack pointer implies address.  */
+ 
+   if (addr.base && GET_CODE (addr.base) == REG)
+     {
+       if (REGNO (addr.base) == BASE_REGISTER
+           || REGNO (addr.base) == STACK_POINTER_REGNUM)
+         return TRUE;
+     }
+ 
+   if (addr.indx && GET_CODE (addr.indx) == REG)
+     {
+       if (REGNO (addr.indx) == BASE_REGISTER
+           || REGNO (addr.indx) == STACK_POINTER_REGNUM)
+         return TRUE;
+     }
+ 
+   return FALSE;
+ }
+ 
  /* Return a legitimate reference for ORIG (an address) using the
     register REG.  If REG is 0, a new pseudo is generated.
  
*************** legitimize_address (x, oldx, mode)
*** 1130,1138 ****
       register rtx oldx ATTRIBUTE_UNUSED;
       enum machine_mode mode;
  {
!   if (flag_pic && SYMBOLIC_CONST (x))
!     return legitimize_pic_address (x, 0);
  
    return x;
  }
  
--- 1164,1211 ----
       register rtx oldx ATTRIBUTE_UNUSED;
       enum machine_mode mode;
  {
!   rtx constant_term = const0_rtx;
  
+   if (flag_pic)
+     {
+       if (SYMBOLIC_CONST (x)
+           || (GET_CODE (x) == PLUS 
+               && (SYMBOLIC_CONST (XEXP (x, 0)) 
+                   || SYMBOLIC_CONST (XEXP (x, 1)))))
+ 	  x = legitimize_pic_address (x, 0);
+ 
+       if (legitimate_address_p (mode, x, FALSE))
+ 	return x;
+     }
+ 
+   x = eliminate_constant_term (x, &constant_term);
+ 
+   if (GET_CODE (x) == PLUS)
+     {
+       if (GET_CODE (XEXP (x, 0)) == REG)
+ 	{
+ 	  register rtx temp = gen_reg_rtx (Pmode);
+ 	  register rtx val  = force_operand (XEXP (x, 1), temp);
+ 	  if (val != temp)
+ 	    emit_move_insn (temp, val);
+ 
+ 	  x = gen_rtx_PLUS (Pmode, XEXP (x, 0), temp);
+ 	}
+ 
+       else if (GET_CODE (XEXP (x, 1)) == REG)
+ 	{
+ 	  register rtx temp = gen_reg_rtx (Pmode);
+ 	  register rtx val  = force_operand (XEXP (x, 0), temp);
+ 	  if (val != temp)
+ 	    emit_move_insn (temp, val);
+ 
+ 	  x = gen_rtx_PLUS (Pmode, temp, XEXP (x, 1));
+ 	}
+     }
+ 
+   if (constant_term != const0_rtx)
+     x = gen_rtx_PLUS (Pmode, x, constant_term);
+ 
    return x;
  }
  
*************** s390_adjust_cost (rtx insn, rtx link, rt
*** 1522,1534 ****
    if (recog_memoized (insn) < 0 || recog_memoized (dep_insn) < 0)
      return cost;
  
-   /* If cost equal 1 nothing needs to be checked. */
- 
-   if (cost == 1)
-     {
-       return cost;
-     }
- 
    dep_rtx = PATTERN (dep_insn);
  
    if (GET_CODE (dep_rtx) == SET)
--- 1595,1600 ----
*************** s390_adjust_cost (rtx insn, rtx link, rt
*** 1542,1548 ****
  	      debug_rtx (dep_insn);
  	      debug_rtx (insn);
  	    }
! 	  return cost;
  	}
      }
  
--- 1608,1614 ----
  	      debug_rtx (dep_insn);
  	      debug_rtx (insn);
  	    }
! 	  return cost + 4;
  	}
      }
  
*************** s390_adjust_cost (rtx insn, rtx link, rt
*** 1560,1572 ****
  		  debug_rtx (dep_insn);
  		  debug_rtx (insn);
  		}
! 	      return cost;
  	    }
  	}
      }
  
    /* default cost.  */
!   return 1;
  }
  
  /* Pool concept for Linux 390:
--- 1626,1638 ----
  		  debug_rtx (dep_insn);
  		  debug_rtx (insn);
  		}
! 	      return cost + 4;
  	    }
  	}
      }
  
    /* default cost.  */
!   return cost;
  }
  
  /* Pool concept for Linux 390:
*************** s390_function_prologue (FILE * file, int
*** 2381,2388 ****
  
        /* Decrement stack.  */
  
!       if (TARGET_BACKCHAIN || (STARTING_FRAME_OFFSET +
! 			       lsize + STACK_POINTER_OFFSET > 4095
  			       || frame_pointer_needed
  			       || current_function_calls_alloca))
  	{
--- 2447,2453 ----
  
        /* Decrement stack.  */
  
!       if (TARGET_BACKCHAIN || (frame_size + STACK_POINTER_OFFSET > 4095
  			       || frame_pointer_needed
  			       || current_function_calls_alloca))
  	{
*************** s390_function_prologue (FILE * file, int
*** 2422,2429 ****
  
        /* Generate backchain.  */
  
!       if (TARGET_BACKCHAIN || (STARTING_FRAME_OFFSET + 
! 			       lsize + STACK_POINTER_OFFSET > 4095
  			       || frame_pointer_needed
  			       || current_function_calls_alloca))
  	{
--- 2487,2493 ----
  
        /* Generate backchain.  */
  
!       if (TARGET_BACKCHAIN || (frame_size + STACK_POINTER_OFFSET > 4095
  			       || frame_pointer_needed
  			       || current_function_calls_alloca))
  	{
Index: gcc/config/s390/s390.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.md,v
retrieving revision 1.1.6.2
diff -c -p -r1.1.6.2 s390.md
*** s390.md	2001/08/10 22:19:19	1.1.6.2
--- s390.md	2001/09/29 22:43:12
***************
*** 77,127 ****
  (define_attr "type" "set,xset,la" (const_string "xset"))
  
  ;
! ; Set operations changing a target register, which could be used for
! ; address generation. Adjust cost will check, if realy applicable. 
  ;
  
  (define_function_unit "memory" 1 0
!   (and (eq_attr "type" "set")
!        (eq_attr "cycle" "1"))
!   5 1 [(eq_attr "atype" "mem")] )
  
  (define_function_unit "memory" 1 0
!   (and (eq_attr "type" "set")
!        (eq_attr "cycle" "2")) 5 2)
  
  (define_function_unit "memory" 1 0
!   (and (eq_attr "type" "set")
!        (eq_attr "cycle" "3")) 5 3)
  
  (define_function_unit "memory" 1 0
!   (and (eq_attr "type" "set")
!        (eq_attr "cycle" "n")) 5 4)
  
- (define_function_unit "memory" 1 0
-   (eq_attr "type" "la") 2 1)
- 
- ;
- ; xset insns, which don't set any valid address register.
- ; Only the issue delay matters.
- ; 
- 
- (define_function_unit "memory" 1 0
-   (and (eq_attr "type" "xset")
-        (eq_attr "cycle" "1")) 1 1)
- 
- (define_function_unit "memory" 1 0
-   (and (eq_attr "type" "xset")
-        (eq_attr "cycle" "2")) 1 2)
- 
- (define_function_unit "memory" 1 0
-   (and (eq_attr "type" "xset")
-        (eq_attr "cycle" "3")) 1 3)
- 
- (define_function_unit "memory" 1 0
-   (and (eq_attr "type" "xset")
-        (eq_attr "cycle" "n")) 1 4)
- 
  ; Operand type. Used to default length attribute values
  
  (define_attr "op_type"
--- 77,98 ----
  (define_attr "type" "set,xset,la" (const_string "xset"))
  
  ;
! ; Dummy function unit.  We only care for the cycle count.
! ; Everthing else is done by ADJUST_COST.
  ;
  
  (define_function_unit "memory" 1 0
!   (eq_attr "cycle" "1") 1 1)
  
  (define_function_unit "memory" 1 0
!   (eq_attr "cycle" "2") 2 2)
  
  (define_function_unit "memory" 1 0
!   (eq_attr "cycle" "3") 3 3)
  
  (define_function_unit "memory" 1 0
!   (eq_attr "cycle" "n") 4 4)
  
  ; Operand type. Used to default length attribute values
  
  (define_attr "op_type"
***************
*** 387,393 ****
  
  (define_insn "*cmpsi_cct"
    [(set (reg 33)
!         (compare (zero_extract:SI (match_operand:SI 0 "register_operand" "%d")
  	                  (match_operand:SI 1 "const1_operand" "")
                            (match_operand:SI 2 "immediate_operand"  "I"))
                   (const_int 0)))]
--- 358,364 ----
  
  (define_insn "*cmpsi_cct"
    [(set (reg 33)
!         (compare (zero_extract:SI (match_operand:SI 0 "register_operand" "d")
  	                  (match_operand:SI 1 "const1_operand" "")
                            (match_operand:SI 2 "immediate_operand"  "I"))
                   (const_int 0)))]
***************
*** 900,905 ****
--- 871,889 ----
  
    if (flag_pic && SYMBOLIC_CONST (operands[1]))
      emit_pic_move (operands, SImode);
+ 
+   /* expr.c tries to load an effective address using 
+      force_reg.  This fails because we don't have a 
+      generic load_address pattern.  Convert the move
+      to a proper arithmetic operation instead, unless
+      it is guaranteed to be OK.  */
+   if (GET_CODE (operands[1]) == PLUS
+       && !legitimate_la_operand_p (operands[1]))
+     {
+       operands[1] = force_operand (operands[1], operands[0]);
+       if (operands[1] == operands[0])
+         DONE;
+     }
  }")
  
  (define_insn "*movsi"
***************
*** 2973,3052 ****
     (set_attr "atype"    "mem")
     (set_attr "type"     "la")])
  
- (define_insn "*addaddr_picR"
-   [(set (match_operand:SI 0 "register_operand" "=d")
-         (plus:SI (match_operand:SI 1 "register_operand" "a")
-                  (unspec:SI [(match_operand:SI 2 "register_operand" "a")] 101)))]
-   ""
-   "la\\t%0,0(%1,%2)"
-   [(set_attr "op_type"  "RX")
-    (set_attr "atype"    "mem")
-    (set_attr "type"     "la")])
- 
- (define_insn "*addaddr_picL"
-   [(set (match_operand:SI 0 "register_operand" "=d")
-         (plus:SI (unspec:SI [(match_operand:SI 2 "register_operand" "a")] 101)
-                  (match_operand:SI 1 "register_operand" "a")))]
-   ""
-   "la\\t%0,0(%1,%2)"
-   [(set_attr "op_type"  "RX")
-    (set_attr "atype"    "mem")
-    (set_attr "type"     "la")])
- 
- (define_insn "*addaddr_picN"
-   [(set (match_operand:SI 0 "register_operand" "=d")
-         (unspec:SI [(match_operand:SI 1 "register_operand" "a")] 101))]
-   ""
-   "la\\t%0,0(%1)"
-   [(set_attr "op_type"  "RX")
-    (set_attr "atype"    "mem")
-    (set_attr "type"     "la")])
- 
- (define_insn "*addsi3_cc"
-   [(set (reg 33) 
-         (compare (plus:SI (match_operand:SI 1 "register_operand" "%0,0,0")
-                           (match_operand:SI 2 "general_operand" "d,K,m"))
-                  (const_int 0)))
-    (set (match_operand:SI 0 "register_operand" "=d,d,d")
-         (plus:SI (match_dup 1) (match_dup 2)))]
-   "s390_match_ccmode(insn, CCSmode)"
-   "@
-    ar\\t%0,%2
-    ahi\\t%0,%h2
-    a\\t%0,%2"
-   [(set_attr "op_type"  "RR,RI,RX")
-    (set_attr "atype"    "reg,reg,mem")  
-    (set_attr "type"     "set")])
- 
- (define_insn "*addsi3_cconly"
-   [(set (reg 33) 
-         (compare (plus:SI (match_operand:SI 1 "register_operand" "%0,0,0")
-                           (match_operand:SI 2 "general_operand" "d,K,m"))
-                  (const_int 0)))
-    (clobber (match_scratch:SI 0 "=d,d,d"))]
-   "s390_match_ccmode(insn, CCSmode)"
-   "@
-    ar\\t%0,%2
-    ahi\\t%0,%h2
-    a\\t%0,%2"
-   [(set_attr "op_type"  "RR,RI,RX")
-    (set_attr "atype"    "reg,reg,mem")  
-    (set_attr "type"     "set")])
- 
- (define_insn "*addsi3_cconly2"
-   [(set (reg 33) 
-         (compare (match_operand:SI 1 "register_operand" "%0,0,0")
-                  (neg:SI (match_operand:SI 2 "general_operand" "d,K,m"))))
-    (clobber (match_scratch:SI 0 "=d,d,d"))]
-   "s390_match_ccmode(insn, CCSmode)"
-   "@
-    ar\\t%0,%2
-    ahi\\t%0,%h2
-    a\\t%0,%2"
-   [(set_attr "op_type"  "RR,RI,RX")
-    (set_attr "atype"    "reg,reg,mem")  
-    (set_attr "type"     "set")])
- 
  (define_insn "addsi3"
    [(set (match_operand:SI 0 "register_operand" "=d,d,d")
          (plus:SI (match_operand:SI 1 "register_operand" "%0,0,0")
--- 2957,2962 ----
***************
*** 3061,3070 ****
     (set_attr "atype"    "reg,reg,mem")
     (set_attr "type"     "set")])
  
! (define_insn "do_la"
    [(set (match_operand:SI 0 "register_operand" "=a")
          (match_operand:QI 1 "address_operand" "p"))]
!   "volatile_ok"
    "la\\t%0,%a1"
    [(set_attr "op_type"  "RX")
     (set_attr "atype"    "mem")
--- 2971,2981 ----
     (set_attr "atype"    "reg,reg,mem")
     (set_attr "type"     "set")])
  
! (define_insn "*do_la"
    [(set (match_operand:SI 0 "register_operand" "=a")
          (match_operand:QI 1 "address_operand" "p"))]
!   "reload_in_progress || reload_completed
!    || legitimate_la_operand_p (operands[1])"
    "la\\t%0,%a1"
    [(set_attr "op_type"  "RX")
     (set_attr "atype"    "mem")
***************
*** 3074,3080 ****
    [(set (match_operand:SI 0 "register_operand" "=d")
          (plus:SI (match_operand:SI 1 "register_operand" "%0")
                   (match_operand:SI 2 "register_operand" "d")))]
!   ""
    "brxle\\t%0,%2,.+4"
    [(set_attr "op_type" "RSI")
     (set_attr "atype"   "reg")
--- 2985,2991 ----
    [(set (match_operand:SI 0 "register_operand" "=d")
          (plus:SI (match_operand:SI 1 "register_operand" "%0")
                   (match_operand:SI 2 "register_operand" "d")))]
!   "reload_in_progress || reload_completed"
    "brxle\\t%0,%2,.+4"
    [(set_attr "op_type" "RSI")
     (set_attr "atype"   "reg")
***************
*** 3284,3318 ****
  ;
  ; subsi3 instruction pattern(s).
  ;
- 
- (define_insn "*subsi3_cc"
-   [(set (reg 33)
-         (compare (minus:SI (match_operand:SI 1 "register_operand" "0,0")
-                            (match_operand:SI 2 "general_operand" "d,m"))
-                  (const_int 0)))
-    (set (match_operand:SI 0 "register_operand" "=d,d")
-         (minus:SI (match_dup 1) (match_dup 2)))]
-   "s390_match_ccmode(insn, CCSmode)"
-   "@
-    sr\\t%0,%2
-    s\\t%0,%2"
-   [(set_attr "op_type"  "RR,RX")
-    (set_attr "atype"    "reg,mem")
-    (set_attr "type"     "set")])
- 
- (define_insn "*subsi3_cconly"
-   [(set (reg 33)
-         (compare (minus:SI (match_operand:SI 1 "register_operand" "0,0")
-                            (match_operand:SI 2 "general_operand" "d,m"))
-                  (const_int 0)))
-    (clobber (match_scratch:SI 0 "=d,d"))]
-   "s390_match_ccmode(insn, CCSmode)"
-   "@
-    sr\\t%0,%2
-    s\\t%0,%2"
-   [(set_attr "op_type"  "RR,RX")
-    (set_attr "atype"    "reg,mem")
-    (set_attr "type"     "set")])
  
  (define_insn "subsi3"
    [(set (match_operand:SI 0 "register_operand" "=d,d")
--- 3195,3200 ----

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]