[CIR][CodeGen] Implement replace global handling of cir::ConstRecordAttr
#1778
Conversation
…lobal replacement
| newElements, mlir::cast<cir::ArrayType>(oldArray.getType())); | ||
| } | ||
| if (auto oldRecord = mlir::dyn_cast<cir::ConstRecordAttr>(oldInit)) { | ||
| mlir::ArrayAttr newMembers = getNewInitElements(oldRecord.getMembers()); |
There was a problem hiding this comment.
Is it possible to get here with anything other than a zero initializer? I'm having a hard time mapping between your test and the behavior here.
There was a problem hiding this comment.
Maybe I misunderstood, but this path comes from replaceGlobal, which just checks if there's an initial value, given this check goes for the top level initial value, looks like it's ok?
There was a problem hiding this comment.
I did some manual fuzzing to test this, it looks like you can only get here with a tentative array initialization (zero-initialized array of length 1).
If the test in globals.c is modified in either of the following ways, then the path isn't exercised:
struct A {
signed *x;
} tentativeF[10]; // sized array
signed glob;
struct A {
signed *x;
} tentativeF[] = {&glob}; // initialized
There was a problem hiding this comment.
I thought that might be the case. The code seems to be handling general initializer possibilities, which I guess is OK, but I also couldn't find a way to get here with anything other than a zero initializer.
| newElements, mlir::cast<cir::ArrayType>(oldArray.getType())); | ||
| } | ||
| if (auto oldRecord = mlir::dyn_cast<cir::ConstRecordAttr>(oldInit)) { | ||
| mlir::ArrayAttr newMembers = getNewInitElements(oldRecord.getMembers()); |
There was a problem hiding this comment.
Maybe I misunderstood, but this path comes from replaceGlobal, which just checks if there's an initial value, given this check goes for the top level initial value, looks like it's ok?
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
lgtm if @bcardosolopes is satisfied
| newElements, mlir::cast<cir::ArrayType>(oldArray.getType())); | ||
| } | ||
| if (auto oldRecord = mlir::dyn_cast<cir::ConstRecordAttr>(oldInit)) { | ||
| mlir::ArrayAttr newMembers = getNewInitElements(oldRecord.getMembers()); |
There was a problem hiding this comment.
I thought that might be the case. The code seems to be handling general initializer possibilities, which I guess is OK, but I also couldn't find a way to get here with anything other than a zero initializer.
…Attr` (llvm#1778) Implemented NYI handling for `cir::ConstRecordAttr` in CIR CodeGen global replacement. Refactored existing support for `cir::ConstArrayAttr` to share duplicate code between the two.
3 participants
Implemented NYI handling for
cir::ConstRecordAttrin CIR CodeGen global replacement.Refactored existing support for
cir::ConstArrayAttrto share duplicate code between the two.