[mlir] `remove-dead-values` pass gives `null` operand error

[pingshiyu]

I was compiling the below program using:
mlir-opt --remove-dead-values out.mlir

"builtin.module"() ({
    ^bb0(): 
        "func.func"() ({
            ^bb0(%1 : i1, %2 : i1): 
                %3 = "arith.constant"() {value=1 : i1} : () -> (i1)
                "func.return"(%2, %3) : (i1, i1) -> ()
            }) {sym_name="func0", function_type=(i1, i1) -> (i1, i1)} : () -> ()
        "func.func"() ({
            ^bb0(%4 : i1): 
                %5 = "arith.constant"() {value=true} : () -> (i1)
                %7, %8 = "func.call"(%5, %4) {callee=@func0} : (i1, i1) -> (i1, i1)
                "func.return"(%7, %8) : (i1, i1) -> ()
            }) {sym_name="func1", function_type=(i1) -> (i1, i1)} : () -> ()
    }) : () -> ()

However, I get the following error:

<source>:11:26: error: null operand found
                %7, %8 = "func.call"(%5, %4) {callee=@func0} : (i1, i1) -> (i1, i1)
                         ^
<source>:11:26: note: see current operation: %0:2 = "func.call"(<<NULL VALUE>>, %arg0) <{callee = @func0}> : (<<NULL TYPE>>, i1) -> (i1, i1)
Compiler returned: 1

I do not understand the error message, as the variables used in the function call do not look null at all to me. The above is the reduced test case. The Pass documentation didn't yield anything useful regarding this error, unfortunately.

[llvmbot]

@llvm/issue-subscribers-mlir

Author: Jacob Yu (pingshiyu)

I was compiling the below program using: `mlir-opt --remove-dead-values out.mlir`

"builtin.module"() ({
    ^bb0(): 
        "func.func"() ({
            ^bb0(%1 : i1, %2 : i1): 
                %3 = "arith.constant"() {value=1 : i1} : () -> (i1)
                "func.return"(%2, %3) : (i1, i1) -> ()
            }) {sym_name="func0", function_type=(i1, i1) -> (i1, i1)} : () -> ()
        "func.func"() ({
            ^bb0(%4 : i1): 
                %5 = "arith.constant"() {value=true} : () -> (i1)
                %7, %8 = "func.call"(%5, %4) {callee=@<!-- -->func0} : (i1, i1) -&gt; (i1, i1)
                "func.return"(%7, %8) : (i1, i1) -&gt; ()
            }) {sym_name="func1", function_type=(i1) -&gt; (i1, i1)} : () -&gt; ()
    }) : () -&gt; ()

However, I get the following error:

<source>:11:26: error: null operand found
                %7, %8 = "func.call"(%5, %4) {callee=@<!-- -->func0} : (i1, i1) -&gt; (i1, i1)
                         ^
&lt;source&gt;:11:26: note: see current operation: %0:2 = "func.call"(&lt;&lt;NULL VALUE&gt;&gt;, %arg0) &lt;{callee = @<!-- -->func0}&gt; : (&lt;&lt;NULL TYPE&gt;&gt;, i1) -&gt; (i1, i1)
Compiler returned: 1

I do not understand the error message, as the variables used in the function call do not look null at all to me. The above is the reduced test case. The Pass documentation didn't yield anything useful regarding this error, unfortunately.

[agentcooper]

I have briefly looked into this issue and would appreciate guidance on how to proceed.

First of all, here is the code after --canonicalize:

module {
  func.func @func0(%arg0: i1, %arg1: i1) -> (i1, i1) {
    %true = arith.constant true
    return %arg1, %true : i1, i1
  }
  func.func @func1(%arg0: i1) -> (i1, i1) {
    %true = arith.constant true
    %0:2 = call @func0(%true, %arg0) : (i1, i1) -> (i1, i1)
    return %0#0, %0#1 : i1, i1
  }
}

The problem is still reproducible:

<source>:8:12: error: null operand found
    %0:2 = call @func0(%true, %arg0) : (i1, i1) -> (i1, i1)
           ^
<source>:8:12: note: see current operation: %0:2 = "func.call"(<<NULL VALUE>>, %arg0) <{callee = @func0}> : (<<NULL TYPE>>, i1) -> (i1, i1)
Compiler returned: 1

What happens is as follows.

In func1, the value %true = arith.constant true is considered not live (as the first argument of func0 is not used) and is erased by the call to cleanSimpleOp:

llvm-project/mlir/lib/Transforms/RemoveDeadValues.cpp

Line 604 in f2bb6c4

cleanSimpleOp(op, la);

The cleanFuncOp is responsible for removing unused parameters from a function declaration, but it bails out because func0 is public:

llvm-project/mlir/lib/Transforms/RemoveDeadValues.cpp

Lines 195 to 198 in f2bb6c4

	static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module,
	RunLivenessAnalysis &la) {
	if (funcOp.isPublic())
	return;

Marking the func0 as private in the input gives the expected (working) output: https://godbolt.org/z/bv8c34hEj.

To fix the issue with the initial code, it seems that before erasing the not live value in cleanSimpleOp we need to bail out if it is being used as an argument to a public function. Or replace all uses with some sort of undefined value (I'm not sure if something like that exists in MLIR).

@srishti-pm @zero9178 @joker-eph What do you think?

[zero9178]

Or replace all uses with some sort of undefined value (I'm not sure if something like that exists in MLIR).

This is probably the best solution although I am not sure what the current consensus is on whether we should start generating poison ops, which are basically "undefined" values for the purpose of this transformation. It is certainly not great that we'd add a dependency on a dialect from what was previously a generic transformation.

To fix the issue with the initial code, it seems that before erasing the not live value in cleanSimpleOp we need to bail out if it is being used as an argument to a public function.

This is certainly a good solution that would fix the bug and a good first step. I think it can be argued that for any external functions, all arguments/parameters are live given they are required by ABI.

[agentcooper]

@zero9178 Thanks! I've created a PR: #83249.