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]

Re: patch for ia64 builtin setjmp


Andrew Macleod wrote on May 22:

> The following patch has been checked into mainline and gcc 3.0 branch 
> which allows the builtin-setjmp testcases in c-torture to compile and 
> execute on ia64.

I've been looking into __builtin_longjmp and non-local gotos on ia64 as
an introduction to gcc internals and believe there is a problem with
Andrew's fix for __builtin_longjmp.

Changing the order of arguments to __ia64_nonlocal_goto() breaks ia64
non-local gotos with optimizations levels of -O2 or higher.  That hasn't
been noticeable yet, however, because non-local gotos are broken on ia64
for the same reason that __builtin_longjmp could cause the compiler to
seg fault.

The fix to expand_builtin_longjmp() needs to be replicated in
expand_nonlocal_goto().  That allows test c-torture/execute/920428-2.c
to execute correctly.  With the change to ia64.md, however, it fails at
higher levels of optimization.

The documentation for __nonlocal_goto is contradictory, and it seems
to be used differently by different architectures.  In md.texi:

  Emit code to generate a non-local goto, e.g., a jump from one function
  to a label in an outer function.  This pattern has four arguments,
  each representing a value to be used in the jump.  The first
  argument is to be loaded into the frame pointer, the second is
  the address to branch to (code to dispatch to the actual label),
  the third is the address of a location where the stack is saved,
  and the last is the address of the label, to be placed in the
  location for the incoming static chain.

  On most machines you need not define this pattern, since GNU CC will
  already generate the correct code, which is to load the frame pointer
  and static chain, restore the stack (using the
  @samp{restore_stack_nonlocal} pattern, if defined), and jump indirectly
  to the dispatcher.  You need only define this pattern if this code will
  not work on your machine.

At the function definition itself is:

// void __ia64_nonlocal_goto(void *target_label, void *save_area,
//                           void *static_chain);

The code to generate the call in expand_nonlocal_goto():

        emit_insn (gen_nonlocal_goto (static_chain, handler_slot,
                                      save_area, label_ref));

The code in expand_builtin_longjmp() to generate a call to nonlocal_goto:

     emit_insn (gen_nonlocal_goto (value, fp, stack, lab));

The expansion of gen_nonlocal_goto ignores the first argument and passes
on the last three.  The second argument (the first passed to
__ia64_nonlocal_goto) really is the handler_slot, and as the
documentation in md.texi says, this is the address that is jumped to.

With optimization levels of -O1 and lower the code at handler_slot
merely jumps to the label, but at higher levels it has additional code
and jumping to the label instead causes some instructions to be skipped.

The arguments passed for __builtin_longjmp seem to be very
architecture-specific; changing how they are passed in general would
allow ia64 to work but would break others.

Adding an expansion rule for __builtin_longjmp for ia64 fixes that
functionality for ia64, doesn't break non-local gotos for ia64, and
doesn't break any functionality for other architecures.

Here's my proposed patch, which assumes that the changes for expanding
nonlocal_goto have been reverted.  It's been tested with bootstrap and
testing on x86-linux and ia64-linux with 3.0 snapshot 20010521 plus
Andrew's changes to builtins.c.

	* stmt.c (expand_goto): A nonlocal goto can be a call or a jump.
	* config/ia64/ia64.md (builtin_longjmp) New pattern for ia64 to
	set up a call to __ia64_nonlocal_goto with parameters in the
	correct order.  Flag calls to __ia64_nonlocal_goto as NO_RETURN.
	* builtins.c (expand_builtin_longjmp): Add comment warning that
	these calls to nonlocal_goto don't use the documented arguments.
	* config/ia64/lib1funcs.asm (__ia64_nonlocal_goto): Fix comment
	describing the parameters.

--- gcc/stmt.c.orig	Tue May 22 17:00:57 2001
+++ gcc/stmt.c	Wed May 23 11:17:00 2001
@@ -842,12 +842,17 @@
 
       /* Search backwards to the jump insn and mark it as a 
 	 non-local goto.  */
-      for (insn = get_last_insn ();
-	   GET_CODE (insn) != JUMP_INSN; 
-	   insn = PREV_INSN (insn))
-	continue;
-      REG_NOTES (insn) = alloc_EXPR_LIST (REG_NON_LOCAL_GOTO, const0_rtx,
-					  REG_NOTES (insn));
+      for (insn = get_last_insn (); insn; insn = PREV_INSN (insn))
+	{
+	  if (GET_CODE (insn) == JUMP_INSN)
+	    {
+	      REG_NOTES (insn) = alloc_EXPR_LIST (REG_NON_LOCAL_GOTO,
+						  const0_rtx, REG_NOTES (insn));
+	      break;
+	    }
+	  else if (GET_CODE (insn) == CALL_INSN)
+	      break;
+	}
     }
   else
     expand_goto_internal (label, label_rtx (label), NULL_RTX);
--- gcc/config/ia64/ia64.md.orig	Wed May 23 15:38:35 2001
+++ gcc/config/ia64/ia64.md	Wed May 23 15:51:21 2001
@@ -4971,7 +4971,7 @@
   "
 {
   emit_library_call (gen_rtx_SYMBOL_REF (Pmode, \"__ia64_nonlocal_goto\"),
-		     0, VOIDmode, 3,
+		     LCT_NORETURN, VOIDmode, 3,
 		     operands[1], Pmode,
 		     copy_to_reg (XEXP (operands[2], 0)), Pmode,
 		     operands[3], Pmode);
@@ -4980,7 +4980,6 @@
 }")
 
 ;; The rest of the setjmp processing happens with the nonlocal_goto expander.
-;; ??? This is not tested.
 (define_expand "builtin_setjmp_setup"
   [(use (match_operand:DI 0 "" ""))]
   ""
@@ -4999,6 +4998,30 @@
   DONE;
 }")
 
+;; The code for __builtin_setjmp stores the frame pointer, the label
+;; address, and the stack save area into a user-supplied buffer.  The
+;; code in expand_builtin_longjmp passes nonlocal goto arguments in a
+;; different order than __ia64_nonlocal_goto expects.  Set things up
+;; here so they are passed in the expected order.
+(define_expand "builtin_longjmp"
+  [(use (match_operand:DI 0 "" ""))]
+  ""
+  "
+{
+  rtx fp = gen_rtx_MEM (Pmode, operands[0]);
+  rtx lab = gen_rtx_MEM (Pmode, plus_constant (operands[0],
+                                               GET_MODE_SIZE (Pmode)));
+  rtx stack = gen_rtx_MEM(Pmode, plus_constant (operands[0],
+                                                2 * GET_MODE_SIZE (Pmode)));
+
+  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, \"__ia64_nonlocal_goto\"),
+                     LCT_NORETURN, VOIDmode, 3,
+                     lab, Pmode,
+                     copy_to_reg (XEXP (stack, 0)), Pmode,
+                     fp, Pmode);
+  DONE;
+}")
+
 (define_expand "eh_epilogue"
   [(use (match_operand:DI 0 "register_operand" "r"))
    (use (match_operand:DI 1 "register_operand" "r"))
--- gcc/builtins.c.orig	Wed May 23 11:23:31 2001
+++ gcc/builtins.c	Wed May 23 11:26:28 2001
@@ -695,6 +695,12 @@
 	 from expand_goto in stmt.c; see there for detailed comments.  */
 #if HAVE_nonlocal_goto
       if (HAVE_nonlocal_goto)
+	/* Documentation of the nonlocal_goto pattern doesn't match its
+	   use here and how it is used to implement non-local gotos.  The
+	   documented arguments are (static_chain, handler_slot, save_area,
+	   label_ref).  The arguments passed here are expected by some
+	   architecutures based on the values saved for __builtin_setjump. */
+
 	/* We have to pass a value to the nonlocal_goto pattern that will
 	   get copied into the static_chain pointer, but it does not matter
 	   what that value is, because builtin_setjmp does not use it.  */
--- gcc/config/ia64/lib1funcs.asm.prev	Wed May 23 11:20:11 2001
+++ gcc/config/ia64/lib1funcs.asm	Wed May 23 11:21:27 2001
@@ -557,8 +557,8 @@
 #endif
 
 #ifdef L__nonlocal_goto
-// void __ia64_nonlocal_goto(void *target_label, void *save_area,
-//			     void *static_chain);
+// void __ia64_nonlocal_goto(void *handler_slot, void *save_area,
+//			     void *label_ref);
 
 	.text
 	.align 16

Janis


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