From patchwork Mon Jun 20 17:31:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Enkovich X-Patchwork-Id: 638179 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 3rYJ2d5mzTz9sdn for ; Tue, 21 Jun 2016 03:37:13 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=GCdzdti9; 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:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=DnprjgJw728ynj5pu 1ejpm9I9UyiJsMKaDvDAtXNKgIvjX3HLrQiJoj8HIWdriUS8sFmbAjIJAHTD3DU8 3faZh2I3NQ8hMqS/rYBOUV7BlTX76rbqLNBg3J3yS6gEQXrratfXZUVRd5OUgalg DkNH/yHfKvrBzdzeKPE7DUIBYY= 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:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=cLX1guySRXPDQ+miJV1S13G ez0c=; b=GCdzdti98HZ5/8KEVmgoWosqVyZYe6i8yNEX7dFjmxQ1PX/9O0R2mKa xSzPlM6pDDUWUNmMN7TJ2XIjg/AZ41EgCRpNRx+IkkiSQPyW7AwJnD/gSOCNXOq6 1o13o/hle/yRrTjivFr7D6jdTHGJHkY3o173/MEBbC1Gjlm1VBkE= Received: (qmail 25895 invoked by alias); 20 Jun 2016 17:36:39 -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 25808 invoked by uid 89); 20 Jun 2016 17:36:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=hongjiuluintelcom, hongjiu.lu@intel.com, ubizjak@gmail.com, ubizjakgmailcom X-HELO: mail-wm0-f42.google.com Received: from mail-wm0-f42.google.com (HELO mail-wm0-f42.google.com) (74.125.82.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 20 Jun 2016 17:36:27 +0000 Received: by mail-wm0-f42.google.com with SMTP id f126so78605839wma.1 for ; Mon, 20 Jun 2016 10:36:26 -0700 (PDT) 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:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=QTSYVBl0CyMImSxmBgBakZsErsxfFQxdd1uexIB1Sn4=; b=Qvob7gkc1teasJ4eUS4NZB08A+86slMRFUoKK8h/ErKTgjhZYogw9x7bMOdSfDDKAS 5SaQOzYcyS8wL5FWEWbk8NnPxIV3DvjyfA96zK2mAz+mq1QK8aqQR1PmeQ6Ac7rpvJIS cgwPBHgQ/xK4JmuJY6dc6WuDh1Lnpw25dqr5E3u3Uyt5himOcTnTgAQswqfE0hWz2uq1 NQg8akbu4M7oXy8wcI11d0vMMapQ1dX1dGc6k7YgDGdHY9C4sWBtZg3RSXCsGCAFZI6k bvPiZxErKhmnxC6d3MR4eRQwzl6hY14e3dHgSWBuMBdlRG8x41iPeUGJzMJNwivQlvCj KVUQ== X-Gm-Message-State: ALyK8tIAGAwFbx6y8+ahwTJmLT/Es6jZX60KE4gIaWmNuveEWFy40V6conqnmP2+ujPG0A== X-Received: by 10.194.173.230 with SMTP id bn6mr7366283wjc.8.1466444184086; Mon, 20 Jun 2016 10:36:24 -0700 (PDT) Received: from msticlxl57.ims.intel.com ([192.198.151.43]) by smtp.gmail.com with ESMTPSA id ue1sm43809160wjc.44.2016.06.20.10.36.22 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Jun 2016 10:36:23 -0700 (PDT) Date: Mon, 20 Jun 2016 20:31:30 +0300 From: Ilya Enkovich To: "H.J. Lu" Cc: Uros Bizjak , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] PR target/71549: Convert V1TImode register to TImode in debug insn Message-ID: <20160620173130.GA30064@msticlxl57.ims.intel.com> References: <20160620115515.GA4587@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes On 20 Jun 09:45, H.J. Lu wrote: > On Mon, Jun 20, 2016 at 7:30 AM, Ilya Enkovich wrote: > > 2016-06-20 16:39 GMT+03:00 Uros Bizjak : > >> On Mon, Jun 20, 2016 at 1:55 PM, H.J. Lu wrote: > >>> TImode register referenced in debug insn can be converted to V1TImode > >>> by scalar to vector optimization. We need to convert a debug insn if > >>> it has a variable in a TImode register. > >>> > >>> Tested on x86-64. OK for trunk? > >> > >> Ilya, does this approach look good to you? Also, does DImode STV > >> conversion need similar handling of debug insns? > > > > DImode conversion doesn't change register mode (i.e. never calls > > PUT_MODE for registers). That keeps debug instructions valid. > > > > Overall I don't like the idea of having debug insns in candidates > > set and in chains. Looks like it is possible to have a chain > > consisting of a debug insn only which is weird (otherwise I don't > > see why we may return false in timode_scalar_chain::convert_insn). > > Yes, it can happen: > > (insn 11 8 12 2 (parallel [ > (set (reg/v:TI 91 [ ]) > (plus:TI (reg/v:TI 92 [ a ]) > (reg/v:TI 96 [ b ]))) > (clobber (reg:CC 17 flags)) > ]) y.i:5 210 {*addti3_doubleword} > (expr_list:REG_UNUSED (reg:CC 17 flags) > (nil))) > (debug_insn 12 11 13 2 (var_location:TI w (reg/v:TI 91 [ ])) y.i:5 -1 > (nil)) > > > > What about other possible register uses? If debug insns are added > > to candidates then NONDEBUG_INSN_P check for uses in > > timode_check_non_convertible_regs becomes invalid, right? > > Debug insn has no impact on STV decision. We just need to convert > register referenced in debug insn from V1TImode to TImode in > timode_scalar_chain::convert_insn. > > > If we have (or want) to fix some register uses then it's probably > > would be better to visit register uses when we convert its mode > > and make required fix-ups. It seems better to me to not involve > > debug insns in analysis phase. > > Here is the updated patch to add debug insn, which references the > TImode register which will be converted to V1TImode to queue. > I am testing it now. > You still count and dump debug insns as optimized ones. Also we try to use virtual functions to cover differences in DI and TI optimizations and introducing additional TARGET_64BIT in common STV code is undesirable. Also your conversion now depends on instructions processing order. You will fail to process debug insn before non-debug ones. Required order is not guaranteed because processing depends on instruction UIDs only. I propose to modify transformation phase only like in the patch (untested) below. I rely on your code which assumes the only possible usage in debug insn is VAR_LOCATION. Thanks, Ilya diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index c5e5e12..ec955f0 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -3139,6 +3139,7 @@ class timode_scalar_chain : public scalar_chain private: void mark_dual_mode_def (df_ref def); + void fix_debug_reg_uses (rtx reg); void convert_insn (rtx_insn *insn); /* We don't convert registers to difference size. */ void convert_registers () {} @@ -3790,6 +3791,34 @@ dimode_scalar_chain::convert_insn (rtx_insn *insn) df_insn_rescan (insn); } +/* Fix uses of converted REG in debug insns. */ + +void +timode_scalar_chain::fix_debug_reg_uses (rtx reg) +{ + df_ref ref; + for (ref = DF_REG_USE_CHAIN (REGNO (reg)); ref; ref = DF_REF_NEXT_REG (ref)) + { + rtx_insn *insn = DF_REF_INSN (ref); + + if (DEBUG_INSN_P (insn)) + { + /* It must be a debug insn with a TImode variable in register. */ + rtx val = PATTERN (insn); + gcc_assert (GET_MODE (val) == TImode + && GET_CODE (val) == VAR_LOCATION); + rtx loc = PAT_VAR_LOCATION_LOC (val); + gcc_assert (REG_P (loc) + && GET_MODE (loc) == V1TImode); + /* Convert V1TImode register, which has been updated by a SET + insn before, to SUBREG TImode. */ + PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0); + df_insn_rescan (insn); + return; + } + } +} + /* Convert INSN from TImode to V1T1mode. */ void @@ -3806,8 +3835,10 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) rtx tmp = find_reg_equal_equiv_note (insn); if (tmp) PUT_MODE (XEXP (tmp, 0), V1TImode); + PUT_MODE (dst, V1TImode); + fix_debug_reg_uses (dst); } - /* FALLTHRU */ + break; case MEM: PUT_MODE (dst, V1TImode); break;