[llvm-commits] [llvm] r167942 - /llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp

[Hal Finkel]

----- Original Message -----
> From: "Sean Silva" <silvas at purdue.edu>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, November 14, 2012 1:31:28 PM
> Subject: Re: [llvm-commits] [llvm] r167942 - /llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
> 
> > There are some remaining iterations over std::*map containers where
> > the order
> > might matter, but so long as the Value* for instructions in a block
> > increase
> > with the order of the instructions in the block (or decrease)
> > monotonically,
> > then this will appear to be deterministic.
> 
> This makes it sound like the other situations aren't worth fixing
> because of this assumption about the addresses (sorry if I
> misunderstood).

I did not mean to imply that this is not worth fixing. I plan on fixing it soon (it is just a more invasive change).

> Do we have any reason to believe that the addresses will actually be
> ordered like this?

No (unless, for example, we know that they come from a BumpPtrAllocator). In practice, however, I think it is common for the address to be ordered w.r.t. to the allocation order (which is likely to be deterministic).

 -Hal

> 
> -- Sean Silva
> 
> On Wed, Nov 14, 2012 at 1:38 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > Author: hfinkel
> > Date: Wed Nov 14 12:38:11 2012
> > New Revision: 167942
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=167942&view=rev
> > Log:
> > Fix the largest offender of determinism in BBVectorize
> >
> > Iterating over the children of each node in the potential
> > vectorization
> > plan must happen in a deterministic order (because it affects which
> > children
> > are erased when two children conflict). There was no need for this
> > data
> > structure to be a map in the first place, so replacing it with a
> > vector
> > is a small change.
> >
> > I believe that this was the last remaining instance if iterating
> > over the
> > elements of a Dense* container where the iteration order could
> > matter.
> > There are some remaining iterations over std::*map containers where
> > the order
> > might matter, but so long as the Value* for instructions in a block
> > increase
> > with the order of the instructions in the block (or decrease)
> > monotonically,
> > then this will appear to be deterministic.
> >
> > Modified:
> >     llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp?rev=167942&r1=167941&r2=167942&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp Wed Nov 14
> > 12:38:11 2012
> > @@ -1485,7 +1485,7 @@
> >        PrunedTree.insert(QTop.first);
> >
> >        // Visit each child, pruning as necessary...
> > -      DenseMap<ValuePair, size_t> BestChildren;
> > +      std::vector<ValuePairWithDepth> BestChildren;
> >        VPPIteratorPair QTopRange =
> >        ConnectedPairs.equal_range(QTop.first);
> >        for (std::multimap<ValuePair, ValuePair>::iterator K =
> >        QTopRange.first;
> >             K != QTopRange.second; ++K) {
> > @@ -1517,7 +1517,7 @@
> >          DenseSet<ValuePair> CurrentPairs;
> >
> >          bool CanAdd = true;
> > -        for (DenseMap<ValuePair, size_t>::iterator C2
> > +        for (std::vector<ValuePairWithDepth>::iterator C2
> >                = BestChildren.begin(), E2 = BestChildren.end();
> >               C2 != E2; ++C2) {
> >            if (C2->first.first == C->first.first ||
> > @@ -1602,22 +1602,22 @@
> >          // to an already-selected child. Check for this here, and
> >          if a
> >          // conflict is found, then remove the previously-selected
> >          child
> >          // before adding this one in its place.
> > -        for (DenseMap<ValuePair, size_t>::iterator C2
> > +        for (std::vector<ValuePairWithDepth>::iterator C2
> >                = BestChildren.begin(); C2 != BestChildren.end();) {
> >            if (C2->first.first == C->first.first ||
> >                C2->first.first == C->first.second ||
> >                C2->first.second == C->first.first ||
> >                C2->first.second == C->first.second ||
> >                pairsConflict(C2->first, C->first,
> >                PairableInstUsers))
> > -            BestChildren.erase(C2++);
> > +            C2 = BestChildren.erase(C2);
> >            else
> >              ++C2;
> >          }
> >
> > -        BestChildren.insert(ValuePairWithDepth(C->first,
> > C->second));
> > +        BestChildren.push_back(ValuePairWithDepth(C->first,
> > C->second));
> >        }
> >
> > -      for (DenseMap<ValuePair, size_t>::iterator C
> > +      for (std::vector<ValuePairWithDepth>::iterator C
> >              = BestChildren.begin(), E2 = BestChildren.end();
> >             C != E2; ++C) {
> >          size_t DepthF = getDepthFactor(C->first.first);
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

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