[ping][PATCH][1 of 2] Add value range info to SSA_NAME for zero sign extension elimination in RTL

Kugan kugan.vivekanandarajah@linaro.org
Mon Sep 2 09:31:00 GMT 2013


I'd like to ping this  patch 1 of 2 that removes redundant zero/sign 
extension using value range information.

Bootstrapped and no new regression for  x86_64-unknown-linux-gnu and 
arm-none-linux-gnueabi.

Thanks you for your time.
Kugan

n 14/08/13 16:49, Kugan wrote:
> 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
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: vrp_extension_elimination_patch1_r2.diff
Type: text/x-patch
Size: 13549 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20130902/0762f51f/attachment.bin>


More information about the Gcc-patches mailing list