This revision adds support for storing ComplexType elements inside of a DenseElementsAttr. We store complex objects as an array of two elements, matching the definition of std::complex. There is no current attribute storage for ComplexType, but DenseElementsAttr provides API for access/creation using std::complex<>. Given that the internal implementation of DenseElementsAttr is already fairly opaque, the only real complexity here is in the printing/parsing. This revision keeps it simple for now and always uses hex when printing complex elements. A followup will add prettier syntax for this.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/IR/Attributes.h | ||
---|---|---|
1057 | Comments need update? | |
1133 | Same | |
mlir/lib/IR/AttributeDetail.h | ||
378 | This is a little bit weird. I see some use C64 to refer to 2x64 bit float complex number while XLA consider std::complex<float> as C64 (so 64 is total). So seems the bitwidth is overloaded for complex already, as long as you document it this seems fine then as there doesn't seem to be 1 convention. |
mlir/lib/IR/AttributeDetail.h | ||
---|---|---|
378 | That is different. We don't suffer from the problematic naming because we explicitly specify the element type. We specify the type exactly the same as C++ complex, i.e. there is no C64 there is only complex<double>/complex<float>/etc. It doesn't seem like there is any ambiguity there? |
Looks good thanks
mlir/lib/IR/AttributeDetail.h | ||
---|---|---|
378 | You are returning the bitwidth of the storage as element bitwidth which could either be the bitwidth of how it is stored or or the elements. But you are correct in that if one looks at the comment in 375, this would seem to return the storage bitwidth rather than anything intrinsic about the type (e.g., we are storing bf16 as 64 bits :-)). So no ambiguity. | |
mlir/lib/Parser/Parser.cpp | ||
2068 | Now the i vs j debate could heat up :) |
Comments need update?