From: Delesley Hutchins <delesley@google.com>
To: gcc-patches@gcc.gnu.org
Cc: Le-Chun Wu <lcwu@google.com>, Diego Novillo <dnovillo@google.com>,
Ollie Wild <aaw@google.com>
Subject: [PATCH] [annotalysis] Support IPA-SRA cloned functions (issue 4591066)
Date: Wed, 22 Jun 2011 20:24:00 -0000 [thread overview]
Message-ID: <BANLkTinpa5DWd9r46GLayepHVYRX1A4YOc8h0QzChm5VuxJHNw@mail.gmail.com> (raw)
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. */
next reply other threads:[~2011-06-22 20:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-22 20:24 Delesley Hutchins [this message]
[not found] <BANLkTi=+MUD=PYV_Lnq0qqKTOcWzhjPnu-yuJBnkHDwHsMwBYQ@mail.gmail.com>
2011-06-28 13:01 ` Diego Novillo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BANLkTinpa5DWd9r46GLayepHVYRX1A4YOc8h0QzChm5VuxJHNw@mail.gmail.com \
--to=delesley@google.com \
--cc=aaw@google.com \
--cc=dnovillo@google.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=lcwu@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).