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

Geoff Berry via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 14 14:31:08 PDT 2016


Author: gberry
Date: Thu Apr 14 16:31:07 2016
New Revision: 266372

URL: http://llvm.org/viewvc/llvm-project?rev=266372&view=rev
Log:
[ScheduleDAGInstrs] Re-factor for based on review feedback. NFC.

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

Reviewers: MatzeB, mcrosier

Subscribers: MatzeB, mcrosier, llvm-commits

Differential Revision: http://reviews.llvm.org/D19128

Modified:
    llvm/trunk/include/llvm/CodeGen/ScheduleDAGInstrs.h
    llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp

Modified: llvm/trunk/include/llvm/CodeGen/ScheduleDAGInstrs.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/ScheduleDAGInstrs.h?rev=266372&r1=266371&r2=266372&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/ScheduleDAGInstrs.h (original)
+++ llvm/trunk/include/llvm/CodeGen/ScheduleDAGInstrs.h Thu Apr 14 16:31:07 2016
@@ -87,8 +87,13 @@ namespace llvm {
     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.

Modified: llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp?rev=266372&r1=266371&r2=266372&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp (original)
+++ llvm/trunk/lib/CodeGen/ScheduleDAGInstrs.cpp Thu Apr 14 16:31:07 2016
@@ -159,49 +159,46 @@ static void getUnderlyingObjectsForInstr
                                          const MachineFrameInfo *MFI,
                                          UnderlyingObjectsVector &Objects,
                                          const DataLayout &DL) {
-  for (auto *MMO : MI->memoperands()) {
-    if (MMO->isVolatile()) {
-      Objects.clear();
-      return;
-    }
-
-    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;
-      }
-
-      // 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;
-      }
-
-      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;
-        }
+  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())
+          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))
+          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))
+            return false;
 
-        Objects.push_back(UnderlyingObjectsVector::value_type(V, true));
-      }
-    } else {
-      Objects.clear();
-      return;
+          Objects.push_back(UnderlyingObjectsVector::value_type(V, true));
+        }
+      } else
+        return false;
     }
-  }
+    return true;
+  };
+
+  if (!allMMOsOkay())
+    Objects.clear();
 }
 
 void ScheduleDAGInstrs::startBlock(MachineBasicBlock *bb) {
@@ -1031,26 +1028,22 @@ void ScheduleDAGInstrs::buildSchedGraph(
       } 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.
@@ -1065,9 +1058,9 @@ void ScheduleDAGInstrs::buildSchedGraph(
 
         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).




More information about the llvm-commits mailing list