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

[Aldy Hernandez <aldyh at redhat dot com>]

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?

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

> 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