public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* FDO usability -- sanity check indirect call target
@ 2011-04-08  6:06 Xinliang David Li
  2011-04-08  9:27 ` Richard Guenther
  0 siblings, 1 reply; 6+ messages in thread
From: Xinliang David Li @ 2011-04-08  6:06 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

Hi, due to race conditions, it is common that the value profile
information for an indirect call site is 'corrupted' -- resulting in
false target function to be recorded. The value profile transformation
won't cause runtime problem as the path will never be executed,
however it may cause compile time ICE because of the incompatible
signature of the callee target. The attached patch does minimal sanity
check to make compiler happy in such cases. The fix was tested with
lots of MT programs and works really well in practice.

Ok for trunk after testing?

Thanks,

David

2011-04-07  Xinliang David Li  <davidxl@google.com>

        * value-profile.c (function_decl_num_args): New function.
        (check_ic_target): New function.
        (gimple_ic_transform): Sanity check indirect call target.

[-- Attachment #2: check_ic.p --]
[-- Type: application/octet-stream, Size: 2552 bytes --]

Index: value-prof.c
===================================================================
--- value-prof.c	(revision 171959)
+++ value-prof.c	(working copy)
@@ -1090,6 +1090,62 @@ find_func_by_pid (int	pid)
   return pid_map [pid];
 }
 
+/* Counts the number of arguments in the function_decl FN_DECL.
+
+   Returns the number of arguments.  */
+static unsigned
+function_decl_num_args (tree fn_decl)
+{
+  tree arg;
+  unsigned count = 0;
+  for (arg = DECL_ARGUMENTS (fn_decl); arg; arg = TREE_CHAIN (arg))
+    count++;
+  return count;
+}
+
+/* Perform sanity check on the indirect call target. Due to race conditions,
+   false function target may be attributed to an indirect call site. If the
+   call expression type mismatches with the target function's type, expand_call
+   may ICE. Here we only do very minimal sanity check just to make compiler happy.
+   Returns true if TARGET is considered ok for call CALL_STMT.  */
+
+static bool
+check_ic_target (gimple call_stmt, struct cgraph_node *target)
+{
+  tree tgt_decl;
+  /* Return types.  */
+  tree icall_type, tgt_type; 
+  tree call_expr;
+
+  tgt_decl = target->decl;
+  tgt_type = TREE_TYPE (TREE_TYPE (tgt_decl));
+
+  call_expr = gimple_call_fn (call_stmt);
+  icall_type = TREE_TYPE (call_expr);
+  if (POINTER_TYPE_P (icall_type))
+    icall_type = TREE_TYPE (icall_type);
+  icall_type = TREE_TYPE (icall_type);
+
+  if (TREE_CODE (icall_type) != TREE_CODE (tgt_type)
+      || TYPE_MODE (icall_type) != TYPE_MODE (tgt_type))
+    return false;
+
+  /* Verify that CALL_STMT has the same number of arguments as TGT_TYPE. */
+  if (gimple_call_num_args (call_stmt) != function_decl_num_args (tgt_decl))
+    return false;
+
+  /* Record types should have matching sizes. */
+  if (TREE_CODE (icall_type) == RECORD_TYPE
+      && TYPE_SIZE (icall_type) != NULL_TREE
+      && TYPE_SIZE (tgt_type) != NULL_TREE
+      && TREE_CODE (TYPE_SIZE (icall_type)) == INTEGER_CST
+      && (TREE_CODE (TYPE_SIZE (tgt_type)) != INTEGER_CST ||
+	  !tree_int_cst_equal (TYPE_SIZE (icall_type), TYPE_SIZE (tgt_type))))
+    return false;
+
+  return true;
+}
+
 /* Do transformation
 
   if (actual_callee_address == address_of_most_common_function/method)
@@ -1271,6 +1327,9 @@ gimple_ic_transform (gimple stmt)
   if (direct_call == NULL)
     return false;
 
+  if (!check_ic_target (stmt, direct_call))
+    return false;
+
   modify = gimple_ic (stmt, direct_call, prob, count, all);
 
   if (dump_file)
@@ -1751,4 +1810,3 @@ gimple_find_values_to_profile (histogram
         }
     }
 }
-

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-11  9:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-08  6:06 FDO usability -- sanity check indirect call target Xinliang David Li
2011-04-08  9:27 ` Richard Guenther
2011-04-08 21:42   ` Xinliang David Li
2011-04-09 16:35     ` Xinliang David Li
2011-04-11  6:06       ` Xinliang David Li
2011-04-11  9:09     ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).