From patchwork Fri Mar 4 14:54:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Modra X-Patchwork-Id: 592064 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BEE11140273 for ; Sat, 5 Mar 2016 01:54:54 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=gpmt4ajd; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=pM36Z1vIAO75Y06DeT03HLDLhdLnybh1vjEZCsgF4hWJkPetV1A1S TdnvPQEFTGNP0D8OmDLyx2ikl0i5chiHt2kZY8fJYlF3AWwi0MawAO5Bxr8owcmi 1ZwYh09Xit07+Adi5DX1oxPYKdZl8SUVo7gKnkrHWAjc5wSNoPjvyU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=QZphFx8QIFm4vWYGtOGVOEs3mmg=; b=gpmt4ajdUL1i90qv9p8D 9wI5LbRw7BcjGYzsXxXZRw2ZfJ8WGVGO7pvosVbvXDLGwQkFjDDq/GUgl20/AI++ MYT9PA0AaViTeFr88Z4QvuruU8ivWphYzZl6RNUfUyeOsvPBwPSPYazxGAGmZv87 VcbEcbs8kDWcv4y1NL68uKY= Received: (qmail 39203 invoked by alias); 4 Mar 2016 14:54:41 -0000 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 Received: (qmail 39122 invoked by uid 89); 4 Mar 2016 14:54:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_20, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=Scan, Protect, n7, similarly X-HELO: mail-pa0-f53.google.com Received: from mail-pa0-f53.google.com (HELO mail-pa0-f53.google.com) (209.85.220.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 04 Mar 2016 14:54:37 +0000 Received: by mail-pa0-f53.google.com with SMTP id fl4so36173088pad.0 for ; Fri, 04 Mar 2016 06:54:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition:user-agent; bh=h9Dvz7RIywYvgqtTocUJNwIvWY/JvOlugDuVc/RcbNU=; b=QrvjPIXBTxK6OFkmh06NVz/f0yvv5pGeUpjq7vVA9z5JORfPrZJZAVZdgT4g4gd2gn QM0gHhB7czgVlFPpb57/Pe5S6k4a6FN2Xv16kmZqqcJuJo3iT5ACyUKhMDcQGVOi7eQl nUf1q3JiiaWMImNz2ROMlJIQFTabxjqYU5pLMgoLbbSDYjaYeHJV7QlsusoOU3bInv9z /U9wj54bNdKXOhXS6AeKPFSnx13cX1i88QvtKxyD9sb4xJlAWzqZELpIPXu1+SbmQbZH FESfA2BaVtHCX4DNv5ujpz4MDPNJYSluWAPsiWyNnBnr3HDVzmdnMBPjcyw/9d9FR0m8 fIpA== X-Gm-Message-State: AD7BkJJYQPgc/eGa9bbEIyeLEkia2AJVJ/HhWmiW2oi5Q1wUW/ivNStTsRDZtpiFvvo3Sg== X-Received: by 10.66.193.226 with SMTP id hr2mr12654824pac.20.1457103275595; Fri, 04 Mar 2016 06:54:35 -0800 (PST) Received: from bubble.grove.modra.org (CPE-58-160-146-233.sa.bigpond.net.au. [58.160.146.233]) by smtp.gmail.com with ESMTPSA id f66sm6212330pff.8.2016.03.04.06.54.34 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 04 Mar 2016 06:54:34 -0800 (PST) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 826B8EA0153; Sat, 5 Mar 2016 01:24:30 +1030 (ACDT) Date: Sat, 5 Mar 2016 01:24:30 +1030 From: Alan Modra To: gcc-patches@gcc.gnu.org Subject: [RFC] PR69195, Reload confused by invalid reg equivs Message-ID: <20160304145430.GD9617@bubble.grove.modra.org> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes This is a fix for two testcases that show reload replacing pseudos that don't get hard regs, with their equivalent mem initialization, but failing to address the mem properly. The short story is that ira analysis creates reg equivalence info for use by reload, based on register lifetimes that are invalidated by ira itself deleting dead insns. My first attempt to fix this problem was to just run delete_trivially_dead_insns early in ira. That's enough to cure the testcase failures. However, ira also deletes unreachable blocks (that are only recognized as such after ira reg equivalence analysis), and I figure it is possible that deleting those insns may similarly affect register lifetimes. So what this patch does is revalidate the reg equivalences after insns have been deleted, recreating them as if insns had been deleted before any analysis occurred. The patch has been bootstrapped and regression tested on powerpc64le-linux. Do we go with this approach, or just run simple delete_trivially_dead_insns early? Or something else? gcc/ PR rtl-optimization/69195 * ira.c (pdx_subregs): New static, extracted from update_equiv_regs. (set_paradoxical_subreg): Delete pdx_subregs param. (add_store_equivs): New function, extracted from.. (update_equiv_regs): ..here. Don't create and free pdx_subregs or reg_equiv. (validate_equiv_regs): New function. (ira): Create and free pdx_subregs and reg_equiv. After deleting insns, call validate_equiv_regs. Call setup_reg_equiv later. (setup_reg_equiv): Protect against deleted insns. gcc/testsuite/ * gcc.dg/pr69195.c: New. * gcc.dg/pr69238.c: New. diff --git a/gcc/ira.c b/gcc/ira.c index 0973258..047e164 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -3284,11 +3284,15 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSED, } } +/* Use pdx_subregs to show whether a reg is used in a paradoxical + subreg. */ +static bool *pdx_subregs; + /* Check whether the SUBREG is a paradoxical subreg and set the result in PDX_SUBREGS. */ static void -set_paradoxical_subreg (rtx_insn *insn, bool *pdx_subregs) +set_paradoxical_subreg (rtx_insn *insn) { subrtx_iterator::array_type array; FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST) @@ -3319,6 +3323,100 @@ adjust_cleared_regs (rtx loc, const_rtx old_rtx ATTRIBUTE_UNUSED, void *data) return NULL_RTX; } +/* For insns that set a MEM to the contents of a REG that is only used + in a single basic block, see if the register is always equivalent + to that memory location and if moving the store from INSN to the + insn that sets REG is safe. If so, put a REG_EQUIV note on the + initializing insn. + + Don't add a REG_EQUIV note if the insn already has one. The + existing REG_EQUIV is likely more useful than the one we are + adding. + + If one of the regs in the address has reg_equiv[REGNO].replace set, + then we can't add this REG_EQUIV note. The reg_equiv[REGNO].replace + optimization may move the set of this register immediately before + insn, which puts it after reg_equiv[REGNO].init_insns, and hence the + mention in the REG_EQUIV note would be to an uninitialized pseudo. */ +static void +add_store_equivs (void) +{ + for (rtx_insn *insn = get_insns (); insn; insn = NEXT_INSN (insn)) + { + if (! INSN_P (insn)) + continue; + + rtx set = single_set (insn); + if (! set) + continue; + + rtx dest = SET_DEST (set); + rtx src = SET_SRC (set); + unsigned int regno; + rtx_insn *init_insn; + + if (MEM_P (dest) + && REG_P (src) + && (regno = REGNO (src)) >= FIRST_PSEUDO_REGISTER + && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS + && DF_REG_DEF_COUNT (regno) == 1 + && ! pdx_subregs[regno] + && reg_equiv[regno].init_insns != 0 + && (init_insn = reg_equiv[regno].init_insns->insn ()) != 0 + && ! find_reg_note (init_insn, REG_EQUIV, NULL_RTX) + && ! contains_replace_regs (XEXP (dest, 0)) + && validate_equiv_mem (init_insn, src, dest) + && ! memref_used_between_p (dest, init_insn, insn) + /* Attaching a REG_EQUIV note will fail if INIT_INSN has + multiple sets. */ + && set_unique_reg_note (init_insn, REG_EQUIV, copy_rtx (dest))) + { + /* This insn makes the equivalence, not the one initializing + the register. */ + ira_reg_equiv[regno].init_insns + = gen_rtx_INSN_LIST (VOIDmode, insn, NULL_RTX); + df_notes_rescan (init_insn); + } + } +} + +/* After deleting dead insns, check that REG_EQUIV notes are still + valid. */ +static void +validate_equiv_regs (void) +{ + bool done_something = false; + unsigned int max = vec_safe_length (reg_equivs); + + for (unsigned int regno = FIRST_PSEUDO_REGISTER; regno < max; regno++) + { + for (rtx_insn_list *elem = reg_equiv[regno].init_insns; + elem; + elem = elem->next ()) + { + rtx_insn *insn = elem->insn (); + rtx set, note; + + if (insn != 0 + && ! insn->deleted () + && (set = single_set (insn)) != 0 + && REG_P (SET_DEST (set)) + && REGNO (SET_DEST (set)) == regno + && MEM_P (SET_SRC (set)) + && (note = find_reg_note (insn, REG_EQUIV, NULL_RTX)) != 0 + && rtx_equal_p (SET_SRC (set), XEXP (note, 0)) + && ! validate_equiv_mem (insn, SET_DEST (set), SET_SRC (set))) + { + remove_note (insn, note); + done_something = true; + } + } + } + + if (done_something) + add_store_equivs (); +} + /* Nonzero if we recorded an equivalence for a LABEL_REF. */ static int recorded_label_ref; @@ -3341,17 +3439,11 @@ update_equiv_regs (void) basic_block bb; int loop_depth; bitmap cleared_regs; - bool *pdx_subregs; /* We need to keep track of whether or not we recorded a LABEL_REF so that we know if the jump optimizer needs to be rerun. */ recorded_label_ref = 0; - /* Use pdx_subregs to show whether a reg is used in a paradoxical - subreg. */ - pdx_subregs = XCNEWVEC (bool, max_regno); - - reg_equiv = XCNEWVEC (struct equivalence, max_regno); grow_reg_equivs (); init_alias_analysis (); @@ -3363,7 +3455,7 @@ update_equiv_regs (void) FOR_EACH_BB_FN (bb, cfun) FOR_BB_INSNS (bb, insn) if (NONDEBUG_INSN_P (insn)) - set_paradoxical_subreg (insn, pdx_subregs); + set_paradoxical_subreg (insn); /* Scan the insns and find which registers have equivalences. Do this in a separate scan of the insns because (due to -fcse-follow-jumps) @@ -3625,65 +3717,7 @@ update_equiv_regs (void) /* A second pass, to gather additional equivalences with memory. This needs to be done after we know which registers we are going to replace. */ - - for (insn = get_insns (); insn; insn = NEXT_INSN (insn)) - { - rtx set, src, dest; - unsigned regno; - - if (! INSN_P (insn)) - continue; - - set = single_set (insn); - if (! set) - continue; - - dest = SET_DEST (set); - src = SET_SRC (set); - - /* If this sets a MEM to the contents of a REG that is only used - in a single basic block, see if the register is always equivalent - to that memory location and if moving the store from INSN to the - insn that set REG is safe. If so, put a REG_EQUIV note on the - initializing insn. - - Don't add a REG_EQUIV note if the insn already has one. The existing - REG_EQUIV is likely more useful than the one we are adding. - - If one of the regs in the address has reg_equiv[REGNO].replace set, - then we can't add this REG_EQUIV note. The reg_equiv[REGNO].replace - optimization may move the set of this register immediately before - insn, which puts it after reg_equiv[REGNO].init_insns, and hence - the mention in the REG_EQUIV note would be to an uninitialized - pseudo. */ - - if (MEM_P (dest) && REG_P (src) - && (regno = REGNO (src)) >= FIRST_PSEUDO_REGISTER - && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS - && DF_REG_DEF_COUNT (regno) == 1 - && reg_equiv[regno].init_insns != NULL - && reg_equiv[regno].init_insns->insn () != NULL - && ! find_reg_note (XEXP (reg_equiv[regno].init_insns, 0), - REG_EQUIV, NULL_RTX) - && ! contains_replace_regs (XEXP (dest, 0)) - && ! pdx_subregs[regno]) - { - rtx_insn *init_insn = - as_a (XEXP (reg_equiv[regno].init_insns, 0)); - if (validate_equiv_mem (init_insn, src, dest) - && ! memref_used_between_p (dest, init_insn, insn) - /* Attaching a REG_EQUIV note will fail if INIT_INSN has - multiple sets. */ - && set_unique_reg_note (init_insn, REG_EQUIV, copy_rtx (dest))) - { - /* This insn makes the equivalence, not the one initializing - the register. */ - ira_reg_equiv[regno].init_insns - = gen_rtx_INSN_LIST (VOIDmode, insn, NULL_RTX); - df_notes_rescan (init_insn); - } - } - } + add_store_equivs (); cleared_regs = BITMAP_ALLOC (NULL); /* Now scan all regs killed in an insn to see if any of them are @@ -3858,8 +3892,6 @@ update_equiv_regs (void) /* Clean up. */ end_alias_analysis (); - free (reg_equiv); - free (pdx_subregs); return recorded_label_ref; } @@ -3882,11 +3914,12 @@ setup_reg_equiv (void) { next_elem = elem->next (); insn = elem->insn (); - set = single_set (insn); /* Init insns can set up equivalence when the reg is a destination or a source (in this case the destination is memory). */ - if (set != 0 && (REG_P (SET_DEST (set)) || REG_P (SET_SRC (set)))) + if (! insn->deleted () + && (set = single_set (insn)) != 0 + && (REG_P (SET_DEST (set)) || REG_P (SET_SRC (set)))) { if ((x = find_reg_note (insn, REG_EQUIV, NULL_RTX)) != NULL) { @@ -5090,7 +5123,6 @@ ira (FILE *f) { bool loops_p; int ira_max_point_before_emit; - int rebuild_p; bool saved_flag_caller_saves = flag_caller_saves; enum ira_region saved_flag_ira_region = flag_ira_region; @@ -5184,10 +5216,9 @@ ira (FILE *f) if (resize_reg_info () && flag_ira_loop_pressure) ira_set_pseudo_classes (true, ira_dump_file); - rebuild_p = update_equiv_regs (); - setup_reg_equiv (); - setup_reg_equiv_init (); - + pdx_subregs = XCNEWVEC (bool, max_regno); + reg_equiv = XCNEWVEC (struct equivalence, max_regno); + bool rebuild_p = update_equiv_regs (); bool update_regstat = false; if (optimize && rebuild_p) @@ -5210,6 +5241,14 @@ ira (FILE *f) update_regstat = true; } + if (update_regstat) + validate_equiv_regs (); + free (reg_equiv); + free (pdx_subregs); + + setup_reg_equiv (); + setup_reg_equiv_init (); + /* It is not worth to do such improvement when we use a simple allocation because of -O0 usage or because the function is too big. */ diff --git a/gcc/testsuite/gcc.dg/pr69195.c b/gcc/testsuite/gcc.dg/pr69195.c new file mode 100644 index 0000000..af373a1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr69195.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-options "-O3 -fno-dce -fno-forward-propagate" } */ + +void __attribute__ ((noinline, noclone)) +foo (int *a, int n) +{ + int *lasta = a + n; + for (; a != lasta; a++) + { + *a *= 2; + a[1] = a[-1] + a[-2]; + } +} + +int +main () +{ + int a[16] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; + int r[16] = { 1, 2, 6, 6, 16, 24, 44, 80, + 136, 248, 432, 768, 1360, 2400, 4256, 3760 }; + unsigned i; + foo (&a[2], 13); + for (i = 0; i < 8; ++i) + if (a[i] != r[i]) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/pr69238.c b/gcc/testsuite/gcc.dg/pr69238.c new file mode 100644 index 0000000..3538e63 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr69238.c @@ -0,0 +1,28 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-dce -fno-forward-propagate -fno-rerun-cse-after-loop -funroll-loops" } */ + + +#define N 32 + +short sa[N]; +short sb[N]; +int ia[N]; +int ib[N]; + +int __attribute__ ((noinline, noclone)) +main1 (int n) +{ + int i; + for (i = 0; i < n; i++) + { + sa[i+7] = sb[i]; + ia[i+3] = ib[i+1]; + } + return 0; +} + +int +main (void) +{ + return main1 (N-7); +}