mlir: add an operation to EmitC for function template instantiation #100895
Conversation
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Rohan Yadav (rohany) ChangesThis commit adds an Doing so is necessary to generate code that interacts with some lower level APIs for launching parallel work into runtime systems. Full diff: https://github.com/llvm/llvm-project/pull/100895.diff 3 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 452302c565139..83f2e14d9407d 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -1260,5 +1260,20 @@ def EmitC_SubscriptOp : EmitC_Op<"subscript", []> {
let assemblyFormat = "$value `[` $indices `]` attr-dict `:` functional-type(operands, results)";
}
+def EmitC_InstantiateFunctionTemplateOp : EmitC_Op<"instantiate_function_template", []> {
+ let summary = "Instantiate template operation";
+ let description = [{
+ Instantiate a function template with a given set of types
+ (given by the values as argument to this operation) to obtain
+ a function pointer.
+ }];
+ let arguments = (ins
+ Arg<StrAttr, "the C++ function to instantiate">:$callee,
+ Variadic<EmitCType>:$args
+ );
+ let results = (outs EmitC_PointerType);
+ let assemblyFormat = "$callee `(` $args `)` attr-dict `:` functional-type($args, results)";
+}
+
#endif // MLIR_DIALECT_EMITC_IR_EMITC
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index 626638282efe1..60c5c7fb906fb 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -656,6 +656,30 @@ static LogicalResult printOperation(CppEmitter &emitter,
return success();
}
+static LogicalResult
+printOperation(CppEmitter &emitter,
+ emitc::InstantiateFunctionTemplateOp instOp) {
+
+ raw_ostream &os = emitter.ostream();
+ Operation &op = *instOp.getOperation();
+
+ if (failed(emitter.emitAssignPrefix(op)))
+ return failure();
+ os << "&" << instOp.getCallee() << "<";
+
+ auto emitArgs = [&](mlir::Value val) -> LogicalResult {
+ os << "decltype(";
+ if (failed(emitter.emitOperand(val)))
+ return failure();
+ os << ")";
+ return success();
+ };
+ if (failed(interleaveCommaWithError(instOp.getArgs(), os, emitArgs)))
+ return failure();
+ os << ">";
+ return success();
+}
+
static LogicalResult printOperation(CppEmitter &emitter,
emitc::ApplyOp applyOp) {
raw_ostream &os = emitter.ostream();
@@ -1508,6 +1532,8 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
.Case<func::CallOp, func::FuncOp, func::ReturnOp>(
[&](auto op) { return printOperation(*this, op); })
.Case<emitc::LiteralOp>([&](auto op) { return success(); })
+ .Case<emitc::InstantiateFunctionTemplateOp>(
+ [&](auto op) { return printOperation(*this, op); })
.Default([&](Operation *) {
return op.emitOpError("unable to find printer for op");
});
diff --git a/mlir/test/Dialect/EmitC/ops.mlir b/mlir/test/Dialect/EmitC/ops.mlir
index 51c484a633eec..9ddcb7b539a48 100644
--- a/mlir/test/Dialect/EmitC/ops.mlir
+++ b/mlir/test/Dialect/EmitC/ops.mlir
@@ -224,6 +224,12 @@ func.func @test_subscript(%arg0 : !emitc.array<2x3xf32>, %arg1 : !emitc.ptr<i32>
return
}
+func.func @test_instantiate_template() {
+ %c1 = "emitc.constant"() <{value = 7 : i32}> : () -> i32
+ %0 = emitc.instantiate_function_template "func_template"(%c1) : (i32) -> !emitc.ptr<!emitc.opaque<"void">>
+ return
+}
+
emitc.verbatim "#ifdef __cplusplus"
emitc.verbatim "extern \"C\" {"
emitc.verbatim "#endif // __cplusplus"
|
|
Requesting a review from @simon-camp @aniragil, folks who have touched emitc recently. |
| def EmitC_InstantiateFunctionTemplateOp : EmitC_Op<"instantiate_function_template", []> { | ||
| let summary = "Instantiate template operation"; | ||
| let description = [{ | ||
| Instantiate a function template with a given set of types |
There was a problem hiding this comment.
Please give an example
| a function pointer. | ||
| }]; | ||
| let arguments = (ins | ||
| Arg<StrAttr, "the C++ function to instantiate">:$callee, |
There was a problem hiding this comment.
Why do you need to derive the type from an SSA value? Why not instead use an ArrayAttr (of TypeAttr) directly here? Is there really a use case where it is difficult to infer that information? When you create this operation, you would could just have a builder with an API like b.create<InstatiateFunctionTemplateOp>(loc, TypeRange(args)) in order to create an ArrayAttr containing all the types.
There was a problem hiding this comment.
In the case where the types are known inside of emitc, this operation is not actually needed (use of verbatim and string formatting with a TypeRange would suffice). In the case that I'm considering, we need to derive the types from the SSA values themselves. For example, consider generating code using the CuTe C++ library. After a few layout transformations, only the C++ compiler knows the exact types of a CuTe object. In such a case, a decltype is needed to correctly instantiate the function template.
| Variadic<EmitCType>:$args | ||
| ); | ||
| let results = (outs EmitC_PointerType); | ||
| let assemblyFormat = "$callee `(` $args `)` attr-dict `:` functional-type($args, results)"; |
There was a problem hiding this comment.
I don't believe there would be an issue with instead using < ... > instead; did you already try that? This format makes the operation look like a "call" instead of an instantiation.
| @@ -656,6 +656,30 @@ static LogicalResult printOperation(CppEmitter &emitter, | |||
| return success(); | |||
| } | |||
|
|
|||
| static LogicalResult | |||
| printOperation(CppEmitter &emitter, | |||
There was a problem hiding this comment.
You are missing a test for this translation
There was a problem hiding this comment.
Where do those tests live? I grepped around but couldnt find where the others are.
There was a problem hiding this comment.
The tests are under llvm-project/mlir/test/Target/Cpp
rohany
force-pushed
the
emitc-instantiate-function-templates
branch
from
2372a35 to
4994322
Compare
This commit adds an `emitc.instantiate_function_template` operation to allow for the expression of function template instantiation. Without this operation, there is no easy way to express a C++ program like: ``` auto x = ...; auto y = ...; const void* fptr = &f<decltype(x), decltype(y)>; ``` Doing so is necessary to generate code that interacts with some lower level APIs for launching parallel work into runtime systems.
rohany
force-pushed
the
emitc-instantiate-function-templates
branch
from
4994322 to
4300427
Compare
|
I wonder how this op relates to the existing |
| Translates to the C++: | ||
| ```c++ | ||
| int32_t v1 = 7; | ||
| void* v2 = &func_template<decltype(v1)>; |
There was a problem hiding this comment.
Could we separate the application of template instantiation from taking its address (which emitc has an op for)?
There was a problem hiding this comment.
I could, but right now there isn't a way to use the returned value except taking a pointer of it, as call excepts a SymbolRefAttr.
There was a problem hiding this comment.
We can add an call_indirect op, similar to the func dialect. But we would need to also add support for function types in the emitter (or add an generic auto type, but that might have problems with wrongly inferred types if used in other places).
OTOH, is it even legal to cast function pointers to void *?
There was a problem hiding this comment.
OTOH, is it even legal to cast function pointers to void *?
I think it's technically undefined because some kinds of hardware cannot support such a cast. However, the runtime library (CUTLASS) I'm trying to interact with only accepts void* function pointers for a particular function, so its not invalid either.
|
Unless we want to add a mode to |
We could use something like an 'instantiate_only' unit attribute to disambiguate, it's the no-call semantic stretch that makes me somewhat uncomfortable with raising this option. OTOH, the new op would probably replicate eventually the (quite complex) capabilities of |
|
I agree that the ops should be modeled the same, but I don't know if unifying these into one op is a good design. As I understand the result type should be something callable (i.e. function type to start with) if only instantiation is desired. |
How about we go the other way around? We could separate the template instantiation part out of |
|
Sorry, should have been more clear: I'm not suggesting to modify
I'm not sure why that would be a problem, as the change means replacing each Another aspect is separating concerns: EmitC is used for various C variants, with templated-C++ being only one of. The Another advantage of separating instantiation from the call is the ability to reuse the same template instance for multiple calls. This may be less useful for |
|
So is the consensus here to
I could embark on this, but I want to make sure that everyone is on board before starting any of the work. |
Sorry for not following up earlier. Isn't Generally speaking, yes we try to keep the dialect as stable as possible and therefore do not make us the decision to change it easy. With other words we try to avoid breaking changes. However, if evolvement is strictly required and we reached consensus we can go forward also with breaking changes. |
Then how about we extend |
I think we don't should go the way to extend options to much, which hasn't worked that well in the past. I think we should rather try to add new functionality with adding new ops. Therefore, I am in favor of adding an |
This commit adds an
emitc.instantiate_function_templateoperation to allow for the expression of function template instantiation. Without this operation, there is no easy way to express a C++ program like:Doing so is necessary to generate code that interacts with some lower level APIs for launching parallel work into runtime systems.