[CIR][Lowering] Support lowering of nested string ConstantArrays. #244
Conversation
| @@ -244,6 +244,8 @@ class CIRGenBuilderTy : public mlir::OpBuilder { | |||
| } | |||
|
|
|||
| if (const auto arrayVal = attr.dyn_cast<mlir::cir::ConstArrayAttr>()) { | |||
| if (arrayVal.getElts().isa<mlir::StringAttr>()) | |||
There was a problem hiding this comment.
While here, what should happen for the following cases below?
- There are no elements:
"". - The string is null terminated
"\0".
There was a problem hiding this comment.
Ideally, I think this should return true if all characters in the string are \0.
There was a problem hiding this comment.
Yes, for zero strings this branch is true as well. I've added tests to check suggested cases.
| @@ -244,6 +244,8 @@ class CIRGenBuilderTy : public mlir::OpBuilder { | |||
| } | |||
|
|
|||
| if (const auto arrayVal = attr.dyn_cast<mlir::cir::ConstArrayAttr>()) { | |||
| if (arrayVal.getElts().isa<mlir::StringAttr>()) | |||
There was a problem hiding this comment.
Ideally, I think this should return true if all characters in the string are \0.
yugr
force-pushed
the
struct-array-init
branch
from
e501bbc
to
c7a4852
Compare
| char y[3]; | ||
| char z[3]; | ||
| } nestedString = {"1", "", "\0"}; | ||
| // CHECK: cir.global external @nestedString = #cir.const_struct<{#cir.const_array<"1\00\00" : !cir.array<!s8i x 3>> : !cir.array<!s8i x 3>, #cir.const_array<"\00\00\00" : !cir.array<!s8i x 3>> : !cir.array<!s8i x 3>, #cir.const_array<"\00\00\00" : !cir.array<!s8i x 3>> : !cir.array<!s8i x 3>}> |
There was a problem hiding this comment.
The initializer for the null strings should leverage #cir.zero in some way. The reason for this is to improve performance when facing large zero/prefix-initialized strings. For example:
struct {
char r[100000];
char g[100000];
char b[100000];
} image = {"", "1234", "\0"};
The resulting CIR code for this example consists of three #cir.const_arrays with thousands \00 hexadecimal chars each, making them unnecessarily large.
There was a problem hiding this comment.
@yugr no need to apply this. It's not caused by your PR. I'm just leaving it here to open an issue for it later.
No description provided.