On Tue, Sep 6, 2016 at 12:07 PM, Kugan Vivekanandarajah
<kugan.vivekanandara...@linaro.org> wrote:
> Hi Richard,
>
>
> On 6 September 2016 at 19:57, Richard Biener <richard.guent...@gmail.com>
> wrote:
>> On Tue, Sep 6, 2016 at 11:33 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)
>>>>
>>>> ?
>>>
>>> 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 ?
>>
>> Well, my comment was not about passing 'i' to the macro but about
>> not being able to skip NULL names. I just copied & pasted my
>> earlier macro.
>
> Sorry, I misunderstood your comment.
>
> I have:
>
> +#define FOR_EACH_SSA_NAME(i, VAR) \
> + for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i)
>
> We will skip the first (0 index) if that is what you are referring here.
>
> I also thought that you wanted to skip any NULL_TREEs after that too.
> After looking at your previous your comment, I don’t you think you
> wanted that anyway.
I _did_ want that, otherwise my suggestion to use
#define FOR_EACH_SSA_NAME(i, VAR) \
for (i = 1; SSANAMES (cfun)->iterate (i, &VAR); ++i) \
if (VAR)
would have made no sense. Btw, please use capital i and put uses
in parens when not lvalues (see FOR_EACH_VEC_ELT). 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)
Thanks,
Richard.
> Therefore I think I am doing what you wanted in my last patch.
>
> Thanks,
> Kugan
>
>
>> 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.
