llvm-project
cb6e1967 - [MLIR] Forward generated OpTy::create arguments (#170012)

Commit
31 days ago
[MLIR] Forward generated OpTy::create arguments (#170012) The recent changes in the MLIR TableGen interface for generated OpTy::build functions involves a new OpTy::create function that is generated passing arguments without forwarding. This is problematic with arguments that are move only such as `std::unique_ptr`. My particular use case involves `std::unique_ptr<mlir::Region>` which is desirable as the `mlir::OperationState` object accepts calls to `addRegion(std::unique_ptr<mlir::Region>`. In Discord, the use of `extraClassDeclarations` was suggested which I may go with regardless since I still have to define the builder function anyways, but perhaps you would consider this trivial change as it supports a broader class of argument types for this approach. Consider the declaration in TableGen: ``` let builders = [ OpBuilder<(ins "::mlir::Value":$cdr, "::mlir::ValueRange":$packs, "std::unique_ptr<::mlir::Region>":$body)> ]; ``` Which currently generates: ```cpp ExpandPacksOp ExpandPacksOp::create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::Value cdr, ::mlir::ValueRange packs, std::unique_ptr<::mlir::Region> body) { ::mlir::OperationState __state__(location, getOperationName()); build(builder, __state__, std::forward<decltype(cdr)>(cdr), std::forward<decltype(packs)>(packs), std::forward<decltype(body)>(body)); auto __res__ = ::llvm::dyn_cast<ExpandPacksOp>(builder.create(__state__)); assert(__res__ && "builder didn't return the right type"); return __res__; } ``` With this change it will generate: ```cpp ExpandPacksOp ExpandPacksOp::create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::Value cdr, ::mlir::ValueRange packs, std::unique_ptr<::mlir::Region>&&body) { ::mlir::OperationState __state__(location, getOperationName()); build(builder, __state__, static_cast<decltype(cdr)>(cdr), std::forward<decltype(packs)>(packs), std::forward<decltype(body)>(body)); auto __res__ = ::llvm::dyn_cast<ExpandPacksOp>(builder.create(__state__)); assert(__res__ && "builder didn't return the right type"); return __res__; } ``` Another option could be to make this function a template but then it would not be hidden in the generated translation unit. I don't know if that was the original intent. Thank you for your consideration.
Author
Parents
Loading