On Thu, Mar 20, 2014 at 11:57:57AM +0000, Joseph S. Myers wrote: > This version is OK (for after 4.9 branches).
Thanks. > What happens if there are two declarations of the function, one with each > attribute? The testcase doesn't cover that. Nothing :(. I had to add some code into diagnose_mismatched_decls to detect such conflicting decls (plus for cold/hot; there may be others). Tested x86_64-linux. Is this version ok? 2014-03-20 Marek Polacek <pola...@redhat.com> PR c/18079 c/ * c-decl.c (diagnose_mismatched_decls): Warn for mismatched always_inline/noinline and hot/cold attributes. c-family/ * c-common.c (handle_noinline_attribute): Warn if the attribute conflicts with always_inline attribute. (handle_always_inline_attribute): Warn if the attribute conflicts with noinline attribute. testsuite/ * gcc.dg/pr18079.c: New test. * gcc.dg/pr18079-2.c: New test. diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index abd96fb..5258e52 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -6666,7 +6666,16 @@ handle_noinline_attribute (tree *node, tree name, int ARG_UNUSED (flags), bool *no_add_attrs) { if (TREE_CODE (*node) == FUNCTION_DECL) - DECL_UNINLINABLE (*node) = 1; + { + if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (*node))) + { + warning (OPT_Wattributes, "%qE attribute ignored due to conflict " + "with %<always_inline%> attribute", name); + *no_add_attrs = true; + } + else + DECL_UNINLINABLE (*node) = 1; + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); @@ -6704,9 +6713,16 @@ handle_always_inline_attribute (tree *node, tree name, { if (TREE_CODE (*node) == FUNCTION_DECL) { - /* Set the attribute and mark it for disregarding inline - limits. */ - DECL_DISREGARD_INLINE_LIMITS (*node) = 1; + if (lookup_attribute ("noinline", DECL_ATTRIBUTES (*node))) + { + warning (OPT_Wattributes, "%qE attribute ignored due to conflict " + "with %<noinline%> attribute", name); + *no_add_attrs = true; + } + else + /* Set the attribute and mark it for disregarding inline + limits. */ + DECL_DISREGARD_INLINE_LIMITS (*node) = 1; } else { diff --git gcc/c/c-decl.c gcc/c/c-decl.c index 2c41bf2..f96cc5e 100644 --- gcc/c/c-decl.c +++ gcc/c/c-decl.c @@ -2099,18 +2099,36 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl, /* Diagnose inline __attribute__ ((noinline)) which is silly. */ if (DECL_DECLARED_INLINE_P (newdecl) && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) - { - warned |= warning (OPT_Wattributes, - "inline declaration of %qD follows " - "declaration with attribute noinline", newdecl); - } + warned |= warning (OPT_Wattributes, + "inline declaration of %qD follows " + "declaration with attribute noinline", newdecl); else if (DECL_DECLARED_INLINE_P (olddecl) && lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl))) - { - warned |= warning (OPT_Wattributes, - "declaration of %q+D with attribute " - "noinline follows inline declaration ", newdecl); - } + warned |= warning (OPT_Wattributes, + "declaration of %q+D with attribute " + "noinline follows inline declaration ", newdecl); + else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (newdecl)) + && lookup_attribute ("always_inline", DECL_ATTRIBUTES (olddecl))) + warned |= warning (OPT_Wattributes, + "declaration of %q+D with attribute " + "noinline follows declaration with attribute " + "always_inline", newdecl); + else if (lookup_attribute ("always_inline", DECL_ATTRIBUTES (newdecl)) + && lookup_attribute ("noinline", DECL_ATTRIBUTES (olddecl))) + warned |= warning (OPT_Wattributes, + "declaration of %q+D with attribute " + "always_inline follows declaration with attribute " + "noinline", newdecl); + else if (lookup_attribute ("cold", DECL_ATTRIBUTES (newdecl)) + && lookup_attribute ("hot", DECL_ATTRIBUTES (olddecl))) + warned |= warning (OPT_Wattributes, + "declaration of %q+D with attribute cold follows " + "declaration with attribute hot", newdecl); + else if (lookup_attribute ("hot", DECL_ATTRIBUTES (newdecl)) + && lookup_attribute ("cold", DECL_ATTRIBUTES (olddecl))) + warned |= warning (OPT_Wattributes, + "declaration of %q+D with attribute hot follows " + "declaration with attribute cold", newdecl); } else /* PARM_DECL, VAR_DECL */ { diff --git gcc/testsuite/gcc.dg/pr18079-2.c gcc/testsuite/gcc.dg/pr18079-2.c index e69de29..5091dd41b 100644 --- gcc/testsuite/gcc.dg/pr18079-2.c +++ gcc/testsuite/gcc.dg/pr18079-2.c @@ -0,0 +1,16 @@ +/* PR c/18079 */ +/* { dg-do compile } */ +/* { dg-options "-Wall" } */ + +__attribute__ ((always_inline)) void fndecl1 (void); +__attribute__ ((noinline)) void fndecl1 (void); /* { dg-warning "attribute noinline follows declaration with attribute always_inline" } */ + +__attribute__ ((noinline)) void fndecl2 (void); +__attribute__ ((always_inline)) void fndecl2 (void); /* { dg-warning "attribute always_inline follows declaration with attribute noinline" } */ + + +__attribute__ ((hot)) void fndecl3 (void); +__attribute__ ((cold)) void fndecl3 (void); /* { dg-warning "attribute cold follows declaration with attribute hot" } */ + +__attribute__ ((cold)) void fndecl4 (void); +__attribute__ ((hot)) void fndecl4 (void); /* { dg-warning "attribute hot follows declaration with attribute cold" } */ diff --git gcc/testsuite/gcc.dg/pr18079.c gcc/testsuite/gcc.dg/pr18079.c index e69de29..b84cdeb 100644 --- gcc/testsuite/gcc.dg/pr18079.c +++ gcc/testsuite/gcc.dg/pr18079.c @@ -0,0 +1,33 @@ +/* PR c/18079 */ +/* { dg-do compile } */ +/* { dg-options "-Wall" } */ + +__attribute__ ((noinline)) +__attribute__ ((always_inline)) +int +fn1 (int r) +{ /* { dg-warning "attribute ignored due to conflict" } */ + return r & 4; +} + +__attribute__ ((noinline, always_inline)) +int +fn2 (int r) +{ /* { dg-warning "attribute ignored due to conflict" } */ + return r & 4; +} + +__attribute__ ((always_inline)) +__attribute__ ((noinline)) +inline int +fn3 (int r) +{ /* { dg-warning "attribute ignored due to conflict" } */ + return r & 8; +} + +__attribute__ ((always_inline, noinline)) +inline int +fn4 (int r) +{ /* { dg-warning "attribute ignored due to conflict" } */ + return r & 8; +} Marek