[cfe-commits] r89773 - in /cfe/trunk: lib/CodeGen/CGStmt.cpp lib/CodeGen/CodeGenFunction.h test/CodeGenCXX/condition.cpp

[Daniel Dunbar]

Hi Doug,

On Tue, Nov 24, 2009 at 8:43 AM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Tue Nov 24 10:43:22 2009
> New Revision: 89773
>
> URL: http://llvm.org/viewvc/llvm-project?rev=89773&view=rev
> Log:
> Introduce cleanup scopes for "if" statements in two places:
>  - Outside the "if", to ensure that we destroy the condition variable
>    at the end of the "if" statement rather than at the end of the
>    block containing the "if" statement.

Yay!

>  - Inside the "then" and "else" branches, so that we emit then- or
>    else-local cleanups at the end of the corresponding block when the
>    block is not a compound statement.

I can't convince myself these are necessary. Cleanups are run in FIFO
order so how is this different than putting them on the cleanup for
the containing statement, since no code is run after the then/else
branches terminate.

> To make adding these new cleanup scopes easier (and since
> switch/do/while will all need the same treatment), added the
> CleanupScope RAII object to introduce a new cleanup scope and make
> sure it gets cleaned up.

Nice, thanks.

 - Daniel

>
> Added:
>    cfe/trunk/test/CodeGenCXX/condition.cpp   (with props)
> Modified:
>    cfe/trunk/lib/CodeGen/CGStmt.cpp
>    cfe/trunk/lib/CodeGen/CodeGenFunction.h
>
> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=89773&r1=89772&r2=89773&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Tue Nov 24 10:43:22 2009
> @@ -153,9 +153,7 @@
>   }
>
>   // Keep track of the current cleanup stack depth.
> -  size_t CleanupStackDepth = CleanupEntries.size();
> -  bool OldDidCallStackSave = DidCallStackSave;
> -  DidCallStackSave = false;
> +  CleanupScope Scope(*this);
>
>   for (CompoundStmt::const_body_iterator I = S.body_begin(),
>        E = S.body_end()-GetLast; I != E; ++I)
> @@ -185,10 +183,6 @@
>     RV = EmitAnyExpr(cast<Expr>(LastStmt), AggLoc);
>   }
>
> -  DidCallStackSave = OldDidCallStackSave;
> -
> -  EmitCleanupBlocks(CleanupStackDepth);
> -
>   return RV;
>  }
>
> @@ -294,8 +288,10 @@
>  void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
>   // C99 6.8.4.1: The first substatement is executed if the expression compares
>   // unequal to 0.  The condition must be a scalar type.
> +  CleanupScope ConditionScope(*this);
> +
>   if (S.getConditionVariable())
> -    EmitDecl(*S.getConditionVariable());
> +    EmitLocalBlockVarDecl(*S.getConditionVariable());
>
>   // If the condition constant folds and can be elided, try to avoid emitting
>   // the condition and the dead arm of the if/else.
> @@ -308,8 +304,10 @@
>     // If the skipped block has no labels in it, just emit the executed block.
>     // This avoids emitting dead code and simplifies the CFG substantially.
>     if (!ContainsLabel(Skipped)) {
> -      if (Executed)
> +      if (Executed) {
> +        CleanupScope ExecutedScope(*this);
>         EmitStmt(Executed);
> +      }
>       return;
>     }
>   }
> @@ -324,14 +322,20 @@
>   EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock);
>
>   // Emit the 'then' code.
> -  EmitBlock(ThenBlock);
> -  EmitStmt(S.getThen());
> +  EmitBlock(ThenBlock);
> +  {
> +    CleanupScope ThenScope(*this);
> +    EmitStmt(S.getThen());
> +  }
>   EmitBranch(ContBlock);
>
>   // Emit the 'else' code if present.
>   if (const Stmt *Else = S.getElse()) {
>     EmitBlock(ElseBlock);
> -    EmitStmt(Else);
> +    {
> +      CleanupScope ElseScope(*this);
> +      EmitStmt(Else);
> +    }
>     EmitBranch(ContBlock);
>   }
>
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=89773&r1=89772&r2=89773&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Nov 24 10:43:22 2009
> @@ -165,6 +165,31 @@
>     }
>   };
>
> +  /// \brief Enters a new scope for capturing cleanups, all of which will be
> +  /// executed once the scope is exited.
> +  class CleanupScope {
> +    CodeGenFunction& CGF;
> +    size_t CleanupStackDepth;
> +    bool OldDidCallStackSave;
> +
> +    CleanupScope(const CleanupScope &); // DO NOT IMPLEMENT
> +    CleanupScope &operator=(const CleanupScope &); // DO NOT IMPLEMENT
> +
> +  public:
> +    /// \brief Enter a new cleanup scope.
> +    explicit CleanupScope(CodeGenFunction &CGF) : CGF(CGF) {
> +      CleanupStackDepth = CGF.CleanupEntries.size();
> +      OldDidCallStackSave = CGF.DidCallStackSave;
> +    }
> +
> +    /// \brief Exit this cleanup scope, emitting any accumulated
> +    /// cleanups.
> +    ~CleanupScope() {
> +      CGF.DidCallStackSave = OldDidCallStackSave;
> +      CGF.EmitCleanupBlocks(CleanupStackDepth);
> +    }
> +  };
> +
>   /// EmitCleanupBlocks - Takes the old cleanup stack size and emits the cleanup
>   /// blocks that have been added.
>   void EmitCleanupBlocks(size_t OldCleanupStackSize);
>
> Added: cfe/trunk/test/CodeGenCXX/condition.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/condition.cpp?rev=89773&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/condition.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/condition.cpp Tue Nov 24 10:43:22 2009
> @@ -0,0 +1,47 @@
> +// RUN: clang-cc -triple x86_64-apple-darwin10 -emit-llvm -o - %s | FileCheck %s
> +void *f();
> +
> +template <typename T> T* g() {
> + if (T* t = f())
> +   return t;
> +
> + return 0;
> +}
> +
> +void h() {
> + void *a = g<void>();
> +}
> +
> +struct X {
> +  X();
> +  ~X();
> +  operator bool();
> +};
> +
> +struct Y {
> +  Y();
> +  ~Y();
> +};
> +
> +void if_destruct(int z) {
> +  // Verify that the condition variable is destroyed at the end of the
> +  // "if" statement.
> +  // CHECK: call void @_ZN1XC1Ev
> +  // CHECK: call zeroext i1 @_ZN1XcvbEv
> +  if (X x = X()) {
> +    // CHECK: store i32 18
> +    z = 18;
> +  }
> +  // CHECK: call void @_ZN1XD1Ev
> +  // CHECK: store i32 17
> +  z = 17;
> +
> +  // CHECK: call void @_ZN1XC1Ev
> +  if (X x = X())
> +    Y y;
> +  // CHECK: if.then
> +  // CHECK: call  void @_ZN1YC1Ev
> +  // CHECK: call  void @_ZN1YD1Ev
> +  // CHECK: if.end
> +  // CHECK: call  void @_ZN1XD1Ev
> +}
>
> Propchange: cfe/trunk/test/CodeGenCXX/condition.cpp
>
> ------------------------------------------------------------------------------
>    svn:eol-style = native
>
> Propchange: cfe/trunk/test/CodeGenCXX/condition.cpp
>
> ------------------------------------------------------------------------------
>    svn:keywords = Id
>
> Propchange: cfe/trunk/test/CodeGenCXX/condition.cpp
>
> ------------------------------------------------------------------------------
>    svn:mime-type = text/plain
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>