Hi Richard, On Fri, 9 Jan 2015 17:19:57, Richard Biener wrote: > > > Yes. As said, you generally need to run folding results through > force_gimple_operand. > > Richard. >
I have now used force_gimple_operand instead of special casing the
VIEW_CONVERT_EXPRs.
And I see that all Ada test cases still work with -fsanitize=thread. So this
feels like an improvement.
I have checked with a large C++ application, to see if the generated code
changes or not.
And although this looked like it should not change the resulting code, I found
one small difference
at -O3 -fsanitize=thread while compiling the function xmlSchemaCompareValuesInt
in xmlschematypes.c
of libxml2. The generated code size did not change, only two blocks of code
changed place.
That was the only difference in about 16 MB of code.
The reason for this seems to be the following changes in the
xmlschemastypes.c.104t.tsan1
<bb 29>:
p1_179 = xmlSchemaDateNormalize (x_7(D), 0.0);
# DEBUG p1 => p1_179
_180 = _xmlSchemaDateCastYMToDays (p1_179);
- _660 = &p1_179->value.date;
- _659 = &MEM[(struct xmlSchemaValDate *)_660 + 8B];
- __builtin___tsan_read2 (_659);
+ _660 = &MEM[(struct xmlSchemaValDate *)p1_179 + 24B];
+ __builtin___tsan_read2 (_660);
_181 = p1_179->value.date.day;
_182 = (long int) _181;
p1d_183 = _180 + _182;
this pattern is repeated everywhere. (- = before the patch. + = with the patch)
So it looks as if the generated code quality slightly improves with this change.
I have also tried to fold &base + offset + bitpos, like this:
--- tsan.c.orig 2015-01-10 00:39:06.465210937 +0100
+++ tsan.c 2015-01-11 09:28:38.109423856 +0100
@@ -213,7 +213,18 @@ instrument_expr (gimple_stmt_iterator gs
align = get_object_alignment (expr);
if (align < BITS_PER_UNIT)
return false;
- expr_ptr = build_fold_addr_expr (unshare_expr (expr));
+ expr_ptr = build_fold_addr_expr (unshare_expr (base));
+ if (bitpos != 0)
+ {
+ if (offset != NULL)
+ offset = size_binop (PLUS_EXPR, offset,
+ build_int_cst (sizetype,
+ bitpos / BITS_PER_UNIT));
+ else
+ offset = build_int_cst (sizetype, bitpos / BITS_PER_UNIT);
+ }
+ if (offset != NULL)
+ expr_ptr = fold_build_pointer_plus (expr_ptr, offset);
}
expr_ptr = force_gimple_operand (expr_ptr, &seq, true, NULL_TREE);
if ((size & (size - 1)) != 0 || size> 16
For simplicity first only in the simple case without
DECL_BIT_FIELD_REPRESENTATIVE.
I tried this change at the same large C++ application, and see the code still
works,
but the binary size increases at -O3 by about 1%.
So my conclusion would be that it is better to use force_gimple_operand
directly
on build_fold_addr_expr (unshare_expr (expr)), without using offset.
Well, I think this still resolves your objections.
Furthermore I used may_be_nonaddressable_p instead of is_gimple_addressable
and just return if it is found to be not true. (That did not happen in my
tests.)
And I reworked the block with the pt_solution_includes.
I found that It can be rewritten, because pt_solution_includes can be
expanded to (is_global_var (decl) || pt_solution_includes_1
(&cfun->gimple_df->escaped, decl)
|| pt_solution_includes_1 (&ipa_escaped_pt, decl))
So, by De Morgan's law, you can rewite that block to
if (DECL_P (base))
{
if (!is_global_var (base)
&& !pt_solution_includes_1 (&cfun->gimple_df->escaped, base)
&& !pt_solution_includes_1 (&ipa_escaped_pt, base))
return false;
if (!is_global_var (base) && !may_be_aliased (base))
return false;
}
Therefore I can move the common term !is_global_var (base) out of the block.
That's what I did.
As far as I can tell, none of the other terms here seem to be redundant.
Attached patch was boot-strapped and regression-tested on x86_64-linux-gnu.
OK for trunk?
Thanks
Bernd.
gcc/ChangeLog:
2015-01-11 Bernd Edlinger <bernd.edlin...@hotmail.de>
* tsan.c (instrument_expr): Use force_gimple_operand.
Use may_be_nonaddressable_p instead of is_gimple_addressable.
patch-tsan-cleanup.diff
Description: Binary data
