----------------------------------------
> Date: Wed, 18 Mar 2015 22:01:18 +0530
> Subject: Re: Proposal for adding splay_tree_find (to find elements without
> updating the nodes).
> From: prathamesh.kulka...@linaro.org
> To: hiradi...@msn.com
> CC: richard.guent...@gmail.com; tbsau...@tbsaunde.org; gcc@gcc.gnu.org
>
> On 18 March 2015 at 21:20, Aditya K <hiradi...@msn.com> wrote:
>>
>>
>> ----------------------------------------
>>> Date: Wed, 18 Mar 2015 11:50:16 +0100
>>> Subject: Re: Proposal for adding splay_tree_find (to find elements without
>>> updating the nodes).
>>> From: richard.guent...@gmail.com
>>> To: hiradi...@msn.com
>>> CC: tbsau...@tbsaunde.org; gcc@gcc.gnu.org
>>>
>>> On Mon, Mar 16, 2015 at 4:33 AM, Aditya K <hiradi...@msn.com> wrote:
>>>>
>>>>
>>>> ----------------------------------------
>>>>> Date: Sun, 15 Mar 2015 02:32:23 -0400
>>>>> From: tbsau...@tbsaunde.org
>>>>> To: gcc@gcc.gnu.org
>>>>> Subject: Re: Proposal for adding splay_tree_find (to find elements
>>>>> without updating the nodes).
>>>>>
>>>>> hi,
>>>>>
>>>>> I'm only commenting on algorithmic stuff at this point, you should make
>>>>> sure this doesn't regress anything in make check. This stuff only
>>>>> effects code using omp stuff so compiling random c++ is unlikely to test
>>>>> this code at all.
>>>>>
>>>>> Also please follow the style in
>>>>> https://gcc.gnu.org/codingconventions.html
>>>>> and usually try to make new code similar to what's around it.
>>>>>
>>>>> @@ -384,7 +386,7 @@ new_omp_context (enum omp_region_type region_type)
>>>>>
>>>>> c = XCNEW (struct gimplify_omp_ctx);
>>>>> c->outer_context = gimplify_omp_ctxp;
>>>>> - c->variables = splay_tree_new (splay_tree_compare_decl_uid, 0, 0);
>>>>> + //c->variables = splay_tree_new (splay_tree_compare_decl_uid, 0, 0);
>>>>>
>>>>> I don't think this is what you want, xcnew is a calloc wrapper and
>>>>> doesn't call the ctor for gimplify_omp_ctx. For now placement new is
>>>>> probably the simplest way to get what you want.
>>>>>
>>>> Thanks for pointing this out. I'll do it the way c->privatized_types has
>>>> been allocated.
>>>> e.g., by making c->variables a pointer to std::map and c->variables = new
>>>> gimplify_tree_t;
>>>>
>>>>
>>>>> -static void
>>>>> -delete_omp_context (struct gimplify_omp_ctx *c)
>>>>> -{
>>>>> - splay_tree_delete (c->variables);
>>>>> - delete c->privatized_types;
>>>>> - XDELETE (c);
>>>>> -}
>>>>>
>>>>> hm, why?
>>>>>
>>>> My bad, I'll restore this.
>>>>
>>>>> -gimplify_adjust_omp_clauses_1 (splay_tree_node n, void *data)
>>>>> +gimplify_adjust_omp_clauses_1 (std::pair<tree, unsigned> n, void *data)
>>>>>
>>>>> You can now change the type of data from void * to const
>>>>> gimplify_adjust_omp_clauses_data *
>>>>
>>>> Done!
>>>>
>>>>
>>>> Thanks for the feedback, they were really helpful. I have updated the
>>>> patch. Please review this.
>>>> Also, although I run `make check` while compiling gcc (with bootstrap
>>>> enabled), I'm not sure if 'omp' related tests were exercised.
>>>> I'm still unfamiliar with several components of gcc. Any pointers on how
>>>> to ensure all tests were run, would be useful.
>>>
>>> I'm not sure we want to use std::map. Can you use GCCs own hash_map
>>> here?
>>
>> Ok, I'll try to use has_map. I was under the impression that we can use
>> standard library features, that's why I used std::map.
>>
> Using std::map caused a bootstrap build problem on AIX.
> see: https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02608.html
> However I am not sure if that's true any more after the following fix
> was commmited:
> https://gcc.gnu.org/ml/libstdc++/2014-10/msg00195.html
>
Thanks for letting me know.
-Aditya
> Regards,
> Prathamesh
>> Thanks,
>> -Aditya
>>
>>>
>>> Richard.
>>>
>>>>
>>>> -Aditya
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> thanks!
>>>>>
>>>>> Trev
>>>>>
>>>>> On Fri, Mar 13, 2015 at 07:32:03PM +0000, Aditya K wrote:
>>>>>> You're right. I'll change this to:
>>>>>>
>>>>>> /* A stable comparison functor to sort trees. */
>>>>>> struct tree_compare_decl_uid {
>>>>>> bool operator ()(const tree &xa, const tree &xb) const
>>>>>> {
>>>>>> return DECL_UID (xa) < DECL_UID (xb);
>>>>>> }
>>>>>> };
>>>>>>
>>>>>> New patch attached.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> -Aditya
>>>>>>
>>>>>>
>>>>>> ----------------------------------------
>>>>>>> Date: Fri, 13 Mar 2015 19:02:11 +0000
>>>>>>> Subject: Re: Proposal for adding splay_tree_find (to find elements
>>>>>>> without updating the nodes).
>>>>>>> From: jwakely....@gmail.com
>>>>>>> To: hiradi...@msn.com
>>>>>>> CC: richard.guent...@gmail.com; stevenb....@gmail.com; gcc@gcc.gnu.org
>>>>>>>
>>>>>>> Are you sure your compare_variables functor is correct?
>>>>>>>
>>>>>>> Subtracting the two values seems very strange for a strict weak
>>>>>>> ordering.
>>>>>>>
>>>>>>> (Also "compare_variables" is a pretty poor name!)
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>
