[llvm-commits] [llvm] r167942 - /llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
Sean Silva
silvas at purdue.edu
Wed Nov 14 11:45:16 PST 2012
> 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).
Ah, ok.
> 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).
I thought this too when I was looking at some parts of TableGen, but
it can actually prove to be false. BumpPtrAllocator does allocate
sequentially, but only within each block (which is not *that* big,
IIRC 4K by default; easily exceeded). It ultimately falls back on
malloc() when it needs to allocate the blocks, and malloc() can return
the blocks out of order.
-- Sean Silva
On Wed, Nov 14, 2012 at 2:40 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- 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
More information about the llvm-commits
mailing list