Jakub Jelinek wrote:
> On Tue, Feb 21, 2012 at 12:38:48PM +0100, Georg-Johann Lay wrote:
>> However, if all insns that are involved in SP/FP manipulation get the
>> RTX_FRAME_RELATED_P, almost every test case that has -g crashes with this
>> dwarf-ICE.
>
> My suggestion actually wasn't to use unspec for reading from sp, but instead
> to use it on the insn which sets sp from hfp, i.e. instead of that
> (insn 47 46 48 2 (set (reg/f:HI 32 __SP_L__)
> (reg/f:HI 28 r28)) pr50063.c:9 -1
> (nil))
> in the testcase use:
> (insn 47 46 48 2 (set (reg/f:HI 32 __SP_L__)
> (unspec [(reg/f:HI 28 r28)] UNSPEC_SET_SP_FROM_FP)) pr50063.c:9 -1
> (nil))
> or so. You should be able to use REG_FRAME_RELATED_EXPR with the old set
> if needed (though, the insn strangely isn't RTX_FRAME_RELATED_P, preexisting
> bug?). Just to hide from cselib/alias code that you are setting hfp from
> sp, then adjusting hfp and then setting sp back from it. With the unspec
> it will just not know what value sp has there.
>
> Jakub
Thanks, it works now :-)) Here is the patch for review now.
It passes the testsuite and fixed several execution fails because of
assumptions in DSE. Besides the normal testsuite run, it passes also a run
with -maccumulate-args.
Ok for trunk?
Johann
PR rtl-optimization/50063
* config/avr/avr.md (movhi_sp_r): Handle -1 (unknown IRQ state)
and 2 (8-bit SP) in operand 2.
* config/avr/avr.c (avr_prologue_setup_frame): Adjust prologue
setup to use movhi_sp_r instead of vanilla move to write SP.
Adjust REG_CFA notes to superseed unspec.
(expand_epilogue): Adjust epilogue setup to use read_sp instead
of vanilla move.
As function body might contain CLI or SEI: Use irq_state 0 (IRQ
known to be off) only with TARGET_NO_INTERRUPTS. Never use
irq_state 1 (IRQ known to be on) here.
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md (revision 184386)
+++ config/avr/avr.md (working copy)
@@ -583,23 +583,26 @@ (define_peephole2
;; Move register $1 to the Stack Pointer register SP.
;; This insn is emit during function prologue/epilogue generation.
-;; $2 = 0: We know that IRQs are off
-;; $2 = 1: We know that IRQs are on
-;; Remaining cases when the state of the I-Flag is unknown are
-;; handled by generic movhi insn.
+;; $2 = 0: We know that IRQs are off
+;; $2 = 1: We know that IRQs are on
+;; $2 = 2: SP has 8 bits only, IRQ state does not matter
+;; $2 = -1: We don't know anything about IRQ on/off
+;; Always write SP via unspec, see PR50063
(define_insn "movhi_sp_r"
- [(set (match_operand:HI 0 "stack_register_operand" "=q,q,q")
- (unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r,r,r")
- (match_operand:HI 2 "const_int_operand" "L,P,LP")]
+ [(set (match_operand:HI 0 "stack_register_operand" "=q,q,q,q,q")
+ (unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r,r,r,r,r")
+ (match_operand:HI 2 "const_int_operand" "L,P,N,K,LPN")]
UNSPECV_WRITE_SP))]
- "!AVR_HAVE_8BIT_SP"
+ ""
"@
- out __SP_H__,%B1\;out __SP_L__,%A1
- cli\;out __SP_H__,%B1\;sei\;out __SP_L__,%A1
- out __SP_L__,%A1\;out __SP_H__,%B1"
- [(set_attr "length" "2,4,2")
- (set_attr "isa" "no_xmega,no_xmega,xmega")
+ out %B0,%B1\;out %A0,%A1
+ cli\;out %B0,%B1\;sei\;out %A0,%A1
+ in __tmp_reg__,__SREG__\;cli\;out %B0,%B1\;out __SREG__,__tmp_reg__\;out %A0,%A1
+ out %A0,%A1
+ out %A0,%A1\;out %B0,%B1"
+ [(set_attr "length" "2,4,5,1,2")
+ (set_attr "isa" "no_xmega,no_xmega,no_xmega,*,xmega")
(set_attr "cc" "none")])
(define_peephole2
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c (revision 184386)
+++ config/avr/avr.c (working copy)
@@ -1062,8 +1062,8 @@ avr_prologue_setup_frame (HOST_WIDE_INT
!frame_pointer_needed can only occur if the function is not a
leaf function and thus X has already been saved. */
+ int irq_state = -1;
rtx fp_plus_insns, fp, my_fp;
- rtx sp_minus_size = plus_constant (stack_pointer_rtx, -size);
gcc_assert (frame_pointer_needed
|| !isr_p
@@ -1076,7 +1076,7 @@ avr_prologue_setup_frame (HOST_WIDE_INT
if (AVR_HAVE_8BIT_SP)
{
/* The high byte (r29) does not change:
- Prefer SUBI (1 cycle) over ABIW (2 cycles, same size). */
+ Prefer SUBI (1 cycle) over SBIW (2 cycles, same size). */
my_fp = all_regs_rtx[FRAME_POINTER_REGNUM];
}
@@ -1092,43 +1092,50 @@ avr_prologue_setup_frame (HOST_WIDE_INT
the frame pointer subtraction is done. */
insn = emit_move_insn (fp, stack_pointer_rtx);
- if (!frame_pointer_needed)
- RTX_FRAME_RELATED_P (insn) = 1;
+ if (frame_pointer_needed)
+ {
+ RTX_FRAME_RELATED_P (insn) = 1;
+ add_reg_note (insn, REG_CFA_ADJUST_CFA,
+ gen_rtx_SET (VOIDmode, fp, stack_pointer_rtx));
+ }
insn = emit_move_insn (my_fp, plus_constant (my_fp, -size));
- RTX_FRAME_RELATED_P (insn) = 1;
-
if (frame_pointer_needed)
{
+ RTX_FRAME_RELATED_P (insn) = 1;
add_reg_note (insn, REG_CFA_ADJUST_CFA,
- gen_rtx_SET (VOIDmode, fp, sp_minus_size));
+ gen_rtx_SET (VOIDmode, fp,
+ plus_constant (fp, -size)));
}
/* Copy to stack pointer. Note that since we've already
changed the CFA to the frame pointer this operation
- need not be annotated if frame pointer is needed. */
-
- if (AVR_HAVE_8BIT_SP || AVR_XMEGA)
- {
- insn = emit_move_insn (stack_pointer_rtx, fp);
- }
- else if (TARGET_NO_INTERRUPTS
- || isr_p
- || cfun->machine->is_OS_main)
- {
- rtx irqs_are_on = GEN_INT (!!cfun->machine->is_interrupt);
-
- insn = emit_insn (gen_movhi_sp_r (stack_pointer_rtx,
- fp, irqs_are_on));
- }
- else
- {
- insn = emit_move_insn (stack_pointer_rtx, fp);
- }
+ need not be annotated if frame pointer is needed.
+ Always move through unspec, see PR50063.
+ For meaning of irq_state see movhi_sp_r insn. */
+
+ if (cfun->machine->is_interrupt)
+ irq_state = 1;
+
+ if (TARGET_NO_INTERRUPTS
+ || cfun->machine->is_signal
+ || cfun->machine->is_OS_main)
+ irq_state = 0;
- if (!frame_pointer_needed)
- RTX_FRAME_RELATED_P (insn) = 1;
+ if (AVR_HAVE_8BIT_SP)
+ irq_state = 2;
+ insn = emit_insn (gen_movhi_sp_r (stack_pointer_rtx,
+ fp, GEN_INT (irq_state)));
+ if (!frame_pointer_needed)
+ {
+ RTX_FRAME_RELATED_P (insn) = 1;
+ add_reg_note (insn, REG_CFA_ADJUST_CFA,
+ gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ -size)));
+ }
+
fp_plus_insns = get_insns ();
end_sequence ();
@@ -1143,9 +1150,13 @@ avr_prologue_setup_frame (HOST_WIDE_INT
start_sequence ();
- insn = emit_move_insn (stack_pointer_rtx, sp_minus_size);
+ insn = emit_move_insn (stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx, -size));
RTX_FRAME_RELATED_P (insn) = 1;
-
+ add_reg_note (insn, REG_CFA_ADJUST_CFA,
+ gen_rtx_SET (VOIDmode, stack_pointer_rtx,
+ plus_constant (stack_pointer_rtx,
+ -size)));
if (frame_pointer_needed)
{
insn = emit_move_insn (fp, stack_pointer_rtx);
@@ -1376,7 +1387,8 @@ expand_epilogue (bool sibcall_p)
if (size)
{
/* Try two methods to adjust stack and select shortest. */
-
+
+ int irq_state = -1;
rtx fp, my_fp;
rtx fp_plus_insns;
@@ -1406,23 +1418,15 @@ expand_epilogue (bool sibcall_p)
emit_move_insn (my_fp, plus_constant (my_fp, size));
/* Copy to stack pointer. */
-
- if (AVR_HAVE_8BIT_SP || AVR_XMEGA)
- {
- emit_move_insn (stack_pointer_rtx, fp);
- }
- else if (TARGET_NO_INTERRUPTS
- || isr_p
- || cfun->machine->is_OS_main)
- {
- rtx irqs_are_on = GEN_INT (!!cfun->machine->is_interrupt);
-
- emit_insn (gen_movhi_sp_r (stack_pointer_rtx, fp, irqs_are_on));
- }
- else
- {
- emit_move_insn (stack_pointer_rtx, fp);
- }
+
+ if (TARGET_NO_INTERRUPTS)
+ irq_state = 0;
+
+ if (AVR_HAVE_8BIT_SP)
+ irq_state = 2;
+
+ emit_insn (gen_movhi_sp_r (stack_pointer_rtx, fp,
+ GEN_INT (irq_state)));
fp_plus_insns = get_insns ();
end_sequence ();
