[all-commits] [llvm/llvm-project] 587d6f: [mlir] Recover the behavior of SliceAnaylsis for l...

[Han-Chung Wang via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 587d6fcbb685e3a57803110695a1996ac895d8b8
      https://github.com/llvm/llvm-project/commit/587d6fcbb685e3a57803110695a1996ac895d8b8
  Author: Han-Chung Wang <hanhan0912 at gmail.com>
  Date:   2025-05-29 (Thu, 29 May 2025)

  Changed paths:
    M mlir/lib/Analysis/SliceAnalysis.cpp

  Log Message:
  -----------
  [mlir] Recover the behavior of SliceAnaylsis for llvm-project at 6a8dde04a07 (#142076)

In
https://github.com/llvm/llvm-project/commit/6a8dde04a07287f837bbabeb93e23e47af366d3d,
it changes the method to return LogicalFailure, so callers can handle
the failure instead of crashing, if I read the intention correctly.
However, it changes the behavior of the implementation; it breaks
several integratino tests in downstream projects (e.g., IREE).

Before the change, processValue does not treat it as a failure if the
check below TODO has a false condition. However, with the new change, it
starts treating it as a failure.

The revision updates the final `else` branch (i.e., `llvm_unreachable`
line) to return a failure, and return success at the end; the behavior
is recovered.

```cpp
auto processValue = [&](Value value) {
  if (auto *definingOp = value.getDefiningOp()) {
    if (backwardSlice->count(definingOp) == 0)
      getBackwardSliceImpl(definingOp, backwardSlice, options);
  } else if (auto blockArg = dyn_cast<BlockArgument>(value)) {
    if (options.omitBlockArguments)
      return;

    Block *block = blockArg.getOwner();
    Operation *parentOp = block->getParentOp();
    // TODO: determine whether we want to recurse backward into the other
    // blocks of parentOp, which are not technically backward unless they flow
    // into us. For now, just bail.
    if (parentOp && backwardSlice->count(parentOp) == 0) {
      assert(parentOp->getNumRegions() == 1 &&
             llvm::hasSingleElement(parentOp->getRegion(0).getBlocks()));
      getBackwardSliceImpl(parentOp, backwardSlice, options);

    }
  } else {
    llvm_unreachable("No definingOp and not a block argument.");
  }
```

No additional tests are added, like the previous commit. This revision
is mostly a post-fix for
https://github.com/llvm/llvm-project/commit/6a8dde04a07287f837bbabeb93e23e47af366d3d

Co-authored-by: Ian Wood <ianwood2024 at u.northwestern.edu>

Signed-off-by: hanhanW <hanhan0912 at gmail.com>



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications