address review comments

Author: Dinistro

mlir/include/mlir/IR/SliceWalk.h

@@ -45,8 +45,7 @@ class WalkContinuation {
   }
 
   /// Creates a continuation that adds the user-specified `nextValues` to the
-  /// work list and advances the walk. Unlike advance, this function does not
-  /// add the control flow predecessor values to the work list.
+  /// work list and advances the walk.
   static WalkContinuation advanceTo(mlir::ValueRange nextValues) {
     return WalkContinuation(WalkAction::AdvanceTo, nextValues);
   }
@@ -90,8 +89,8 @@ WalkContinuation walkSlice(mlir::ValueRange rootValues,
 
 /// Computes a vector of all control predecessors of `value`. Relies on
 /// RegionBranchOpInterface and BranchOpInterface to determine predecessors.
-/// Returns nullopt if value has no predecessors or when the relevant operations
-/// are missing the interface implementations.
+/// Returns nullopt if `value` has no predecessors or when the relevant
+/// operations are missing the interface implementations.
 std::optional<SmallVector<Value>> getControlFlowPredecessors(Value value);
 
 } // namespace mlir

mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp

@@ -253,8 +253,8 @@ getUnderlyingObjectSet(Value pointerValue) {
     }
 
     // If this place is reached, `val` is a block argument that is not
-    // understood. Therfore, we conservatively interrupt.
-    // Note: Dealing with the function arguments is not necessary, as the slice
+    // understood. Therefore, we conservatively interrupt.
+    // Note: Dealing with function arguments is not necessary, as the slice
     // would have to go through an SSACopyOp first.
     return WalkContinuation::interrupt();
   });

mlir/lib/IR/SliceWalk.cpp

@@ -78,9 +78,8 @@ getRegionPredecessorOperands(RegionBranchOpInterface regionOp,
   return predecessorOperands;
 }
 
-/// Returns the predecessor branch operands that match `blockArg`. Returns a
-/// nullopt when some of the predecessor terminators do not implement the
-/// BranchOpInterface.
+/// Returns the predecessor branch operands that match `blockArg`, or nullopt if
+/// some of the predecessor terminators do not implement the BranchOpInterface.
 static std::optional<SmallVector<Value>>
 getBlockPredecessorOperands(BlockArgument blockArg) {
   Block *block = blockArg.getOwner();
@@ -130,8 +129,8 @@ mlir::getControlFlowPredecessors(Value value) {
           regionBranchOp, region, blockArg.getArgNumber());
       return predecessorOperands;
     }
-    // Unclear how to deal with this operation, conservatively return a
-    // failure.
+    // If the interface is not implemented, there are no control flow
+    // predecessors to work with.
     return std::nullopt;
   }