From patchwork Sun Jul 28 00:51:02 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bill Schmidt X-Patchwork-Id: 262543 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 3745E2C00FE for ; Sun, 28 Jul 2013 10:51:27 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:cc:date:content-type :content-transfer-encoding:mime-version; q=dns; s=default; b=oF6 iUegI4ByrVQ+sMrOdejKUuWewlQCxUmcSceWyeE0T7Xc97fCjnEnIWp0iE/Ww1bM decCm9rUk1jl3St9PvDqk36PsU1L7l3+3aLYT6VWC0PW4N+cQ4ZVh0XQqH4laSfK 3ImVC9gmkXu3RS8wIBpemRF9WZ7wA/JNd/a3i98U= 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 :message-id:subject:from:to:cc:date:content-type :content-transfer-encoding:mime-version; s=default; bh=/93y1rlRC qON+HY8l4YqxJhbNBU=; b=LTRmyAbusSt+aqcfoL1L6coPHfWJMEQEdPZnObAPB 6jy6eZmsvVHeeBJ0aXEKB79R2GNhtJc1J4Ny6s5K7S25JIaPQ0Pcwxc/GfMw0wdu Nr60+X6o9wQr0O80ZJpsikx38xP+KBOx6213FL7wae/86pA9WXmUJmiED0tK1Ip1 Ds= Received: (qmail 30137 invoked by alias); 28 Jul 2013 00:51:20 -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 30112 invoked by uid 89); 28 Jul 2013 00:51:19 -0000 X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_MED, RCVD_IN_HOSTKARMA_W, RDNS_NONE autolearn=ham version=3.3.1 Received: from Unknown (HELO e28smtp01.in.ibm.com) (122.248.162.1) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sun, 28 Jul 2013 00:51:18 +0000 Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 28 Jul 2013 06:13:03 +0530 Received: from d28dlp02.in.ibm.com (9.184.220.127) by e28smtp01.in.ibm.com (192.168.1.131) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Sun, 28 Jul 2013 06:13:00 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 806B5394004D for ; Sun, 28 Jul 2013 06:20:57 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r6S0ow2C40566854 for ; Sun, 28 Jul 2013 06:20:59 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r6S0p2Zo005777 for ; Sun, 28 Jul 2013 10:51:02 +1000 Received: from [9.57.65.224] ([9.57.65.224]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id r6S0oxQA005702; Sun, 28 Jul 2013 10:51:01 +1000 Message-ID: <1374972662.3633.164.camel@gnopaine> Subject: [PATCH] Fix PR57993 (ICE in slsr) From: Bill Schmidt To: gcc-patches@gcc.gnu.org Cc: jakub@redhat.com Date: Sat, 27 Jul 2013 19:51:02 -0500 Mime-Version: 1.0 X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13072800-4790-0000-0000-0000097F18CA PR57993 reports a scenario where a conditional candidate is incorrectly replaced when its putative "hidden basis" (the basis hidden by one or more PHI nodes) does not dominate all of the PHI nodes on which the candidate depends. This indicates that the hidden basis was incorrectly identified as a useful basis for the candidate. There are two ways we can fix this. One would be to add more complexity to the code that determines the hidden basis. Currently that code does not recursively follow the PHI definitions backwards to ensure that the basis dominates all the PHIs. Adding code to do this would ensure we didn't identify an incorrect basis in the first place, but would duplicate quite a bit of existing logic in the later analysis phase, and increase compile time. What I've done here is to instead delay detection of the problem until the analysis phase. If we detect that we chose an invalid basis, we assign an "infinite" cost to the replacement so that we won't pursue it further. This requires that we know which basic block contains the basis statement. The basis statement may have itself been replaced by another statement earlier, so we need to keep the candidate table up to date with respect to such replacements. Bootstrapped and tested on powerpc64-linux-gnu with no new regressions. Ok for trunk? Thanks, Bill gcc: 2013-07-27 Bill Schmidt * gimple-ssa-strength-reduction.c (replace_mult_candidate): Record replaced statement in the candidate table. (phi_add_costs): Return infinite cost when the hidden basis does not dominate all phis on which the candidate is dependent. (replace_one_candidate): Record replaced statement in the candidate table. gcc/testsuite: 2013-07-27 Bill Schmidt * gcc.dg/torture/pr57993.c: New test. Index: gcc/testsuite/gcc.dg/torture/pr57993.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr57993.c (revision 0) +++ gcc/testsuite/gcc.dg/torture/pr57993.c (revision 0) @@ -0,0 +1,30 @@ +/* This ICEd prior to fixing PR57993. */ +/* { dg-do compile } */ + +int a, b, c, d; +char e; +unsigned g; + +void f(void) +{ + int h; + + for(; d; d++) + if(d) +lbl: + g + a || (d = 0); + + b && (a = e); + + for(h = 0; h < 1; ++h) + { + h = c ? : (d = 0); + g = a = (e | 0); + } + + if(a) + goto lbl; + + a = e = 0; + goto lbl; +} Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 201267) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -1882,6 +1882,7 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); gsi_replace (&gsi, copy_stmt, false); + c->cand_stmt = copy_stmt; if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = copy_stmt; } @@ -2179,6 +2180,18 @@ phi_add_costs (gimple phi, slsr_cand_t c, int one_ int cost = 0; slsr_cand_t phi_cand = base_cand_from_table (gimple_phi_result (phi)); + /* If we work our way back to a phi that isn't dominated by the hidden + basis, this isn't a candidate for replacement. Indicate this by + returning an unreasonably high cost. It's not easy to detect + these situations when determining the basis, so we defer the + decision until now. */ + basic_block phi_bb = gimple_bb (phi); + slsr_cand_t basis = lookup_cand (c->basis); + basic_block basis_bb = gimple_bb (basis->cand_stmt); + + if (phi_bb == basis_bb || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb)) + return COST_INFINITE; + for (i = 0; i < gimple_phi_num_args (phi); i++) { tree arg = gimple_phi_arg_def (phi, i); @@ -3226,6 +3239,7 @@ replace_one_candidate (slsr_cand_t c, unsigned i, gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); gsi_replace (&gsi, copy_stmt, false); + c->cand_stmt = copy_stmt; if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = copy_stmt; @@ -3238,6 +3252,7 @@ replace_one_candidate (slsr_cand_t c, unsigned i, NULL_TREE); gimple_set_location (cast_stmt, gimple_location (c->cand_stmt)); gsi_replace (&gsi, cast_stmt, false); + c->cand_stmt = cast_stmt; if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = cast_stmt;