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]

Splitting call_insns


Even though this patch fixes a SH-specific problem, there is one fix
in generic code (emit-rtl.c), so I'll need approval from someone with
global write privileges.

The problem, as NIIBE Yutaka observed, is that we've abused
CODE_LABELs to emit PC-relative calls.  A simple testcase that
demonstrates the problem is included in the patch below.

The solution, proposed originally by Richard Henderson, then partially
implemented by Yutaka-san (or should it be NIIBE-san; sorry, I don't
know :-(, was to create an intermediate call pattern, that would only
be split after all loop optimizations, so we would generate one label
for each call site.  For some reason, the original test on
`!flag_unroll_loops' to decide whether to emit a PC-relative call
didn't work; we seem to be doing some loop unrolling even when
-funroll-loops isn't given.  I haven't investigated it any further.
Anyway, this new approach is far superior, since we won't disable it
just because loop unrolling is enabled.  However, we won't do
PC-relative calls if optimization isn't enabled, because otherwise the
intermediate call pattern would be split too late.  Without
optimization, we'll take the function address from the GOT.  No big
deal, since it's not supposed to be efficient anyway :-)

I had to adjust emit-rtl.c so as to propagate function usage
information from the a split insn into call insns that appear in the
generated SEQ.

It turned out that the new predicate symbol_ref_operand was necessary,
after all.  immediate_operand isn't suitable because, even though it
accepts SYMBOL_REFs, it checks whether the operand is a valid
constant, but, when generating PIC, we don't accept SYMBOL_REFs as
valid constants.

No regressions on sh-elf (m3, m3e, m4 and m4/ml) both with and without
-fPIC.  Ok to install?

Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.c-torture/compile/20001123-1.c: New.

Index: gcc/testsuite/gcc.c-torture/compile/20001123-1.c
===================================================================
RCS file: 20001123-1.c
diff -N 20001123-1.c
--- /dev/null	Tue May  5 13:32:27 1998
+++ gcc/testsuite/gcc.c-torture/compile/20001123-1.c Thu Nov 23 02:09:49 2000
@@ -0,0 +1,21 @@
+/* Copyright 2000 Free Software Foundation
+
+   by Alexandre Oliva  <aoliva@redhat.com>
+
+   Based on zlib/gzio.c.
+
+   This used to generate duplicate labels when compiled with
+   sh-elf-gcc -O2 -m3 -fPIC.
+
+   Bug reported by NIIBE Yutaka <gniibe@m17n.org>.  */
+
+void foo (void);
+
+void
+bar ()
+{
+    unsigned len;
+
+    for (len = 0; len < 2; len++)
+	foo ();
+}
Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>, NIIBE Yutaka  <gniibe@m17n.org>

	* config/sh/sh-protos.h (symbol_ref_operand): Declare.
	* config/sh/sh.md (UNSPEC_CALLER): New constant.
	(calli_pcrel, call_valuei_pcrel): Use PIC_REG.
	(call_pcrel, call_value_pcrel): New insn_and_splits.
	(call, call_value): Use them.
	(call_site): New expand.
	(sym_label2reg, symPLT_label2reg): Adjust to hold call_sites.
	* config/sh/sh.h (OUTPUT_ADDR_CONST_EXTRA) [UNSPEC_CALLER]:
	Output call_site label.
	(PREDICATE_CODES): Added symbol_ref_operand.
	* config/sh/sh.c (symbol_ref_operand): Define.
	* emit-rtl.c (try_split): Propagate CALL_INSN_FUNCTION_USAGE
	to CALL_INSNs in the split sequence.

Index: gcc/emit-rtl.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/emit-rtl.c,v
retrieving revision 1.154
diff -u -p -r1.154 emit-rtl.c
--- gcc/emit-rtl.c 2000/11/10 16:01:13 1.154
+++ gcc/emit-rtl.c 2000/11/23 04:04:43
@@ -2435,6 +2435,14 @@ try_split (pat, trial, last)
 		    LABEL_NUSES (JUMP_LABEL (trial))++;
 		}
 
+	  /* If we are splitting a CALL_INSN, look for the CALL_INSN
+	     in SEQ and copy our CALL_INSN_FUNCTION_USAGE to it.  */
+	  if (GET_CODE (trial) == CALL_INSN)
+	    for (i = XVECLEN (seq, 0) - 1; i >= 0; i--)
+	      if (GET_CODE (XVECEXP (seq, 0, i)) == CALL_INSN)
+		CALL_INSN_FUNCTION_USAGE (XVECEXP (seq, 0, i))
+		  = CALL_INSN_FUNCTION_USAGE (trial);
+
 	  tem = emit_insn_after (seq, before);
 
 	  delete_insn (trial);
Index: gcc/config/sh/sh-protos.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sh/sh-protos.h,v
retrieving revision 1.11
diff -u -p -r1.11 sh-protos.h
--- gcc/config/sh/sh-protos.h 2000/10/31 15:34:27 1.11
+++ gcc/config/sh/sh-protos.h 2000/11/23 12:55:30
@@ -77,6 +77,7 @@ extern int regs_used PARAMS ((rtx, int))
 extern void fixup_addr_diff_vecs PARAMS ((rtx));
 extern int get_dest_uid PARAMS ((rtx, int));
 extern void final_prescan_insn PARAMS ((rtx, rtx *, int));
+extern int symbol_ref_operand PARAMS ((rtx, enum machine_mode));
 extern int system_reg_operand PARAMS ((rtx, enum machine_mode));
 extern int general_movsrc_operand PARAMS ((rtx, enum machine_mode));
 extern int general_movdst_operand PARAMS ((rtx, enum machine_mode));
Index: gcc/config/sh/sh.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sh/sh.c,v
retrieving revision 1.75
diff -u -p -r1.75 sh.c
--- gcc/config/sh/sh.c 2000/11/23 06:37:23 1.75
+++ gcc/config/sh/sh.c 2000/11/23 12:55:36
@@ -4826,6 +4795,14 @@ fpul_operand (op, mode)
   return (GET_CODE (op) == REG
 	  && (REGNO (op) == FPUL_REG || REGNO (op) >= FIRST_PSEUDO_REGISTER)
 	  && GET_MODE (op) == mode);
+}
+
+int
+symbol_ref_operand (op, mode)
+     rtx op;
+     enum machine_mode mode ATTRIBUTE_UNUSED;
+{
+  return (GET_CODE (op) == SYMBOL_REF);
 }
 
 int
Index: gcc/config/sh/sh.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sh/sh.h,v
retrieving revision 1.84
diff -u -p -r1.84 sh.h
--- gcc/config/sh/sh.h 2000/11/23 06:37:23 1.84
+++ gcc/config/sh/sh.h 2000/11/23 12:55:38
@@ -2182,6 +2169,15 @@ do { char dstr[30];					\
 	    output_addr_const ((STREAM), XVECEXP ((X), 0, 0));		\
 	    fputs ("@PLT", (STREAM));					\
 	    break;							\
+	  case UNSPEC_CALLER:						\
+	    {								\
+	      char name[32];						\
+	      /* LPCS stands for Label for PIC Call Site.  */		\
+	      ASM_GENERATE_INTERNAL_LABEL				\
+		(name, "LPCS", XINT (XVECEXP ((X), 0, 0), 0));		\
+	      assemble_name ((STREAM), name);				\
+	    }								\
+	    break;							\
 	  default:							\
 	    goto FAIL;							\
 	  }								\
@@ -2278,7 +2274,8 @@ extern struct rtx_def *fpscr_rtx;
   {"general_movdst_operand", {SUBREG, REG, MEM}},			\
   {"logical_operand", {SUBREG, REG, CONST_INT}},			\
   {"noncommutative_float_operator", {MINUS, DIV}},			\
-  {"register_operand", {SUBREG, REG}},
+  {"register_operand", {SUBREG, REG}},					\
+  {"symbol_ref_operand", {SYMBOL_REF}},
 
 /* Define this macro if it is advisable to hold scalars in registers
    in a wider mode than that declared by the program.  In such cases, 
Index: gcc/config/sh/sh.md
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sh/sh.md,v
retrieving revision 1.59
diff -u -p -r1.59 sh.md
--- gcc/config/sh/sh.md 2000/11/23 06:37:23 1.59
+++ gcc/config/sh/sh.md 2000/11/23 12:55:41
@@ -108,6 +108,7 @@
   (UNSPEC_GOT		7)
   (UNSPEC_GOTOFF	8)
   (UNSPEC_PLT		9)
+  (UNSPEC_CALLER	10)
   (UNSPEC_ICACHE	12)
 
   ;; These are used with unspec_volatile.
@@ -3385,6 +3386,7 @@
   [(call (mem:SI (match_operand:SI 0 "arith_reg_operand" "r"))
 	 (match_operand 1 "" ""))
    (use (reg:SI FPSCR_REG))
+   (use (reg:SI PIC_REG))
    (use (match_operand 2 "" ""))
    (clobber (reg:SI PR_REG))]
   "TARGET_SH2"
@@ -3395,6 +3397,34 @@
 		      (const_string "single") (const_string "double")))
    (set_attr "needs_delay_slot" "yes")])
 
+(define_insn_and_split "call_pcrel"
+  [(call (mem:SI (match_operand:SI 0 "symbol_ref_operand" ""))
+	 (match_operand 1 "" ""))
+   (use (reg:SI FPSCR_REG))
+   (use (reg:SI PIC_REG))
+   (clobber (reg:SI PR_REG))
+   (clobber (match_scratch:SI 2 "=r"))]
+  "TARGET_SH2 && optimize"
+  "#"
+  "reload_completed"
+  [(const_int 0)]
+  "
+{
+  rtx lab = gen_call_site ();
+
+  if (SYMBOL_REF_FLAG (operands[0]))
+    emit_insn (gen_sym_label2reg (operands[2], operands[0], lab));
+  else
+    emit_insn (gen_symPLT_label2reg (operands[2], operands[0], lab));
+  emit_call_insn (gen_calli_pcrel (operands[2], operands[1], lab));
+  DONE;
+}"
+  [(set_attr "type" "call")
+   (set (attr "fp_mode")
+	(if_then_else (eq_attr "fpu_single" "yes")
+		      (const_string "single") (const_string "double")))
+   (set_attr "needs_delay_slot" "yes")])
+
 (define_insn "call_valuei"
   [(set (match_operand 0 "" "=rf")
 	(call (mem:SI (match_operand:SI 1 "arith_reg_operand" "r"))
@@ -3414,6 +3444,7 @@
 	(call (mem:SI (match_operand:SI 1 "arith_reg_operand" "r"))
 	      (match_operand 2 "" "")))
    (use (reg:SI FPSCR_REG))
+   (use (reg:SI PIC_REG))
    (use (match_operand 3 "" ""))
    (clobber (reg:SI PR_REG))]
   "TARGET_SH2"
@@ -3424,6 +3455,36 @@
 		      (const_string "single") (const_string "double")))
    (set_attr "needs_delay_slot" "yes")])
 
+(define_insn_and_split "call_value_pcrel"
+  [(set (match_operand 0 "" "=rf")
+	(call (mem:SI (match_operand:SI 1 "symbol_ref_operand" ""))
+	      (match_operand 2 "" "")))
+   (use (reg:SI FPSCR_REG))
+   (use (reg:SI PIC_REG))
+   (clobber (reg:SI PR_REG))
+   (clobber (match_scratch:SI 3 "=r"))]
+  "TARGET_SH2 && optimize"
+  "#"
+  "reload_completed"
+  [(const_int 0)]
+  "
+{
+  rtx lab = gen_call_site ();
+
+  if (SYMBOL_REF_FLAG (operands[1]))
+    emit_insn (gen_sym_label2reg (operands[3], operands[1], lab));
+  else
+    emit_insn (gen_symPLT_label2reg (operands[3], operands[1], lab));
+  emit_call_insn (gen_call_valuei_pcrel (operands[0], operands[3],
+					 operands[2], lab));
+  DONE;
+}"
+  [(set_attr "type" "call")
+   (set (attr "fp_mode")
+	(if_then_else (eq_attr "fpu_single" "yes")
+		      (const_string "single") (const_string "double")))
+   (set_attr "needs_delay_slot" "yes")])
+
 (define_expand "call"
   [(parallel [(call (mem:SI (match_operand 0 "arith_reg_operand" ""))
 			    (match_operand 1 "" ""))
@@ -3432,18 +3493,12 @@
   ""
   "
 {
-  if (flag_pic && TARGET_SH2 && ! flag_unroll_loops
+  if (flag_pic && TARGET_SH2 && optimize
       && GET_CODE (operands[0]) == MEM
       && GET_CODE (XEXP (operands[0], 0)) == SYMBOL_REF)
     {
-      rtx reg = gen_reg_rtx (SImode), lab = gen_label_rtx ();
-
-      if (SYMBOL_REF_FLAG (XEXP (operands[0], 0)))
-	emit_insn (gen_sym_label2reg (reg, XEXP (operands[0], 0), lab));
-      else
-	emit_insn (gen_symPLT_label2reg (reg, XEXP (operands[0], 0), lab));
-      operands[0] = reg;
-      emit_call_insn (gen_calli_pcrel (operands[0], operands[1], lab));
+      emit_call_insn (gen_call_pcrel (XEXP (operands[0], 0), operands[1]));
+      current_function_uses_pic_offset_table = 1;
       DONE;
     }
   else
@@ -3459,19 +3513,13 @@
   ""
   "
 {
-  if (flag_pic && TARGET_SH2 && ! flag_unroll_loops
+  if (flag_pic && TARGET_SH2 && optimize
       && GET_CODE (operands[1]) == MEM
       && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF)
     {
-      rtx reg = gen_reg_rtx (SImode), lab = gen_label_rtx ();
-
-      if (SYMBOL_REF_FLAG (XEXP (operands[1], 0)))
-	emit_insn (gen_sym_label2reg (reg, XEXP (operands[1], 0), lab));
-      else
-	emit_insn (gen_symPLT_label2reg (reg, XEXP (operands[1], 0), lab));
-      operands[1] = reg;
-      emit_call_insn (gen_call_valuei_pcrel (operands[0], operands[1],
-					     operands[2], lab));
+      emit_call_insn (gen_call_value_pcrel (operands[0], XEXP (operands[1], 0),
+					    operands[2]));
+      current_function_uses_pic_offset_table = 1;
       DONE;
     }
   else
@@ -3588,13 +3635,22 @@
 }
 ")
 
+(define_expand "call_site"
+  [(unspec [(match_dup 0)] UNSPEC_CALLER)]
+  ""
+  "
+{
+  static HOST_WIDE_INT i = 0;
+  operands[0] = GEN_INT (i);
+  i++;
+}")
+
 (define_expand "sym_label2reg"
   [(set (match_operand:SI 0 "" "")
 	(const (minus:SI
 		(const (unspec [(match_operand:SI 1 "" "")] UNSPEC_PIC))
 		(const (plus:SI
-			(unspec [(label_ref (match_operand:SI 2 "" ""))]
-				UNSPEC_PIC)
+			(match_operand:SI 2 "" "")
 			(const_int 2))))))]
   "" "")
 
@@ -3633,8 +3686,7 @@
 			(unspec [(match_operand:SI 1 "" "")] UNSPEC_PLT)
 			(pc)))
 		(const (plus:SI
-			(unspec [(label_ref (match_operand:SI 2 "" ""))]
-				UNSPEC_PIC)
+			(match_operand:SI 2 "" "")
 			(const_int 2))))))
    (use (match_dup 3))]
   ;; Even though the PIC register is not really used by the call

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

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