[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd35a454170da: [CodeGen] Emit destructor calls to destruct
non-trivial C struct objects… (authored by ahatanak).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/lib/AST/Expr.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,6 +89,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+- (StrongSmall)getStrongSmall;
++ (StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -476,6 +483,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -520,7 +539,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -808,4 +829,62 @@
func(0);
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thank, this looks great. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak updated this revision to Diff 251463.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Teach `Expr::isConstantInitializer` that `ExprWithCleanups` can be a constant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/lib/AST/Expr.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,6 +89,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+- (StrongSmall)getStrongSmall;
++ (StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -476,6 +483,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -520,7 +539,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -808,4 +829,62 @@
func(0);
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second call.
-
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:677
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > ahatanak wrote:
> > > > rjmccall wrote:
> > > > > ahatanak wrote:
> > > > > > rjmccall wrote:
> > > > > > > Why only when the l-value is volatile?
> > > > > > I was trying to avoid emitting `ExprWithCleanups` when it wasn't
> > > > > > needed and it seemed that it wasn't needed for non-volatile types.
> > > > > > I'm not sure it's important, so I've removed the check for
> > > > > > volatile. Also, `ExprWithCleanups` shouldn't' be emitted when this
> > > > > > is in file scope, so I fixed that too.
> > > > > Hmm, not sure about this file-scope thing, since the combination of
> > > > > C++ dynamic initializers and statement-expressions means we can have
> > > > > pretty unrestricted code there.
> > > > I should have explained why this was needed, but I wanted to prevent
> > > > emitting `ExprWithCleanups` in the following example:
> > > >
> > > > ```
> > > > struct A {
> > > > id f0;
> > > > };
> > > >
> > > > typedef struct A A;
> > > >
> > > > A g = (A){ .f0 = 0 };
> > > > ```
> > > >
> > > > The l-value to r-value conversion happens here because compound
> > > > literals are l-values. Since `g` is a global of a non-trivial C struct
> > > > type, we shouldn't try to push a cleanup and destruct the object.
> > > >
> > > > We don't have to think about the C++ case since the line below checks
> > > > the type is a non-trivial C type. I didn't think about statement
> > > > expressions, but they aren't allowed in file scope, so I guess that's
> > > > not a problem either.
> > > I would hope that the constant-evaluator here might be smart enough to
> > > ignore some elidable temporaries.
> > Do you mean the constant-evaluator should evaluate initializers that are
> > `ExprWithCleanups` to constants in a case like this? It's possible to do
> > so by seeing whether the sub-expression of `ExprWithCleanups` is constant,
> > but it looks like we also have to set the
> > `CleanupInfo::CleanupsHaveSideEffects` flag to false when it's a file scope
> > expression so that `ConstExprEmitter::VisitExprWithCleanups` can
> > constant-fold the `ExprWithCleanups` initializer.
> In this case, what we're doing is eliding a "temporary" (really an object
> with wider lifetime, but if the object isn't referenceable, we can compile
> as-if the object is really temporary) and therefore bypassing the need for a
> cleanup entirely. I wouldn't expect the AST to change to reflect that this
> is possible, just the constant evaluator. Basically, it needs to look
> through `ExprWithCleanups`; we probably already peephole the
> `LValueToRValue(CompoundLiteralExpr)` combination.
>
> We do try to do constant-evaluation on local initializers as well as an
> optimization, and abstractly that should also work with these constructs.
I taught `Expr::isConstantInitializer` to look through `ExprWithCleanups` and
`ConstExprEmitter::VisitExprWithCleanups` to ignore whether the expression has
side effects. For the latter, I'm assuming we don't have to care about the side
effect of the cleanup if the need for a cleanup is elided. This change causes
`CodeGenFunction::EmitAutoVarInit` to emit the initializer as a constant too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:677
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+
ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > ahatanak wrote:
> > > > > rjmccall wrote:
> > > > > > Why only when the l-value is volatile?
> > > > > I was trying to avoid emitting `ExprWithCleanups` when it wasn't
> > > > > needed and it seemed that it wasn't needed for non-volatile types.
> > > > > I'm not sure it's important, so I've removed the check for volatile.
> > > > > Also, `ExprWithCleanups` shouldn't' be emitted when this is in file
> > > > > scope, so I fixed that too.
> > > > Hmm, not sure about this file-scope thing, since the combination of C++
> > > > dynamic initializers and statement-expressions means we can have
> > > > pretty unrestricted code there.
> > > I should have explained why this was needed, but I wanted to prevent
> > > emitting `ExprWithCleanups` in the following example:
> > >
> > > ```
> > > struct A {
> > > id f0;
> > > };
> > >
> > > typedef struct A A;
> > >
> > > A g = (A){ .f0 = 0 };
> > > ```
> > >
> > > The l-value to r-value conversion happens here because compound literals
> > > are l-values. Since `g` is a global of a non-trivial C struct type, we
> > > shouldn't try to push a cleanup and destruct the object.
> > >
> > > We don't have to think about the C++ case since the line below checks the
> > > type is a non-trivial C type. I didn't think about statement expressions,
> > > but they aren't allowed in file scope, so I guess that's not a problem
> > > either.
> > I would hope that the constant-evaluator here might be smart enough to
> > ignore some elidable temporaries.
> Do you mean the constant-evaluator should evaluate initializers that are
> `ExprWithCleanups` to constants in a case like this? It's possible to do so
> by seeing whether the sub-expression of `ExprWithCleanups` is constant, but
> it looks like we also have to set the `CleanupInfo::CleanupsHaveSideEffects`
> flag to false when it's a file scope expression so that
> `ConstExprEmitter::VisitExprWithCleanups` can constant-fold the
> `ExprWithCleanups` initializer.
In this case, what we're doing is eliding a "temporary" (really an object with
wider lifetime, but if the object isn't referenceable, we can compile as-if the
object is really temporary) and therefore bypassing the need for a cleanup
entirely. I wouldn't expect the AST to change to reflect that this is
possible, just the constant evaluator. Basically, it needs to look through
`ExprWithCleanups`; we probably already peephole the
`LValueToRValue(CompoundLiteralExpr)` combination.
We do try to do constant-evaluation on local initializers as well as an
optimization, and abstractly that should also work with these constructs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak marked an inline comment as done.
ahatanak added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:677
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > ahatanak wrote:
> > > > rjmccall wrote:
> > > > > Why only when the l-value is volatile?
> > > > I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed
> > > > and it seemed that it wasn't needed for non-volatile types. I'm not
> > > > sure it's important, so I've removed the check for volatile. Also,
> > > > `ExprWithCleanups` shouldn't' be emitted when this is in file scope, so
> > > > I fixed that too.
> > > Hmm, not sure about this file-scope thing, since the combination of C++
> > > dynamic initializers and statement-expressions means we can have pretty
> > > unrestricted code there.
> > I should have explained why this was needed, but I wanted to prevent
> > emitting `ExprWithCleanups` in the following example:
> >
> > ```
> > struct A {
> > id f0;
> > };
> >
> > typedef struct A A;
> >
> > A g = (A){ .f0 = 0 };
> > ```
> >
> > The l-value to r-value conversion happens here because compound literals
> > are l-values. Since `g` is a global of a non-trivial C struct type, we
> > shouldn't try to push a cleanup and destruct the object.
> >
> > We don't have to think about the C++ case since the line below checks the
> > type is a non-trivial C type. I didn't think about statement expressions,
> > but they aren't allowed in file scope, so I guess that's not a problem
> > either.
> I would hope that the constant-evaluator here might be smart enough to ignore
> some elidable temporaries.
Do you mean the constant-evaluator should evaluate initializers that are
`ExprWithCleanups` to constants in a case like this? It's possible to do so by
seeing whether the sub-expression of `ExprWithCleanups` is constant, but it
looks like we also have to set the `CleanupInfo::CleanupsHaveSideEffects` flag
to false when it's a file scope expression so that
`ConstExprEmitter::VisitExprWithCleanups` can constant-fold the
`ExprWithCleanups` initializer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:677
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+
ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > Why only when the l-value is volatile?
> > > I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed
> > > and it seemed that it wasn't needed for non-volatile types. I'm not sure
> > > it's important, so I've removed the check for volatile. Also,
> > > `ExprWithCleanups` shouldn't' be emitted when this is in file scope, so I
> > > fixed that too.
> > Hmm, not sure about this file-scope thing, since the combination of C++
> > dynamic initializers and statement-expressions means we can have pretty
> > unrestricted code there.
> I should have explained why this was needed, but I wanted to prevent emitting
> `ExprWithCleanups` in the following example:
>
> ```
> struct A {
> id f0;
> };
>
> typedef struct A A;
>
> A g = (A){ .f0 = 0 };
> ```
>
> The l-value to r-value conversion happens here because compound literals are
> l-values. Since `g` is a global of a non-trivial C struct type, we shouldn't
> try to push a cleanup and destruct the object.
>
> We don't have to think about the C++ case since the line below checks the
> type is a non-trivial C type. I didn't think about statement expressions, but
> they aren't allowed in file scope, so I guess that's not a problem either.
I would hope that the constant-evaluator here might be smart enough to ignore
some elidable temporaries.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak marked an inline comment as done.
ahatanak added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:677
+ E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+Cleanup.setExprNeedsCleanups(true);
+
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Why only when the l-value is volatile?
> > I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed and
> > it seemed that it wasn't needed for non-volatile types. I'm not sure it's
> > important, so I've removed the check for volatile. Also, `ExprWithCleanups`
> > shouldn't' be emitted when this is in file scope, so I fixed that too.
> Hmm, not sure about this file-scope thing, since the combination of C++
> dynamic initializers and statement-expressions means we can have pretty
> unrestricted code there.
I should have explained why this was needed, but I wanted to prevent emitting
`ExprWithCleanups` in the following example:
```
struct A {
id f0;
};
typedef struct A A;
A g = (A){ .f0 = 0 };
```
The l-value to r-value conversion happens here because compound literals are
l-values. Since `g` is a global of a non-trivial C struct type, we shouldn't
try to push a cleanup and destruct the object.
We don't have to think about the C++ case since the line below checks the type
is a non-trivial C type. I didn't think about statement expressions, but they
aren't allowed in file scope, so I guess that's not a problem either.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:677 + E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) +Cleanup.setExprNeedsCleanups(true); + ahatanak wrote: > rjmccall wrote: > > Why only when the l-value is volatile? > I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed and > it seemed that it wasn't needed for non-volatile types. I'm not sure it's > important, so I've removed the check for volatile. Also, `ExprWithCleanups` > shouldn't' be emitted when this is in file scope, so I fixed that too. Hmm, not sure about this file-scope thing, since the combination of C++ dynamic initializers and statement-expressions means we can have pretty unrestricted code there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:677 + E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) +Cleanup.setExprNeedsCleanups(true); + rjmccall wrote: > Why only when the l-value is volatile? I was trying to avoid emitting `ExprWithCleanups` when it wasn't needed and it seemed that it wasn't needed for non-volatile types. I'm not sure it's important, so I've removed the check for volatile. Also, `ExprWithCleanups` shouldn't' be emitted when this is in file scope, so I fixed that too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak updated this revision to Diff 250934.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,6 +89,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -476,6 +483,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -520,7 +539,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -808,4 +829,62 @@
func(0);
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second call.
- //
// CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
// CHECK: call void
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:677 + E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) +Cleanup.setExprNeedsCleanups(true); + Why only when the l-value is volatile? Comment at: clang/lib/Sema/SemaExpr.cpp:5733 + QualType::DK_nontrivial_c_struct) +Cleanup.setExprNeedsCleanups(true); + You should do this in `MaybeBindToTemporary`, and then you probably don't need it in most of these other places. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak updated this revision to Diff 249190.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Fix the way `IsExternallyDestructed` is used.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,6 +89,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -476,6 +483,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -520,7 +539,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -709,4 +730,62 @@
VolatileArray t = *a;
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second call.
- //
// CHECK: %[[T:[^ ]+]] = bitcast
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGExprAgg.cpp:822 +Dest.setExternallyDestructed(); + } + I don't think `setExternallyDestructed` can be used to communicate outwards like this; the code isn't set up to just do modifications on a single `AggValueSlot` that's passed around by reference. Instead, the flags are used to communicate downwards to the callees, and the expectation needs to be that callees will push a destructor when they're done initializing unless `isExternallyDestructed` is set on the dest slot they receive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak updated this revision to Diff 248363.
ahatanak marked 2 inline comments as done.
ahatanak removed a project: LLVM.
ahatanak removed a subscriber: llvm-commits.
ahatanak added a comment.
Herald added a subscriber: ributzka.
Address review comments.
- Declare the flags in `ReturnValueSlot` as bitfields.
- Replace flag `RequiresDestruction` with flag `IsExternallyDestructed` and
move the `pushDestroy` call to `EmitCall`. I audited all the places where
`ReturnValueSlot` is constructed and there were only a few places where
`IsExternallyDestructed` had to be set to true.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -89,6 +89,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+- (StrongSmall)getStrongSmall;
++ (StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -476,6 +483,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -520,7 +539,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -709,4 +730,66 @@
VolatileArray t = *a;
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{
+StrongSmall s;
+return s;
+ }()
+.f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGCall.h:372 + REQUIRES_DESTRUCTION = 0x4, }; Does `llvm::Value*` actually guarantee three bits? Presumably on 64-bit platforms, but we do support 32-bit hosts. Fortunately, I don't actually think there's any reason to be space-conscious in this class; all the values are short-lived. Might as well pull all the fields out into a bit-field or something. Comment at: clang/lib/CodeGen/CGCall.h:390 + return Value.getInt() & REQUIRES_DESTRUCTION; +} }; I think the "is externally destructed" semantics make more sense — that is, the *default* should be that a destructor needs to get pushed, and maybe specific contexts should suppress that. That will also let you avoid all the ugly code to compute whether destruction is required in a bunch of generic places. Actually, "externally destructed" is a really problematic idea; we need to come up with a better solution for contexts that need to be careful about destructor ordering like this, and it should probably be based on passing around and deactivating destructors. But that's for later. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66094/new/ https://reviews.llvm.org/D66094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak updated this revision to Diff 214660.
ahatanak added a comment.
Revert changes I made to llvm that are unrelated to this patch.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66094/new/
https://reviews.llvm.org/D66094
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -90,6 +90,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -477,6 +484,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -521,7 +540,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -718,4 +739,62 @@
VolatileArray t = *a;
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
// CHECK-LABEL: define void @test71
void test71(void) {
- // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
- // the following lifetime.end) came before the second call.
- //
// CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
// CHECK: call void
[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: llvm-commits, dexonsmith, jkorous.
Herald added a project: LLVM.
This is the patch I split out of https://reviews.llvm.org/D64464.
The cleanup is pushed in `EmitCallExpr` and `EmitObjCMessageExpr` so that the
destructor is called to destruct function call and ObjC message returns. I also
added test cases for block function calls since the patch in
https://reviews.llvm.org/D64464 wasn't handling that case correctly.
rdar://problem/51867864
Repository:
rC Clang
https://reviews.llvm.org/D66094
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGCall.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/Parse/ParseObjc.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/strong-in-c-struct.m
llvm/test/Bitcode/upgrade-arc-runtime-calls.bc
llvm/test/Bitcode/upgrade-mrr-runtime-calls.bc
Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -90,6 +90,13 @@
void calleeStrongSmall(StrongSmall);
void func(Strong *);
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
// CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
// CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
// CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -477,6 +484,18 @@
getStrongSmall();
}
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+ [c getStrongSmall];
+}
+
// CHECK: define void @test_copy_constructor_StrongBlock(
// CHECK: call void @__copy_constructor_8_8_sb0(
// CHECK: call void @__destructor_8_sb0(
@@ -521,7 +540,9 @@
// CHECK: define void @test_copy_constructor_StrongVolatile0(
// CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
// CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
// CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -718,4 +739,62 @@
VolatileArray t = *a;
}
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+ g0 = getStrongSmall().f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+ g0 = [c getStrongSmall].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+ g0 = [C getStrongSmallClass].f1;
+ func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+ g0 = ^{ StrongSmall s; return s; }().f1;
+ func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+ (void)*a;
+ func(0);
+}
+
#endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
