Add support for representing complex array constants with MLIR
dense attribute. This improves compile time and greatly reduces
memory usage of programs with large complex array constants.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
flang/test/Lower/array.f90 | ||
---|---|---|
133 | I do not think it is a good idea to mix mlir.complex and fir.complex types here. fir.global might be allowing this right now, but there is no guarantee that mlir.complex and fir.complex will be converted to the same type during fir.global to llvm.global codegen. Can we at least add an assertion in fir.global codegen that the init attribute data type and the operation's result data type match? |
flang/test/Lower/array.f90 | ||
---|---|---|
133 | Assert added. |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
2909 | I think this assert is a no-op, because tyAttr is the created op's explicit result type. What I suggested to assert is that initAttr's type and tyAttr comply. |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
2909 | Sorry for the delayed response, I've been away for a few days. initAttr's type and tyAttr doesn't match. Considering a complex array of 10 elements, they will be: tyAttr: !llvm.array<10 x struct<(f32, f32)>> initAttr's type: tensor<10xcomplex<f32>> global: llvm.mlir.global internal @_QFEc1(dense<[(3.000000e+00,4.000000e+00), ...]> : tensor<10xcomplex<f32>>) {addr_space = 0 : i32} : !llvm.array<10 x struct<(f32, f32)>> It seems the global's dense attribute and its tensor<10xcomplex<f32>> type are lowered to LLVM on a later pass. It's easy to extract the element type from both (struct<(f32, f32) and complex<f32>), but is there a function that can be called from codegen to convert from MLIR to LLVM type? |
flang/lib/Optimizer/CodeGen/CodeGen.cpp | ||
---|---|---|
2909 | A new function was added to compare initAttr's type and tyAttr. It extracts the element type of both and convert initAttr's element type to its corresponding LLVM type, to make it possible to compare it with tyAttr's type. The new function is only called by the assert. | |
flang/test/Lower/array.f90 | ||
133 | Just expanding a bit more on this, I don't see any good way to avoid mixing mlir.complex and fir.complex types here. fir.complex can't be changed to mlir.complex without adding casts to the code that manipulates the global's elements, which seems worse. Or that code would need to be changed to operate with mlir.complex values, which would probably require a lot of work. mlir.complex can't be changed to fir.complex, as it is not a supported attribute type. Maybe it could be wrapped inside an OpaqueAttr, but I don't know if it would work and it doesn't seem a good idea either. |
Thank you for the extra steps to improve the verification and for the explanations! I think it is as good as it can get currently.
I think this assert is a no-op, because tyAttr is the created op's explicit result type. What I suggested to assert is that initAttr's type and tyAttr comply.