[MLIR] NFC. Clean up logic of hasSingleEffect

[bondhugula]

Summary

Clean up conditional logic of hasSingleEffect. NFC.

[rriddle]

mlir/lib/Interfaces/SideEffectInterfaces.cpp

99–110

If we check early for effects.empty() and return false, do we even need to track hasSingleEffectOnVal through the loop?

[bondhugula]

mlir/lib/Interfaces/SideEffectInterfaces.cpp

99–110

We would still need to --- since you could have other effects that are not on value. This method needs to check if EffectTy is the only effect that this op has *on value*. In the logic you suggested, you'd return false even whenever there is an effect on a different value.

If the desired semantics of the method had been to only restrict effects to only this value, then the above logic would implement it compactly. But that's not what is desired here. For eg., a common scenario is check whether an operation only has a "read" memory effect on a specific memref value which happens to be one or more of its operands. We aren't concerned about what other effects the op has on other value operands.

mlir/include/mlir/Interfaces/SideEffectInterfaces.h

 /// dereference or read.
 struct Write : public Effect::Base<Write> {};
 } // namespace MemoryEffects
 
 //===----------------------------------------------------------------------===//
 // SideEffect Utilities
 //===----------------------------------------------------------------------===//
 
-/// Returns true if this operation only has the given effect on `value`.
+/// Returns true if `op` has only an effect of type `EffectTy` (and of no other
+/// type) on `value`.
 template <typename EffectTy>
 bool hasSingleEffect(Operation *op, Value value);
 
 /// Return true if the given operation is unused, and has no side effects on
 /// memory that prevent erasing.
 bool isOpTriviallyDead(Operation *op);
 
 /// Return true if the given operation would be dead if unused, and has no side

mlir/lib/Interfaces/SideEffectInterfaces.cpp

 }
 
 template <typename EffectTy>
 bool mlir::hasSingleEffect(Operation *op, Value value) {
   auto memOp = dyn_cast<MemoryEffectOpInterface>(op);
   if (!memOp)
     return false;
   SmallVector<SideEffects::EffectInstance<MemoryEffects::Effect>, 4> effects;
   memOp.getEffects(effects);
-  bool doesOpOnlyHaveSingleEffectOnVal = false;
-  // Iterate through `effects` and check if and only if effect of type
-  // `EffectTy` is present.
+  bool hasSingleEffectOnVal = false;
+  // Iterate through `effects` and check if an effect of type `EffectTy` and
+  // only of that type is present.
   for (auto &effect : effects) {
-    if (effect.getValue() == value && isa<EffectTy>(effect.getEffect()))
-      doesOpOnlyHaveSingleEffectOnVal = true;
-    if (effect.getValue() == value && !isa<EffectTy>(effect.getEffect())) {
-      doesOpOnlyHaveSingleEffectOnVal = false;
-      break;
-    }
+    if (effect.getValue() != value)
+      continue;
+    hasSingleEffectOnVal = isa<EffectTy>(effect.getEffect());
+    if (!hasSingleEffectOnVal)
+      return false;
   }
-  return doesOpOnlyHaveSingleEffectOnVal;
+  return hasSingleEffectOnVal;
 }
 
 template bool mlir::hasSingleEffect<MemoryEffects::Allocate>(Operation *,
                                                              Value);
 template bool mlir::hasSingleEffect<MemoryEffects::Free>(Operation *, Value);
-template bool mlir::hasSingleEffect<MemoryEffects::Write>(Operation *, Value);
 template bool mlir::hasSingleEffect<MemoryEffects::Read>(Operation *, Value);
+template bool mlir::hasSingleEffect<MemoryEffects::Write>(Operation *, Value);
 
 bool mlir::wouldOpBeTriviallyDead(Operation *op) {
   if (op->mightHaveTrait<OpTrait::IsTerminator>())
     return false;
   return wouldOpBeTriviallyDeadImpl(op);
 }