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

[Hal Finkel]

----- Original Message -----
> From: "Chandler Carruth" <chandlerc at google.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Wednesday, November 14, 2012 3:20:21 PM
> Subject: Re: [llvm-commits] [llvm] r167942 - /llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
> 
> On Wed, Nov 14, 2012 at 10:38 AM, 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.
> 
> Pointers should not be assumed to be deterministically ordered. Any
> ordering which is based on the numerical sort of addresses is a
> non-determinism bug.

Agreed. As I said to Sean just a few minutes ago:

Upon further review, I think that the current output is now deterministic so long as the ordering of std::multimap is stable. This is not guaranteed by the C++03 standard (but, as used in BBVectorize, is this way in all known implementations, see n1780), but is guaranteed by the C++11 standard.

http://stackoverflow.com/questions/2643473/does-stdmultiset-guarantee-insertion-order?rq=1
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1780.html  

I should probably get rid of the std::multimap use anyway, although I'm not sure what to replace it with. DenseMap<K, SmallVector<V, N>> perhaps?

 -Hal

> 

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