[ScheduleDAGInstrs] Re-factor for based on review feedback. NFC.

[gberry]

Summary

Re-factor some code to improve clarity and style based on review
comments from http://reviews.llvm.org/D18093.

[mcrosier]

Nifty. LGTM.

include/llvm/CodeGen/ScheduleDAGInstrs.h

namespace llvm {
   /// builder and may be consulted by the scheduler to avoid iterating an entire
   /// vreg use list.
   typedef SparseMultiSet<VReg2SUnit, VirtReg2IndexFunctor> VReg2SUnitMultiMap;
 
   typedef SparseMultiSet<VReg2SUnitOperIdx, VirtReg2IndexFunctor>
     VReg2SUnitOperIdxMultiMap;
 
   typedef PointerUnion<const Value *, const PseudoSourceValue *> ValueType;
-  typedef SmallVector<PointerIntPair<ValueType, 1, bool>, 4>
-          UnderlyingObjectsVector;
+  struct UnderlyingObject : PointerIntPair<ValueType, 1, bool> {
+    UnderlyingObject(ValueType V, bool MayAlias)
+        : PointerIntPair<ValueType, 1, bool>(V, MayAlias) {}
+    ValueType getValue() const { return getPointer(); }
+    bool mayAlias() const { return getInt(); }
+  };
+  typedef SmallVector<UnderlyingObject, 4> UnderlyingObjectsVector;
 
   /// ScheduleDAGInstrs - A ScheduleDAG subclass for scheduling lists of
   /// MachineInstrs.
   class ScheduleDAGInstrs : public ScheduleDAG {
   protected:
     const MachineLoopInfo *MLI;
     const MachineFrameInfo *MFI;
 

lib/CodeGen/ScheduleDAGInstrs.cpp

 
 /// getUnderlyingObjectsForInstr - If this machine instr has memory reference
 /// information and it can be tracked to a normal reference to a known
 /// object, return the Value for that object.
 static void getUnderlyingObjectsForInstr(const MachineInstr *MI,
                                          const MachineFrameInfo *MFI,
                                          UnderlyingObjectsVector &Objects,
                                          const DataLayout &DL) {
-  for (auto *MMO : MI->memoperands()) {
-    if (MMO->isVolatile()) {
-      Objects.clear();
-      return;
-    }
+  auto allMMOsOkay = [&]() {
+    for (const MachineMemOperand *MMO : MI->memoperands()) {
+      if (MMO->isVolatile())
+        return false;
 
       if (const PseudoSourceValue *PSV = MMO->getPseudoValue()) {
         // Function that contain tail calls don't have unique PseudoSourceValue
-      // objects. Two PseudoSourceValues might refer to the same or overlapping
-      // locations. The client code calling this function assumes this is not the
-      // case. So return a conservative answer of no known object.
-      if (MFI->hasTailCall()) {
-        Objects.clear();
-        return;
-      }
+        // objects. Two PseudoSourceValues might refer to the same or
+        // overlapping locations. The client code calling this function assumes
+        // this is not the case. So return a conservative answer of no known
+        // object.
+        if (MFI->hasTailCall())
+          return false;
 
         // For now, ignore PseudoSourceValues which may alias LLVM IR values
         // because the code that uses this function has no way to cope with
         // such aliases.
-      if (PSV->isAliased(MFI)) {
-        Objects.clear();
-        return;
-      }
+        if (PSV->isAliased(MFI))
+          return false;
 
         bool MayAlias = PSV->mayAlias(MFI);
         Objects.push_back(UnderlyingObjectsVector::value_type(PSV, MayAlias));
       } else if (const Value *V = MMO->getValue()) {
         SmallVector<Value *, 4> Objs;
         getUnderlyingObjects(V, Objs, DL);
 
         for (Value *V : Objs) {
-        if (!isIdentifiedObject(V)) {
-          Objects.clear();
-          return;
-        }
+          if (!isIdentifiedObject(V))
+            return false;
 
           Objects.push_back(UnderlyingObjectsVector::value_type(V, true));
         }
-    } else {
-      Objects.clear();
-      return;
-    }
+      } else
+        return false;
     }
+    return true;
+  };
+
+  if (!allMMOsOkay())
+    Objects.clear();
 }
 
 void ScheduleDAGInstrs::startBlock(MachineBasicBlock *bb) {
   BB = bb;
 }
 
 void ScheduleDAGInstrs::finishBlock() {
   // Subclasses should no longer refer to the old block.
if (MI->mayStore()) {
         addChainDependencies(SU, Loads);
         addChainDependencies(SU, NonAliasLoads);
 
         // Map this store to 'UnknownValue'.
         Stores.insert(SU, UnknownValue);
       } else {
         // Add precise dependencies against all previously seen memory
         // accesses mapped to the same Value(s).
-        for (auto &underlObj : Objs) {
-          ValueType V = underlObj.getPointer();
-          bool ThisMayAlias = underlObj.getInt();
-
-          Value2SUsMap &stores_ = (ThisMayAlias ? Stores : NonAliasStores);
+        for (const UnderlyingObject &UnderlObj : Objs) {
+          ValueType V = UnderlObj.getValue();
+          bool ThisMayAlias = UnderlObj.mayAlias();
 
           // Add dependencies to previous stores and loads mapped to V.
-          addChainDependencies(SU, stores_, V);
+          addChainDependencies(SU, (ThisMayAlias ? Stores : NonAliasStores), V);
           addChainDependencies(SU, (ThisMayAlias ? Loads : NonAliasLoads), V);
         }
         // Update the store map after all chains have been added to avoid adding
         // self-loop edge if multiple underlying objects are present.
-        for (auto &underlObj : Objs) {
-          ValueType V = underlObj.getPointer();
-          bool ThisMayAlias = underlObj.getInt();
-
-          Value2SUsMap &stores_ = (ThisMayAlias ? Stores : NonAliasStores);
+        for (const UnderlyingObject &UnderlObj : Objs) {
+          ValueType V = UnderlObj.getValue();
+          bool ThisMayAlias = UnderlObj.mayAlias();
 
           // Map this store to V.
-          stores_.insert(SU, V);
+          (ThisMayAlias ? Stores : NonAliasStores).insert(SU, V);
         }
         // The store may have dependencies to unanalyzable loads and
         // stores.
         addChainDependencies(SU, Loads, UnknownValue);
         addChainDependencies(SU, Stores, UnknownValue);
       }
     } else { // SU is a load.
       if (Objs.empty()) {
         // An unknown load depends on all stores.
         addChainDependencies(SU, Stores);
         addChainDependencies(SU, NonAliasStores);
 
         Loads.insert(SU, UnknownValue);
       } else {
-        for (auto &underlObj : Objs) {
-          ValueType V = underlObj.getPointer();
-          bool ThisMayAlias = underlObj.getInt();
+        for (const UnderlyingObject &UnderlObj : Objs) {
+          ValueType V = UnderlObj.getValue();
+          bool ThisMayAlias = UnderlObj.mayAlias();
 
           // Add precise dependencies against all previously seen stores
           // mapping to the same Value(s).
           addChainDependencies(SU, (ThisMayAlias ? Stores : NonAliasStores), V);
 
           // Map this load to V.
           (ThisMayAlias ? Loads : NonAliasLoads).insert(SU, V);
         }