[Mlir-commits] [mlir] [MLIR][SideEffects][MemoryEffects] Modified LICM to be more aggressive when checking movability of ops with MemWrite effects (PR #155344)

[Mehdi Amini]

================
@@ -338,9 +379,101 @@ bool mlir::isMemoryEffectFree(Operation *op) {
     for (Operation &op : region.getOps())
       if (!isMemoryEffectFree(&op))
         return false;
+
   return true;
 }
 
+bool mlir::isMemoryEffectConflictFree(Operation *op) {
+  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) return false;
+
+  // check parent loop to make sure it's not dead
+  Operation *parent = op->getParentOp();
+  if (!parent)
+    return false;
+
+  auto loopInterface = dyn_cast<LoopLikeOpInterface>(parent);
+  if (!loopInterface)
+    return false;
+
+  auto isDead = isZeroTrip(loopInterface);
+  if (!isDead.has_value() || isDead.value())
+    return false;
+
+  // gather all effects on op
+  llvm::SmallVector<MemoryEffects::EffectInstance> effects;
+  memInterface.getEffects(effects);
+
+  // op has interface but no effects, be conservative
+  if (effects.empty()) return false;
+
+  DenseMap<TypeID, int> resourceCounts;
+
+  // ensure op only has Write effects and gather unique
+  // resource names
+  for (const MemoryEffects::EffectInstance &effect : effects) {
+    if (!isa<MemoryEffects::Write>(effect.getEffect()))
+      return false;
+
+    resourceCounts.try_emplace(effect.getResource()->getResourceID(), 0);
+  }
+
+  mlir::DominanceInfo dom(parent);
+
+  for (Region &region : parent->getRegions())
+    for (Operation &opI : region.getOps())
+      if (hasMemoryEffectConflict(op, &opI, dom, resourceCounts))
+        return false;
+
+  return true;
+}
+
+bool mlir::hasMemoryEffectConflict(
+    Operation *mainOp, Operation *op,
+    mlir::DominanceInfo &dom, DenseMap<TypeID, int> &resourceCounts) {
+
+  if (auto memInterface = dyn_cast<MemoryEffectOpInterface>(op)) {
+    
+    llvm::SmallVector<MemoryEffects::EffectInstance> effects;
+    memInterface.getEffects(effects);
+
+    auto isDominated = dom.properlyDominates(mainOp, op);
----------------
joker-eph wrote:

```suggestion
    bool isDominated = dom.properlyDominates(mainOp, op);
```

Nit: reserve auto to the cases where it clearly improves the readability (types that are very long and unambiguous in the context)

https://github.com/llvm/llvm-project/pull/155344