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.vivekanandarajah@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?

Thanks,
Kugan