From patchwork Wed Nov 7 09:41:56 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 197642 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 240462C00DB for ; Wed, 7 Nov 2012 20:42:10 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1352886131; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=s9qPNraJntNBT2NZmQQG20K11pc=; b=kCBVjF762MxUlidFxDcpw232srekMI5hAXu1TS1asnaaDElQYa9FG3BtUqS7bE dh4gVKWgbX1G752cnkFi2dqsIz+YEjZ2jsocfzZPFFN9ySscqjr7FoWhNbw/+w32 IHeJbTN5SbMURflUUg0YMpZqXPeULBZQxsrRDYKCjErok= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=JHs9XVl5g3RpYyMtXeFohsX/mv4WoI2RXtqT9DMBpqdE2p2WtiAv086mV/LDW8 jwScg4/X8GqaX22vfkJ3HqRXkyKoTV/Y89PZ2LafE4fX7iubjoF3tDQQBfO67HuN 0eCEXFOaCKM63rJ2BUvAjjD2gH18IUoRFCZg95iFXTlAA=; Received: (qmail 18974 invoked by alias); 7 Nov 2012 09:42:08 -0000 Received: (qmail 18966 invoked by uid 22791); 7 Nov 2012 09:42:08 -0000 X-SWARE-Spam-Status: No, hits=-4.9 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, TW_AV, TW_ZJ X-Spam-Check-By: sourceware.org Received: from mail-pa0-f47.google.com (HELO mail-pa0-f47.google.com) (209.85.220.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Nov 2012 09:41:57 +0000 Received: by mail-pa0-f47.google.com with SMTP id fa11so977492pad.20 for ; Wed, 07 Nov 2012 01:41:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.68.189.163 with SMTP id gj3mr11967946pbc.110.1352281316593; Wed, 07 Nov 2012 01:41:56 -0800 (PST) Received: by 10.66.246.232 with HTTP; Wed, 7 Nov 2012 01:41:56 -0800 (PST) In-Reply-To: <20121107080421.GA1881@tucnak.redhat.com> References: <20121106221813.GZ1881@tucnak.redhat.com> <20121107080421.GA1881@tucnak.redhat.com> Date: Wed, 7 Nov 2012 10:41:56 +0100 Message-ID: Subject: Re: [PATCH] Vzeroupper placement/47440 From: Uros Bizjak To: Jakub Jelinek Cc: "H.J. Lu" , Kirill Yukhin , Vladimir Yakovlev , 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 Wed, Nov 7, 2012 at 9:04 AM, Jakub Jelinek wrote: > On Wed, Nov 07, 2012 at 08:08:08AM +0100, Uros Bizjak wrote: >> >> 2012-11-06 Jakub Jelinek >> >> >> >> * config/i386/i386.c (ix86_avx_u128_mode_after): Don't >> >> look for reg in CALL operand. >> > >> > OK. >> >> You can also break the loop after reg is found. > > I have committed the patch as is to fix the bootstrap, as anything else > needs another bootstrap/regtest. I don't think breaking out of the loop > would be correct, then say for *{,sib}call_value_pop patterns reg would be > stack pointer rather than the return value of the call. Due to that pattern > we can't use single_set, but I wonder if we just can't use XVECEXP (pat, 0, 0) > unconditionally for the return value, or perhaps check > the condition inside of the loop (REG_P (reg) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE > (reg))), return AVX_U128_DIRTY if true (and that way break out of the loop), > and return AVX_U128_CLEAN after the loop. Indeed, I didn't notice reverse loop. > Or I wonder why is call handled specially at all, doesn't > /* Check if a 256bit AVX register is referenced in stores. */ > state = unused; > note_stores (pat, check_avx256_stores, &state); > if (state == used) > return AVX_U128_DIRTY; > handle it? Then it would just need to be if (CALL_P (insn)) return AVX_U128_CLEAN. You are right, I am testing the attached patch. > BTW, the formatting is wrong in some spots, e.g. > check_avx256_stores (rtx dest, const_rtx set, void *data) > { > if (((REG_P (dest) || MEM_P(dest)) I have some doubts that this function is fully correct. The comment says: /* Check if a 256bit AVX register is referenced in stores. */ But, we are in fact checking stores and uses. IIRC, U128_DIRTY state is only set for stores to YMM register, so: static void check_avx256_stores (rtx dest, const_rtx set ATTRIBUTE_UNUSED, void *data) { if (REG_P (dest) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (dest))) { enum upper_128bits_state *state = (enum upper_128bits_state *) data; *state = used; } } > I'd prefer to leave this to the original submitter. Yes, Vladimir, can you please comment on these issues? Uros. Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 193280) +++ config/i386/i386.c (working copy) @@ -15079,30 +15079,8 @@ ix86_avx_u128_mode_after (int mode, rtx insn) { rtx pat = PATTERN (insn); - rtx reg = NULL; - int i; enum upper_128bits_state state; - /* Check for CALL instruction. */ - if (CALL_P (insn)) - { - if (GET_CODE (pat) == SET || GET_CODE (pat) == CALL) - reg = SET_DEST (pat); - else if (GET_CODE (pat) == PARALLEL) - for (i = XVECLEN (pat, 0) - 1; i >= 0; i--) - { - rtx x = XVECEXP (pat, 0, i); - if (GET_CODE(x) == SET) - reg = SET_DEST (x); - } - /* Mode after call is set to AVX_U128_DIRTY if there are - 256bit modes used in the function return register. */ - if (reg && REG_P (reg) && VALID_AVX256_REG_OR_OI_MODE (GET_MODE (reg))) - return AVX_U128_DIRTY; - else - return AVX_U128_CLEAN; - } - if (vzeroupper_operation (pat, VOIDmode) || vzeroall_operation (pat, VOIDmode)) return AVX_U128_CLEAN; @@ -15112,6 +15090,10 @@ note_stores (pat, check_avx256_stores, &state); if (state == used) return AVX_U128_DIRTY; + /* We know that state is clean after CALL insn if there are no + 256bit modes used in the function return register. */ + else if (CALL_P (insn) && state == unused) + return AVX_U128_CLEAN; return mode; }