Repository: llvm/llvm-project
File: mlir/lib/Transforms/RemoveDeadValues.cpp (C++)

Code before:
    LDBG() << "Function is public or external, skipping: "
           << funcOp.getOperation()->getName();
    return;
  }

  // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`.
  SmallVector<Value> arguments(funcOp.getArguments());
  BitVector nonLiveArgs = markLives(arguments, nonLiveSet, la);
  nonLiveArgs = nonLiveArgs.flip();

  // Do (1).
  for (auto [index, arg] : llvm::enumerate(arguments))
    if (arg && nonLiveArgs[index])
      nonLiveSet.insert(arg);

  // Do (2). (Skip creating generic operand cleanup entries for call ops.
  // Call arguments will be removed in the call-site specific segment-aware
  // cleanup, avoiding generic eraseOperands bitvector mechanics.)
  SymbolTable::UseRange uses = *funcOp.getSymbolUses(module);
  for (SymbolTable::SymbolUse use : uses) {
    Operation *callOp = use.getUser();
    // If a non-call operation references the function (e.g. spirv.EntryPoint),
    // we cannot safely remove arguments or return values since we don't know
    // what the user expects. Skip this function entirely.
    if (!isa<CallOpInterface>(callOp))
      return;
    // Push an empty operand cleanup entry so that call-site specific logic in
    // cleanUpDeadVals runs (it keys off CallOpInterface). The BitVector is
    // intentionally all false to avoid generic erasure.
    // Store the funcOp as the callee to avoid expensive symbol lookup later.
    cl.operands.push_back({callOp, BitVector(callOp->getNumOperands(), false),
                           funcOp.getOperation()});
  }

  // Do (3).
  // Get the list of unnecessary terminator operands (return values that are
  // non-live across all callers) in `nonLiveRets`. There is a very important
  // subtlety here. Unnecessary terminator operands are NOT the operands of the
  // terminator that are non-live. Instead, these are the return values of the
  // callers such that a given return value is non-live across all callers. Such
  // corresponding operands in the terminator could be live. An example to
  // demonstrate this:
  //  func.func private @f(%arg0: memref<i32>) -> (i32, i32) {
  //    %c0_i32 = arith.constant 0 : i32
  //    %0 = arith.addi %c0_i32, %c0_i32 : i32
  //    memref.store %0, %arg0[] : memref<i32>
  //    return %c0_i32, %0 : i32, i32
  //  }
  //  func.func @main(%arg0: i32, %arg1: memref<i32>) -> (i32) {
  //    %1:2 = call @f(%arg1) : (memref<i32>) -> i32
  //    return %1#0 : i32

Review comment (refactor):
This could happen in the second iteration of the loop. From the comment, it sounds like the statement below (`cl.operands.push_back({callOp, BitVector(callOp>getNumOperands(), false), funcOp.getOperation()});`) would be a no-op, but I'm not entirely sure.

You may want to duplicate the loop, to exit before pushing anything to `cl.operands`. (Or convince yourself that the current implementation is really safe.)


Code after:
    LDBG() << "Function is public or external, skipping: "
           << funcOp.getOperation()->getName();
    return;
  }
  SymbolTable::UseRange uses = *funcOp.getSymbolUses(module);
  if (llvm::any_of(uses, [](SymbolTable::SymbolUse use) {
        return !isa<CallOpInterface>(use.getUser());
      })) {
    // If a non-call operation references the function (e.g. spirv.EntryPoint),
    // we cannot safely remove arguments or return values since we don't know
    // what the user expects. Skip this function entirely.
    return;
  }
  // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`.
  SmallVector<Value> arguments(funcOp.getArguments());
  BitVector nonLiveArgs = markLives(arguments, nonLiveSet, la);
  nonLiveArgs = nonLiveArgs.flip();

  // Do (1).
  for (auto [index, arg] : llvm::enumerate(arguments))
    if (arg && nonLiveArgs[index])
      nonLiveSet.insert(arg);

  // Do (2). (Skip creating generic operand cleanup entries for call ops.
  // Call arguments will be removed in the call-site specific segment-aware
  // cleanup, avoiding generic eraseOperands bitvector mechanics.)
  for (SymbolTable::SymbolUse use : uses) {
    Operation *callOp = use.getUser();
    // Push an empty operand cleanup entry so that call-site specific logic in
    // cleanUpDeadVals runs (it keys off CallOpInterface). The BitVector is
    // intentionally all false to avoid generic erasure.
    // Store the funcOp as the callee to avoid expensive symbol lookup later.
    cl.operands.push_back({callOp, BitVector(callOp->getNumOperands(), false),
                           funcOp.getOperation()});
  }

  // Do (3).
  // Get the list of unnecessary terminator operands (return values that are
  // non-live across all callers) in `nonLiveRets`. There is a very important
  // subtlety here. Unnecessary terminator operands are NOT the operands of the
  // terminator that are non-live. Instead, these are the return values of the
  // callers such that a given return value is non-live across all callers. Such
  // corresponding operands in the terminator could be live. An example to
  // demonstrate this:
  //  func.func private @f(%arg0: memref<i32>) -> (i32, i32) {
  //    %c0_i32 = arith.constant 0 : i32
  //    %0 = arith.addi %c0_i32, %c0_i32 : i32
  //    memref.store %0, %arg0[] : memref<i32>
  //    return %c0_i32, %0 : i32, i32
  //  }
  //  func.func @main(%arg0: i32, %arg1: memref<i32>) -> (i32) {