public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [h8300] Add dwarf2 debugging support
@ 2009-10-13 21:57 DJ Delorie
  2009-10-13 22:48 ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-13 21:57 UTC (permalink / raw)
  To: gcc-patches


This patch adds support for dwarf2 debug info, but without changing
unwinding.  Ok?
	
	* config/h8300/h8300.c (F): New.
	(Fpa): New.
	(h8300_emit_stack_adjustment): Call them.
	(push): Likewise.
	(h8300_expand_prologue): Likewise.
	* config/h8300/h8300.h (DWARF2_FRAME_INFO): Define.
	(DWARF2_UNWIND_INFO): Define.
	(MUST_USE_SJLJ_EXCEPTIONS): Define.
	(INCOMING_RETURN_ADDR_RTX): Define.
	(INCOMING_FRAME_SP_OFFSET): Define.
	(DWARF2_ADDR_SIZE): Define.


Index: config/h8300/h8300.c
===================================================================
--- config/h8300/h8300.c	(revision 152732)
+++ config/h8300/h8300.c	(working copy)
@@ -504,12 +504,38 @@ byte_reg (rtx x, int b)
        /* Save call clobbered registers in non-leaf interrupt		\
 	  handlers.  */							\
        || (h8300_current_function_interrupt_function_p ()		\
 	   && call_used_regs[regno]					\
 	   && !current_function_is_leaf)))
 
+/* We use this to wrap all emitted insns in the prologue.  */
+static rtx
+F (rtx x)
+{
+  RTX_FRAME_RELATED_P (x) = 1;
+  return x;
+}
+
+/* Mark all the subexpressions of the PARALLEL rtx PAR as
+   frame-related.  Return PAR.
+
+   dwarf2out.c:dwarf2out_frame_debug_expr ignores sub-expressions of a
+   PARALLEL rtx other than the first if they do not have the
+   FRAME_RELATED flag set on them.  */
+static rtx
+Fpa (rtx par)
+{
+  int len = XVECLEN (par, 0);
+  int i;
+
+  for (i = 0; i < len; i++)
+    F (XVECEXP (par, 0, i));
+
+  return par;
+}
+
 /* Output assembly language to FILE for the operation OP with operand size
    SIZE to adjust the stack pointer.  */
 
 static void
 h8300_emit_stack_adjustment (int sign, HOST_WIDE_INT size)
 {
@@ -523,28 +549,28 @@ h8300_emit_stack_adjustment (int sign, H
   if (TARGET_H8300
       && size > 4
       && !h8300_current_function_interrupt_function_p ()
       && !(cfun->static_chain_decl != NULL && sign < 0))
     {
       rtx r3 = gen_rtx_REG (Pmode, 3);
-      emit_insn (gen_movhi (r3, GEN_INT (sign * size)));
-      emit_insn (gen_addhi3 (stack_pointer_rtx,
-			     stack_pointer_rtx, r3));
+      F (emit_insn (gen_movhi (r3, GEN_INT (sign * size))));
+      F (emit_insn (gen_addhi3 (stack_pointer_rtx,
+				stack_pointer_rtx, r3)));
     }
   else
     {
       /* The stack adjustment made here is further optimized by the
 	 splitter.  In case of H8/300, the splitter always splits the
 	 addition emitted here to make the adjustment
 	 interrupt-safe.  */
       if (Pmode == HImode)
-	emit_insn (gen_addhi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	F (emit_insn (gen_addhi3 (stack_pointer_rtx,
+				  stack_pointer_rtx, GEN_INT (sign * size))));
       else
-	emit_insn (gen_addsi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	F (emit_insn (gen_addsi3 (stack_pointer_rtx,
+				  stack_pointer_rtx, GEN_INT (sign * size))));
     }
 }
 
 /* Round up frame size SIZE.  */
 
 static HOST_WIDE_INT
@@ -588,13 +614,13 @@ push (int rn)
   if (TARGET_H8300)
     x = gen_push_h8300 (reg);
   else if (!TARGET_NORMAL_MODE)
     x = gen_push_h8300hs_advanced (reg);
   else
     x = gen_push_h8300hs_normal (reg);
-  x = emit_insn (x);
+  x = F (emit_insn (x));
   REG_NOTES (x) = gen_rtx_EXPR_LIST (REG_INC, stack_pointer_rtx, 0);
 }
 
 /* Emit an insn to pop register RN.  */
 
 static void
@@ -682,13 +708,13 @@ h8300_push_pop (int regno, int nregs, in
 
   /* Add the stack adjustment.  */
   offset = GEN_INT ((pop_p ? nregs : -nregs) * 4);
   RTVEC_ELT (vec, i + j) = gen_rtx_SET (VOIDmode, sp,
 					gen_rtx_PLUS (Pmode, sp, offset));
 
-  emit_insn (gen_rtx_PARALLEL (VOIDmode, vec));
+  Fpa (emit_insn (gen_rtx_PARALLEL (VOIDmode, vec)));
 }
 
 /* Return true if X has the value sp + OFFSET.  */
 
 static int
 h8300_stack_offset_p (rtx x, int offset)
@@ -817,13 +843,13 @@ h8300_expand_prologue (void)
     emit_insn (gen_monitor_prologue ());
 
   if (frame_pointer_needed)
     {
       /* Push fp.  */
       push (HARD_FRAME_POINTER_REGNUM);
-      emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
+      F (emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx));
     }
 
   /* Push the rest of the registers in ascending order.  */
   saved_regs = compute_saved_regs ();
   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno += n_regs)
     {
Index: config/h8300/h8300.h
===================================================================
--- config/h8300/h8300.h	(revision 152732)
+++ config/h8300/h8300.h	(working copy)
@@ -144,12 +144,26 @@ extern const char * const *h8_reg_names;
 #define TARGET_DEFAULT (MASK_QUICKCALL)
 #endif
 
 /* Show we can debug even without a frame pointer.  */
 /* #define CAN_DEBUG_WITHOUT_FP */
 
+/* We want dwarf2 info available to gdb.  */
+#define DWARF2_FRAME_INFO        1
+/* We need the RA column in the frame info for gdb, but... */
+#define DWARF2_UNWIND_INFO       1
+/* ... we don't actually support full dwarf2 EH.  */
+#define MUST_USE_SJLJ_EXCEPTIONS 1
+
+/* The return address is pushed on the stack.  */
+#define INCOMING_RETURN_ADDR_RTX   gen_rtx_MEM (VOIDmode, gen_rtx_REG (VOIDmode, STACK_POINTER_REGNUM))
+#define INCOMING_FRAME_SP_OFFSET   (POINTER_SIZE / 8)
+
+/* Readelf and gdb expect this size.  */
+#define DWARF2_ADDR_SIZE	   (POINTER_SIZE / 8)
+
 /* Define this if addresses of constant functions
    shouldn't be put through pseudo regs where they can be cse'd.
    Desirable on machines where ordinary constants are expensive
    but a CALL with constant address is cheap.
 
    Calls through a register are cheaper than calls to named

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-13 21:57 [h8300] Add dwarf2 debugging support DJ Delorie
@ 2009-10-13 22:48 ` Richard Henderson
  2009-10-14  6:04   ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-13 22:48 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/13/2009 02:53 PM, DJ Delorie wrote:
>         /* The stack adjustment made here is further optimized by the
>   	 splitter.  In case of H8/300, the splitter always splits the
>   	 addition emitted here to make the adjustment
>   	 interrupt-safe.  */
>         if (Pmode == HImode)
> -	emit_insn (gen_addhi3 (stack_pointer_rtx,
> -			       stack_pointer_rtx, GEN_INT (sign * size)));
> +	F (emit_insn (gen_addhi3 (stack_pointer_rtx,
> +				  stack_pointer_rtx, GEN_INT (sign * size))));

We don't split insns that are marked frame-related.  So if the comment 
is correct, you'll have to do the split by hand here.

> +/* We want dwarf2 info available to gdb.  */
> +#define DWARF2_FRAME_INFO        1

I'm pretty sure you don't want to define this.  The default in 
dwarf2out.c only enables it with the proper write_symbols setting.

> +/* We need the RA column in the frame info for gdb, but... */
> +#define DWARF2_UNWIND_INFO       1

You don't need to define this either.  This was off only because you 
hadn't yet defined INCOMING_RETURN_ADDR_RTX.

> +/* ... we don't actually support full dwarf2 EH.  */
> +#define MUST_USE_SJLJ_EXCEPTIONS 1

This one you do want though.  =)

> +/* The return address is pushed on the stack.  */
> +#define INCOMING_RETURN_ADDR_RTX   gen_rtx_MEM (VOIDmode, gen_rtx_REG (VOIDmode, STACK_POINTER_REGNUM))

A proper mode for the mem would be good for your karma.

> +/* Readelf and gdb expect this size.  */
> +#define DWARF2_ADDR_SIZE	   (POINTER_SIZE / 8)

You shouldn't need this; that's the default definition.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-13 22:48 ` Richard Henderson
@ 2009-10-14  6:04   ` DJ Delorie
  2009-10-14 16:26     ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-14  6:04 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


How's this one?  I fixed a few other bugs that a full build uncovered,
too.  I chose to just not always tag that to-be-split insn, because
it's difficult to determine in advance what the splitter will do.  The
only way I got that code to trigger was to define an interrupt
function anyway, which one normally wouldn't debug with gdb.


Index: h8300.c
===================================================================
--- h8300.c	(revision 152732)
+++ h8300.c	(working copy)
@@ -504,12 +504,38 @@ byte_reg (rtx x, int b)
        /* Save call clobbered registers in non-leaf interrupt		\
 	  handlers.  */							\
        || (h8300_current_function_interrupt_function_p ()		\
 	   && call_used_regs[regno]					\
 	   && !current_function_is_leaf)))
 
+/* We use this to wrap all emitted insns in the prologue.  */
+static rtx
+F (rtx x)
+{
+  RTX_FRAME_RELATED_P (x) = 1;
+  return x;
+}
+
+/* Mark all the subexpressions of the PARALLEL rtx PAR as
+   frame-related.  Return PAR.
+
+   dwarf2out.c:dwarf2out_frame_debug_expr ignores sub-expressions of a
+   PARALLEL rtx other than the first if they do not have the
+   FRAME_RELATED flag set on them.  */
+static rtx
+Fpa (rtx par)
+{
+  int len = XVECLEN (par, 0);
+  int i;
+
+  for (i = 0; i < len; i++)
+    F (XVECEXP (par, 0, i));
+
+  return par;
+}
+
 /* Output assembly language to FILE for the operation OP with operand size
    SIZE to adjust the stack pointer.  */
 
 static void
 h8300_emit_stack_adjustment (int sign, HOST_WIDE_INT size)
 {
@@ -523,28 +549,33 @@ h8300_emit_stack_adjustment (int sign, H
   if (TARGET_H8300
       && size > 4
       && !h8300_current_function_interrupt_function_p ()
       && !(cfun->static_chain_decl != NULL && sign < 0))
     {
       rtx r3 = gen_rtx_REG (Pmode, 3);
-      emit_insn (gen_movhi (r3, GEN_INT (sign * size)));
-      emit_insn (gen_addhi3 (stack_pointer_rtx,
-			     stack_pointer_rtx, r3));
+      F (emit_insn (gen_movhi (r3, GEN_INT (sign * size))));
+      F (emit_insn (gen_addhi3 (stack_pointer_rtx,
+				stack_pointer_rtx, r3)));
     }
   else
     {
       /* The stack adjustment made here is further optimized by the
 	 splitter.  In case of H8/300, the splitter always splits the
-	 addition emitted here to make the adjustment
-	 interrupt-safe.  */
+	 addition emitted here to make the adjustment interrupt-safe.
+	 FIXME: We don't always tag those, because we don't know what
+	 the splitter will do.  */
       if (Pmode == HImode)
-	emit_insn (gen_addhi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	{
+	  rtx x = emit_insn (gen_addhi3 (stack_pointer_rtx,
+					 stack_pointer_rtx, GEN_INT (sign * size)));
+	  if (size < 4)
+	    F (x);
+	}
       else
-	emit_insn (gen_addsi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	F (emit_insn (gen_addsi3 (stack_pointer_rtx,
+				  stack_pointer_rtx, GEN_INT (sign * size))));
     }
 }
 
 /* Round up frame size SIZE.  */
 
 static HOST_WIDE_INT
@@ -588,13 +619,13 @@ push (int rn)
   if (TARGET_H8300)
     x = gen_push_h8300 (reg);
   else if (!TARGET_NORMAL_MODE)
     x = gen_push_h8300hs_advanced (reg);
   else
     x = gen_push_h8300hs_normal (reg);
-  x = emit_insn (x);
+  x = F (emit_insn (x));
   REG_NOTES (x) = gen_rtx_EXPR_LIST (REG_INC, stack_pointer_rtx, 0);
 }
 
 /* Emit an insn to pop register RN.  */
 
 static void
@@ -631,13 +662,13 @@ pop (int rn)
 
 static void
 h8300_push_pop (int regno, int nregs, int pop_p, int return_p)
 {
   int i, j;
   rtvec vec;
-  rtx sp, offset;
+  rtx sp, offset, x;
 
   /* See whether we can use a simple push or pop.  */
   if (!return_p && nregs == 1)
     {
       if (pop_p)
 	pop (regno);
@@ -682,13 +713,16 @@ h8300_push_pop (int regno, int nregs, in
 
   /* Add the stack adjustment.  */
   offset = GEN_INT ((pop_p ? nregs : -nregs) * 4);
   RTVEC_ELT (vec, i + j) = gen_rtx_SET (VOIDmode, sp,
 					gen_rtx_PLUS (Pmode, sp, offset));
 
-  emit_insn (gen_rtx_PARALLEL (VOIDmode, vec));
+  x = gen_rtx_PARALLEL (VOIDmode, vec);
+  if (!pop_p)
+    x = Fpa (x);
+  emit_insn (x);
 }
 
 /* Return true if X has the value sp + OFFSET.  */
 
 static int
 h8300_stack_offset_p (rtx x, int offset)
@@ -817,13 +851,13 @@ h8300_expand_prologue (void)
     emit_insn (gen_monitor_prologue ());
 
   if (frame_pointer_needed)
     {
       /* Push fp.  */
       push (HARD_FRAME_POINTER_REGNUM);
-      emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
+      F (emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx));
     }
 
   /* Push the rest of the registers in ascending order.  */
   saved_regs = compute_saved_regs ();
   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno += n_regs)
     {
Index: h8300.h
===================================================================
--- h8300.h	(revision 152732)
+++ h8300.h	(working copy)
@@ -144,12 +144,23 @@ extern const char * const *h8_reg_names;
 #define TARGET_DEFAULT (MASK_QUICKCALL)
 #endif
 
 /* Show we can debug even without a frame pointer.  */
 /* #define CAN_DEBUG_WITHOUT_FP */
 
+/* We want dwarf2 info available to gdb...  */
+#define DWARF2_DEBUGGING_INFO        1
+/* ... but we don't actually support full dwarf2 EH.  */
+#define MUST_USE_SJLJ_EXCEPTIONS 1
+
+/* The return address is pushed on the stack.  */
+#define INCOMING_RETURN_ADDR_RTX   gen_rtx_MEM (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM))
+#define INCOMING_FRAME_SP_OFFSET   (POINTER_SIZE / 8)
+
+#define DWARF_CIE_DATA_ALIGNMENT	2
+
 /* Define this if addresses of constant functions
    shouldn't be put through pseudo regs where they can be cse'd.
    Desirable on machines where ordinary constants are expensive
    but a CALL with constant address is cheap.
 
    Calls through a register are cheaper than calls to named

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14  6:04   ` DJ Delorie
@ 2009-10-14 16:26     ` Richard Henderson
  2009-10-14 16:55       ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-14 16:26 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/13/2009 06:13 PM, DJ Delorie wrote:
> How's this one?  I fixed a few other bugs that a full build uncovered,
> too.  I chose to just not always tag that to-be-split insn, because
> it's difficult to determine in advance what the splitter will do.

It's fine by me.  However...

It should be easy to determine what the splitter will do -- call 
split_adds_subs.  Do that yourself by hand and mark all of the 
instructions that it emits and that's all you need.

That said, it would appear that split_adds_subs doesn't optimize one 
case -- the J constraint where you modify the top 8-bits of the 
register.  It would seem to me to be interrupt safe to do

   (set sp (plus:HI sp -260))
	add.b	#-1, r7h
	subs	#2, r7
	subs	#2, r7

it's just the addx that's no good.  Of course, it can't take too many of 
those before it's smaller to just push a scratch register.

   (set sp (plus:HI sp 30))
	mov.w	r3,@-r7
	mov.w	#-28,r3
	add.w	r3,r7
	mov.w	@(28,r7),r3
vs
	subs	#2,r7		x15

Indeed, the crossover point would seem to be 10.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 16:26     ` Richard Henderson
@ 2009-10-14 16:55       ` DJ Delorie
  2009-10-14 17:57         ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-14 16:55 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


Can we do this, as long as the first add is non-zero?

	add.b	#-1, r7h
	add.b	#-48, r7l
	addx.b	#0, r7h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 16:55       ` DJ Delorie
@ 2009-10-14 17:57         ` Richard Henderson
  2009-10-14 19:26           ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-14 17:57 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/14/2009 09:53 AM, DJ Delorie wrote:
> Can we do this, as long as the first add is non-zero?
>
> 	add.b	#-1, r7h
> 	add.b	#-48, r7l
> 	addx.b	#0, r7h

I have no idea.  Frankly, I don't know why an add/addx sequence against 
the stack pointer isn't interrupt safe.  It seems to me that an 
interrupt had better preserve both the sp and flags registers or the 
program is going to have all sorts of problems everywhere.  And if they 
are preserved, I can't think of any reason why addx wouldn't work.

I think this is something you'll have to ask the customer.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 17:57         ` Richard Henderson
@ 2009-10-14 19:26           ` DJ Delorie
  2009-10-14 20:56             ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-14 19:26 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


> I have no idea.  Frankly, I don't know why an add/addx sequence against 
> the stack pointer isn't interrupt safe.

Because the first add leaves you with a $sp that may point inside a live stack.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 19:26           ` DJ Delorie
@ 2009-10-14 20:56             ` Richard Henderson
  2009-10-14 22:13               ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-14 20:56 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/14/2009 11:01 AM, DJ Delorie wrote:
>> I have no idea.  Frankly, I don't know why an add/addx sequence against
>> the stack pointer isn't interrupt safe.
>
> Because the first add leaves you with a $sp that may point inside a live stack.

Oh, yes, I see.  Which suggests that we only need this splitting when 
the adjustment is supposed to be negative.  For positive values (i.e. 
deallocating stack) don't have the problem of the live stack.

Perhaps something like

;; For H8/300, numbers that cannot be satisfied by
(define_prediate "split_stack_operand"
   (and (match_code "const_int")
        (match_test "ival < -2 && (ival & 0xff) != 0")))

void
split_stack_subtract (const_rtx x)
{
   HOST_WIDE_INT val = INTVAL (x);
   HOST_WIDE_INT hval, lval;

   gcc_assert (val < -2);

   /* Notice values -4 and -3 and arrange to use ADDS.
      Otherwise round up to a multiple of -256 so that
      we subtract more than we need, then add back the
      difference.  */
   if (val >= -4)
     hval = -2;
   else
     hval = val & ~(HOST_WIDE_INT)0xff;
   lval = val - hval;

   emit_insn (gen_addhi3 (stack_pointer_rtx,
			 stack_pointer_rtx, GEN_INT (hval)));
   emit_insn (gen_addhi3 (stack_pointer_rtx,
			 stack_pointer_rtx, GEN_INT (lval)));
}

(define_split
   [(set (match_operand:HI 0 "stack_pointer_operand" "")
	(plus:HI (match_dup 0)
		 (match_operand 1 "split_stack_operand" "")))]
   "TARGET_H8300"
   "split_stack_subtract (operands[1]); DONE;")

h8300_emit_stack_adjustment
...
   rtx offset = GEN_INT (sign * size);
   rtx last = get_last_insn ();

   if (TARGET_H8300 && satisfies_constraint_split_stack_operand (offset))
     split_stack_subtract (offset);
   else
     emit_insn (gen_add2_insn (stack_pointer_rtx, offset));
   for (last = NEXT_INSN (last); last; last = NEXT_INSN (last))
     F (last);



r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 20:56             ` Richard Henderson
@ 2009-10-14 22:13               ` DJ Delorie
  2009-10-14 22:14                 ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-14 22:13 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


> Oh, yes, I see.  Which suggests that we only need this splitting when 
> the adjustment is supposed to be negative.  For positive values (i.e. 
> deallocating stack) don't have the problem of the live stack.

Since ADD.B only affects the lower byte, it doesn't matter what the
overall adjustment is - there's *always* a possibility of the
intermediate value being either higher or lower, depending on whether
or not there's a carry.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 22:13               ` DJ Delorie
@ 2009-10-14 22:14                 ` Richard Henderson
  2009-10-14 22:23                   ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-14 22:14 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/14/2009 02:49 PM, DJ Delorie wrote:
>> Oh, yes, I see.  Which suggests that we only need this splitting when
>> the adjustment is supposed to be negative.  For positive values (i.e.
>> deallocating stack) don't have the problem of the live stack.
>
> Since ADD.B only affects the lower byte, it doesn't matter what the
> overall adjustment is - there's *always* a possibility of the
> intermediate value being either higher or lower, depending on whether
> or not there's a carry.

Correct.  But in the case of deallocating stack, i.e. adding a positive 
value, we don't mind if the intermediate value happens to have a lower 
value, i.e. temporarily allocates more stack.  We only care when 
allocating stack, i.e. adding a negative value, that the intermediate 
value must not have a higher value, i.e. temporarily deallocates stack.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 22:14                 ` Richard Henderson
@ 2009-10-14 22:23                   ` DJ Delorie
  2009-10-15  1:03                     ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-14 22:23 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


Ah, right.

I was thinking about the add/add/addx option - the only drawback is
that you may create an $sp that points beyond the end of the available
stack space, if the stack space is small.  It might end up pointing
into, for example, user data.  The current code never leaves you with
$sp as anything other than the two values you know are good, or
something known to be between them.  I suspect we'll have to stick to
that.

All this for some debugging support.  At this point, I think I'd
rather FIXME that line and go with everything else; that gives full
debugging for H and S families, without messing with the H8/300 logic.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-14 22:23                   ` DJ Delorie
@ 2009-10-15  1:03                     ` Richard Henderson
  2009-10-15  1:16                       ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-15  1:03 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/14/2009 03:21 PM, DJ Delorie wrote:
> I was thinking about the add/add/addx option - the only drawback is
> that you may create an $sp that points beyond the end of the available
> stack space, if the stack space is small.

True.  I'd sort of ignored that, assuming the maximum of 254 extra stack 
used wasn't significant; if you're that low on stack is an interrupt 
likely to work anyway?  I just hate the thought of using 10 adds to 
allocate 20 bytes of stack.

You should definitely attack this as a separate patch from the rest.

You might ask the customer whether the possible 254 bytes of extra stack 
is significant.  If so, implement the push/mov/add/restore sequence; if 
not, implement the add/add/addx sequence.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-15  1:03                     ` Richard Henderson
@ 2009-10-15  1:16                       ` DJ Delorie
  2009-10-15  2:31                         ` Richard Henderson
  0 siblings, 1 reply; 15+ messages in thread
From: DJ Delorie @ 2009-10-15  1:16 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


Ok, then, here's the patch as-is without that case being handled.  I'm
open to alternate wordings on that comment, otherwise, OK to commit?


Index: h8300.c
===================================================================
--- h8300.c	(revision 152793)
+++ h8300.c	(working copy)
@@ -504,12 +504,38 @@ byte_reg (rtx x, int b)
        /* Save call clobbered registers in non-leaf interrupt		\
 	  handlers.  */							\
        || (h8300_current_function_interrupt_function_p ()		\
 	   && call_used_regs[regno]					\
 	   && !current_function_is_leaf)))
 
+/* We use this to wrap all emitted insns in the prologue.  */
+static rtx
+F (rtx x)
+{
+  RTX_FRAME_RELATED_P (x) = 1;
+  return x;
+}
+
+/* Mark all the subexpressions of the PARALLEL rtx PAR as
+   frame-related.  Return PAR.
+
+   dwarf2out.c:dwarf2out_frame_debug_expr ignores sub-expressions of a
+   PARALLEL rtx other than the first if they do not have the
+   FRAME_RELATED flag set on them.  */
+static rtx
+Fpa (rtx par)
+{
+  int len = XVECLEN (par, 0);
+  int i;
+
+  for (i = 0; i < len; i++)
+    F (XVECEXP (par, 0, i));
+
+  return par;
+}
+
 /* Output assembly language to FILE for the operation OP with operand size
    SIZE to adjust the stack pointer.  */
 
 static void
 h8300_emit_stack_adjustment (int sign, HOST_WIDE_INT size)
 {
@@ -523,28 +549,33 @@ h8300_emit_stack_adjustment (int sign, H
   if (TARGET_H8300
       && size > 4
       && !h8300_current_function_interrupt_function_p ()
       && !(cfun->static_chain_decl != NULL && sign < 0))
     {
       rtx r3 = gen_rtx_REG (Pmode, 3);
-      emit_insn (gen_movhi (r3, GEN_INT (sign * size)));
-      emit_insn (gen_addhi3 (stack_pointer_rtx,
-			     stack_pointer_rtx, r3));
+      F (emit_insn (gen_movhi (r3, GEN_INT (sign * size))));
+      F (emit_insn (gen_addhi3 (stack_pointer_rtx,
+				stack_pointer_rtx, r3)));
     }
   else
     {
       /* The stack adjustment made here is further optimized by the
 	 splitter.  In case of H8/300, the splitter always splits the
-	 addition emitted here to make the adjustment
-	 interrupt-safe.  */
+	 addition emitted here to make the adjustment interrupt-safe.
+	 FIXME: We don't always tag those, because we don't know what
+	 the splitter will do.  */
       if (Pmode == HImode)
-	emit_insn (gen_addhi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	{
+	  rtx x = emit_insn (gen_addhi3 (stack_pointer_rtx,
+					 stack_pointer_rtx, GEN_INT (sign * size)));
+	  if (size < 4)
+	    F (x);
+	}
       else
-	emit_insn (gen_addsi3 (stack_pointer_rtx,
-			       stack_pointer_rtx, GEN_INT (sign * size)));
+	F (emit_insn (gen_addsi3 (stack_pointer_rtx,
+				  stack_pointer_rtx, GEN_INT (sign * size))));
     }
 }
 
 /* Round up frame size SIZE.  */
 
 static HOST_WIDE_INT
@@ -588,13 +619,13 @@ push (int rn)
   if (TARGET_H8300)
     x = gen_push_h8300 (reg);
   else if (!TARGET_NORMAL_MODE)
     x = gen_push_h8300hs_advanced (reg);
   else
     x = gen_push_h8300hs_normal (reg);
-  x = emit_insn (x);
+  x = F (emit_insn (x));
   REG_NOTES (x) = gen_rtx_EXPR_LIST (REG_INC, stack_pointer_rtx, 0);
 }
 
 /* Emit an insn to pop register RN.  */
 
 static void
@@ -631,13 +662,13 @@ pop (int rn)
 
 static void
 h8300_push_pop (int regno, int nregs, int pop_p, int return_p)
 {
   int i, j;
   rtvec vec;
-  rtx sp, offset;
+  rtx sp, offset, x;
 
   /* See whether we can use a simple push or pop.  */
   if (!return_p && nregs == 1)
     {
       if (pop_p)
 	pop (regno);
@@ -682,13 +713,16 @@ h8300_push_pop (int regno, int nregs, in
 
   /* Add the stack adjustment.  */
   offset = GEN_INT ((pop_p ? nregs : -nregs) * 4);
   RTVEC_ELT (vec, i + j) = gen_rtx_SET (VOIDmode, sp,
 					gen_rtx_PLUS (Pmode, sp, offset));
 
-  emit_insn (gen_rtx_PARALLEL (VOIDmode, vec));
+  x = gen_rtx_PARALLEL (VOIDmode, vec);
+  if (!pop_p)
+    x = Fpa (x);
+  emit_insn (x);
 }
 
 /* Return true if X has the value sp + OFFSET.  */
 
 static int
 h8300_stack_offset_p (rtx x, int offset)
@@ -817,13 +851,13 @@ h8300_expand_prologue (void)
     emit_insn (gen_monitor_prologue ());
 
   if (frame_pointer_needed)
     {
       /* Push fp.  */
       push (HARD_FRAME_POINTER_REGNUM);
-      emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
+      F (emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx));
     }
 
   /* Push the rest of the registers in ascending order.  */
   saved_regs = compute_saved_regs ();
   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno += n_regs)
     {
Index: h8300.h
===================================================================
--- h8300.h	(revision 152793)
+++ h8300.h	(working copy)
@@ -144,12 +144,23 @@ extern const char * const *h8_reg_names;
 #define TARGET_DEFAULT (MASK_QUICKCALL)
 #endif
 
 /* Show we can debug even without a frame pointer.  */
 /* #define CAN_DEBUG_WITHOUT_FP */
 
+/* We want dwarf2 info available to gdb...  */
+#define DWARF2_DEBUGGING_INFO        1
+/* ... but we don't actually support full dwarf2 EH.  */
+#define MUST_USE_SJLJ_EXCEPTIONS 1
+
+/* The return address is pushed on the stack.  */
+#define INCOMING_RETURN_ADDR_RTX   gen_rtx_MEM (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM))
+#define INCOMING_FRAME_SP_OFFSET   (POINTER_SIZE / 8)
+
+#define DWARF_CIE_DATA_ALIGNMENT	2
+
 /* Define this if addresses of constant functions
    shouldn't be put through pseudo regs where they can be cse'd.
    Desirable on machines where ordinary constants are expensive
    but a CALL with constant address is cheap.
 
    Calls through a register are cheaper than calls to named

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-15  1:16                       ` DJ Delorie
@ 2009-10-15  2:31                         ` Richard Henderson
  2009-10-15  2:56                           ` DJ Delorie
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Henderson @ 2009-10-15  2:31 UTC (permalink / raw)
  To: DJ Delorie; +Cc: gcc-patches

On 10/14/2009 06:03 PM, DJ Delorie wrote:
> Ok, then, here's the patch as-is without that case being handled.  I'm
> open to alternate wordings on that comment, otherwise, OK to commit?
>

Ok.


r~

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [h8300] Add dwarf2 debugging support
  2009-10-15  2:31                         ` Richard Henderson
@ 2009-10-15  2:56                           ` DJ Delorie
  0 siblings, 0 replies; 15+ messages in thread
From: DJ Delorie @ 2009-10-15  2:56 UTC (permalink / raw)
  To: Richard Henderson; +Cc: gcc-patches


Thanks, committed.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2009-10-15  2:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-13 21:57 [h8300] Add dwarf2 debugging support DJ Delorie
2009-10-13 22:48 ` Richard Henderson
2009-10-14  6:04   ` DJ Delorie
2009-10-14 16:26     ` Richard Henderson
2009-10-14 16:55       ` DJ Delorie
2009-10-14 17:57         ` Richard Henderson
2009-10-14 19:26           ` DJ Delorie
2009-10-14 20:56             ` Richard Henderson
2009-10-14 22:13               ` DJ Delorie
2009-10-14 22:14                 ` Richard Henderson
2009-10-14 22:23                   ` DJ Delorie
2009-10-15  1:03                     ` Richard Henderson
2009-10-15  1:16                       ` DJ Delorie
2009-10-15  2:31                         ` Richard Henderson
2009-10-15  2:56                           ` DJ Delorie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).