Moving eraseArguments from FuncOp to FunctionLike

I have a use case where I’ve got a FunctionLike operation, and after some lowerings, I’d like to erase some of its arguments. This is an admittedly tricky process, and a helper was added to FuncOp to simplify this. Back when that was added, @_sean_silva noted in the commit message that it would be nice to pull that into FunctionLike eventually.

What do we think about doing that now? In a brief test, I copied the eraseArguments definition from FuncOp to my FunctionLike operation, and it worked for my use case (i.e. it doesn’t seem to be FuncOp specific anymore). Are there any other considerations than just moving the helper from FuncOp to FunctionLike? If this makes sense to the community, I can put together a patch.

The part that isn’t general is the assumption that the function type is represented with FunctionType, which does not always hold true(e.g. LLVMFuncOp). As long as you rectify that aspect, the rest of the logic(mutating attributes/block arguments) could be moved to FunctionLike. You’ll need to have some way for the concrete function op to update its type.

We’ve run into this before. Could llvm.func just use FunctionType? I just saw this doc comment on LLVMFunctionType, which I assume is the whole reason it uses a different type. Could the necessary accessors just be added to LLVMFuncOp instead of creating a new function type? I think llvm.func is the only outlier among those using FunctionLike trait, and this is creating an issue implementing common functionality on FunctionLike, and is potentially a thorn when more LLVM dialect transforms get implemented (when looking at llvm.return which is not yet marked ReturnLike but should be).

/// LLVM dialect function type. It consists of a single return type (unlike MLIR
/// which can have multiple), a list of parameter types and can optionally be
/// variadic.

I think being able to use a helper function in a nicer way is a terrible argument for changing the underlying assumption of an entire dialect. LLVM dialect uses LLVM types and does not accept standard types, neither it wants to. We used to have LLVMFuncOp based on FunctionType, which was a huge mess that took me a lot of time to clean up, so I am strongly opposed to going back there. Type matching becomes complicated because function calls, especially indirect ones, have types different from function declarations. Same for other attributes referring to functions (e.g., personality functions in exception handling). Everything essentially needs to do function type conversion on-the-fly, with argument packing, vararg handling (by the way, FunctionType does not support vararg which LLVM requires) and other complexity. Translation to LLVM IR suddenly needs to handle type conversion in a non-trivial way. This will be adding a pile of complexity.

Just add make some functions templates, or pass in a callback. Or use type interfaces.

Thanks for the pointers and discussion on the history here. I’m interested in exploring what it would take to make this happen in a way that is not obtrusive to llvm.func or any of the other existing FunctionLike ops.

Now I’m trying to gauge the best way to relax this assumption.

I haven’t tried using type interfaces before, but that seems like an interesting angle. I’m thinking that would look like:

  • defining a new type interface with methods like what is on FunctionType and LLVMFunctionType already (get input type(s), get result type(s), etc.)
  • updating FunctionLike in terms of that interface instead of FunctionType
  • implementing the interface for both FunctionType and LLVMFunctionType (or any other, but these are the only types I found in-tree for FunctionLike ops)

Does that sound reasonable? Is there a better approach worth exploring?

I had another thought along these lines.

Maybe the FunctionLike trait could be parameterized by a function signature type. This could be FunctionType for most cases (maybe as a default), but for LLVMFuncOp it would be LLVMFunctionType. The function signature type would then be used within FunctionLike for getType, setType, etc.

This sounds right. I don’t know how much overhead is there because of interfaces, maybe @River707 does.

FunctionLike already follows the CRTP pattern, so maybe you can just add a requirement that derived classes implement a Type getTypeWithoutArguments(ArrayRef<unsigned>) that returns a new type of whatever type the derived class needs. We already require getNumArguments and getNumResults, so it doesn’t make the coupling much worse.

Thanks for the input. I’ll try something out along these lines.

I put together this patch to try out the ideas above:

I think this is a reasonable step, but really, I am starting to see how a TypeInterface would be nice here. These required hooks on FunctionLike are all delegating to the type used by the concrete operation to represent its signature. If we had a new TypeInterface for function signature types, we could use that interface everywhere in FunctionLike instead of FunctionType. Then, we could remove the requirement for the concrete class to define those hooks and FunctionLike could just call getType() and then use the interface methods.

I am curious what others think. The current revision seems OK, as a pragmatic way to be able to move eraseArguments into FunctionLike. But as a long term solution, it seems worth having the new interface for function signature types as well.

This looks like a good pragmatic solution.

A quick heads up that I recently added FuncOp::eraseResults, so if possible it would be great if we could apply the same kind of solution there as well.

Yep I saw that. I can also move eraseResults over to FunctionLike at the same time. I was wondering if getTypeWithoutArguments might need to be more general… I can update that method to support both arguments and results.

1 Like

I think the general idea of the patch looks good.

Interfaces are a good idea in the longer term. FunctionLike was introduced before we even had op interfaces, let alone type interfaces. I am curious about different kinds of overhead using interfaces may create, e.g., is the code more difficult to follow, is there a noticeable difference in compilation time or run time?

I will clean up that patch, including support for eraseResults, and try to get it to a place we could consider landing it. Maybe we can get that in first, and take a look at interfaces in the future as a way to improve FunctionLike?

I can see that we would need some time to experiment with the TypeInterface before we are ready to commit to using it for such a central piece of functionality. I think we can agree this would be nice in the long term, and I’m interested in helping out down the road.

1 Like