[SROA] Fix non-determinism caused by differing argument evaluation order

[dstenb]

Summary

When running the provided test file with opt binaries built with gcc 5.3 respectively clang 3.9, the order of the extractvalue and gep instructions differs.

[patrik.h.hagglund]

LGTM.

[chandlerc]

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

3113–3117

The formatting of this code is pretty egregiously outside of the standards for LLVM code (and this file). Please run clang-format over patches?

[dstenb]

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

3113–3117

Yes, sorry about that! Mea culpa. It is fixed in r273149.

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

struct StoreOpSplitter : public OpSplitter<StoreOpSplitter> {
     StoreOpSplitter(Instruction *InsertionPoint, Value *Ptr)
         : OpSplitter<StoreOpSplitter>(InsertionPoint, Ptr) {}
 
     /// Emit a leaf store of a single value. This is called at the leaves of the
     /// recursive emission to actually produce stores.
     void emitFunc(Type *Ty, Value *&Agg, const Twine &Name) {
       assert(Ty->isSingleValueType());
       // Extract the single value and store it using the indices.
-      Value *Store = IRB.CreateStore(
-          IRB.CreateExtractValue(Agg, Indices, Name + ".extract"),
-          IRB.CreateInBoundsGEP(nullptr, Ptr, GEPIndices, Name + ".gep"));
+      //
+      // The gep and extractvalue values are factored out of the CreateStore
+      // call to make the output independent of the argument evaluation order.
+      Value *ExtractValue = IRB.CreateExtractValue(Agg, Indices,
+        Name + ".extract");
+      Value *InBoundsGEP = IRB.CreateInBoundsGEP(nullptr, Ptr, GEPIndices,
+        Name + ".gep");
+      Value *Store = IRB.CreateStore(ExtractValue, InBoundsGEP);
       (void)Store;
       DEBUG(dbgs() << "          to: " << *Store << "\n");
     }
   };
 
   bool visitStoreInst(StoreInst &SI) {
     if (!SI.isSimple() || SI.getPointerOperand() != *U)
       return false;