-
Notifications
You must be signed in to change notification settings - Fork 198
[CIR][Dialect] Add tmp attribute to AllocaOp #2114
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -580,6 +580,12 @@ def CIR_AllocaOp : CIR_Op<"alloca", [ | |
| The presence of the `const` attribute indicates that the local variable is | ||
| declared with C/C++ `const` keyword. | ||
|
|
||
| The presence of the `tmp` attribute indicates that the allocation | ||
| represents a compiler-generated temporary (e.g., from MaterializeTemporaryExpr, | ||
| aggregate temporaries, reference binding temporaries). This is used by the | ||
| lifetime checker to identify temporaries that may be skipped for certain | ||
| analyses. | ||
|
|
||
| The `dynAllocSize` specifies the size to dynamically allocate on the stack | ||
| and ignores the allocation size based on the original type. This is useful | ||
| when handling VLAs and is omitted when declaring regular local variables. | ||
|
|
@@ -604,6 +610,7 @@ def CIR_AllocaOp : CIR_Op<"alloca", [ | |
| StrAttr:$name, | ||
| UnitAttr:$init, | ||
| UnitAttr:$constant, | ||
| UnitAttr:$tmp, | ||
| ConfinedAttr<OptionalAttr<I64Attr>, [IntMinValue<0>]>:$alignment, | ||
| OptionalAttr<ArrayAttr>:$annotations, | ||
| OptionalAttr<ASTVarDeclInterface>:$ast | ||
|
|
@@ -637,13 +644,12 @@ def CIR_AllocaOp : CIR_Op<"alloca", [ | |
| bool isDynamic() { return (bool)getDynAllocSize(); } | ||
| }]; | ||
|
|
||
| // Custom parse/print allows flags in any order and supports tmp-only or | ||
| // init/const-only spellings, which the compact format can't express. | ||
| let assemblyFormat = [{ | ||
| $allocaType `,` qualified(type($addr)) `,` | ||
| ($dynAllocSize^ `:` type($dynAllocSize) `,`)? | ||
| `[` $name | ||
| (`,` `init` $init^)? | ||
| (`,` `const` $constant^)? | ||
| `]` | ||
| custom<AllocaNameAndFlags>($name, $init, $constant, $tmp) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't Seems like adding another
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree they should be orthogonal, not mutually exclusive. With the custom parser we can already represent both together (
Even after doing that, there is still an MLIR ODS parsing limitation with adjacent optional groups that share the same leading
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
How, you can always setInit in the appropriate place, this functionality shouldn't be part of createTemp*, but can operate on the result / use side.
I don't see how that is true, what am I missing?
The const thing looks like a silly limitation, perhaps that should be fixed. |
||
| ($annotations^)? | ||
| (`ast` $ast^)? attr-dict | ||
| }]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,7 +263,7 @@ class AtomicInfo { | |
| // This function emits any expression (scalar, complex, or aggregate) | ||
| // into a temporary alloca. | ||
| static Address emitValToTemp(CIRGenFunction &CGF, Expr *E) { | ||
| Address DeclPtr = CGF.CreateMemTemp( | ||
| Address DeclPtr = CGF.CreateMemTempWithName( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand why all these changes to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I introduced the naming split intentionally to keep API intent explicit:
If API intent clarity is valuable here, we can keep this split. If you prefer less API surface, I can also remove the split and follow your suggestion directly: keep @bcardosolopes what do you prefer?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, please follow my suggestion then! |
||
| E->getType(), CGF.getLoc(E->getSourceRange()), ".atomictmp"); | ||
| CGF.emitAnyExprToMem(E, DeclPtr, E->getType().getQualifiers(), | ||
| /*Init*/ true); | ||
|
|
@@ -322,7 +322,7 @@ Address AtomicInfo::convertToAtomicIntPointer(Address Addr) const { | |
| } | ||
|
|
||
| Address AtomicInfo::CreateTempAlloca() const { | ||
| Address TempAlloca = CGF.CreateMemTemp( | ||
| Address TempAlloca = CGF.CreateMemTempWithName( | ||
| (LVal.isBitField() && ValueSizeInBits > AtomicSizeInBits) ? ValueTy | ||
| : AtomicTy, | ||
| getAtomicAlignment(), loc, "atomic-temp"); | ||
|
|
@@ -1031,7 +1031,8 @@ RValue CIRGenFunction::emitAtomicExpr(AtomicExpr *E) { | |
| if (ShouldCastToIntPtrTy) | ||
| Dest = Atomics.castToAtomicIntPointer(Dest); | ||
| } else if (E->isCmpXChg()) | ||
| Dest = CreateMemTemp(RValTy, getLoc(E->getSourceRange()), "cmpxchg.bool"); | ||
| Dest = CreateMemTempWithName(RValTy, getLoc(E->getSourceRange()), | ||
| "cmpxchg.bool"); | ||
| else if (!RValTy->isVoidType()) { | ||
| Dest = Atomics.CreateTempAlloca(); | ||
| if (ShouldCastToIntPtrTy) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.