Patchwork [google] Handle incompatible cg options more generally in LIPO (issue6476057)

login
register
mail settings
Submitter Xinliang David Li
Date Aug. 23, 2012, 6:32 p.m.
Message ID <20120823183217.B13BB1B40B3@aples.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/179702/
State New
Headers show

Comments

Xinliang David Li - Aug. 23, 2012, 6:32 p.m.
--
This patch is available for review at http://codereview.appspot.com/6476057
Xinliang David Li - Aug. 23, 2012, 9:27 p.m.
The patch is needed to handle more options that will causes invalid
LIPO module grouping. The new option handled is  -fsized-delete.

David

On Thu, Aug 23, 2012 at 11:32 AM, David Li <davidxl@google.com> wrote:
> Index: coverage.c
> ===================================================================
> --- coverage.c  (revision 190369)
> +++ coverage.c  (working copy)
> @@ -261,6 +261,56 @@ str_eq (const void *p1, const void *p2)
>    return !strcmp (s1, s2);
>  }
>
> +/* Command line option descriptor.  */
> +
> +struct opt_desc
> +{
> +  const char *opt_str;
> +  const char *opt_neg_str;
> +  bool default_val;  /* TODO better handling of default  */
> +};
> +
> +static struct opt_desc force_matching_cg_opts[] =
> +  {
> +    { "-fexceptions", "-fno-exceptions", true },
> +    { "-fsized-delete", "-fno-sized-delete", false },
> +    { NULL, NULL, false }
> +  };
> +
> +/* A helper function to check if OPTION_STRING is one of the codegen
> +   options specified in FORCE_MATCHING_CG_ARGS. If yes, set the
> +   corresponding entry in CG_ARG_VAL to the value of the option specified
> +   in OPTION_STRING.  */
> +
> +static void
> +check_cg_opts (bool *cg_opt_val, const char *option_str)
> +{
> +  unsigned int i;
> +  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
> +    {
> +      if (!strcmp (force_matching_cg_opts[i].opt_str, option_str))
> +        cg_opt_val[i] = true;
> +      else if (!strcmp (force_matching_cg_opts[i].opt_neg_str, option_str))
> +        cg_opt_val[i] = false;
> +    }
> +}
> +
> +/* A helper function to check if CG_OPTS1 and CG_OPTS are identical. It returns
> +   true if yes, false otherwise.  */
> +
> +static bool
> +has_incompatible_cg_opts (bool *cg_opts1, bool *cg_opts2, unsigned num_cg_opts)
> +{
> +  unsigned i;
> +
> +  for (i = 0; i < num_cg_opts; i++)
> +    {
> +      if (cg_opts1[i] != cg_opts2[i])
> +        return true;
> +    }
> +
> +  return false;
> +}
>
>  /* Returns true if the command-line arguments stored in the given module-infos
>     are incompatible.  */
> @@ -276,8 +326,6 @@ incompatible_cl_args (struct gcov_module
>    unsigned int num_non_warning_opts1 = 0, num_non_warning_opts2 = 0;
>    bool warning_mismatch = false;
>    bool non_warning_mismatch = false;
> -  bool with_fexceptions1 = true;
> -  bool with_fexceptions2 = true;
>    htab_t option_tab1, option_tab2;
>    unsigned int start_index1 = mod_info1->num_quote_paths +
>      mod_info1->num_bracket_paths + mod_info1->num_cpp_defines +
> @@ -286,6 +334,22 @@ incompatible_cl_args (struct gcov_module
>      mod_info2->num_bracket_paths + mod_info2->num_cpp_defines +
>      mod_info2->num_cpp_includes;
>
> +  bool *cg_opts1, *cg_opts2, has_any_incompatible_cg_opts;
> +  unsigned int num_cg_opts = 0;
> +
> +  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
> +    num_cg_opts++;
> +
> +  cg_opts1 = XCNEWVEC (bool, num_cg_opts);
> +  cg_opts2 = XCNEWVEC (bool, num_cg_opts);
> +
> +  /* Initialize the array to default value  */
> +  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
> +    {
> +      cg_opts1[i] = force_matching_cg_opts[i].default_val;
> +      cg_opts2[i] = force_matching_cg_opts[i].default_val;
> +    }
> +
>    option_tab1 = htab_create (10, str_hash, str_eq, NULL);
>    option_tab2 = htab_create (10, str_hash, str_eq, NULL);
>
> @@ -297,12 +361,9 @@ incompatible_cl_args (struct gcov_module
>      else
>        {
>          void **slot;
> -        char* option_string = mod_info1->string_array[start_index1 + i];
> +        char *option_string = mod_info1->string_array[start_index1 + i];
>
> -        if (!strcmp ("-fexceptions", option_string))
> -          with_fexceptions1 = true;
> -        else if (!strcmp ("-fno-exceptions", option_string))
> -          with_fexceptions1 = false;
> +        check_cg_opts (cg_opts1, option_string);
>
>          slot = htab_find_slot (option_tab1, option_string, INSERT);
>          if (!*slot)
> @@ -319,12 +380,10 @@ incompatible_cl_args (struct gcov_module
>      else
>        {
>          void **slot;
> -        char* option_string = mod_info2->string_array[start_index2 + i];
> +        char *option_string = mod_info2->string_array[start_index2 + i];
> +
> +        check_cg_opts (cg_opts2, option_string);
>
> -        if (!strcmp ("-fexceptions", option_string))
> -          with_fexceptions2 = true;
> -        else if (!strcmp ("-fno-exceptions", option_string))
> -          with_fexceptions2 = false;
>          slot = htab_find_slot (option_tab2, option_string, INSERT);
>          if (!*slot)
>            {
> @@ -354,7 +413,7 @@ incompatible_cl_args (struct gcov_module
>      warning (OPT_Wripa_opt_mismatch, "command line arguments mismatch for %s "
>              "and %s", mod_info1->source_filename, mod_info2->source_filename);
>
> -   if (warn_ripa_opt_mismatch && non_warning_mismatch
> +   if (warn_ripa_opt_mismatch && non_warning_mismatch
>         && (flag_opt_info >= OPT_INFO_MED))
>       {
>         inform (UNKNOWN_LOCATION, "Options for %s", mod_info1->source_filename);
> @@ -365,14 +424,19 @@ incompatible_cl_args (struct gcov_module
>           inform (UNKNOWN_LOCATION, non_warning_opts2[i]);
>       }
>
> -  XDELETEVEC (warning_opts1);
> -  XDELETEVEC (warning_opts2);
> -  XDELETEVEC (non_warning_opts1);
> -  XDELETEVEC (non_warning_opts2);
> -  htab_delete (option_tab1);
> -  htab_delete (option_tab2);
> -  return ((flag_ripa_disallow_opt_mismatch && non_warning_mismatch)
> -          || (with_fexceptions1 != with_fexceptions2));
> +   has_any_incompatible_cg_opts
> +       = has_incompatible_cg_opts (cg_opts1, cg_opts2, num_cg_opts);
> +
> +   XDELETEVEC (warning_opts1);
> +   XDELETEVEC (warning_opts2);
> +   XDELETEVEC (non_warning_opts1);
> +   XDELETEVEC (non_warning_opts2);
> +   XDELETEVEC (cg_opts1);
> +   XDELETEVEC (cg_opts2);
> +   htab_delete (option_tab1);
> +   htab_delete (option_tab2);
> +   return ((flag_ripa_disallow_opt_mismatch && non_warning_mismatch)
> +           || has_any_incompatible_cg_opts);
>  }
>
>  /* Read in the counts file, if available. DA_FILE_NAME is the
>
> --
> This patch is available for review at http://codereview.appspot.com/6476057

Patch

Index: coverage.c
===================================================================
--- coverage.c	(revision 190369)
+++ coverage.c	(working copy)
@@ -261,6 +261,56 @@  str_eq (const void *p1, const void *p2)
   return !strcmp (s1, s2);
 }
 
+/* Command line option descriptor.  */
+
+struct opt_desc
+{
+  const char *opt_str;
+  const char *opt_neg_str;
+  bool default_val;  /* TODO better handling of default  */
+};
+
+static struct opt_desc force_matching_cg_opts[] =
+  {
+    { "-fexceptions", "-fno-exceptions", true },
+    { "-fsized-delete", "-fno-sized-delete", false },
+    { NULL, NULL, false }
+  };
+
+/* A helper function to check if OPTION_STRING is one of the codegen
+   options specified in FORCE_MATCHING_CG_ARGS. If yes, set the
+   corresponding entry in CG_ARG_VAL to the value of the option specified
+   in OPTION_STRING.  */
+
+static void
+check_cg_opts (bool *cg_opt_val, const char *option_str)
+{
+  unsigned int i;
+  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
+    {
+      if (!strcmp (force_matching_cg_opts[i].opt_str, option_str))
+        cg_opt_val[i] = true;
+      else if (!strcmp (force_matching_cg_opts[i].opt_neg_str, option_str))
+        cg_opt_val[i] = false;
+    }
+}
+
+/* A helper function to check if CG_OPTS1 and CG_OPTS are identical. It returns
+   true if yes, false otherwise.  */
+
+static bool
+has_incompatible_cg_opts (bool *cg_opts1, bool *cg_opts2, unsigned num_cg_opts)
+{
+  unsigned i;
+
+  for (i = 0; i < num_cg_opts; i++)
+    {
+      if (cg_opts1[i] != cg_opts2[i])
+        return true;
+    }
+
+  return false;
+}
 
 /* Returns true if the command-line arguments stored in the given module-infos
    are incompatible.  */
@@ -276,8 +326,6 @@  incompatible_cl_args (struct gcov_module
   unsigned int num_non_warning_opts1 = 0, num_non_warning_opts2 = 0;
   bool warning_mismatch = false;
   bool non_warning_mismatch = false;
-  bool with_fexceptions1 = true;
-  bool with_fexceptions2 = true;
   htab_t option_tab1, option_tab2;
   unsigned int start_index1 = mod_info1->num_quote_paths +
     mod_info1->num_bracket_paths + mod_info1->num_cpp_defines +
@@ -286,6 +334,22 @@  incompatible_cl_args (struct gcov_module
     mod_info2->num_bracket_paths + mod_info2->num_cpp_defines +
     mod_info2->num_cpp_includes;
 
+  bool *cg_opts1, *cg_opts2, has_any_incompatible_cg_opts;
+  unsigned int num_cg_opts = 0;
+
+  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
+    num_cg_opts++;
+
+  cg_opts1 = XCNEWVEC (bool, num_cg_opts);
+  cg_opts2 = XCNEWVEC (bool, num_cg_opts);
+
+  /* Initialize the array to default value  */
+  for (i = 0; force_matching_cg_opts[i].opt_str; i++)
+    {
+      cg_opts1[i] = force_matching_cg_opts[i].default_val;
+      cg_opts2[i] = force_matching_cg_opts[i].default_val;
+    }
+
   option_tab1 = htab_create (10, str_hash, str_eq, NULL);
   option_tab2 = htab_create (10, str_hash, str_eq, NULL);
 
@@ -297,12 +361,9 @@  incompatible_cl_args (struct gcov_module
     else
       {
         void **slot;
-        char* option_string = mod_info1->string_array[start_index1 + i];
+        char *option_string = mod_info1->string_array[start_index1 + i];
 
-        if (!strcmp ("-fexceptions", option_string))
-          with_fexceptions1 = true;
-        else if (!strcmp ("-fno-exceptions", option_string))
-          with_fexceptions1 = false;
+        check_cg_opts (cg_opts1, option_string);
 
         slot = htab_find_slot (option_tab1, option_string, INSERT);
         if (!*slot)
@@ -319,12 +380,10 @@  incompatible_cl_args (struct gcov_module
     else
       {
         void **slot;
-        char* option_string = mod_info2->string_array[start_index2 + i];
+        char *option_string = mod_info2->string_array[start_index2 + i];
+
+        check_cg_opts (cg_opts2, option_string);
 
-        if (!strcmp ("-fexceptions", option_string))
-          with_fexceptions2 = true;
-        else if (!strcmp ("-fno-exceptions", option_string))
-          with_fexceptions2 = false;
         slot = htab_find_slot (option_tab2, option_string, INSERT);
         if (!*slot)
           {
@@ -354,7 +413,7 @@  incompatible_cl_args (struct gcov_module
     warning (OPT_Wripa_opt_mismatch, "command line arguments mismatch for %s "
 	     "and %s", mod_info1->source_filename, mod_info2->source_filename);
 
-   if (warn_ripa_opt_mismatch && non_warning_mismatch 
+   if (warn_ripa_opt_mismatch && non_warning_mismatch
        && (flag_opt_info >= OPT_INFO_MED))
      {
        inform (UNKNOWN_LOCATION, "Options for %s", mod_info1->source_filename);
@@ -365,14 +424,19 @@  incompatible_cl_args (struct gcov_module
          inform (UNKNOWN_LOCATION, non_warning_opts2[i]);
      }
 
-  XDELETEVEC (warning_opts1);
-  XDELETEVEC (warning_opts2);
-  XDELETEVEC (non_warning_opts1);
-  XDELETEVEC (non_warning_opts2);
-  htab_delete (option_tab1);
-  htab_delete (option_tab2);
-  return ((flag_ripa_disallow_opt_mismatch && non_warning_mismatch)
-          || (with_fexceptions1 != with_fexceptions2));
+   has_any_incompatible_cg_opts
+       = has_incompatible_cg_opts (cg_opts1, cg_opts2, num_cg_opts);
+
+   XDELETEVEC (warning_opts1);
+   XDELETEVEC (warning_opts2);
+   XDELETEVEC (non_warning_opts1);
+   XDELETEVEC (non_warning_opts2);
+   XDELETEVEC (cg_opts1);
+   XDELETEVEC (cg_opts2);
+   htab_delete (option_tab1);
+   htab_delete (option_tab2);
+   return ((flag_ripa_disallow_opt_mismatch && non_warning_mismatch)
+           || has_any_incompatible_cg_opts);
 }
 
 /* Read in the counts file, if available. DA_FILE_NAME is the