From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28392 invoked by alias); 27 Oct 2015 08:56:53 -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 28380 invoked by uid 89); 27 Oct 2015 08:56:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qg0-f50.google.com Received: from mail-qg0-f50.google.com (HELO mail-qg0-f50.google.com) (209.85.192.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 27 Oct 2015 08:56:50 +0000 Received: by qgem9 with SMTP id m9so140468692qge.1 for ; Tue, 27 Oct 2015 01:56:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=a+Y5AbL1JGcbFUUHYR2eTzMuGrPQ5W/WFrRECCPQmw4=; b=iZT8oEuYgb7LhlepeigKwPy7mQYSsduib2T3cOB4puFkChQVAWhjV4YMfr0RwdGTXr qy0Egs1OmdSF/pydmeHbsLtjlo9cMlCcQuzh3UR1txBvcNIHkIG7W6wpLFLbawmxRLxr OzxqX1f3+98yLe5EJ9uWURtA60M9mwJUnks3OclCQgaA+phSUdI3xBZpRLGZrePDKrGH xSTDopxtPOWmam10+DbhYf81qOSOE/zmPRCk6DGyQ+S0vfWmQcw+t7rTx4qfU4y+HdlM uwcN+Ff9hvWDHBmFkQj4TqJ2S6VCTIYGgaR2N/HpS9IvM4wJpkpA0QqzDVKyQU0rrjCP Qrlg== X-Gm-Message-State: ALoCoQkKLZfWc2YqduAe3XWPnF1ASTFq8/R+A9r8p5sQKknnWIvzp3IKXMuix8ilADRBI1H2ffld MIME-Version: 1.0 X-Received: by 10.140.133.198 with SMTP id 189mr48450770qhf.99.1445936208292; Tue, 27 Oct 2015 01:56:48 -0700 (PDT) Received: by 10.140.44.10 with HTTP; Tue, 27 Oct 2015 01:56:48 -0700 (PDT) In-Reply-To: References: <20151008153345.GO7998@virgil.suse.cz> Date: Tue, 27 Oct 2015 09:15:00 -0000 Message-ID: Subject: Re: [PR 67794] Also remap SSA_NAMEs defined in ASMs in IPA-SRA From: Christophe Lyon To: Richard Biener Cc: Martin Jambor , GCC Patches , Alexandre Oliva Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg02846.txt.bz2 On 9 October 2015 at 09:46, Richard Biener wrote: > On Thu, 8 Oct 2015, Martin Jambor wrote: > >> Hi, >> >> the following fixes PR 67794 by properly remapping SSA_NAMEs which are >> based on PARM_DECLs which are about to be removed as unnecessary. And >> by "properly" I mean also when they are defined by a GIMPL_ASM >> statement. In fact, it switches to using an iterator over definitions >> to make sure it always handles everything... well,except for PHIs >> which are still handled specially because, from a quick glance over >> their source, it seemed to me that the iterator does not support them. >> >> Bootstrapped and tested on x86_64-linux. OK for trunk? >> The issue is most probably latent on a number of old branches, do we >> want to backport the patch to any of them? >> >> Thanks, >> >> Martin >> >> >> 2015-10-08 Martin Jambor >> >> tree-optimization/67794 >> * tree-sra.c (replace_removed_params_ssa_names): Do not distinguish >> between types of state,ents but accept original definitions as a >> parameter. >> (ipa_sra_modify_function_body): Use FOR_EACH_SSA_DEF_OPERAND to >> iterate over definitions. >> >> testsuite/ >> * gcc.dg/ipa/ipa-sra-10.c: Nw test. >> * gcc.dg/torture/pr67794.c: Likewise. >> >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-10.c b/gcc/testsuite/gcc.d= g/ipa/ipa-sra-10.c >> new file mode 100644 >> index 0000000..24b64d1 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-10.c >> @@ -0,0 +1,34 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fipa-sra -fdump-tree-eipa_sra-details" } */ >> + >> +extern void consume (int); >> +extern int glob, glob1, glob2; >> +extern int get (void); >> + >> + >> +static void __attribute__ ((noinline)) >> +foo (int a) >> +{ >> + a =3D glob; >> + consume (a); >> + a =3D get (); >> + consume (a); >> + __asm__ volatile("" : : ""(a)); >> + consume (a); >> + >> + if (glob1) >> + a =3D glob1; >> + else >> + a =3D glob2; >> + consume (a); >> +} >> + >> +int >> +bar (int a) >> +{ >> + foo (a); >> + glob =3D a; >> + return 0; >> +} >> + >> +/* { dg-final { scan-tree-dump-times "replacing an SSA name of a remove= d param" 4 "eipa_sra" } } */ >> diff --git a/gcc/testsuite/gcc.dg/torture/pr67794.c b/gcc/testsuite/gcc.= dg/torture/pr67794.c >> new file mode 100644 >> index 0000000..5489e56 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/torture/pr67794.c >> @@ -0,0 +1,15 @@ >> +/* { dg-do compile } */ >> + >> +int *b; >> +static void fn1(int *best, int *dmin) { >> + int a[64]; >> + dmin =3D a; >> + __asm__ volatile("" : "+&r"(dmin) : ""(best)); >> +} >> + >> +__attribute__((always_inline)) static inline void fn2(int *best) { fn1(= best, b); } >> + >> +void fn3(void) { >> + int c[1]; >> + fn2(c); >> +} >> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c >> index 4327990..f2a4e72 100644 >> --- a/gcc/tree-sra.c >> +++ b/gcc/tree-sra.c >> @@ -4612,61 +4612,45 @@ get_adjustment_for_base (ipa_parm_adjustment_vec= adjustments, tree base) >> return NULL; >> } >> >> -/* If the statement STMT defines an SSA_NAME of a parameter which is to= be >> - removed because its value is not used, replace the SSA_NAME with a o= ne >> - relating to a created VAR_DECL together all of its uses and return t= rue. >> - ADJUSTMENTS is a pointer to an adjustments vector. */ >> +/* If OLD_NAME, which is being defined by statement STMT, is an SSA_NAM= E of a >> + parameter which is to be removed because its value is not used, crea= te a new >> + SSA_NAME relating to a replacement VAR_DECL, replace all uses of the >> + original with it and return it. If there is no need to re-map, retu= rn true. >> + ADJUSTMENTS is a pointer to a vector of IPA-SRA adjustments. */ > > The docs still say we return a bool > > Otherwise the patch looks ok to me. I think we want to backport it > to branche(s) after a while. > Hi Martin, After your backport in the gcc-5 branch, I see build failures: /tmp/2849532_27.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-sra.c: In function =E2=80=98tree_node* replace_removed_params_ssa_names(tree_node*, gimple_statement_base**, ipa_parm_adjustment_vec)=E2=80=99: /tmp/2849532_27.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-sra.c:46= 09: error: cannot convert =E2=80=98gimple_statement_base**=E2=80=99 to =E2=80=98gimple_statement_base*=E2=80=99 for argument =E2=80=982=E2=80=99 t= o =E2=80=98tree_node* make_ssa_name(tree_node*, gimple_statement_base*)=E2=80=99 /tmp/2849532_27.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-sra.c: In function =E2=80=98bool ipa_sra_modify_function_body(ipa_parm_adjustment_vec)=E2=80=99: /tmp/2849532_27.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-sra.c:47= 03: error: cannot convert =E2=80=98gphi*=E2=80=99 to =E2=80=98gimple_statement_= base**=E2=80=99 for argument =E2=80=982=E2=80=99 to =E2=80=98tree_node* replace_removed_params_ssa_names(tree_node*, gimple_statement_base**, ipa_parm_adjustment_vec)=E2=80=99 /tmp/2849532_27.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/tree-sra.c:47= 72: error: cannot convert =E2=80=98gimple_statement_base*=E2=80=99 to =E2=80=98gimple_statement_base**=E2=80=99 for argument =E2=80=982=E2=80=99 = to =E2=80=98tree_node* replace_removed_params_ssa_names(tree_node*, gimple_statement_base**, ipa_parm_adjustment_vec)=E2=80=99 make[2]: *** [tree-sra.o] Error 1 I see this on aarch64* and arm* targets. Can you fix this? Thanks, Christophe. > Thanks, > Richard. > >> -static bool >> -replace_removed_params_ssa_names (gimple *stmt, >> +static tree >> +replace_removed_params_ssa_names (tree old_name, gimple *stmt, >> ipa_parm_adjustment_vec adjustments) >> { >> struct ipa_parm_adjustment *adj; >> - tree lhs, decl, repl, name; >> - >> - if (gimple_code (stmt) =3D=3D GIMPLE_PHI) >> - lhs =3D gimple_phi_result (stmt); >> - else if (is_gimple_assign (stmt)) >> - lhs =3D gimple_assign_lhs (stmt); >> - else if (is_gimple_call (stmt)) >> - lhs =3D gimple_call_lhs (stmt); >> - else >> - gcc_unreachable (); >> + tree decl, repl, new_name; >> >> - if (TREE_CODE (lhs) !=3D SSA_NAME) >> - return false; >> + if (TREE_CODE (old_name) !=3D SSA_NAME) >> + return NULL; >> >> - decl =3D SSA_NAME_VAR (lhs); >> + decl =3D SSA_NAME_VAR (old_name); >> if (decl =3D=3D NULL_TREE >> || TREE_CODE (decl) !=3D PARM_DECL) >> - return false; >> + return NULL; >> >> adj =3D get_adjustment_for_base (adjustments, decl); >> if (!adj) >> - return false; >> + return NULL; >> >> repl =3D get_replaced_param_substitute (adj); >> - name =3D make_ssa_name (repl, stmt); >> + new_name =3D make_ssa_name (repl, stmt); >> >> if (dump_file) >> { >> fprintf (dump_file, "replacing an SSA name of a removed param "); >> - print_generic_expr (dump_file, lhs, 0); >> + print_generic_expr (dump_file, old_name, 0); >> fprintf (dump_file, " with "); >> - print_generic_expr (dump_file, name, 0); >> + print_generic_expr (dump_file, new_name, 0); >> fprintf (dump_file, "\n"); >> } >> >> - if (is_gimple_assign (stmt)) >> - gimple_assign_set_lhs (stmt, name); >> - else if (is_gimple_call (stmt)) >> - gimple_call_set_lhs (stmt, name); >> - else >> - gimple_phi_set_result (as_a (stmt), name); >> - >> - replace_uses_by (lhs, name); >> - release_ssa_name (lhs); >> - return true; >> + replace_uses_by (old_name, new_name); >> + return new_name; >> } >> >> /* If the statement STMT contains any expressions that need to replaced= with a >> @@ -4745,7 +4729,16 @@ ipa_sra_modify_function_body (ipa_parm_adjustment= _vec adjustments) >> gimple_stmt_iterator gsi; >> >> for (gsi =3D gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gs= i)) >> - replace_removed_params_ssa_names (gsi_stmt (gsi), adjustments); >> + { >> + gphi *phi =3D as_a (gsi_stmt (gsi)); >> + tree new_lhs, old_lhs =3D gimple_phi_result (phi); >> + new_lhs =3D replace_removed_params_ssa_names (old_lhs, phi, adju= stments); >> + if (new_lhs) >> + { >> + gimple_phi_set_result (phi, new_lhs); >> + release_ssa_name (old_lhs); >> + } >> + } >> >> gsi =3D gsi_start_bb (bb); >> while (!gsi_end_p (gsi)) >> @@ -4765,7 +4758,6 @@ ipa_sra_modify_function_body (ipa_parm_adjustment_= vec adjustments) >> >> case GIMPLE_ASSIGN: >> modified |=3D sra_ipa_modify_assign (stmt, &gsi, adjustments= ); >> - modified |=3D replace_removed_params_ssa_names (stmt, adjust= ments); >> break; >> >> case GIMPLE_CALL: >> @@ -4780,8 +4772,6 @@ ipa_sra_modify_function_body (ipa_parm_adjustment_= vec adjustments) >> { >> t =3D gimple_call_lhs_ptr (stmt); >> modified |=3D ipa_modify_expr (t, false, adjustments); >> - modified |=3D replace_removed_params_ssa_names (stmt, >> - adjustment= s); >> } >> break; >> >> @@ -4805,6 +4795,20 @@ ipa_sra_modify_function_body (ipa_parm_adjustment= _vec adjustments) >> break; >> } >> >> + def_operand_p defp; >> + ssa_op_iter iter; >> + FOR_EACH_SSA_DEF_OPERAND (defp, stmt, iter, SSA_OP_DEF) >> + { >> + tree old_def =3D DEF_FROM_PTR (defp); >> + if (tree new_def =3D replace_removed_params_ssa_names (old_d= ef, stmt, >> + adjustm= ents)) >> + { >> + SET_DEF (defp, new_def); >> + release_ssa_name (old_def); >> + modified =3D true; >> + } >> + } >> + >> if (modified) >> { >> update_stmt (stmt); >> >> > > -- > Richard Biener > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HR= B 21284 (AG Nuernberg)