From patchwork Thu Aug 5 08:46:36 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 60937 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 28C94B70DB for ; Thu, 5 Aug 2010 18:46:48 +1000 (EST) Received: (qmail 28861 invoked by alias); 5 Aug 2010 08:46:45 -0000 Received: (qmail 28851 invoked by uid 22791); 5 Aug 2010 08:46:44 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, TW_ZJ X-Spam-Check-By: sourceware.org Received: from mail-qw0-f47.google.com (HELO mail-qw0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 05 Aug 2010 08:46:38 +0000 Received: by qwg8 with SMTP id 8so3886781qwg.20 for ; Thu, 05 Aug 2010 01:46:36 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.231.7 with SMTP id jo7mr3287164qcb.38.1280997996650; Thu, 05 Aug 2010 01:46:36 -0700 (PDT) Received: by 10.229.35.4 with HTTP; Thu, 5 Aug 2010 01:46:36 -0700 (PDT) In-Reply-To: References: Date: Thu, 5 Aug 2010 10:46:36 +0200 Message-ID: Subject: Re: [PATCH, rtl]: Do not generate insns with mismatched REG_EQUAL mode for multiword MULT RTXes. From: Uros Bizjak To: Steven Bosscher Cc: gcc-patches@gcc.gnu.org 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 On Thu, Aug 5, 2010 at 1:09 AM, Steven Bosscher wrote: > On Thu, Aug 5, 2010 at 12:20 AM, Uros Bizjak wrote: >> Perhaps we should generate a new insn that would survive optimization passes? > > I think you should just take a new register, instead of generating a > no-op move. This probably helps for other things as well, e.g. for > passes that look at DF_REG_DEF_COUNT (reg_scan, alias, ...). OTOH, > maybe that only "helps" up to CPROP pass, if the register is > copy-propagated and the set is made useless.  Dunno... Thanks for your comments! Following your suggestion, I propose attached patch, where we expand shift into temporary and copy this temporary to final target (the same way other algorithms do via force_operand). The note now attaches to non-noop insn: ;; D.2028_13 = acc_24 * 10; (insn 51 50 52 920501-6.c:17 (set (reg:DI 107) (reg/v:DI 87 [ acc ])) -1 (nil)) (insn 52 51 53 920501-6.c:17 (set (reg:SI 109) (lshiftrt:SI (subreg:SI (reg:DI 107) 4) (const_int 31 [0x1f]))) -1 (nil)) (insn 53 52 54 920501-6.c:17 (set (subreg:SI (reg:DI 108) 0) (ashift:SI (subreg:SI (reg:DI 107) 0) (const_int 1 [0x1]))) -1 (nil)) (insn 54 53 55 920501-6.c:17 (set (subreg:SI (reg:DI 108) 0) (ior:SI (reg:SI 109) (subreg:SI (reg:DI 108) 0))) -1 (nil)) (insn 55 54 56 920501-6.c:17 (set (subreg:SI (reg:DI 108) 4) (ashift:SI (subreg:SI (reg:DI 107) 4) (const_int 1 [0x1]))) -1 (nil)) (insn 56 55 57 920501-6.c:17 (set (reg:DI 107) (reg:DI 108)) -1 (expr_list:REG_EQUAL (mult:DI (reg/v:DI 87 [ acc ]) (const_int 2 [0x2])) (nil))) When this stream is processed by subreg1 pass, the register is split into SImode regs, and REG_EQUAL note is *correctly* removed. OTOH, without the patch, REG_EQUAL survives through subreg1 pass, leaving following semi-invalid RTX: (insn 30 29 31 4 920501-6.c:17 (set (reg:SI 225 [+4 ]) (ashift:SI (reg:SI 223 [+4 ]) (const_int 1 [0x1]))) 41 {ashlsi3} (expr_list:REG_EQUAL (mult:DI (reg/v:DI 41 [ s ]) (const_int 2 [0x2])) (nil))) Also, looking through the dumps, I saw: 920501-6.c.152r.fwprop1: Setting REG_EQUAL note 920501-6.c.152r.fwprop1: Setting REG_EQUAL note 920501-6.c.152r.fwprop1: Setting REG_EQUAL note So, it looks to me, that somebody actually thought on copying REG_EQUALs when values are propagated... 2010-08-04 Uros Bizjak * expmed.c (expand_mult_const) : Expand shift into temporary. Emit move from temporary to accum, so REG_EQUAL note will be attached to this insn in correct mode. This patch fixes original ivopts failure as well. Patch is currently regression testing on x86_64-pc-linux-gnu. OK for mainline and release branches, if it passes testing? Uros. Index: expmed.c =================================================================== --- expmed.c (revision 162856) +++ expmed.c (working copy) @@ -3006,9 +3006,10 @@ switch (alg->op[opno]) { case alg_shift: - accum = expand_shift (LSHIFT_EXPR, mode, accum, - build_int_cst (NULL_TREE, log), - NULL_RTX, 0); + tem = expand_shift (LSHIFT_EXPR, mode, accum, + build_int_cst (NULL_TREE, log), + NULL_RTX, 0); + emit_move_insn (accum, tem); val_so_far <<= log; break;