* [PATCH] [annotalysis] Support IPA-SRA cloned functions (issue 4591066)
@ 2011-06-22 20:24 Delesley Hutchins
0 siblings, 0 replies; 2+ messages in thread
From: Delesley Hutchins @ 2011-06-22 20:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Le-Chun Wu, Diego Novillo, Ollie Wild
Hi,
This patch is merely a port of an earlier patch, made by Le-Chun Wu,
from google/main
to annotalysis. It extends Annotalysis to support cloned
functions/methods (especially
created by IPA-SRA).
Bootstrapped and passed GCC regression testsuite on x86_64-unknown-linux-gnu.
Okay for branches/annotalysis?
-DeLesley
2011-06-22 Le-Chun Wu <lcwu@google.com>, DeLesley Hutchins
<delesley@google.com>
* tree-threadsafe-analyze.c (build_fully_qualified_lock): Handle
IPA-SRA cloned methods.
(get_canonical_lock_expr): Fold expressions that are INDIRECT_REF on
top of ADDR_EXPR.
(check_lock_required): Handle IPA-SRA cloned methods.
(check_func_lock_excluded): Likewise.
(process_function_attrs): Likewise.
Index: gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C
===================================================================
--- gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C (revision 0)
+++ gcc/testsuite/g++.dg/thread-ann/thread_annot_lock-78.C (revision 175062)
@@ -0,0 +1,28 @@
+// Test the support to handle cloned methods.
+// { dg-do compile }
+// { dg-options "-O2 -Wthread-safety -fipa-sra" }
+
+#include "thread_annot_common.h"
+
+struct A {
+ int *data_ PT_GUARDED_BY(mu_);
+ mutable Mutex mu_;
+ void Reset(void) EXCLUSIVE_LOCKS_REQUIRED(mu_) {
+ delete data_;
+ }
+};
+
+struct B {
+ A a_;
+ void TestFunc1();
+ void TestFunc2();
+};
+
+void B::TestFunc1() {
+ MutexLock l(&a_.mu_);
+ a_.Reset(); // This call is an IPA-SRA clone candidate.
+}
+
+void B::TestFunc2() {
+ a_.Reset(); // { dg-warning "Calling function 'Reset' requires lock" }
+}
Index: gcc/tree-threadsafe-analyze.c
===================================================================
--- gcc/tree-threadsafe-analyze.c (revision 175061)
+++ gcc/tree-threadsafe-analyze.c (working copy)
@@ -696,9 +696,13 @@
rid of the ADDR_EXPR operator before we form the new lock expression. */
if (TREE_CODE (canon_base) != ADDR_EXPR)
{
- gcc_assert (POINTER_TYPE_P (TREE_TYPE (canon_base)));
- canon_base = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ /* Note that if the base object is neither an ADDR_EXPR, nor a pointer,
+ most likely we have done IPA-SRA optimization and the DECL is a
+ cloned method, where reference parameters are changed to be passed
+ by value. So in this case, we don't need to do anything. */
+ if (POINTER_TYPE_P (TREE_TYPE (canon_base)))
+ canon_base = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (canon_base)),
+ canon_base);
}
else
canon_base = TREE_OPERAND (canon_base, 0);
@@ -956,8 +960,18 @@
true /* is_temp_expr */,
new_leftmost_base_var);
if (base != canon_base)
- lock = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ {
+ /* If CANON_BASE is an ADDR_EXPR (e.g. &a), doing an indirect or
+ memory reference on top of it is equivalent to accessing the
+ variable itself. That is, *(&a) == a. So if that's the case,
+ simply return the variable. Otherwise, build an indirect ref
+ expression. */
+ if (TREE_CODE (canon_base) == ADDR_EXPR)
+ lock = TREE_OPERAND (canon_base, 0);
+ else
+ lock = build1 (INDIRECT_REF,
+ TREE_TYPE (TREE_TYPE (canon_base)), canon_base);
+ }
break;
}
default:
@@ -1135,10 +1149,18 @@
NULL_TREE);
if (TREE_CODE (canon_base) != ADDR_EXPR)
{
- gcc_assert (POINTER_TYPE_P (TREE_TYPE (canon_base)));
- base_obj = build1 (INDIRECT_REF,
- TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ if (POINTER_TYPE_P (TREE_TYPE (canon_base)))
+ base_obj = build1 (INDIRECT_REF,
+ TREE_TYPE (TREE_TYPE (canon_base)),
+ canon_base);
+ /* If the base object is neither an ADDR_EXPR, nor a pointer,
+ and DECL is a cloned method, most likely we have done IPA-SRA
+ optimization, where reference parameters are changed to be
+ passed by value. So in this case, just use the CANON_BASE. */
+ else if (DECL_ABSTRACT_ORIGIN (decl))
+ base_obj = canon_base;
+ else
+ gcc_unreachable ();
}
else
base_obj = TREE_OPERAND (canon_base, 0);
@@ -1154,9 +1176,9 @@
/* We want to use fully-qualified expressions (i.e. including base_obj
if any) for DECL when emitting warning messages. */
- if (base_obj)
+ if (TREE_CODE (decl) != FUNCTION_DECL)
{
- if (TREE_CODE (decl) != FUNCTION_DECL)
+ if (base_obj)
{
tree full_decl = build3 (COMPONENT_REF, TREE_TYPE (decl),
base_obj, decl, NULL_TREE);
@@ -1164,6 +1186,10 @@
true /* is_temp_expr */, NULL_TREE);
}
}
+ else
+ /* If DECL is a function, call DECL_ORIGIN first in case it is a clone so
+ that we can avoid using the cloned name in the warning messages. */
+ decl = DECL_ORIGIN (decl);
if (!lock)
{
@@ -1963,10 +1989,18 @@
NULL_TREE);
if (TREE_CODE (canon_base) != ADDR_EXPR)
{
- gcc_assert (POINTER_TYPE_P (TREE_TYPE (canon_base)));
- base_obj = build1 (INDIRECT_REF,
- TREE_TYPE (TREE_TYPE (canon_base)),
- canon_base);
+ if (POINTER_TYPE_P (TREE_TYPE (canon_base)))
+ base_obj = build1 (INDIRECT_REF,
+ TREE_TYPE (TREE_TYPE (canon_base)),
+ canon_base);
+ /* If the base object is neither an ADDR_EXPR, nor a pointer,
+ and DECL is a cloned method, most likely we have done IPA-SRA
+ optimization, where reference parameters are changed to be
+ passed by value. So in this case, just use the CANON_BASE. */
+ else if (DECL_ABSTRACT_ORIGIN (fdecl))
+ base_obj = canon_base;
+ else
+ gcc_unreachable ();
}
else
base_obj = TREE_OPERAND (canon_base, 0);
@@ -2116,8 +2150,17 @@
current_bb_info->writes_ignored = false;
/* If the function is a class member, the first argument of the function
- (i.e. "this" pointer) would be the base object. */
- if (TREE_CODE (TREE_TYPE (fdecl)) == METHOD_TYPE)
+ (i.e. "this" pointer) would be the base object. Note that here we call
+ DECL_ORIGIN on fdecl first before we check whether it's a METHOD_TYPE
+ because if fdecl is a cloned method, the TREE_CODE of its type would be
+ FUNCTION_DECL instead of METHOD_DECL, which would lead us to not grab
+ its base object. One possible situation where fdecl could be a clone is
+ when -fipa-sra is enabled. (-fipa-sra is enabled by default at -O2
+ starting from GCC-4.5.). The clones could be created as early as when
+ constructing SSA. Also note that the parameters of a cloned method could
+ be optimized away. */
+ if (TREE_CODE (TREE_TYPE (DECL_ORIGIN (fdecl))) == METHOD_TYPE
+ && gimple_call_num_args(call) > 0)
base_obj = gimple_call_arg (call, 0);
/* Check whether this is a locking primitive of any kind. */
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] [annotalysis] Support IPA-SRA cloned functions (issue 4591066)
[not found] <BANLkTi=+MUD=PYV_Lnq0qqKTOcWzhjPnu-yuJBnkHDwHsMwBYQ@mail.gmail.com>
@ 2011-06-28 13:01 ` Diego Novillo
0 siblings, 0 replies; 2+ messages in thread
From: Diego Novillo @ 2011-06-28 13:01 UTC (permalink / raw)
To: Delesley Hutchins; +Cc: gcc-patches, Le-Chun Wu, Ollie Wild
On Wed, Jun 22, 2011 at 16:10, Delesley Hutchins <delesley@google.com> wrote:
>
> Hi,
> This patch is merely a port of an earlier patch, made by Le-Chun Wu, from
> google/main to annotalysis. It extends Annotalysis to support cloned
> functions/methods (especially created by IPA-SRA).
> Bootstrapped and passed GCC regression testsuite on
> x86_64-unknown-linux-gnu.
> Okay for branches/annotalysis?
> -DeLesley
>
> 2011-06-22 Le-Chun Wu <lcwu@google.com>, DeLesley Hutchins
> <delesley@google.com>
Minor nit. Align names vertically:
2011-06-22 Le-Chun Wu <lcwu@google.com>
DeLesley Hutchins <delesley@google.com>
> * tree-threadsafe-analyze.c (build_fully_qualified_lock): Handle
> IPA-SRA cloned methods.
> (get_canonical_lock_expr): Fold expressions that are INDIRECT_REF on
> top of ADDR_EXPR.
> (check_lock_required): Handle IPA-SRA cloned methods.
> (check_func_lock_excluded): Likewise.
> (process_function_attrs): Likewise.
OK.
Incidentally, I think it would make sense to have you added to the
list of maintainers for the annotalysis branch. Le-Chun, what do you
think?
Diego.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-06-28 12:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-22 20:24 [PATCH] [annotalysis] Support IPA-SRA cloned functions (issue 4591066) Delesley Hutchins
[not found] <BANLkTi=+MUD=PYV_Lnq0qqKTOcWzhjPnu-yuJBnkHDwHsMwBYQ@mail.gmail.com>
2011-06-28 13:01 ` Diego Novillo
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).