Hi!
As discussed earlier on mailing list and on IRC with Richard,
pt_solution_includes can be used for testing of whether var
might escape current function (for -O0 unfortunately returns
true even for !may_be_aliased automatic vars, so I'm testing
for that too), and there are IMHO tons of completely unnecessary tests
and restrictions what we want to instrument and what not.
instrument_expr is guarded with gimple_store_p or
gimple_assign_load_p, so it shouldn't see SSA_NAMEs, and many other
ops, basically just DECL_Ps, handled_components, MEM_REF or TARGET_MEM_REF
where for handled_components the base address is one of DECL_P, MEM_REF
or TARGET_MEM_REF.
I'm actually surprised that insturment_expr handles also is_gimple_call
in it, yet nothing calls it for calls, I bet for calls we also want to call
it for gimple_store_p calls.
Dmitry, could you please test this on something where the previous tsan
code has been tested on (if anything)? All I've done was eyeball a few
short testcases.
TSAN will badly need similar optimization pass to what ASAN needs (after
deferring expansion of the shadow memory checks), e.g. var++ right now
results in __tsan_read4 (&var); followed soon by __tsan_write4 (&var);.
With no intervening calls (we could ignore many string/memory builtins
I guess) and no intervening atomics it should be fine to just use
__tsan_write4 (&var); for that, right? Similarly for multiple accesses
to the same var, where first use dominates the other accesses and there
are no intervening calls.
As for bitfields, I think we want to use DECL_BIT_FIELD_REPRESENTATIVE.
2012-11-29 Jakub Jelinek <ja...@redhat.com>
* tsan.c (is_load_of_const_p): Removed.
(instrument_expr): Use result of get_inner_reference
instead of get_base_address, avoid some unnecessary tests,
use !pt_solution_includes and !may_be_aliased tests to
check whether base might escape current function.
--- gcc/tsan.c.jj 2012-11-23 15:27:38.000000000 +0100
+++ gcc/tsan.c 2012-11-29 12:41:26.816857288 +0100
@@ -84,65 +84,18 @@ is_vptr_store (gimple stmt, tree expr, b
return NULL;
}
-/* Checks as to whether EXPR refers to constant var/field/param.
- Don't bother to instrument them. */
-
-static bool
-is_load_of_const_p (tree expr, bool is_write)
-{
- if (is_write)
- return false;
- if (TREE_CODE (expr) == COMPONENT_REF)
- expr = TREE_OPERAND (expr, 1);
- if (TREE_CODE (expr) == VAR_DECL
- || TREE_CODE (expr) == PARM_DECL
- || TREE_CODE (expr) == FIELD_DECL)
- {
- if (TREE_READONLY (expr))
- return true;
- }
- return false;
-}
-
/* Instruments EXPR if needed. If any instrumentation is inserted,
return true. */
static bool
instrument_expr (gimple_stmt_iterator gsi, tree expr, bool is_write)
{
- enum tree_code tcode;
tree base, rhs, expr_type, expr_ptr, builtin_decl;
basic_block bb;
HOST_WIDE_INT size;
gimple stmt, g;
location_t loc;
- base = get_base_address (expr);
- if (base == NULL_TREE
- || TREE_CODE (base) == SSA_NAME
- || TREE_CODE (base) == STRING_CST)
- return false;
-
- tcode = TREE_CODE (expr);
-
- /* Below are things we do not instrument
- (no possibility of races or not implemented yet). */
- if (/* Compiler-emitted artificial variables. */
- (DECL_P (expr) && DECL_ARTIFICIAL (expr))
- /* The var does not live in memory -> no possibility of races. */
- || (tcode == VAR_DECL
- && !TREE_ADDRESSABLE (expr)
- && TREE_STATIC (expr) == 0)
- /* Not implemented. */
- || TREE_CODE (TREE_TYPE (expr)) == RECORD_TYPE
- /* Not implemented. */
- || tcode == CONSTRUCTOR
- /* Not implemented. */
- || tcode == PARM_DECL
- /* Load of a const variable/parameter/field. */
- || is_load_of_const_p (expr, is_write))
- return false;
-
size = int_size_in_bytes (TREE_TYPE (expr));
if (size == -1)
return false;
@@ -153,18 +106,29 @@ instrument_expr (gimple_stmt_iterator gs
tree offset;
enum machine_mode mode;
int volatilep = 0, unsignedp = 0;
- get_inner_reference (expr, &bitsize, &bitpos, &offset,
- &mode, &unsignedp, &volatilep, false);
- if (bitpos % (size * BITS_PER_UNIT)
- || bitsize != size * BITS_PER_UNIT)
+ base = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+ &mode, &unsignedp, &volatilep, false);
+
+ /* No need to instrument accesses to decls that don't escape,
+ they can't escape to other threads then. */
+ if (DECL_P (base))
+ {
+ struct pt_solution pt;
+ memset (&pt, 0, sizeof (pt));
+ pt.escaped = 1;
+ pt.ipa_escaped = flag_ipa_pta != 0;
+ pt.nonlocal = 1;
+ if (!pt_solution_includes (&pt, base))
+ return false;
+ if (!is_global_var (base) && !may_be_aliased (base))
+ return false;
+ }
+
+ if (TREE_READONLY (base))
return false;
- /* TODO: handle other case: ARRAY_RANGE_REF. */
- if (tcode != ARRAY_REF
- && tcode != VAR_DECL
- && tcode != COMPONENT_REF
- && tcode != INDIRECT_REF
- && tcode != MEM_REF)
+ if (bitpos % (size * BITS_PER_UNIT)
+ || bitsize != size * BITS_PER_UNIT)
return false;
stmt = gsi_stmt (gsi);
Jakub
