[llvm-commits] [PATCH] Allow SelectionDAGBuilder to reorder loads past stores

[Hal Finkel]

On Wed, 2012-01-11 at 18:45 -0800, Andrew Trick wrote:
> On Dec 19, 2011, at 9:42 AM, Hal Finkel wrote:
> 
> > The current SelectionDAGBuilder does not allow loads to be reordered
> > past stores, and does not allow stores to be reordered. This is a side
> > effect of the way the critical chain is constructed: there is a queue of
> > pending loads that is flushed (in parallel) to the root of the chain
> > upon encountering any store (and that store is also appended to the root
> > of the chain). Among other things, loop unrolling is far less effective
> > than it otherwise could be.
> > 
> > The attached patch allows SelectionDAGBuilder to use the available alias
> > analysis to reorder independent loads and stores. It changes the queue
> > of pending loads into a more general queue of pending memory operations,
> > and flushes, in parallel, all potentially-conflicting loads and stores
> > as necessary.
> 
> Hal,
> 
> I haven't tried out your patch, so correct me if I'm wrong. I don't
> understand how this can work with a simple alias analysis interface
> like we have in LLVM.
> 
> Root
> ld A
> st B
> st C
> 
> mayAlias(A,B) = true
> mayAlias(A,C) = true
> mayAlias(B,C) = false

This is a good point, your situation is something like this:

-------------------
|        A        |
|    B   |   C    |
-------------------

and without strict aliasing rules, this could certainly be an issue.

> 
> So we have the chain Root->A->B and Root->C which may result in the schedule:
> 
> st C
> ld A
> st B
> 
> It seems that we have to order all stores if we want them to "cover" preceding loads. What am I missing?
> 

In the patch, aliasing loads and stores still set the current root to
their combined token factor. As a result, the reordering will never be
as severe as in your example; the patch is safe in this particular case
because the DAG root will be set to A once B is visited and so C will
also be forced to come after A. I'll need to give your point further
thought, however, because maybe this kind of thing can bite in some
other way.

We may need to build an aliasing set per chain in order to check for
this kind of thing.

Thanks again,
Hal

> -Andy

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory