On Thu, Sep 8, 2016 at 5:29 AM, Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> wrote: > Hi Richard, > > On 7 September 2016 at 19:35, Richard Biener <richard.guent...@gmail.com> > wrote: >> On Wed, Sep 7, 2016 at 2:21 AM, Kugan Vivekanandarajah >> <kugan.vivekanandara...@linaro.org> wrote: >>> Hi Richard, >>> >>> On 6 September 2016 at 19:08, Richard Biener <richard.guent...@gmail.com> >>> wrote: >>>> On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah >>>> <kugan.vivekanandara...@linaro.org> wrote: >>>>> Hi Richard, >>>>> >>>>> On 5 September 2016 at 17:57, Richard Biener <richard.guent...@gmail.com> >>>>> wrote: >>>>>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah >>>>>> <kugan.vivekanandara...@linaro.org> 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) >>>> >>>> ? >>> >>> Indeed. I missed the if at the end :(. Here is an updated patch. >>> Bootstrapped and regression tested on x86_64-linux-gnu with no new >>> regressions. >> >> Quoting myself: >> >>> As I said >>> I'd like 'cfun' to be explicit, thus >>> >>> #define FOR_EACH_SSA_NAME(I, VAR, FN) \ >>> for (I = 1; SSANAMES (FN)->iterate ((I), &(VAR)); ++(I)) \ >>> if (VAR) >> >> the patch doesn't seem to contain that FN part. Please also put declarations >> of 'name' you need to add right before the FOR_EACH_SSA_NAME rather >> than far away. > > Please find the attached patch does this. Bootstrapped and regression > tested on x86_64-linux-gnu with no new regressions. > > Is this OK?
Ok. Thanks, Richard. > Thanks, > Kugan > >> >> Thanks, >> Richard. >> >>> 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 <kug...@linaro.org> >>>>> >>>>> * 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 <kug...@linaro.org> >>>>>>> >>>>>>> * 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.
