Hi,
On Fri, 9 Jan 2015 10:57:14, Richard Biener wrote:
>
> On Mon, Jan 5, 2015 at 9:00 PM, Jeff Law <l...@redhat.com> wrote:
>> On 01/03/15 06:49, Bernd Edlinger wrote:
>>>
>>> Hi,
>>>
>>> I was experimenting with enabling TSAN for Ada recently.
>>> I think this gives rather interesting results.
>>>
>>> The Instrumentation worked almost out of the box, we just have
>>> the problem that it is not gimple-OK to fold something like
>>> "& VIEW_CONVERT_EXPR(x)", and this happens in Ada all the time.
>>>
>>> Boot-Strapped and regression-tested on x86_64-linux-gnu.
>>> OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>>
>>> changelog-tsan-ada.txt
>>>
>>>
>>> gcc/ChangeLog:
>>> 2015-01-03 Bernd Edlinger<bernd.edlin...@hotmail.de>
>>>
>>> Enable experimental TSAN support for Ada.
>>> * tsan.c (instrument_expr): Handle VIEW_CONVERT_EXPR.
>>
>> OK for the trunk with a comment before the new block of code indicating why
>> we need to handle VIEW_CONVERT_EXPR specially here (specifically we can't
>> call build_fold_addr_expr on the VIEW_CONVERT_EXPR).
>
> There may be multiple VIEW_CONVERT_EXPRs in a reference chain
> so simply stripping the outermost only doesn't work (the assert).
>
Hmm, that did not happen in any of the Ada tests in ada/acats nor in gnat.dg,
but with Ada anything may be possible...
Would you like it better if I do it this way:
align = get_object_alignment (expr);
if (align < BITS_PER_UNIT)
return false;
do
{
expr = TREE_OPERAND (expr, 0);
} while (TREE_CODE (expr) == VIEW_CONVERT_EXPR);
gcc_checking_assert (is_gimple_addressable (expr));
expr_ptr = build_fold_addr_expr (unshare_expr (expr));
> I wonder why you do all the special-casing when you have already
> called get_inner_reference on the reference. The address is
> simply &base + offset + bitpos / BITS_PER_UNIT, the bitfield
> case is detectable via bitpos % BITS_PER_UNIT != 0.
>
I tried that first, but for something lile S.A[x].B
offset is someting like a+b*x, and while we handle that in expansion,
it is pretty hard to fold gimple code for &base + offset in this case.
But it is relatively easy to fold &S.A[x] and add a contant byte offset.
> Sth else I noticed, instead of checking points-to in the weird way
> you do you simply want if (DECL_P (base) && !may_be_aliased (base)).
>
you mean,
/* 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;
}
should be equivalent to
if (DECL_P (base) && !may_be_aliased (base))
return false;
is that right?
Thanks,
Bernd
> Richard.
>
>> Jeff
>>
