So I have modified the patch to use hash_map instead of std::map. The patch is attached.
However, I got one regression after that. # Comparing directories ## Dir1=../build-pristine/: 11 sum files ## Dir2=../build-test/: 11 sum files # Comparing 11 common sum files ## /bin/sh ../contrib/compare_tests /tmp/gxx-sum1.29214 /tmp/gxx-sum2.29214 Tests that now fail, but worked before: c-c++-common/goacc/loop-private-1.c -std=c++98 scan-tree-dump-times gimple "#pragma acc loop collapse\\(2\\) private\\(j\\) private\\(i\\)" 1 c-c++-common/goacc/loop-private-1.c scan-tree-dump-times gimple "#pragma acc loop collapse\\(2\\) private\\(j\\) private\\(i\\)" 1 ## Differences found: # 1 differences in 11 common sum files found The error is due to mis-comparison because of reordering of private(i) private(j). I wanted to know if the order of the private flags matter. Thanks, -Aditya ---------------------------------------- > From: hiradi...@msn.com > To: prathamesh.kulka...@linaro.org > CC: richard.guent...@gmail.com; tbsau...@tbsaunde.org; gcc@gcc.gnu.org > Subject: RE: Proposal for adding splay_tree_find (to find elements without > updating the nodes). > Date: Wed, 18 Mar 2015 18:14:49 +0000 > > > > ---------------------------------------- >> 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!) >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>> >
splay_tree.patch
Description: Binary data
