On Mon, Feb 27, 2012 at 10:24 AM, Torvald Riegel <trie...@redhat.com> wrote:
> On Fri, 2012-02-24 at 10:34 -0600, Aldy Hernandez wrote:
>> On 02/24/12 07:10, Torvald Riegel wrote:
>> > safety. I didn't have time to look at Aldy's patch yet, but a first
>> > safe and conservative way would be to treat transactions as full
>> > transformation barriers, and prevent publication-safety-violating
>> > transformations _within_ transactions. Which I would prefer until we're
>> > confident that we understood all of it.
>>
>> Do you mean disallow hoisting of *any* loads that happen inside of a
>> transaction (regardless of whether a subsequent load happens on every
>> path out of the loop)? This would definitely be safe and quite easily
>> doable, simply by checking if loads to be hoisted are within a transaction.
>
> If we cannot get the less conservative approach in on time (the one
> you've been working on), then we could also use this big hammer. I
> don't quite know how high performance degradation would be (we could
> measure number of loads/stores at runtime with STAMP, for example, and I
> could build the libitm code for that if you want), but having
> publication-safety work correctly in 4.7 might be more important.
>
>>
>> >
>> > For hoisting out of or across transactions, we have to reason about more
>> > than just publication safety.
>>
>> Again, __transactions being barriers and all, I don't think we should
>> complicate things unnecessarily at this point, since it doesn't happen.
>
> Yes. Based on Richard Guenther's examples, my question was whether your
> code (without having actually looked at it ;) ) would also allow
> post-dominating loads in nontransactional code to enable hoisting (as in
> the __transaction_atomic { if (foo) load; } load case. I believe only
> loads within the same transaction should count, and I wasn't sure
> whether you were ensuring that (and/or whether a barrier would enforce
> this either).
We should at this point not disrupt the tree by complex changes to optimizers.
You can try patching tree_could_trap_p, though I am not sure there is enough
context provided, or add a few TM checks in selected places (loop IM and PRE).
We are close to a release.
Richard.
> Torvald
>
