From patchwork Tue Sep 6 09:33:24 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kugan Vivekanandarajah X-Patchwork-Id: 75483 Delivered-To: patch@linaro.org Received: by 10.140.106.11 with SMTP id d11csp451489qgf; Tue, 6 Sep 2016 02:33:53 -0700 (PDT) X-Received: by 10.66.43.164 with SMTP id x4mr70212266pal.11.1473154433406; Tue, 06 Sep 2016 02:33:53 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 75si25467929pfv.149.2016.09.06.02.33.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Sep 2016 02:33:53 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-435309-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-435309-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-435309-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=Rr5/KJuH9b4ABtO QANhCpyld9t2KxPkULxR/FCC37eM5BqFQP260UU1bM0qaJ9tb9L6fwiQHxl1ArpZ Yp5C6evT9gea/fIEObqdR/Dz6I9xUcWXtoDfaSHEzgq7fPNEhz7EFsSNmAHU58kd Rhmg9rgIHCP5VggB5MX/w9rti0rw= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=8mfDEv5PRJgrcY7Leg3ID dTzKI4=; b=rncJi4zZViofQncjDVyvgejfsEG3PYOJNM/dKR23jXxDLxedolnfE uRYEHkBeWBoO2x/C15AbXNa17EXI5xJlrHy5UCtkfGb6m8pCYibiPLu4EL/7fJhN bq9lFWz4MK2+if3yjIQAFky0D8gYIk+cCED+APq7v1i2jLqFK8QnJY= Received: (qmail 118979 invoked by alias); 6 Sep 2016 09:33:38 -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 118970 invoked by uid 89); 6 Sep 2016 09:33:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.1 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=skill, ssa_name, var_map, kuganv@linaro.org X-HELO: mail-qk0-f169.google.com Received: from mail-qk0-f169.google.com (HELO mail-qk0-f169.google.com) (209.85.220.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Sep 2016 09:33:27 +0000 Received: by mail-qk0-f169.google.com with SMTP id l2so211584965qkf.3 for ; Tue, 06 Sep 2016 02:33:27 -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:from:date :message-id:subject:to:cc; bh=OfZjaHSJ+Axl1M4KCZSSpFdUVrgNP+8+Q+cDTILn5vM=; b=YRYu7fE/PouWWr2qHWvq8sure4ZFlreEJu5nL7wsK0QXUBGxzYiGmfYYXKWCJ+ywdG DcktlZOAnmPdwjZXLLL3m+aO2kbOdVntyH2Co4/uP64IOtnPFNFZ6yp7HAlLdcA3L4tl qYvZoLqY+Nxn4ZlESR6wen8/d0G+0eHhebK3IKOjmizUZ5BdALSROlvB+SM7S+BG9jwi Jv0MwPwPtGeb1MhVU93zm09I106UezeWz1Swyfhzzhr4OKhtI934Cz6Qrz3al+Uu7tlm c8YChcnmD1ALXMr3lk4e/rkNn1ee0UgVCGRP1lT5+NGOJvvZZh/0Ev/xCt/MxF5Db4uO Gy+g== X-Gm-Message-State: AE9vXwMLz3T7run8Zvb9nezY0i92pUKVqYtliBoo+5p2X7SICV+FiViJMdoWpJXPK5CcLRKlFUzJPgxrUVAoZJka X-Received: by 10.55.200.201 with SMTP id t70mr29275862qkl.238.1473154405492; Tue, 06 Sep 2016 02:33:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.57.97 with HTTP; Tue, 6 Sep 2016 02:33:24 -0700 (PDT) In-Reply-To: References: From: Kugan Vivekanandarajah Date: Tue, 6 Sep 2016 19:33:24 +1000 Message-ID: Subject: Re: [RFC][SSA] Iterator to visit SSA To: Richard Biener Cc: "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes Hi Richard, On 6 September 2016 at 19:08, Richard Biener wrote: > On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah > wrote: >> Hi Richard, >> >> On 5 September 2016 at 17:57, Richard Biener wrote: >>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah >>> wrote: >>>> Hi All, >>>> >>>> While looking at gcc source, I noticed that we are iterating over SSA >>>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names >>>> in others. It seems that variable 0 is always NULL TREE. >>> >>> Yeah, that's artificial because we don't assign SSA name version 0 (for some >>> unknown reason). >>> >>>> But it can >>>> confuse people who are looking for the first time. Therefore It might >>>> be good to follow some consistent usage here. >>>> >>>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in >>>> other case. Here is attempt to do this based on what is done in other >>>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no >>>> new regressions. is this OK? >>> >>> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better >>> as SSAVAR might be confused with iterating over SSA_NAME_VAR. >>> >>> Then, if you add an iterator why leave the name == NULL handling to consumers? >>> That looks odd. >>> >>> Then, SSA names are just in a vector thus why not re-use the vector iterator? >>> >>> That is, >>> >>> #define FOR_EACH_SSA_NAME (name) \ >>> for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) >>> >>> would be equivalent to your patch? >>> >>> Please also don't add new iterators that implicitely use 'cfun' but always use >>> one that passes it as explicit arg. >> >> I think defining FOR_EACH_SSA_NAME with vector iterator is better. But >> we will not be able to skill NULL ssa_names with that. > > Why? Can't you simply do > > #define FOR_EACH_SSA_NAME (name) \ > for (unsigned i = 1; SSANAMES (cfun).iterate (i, &name); ++i) \ > if (name) > > ? For example, with the test patch where loop body also defines i which is giving error: ../../trunk/gcc/tree-ssa-ter.c: In function ‘temp_expr_table* new_temp_expr_table(var_map)’: ../../trunk/gcc/tree-ssa-ter.c:206:11: error: redeclaration of ‘int i’ int i; ^ In file included from ../../trunk/gcc/ssa.h:29:0, from ../../trunk/gcc/tree-ssa-ter.c:28: ../../trunk/gcc/tree-ssanames.h:66:17: note: ‘unsigned int i’ previously declared here for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i) ^ ../../trunk/gcc/tree-ssa-ter.c:204:3: note: in expansion of macro ‘FOR_EACH_SSA_NAME’ FOR_EACH_SSA_NAME (name) ^ Makefile:1097: recipe for target 'tree-ssa-ter.o' failed Am I missing something here ? Thanks, Kugan > >> I also added >> index variable to the macro so that there want be any conflicts if the >> index variable "i" (or whatever) is also defined in the loop. >> >> Bootstrapped and regression tested on x86_64-linux-gnu with no new >> regressions. Is this OK for trunk? >> >> Thanks, >> Kugan >> >> >> gcc/ChangeLog: >> >> 2016-09-06 Kugan Vivekanandarajah >> >> * tree-ssanames.h (FOR_EACH_SSA_NAME): New. >> * cfgexpand.c (update_alias_info_with_stack_vars): Use >> FOR_EACH_SSA_NAME to iterate over SSA variables. >> (pass_expand::execute): Likewise. >> * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise. >> * tree-cfg.c (dump_function_to_file): Likewise. >> * tree-into-ssa.c (pass_build_ssa::execute): Likewise. >> (update_ssa): Likewise. >> * tree-ssa-alias.c (dump_alias_info): Likewise. >> * tree-ssa-ccp.c (ccp_finalize): Likewise. >> * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise. >> (create_outofssa_var_map): Likewise. >> (coalesce_ssa_name): Likewise. >> * tree-ssa-operands.c (dump_immediate_uses): Likewise. >> * tree-ssa-pre.c (compute_avail): Likewise. >> * tree-ssa-sccvn.c (init_scc_vn): Likewise. >> (scc_vn_restore_ssa_info): Likewise. >> (free_scc_vn): Likwise. >> (run_scc_vn): Likewise. >> * tree-ssa-structalias.c (compute_points_to_sets): Likewise. >> * tree-ssa-ter.c (new_temp_expr_table): Likewise. >> * tree-ssa-copy.c (fini_copy_prop): Likewise. >> * tree-ssa.c (verify_ssa): Likewise. >> >>> >>> Thanks, >>> Richard. >>> >>> >>>> Thanks, >>>> Kugan >>>> >>>> >>>> gcc/ChangeLog: >>>> >>>> 2016-09-05 Kugan Vivekanandarajah >>>> >>>> * tree-ssanames.h (ssa_iterator::ssa_iterator): New. >>>> (ssa_iterator::get): Likewise. >>>> (ssa_iterator::next): Likewise. >>>> (FOR_EACH_SSAVAR): Likewise. >>>> * cfgexpand.c (update_alias_info_with_stack_vars): Use >>>> FOR_EACH_SSAVAR to iterate over SSA variables. >>>> (pass_expand::execute): Likewise. >>>> * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise. >>>> * tree-cfg.c (dump_function_to_file): Likewise. >>>> * tree-into-ssa.c (pass_build_ssa::execute): Likewise. >>>> (update_ssa): Likewise. >>>> * tree-ssa-alias.c (dump_alias_info): Likewise. >>>> * tree-ssa-ccp.c (ccp_finalize): Likewise. >>>> * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise. >>>> (create_outofssa_var_map): Likewise. >>>> (coalesce_ssa_name): Likewise. >>>> * tree-ssa-operands.c (dump_immediate_uses): Likewise. >>>> * tree-ssa-pre.c (compute_avail): Likewise. >>>> * tree-ssa-sccvn.c (init_scc_vn): Likewise. >>>> (scc_vn_restore_ssa_info): Likewise. >>>> (free_scc_vn): Likwise. >>>> (run_scc_vn): Likewise. >>>> * tree-ssa-structalias.c (compute_points_to_sets): Likewise. >>>> * tree-ssa-ter.c (new_temp_expr_table): Likewise. diff --git a/gcc/tree-ssa-ter.c b/gcc/tree-ssa-ter.c index 2a772b2..15f30ee 100644 --- a/gcc/tree-ssa-ter.c +++ b/gcc/tree-ssa-ter.c @@ -185,7 +185,7 @@ extern void debug_ter (FILE *, temp_expr_table *); static temp_expr_table * new_temp_expr_table (var_map map) { - unsigned x; + tree name; temp_expr_table *t = XNEW (struct temp_expr_table); t->map = map; @@ -201,15 +201,14 @@ new_temp_expr_table (var_map map) t->replaceable_expressions = NULL; t->num_in_part = XCNEWVEC (int, num_var_partitions (map)); - for (x = 1; x < num_ssa_names; x++) + FOR_EACH_SSA_NAME (name) { - int p; - tree name = ssa_name (x); + int i; if (!name) continue; - p = var_to_partition (map, name); - if (p != NO_PARTITION) - t->num_in_part[p]++; + i = var_to_partition (map, name); + if (i != NO_PARTITION) + t->num_in_part[i]++; } t->call_cnt = XCNEWVEC (int, num_ssa_names + 1); diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h index 8e66ce6..538e35f 100644 --- a/gcc/tree-ssanames.h +++ b/gcc/tree-ssanames.h @@ -62,6 +62,9 @@ struct GTY ((variable_size)) range_info_def { #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names)) #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)]) +#define FOR_EACH_SSA_NAME(VAR) \ + for (unsigned i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i) + /* Sets the value range to SSA. */ extern void set_range_info (tree, enum value_range_type, const wide_int_ref &, const wide_int_ref &);