[Mlir-commits] [mlir] [mlir][llvm] Fixes CallOp builder for the case of indirect call (PR #76240)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Sun Dec 24 06:29:51 PST 2023


================
@@ -908,8 +908,9 @@ void CallOp::build(OpBuilder &builder, OperationState &state, TypeRange results,
 
 void CallOp::build(OpBuilder &builder, OperationState &state, TypeRange results,
                    FlatSymbolRefAttr callee, ValueRange args) {
+  auto fargs = callee ? args : args.drop_front();
----------------
gitoleg wrote:

well, let's have one more round :) And I'll do the fix!

well, I have some doubts about the idea to pass a callee as a value ( or I just misunderstood something :) ) The reason is that everywhere the behavior is assumed as it stated in the docs:
with the first operand being a callee in the operand list in the case of indirect call. So passing a callee separately as a value may be another source of confusion - if the operands must contain the callee in this case or not. Let's just add a builder that doesn't have a callee as an argument - and it will be a true indirect call!

And one more thought about: the fact we don't want to check the attribute here means that it need to be checked in the user code - we just move the problem from this place to another. You can consider the ClangIR [code](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp#L880l) that lowers CIR to MLIR. So choosing the proper builder would cause adding extra checks in there. 

But of course it's up to you to decide. I would say that the crox of the problem is that we probably need two different operations for direct and indirect calls - in this case no confusion would happen. 

So what's the plan?) What do I need to do? 

@ivanradanov  @gysit 


https://github.com/llvm/llvm-project/pull/76240


More information about the Mlir-commits mailing list