[Bug c++/55742] __attribute__ in class function declaration cause "prototype does not match" errors.

["tmsriram at google dot com" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55742

Sriraman Tallam <tmsriram at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |davidxl at google dot com

--- Comment #1 from Sriraman Tallam <tmsriram at google dot com> 2012-12-19 23:29:51 UTC ---
(In reply to comment #0)
> The following code used to compile with GCC 4.7.  It fails on trunk with:
> 
> $ bld/xgcc -Bbld/ -c a.cc
> a.cc:10:6: error: prototype for 'void A::E(uint64*, uint64*, const void*,
> int64) const' does not match any in class 'A'
>  void A::E(uint64 *l, uint64 *h, const void *b, int64 g) const
>       ^
> a.cc:6:18: error: candidate is: virtual void A::E(uint64*, uint64*, const
> void*, int64) const
>      virtual void E(uint64 *l, uint64 *h, const void *b, int64 g) const
> 
> 
> typedef unsigned long long uint64;
> typedef long long int64;
> 
> class A {
>   public:
>     virtual void E(uint64 *l, uint64 *h, const void *b, int64 g) const
>         __attribute__ ((__target__ ("sse4")));
> };
> 
> void A::E(uint64 *l, uint64 *h, const void *b, int64 g) const
> {
>     *l = *h + g;
>     if (b) return;
> }
> 
> This seems to be a bug in the multiversioning logic.  We fail to match the
> function to its declaration in decl2.c:check_classfn because
> ix86_function_versions returns true.
> 
> 676
> 677               /* While finding a match, same types and params are not
> enough
> 678                  if the function is versioned.  Also check version
> ("target")
> 679                  attributes.  */
> 680               if (same_type_p (TREE_TYPE (TREE_TYPE (function)),
> 681                                TREE_TYPE (TREE_TYPE (fndecl)))
> 682                   && compparms (p1, p2)
> 683                   && !targetm.target_option.function_versions (function,
> fndecl)
> 684                   && (!is_template
> 685                       || comp_template_parms (template_parms,
> 686                                               DECL_TEMPLATE_PARMS
> (fndecl)))
> 687                   && (DECL_TEMPLATE_SPECIALIZATION (function)
> 688                       == DECL_TEMPLATE_SPECIALIZATION (fndecl))
> 689                   && (!DECL_TEMPLATE_SPECIALIZATION (function)
> 690                       || (DECL_TI_TEMPLATE (function)
> 691                           == DECL_TI_TEMPLATE (fndecl))))
> 692                 break;
> 
> While this agrees with the logic of the multiversion test, it is not the
> appropriate context to be checking for multiversions, I think.
> 
> Here we are comparing a function *declaration* with a function *definition*. 
> The function definition can never have attributes (only declarations do).

AFAIU, this is not true. Definitions can have target attributes too.

So, your example can be fixed by adding __attribute__ ((__target__ ("sse4")))
to the front of the definition:

__attribute__ ((__target__ ("sse4")))
void A::E(uint64 *l, uint64 *h, const void *b, int64 g) const
{
    *l = *h + g;
    if (b) return;
}

will now make that code compile.


Infact, I think this is necessary. It is *not correct* to add the target
attribute only to the declaration.

Example:


test.cc
=======

char a[17];
char c[16];

int bar () __attribute__ ((target ("sse4.2")));

int bar ()
{
  char *b = a+1;
  fprintf (stderr, "Foo\n");

  for (int i = 0; i< 16; i++)
    c[i] = b[i];

  return 255;
}

Here, only the declaration has sse4.2, not the definition.

$ g++ test.cc -ftree-vectorize  -c -O2 -mno-sse

Now, test.o does not contain any sse4.2 instructions.

Add "__attribute__ ((target ("sse4.2")))" to the definition, and recompile with
same command. test.o will have movdqa and movdqu instructions. So, the target
attributes should also be on the definition for functionality to be correct.

With Multiversioning, this problem is exposed. I think the source change is the
correct way to go here.








  So I
> *think* the right fix here is to not call the multiversion hook.
> 
> Sri, could you take a look?  This bug is causing build failures with our
> internal code base.
> 
> 
> Thanks.