Patchwork Function Multiversioning Bug, checking for function versions

login
register
mail settings
Submitter Sriraman Tallam
Date Jan. 2, 2013, 8:23 p.m.
Message ID <CAAs8HmzTimSvEUXQ7AMLi2Q2Y+=RmCs8FkLpDZTpRmNkLBUxgQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/209143/
State New
Headers show

Comments

Sriraman Tallam - Jan. 2, 2013, 8:23 p.m.
I committed this trivial patch to fix some corner case bugs in
Function Multiversioning.

        * config/i386/i386.c (ix86_get_function_versions_dispatcher): Fix bug
        in loop predicate.
        (fold_builtin_cpu): Do not share cpu model decls across statements.




Thanks,
-Sri.

On Thu, Dec 27, 2012 at 2:05 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 148388d..575e03a 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,4 +1,7 @@
> -<<<<<<< .mine
> +2012-12-27  Andreas Schwab  <schwab@linux-m68k.org>
> +
> +       * target.def (supports_function_versions): Fix typo.
> +
>  2012-12-26  Sriraman Tallam  <tmsriram@google.com>
>
>         * doc/tm.texi.in (TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS): Document
> @@ -15,12 +18,10 @@
>         * (is_function_default_version): Check target string.
>         * TARGET_OPTION_SUPPORTS_FUNCTION_VERSIONS: New macro.
>
> -=======
>  2012-12-27  Steven Bosscher  <steven@gcc.gnu.org>
>
>         * cgraph.c (verify_cgraph_node): Don't allocate/free visited_nodes set.
>
> ->>>>>>> .r194729
>  2012-12-25  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
>
>         PR target/53789
> diff --git a/gcc/target.def b/gcc/target.def
> index 79bb955..d0547be 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -2839,7 +2839,7 @@ DEFHOOK
>  (supports_function_versions,
>   "",
>   bool, (void),
> - hool_bool_void_false)
> + hook_bool_void_false)
>
>  /* Function to determine if one function can inline another function.  */
>  #undef HOOK_PREFIX
> --
> 1.8.0.2
>
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
Jakub Jelinek - Jan. 2, 2013, 9:01 p.m.
On Wed, Jan 02, 2013 at 12:23:45PM -0800, Sriraman Tallam wrote:
> --- config/i386/i386.c  (revision 194817)
> +++ config/i386/i386.c  (working copy)
> @@ -29290,7 +29290,7 @@ ix86_get_function_versions_dispatcher (void *decl)
> 
>    /* Set the dispatcher for all the versions.  */
>    it_v = default_version_info;
> -  while (it_v->next != NULL)
> +  while (it_v != NULL)
>      {
>        it_v->dispatcher_resolver = dispatch_decl;
>        it_v = it_v->next;
> @@ -29626,8 +29626,8 @@ fold_builtin_cpu (tree fndecl, tree *args)
>        {"avx2",   F_AVX2}
>      };
> 
> -  static tree __processor_model_type = NULL_TREE;
> -  static tree __cpu_model_var = NULL_TREE;
> +  tree __processor_model_type = NULL_TREE;
> +  tree __cpu_model_var = NULL_TREE;
> 
>    if (__processor_model_type == NULL_TREE)
>      __processor_model_type = build_processor_model_struct ();

If __processor_model_type is always NULL_TREE above, then you should write
  tree __processor_model_type = build_processor_model_struct ();
rather than the extra if with an always true condition.

	Jakub
Sriraman Tallam - Jan. 2, 2013, 9:48 p.m.
On Wed, Jan 2, 2013 at 1:01 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Jan 02, 2013 at 12:23:45PM -0800, Sriraman Tallam wrote:
>> --- config/i386/i386.c  (revision 194817)
>> +++ config/i386/i386.c  (working copy)
>> @@ -29290,7 +29290,7 @@ ix86_get_function_versions_dispatcher (void *decl)
>>
>>    /* Set the dispatcher for all the versions.  */
>>    it_v = default_version_info;
>> -  while (it_v->next != NULL)
>> +  while (it_v != NULL)
>>      {
>>        it_v->dispatcher_resolver = dispatch_decl;
>>        it_v = it_v->next;
>> @@ -29626,8 +29626,8 @@ fold_builtin_cpu (tree fndecl, tree *args)
>>        {"avx2",   F_AVX2}
>>      };
>>
>> -  static tree __processor_model_type = NULL_TREE;
>> -  static tree __cpu_model_var = NULL_TREE;
>> +  tree __processor_model_type = NULL_TREE;
>> +  tree __cpu_model_var = NULL_TREE;
>>
>>    if (__processor_model_type == NULL_TREE)
>>      __processor_model_type = build_processor_model_struct ();
>
> If __processor_model_type is always NULL_TREE above, then you should write
>   tree __processor_model_type = build_processor_model_struct ();
> rather than the extra if with an always true condition.

Right, sorry! Oversight in a hurry to fix the bug.

-Sri.

>
>         Jakub

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c  (revision 194817)
+++ config/i386/i386.c  (working copy)
@@ -29290,7 +29290,7 @@  ix86_get_function_versions_dispatcher (void *decl)

   /* Set the dispatcher for all the versions.  */
   it_v = default_version_info;
-  while (it_v->next != NULL)
+  while (it_v != NULL)
     {
       it_v->dispatcher_resolver = dispatch_decl;
       it_v = it_v->next;
@@ -29626,8 +29626,8 @@  fold_builtin_cpu (tree fndecl, tree *args)
       {"avx2",   F_AVX2}
     };

-  static tree __processor_model_type = NULL_TREE;
-  static tree __cpu_model_var = NULL_TREE;
+  tree __processor_model_type = NULL_TREE;
+  tree __cpu_model_var = NULL_TREE;

   if (__processor_model_type == NULL_TREE)
     __processor_model_type = build_processor_model_struct ();