This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC][SSA] Iterator to visit SSA
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Kugan Vivekanandarajah <kugan dot vivekanandarajah at linaro dot org>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 6 Sep 2016 11:08:19 +0200
- Subject: Re: [RFC][SSA] Iterator to visit SSA
- Authentication-results: sourceware.org; auth=none
- References: <CAELXzTPYD4Esiu_r-paNm9K4vu_MH1N=fWfKhoRG1G0gaG2w_Q@mail.gmail.com> <CAFiYyc1cvTTphTbQh0+h=yKtnkJa1JSTLDFm5=nLUabC2indsw@mail.gmail.com> <CAELXzTNCNF-wJu6wgWnE8u5H_13mAL9QL91px4mT+zehiMFXnw@mail.gmail.com>
On Tue, Sep 6, 2016 at 2:24 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> On 5 September 2016 at 17:57, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@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)
?
> 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 <kuganv@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 <kuganv@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.