ahatanak updated this revision to Diff 437248.
ahatanak added a comment.
Stop wrapping a `GCCAsmStmt` with an `ExprWithCleanups`. Instead, just pop the
cleanups after the asm statement to destruct the temporaries.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125936/new/
https://reviews.llvm.org/D125936
Files:
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/test/CodeGenCXX/asm.cpp
clang/test/SemaTemplate/instantiate-expr-1.cpp
Index: clang/test/SemaTemplate/instantiate-expr-1.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-expr-1.cpp
+++ clang/test/SemaTemplate/instantiate-expr-1.cpp
@@ -190,3 +190,19 @@
test_asm_tied(1.f); // expected-note {{instantiation of}}
}
}
+
+namespace TestAsmCleanup {
+struct S {
+ operator int() const { return 1; }
+ ~S();
+};
+
+template <class T>
+void foo() {
+ __asm__ __volatile__("%[i]"
+ :
+ : [i] "r"(-S()));
+}
+
+void test() { foo<void>(); }
+} // namespace TestAsmCleanup
Index: clang/test/CodeGenCXX/asm.cpp
===================================================================
--- clang/test/CodeGenCXX/asm.cpp
+++ clang/test/CodeGenCXX/asm.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fblocks -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[STRUCT_A:.*]] = type { i8 }
struct A
{
@@ -12,3 +14,39 @@
asm("" : : "r"(foo(a)) ); // rdar://8540491
// CHECK: call void @_ZN1AD1Ev
}
+
+namespace TestTemplate {
+template <class T>
+void foo0(A &a) {
+ asm("" : : "r"(foo(a)) );
+ asm("");
+}
+
+// Check that the temporary is destructed after the first asm statement.
+
+// CHECK: define {{.*}}void @_ZN12TestTemplate4foo0IvEEvR1A(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_A]],
+// CHECK: %[[CALL:.*]] = call noundef i32 @_Z3foo1A({{.*}}%[[AGG_TMP]])
+// CHECK: call void asm sideeffect "", "r,~{dirflag},~{fpsr},~{flags}"(i32 %[[CALL]])
+// CHECK: call void @_ZN1AD1Ev({{.*}}%[[AGG_TMP]])
+// CHECK: call void asm sideeffect "",
+
+void test0(A &a) { foo0<void>(a); }
+
+// Check that the block capture is destructed at the end of the enclosing scope.
+
+// CHECK: define {{.*}}void @_ZN12TestTemplate4foo1IvEEv1A(
+// CHECK: %[[BLOCK:.*]] = alloca <{ ptr, i32, i32, ptr, ptr, %[[STRUCT_A]] }>, align 4
+// CHECK: %[[BLOCK_CAPTURED:.*]] = getelementptr inbounds <{ ptr, i32, i32, ptr, ptr, %[[STRUCT_A]] }>, ptr %[[BLOCK]], i32 0, i32 5
+// CHECK: call void asm sideeffect "", "r,~{dirflag},~{fpsr},~{flags}"(i32 %{{.*}})
+// CHECK: call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void @_ZN1AD1Ev({{.*}} %[[BLOCK_CAPTURED]])
+
+template <class T>
+void foo1(A a) {
+ asm("" : : "r"(^{ (void)a; return 0; }()));
+ asm("");
+}
+
+void test1(A &a) { foo1<void>(a); }
+} // namespace TestTemplate
Index: clang/lib/Sema/SemaStmtAsm.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -733,6 +733,9 @@
}
if (NS->isAsmGoto())
setFunctionHasBranchIntoScope();
+
+ CleanupVarDeclMarking();
+ DiscardCleanupsInEvaluationContext();
return NS;
}
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7283,26 +7283,6 @@
return E;
}
-Stmt *Sema::MaybeCreateStmtWithCleanups(Stmt *SubStmt) {
- assert(SubStmt && "sub-statement can't be null!");
-
- CleanupVarDeclMarking();
-
- if (!Cleanup.exprNeedsCleanups())
- return SubStmt;
-
- // FIXME: In order to attach the temporaries, wrap the statement into
- // a StmtExpr; currently this is only used for asm statements.
- // This is hacky, either create a new CXXStmtWithTemporaries statement or
- // a new AsmStmtWithTemporaries.
- CompoundStmt *CompStmt = CompoundStmt::Create(
- Context, SubStmt, SourceLocation(), SourceLocation());
- Expr *E = new (Context)
- StmtExpr(CompStmt, Context.VoidTy, SourceLocation(), SourceLocation(),
- /*FIXME TemplateDepth=*/0);
- return MaybeCreateExprWithCleanups(E);
-}
-
/// Process the expression contained within a decltype. For such expressions,
/// certain semantic checks on temporaries are delayed until this point, and
/// are omitted for the 'topmost' call in the decltype expression. If the
@@ -8803,12 +8783,6 @@
return MaybeCreateExprWithCleanups(FullExpr);
}
-StmtResult Sema::ActOnFinishFullStmt(Stmt *FullStmt) {
- if (!FullStmt) return StmtError();
-
- return MaybeCreateStmtWithCleanups(FullStmt);
-}
-
Sema::IfExistsResult
Sema::CheckMicrosoftIfExistsSymbol(Scope *S,
CXXScopeSpec &SS,
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1517,7 +1517,6 @@
} else if (HasRVoid) {
Fallthrough = S.BuildCoreturnStmt(FD.getLocation(), nullptr,
/*IsImplicit*/false);
- Fallthrough = S.ActOnFinishFullStmt(Fallthrough.get());
if (Fallthrough.isInvalid())
return false;
}
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -300,7 +300,6 @@
ProhibitAttributes(Attrs);
bool msAsm = false;
Res = ParseAsmStatement(msAsm);
- Res = Actions.ActOnFinishFullStmt(Res.get());
if (msAsm) return Res;
SemiError = "asm";
break;
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2286,6 +2286,10 @@
}
void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
+ // Temporaries created in GCC AsmStmts have to be destructed at the end of the
+ // statement.
+ CodeGenFunction::RunCleanupsScope Cleanups(*this);
+
// Assemble the final asm string.
std::string AsmString = S.generateAsmString(getContext());
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6591,7 +6591,6 @@
/// requires any cleanups, surround it with a ExprWithCleanups node.
/// Otherwise, just returns the passed-in expression.
Expr *MaybeCreateExprWithCleanups(Expr *SubExpr);
- Stmt *MaybeCreateStmtWithCleanups(Stmt *SubStmt);
ExprResult MaybeCreateExprWithCleanups(ExprResult SubExpr);
MaterializeTemporaryExpr *
@@ -6604,7 +6603,6 @@
}
ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
bool DiscardedValue, bool IsConstexpr = false);
- StmtResult ActOnFinishFullStmt(Stmt *Stmt);
// Marks SS invalid if it represents an incomplete type.
bool RequireCompleteDeclContext(CXXScopeSpec &SS, DeclContext *DC);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
