Comments

Hi,
I found that all thread local variables are instrumented with _ITM_W/R*
calls whereas they should not be shared with other threads. This patch
takes care of thread locals into requires_barrier and also adds the
local save/restore for them. This patch also includes a testcase.
I did not fill any PR since I have already a patch for it but tell me if
I have to.
Tested on x86_64-unknown-linux-gnu, ok for trunk?
Thanks.
--
Patrick Marlier.
ChangeLog
2012-01-23 Patrick Marlier <patrick.marlier@gmail.com>
* trans-mem.c (requires_barrier): Do not instrument thread local
variables and emit save/restore for them.
testsuite/ChangeLog
2012-01-23 Patrick Marlier <patrick.marlier@gmail.com>
* gcc.dg/tm/threadlocal-1.c: New test.

Patrick Marlier <patrick.marlier@gmail.com> writes:
> Hi,>> I found that all thread local variables are instrumented with> _ITM_W/R* calls whereas they should not be shared with other> threads. This patch takes care of thread locals into requires_barrier> and also adds the local save/restore for them. This patch also> includes a testcase.
What happens when the address of the thread local escapes?
It could well be written by another thread then.
I guess you have to check for escapes here.
-Andi

On 01/24/2012 08:32 PM, Andi Kleen wrote:
>> Hi,>> >>> > I found that all thread local variables are instrumented with>> > _ITM_W/R* calls whereas they should not be shared with other>> > threads. This patch takes care of thread locals into requires_barrier>> > and also adds the local save/restore for them. This patch also>> > includes a testcase.> What happens when the address of the thread local escapes?> It could well be written by another thread then.>> I guess you have to check for escapes here.
Thanks to raise the question and I hope I understand your point.
Did you mean something like this?
__thread int myvalue;
void bar()
{
foo(&myvalue);
__transaction_atomic {
myvalue++;
}
}
where foo shares the pointer to other threads?
From my point of view, no. When it is a thread local, it should not be
shared to someone else. If the thread dies, what happens to the thread
local variable? Should it be discarded completely and this piece of
memory never reallocated? Even if the programmer take care of this
situation, does it make sense to share a thread local to other threads?
Anyway, you are probably right but I would prefer let the knowledgeable
people answer instead of me, the little jedi...
--
Patrick.

On 01/25/2012 01:30 PM, Patrick Marlier wrote:
> From my point of view, no. When it is a thread local, it should not> be shared to someone else. If the thread dies, what happens to the> thread local variable? Should it be discarded completely and this> piece of memory never reallocated? Even if the programmer take care> of this situation, does it make sense to share a thread local to> other threads?
No, Andi has a point. It's no more invalid than sharing a variable
off the local stack with another thread. All that's required is that
the foreign thread not keep the pointer permanently; that the use of
the tls variable end before the thread ends.
And it's entirely likely that I'd thought of exactly that two years
ago when the DECL_THREAD_LOCAL test was omitted from that bit of code,
but I failed to add a comment.
I guess this patch needs to be reverted...
r~

> And it's entirely likely that I'd thought of exactly that two years> ago when the DECL_THREAD_LOCAL test was omitted from that bit of code,> but I failed to add a comment.> > I guess this patch needs to be reverted...
It may be still a valid optimization, but only if you know there
is no escapes of a static thread variable. But I didn't think
this is known at this point.
I suspect this would require adding another pass for this later in the
optimization pipeline.
-Andi

On 01/25/12 14:16, Richard Henderson wrote:
> On 01/25/2012 01:30 PM, Patrick Marlier wrote:>> From my point of view, no. When it is a thread local, it should not>> be shared to someone else. If the thread dies, what happens to the>> thread local variable? Should it be discarded completely and this>> piece of memory never reallocated? Even if the programmer take care>> of this situation, does it make sense to share a thread local to>> other threads?>> No, Andi has a point. It's no more invalid than sharing a variable> off the local stack with another thread. All that's required is that> the foreign thread not keep the pointer permanently; that the use of> the tls variable end before the thread ends.>> And it's entirely likely that I'd thought of exactly that two years> ago when the DECL_THREAD_LOCAL test was omitted from that bit of code,> but I failed to add a comment.>> I guess this patch needs to be reverted...
Will do so.