-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CIR] Add support for array cleanups #150499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -607,8 +607,8 @@ def CIR_ConditionOp : CIR_Op<"condition", [ | |||||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||||||
|
|
||||||||||||||||||
| defvar CIR_YieldableScopes = [ | ||||||||||||||||||
| "ArrayCtor", "CaseOp", "DoWhileOp", "ForOp", "IfOp", "ScopeOp", "SwitchOp", | ||||||||||||||||||
| "TernaryOp", "WhileOp" | ||||||||||||||||||
| "ArrayCtor", "ArrayDtor", "CaseOp", "DoWhileOp", "ForOp", "IfOp", "ScopeOp", | ||||||||||||||||||
| "SwitchOp", "TernaryOp", "WhileOp" | ||||||||||||||||||
| ]; | ||||||||||||||||||
|
|
||||||||||||||||||
| def CIR_YieldOp : CIR_Op<"yield", [ | ||||||||||||||||||
|
|
@@ -2229,7 +2229,7 @@ def CIR_TrapOp : CIR_Op<"trap", [Terminator]> { | |||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||||||
| // ArrayCtor | ||||||||||||||||||
| // ArrayCtor & ArrayDtor | ||||||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||||||
|
|
||||||||||||||||||
| class CIR_ArrayInitDestroy<string mnemonic> : CIR_Op<mnemonic> { | ||||||||||||||||||
|
|
@@ -2272,6 +2272,23 @@ def CIR_ArrayCtor : CIR_ArrayInitDestroy<"array.ctor"> { | |||||||||||||||||
| }]; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| def CIR_ArrayDtor : CIR_ArrayInitDestroy<"array.dtor"> { | ||||||||||||||||||
| let summary = "Destroy array elements with C++ dtors"; | ||||||||||||||||||
| let description = [{ | ||||||||||||||||||
| Destroy each array element using the same C++ destructor. This | ||||||||||||||||||
| operation has one region, with one single block. The block has an | ||||||||||||||||||
| incoming argument for the current array index to initialize. | ||||||||||||||||||
|
|
||||||||||||||||||
| ```mlir | ||||||||||||||||||
|
||||||||||||||||||
| incoming argument for the current array index to initialize. | |
| ```mlir | |
| incoming argument for the current array index to initialize. | |
| Example: | |
| ```mlir |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -649,6 +649,41 @@ void CIRGenFunction::emitNullabilityCheck(LValue lhs, mlir::Value rhs, | |||||
| assert(!cir::MissingFeatures::sanitizers()); | ||||||
| } | ||||||
|
|
||||||
| /// Destroys all the elements of the given array, beginning from last to first. | ||||||
| /// The array cannot be zero-length. | ||||||
| /// | ||||||
| /// \param begin - a type* denoting the first element of the array | ||||||
| /// \param end - a type* denoting one past the end of the array | ||||||
| /// \param elementType - the element type of the array | ||||||
| /// \param destroyer - the function to call to destroy elements | ||||||
| void CIRGenFunction::emitArrayDestroy(mlir::Value begin, mlir::Value end, | ||||||
| QualType elementType, | ||||||
| CharUnits elementAlign, | ||||||
| Destroyer *destroyer, | ||||||
| bool checkZeroLength) { | ||||||
| assert(!elementType->isArrayType()); | ||||||
| if (checkZeroLength) | ||||||
| cgm.errorNYI("emitArrayDestroy: check for zero length"); | ||||||
|
||||||
|
|
||||||
| // Differently from LLVM traditional codegen, use a higher level | ||||||
| // representation instead of lowering directly to a loop. | ||||||
| mlir::Type cirElementType = convertTypeForMem(elementType); | ||||||
| cir::PointerType ptrToElmType = builder.getPointerTo(cirElementType); | ||||||
|
|
||||||
| // Emit the dtor call that will execute for every array element. | ||||||
| builder.create<cir::ArrayDtor>( | ||||||
|
||||||
| *currSrcLoc, begin, [&](mlir::OpBuilder &b, mlir::Location loc) { | ||||||
| auto arg = b.getInsertionBlock()->addArgument(ptrToElmType, loc); | ||||||
| Address curAddr = Address(arg, cirElementType, elementAlign); | ||||||
| assert(!cir::MissingFeatures::dtorCleanups()); | ||||||
|
|
||||||
| // Perform the actual destruction there. | ||||||
| destroyer(*this, curAddr, elementType); | ||||||
|
|
||||||
| builder.create<cir::YieldOp>(loc); | ||||||
|
||||||
| builder.create<cir::YieldOp>(loc); | |
| cir::YieldOp::create(builder, loc); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| auto constantCount = dyn_cast<cir::ConstantOp>(length.getDefiningOp()); | |
| auto constantCount = length.getDefiningOp<cir::ConstantOp>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a cir::MissingFeature::vla() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can simplify the control flow a little if you invert the condition up there. Something like:
if(!constantCount) {
assert(!cir::MissingFeature::vlas());
cgm.errorNYI("emitDestroy: variable length array");
return;
}
auto constIntAttr = mlir::dyn_cast<cir::IntAttr>(constantCount.getValue());
// ...and if it's constant zero, we can just skip the entire thing.
if (constIntAttr && constIntAttr.getUInt() == 0)
return;
checkZeroLength = false;
mlir::Value begin = addr.getPointer();
mlir::Value end; // This will be used for future non-constant counts.
emitArrayDestroy(begin, end, type, elementAlign, destroyer, checkZeroLength);
if (constantCount.use_empty())
constantCount.erase();There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably need to switch back to the if-else structure when we support VLAs, but for now your suggestion looks better. I'll make the change.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,7 @@ struct LoweringPreparePass : public LoweringPrepareBase<LoweringPreparePass> { | |||||||||
| void runOnOp(mlir::Operation *op); | ||||||||||
| void lowerCastOp(cir::CastOp op); | ||||||||||
| void lowerUnaryOp(cir::UnaryOp op); | ||||||||||
| void lowerArrayDtor(ArrayDtor op); | ||||||||||
| void lowerArrayCtor(ArrayCtor op); | ||||||||||
|
||||||||||
| void lowerArrayDtor(ArrayDtor op); | |
| void lowerArrayCtor(ArrayCtor op); | |
| void lowerArrayDtor(cir::ArrayDtor op); | |
| void lowerArrayCtor(cir::ArrayCtor op); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| nextElement = builder.create<cir::PtrStrideOp>( | |
| loc, eltTy, currentElement, stride); | |
| nextElement = cir::PtrStrideOp::create( | |
| builder, loc, eltTy, currentElement, stride); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| nextElement = builder.create<cir::PtrStrideOp>( | |
| loc, eltTy, currentElement, stride); | |
| nextElement = cir::PtrStrideOp::create( | |
| builder, loc, eltTy, currentElement, stride); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your loop is correct. But I also think if you initialize stop with end - 1 you don't have to use moveAfter here and can share more code between the ctor and dtor case (the zero case is handled elsewhere so that would be safe to do). But that's more of a comment and less of a request to change your patch. I don't care either way :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArrayCtor asserts here !cir::MissingFeatures::vlas(), shouldn't it be asserted also here?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to keep this if/else cascade in lexicographical order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.