This patch lowers the complex operations supported by the MLIR complex
dialect to those operations rather than libm. When the math runtime flag
is set to precise, libm lowering is used instead.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
3538 | I think this variable is mis-spelt. I didn't change it here because I didn't think it was in scope for this patch. I guess this should be fixed as a separate commit? Or I can just tack it on to this one. |
flang/lib/Lower/IntrinsicCall.cpp | ||
---|---|---|
1337 | Can you please put this whole change under an engineering option? You may enable it by default, but I would like to have a way to switch to the old behavior easily to deal with any issues in our testing. I may take an AR to remove the option, when eveything works well. | |
1339 | It is better to just return result here and avoid the else clause. | |
1348 | ||
1360 | Does it mean a Complex dialect operation will return fir::ComplexType result? Shouldn't it be mlir::ComplexType? This works for abs, but it does not look correct in general. | |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
3538 | My bad. I am okay with fixing it here. | |
3540 | Alternatively, you may call populateComplexToStandardConversionPatterns around line 3395, since ComplexToStandard convertor does not modify parent operations. |
flang/lib/Lower/IntrinsicCall.cpp | ||
---|---|---|
1337 | Happy to do this, the only reason I didn't is that I thought it was already covered by setting the fp-contract to precise. Did you want something more fine-grained for just this option? | |
1360 | I think you're right, we should take this result as the mlir::ComplexType and convert it to a fir::ComplexType. I'll update with this change. | |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
3540 | Originally I did this, but it interacts with MathToFuncsPass and so needs to be run in the same pipeline as that. I couldn't see a better way of doing that than putting it here. When I did put it around 3395 as populateComplexToLLVMConversionPatterns it didn't work for this reason. |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
3540 | Hmm, it should not interact with MathToFuncs, because MathToFuncs currently only work for math::IPowIOp. I wonder in which way it did not work. Is it possible that due to the ordering we ended up converting some complex ops into LLVM rather than to Standard? |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
3540 | Just want to clarify that my previous comment did not intend to hold back the change. I have no remarks besides line 1342. |
Add flag to control new behaviour.
Take results of operations as mlir::complex and convert to fir::complex.
Thank you for the changes! LGTM with minor remarks.
flang/include/flang/Optimizer/Support/InitFIR.h | ||
---|---|---|
26 | Why use '<...>'? Typo? | |
flang/lib/Lower/IntrinsicCall.cpp | ||
43 | Same here - why '<...>'? | |
1352 | Still recommend specifying some fixed allocation size for SmallVector, e.g. 2. | |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
40 | '<...>' again. |
This broke my build:
ld.lld: error: undefined symbol: mlir::createConvertComplexToStandardPass() >>> referenced by CodeGen.cpp:3540 (/llvm-project/flang/lib/Optimizer/CodeGen/CodeGen.cpp:3540) >>> CMakeFiles/obj.FIRCodeGen.dir/CodeGen.cpp.o:((anonymous namespace)::FIRToLLVMLowering::runOnOperation()) ld.lld: error: undefined symbol: mlir::populateComplexToLLVMConversionPatterns(mlir::LLVMTypeConverter&, mlir::RewritePatternSet&) >>> referenced by CodeGen.cpp:3605 (/llvm-project/flang/lib/Optimizer/CodeGen/CodeGen.cpp:3605) >>> CMakeFiles/obj.FIRCodeGen.dir/CodeGen.cpp.o:((anonymous namespace)::FIRToLLVMLowering::runOnOperation()) collect2: error: ld returned 1 exit status
There are missing dependencies in CMake files. David, I can fix it, if you do not have time to do this today.
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
3540 | This requires linking MLIRComplexToStandard in CMakeLists.txt. | |
3605 | This requires linking MLIRComplexToLLVM in CMakeLists.txt. |
I think we should still merge the document. We need to update it mentioning special handling for some operations (like power).
Why use '<...>'? Typo?