Hi Richard,

Here is an attempt to address your earlier review comments. Bootstrapped 
and there is no new regression for X86_64 and arm. Thank you very much 
for your time.

Thanks,
Kugan

--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,25 @@
+2013-08-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* tree-flow.h (mark_range_info_unknown): New function definition.
+	* tree-ssa-alias.c (dump_alias_info) : Check pointer type.
+	* tree-ssa-copy.c (fini_copy_prop) : Check pointer type and copy
+	range info.
+	* tree-ssanames.c (make_ssa_name_fn) : Check pointer type in
+	initialize.
+	* (mark_range_info_unknown) : New function.
+	* (duplicate_ssa_name_range_info) : Likewise.
+	* (duplicate_ssa_name_fn) : Check pointer type and call correct
+	duplicate function.
+	* tree-vrp.c (extract_exp_value_range): New function.
+	* (simplify_stmt_using_ranges): Call extract_exp_value_range and
+	tree_ssa_set_value_range.
+	* tree-ssaname.c (ssa_range_info): New function.
+	* tree.h (SSA_NAME_PTR_INFO) : changed to access via union
+	* tree.h (SSA_NAME_RANGE_INFO) : New macro
+	* gimple-pretty-print.c (print_double_int) : New function.
+	* gimple-pretty-print.c (dump_gimple_phi) : Dump range info.
+	* (pp_gimple_stmt_1) : Likewise.
+
   2013-08-09  Jan Hubicka  <jh@suse.cz>

   	* cgraph.c (cgraph_create_edge_1): Clear speculative flag.

On 03/07/13 21:55, Kugan wrote:
> On 17/06/13 18:33, Richard Biener wrote:
>> On Mon, 17 Jun 2013, Kugan wrote:
>> +/* Extract the value range of assigned exprassion for GIMPLE_ASSIGN
>> stmt.
>> +   If the extracted value range is valid, return true else return
>> +   false.  */
>> +static bool
>> +extract_exp_value_range (gimple stmt, value_range_t *vr)
>> +{
>> +  gcc_assert (is_gimple_assign (stmt));
>> +  tree rhs1 = gimple_assign_rhs1 (stmt);
>> +  tree lhs = gimple_assign_lhs (stmt);
>> +  enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
>> ...
>> @@ -8960,6 +9016,23 @@ simplify_stmt_using_ranges (gimple_stmt_iterator
>> *gsi)
>>       {
>>         enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
>>         tree rhs1 = gimple_assign_rhs1 (stmt);
>> +      tree lhs = gimple_assign_lhs (stmt);
>> +
>> +      /* Set value range information for ssa.  */
>> +      if (!POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
>> +          && (TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME)
>> +          && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt)))
>> +          && !SSA_NAME_RANGE_INFO (lhs))
>> +        {
>> +          value_range_t vr = VR_INITIALIZER;
>> ...
>> +          if (extract_exp_value_range (stmt, &vr))
>> +            tree_ssa_set_value_range (lhs,
>> +                                      tree_to_double_int (vr.min),
>> +                                      tree_to_double_int (vr.max),
>> +                                      vr.type == VR_RANGE);
>> +        }
>>
>> This looks overly complicated to me.  In vrp_finalize you can simply do
>>
>>    for (i = 0; i < num_vr_values; i++)
>>      if (vr_value[i])
>>        {
>>          tree name = ssa_name (i);
>>          if (POINTER_TYPE_P (name))
>>            continue;
>>          if (vr_value[i].type == VR_RANGE
>>              || vr_value[i].type == VR_ANTI_RANGE)
>>            tree_ssa_set_value_range (name, tree_to_double_int
>> (vr_value[i].min), tree_to_double_int (vr_value[i].max), vr_value[i].type
>> == VR_RANGE);
>>        }
>>
>
> Thanks Richard for taking time to review it.
>
> I was doing something like what you are suggesting earlier but noticed
> some problems and that’s the reason why I changed.
>
> For example, for the following testcase from the test suite,
>
> unsigned long l = (unsigned long)-2;
> unsigned short s;
>
> int main () {
>    long t = l + 1;
>    s = l;
>    if (s != (unsigned short) -2)
>      abort ();
>    exit (0);
> }
>
> with the following gimple stmts
>
> main ()
> {
>    short unsigned int s.1;
>    long unsigned int l.0;
>
> ;;   basic block 2, loop depth 0
> ;;    pred:       ENTRY
>    l.0_2 = l;
>    s.1_3 = (short unsigned int) l.0_2;
>    s = s.1_3;
>    if (s.1_3 != 65534)
>      goto <bb 3>;
>    else
>      goto <bb 4>;
> ;;    succ:       3
> ;;                4
>
> ;;   basic block 3, loop depth 0
> ;;    pred:       2
>    abort ();
> ;;    succ:
>
> ;;   basic block 4, loop depth 0
> ;;    pred:       2
>    exit (0);
> ;;    succ:
>
> }
>
>
>
> has the following value range.
>
> l.0_2: VARYING
> s.1_3: [0, +INF]
>
>
>  From zero/sign extension point of view, the variable s.1_3 is expected
> to have a value that will overflow (or varying) as this is what is
> assigned to a smaller variable. extract_range_from_assignment initially
> calculates the value range as VARYING but later changed to [0, +INF] by
> extract_range_basic. What I need here is the value that will be assigned
> from the rhs expression and not the value that we will have with proper
> assignment.
>
> I understand that the above code of mine needs to be changed but not
> convinced about the best way to do that.
>
> I can possibly re-factor extract_range_from_assignment to give me this
> information with an additional argument. Could you kindly let me know
> your preference.
>

>>
>> /* SSA name annotations.  */
>>
>> +  union vrp_info_type {
>> +    /* Pointer attributes used for alias analysis.  */
>> +    struct GTY ((tag ("TREE_SSA_PTR_INFO"))) ptr_info_def *ptr_info;
>> +    /* Value range attributes used for zero/sign extension elimination.
>> */
>>
>> /* Value range information.  */
>>
>> +    struct GTY ((tag ("TREE_SSA_RANGE_INFO"))) range_info_def
>> *range_info;
>> +  } GTY ((desc ("%1.def_stmt && !POINTER_TYPE_P (TREE_TYPE
>> ((tree)&%1))"))) vrp;
>>
>> why do you need to test %1.def_stmt here?
>
>
> I have seen some tree_ssa_name with def_stmt NULL. Thats why I added
> this. Is that something that should never happen.
>
>
> Thanks,
> Kugan