On Wed, Aug 17, 2016 at 4:50 AM, kugan <kugan.vivekanandara...@linaro.org> wrote: > Hi Richard, > > > On 17/08/16 08:20, kugan wrote: >> >> Hi, >> >> On 16/08/16 21:56, Richard Biener wrote: >>> >>> On Tue, Aug 16, 2016 at 10:09 AM, kugan >>> <kugan.vivekanandara...@linaro.org> wrote: >>>> >>>> >>>> >>>> >>>> On 23/07/16 20:12, kugan wrote: >>>>> >>>>> >>>>> Hi Richard, >>>>> >>>>>>> As we had value_range_type in tree-ssanames.h why not put value_range >>>>>>> there? >>>>>>> >>>>>> For IPA_VRP, we now need value_range used in ipa-prop.h (in ipa-vrp >>>>>> patch). Based on this, attached patch now adds struct value_range to >>>>>> tree-ssanames.h and fixes the header files. Please note that I also >>>>>> had >>>>>> to add other headers in few places due to the dependency. Are you OK >>>>>> with this ? >>>>> >>>>> >>>>> Here is alternate patch where we keep struct value_range and enum >>>>> value_range_type to tree-vrp.h. May be it is a better approach? Please >>>>> let me know what is your preference. >>>>> >>>> >>>> Ping? >>>> >>>> This patch places value_range_type and value_range in tree-vrp.h. May be >>>> this is better? >>>> >>>> Alternate patch which keeps value_range_type and value_range in >>>> tree-ssanames.h is in: >>>> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01491.html >>>> >>>> I also added the necessary header files changed needed for ipa-vrp as >>>> part >>>> of this patch so that changes needed are clear. >>> >>> >>> I think tree-vrp.h is a better place. Please don't export functions >>> you don't need >>> (the _1 helpers). > > I had unintentionally removed a static from a _1 helper. I think thats what > caused the confusion. I also added constant modifiers to parameters mainly > due to ipa-vrp passing second parameters to lattice operation as const. > >> Agreed. >> >> I have exported the following for now: >> +extern void vrp_intersect_ranges (value_range *vr0, value_range *vr1); >> +extern void vrp_meet (value_range *vr0, const value_range *vr1); >> +extern void dump_value_range (FILE *, const value_range *); > > > This again is the exported operations. > >> It might be useful to add vrp_unary_op, vrp_binary_op on value_range. >> But that is for later if that is needed. >> >>> >>> I still believe sharing vrp_initialize/finalize is wrong and the >>> lattice setup / teardown >>> should be split out. >> >> >> I have done that too as part of the early-vrp patch in: >> >> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01155.html >> >> I just wanted to focus on the functionality required for the IPA-VRP on >> this patch. > > > > Attaching the latest version of the patch. Is this OK?
Yes. Thanks, Richard. > Thanks, > Kugan