Convert complex.abs to libm library
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Conversion/ComplexToLibm/ComplexToLibm.cpp | ||
---|---|---|
40 | This template class implementation is very similar to ScalarOpToLibmCall, with the only difference that one handle operations that produce a complex type as a result while the other handles operations that produce a float type as a result. It would be nice if we can somehow combine these two template classes into one. |
@bixia Thank you for the feedback. I tried to combine them into one template function but the following error keeps showing up. Looks like complex::AbsOp fails to be embedded into the function in ScalarOpToLibmCall. I think we may need to change the op definition somewhat. Do you have any idea about that?
Assertion failed: (isa<U>()), function cast, file Types.h, line 251. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. Program arguments: /Users/sasakikai/dev/llvm-project/build/bin/mlir-opt /Users/sasakikai/dev/llvm-project/mlir/test/Conversion/ComplexToLibm/convert-to-libm.mlir -convert-complex-to-libm -canonicalize Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 mlir-opt 0x000000010bd95a8a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 42 1 mlir-opt 0x000000010bd94928 llvm::sys::RunSignalHandlers() + 248 2 mlir-opt 0x000000010bd96120 SignalHandler(int) + 288 3 libsystem_platform.dylib 0x00007ff80e9d2dfd _sigtramp + 29 4 libsystem_platform.dylib 0x00007f9225042b20 _sigtramp + 18446743635998735680 5 libsystem_c.dylib 0x00007ff80e908d24 abort + 123 6 libsystem_c.dylib 0x00007ff80e9080cb err + 0 7 mlir-opt 0x000000010d2fc003 (anonymous namespace)::ScalarOpToLibmCall<mlir::complex::AbsOp>::matchAndRewrite(mlir::complex::AbsOp, mlir::PatternRewriter&) const (.cold.10) + 35 8 mlir-opt 0x000000010c8c3d29 (anonymous namespace)::ScalarOpToLibmCall<mlir::complex::AbsOp>::matchAndRewrite(mlir::complex::AbsOp, mlir::PatternRewriter&) const + 1257 9 mlir-opt 0x000000010cf667d0 mlir::PatternApplicator::matchAndRewrite(mlir::Operation*, mlir::PatternRewriter&, llvm::function_ref<bool (mlir::Pattern const&)>, llvm::function_ref<void (mlir::Pattern const&)>, llvm::function_ref<mlir::LogicalResult (mlir::Pattern const&)>) + 1552 10 mlir-opt 0x000000010cd51e92 (anonymous namespace)::OperationLegalizer::legalize(mlir::Operation*, mlir::ConversionPatternRewriter&) + 2290 11 mlir-opt 0x000000010cd4a43d (anonymous namespace)::OperationConverter::convertOperations(llvm::ArrayRef<mlir::Operation*>, llvm::function_ref<void (mlir::Diagnostic&)>) + 1117 12 mlir-opt 0x000000010cd4ce9b mlir::applyPartialConversion(mlir::Operation*, mlir::ConversionTarget&, mlir::FrozenRewritePatternSet const&, llvm::DenseSet<mlir::Operation*, llvm::DenseMapInfo<mlir::Operation*, void>>*) + 75 13 mlir-opt 0x000000010c8c410d (anonymous namespace)::ConvertComplexToLibmPass::runOnOperation() + 669 14 mlir-opt 0x000000010cd0da95 mlir::detail::OpToOpPassAdaptor::run(mlir::Pass*, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int) + 485 15 mlir-opt 0x000000010cd0e147 mlir::detail::OpToOpPassAdaptor::runPipeline(mlir::OpPassManager&, mlir::Operation*, mlir::AnalysisManager, bool, unsigned int, mlir::PassInstrumentor*, mlir::PassInstrumentation::PipelineParentInfo const*) + 295 16 mlir-opt 0x000000010cd0fb72 mlir::PassManager::run(mlir::Operation*) + 642 17 mlir-opt 0x000000010cce4377 performActions(llvm::raw_ostream&, bool, bool, llvm::SourceMgr&, mlir::MLIRContext*, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>) + 503 18 mlir-opt 0x000000010cce20c1 processBuffer(llvm::raw_ostream&, std::__1::unique_ptr<llvm::MemoryBuffer, std::__1::default_delete<llvm::MemoryBuffer>>, bool, bool, bool, bool, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, llvm::ThreadPool*) + 641 19 mlir-opt 0x000000010cce1d66 mlir::MlirOptMain(llvm::raw_ostream&, std::__1::unique_ptr<llvm::MemoryBuffer, std::__1::default_delete<llvm::MemoryBuffer>>, llvm::function_ref<mlir::LogicalResult (mlir::PassManager&)>, mlir::DialectRegistry&, bool, bool, bool, bool, bool) + 182 20 mlir-opt 0x000000010cce2bcd mlir::MlirOptMain(int, char**, llvm::StringRef, mlir::DialectRegistry&, bool) + 2525 21 mlir-opt 0x000000010bc2a9e6 main + 198 22 dyld 0x0000000111ccf51e start + 462
How did you combine the two template? I was thinking the current template only take one parameter Op. The new template will take one additional parameter, that is, a functor that returns the return-type from Op.
The result type is used for defining the function type.
auto opFunctionTy = FunctionType::get( rewriter.getContext(), op->getOperandTypes(), op->getResultTypes());
I thought abs op returns the float-like type so that we do not need to pass the functor to the template explicitly. Is this assumption wrong?
mlir/lib/Conversion/ComplexToLibm/ComplexToLibm.cpp | ||
---|---|---|
21 | Would you please update the document to describe the new parameter, such as it must correspond to the result type of the operation? | |
54 | Would this help make it clear that the struct is used for the result type of the op? | |
56–57 | This can be simplified to return a boolean, such as true for float and false for double. | |
67 | Similar to the above. |
mlir/lib/Conversion/ComplexToLibm/ComplexToLibm.cpp | ||
---|---|---|
56–57 | We should return some value specifying the error for the incompatible element type. If we change the signature to bool, we can only return true or false which does not keep error information. |
mlir/lib/Conversion/ComplexToLibm/ComplexToLibm.cpp | ||
---|---|---|
56–57 | can you use std::optional<bool> for the return type |
@Lewuathe, thank you for all your contributions to MLIR and CompexOps dialect! Would you be interested also in adding a dialect-specific attribute for complex numbers so that complex.constant would accept a dedicated attribute, e.g. F32ComplexAttr instead of ArrayAttr. That would make the code cleaner and also enable more opportunities for folding.
@pifon2a Sounds interesting. Let me try to work on that. Do you have any specific idea on the interface of the custom attribute for complex types? (e.g. complex.constant<1.0, 2.0> ?)
Thank you! I think if we can print/parse #complex.number<1.0, 2.0>: f32, then it would be pretty readable and tidy. Implementation-wise, this attribute should be pretty close to Builtin_FloatAttr, but with 3 parameters instead of 2.
Would you please update the document to describe the new parameter, such as it must correspond to the result type of the operation?
You may also want to rename the parameter and assign a default value to it, such as TypeResolver = ComplexTypeResolver.