Re: [PR51752] publication safety violations in loop invariant motion pass

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Feb 24, 2012 at 5:34 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 02/24/12 07:10, Torvald Riegel wrote:
>>
>> On Fri, 2012-02-24 at 09:58 +0100, Richard Guenther wrote:
>>>
>>> On Thu, Feb 23, 2012 at 10:11 PM, Aldy Hernandez<aldyh@redhat.com>
>>> ?wrote:
>>>>
>>>> On 02/23/12 12:19, Aldy Hernandez wrote:
>>>>
>>>>> about hit me. Instead now I save all loads in a function and iterate
>>>>> through them in a brute force way. I'd like to rewrite this into a hash
>>>>> of some sort, but before I go any further I'm interested to know if the
>>>>> main idea is ok.
>>>>
>>>>
>>>>
>>>> For the record, it may be ideal to reuse some of the iterations we
>>>> already
>>>> do over the function's basic blocks, so we don't have to iterate yet
>>>> again
>>>> over the IL stream. ?Though it may complicate the pass unnecessarily.
>>>
>>>
>>> It's definitely ugly that you need to do this.
>>
>>
>> Indeed. ?But that's simply the price we have to pay for making
>> publication with transactions easier for the programmer yet still at
>> acceptable performance.
>>
>> Also note that what we primarily have to care about for this PR is to
>> not hoist loads _within_ transactions if they would violate publication
>
>
> For that matter, didn't rth add a memory barrier at the beginning of
> transactions last week? ?That would mean that we can't hoist anything
> outside of a transaction anyhow. ?Or was it not a full memory barrier?

It's now a full memory barrier for all global memory and for local statics
if their address is taken (and for automatic vars with their address taken).
Do we need to be concerned about non-address-taken local statics?

>
>> 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.

Yes, simply handle memory accesses inside a transaction as if they may
trap.  Then all optimizers should be fine (or they are buggy even without
TM).

Richard.

>
>>
>> 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.
>
> Aldy