fixed formatting

Author: mbagherbeikTT

mlir/include/mlir/Interfaces/SideEffectInterfaces.h

@@ -348,11 +348,11 @@ struct AlwaysSpeculatableImplTrait
 
 namespace MemoryEffects {
 /// Defines the priority of the different memory effects.
-/// 
+///
 /// Sorting/ordering memory effects of an operation is done based on
-/// their defined stage and priority, in that order. If stage values for two effect instances are
-/// equal, they are then sorted by priority. Lower priority values indicate higher
-/// precedence.
+/// their defined stage and priority, in that order. If stage values for two
+/// effect instances are equal, they are then sorted by priority. Lower priority
+/// values indicate higher precedence.
 enum Priority {
   DefaultPriority = 0,
   AllocPriority = 1,
@@ -389,7 +389,9 @@ getMemoryEffectsSorted(Operation *op);
 /// resource. An 'allocate' effect implies only allocation of the resource, and
 /// not any visible mutation or dereference.
 struct Allocate : public Effect::Base<Allocate> {
-  Allocate() : Effect::Base<Allocate>() { this->priority = Priority::AllocPriority; }
+  Allocate() : Effect::Base<Allocate>() {
+    this->priority = Priority::AllocPriority;
+  }
 };
 
 /// The following effect indicates that the operation frees some resource that

mlir/include/mlir/Transforms/LoopInvariantCodeMotionUtils.h

@@ -25,16 +25,17 @@ class Value;
 
 /// Gathers potential conflicts on all memory resources used within loop.
 ///
-/// Given a target loop and an op within it (or the loop op itself), 
-/// gathers op's memory effects and flags potential resource conflicts 
-/// in a map and then recurses into the op's regions to gather nested 
-/// resource conflicts. 
+/// Given a target loop and an op within it (or the loop op itself),
+/// gathers op's memory effects and flags potential resource conflicts
+/// in a map and then recurses into the op's regions to gather nested
+/// resource conflicts.
 ///
 /// Typical usage:
 /// \code
 ///   LoopLikeOpInterface myLoop = ...;
-///   DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> myConflicts;
-///   gatherResourceConflicts(myLoop, myLoop.getOperation(), resourceConflicts);
+///   DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+///   myConflicts; gatherResourceConflicts(myLoop, myLoop.getOperation(),
+///   resourceConflicts);
 /// \endcode
 ///
 /// \param loop The loop to gather resource conflicts for.
@@ -44,15 +45,17 @@ class Value;
 /// Key is the resource ID that effects are applied to. Value is a pair of
 /// a boolean, indicating if the resource has a conflict, and the last effect
 /// that was applied to the resource (if no conflicts exist) or the effect
-/// that caused the conflict (if conflicts exist). This was done so we 
+/// that caused the conflict (if conflicts exist). This was done so we
 /// check the effect that causes the conflict for debugging purposes.
 /// First call should use loop = someLoop and op = someLoop.getOperation()
 ///
 /// resourceConflicts is modified by the function and will be non-empty
 /// as long as there are memory effects within the loop, even if there are
 /// no conflicts.
-void gatherResourceConflicts(LoopLikeOpInterface loop, Operation *op,
-    DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> &resourceConflicts);
+void gatherResourceConflicts(
+    LoopLikeOpInterface loop, Operation *op,
+    DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+        &resourceConflicts);
 
 /// Given a list of regions, perform loop-invariant code motion. An operation is
 /// loop-invariant if it depends only of values defined outside of the loop.

mlir/lib/Interfaces/SideEffectInterfaces.cpp

@@ -330,19 +330,19 @@ mlir::MemoryEffects::getMemoryEffectsSorted(Operation *op) {
 
   memInterface.getEffects(effectsSorted);
 
-  auto sortEffects = 
-    [](llvm::SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
-    llvm::stable_sort(effects, [](const MemoryEffects::EffectInstance &a,
-                                  const MemoryEffects::EffectInstance &b) {
-      if (a.getStage() < b.getStage())
-        return true;
-      
-      if (a.getStage() == b.getStage())
-        return a.getEffect()->getPriority() < b.getEffect()->getPriority();
-
-      return false; // b before a
-    });
-  };
+  auto sortEffects =
+      [](llvm::SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+        llvm::stable_sort(effects, [](const MemoryEffects::EffectInstance &a,
+                                      const MemoryEffects::EffectInstance &b) {
+          if (a.getStage() < b.getStage())
+            return true;
+
+          if (a.getStage() == b.getStage())
+            return a.getEffect()->getPriority() < b.getEffect()->getPriority();
+
+          return false; // b before a
+        });
+      };
   sortEffects(effectsSorted);
 
   return effectsSorted;
@@ -352,12 +352,12 @@ bool mlir::isMemoryEffectFree(Operation *op) {
   if (auto memInterface = dyn_cast<MemoryEffectOpInterface>(op)) {
     if (!memInterface.hasNoEffect())
       return false;
-    
+
     // If the op does not have recursive side effects, then it is memory effect
     // free.
     if (!op->hasTrait<OpTrait::HasRecursiveMemoryEffects>())
       return true;
-    
+
   } else if (!op->hasTrait<OpTrait::HasRecursiveMemoryEffects>()) {
     // Otherwise, if the op does not implement the memory effect interface and
     // it does not have recursive side effects, then it cannot be known that the

mlir/lib/Transforms/Utils/LoopInvariantCodeMotionUtils.cpp

@@ -60,33 +60,35 @@ static bool canBeHoisted(Operation *op,
       op, [&](OpOperand &operand) { return definedOutside(operand.get()); });
 }
 
-/// Merges srcEffect's Memory Effect on its resource into the 
+/// Merges srcEffect's Memory Effect on its resource into the
 /// resourceConflicts map, flagging the resource if the srcEffect
 /// results in a conflict.
 ///
 /// \param resourceConflicts The map to store resources' conflicts status.
 /// \param srcEffect The effect to merge into the resourceConflicts map.
-/// \param srcHasConflict Whether the srcEffect results in a conflict based 
+/// \param srcHasConflict Whether the srcEffect results in a conflict based
 /// on higher level analysis.
 ///
 /// resourceConflicts is modified by the function and will be non-empty
-static void mergeResource(
-  DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> &resourceConflicts,
-  const MemoryEffects::EffectInstance &srcEffect,
-  bool srcHasConflict) {
+static void
+mergeResource(DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+                  &resourceConflicts,
+              const MemoryEffects::EffectInstance &srcEffect,
+              bool srcHasConflict) {
 
   TypeID srcResourceID = srcEffect.getResource()->getResourceID();
 
-  bool srcIsAllocOrFree = isa<MemoryEffects::Allocate>(srcEffect.getEffect())
-    || isa<MemoryEffects::Free>(srcEffect.getEffect());
+  bool srcIsAllocOrFree = isa<MemoryEffects::Allocate>(srcEffect.getEffect()) ||
+                          isa<MemoryEffects::Free>(srcEffect.getEffect());
 
   bool conflict = srcHasConflict || srcIsAllocOrFree;
 
   auto dstIt = resourceConflicts.find(srcResourceID);
 
   // if it doesn't already exist, create entry for resource in map
   if (dstIt == resourceConflicts.end()) {
-    resourceConflicts.insert(std::make_pair(srcResourceID, std::make_pair(conflict, srcEffect)));
+    resourceConflicts.insert(
+        std::make_pair(srcResourceID, std::make_pair(conflict, srcEffect)));
     return;
   }
 
@@ -100,10 +102,10 @@ static void mergeResource(
   bool srcWrite = isa<MemoryEffects::Write>(srcEffect.getEffect());
   bool dstRead = isa<MemoryEffects::Read>(dstEffect.getEffect());
   bool readBeforeWrite = dstRead && srcWrite;
-  
+
   conflict = conflict || readBeforeWrite;
 
-  dstIt->second =std::make_pair(conflict, srcEffect);
+  dstIt->second = std::make_pair(conflict, srcEffect);
 }
 
 /// Returns true if any of op's OpOperands are defined outside of loopLike
@@ -120,22 +122,24 @@ static bool hasLoopVariantInput(LoopLikeOpInterface loopLike, Operation *op) {
 /// flagged as having a conflict within the resourceConflicts map OR
 /// (b) op doesn't have a MemoryEffectOpInterface or has one but
 /// without any specific effects.
-static bool mayHaveMemoryEffectConflict(Operation *op,
-  DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> &resourceConflicts) {
+static bool mayHaveMemoryEffectConflict(
+    Operation *op,
+    DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+        &resourceConflicts) {
 
   auto memInterface = dyn_cast<MemoryEffectOpInterface>(op);
-  
+
   // op does not implement the memory effect op interface
   // shouldn't be flagged as movable to be conservative
-  if (!memInterface) 
+  if (!memInterface)
     return true;
 
   // gather all effects on op
   llvm::SmallVector<MemoryEffects::EffectInstance> effects;
   memInterface.getEffects(effects);
 
   // op has interface but no effects, be conservative
-  if (effects.empty()) 
+  if (effects.empty())
     return true;
 
   // RFC moving ops with HasRecursiveMemoryEffects that have nested ops
@@ -145,10 +149,10 @@ static bool mayHaveMemoryEffectConflict(Operation *op,
   for (const MemoryEffects::EffectInstance &effect : effects) {
     auto resourceID = effect.getResource()->getResourceID();
 
-    auto resConIt = resourceConflicts.find(resourceID); 
+    auto resConIt = resourceConflicts.find(resourceID);
     if (resConIt == resourceConflicts.end())
       return true; // RFC realistically shouldn't reach here but just in case?
-    
+
     bool hasConflict = resConIt->second.first;
     if (hasConflict)
       return true;
@@ -157,13 +161,15 @@ static bool mayHaveMemoryEffectConflict(Operation *op,
   return false;
 }
 
-void mlir::gatherResourceConflicts(LoopLikeOpInterface loopLike, Operation *op,
-  DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> &resourceConflicts) {
+void mlir::gatherResourceConflicts(
+    LoopLikeOpInterface loopLike, Operation *op,
+    DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+        &resourceConflicts) {
 
   if (auto memInterface = dyn_cast<MemoryEffectOpInterface>(op)) {
     // gather all effects on op
     SmallVector<MemoryEffects::EffectInstance> effects =
-      MemoryEffects::getMemoryEffectsSorted(op);
+        MemoryEffects::getMemoryEffectsSorted(op);
 
     if (!effects.empty()) {
       // any variant input to the op could be the data source
@@ -173,24 +179,24 @@ void mlir::gatherResourceConflicts(LoopLikeOpInterface loopLike, Operation *op,
       for (const MemoryEffects::EffectInstance &effect : effects) {
         bool conflict = false;
         bool isWrite = isa<MemoryEffects::Write>(effect.getEffect());
-        
+
         // all writes to a resource that follow a read on any other resource
         // have to be considered a conflict as guaranteeing that the read
         // is invariant and won't affect the write requires more robust logic
         if (isa<MemoryEffects::Read>(effect.getEffect()))
           writesConflict = true;
 
         if (isWrite && writesConflict)
-            conflict = true;
+          conflict = true;
 
         mergeResource(resourceConflicts, effect, conflict);
       }
     }
   }
 
   for (Region &region : op->getRegions())
-    for (Operation &opInner : region.getOps()) 
-        gatherResourceConflicts(loopLike, &opInner, resourceConflicts);
+    for (Operation &opInner : region.getOps())
+      gatherResourceConflicts(loopLike, &opInner, resourceConflicts);
 }
 
 size_t mlir::moveLoopInvariantCode(
@@ -213,7 +219,8 @@ size_t mlir::moveLoopInvariantCode(
   // continuous region --> need to add fork checking.
   //
   // loop "do" and "then" regions also merged.
-  DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>> resourceConflicts;
+  DenseMap<TypeID, std::pair<bool, MemoryEffects::EffectInstance>>
+      resourceConflicts;
   gatherResourceConflicts(loopLike, loopLike.getOperation(), resourceConflicts);
 
   auto regions = loopLike.getLoopRegions();
@@ -239,12 +246,12 @@ size_t mlir::moveLoopInvariantCode(
       LDBG() << "Checking op: "
              << OpWithFlags(op, OpPrintingFlags().skipRegions());
 
-      bool noMemoryConflicts = isMemoryEffectFree(op) 
-        || !mayHaveMemoryEffectConflict(op, resourceConflicts);
-        
-      if (!noMemoryConflicts
-        || !shouldMoveOutOfRegion(op, region)
-        || !canBeHoisted(op, definedOutside))
+      bool noMemoryConflicts =
+          isMemoryEffectFree(op) ||
+          !mayHaveMemoryEffectConflict(op, resourceConflicts);
+
+      if (!noMemoryConflicts || !shouldMoveOutOfRegion(op, region) ||
+          !canBeHoisted(op, definedOutside))
         continue;
 
       LDBG() << "Moving loop-invariant op: " << *op;
@@ -268,9 +275,7 @@ size_t mlir::moveLoopInvariantCode(LoopLikeOpInterface loopLike) {
       [&](Value value, Region *) {
         return loopLike.isDefinedOutsideOfLoop(value);
       },
-      [&](Operation *op, Region *) {
-        return isSpeculatable(op);
-      },
+      [&](Operation *op, Region *) { return isSpeculatable(op); },
       [&](Operation *op, Region *) { loopLike.moveOutOfLoop(op); });
 }