[Mlir-commits] [mlir] [MLIR][LLVMIR] Add an operation to handle constrained FP intrinsics (PR #199754)

[Andy Kaylor]

andykaylor wrote:

> I have som high-level comments and questions.
> 
> Can you quantify the number of extra intrinsics needed? If it is a single digit number we may consider adding explicit intrinsics but I doubt it.

There are 38 of these intrinsics that aren't currently handled.

> If we go for a call operation, I would have a slight preference for reusing the existing `CallIntrinsicOp`. That probably requires some plumbing to import and export the metadata.

We could handle it that way. As you say, the difficulty is in representing the metadata arguments. I don't like the idea of reusing the existing `CallIntrinsicOp` with a caveat that some of the arguments are attached as attributes, and I don't know of a way to directly model metadata arguments in MLIR. I also don't like the idea of adding the `FPExceptionBehaviorOpInterface` and `RoundingModeOpInterface` to the general operation when it won't apply to most intrinsics.

> Alternatively, we could also go the route of your PR. However, then I would probably avoid the work `call` in the name and just use something like `ConstrainedFPIntrinsicOp` that has an enum argument that specifies the op code rather than the string attribute? Also for consistency we should then probably replace the existing intrinsics with this new "generic" intrinsic.

Dropping "Call" from the name makes sense to me. I did intend to be able to support the currently supported constrained intrinsics using this new operation, but I wasn't sure if there would be any out-of-tree impact to removing the existing operations.

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