From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15530 invoked by alias); 22 Jan 2016 23:02:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 15520 invoked by uid 89); 22 Jan 2016 23:02:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Our, supplied, our X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 22 Jan 2016 23:02:49 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id B1FAA542F92; Sat, 23 Jan 2016 00:02:44 +0100 (CET) Date: Fri, 22 Jan 2016 23:02:00 -0000 From: Jan Hubicka To: Jakub Jelinek Cc: Jan Hubicka , Uros Bizjak , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix -minline-stringops-dynamically (PR target/69432) Message-ID: <20160122230244.GF5527@kam.mff.cuni.cz> References: <20160122221449.GB3017@tucnak.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160122221449.GB3017@tucnak.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2016-01/txt/msg01799.txt.bz2 > Hi! > > With this option we generate a conditional library call. When expanding > such a call, do_pending_stack_adjust () is performed from expand_call > and if there are any needed sp adjustments, they are emitted. The > problem is that this happens only in conditionally executed code, so on some > path the args size level will be correct, on others it might be wrong. > > Fixed by doing the adjustment before the first conditional jump if we know > we'll emit a call conditionally. Bootstrapped/regtested on x86_64-linux and > i686-linux, ok for trunk? > > BTW, when looking at this, I've noticed something strange, > expand_set_or_movmem_prologue_epilogue_by_misaligned_moves has bool > dynamic_check argument, but the caller has int dynamic_check and in the > caller as I understand the meaning is dynamic_check == -1 means no > conditional library call, otherwise there is a conditional library call with > some specific max size. When calling > expand_set_or_movmem_prologue_epilogue_by_misaligned_moves, the > dynamic_check value is passed to the bool argument though, so that means > dynamic_check != 0 instead of dynamic_check != -1. Honza, can you please > have a look at that? Yep, that indeed looks odd. I will take a look. Path is OK. Thanks, Honza > > 2016-01-22 Jakub Jelinek > > PR target/69432 > * config/i386/i386.c: Include dojump.h. > (expand_small_movmem_or_setmem, > expand_set_or_movmem_prologue_epilogue_by_misaligned_moves): Spelling > fixes. > (ix86_expand_set_or_movmem): Call do_pending_stack_adjust () early > if dynamic_check != -1. > > * g++.dg/opt/pr69432.C: New test. > > --- gcc/config/i386/i386.c.jj 2016-01-19 09:20:34.000000000 +0100 > +++ gcc/config/i386/i386.c 2016-01-22 20:39:32.289457234 +0100 > @@ -75,6 +75,7 @@ along with GCC; see the file COPYING3. > #include "dbgcnt.h" > #include "case-cfn-macros.h" > #include "regrename.h" > +#include "dojump.h" > > /* This file should be included last. */ > #include "target-def.h" > @@ -25700,7 +25701,7 @@ expand_small_movmem_or_setmem (rtx destm > if (DYNAMIC_CHECK) > Round COUNT down to multiple of SIZE > << optional caller supplied zero size guard is here >> > - << optional caller suppplied dynamic check is here >> > + << optional caller supplied dynamic check is here >> > << caller supplied main copy loop is here >> > } > done_label: > @@ -25875,8 +25876,8 @@ expand_set_or_movmem_prologue_epilogue_b > else > *min_size = 0; > > - /* Our loops always round down the bock size, but for dispatch to library > - we need precise value. */ > + /* Our loops always round down the block size, but for dispatch to > + library we need precise value. */ > if (dynamic_check) > *count = expand_simple_binop (GET_MODE (*count), AND, *count, > GEN_INT (-size), *count, 1, OPTAB_DIRECT); > @@ -26469,6 +26470,13 @@ ix86_expand_set_or_movmem (rtx dst, rtx > size_needed = GET_MODE_SIZE (move_mode) * unroll_factor; > epilogue_size_needed = size_needed; > > + /* If we are going to call any library calls conditionally, make sure any > + pending stack adjustment happen before the first conditional branch, > + otherwise they will be emitted before the library call only and won't > + happen from the other branches. */ > + if (dynamic_check != -1) > + do_pending_stack_adjust (); > + > desired_align = decide_alignment (align, alg, expected_size, move_mode); > if (!TARGET_ALIGN_STRINGOPS || noalign) > align = desired_align; > --- gcc/testsuite/g++.dg/opt/pr69432.C.jj 2016-01-22 20:51:19.463428357 +0100 > +++ gcc/testsuite/g++.dg/opt/pr69432.C 2016-01-22 20:51:04.000000000 +0100 > @@ -0,0 +1,62 @@ > +// PR target/69432 > +// { dg-do compile } > +// { dg-options "-O3" } > +// { dg-additional-options "-minline-stringops-dynamically" { target i?86-*-* x86_64-*-* } } > + > +template > +void > +f1 (S x, T y, U z) > +{ > + for (; y; --y, ++x) > + *x = z; > +} > + > +template > +void f2 (S x, T y, U z) > +{ > + f1 (x, y, z); > +} > + > +struct A {}; > +struct B { static char f3 (A, unsigned); }; > + > +template > +void f4 (S, U); > + > +struct C > +{ > + template > + static S f5 (S x, T y, U z) { f2 (x, y, z); } > +}; > + > +template > +void f6 (S x, T y, U z) { C::f5 (x, y, z); } > + > +template > +void f7 (S x, T y, U z, V) { f6 (x, y, z); } > + > +struct E > +{ > + struct D : A { char e; D (A); }; > + A f; > + E (int x) : g(f) { f8 (x); } > + ~E (); > + D g; > + void f9 (int x) { x ? B::f3 (g, x) : char (); } > + void f8 (int x) { f9 (x); } > +}; > + > +struct F : E > +{ > + F (int x) : E(x) { f10 (x); f4 (this, 0); } > + char h; > + void f10 (int x) { f7 (&g.e, x, h, 0); } > +}; > + > +long a; > + > +void > +test () > +{ > + F b(a); > +} > > Jakub