RE: EH on i960 (and nested functions)

RE: EH on i960 (and nested functions)

[Don Bowman]

> 
> > Compile with -O -S, and note that main sets the static chain
> > pointer, r3, before the call to sub, and sub sets `i' by storing
> > through r3.  This however won't work because r3 is a local register,
> > and local registers are switched on calls.
> 
> Apparently not $r3 on my i960 CA, nor, as documented in the Intel i960
> Cx users manual.
> 

r3 is a local register on the i960 (all r registers are).
Its been my experience that the compiler distains r3 since
it is at the end of a quad that are used (r0, r1, r2 have
special purposes).
r0 == previous frame pointer
r1 == stack pointer
r2 == return instruction pointer
In 2-1 of the i960 CA Users Manual (1992 ed), 
 "The global register numbers are g0 through g15; local register numbers
are r0 through r15.

In fact, the code generated may works sometimes. The i960 does 
not always clear or initialize the set of local registers in a new
procedure. It depends on the amount of the data ram used for a register
cache, and the depth of the call-tree. If the registers are spilled to
ram the results may be unpredictable.

FYI: the code shown below ran on my i960 board even though it is
clearly incorrect.

$ i960-wrs-vxworks-gcc -v
Reading specs from \usr\local\lib\gcc-lib\i960-wrs-vxworks\egcs-2.91.15\specs
gcc version egcs-2.91.15 980321 (gcc-2.8.0 release)
$ i960-wrs-vxworks-gcc -O -S test.c
$ cat test.s
        .file   "test.c"
gcc2_compiled.:
___gnu_compiled_c:
.text
        .align 4
        #  Function 'sub.2'
        #  Registers used: g4 r3*
        #
_sub.2:
        addo    16,sp,sp
        #Prologue stats:
        #  Total Frame Size: 16 bytes
        #  Local Variable Size: 16 bytes
        #End Prologue#
        st      r3,-16(sp)
        mov     10,g4
        st      g4,(r3)
        ret
        .align 3
LC0:
        .ascii "i = %d\12\0"
        .align 4
        .globl _main
        #  Function 'main'
        #  Registers used: g0 fp sp
        #                  r3*
_main:
        addo    16,sp,sp
        #Prologue stats:
        #  Total Frame Size: 16 bytes
        #  Local Variable Size: 16 bytes
        #End Prologue#
        callx   ___main
        st      g14,64(fp)
        lda     64(fp),r3    <<<<<<<<<<<<<<<
        callx   _sub.2
        ldconst LC0,g0
        callx   _printf
        ret

Re: EH on i960 (and nested functions)

[Mike Stump]

> Date: Wed, 22 Apr 1998 13:06:54 -0700
> From: Jim Wilson <wilson@cygnus.com>

> These patches will fix some nested function problems, but not the
> particular one that I was talking about.

> main()
> {
>   int i = 0;

>   void sub ()
>     {
>       i = 10;
>     };

>   sub ();
>   printf ("i = %d\n");
> }

If I correct it to read:

 printf ("i = %d\n", i);

it works as expected for me.  When I review the code, I see the what
you were talking about, but my i960 CA doesn't clobber r3 in this
testcase before it uses it, so while a problem may exist, this isn't a
testcase for it.

> Compile with -O -S, and note that main sets the static chain
> pointer, r3, before the call to sub, and sub sets `i' by storing
> through r3.  This however won't work because r3 is a local register,
> and local registers are switched on calls.

Apparently not $r3 on my i960 CA, nor, as documented in the Intel i960
Cx users manual.

If you can pull out of your bug bin, something that doesn't work, I
can seek to ensure that it works.

> What is the call1f pattern for?

Ah, thanks for pointing that out.  I think it's a leftover.  I'll
remove it.

I'm still working out -O problems...  (mainly inlining problems, and
the differing notion of what a frame is)

Re: EH on i960 (and nested functions)

[Jim Wilson]

These patches will fix some nested function problems, but not the particular
one that I was talking about.  That is probably my fault for not giving you
a testcase though.  So here's a testcase.

main()
{
  int i = 0;

  void sub ()
    {
      i = 10;
    };

  sub ();
  printf ("i = %d\n");
}

Compile with -O -S, and note that main sets the static chain pointer, r3,
before the call to sub, and sub sets `i' by storing through r3.  This however
won't work because r3 is a local register, and local registers are switched
on calls.

This would work if we used a global register, but all 16 of global registers
already are being used for one purpose or another, so there do not seem to be
any free ones for use as a static chain pointer.  That is why I mentioned a
possible ABI change.  This could work if we did a flushregs in the prologue
and then loaded the static chain pointer into r3 from the stack, but that
would probably be too slow to be really useful.

What is the call1f pattern for?  I suspect it is there to do a local register
bank switch.  If so, this should be documented with a comment.  Otherwise, you
should just be doing an IP (aka PC) relative lda, since the normal reason for
a call to the next instruction is to load the PC into a register.   It isn't
clear why you are doing a register bank switch here.  Doing so seems to cause
problems, because if any of fp/chain/lab get assigned to a local register
by the optimizer, then they will be lost after the call1f.

Overall, the patch looks reasonable.  My only real concern is better comments
explaining what the nonlocal_goto pattern is doing.

Jim

Re: EH on i960 (and nested functions)

[Mike Stump]

> Date: Tue, 21 Apr 1998 13:23:28 -0700
> From: Jim Wilson <wilson@cygnus.com>

> In addition to the changes you mentioned, you presumably also need
> to do something about STATIC_CHAIN_REGNUM. [ ... ]  I don't know
> what to do to solve this though.  I suspect we may need to make an
> ABI change that will affect non-nested functions too.

I am nearly done...  I should have the STATIC_CHAIN_REGNUM restored
properly.  No abi changes seem necessary, though one will need to
recompile all code that had non-local gotos in them (which is ok,
because that code didn't work before).

These changes aren't done, addition work (make sure -O doesn't screw
the code, nonlocal_goto's interface is different) is needed.  Nobody
said how hot kenenr was for the fixes, so I provide them here, just in
case he needs them in the nearer term.  I'll spruce them up and
resubmit them later.  Kenner, if you want to verify these fix the
problem you saw, you can take a look at sparc/sparc.md's nonlocal_goto
pattern for the right operand ordering, the one below was current as
of two years back, and just avoid -O.  If they don't work, let me know
what's missing and I will fix it for you.

Doing diffs in i960.h.~1~:
*** i960.h.~1~	Thu Apr 16 15:03:46 1998
--- i960.h	Thu Apr 16 15:04:24 1998
*************** extern struct rtx_def *gen_compare_reg (
*** 1472,1477 ****
--- 1472,1482 ----
  		  CXT);							\
  }
  
+ /* Generate RTL to flush the register windows so as to make arbitrary frames
+    available.  */
+ #define SETUP_FRAME_ADDRESSES()		\
+   emit_insn (gen_flush_register_windows ())
+ 
  #if 0
  /* Promote char and short arguments to ints, when want compatibility with
     the iC960 compilers.  */
--------------
Doing diffs in i960.md.~1~:
*** i960.md.~1~	Thu Apr 16 15:03:54 1998
--- i960.md	Mon Apr 20 18:05:15 1998
***************
*** 2419,2424 ****
--- 2419,2485 ----
    "* return i960_output_ret_insn (insn);"
    [(set_attr "type" "branch")])
  
+ (define_insn "ret"
+   [(use (reg:SI 16))
+    (unspec_volatile [(const_int 0)] 3)]
+   ""
+   "ret"
+   [(set_attr "type" "branch")
+    (set_attr "length" "1")])
+ 
+ (define_insn "call1f"
+   [(unspec_volatile [(const_int 0)] 4)]
+   ""
+   "call    1f\;1:"
+   [(set_attr "type" "misc")
+    (set_attr "length" "1")])
+ 
+ (define_expand "nonlocal_goto"
+   [(match_operand:SI 0 "" "")
+    (match_operand:SI 1 "general_operand" "")
+    (match_operand:SI 2 "general_operand" "")
+    (match_operand:SI 3 "general_operand" "")]
+   ""
+   "
+ {
+   rtx fp = operands[0];
+   rtx chain = operands[1];
+   rtx stack = operands[2];
+   rtx lab = operands[3];
+ 
+   if (GET_CODE (fp) != REG)
+     fp = force_reg (Pmode, fp);
+ #if 0  
+   /* Is this needed?  */
+   emit_move_insn (virtual_stack_vars_rtx, fp);
+ #endif
+   if (GET_CODE (lab) != REG)
+     lab = force_reg (Pmode, lab);
+   if (GET_CODE (chain) != REG)
+     chain = force_reg (Pmode, chain);
+   emit_insn (gen_call1f ());
+   emit_insn (gen_flush_register_windows ());
+   emit_move_insn (gen_rtx (MEM, SImode,
+ 			   plus_constant (fp, 8-STARTING_FRAME_OFFSET)),
+ 		  chain);
+   emit_move_insn (gen_rtx (MEM, SImode,
+ 			   plus_constant (fp, 12-STARTING_FRAME_OFFSET)),
+ 		  lab);
+   emit_move_insn (gen_rtx (REG, SImode, 16),
+ 		  plus_constant (fp, -STARTING_FRAME_OFFSET));
+   emit_insn (gen_ret ());
+   emit_barrier ();
+   DONE;
+ }")
+ 
+ ;; Special insn to flush register windows.
+ (define_insn "flush_register_windows"
+   [(unspec_volatile [(const_int 0)] 1)]
+   ""
+   "flushreg"
+   [(set_attr "type" "misc")
+    (set_attr "length" "1")])
+ 
  (define_insn "nop"
    [(const_int 0)]
    ""
--------------

Re: EH on i960 (and nested functions)

[Jim Wilson]

In addition to the changes you mentioned, you presumably also need to do
something about STATIC_CHAIN_REGNUM.  This is currently a local register,
and hence gets lost when a nested function is called.  This means any nested
function that references locals in the parent will fail.  This is why Ada
doesn't work.  I don't know what to do to solve this though.  I suspect we
may need to make an ABI change that will affect non-nested functions too.

	A open question, are the units on the lengths on the i960 in bytes, or
	4 byte words?  It wasn't readily obvious to me.

The lengths are 4 byte words, but this isn't strictly correct.  The lengths
should be in bytes.  This is a harmless bug for now, but if you want to fix
this problem also, that would be great.

Jim

EH on i960 (and nested functions)

[Mike Stump]

If I guess right, nested functions and setjmp based exception handling
do not work well on the i960.  I spoke with Jim about it during the
weekend, and he said someone reported the problem against Ada to
kenner.  Well, I'm working on it now...

The problem is the the various ($fp, $pfp, $rip $sp) registers are not
all set correctly on nonlocal_gotos.  The direction I am going, after
ruling out changing save_stack_nonlocal, is to add a nonlocal_goto to
do all the right stuff.

Also, one also might need:

/* Generate RTL to flush the register windows so as to make arbitrary frames
   available.  */
#define SETUP_FRAME_ADDRESSES()		\
  emit_insn (gen_flush_register_windows ())

;; Special insn to flush register windows.
(define_insn "flush_register_windows"
  [(unspec_volatile [(const_int 0)] 1)]
  ""
  "flushreg"
  [(set_attr "type" "misc")
   (set_attr "length" "1")])

A open question, are the units on the lengths on the i960 in bytes, or
4 byte words?  It wasn't readily obvious to me.

Anyway, once I get this puppy licked, I'll send you guys the changes.