This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]