[llvm] [mlir] [mlir][EmitC] Add MathToEmitC pass for math function lowering to EmitC (PR #113799)

Marius Brehler via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 5 07:23:55 PST 2024


================
@@ -62,4 +62,13 @@ def EmitC_OpaqueAttr : EmitC_Attr<"Opaque", "opaque"> {
 
 def EmitC_OpaqueOrTypedAttr : AnyAttrOf<[EmitC_OpaqueAttr, TypedAttrInterface]>;
 
+def MathToEmitCLanguageTarget : I32EnumAttr<"MathToEmitCLanguageTarget",
+  "Specifies the language target for generating callees.", [
+    I32EnumAttrCase<"C", 0, "Use C-style function names">,
+    I32EnumAttrCase<"CPP", 1, "Use C++-style function names">
+  ]> {
+  let cppNamespace = "::mlir::emitc";
+}
----------------
marbre wrote:

No need to ass C23 from the start but I would prefer a naming (c99, cpp11, ...) that allows to extend if we want to (we would be a little stuck with just `CPP` and `C`).

Let me try to also pull in the other parallel discussion. With regards to @cferry-AMD's comment
>  How specific to Math functions do you see this pass? The second choice makes the lowering much more generic than I thought: it may apply to any op that could be lowered to opaque library calls, and not just Math dialect ops to `math.h` functions. I like this ability to lower anything to opaque calls with language filters, but this may not be what you intended.

and your respond
> Thank you for the feedback! I intended this pass to focus on lowering math functions specifically. I accepted the suggestion to use libraries like and am also considering the option to add custom math libraries, not just the standard ones, which motivated me to use a map. I can enforce that only math dialect ops will be converted to ensure it remains targeted.

I feel like shouldn't put to much into this single PR. We can still use what you have to iterate on it under the assumption that we don't land an approach that introduces drawbacks (for example ops, parameters, ..) that will break with the first refactoring commit and thus break users. Having something that can be consistently used in multiple conversions could be nice, but I currently don't have a concrete idea of how this could look like and lack time to spend much though on it. It would indeed be nice to solve some of the some of issues to which @aniragil pointed but I don't know if we can solve all these with this kind of the proposed attribute. Target module ops like EmitC::C99ModuleOp can be an alternative but are a different concept. Spontaneously, I would prefer to just go a different conversion route (which is what you're doing with the proposed patch) than introducing completely new ops.

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


More information about the llvm-commits mailing list