----------------------------------------
> 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.
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!)
>>>>
>>>
>>>
>>
>>
>>
