From patchwork Tue Feb 21 11:38:48 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Georg-Johann Lay X-Patchwork-Id: 142289 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2A50EB6FA8 for ; Tue, 21 Feb 2012 22:39:51 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1330429192; h=Comment: DomainKey-Signature:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=ecmdnk3 XYyiHcqGhqI5XsNiyyFE=; b=HcBa1KQVkOwJ1qKyNwiyqsMlQZ+igKTtID/7pQq /6Y+FjbNh9rRIy/JMu8Z366B7C0TB0yQXUtyNG2XLpNtAWhlBI8GCTKJ6pl1xENp 9zBNm8R/EYaFiEJia53YMnd3O9rCxo+AhAvHjPZGCAJviADMhL4mV1iyTwRMTWJb G05I= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:X-RZG-AUTH:X-RZG-CLASS-ID:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=sE6SpYFbmNE73SPzWXmIZJ3AXggIzVBKuOPSWkDw02UCKLzzslAYZDUIPfRoSx Xwadv+7OXJ/nwo1yusSVHe7G7AW4VXi7Y//Jm41FgKJr0E9gB3Ez539eAK4RG28I mLpJc+jAR+loxfp+1plnEeC/cnwPDq7GBWaBL7lAnAwJ8=; Received: (qmail 20912 invoked by alias); 21 Feb 2012 11:39:47 -0000 Received: (qmail 20898 invoked by uid 22791); 21 Feb 2012 11:39:43 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE, TW_DW, TW_OV, TW_VH X-Spam-Check-By: sourceware.org Received: from mo-p00-ob.rzone.de (HELO mo-p00-ob.rzone.de) (81.169.146.161) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 21 Feb 2012 11:39:23 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT2k715jHQaJercGObUOFkj18odoYNahU4Q== X-RZG-CLASS-ID: mo00 Received: from [192.168.0.22] (business-188-111-022-002.static.arcor-ip.net [188.111.22.2]) by smtp.strato.de (fruni mo24) (RZmta 27.7 AUTH) with ESMTPA id v01f2do1LB9Hok ; Tue, 21 Feb 2012 12:38:49 +0100 (MET) Message-ID: <4F438248.70503@gjlay.de> Date: Tue, 21 Feb 2012 12:38:48 +0100 From: Georg-Johann Lay User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org CC: Richard Henderson , Denis Chertykov , Eric Weddington Subject: [Patch,AVR]: Fix PR50063 GCC does not support FP = SP X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org This is a tentative patch to the PR. As Jakub explained, GCC does not support configurations with FP = SP and DSE optimizer generates wrong code for AVR. This patch implements Jakub's proposal to hack around by hiding the action of setting/moving between SP and SP into UNSPEC[_VOLATILE]s. All works fine with respect to code generation. But what I cannot manage getting debug information right. Without RTX_FRAME_RELATED_P at most insns, I just get one new dwarf-ICE: dwarf2out_frame_debug_adjust_cfa[test2:dwarf2(233)]: pat = (set (reg/f:HI 28 r28) (plus:HI (reg/f:HI 32 __SP_L__) (const_int -40 [0xffffffd8]))) dwf_regno (XEXP (src, 0)) = 32 cur_cfa->reg = 28 ./gcc/testsuite/gcc.c-torture/execute/builtins/snprintf-chk.c: In function 'test2': ./gcc/testsuite/gcc.c-torture/execute/builtins/snprintf-chk.c:159:1: internal compiler error: in dwarf2out_frame_debug_adjust_cfa, at dwarf2cfi.c:1098 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. I am stuck now and don't know how to proceed with this. Many thanks, Richard, for your help with the CFA notes. Unfortunately, adding the RTX_FRAME_RELATED_P markers shreds every... Attached the patch as-is and a plain work-copy of avr.c:avr_prologue_setup_frame Johann PR other/50063 * config/avr/avr.md (UNSPEC_READ_SP): New define_c_enum "unspec". (read_sp): New insn. (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 read_sp instead of vanilla move. 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) @@ -69,6 +69,7 @@ (define_c_enum "unspec" UNSPEC_COPYSIGN UNSPEC_IDENTITY UNSPEC_INSERT_BITS + UNSPEC_READ_SP ]) (define_c_enum "unspecv" @@ -581,25 +582,42 @@ (define_peephole2 ;;============================================================================ ;; move word (16 bit) +;; Always read SP via unspec, see PR50063 + +(define_insn "read_sp" + [(set (match_operand:HI 0 "register_operand" "=r") + (unspec:HI [(match_operand:HI 1 "stack_register_operand" "q")] + UNSPEC_READ_SP))] + "" + { + return AVR_HAVE_8BIT_SP + ? "in %A0,%A1\;clr %B0" + : "in %A0,%A1\;in %B0,%B1"; + } + [(set_attr "length" "2") + (set_attr "cc" "clobber")]) + ;; 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 (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) @@ -1035,8 +1035,11 @@ avr_prologue_setup_frame (HOST_WIDE_INT if (frame_pointer_needed && size == 0) { - insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); + insn = emit_insn (gen_read_sp (frame_pointer_rtx, stack_pointer_rtx)); RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_ADJUST_CFA, + gen_rtx_SET (VOIDmode, frame_pointer_rtx, + stack_pointer_rtx)); } if (size != 0) @@ -1062,8 +1065,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 +1079,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 ADIW (2 cycles, same size). */ my_fp = all_regs_rtx[FRAME_POINTER_REGNUM]; } @@ -1091,44 +1094,50 @@ avr_prologue_setup_frame (HOST_WIDE_INT instead indicate that the entire operation is complete after the frame pointer subtraction is done. */ - insn = emit_move_insn (fp, stack_pointer_rtx); - if (!frame_pointer_needed) - RTX_FRAME_RELATED_P (insn) = 1; + insn = emit_insn (gen_read_sp (fp, stack_pointer_rtx)); + 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 (stack_pointer_rtx, + -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, fp)); + } + fp_plus_insns = get_insns (); end_sequence (); @@ -1143,13 +1152,19 @@ 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); + insn = emit_insn (gen_read_sp (fp, stack_pointer_rtx)); RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_ADJUST_CFA, + gen_rtx_SET (VOIDmode, fp, stack_pointer_rtx)); } sp_plus_insns = get_insns (); @@ -1360,7 +1375,7 @@ expand_epilogue (bool sibcall_p) if (!frame_pointer_needed) { - emit_move_insn (frame_pointer_rtx, stack_pointer_rtx); + emit_insn (gen_read_sp (frame_pointer_rtx, stack_pointer_rtx)); } if (size) @@ -1376,7 +1391,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; @@ -1401,28 +1417,20 @@ expand_epilogue (bool sibcall_p) start_sequence (); if (!frame_pointer_needed) - emit_move_insn (fp, stack_pointer_rtx); + emit_insn (gen_read_sp (fp, stack_pointer_rtx)); 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 ();