From patchwork Sat Dec 18 17:27:14 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John David Anglin X-Patchwork-Id: 76086 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 0B837B70AF for ; Sun, 19 Dec 2010 04:27:27 +1100 (EST) Received: (qmail 8296 invoked by alias); 18 Dec 2010 17:27:25 -0000 Received: (qmail 8287 invoked by uid 22791); 18 Dec 2010 17:27:23 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, TW_BV, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from hiauly1.hia.nrc.ca (HELO hiauly1.hia.nrc.ca) (132.246.10.84) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 18 Dec 2010 17:27:17 +0000 Received: by hiauly1.hia.nrc.ca (Postfix, from userid 1000) id 63FF34E6A; Sat, 18 Dec 2010 12:27:15 -0500 (EST) Subject: [committed] Fix wrong code generated for conditional branch followed by zero length asm on PA To: gcc-patches@gcc.gnu.org Date: Sat, 18 Dec 2010 12:27:14 -0500 (EST) From: "John David Anglin" MIME-Version: 1.0 Message-Id: <20101218172715.63FF34E6A@hiauly1.hia.nrc.ca> 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 The attached patch fixes a wrong code problem on the PA target. the problem was seen in the Linux kernel. When a conditional branch is followed by a zero length asm, we would sometimes not add a nop in the delay slot. As a result, the branch could jump to the instruction in the delay slot. This is not handled correctly by the hardware and must be prohibited. The patch below improves the branch_to_delay_slot_p and branch_needs_nop_p functions, adding checks for ASM_OPERANDS asms. The search is also iterated to handle other zero length insns if they occur. A new function use_skip_p is added. It is used to determine when nullification may be used to skip the following instruction. It also needs to check for asms because the length of asms is just an estimate. Tested on hppa2.0w-hp-hpux11.11, hppa64-hp-hpux11.11 and hppa-unknown-linux-gnu with no observed regressions. Committed to trunk. Dave Index: config/pa/pa.c =================================================================== --- config/pa/pa.c (revision 167794) +++ config/pa/pa.c (working copy) @@ -6188,37 +6188,94 @@ } /* Return TRUE if INSN, a jump insn, has an unfilled delay slot and - it branches to the next real instruction. Otherwise, return FALSE. */ + it branches into the delay slot. Otherwise, return FALSE. */ static bool branch_to_delay_slot_p (rtx insn) { + rtx jump_insn; + if (dbr_sequence_length ()) return FALSE; - return next_real_insn (JUMP_LABEL (insn)) == next_real_insn (insn); + jump_insn = next_active_insn (JUMP_LABEL (insn)); + while (insn) + { + insn = next_active_insn (insn); + if (jump_insn == insn) + return TRUE; + + /* We can't rely on the length of asms. So, we return FALSE when + the branch is followed by an asm. */ + if (!insn + || GET_CODE (PATTERN (insn)) == ASM_INPUT + || extract_asm_operands (PATTERN (insn)) != NULL_RTX + || get_attr_length (insn) > 0) + break; + } + + return FALSE; } -/* Return TRUE if INSN, a jump insn, needs a nop in its delay slot. +/* Return TRUE if INSN, a forward jump insn, needs a nop in its delay slot. This occurs when INSN has an unfilled delay slot and is followed - by an ASM_INPUT. Disaster can occur if the ASM_INPUT is empty and - the jump branches into the delay slot. So, we add a nop in the delay - slot just to be safe. This messes up our instruction count, but we - don't know how big the ASM_INPUT insn is anyway. */ + by an asm. Disaster can occur if the asm is empty and the jump + branches into the delay slot. So, we add a nop in the delay slot + when this occurs. */ static bool branch_needs_nop_p (rtx insn) { - rtx next_insn; + rtx jump_insn; if (dbr_sequence_length ()) return FALSE; - next_insn = next_real_insn (insn); - return GET_CODE (PATTERN (next_insn)) == ASM_INPUT; + jump_insn = next_active_insn (JUMP_LABEL (insn)); + while (insn) + { + insn = next_active_insn (insn); + if (!insn || jump_insn == insn) + return TRUE; + + if (!(GET_CODE (PATTERN (insn)) == ASM_INPUT + || extract_asm_operands (PATTERN (insn)) != NULL_RTX) + && get_attr_length (insn) > 0) + break; + } + + return FALSE; } +/* Return TRUE if INSN, a forward jump insn, can use nullification + to skip the following instruction. This avoids an extra cycle due + to a mis-predicted branch when we fall through. */ + +static bool +use_skip_p (rtx insn) +{ + rtx jump_insn = next_active_insn (JUMP_LABEL (insn)); + + while (insn) + { + insn = next_active_insn (insn); + + /* We can't rely on the length of asms, so we can't skip asms. */ + if (!insn + || GET_CODE (PATTERN (insn)) == ASM_INPUT + || extract_asm_operands (PATTERN (insn)) != NULL_RTX) + break; + if (get_attr_length (insn) == 4 + && jump_insn == next_active_insn (insn)) + return TRUE; + if (get_attr_length (insn) > 0) + break; + } + + return FALSE; +} + /* This routine handles all the normal conditional branch sequences we might need to generate. It handles compare immediate vs compare register, nullification of delay slots, varying length branches, @@ -6230,7 +6287,7 @@ output_cbranch (rtx *operands, int negated, rtx insn) { static char buf[100]; - int useskip = 0; + bool useskip; int nullify = INSN_ANNULLED_BRANCH_P (insn); int length = get_attr_length (insn); int xdelay; @@ -6268,12 +6325,7 @@ /* A forward branch over a single nullified insn can be done with a comclr instruction. This avoids a single cycle penalty due to mis-predicted branch if we fall through (branch not taken). */ - if (length == 4 - && next_real_insn (insn) != 0 - && get_attr_length (next_real_insn (insn)) == 4 - && JUMP_LABEL (insn) == next_nonnote_insn (next_real_insn (insn)) - && nullify) - useskip = 1; + useskip = (length == 4 && nullify) ? use_skip_p (insn) : FALSE; switch (length) { @@ -6561,7 +6613,7 @@ output_bb (rtx *operands ATTRIBUTE_UNUSED, int negated, rtx insn, int which) { static char buf[100]; - int useskip = 0; + bool useskip; int nullify = INSN_ANNULLED_BRANCH_P (insn); int length = get_attr_length (insn); int xdelay; @@ -6587,14 +6639,8 @@ /* A forward branch over a single nullified insn can be done with a extrs instruction. This avoids a single cycle penalty due to mis-predicted branch if we fall through (branch not taken). */ + useskip = (length == 4 && nullify) ? use_skip_p (insn) : FALSE; - if (length == 4 - && next_real_insn (insn) != 0 - && get_attr_length (next_real_insn (insn)) == 4 - && JUMP_LABEL (insn) == next_nonnote_insn (next_real_insn (insn)) - && nullify) - useskip = 1; - switch (length) { @@ -6752,7 +6798,7 @@ output_bvb (rtx *operands ATTRIBUTE_UNUSED, int negated, rtx insn, int which) { static char buf[100]; - int useskip = 0; + bool useskip; int nullify = INSN_ANNULLED_BRANCH_P (insn); int length = get_attr_length (insn); int xdelay; @@ -6778,14 +6824,8 @@ /* A forward branch over a single nullified insn can be done with a extrs instruction. This avoids a single cycle penalty due to mis-predicted branch if we fall through (branch not taken). */ + useskip = (length == 4 && nullify) ? use_skip_p (insn) : FALSE; - if (length == 4 - && next_real_insn (insn) != 0 - && get_attr_length (next_real_insn (insn)) == 4 - && JUMP_LABEL (insn) == next_nonnote_insn (next_real_insn (insn)) - && nullify) - useskip = 1; - switch (length) {