From patchwork Mon Feb 28 16:17:48 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 252 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.159.109) by localhost6.localdomain6 with IMAP4-SSL; 08 Jun 2011 14:41:12 -0000 Delivered-To: patches@linaro.org Received: by 10.224.19.208 with SMTP id c16cs103232qab; Mon, 28 Feb 2011 08:17:52 -0800 (PST) Received: by 10.216.89.71 with SMTP id b49mr5050500wef.28.1298909872116; Mon, 28 Feb 2011 08:17:52 -0800 (PST) Received: from mail-wy0-f178.google.com (mail-wy0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id h47si6812236wer.180.2011.02.28.08.17.51 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 28 Feb 2011 08:17:52 -0800 (PST) Received-SPF: neutral (google.com: 74.125.82.178 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) client-ip=74.125.82.178; Authentication-Results: mx.google.com; spf=neutral (google.com: 74.125.82.178 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) smtp.mail=richard.sandiford@linaro.org Received: by wyf28 with SMTP id 28so4441860wyf.37 for ; Mon, 28 Feb 2011 08:17:51 -0800 (PST) Received: by 10.227.137.5 with SMTP id u5mr5107959wbt.6.1298909871479; Mon, 28 Feb 2011 08:17:51 -0800 (PST) Received: from richards-thinkpad (gbibp9ph1--blueice4n2.emea.ibm.com [195.212.29.92]) by mx.google.com with ESMTPS id j49sm1959518wer.14.2011.02.28.08.17.49 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 28 Feb 2011 08:17:50 -0800 (PST) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, patches@linaro.org, richard.sandiford@linaro.org Cc: patches@linaro.org Subject: PR rtl-optimization/47925: deleting trivially live instructions Date: Mon, 28 Feb 2011 16:17:48 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 If we have a sequence like: (set (reg A) ...) (set (reg A) (mem/v (reg A))) [REG_DEAD A] then a long-standing bug in delete_trivially_dead_insns will cause it to delete the first instruction. count_reg_usage counts out how many times each register is used in the entire function, but it tries to ignore uses in self-modifications of the form (set (reg A) (... (reg A) ...)). The problem is that it is ignoring such uses even if the insn contains an access to volatile memory. insn_live_p rightly returns true for such insns, regardless of register counts, so we end up keeping the self-modification but deleting the instruction that sets its input. count_reg_usage is set up to predict when insn_live_p would always return true regardless of register usage. It is doing this correctly for instructions that might throw an exception, and for volatile asms, but it is failing to do it for other side effects that insn_live_p detects. These include volatile MEMs and pre-/post-modifications. Bootstrapped & regression-tested on x86_64-linux-gnu. OK to install? The bug showed up as a wrong-code regression in a modified version of Qt, but I haven't yet found a testcase that fails with 4.6 but passes with an older GCC. Is the patch OK now regardless, or should it wait for 4.7? Richard gcc/ PR rtl-optimization/47925 * cse.c (count_reg_usage): Don't ignore the SET_DEST of instructions with side effects. Remove the more-specific check for volatile asms. gcc/testsuite/ PR rtl-optimization/47925 * gcc.c-torture/execute/pr47925.c: New test. Index: gcc/cse.c =================================================================== --- gcc/cse.c 2010-10-18 10:53:30.000000000 +0100 +++ gcc/cse.c 2011-02-28 15:22:51.000000000 +0000 @@ -6629,9 +6629,10 @@ count_reg_usage (rtx x, int *counts, rtx case CALL_INSN: case INSN: case JUMP_INSN: - /* We expect dest to be NULL_RTX here. If the insn may trap, mark - this fact by setting DEST to pc_rtx. */ - if (insn_could_throw_p (x)) + /* We expect dest to be NULL_RTX here. If the insn may trap, + or if it cannot be deleted due to side-effects, mark this fact + by setting DEST to pc_rtx. */ + if (insn_could_throw_p (x) || side_effects_p (PATTERN (x))) dest = pc_rtx; if (code == CALL_INSN) count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, dest, incr); @@ -6671,10 +6672,6 @@ count_reg_usage (rtx x, int *counts, rtx return; case ASM_OPERANDS: - /* If the asm is volatile, then this insn cannot be deleted, - and so the inputs *must* be live. */ - if (MEM_VOLATILE_P (x)) - dest = NULL_RTX; /* Iterate over just the inputs, not the constraints as well. */ for (i = ASM_OPERANDS_INPUT_LENGTH (x) - 1; i >= 0; i--) count_reg_usage (ASM_OPERANDS_INPUT (x, i), counts, dest, incr); Index: gcc/testsuite/gcc.c-torture/execute/pr47925.c =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ gcc/testsuite/gcc.c-torture/execute/pr47925.c 2011-02-28 16:05:17.000000000 +0000 @@ -0,0 +1,24 @@ +struct s { volatile struct s *next; }; + +void __attribute__((noinline)) +bar (int ignored, int n) +{ + asm volatile (""); +} + +int __attribute__((noinline)) +foo (volatile struct s *ptr, int n) +{ + int i; + + bar (0, n); + for (i = 0; i < n; i++) + ptr = ptr->next; +} + +int main (void) +{ + volatile struct s rec = { &rec }; + foo (&rec, 10); + return 0; +}