ahatanak updated this revision to Diff 83680.
ahatanak added a comment.
Herald added a subscriber: mgorny.

I added an AST analysis pass that is run before IRGen and decides which 
VarDecls need their lifetimes to be extended or disabled. It only looks at 
VarDecls which have their addresses taken and are declared after a label is 
seen. It extends the lifetimes of VarDecls declared in a compound statement if 
there are backward gotos that don't leave the scope of the compound statement. 
However, if there are gotos that jump from outside into the compound statement 
too, it drops all lifetime markers for those VarDecls since emitting 
lifetime.start at the beginning of the compound statement might break 
use-after-scope check. For example:

goto label2;

{

  // cannot move lifetime.start for "x" to the start of this block since the 
"goto label2" will
  // jump over lifetime.start.
  ...

label2:

  ...

label1:

  ...
  int x;
  foo1(&x);
  goto label1; // backward goto.

}


https://reviews.llvm.org/D27680

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/LifetimeExtend.cpp
  lib/CodeGen/LifetimeExtend.h
  test/CodeGen/lifetime2.c

Index: test/CodeGen/lifetime2.c
===================================================================
--- test/CodeGen/lifetime2.c
+++ test/CodeGen/lifetime2.c
@@ -1,5 +1,5 @@
-// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefixes=CHECK,O2
-// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0
+// RUN: %clang_cc1 -S -emit-llvm -o - -O2 -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,O2
+// RUN: %clang_cc1 -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0
 
 extern int bar(char *A, int n);
 
@@ -19,17 +19,16 @@
 
 // CHECK-LABEL: @no_goto_bypass
 void no_goto_bypass() {
+  // O2: @llvm.lifetime.start(i64 5
   // O2: @llvm.lifetime.start(i64 1
   char x;
 l1:
   bar(&x, 1);
-  // O2: @llvm.lifetime.start(i64 5
-  // O2: @llvm.lifetime.end(i64 5
   char y[5];
   bar(y, 5);
   goto l1;
   // Infinite loop
-  // O2-NOT: @llvm.lifetime.end(i64 1
+  // O2-NOT: @llvm.lifetime.end(
 }
 
 // CHECK-LABEL: @goto_bypass
@@ -89,3 +88,59 @@
 L:
   bar(&x, 1);
 }
+
+// O2-LABEL: define i32 @move_lifetime_start
+// O2: %i = alloca i32, align 4
+// O2: %[[BITCAST:[0-9]+]] = bitcast i32* %i to i8*
+// O2: call void @llvm.lifetime.start(i64 4, i8* %[[BITCAST]])
+// O2: br label %[[DESTLABEL:[a-z0-9]+]]
+// O2: [[DESTLABEL]]:
+// O2: br label %[[DESTLABEL]]
+// O2: %[[BITCAST2:[0-9]+]] = bitcast i32* %i to i8*
+// O2: call void @llvm.lifetime.end(i64 4, i8* %[[BITCAST2]])
+
+extern void foo2(int p);
+
+int move_lifetime_start(int a) {
+  int *p = 0;
+label1:
+  if (p) {
+    foo2(*p);
+    return 0;
+  }
+
+  int i = 999;
+  if (a != 2) {
+    p = &i;
+    goto label1;
+  }
+  return -1;
+}
+
+// O2-LABEL: define i32 @dont_move_lifetime_start
+// O2: %i = alloca i32, align 4
+// O2: br label %[[DESTLABEL:[a-z0-9]+]]
+// O2: [[DESTLABEL]]:
+// O2: %[[BITCAST:[0-9]+]] = bitcast i32* %i to i8*
+// O2: call void @llvm.lifetime.start(i64 4, i8* %[[BITCAST]])
+// O2: switch i32 %{{.*}}, label %{{.*}} [
+// O2-NEXT: i32 0, label %{{.*}}
+// O2-NEXT: i32 2, label %[[DESTLABEL]]
+
+int dont_move_lifetime_start(int a) {
+  int *p = 0;
+label1:
+  {
+    if (p) {
+      foo2(*p);
+      return 0;
+    }
+
+    int i = 999;
+    if (a != 2) {
+      p = &i;
+      goto label1;
+    }
+  }
+  return -1;
+}
Index: lib/CodeGen/LifetimeExtend.h
===================================================================
--- /dev/null
+++ lib/CodeGen/LifetimeExtend.h
@@ -0,0 +1,43 @@
+#ifndef LLVM_CLANG_LIB_CODEGEN_LIFETIMEEXTEND_H
+#define LLVM_CLANG_LIB_CODEGEN_LIFETIMEEXTEND_H
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+
+class CompoundStmt;
+class Stmt;
+class VarDecl;
+
+namespace CodeGen {
+class VarBypassDetector;
+
+class LifetimeExtend {
+public:
+  typedef llvm::SmallVector<const VarDecl *, 2> VarDeclList;
+  void init(Stmt *Body, const VarBypassDetector &Bypass);
+  const VarDeclList *extendedLifetimeVarDecls(const CompoundStmt *CS) const;
+  bool lifetimeExtendedOrDisabled(const VarDecl *VD) const;
+
+  void addExtendedOrDisabledLifetime(const VarDecl *VD) {
+    ExtendedOrDisabledLifetimes.insert(VD);
+  }
+
+  void addExtendedLifetime(const CompoundStmt *CS, const VarDecl *VD) {
+    ExtendedLifetimes[CS].push_back(VD);
+  }
+
+private:
+  // Lifetimes for these VarDecls are either extended or disabled.
+  llvm::SmallSet<const VarDecl *, 2> ExtendedOrDisabledLifetimes;
+
+  // These are the VarDecls whose lifetimes are emitted at the beginning of the
+  // compound statements with which they are associated.
+  llvm::DenseMap<const CompoundStmt *, VarDeclList> ExtendedLifetimes;
+};
+
+}
+}
+
+#endif
Index: lib/CodeGen/LifetimeExtend.cpp
===================================================================
--- /dev/null
+++ lib/CodeGen/LifetimeExtend.cpp
@@ -0,0 +1,214 @@
+#include "LifetimeExtend.h"
+#include "VarBypassDetector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+using namespace clang;
+using namespace CodeGen;
+
+namespace {
+
+class AnalyzeAST : public RecursiveASTVisitor<AnalyzeAST> {
+public:
+  bool dataTraverseStmtPost(Stmt *S);
+  bool VisitCompoundStmt(CompoundStmt *S);
+  bool VisitGotoStmt(GotoStmt *S);
+  bool VisitLabelStmt(LabelStmt *S);
+  bool VisitVarDecl(VarDecl *D);
+  bool VisitUnaryOperator(UnaryOperator *UO);
+  bool VisitImplicitCastExpr(ImplicitCastExpr *E);
+  void setAddressTaken(const DeclRefExpr *DRE);
+  void collectedVarDecls(LifetimeExtend &LE, const VarBypassDetector &Bypass);
+
+private:
+  struct CSInfo {
+    CSInfo(const CompoundStmt *P)
+        : Done(false), Parent(P), LabelSeen(false), JumpFromOutside(false),
+          JumpFromInside(false) {}
+    CSInfo() {}
+
+    bool Done;
+    const CompoundStmt *Parent;
+
+    // Gotos with unresolved destinations.
+    llvm::DenseSet<const GotoStmt *> UnresolvedGotos;
+
+    // This is set to true if a label was seen in this CompoundStmt (including
+    // nested CompoundStmts that have been visited).
+    bool LabelSeen;
+
+    // This is true if a goto that is not in this CompoundStmt jumps to a label
+    // in this CompoundStmt.
+    bool JumpFromOutside;
+
+    // This is true if a goto that is in this CompoundStmt (including
+    // CompoundStmts nested within this CompoundStmt) jumps backward to a label
+    // in this CompoundStmt.
+    bool JumpFromInside;
+  };
+
+  CSInfo &getCSInfo(const CompoundStmt *CS) {
+    return CSInfoMap[CS];
+  }
+
+  // Get current CompoundStmt info.
+  CSInfo &curCSInfo() {
+    return getCSInfo(CompoundStmtStack.back());
+  }
+
+  void transertToParent(const CSInfo &Info) {
+    auto *ParentCS = Info.Parent;
+    auto &ParentInfo = getCSInfo(ParentCS);
+    ParentInfo.LabelSeen |= Info.LabelSeen;
+    ParentInfo.UnresolvedGotos.insert(Info.UnresolvedGotos.begin(),
+                                      Info.UnresolvedGotos.end());
+    for (const auto *Goto : Info.UnresolvedGotos)
+      GotoToCS[Goto] = ParentCS;
+  }
+
+  llvm::DenseMap<const CompoundStmt *, CSInfo> CSInfoMap;
+  llvm::SmallVector<const CompoundStmt *, 2> CompoundStmtStack;
+
+  typedef llvm::PointerIntPair<const CompoundStmt *, 1, bool> CSIntP;
+  llvm::DenseMap<const VarDecl *, CSIntP> VarDecls;
+
+  llvm::DenseMap<const GotoStmt *, const CompoundStmt *> GotoToCS;
+  llvm::DenseMap<const LabelDecl *, const CompoundStmt *> LabelToCS;
+  typedef llvm::SmallVector<const GotoStmt *, 2> GotoList;
+  llvm::DenseMap<const LabelDecl *, GotoList> LabelToGotos;
+};
+
+}
+
+bool AnalyzeAST::dataTraverseStmtPost(Stmt *S) {
+  if (isa<CompoundStmt>(S)) {
+    auto &Info = curCSInfo();
+    if (Info.Parent)
+      transertToParent(Info);
+    else
+      assert(LabelToGotos.empty() && Info.UnresolvedGotos.empty() &&
+             "Unresolved gotos exist");
+
+    Info.Done = true;
+    CompoundStmtStack.pop_back();
+  }
+  return true;
+}
+
+bool AnalyzeAST::VisitCompoundStmt(CompoundStmt *S) {
+  const CompoundStmt *Parent = nullptr;
+
+  if (!CompoundStmtStack.empty())
+    Parent = CompoundStmtStack.back();
+
+  CSInfoMap[S] = CSInfo(Parent);
+  CompoundStmtStack.push_back(S);
+  return true;
+}
+
+bool AnalyzeAST::VisitGotoStmt(GotoStmt *S) {
+  const LabelDecl *Dst = S->getLabel();
+  auto I = LabelToCS.find(Dst);
+
+  // If the destination is unknown, add it to the list of unresolved gotos.
+  if (I == LabelToCS.end()) {
+    curCSInfo().UnresolvedGotos.insert(S);
+    GotoToCS[S] = CompoundStmtStack.back();
+    LabelToGotos[Dst].push_back(S);
+  } else {
+    auto &LabelCSInfo = getCSInfo(I->second);
+    if (LabelCSInfo.Done)
+      LabelCSInfo.JumpFromOutside = true;
+    else
+      LabelCSInfo.JumpFromInside = true;
+  }
+
+  return true;
+}
+
+bool AnalyzeAST::VisitLabelStmt(LabelStmt *S) {
+  const LabelDecl *LD = S->getDecl();
+  const auto *CurCS = CompoundStmtStack.back();
+
+  for (const auto *Goto : LabelToGotos[LD]) {
+    const auto *GotoCS = GotoToCS[Goto];
+    // Do nothing if GotoCS == CurCS as we aren't interested in gotos that
+    // jump forward.
+    if (GotoCS != CurCS)
+      curCSInfo().JumpFromOutside = true;
+    getCSInfo(GotoCS).UnresolvedGotos.erase(Goto);
+  }
+
+  LabelToGotos.erase(LD);
+  LabelToCS[LD] = CompoundStmtStack.back();
+  curCSInfo().LabelSeen = true;
+  return true;
+}
+
+bool AnalyzeAST::VisitVarDecl(VarDecl *D) {
+  if (curCSInfo().LabelSeen && D->hasLocalStorage() &&
+      D->getType()->isConstantSizeType())
+    VarDecls[D] = CSIntP(CompoundStmtStack.back(), false);
+
+  return true;
+}
+
+void AnalyzeAST::setAddressTaken(const DeclRefExpr *DRE) {
+  if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+    auto I = VarDecls.find(VD);
+    if (I != VarDecls.end())
+      I->second.setInt(true);
+  }
+}
+
+bool AnalyzeAST::VisitUnaryOperator(UnaryOperator *UO) {
+  if (UO->getOpcode() == UO_AddrOf)
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()))
+      setAddressTaken(DRE);
+
+  return true;
+}
+
+bool AnalyzeAST::VisitImplicitCastExpr(ImplicitCastExpr *E) {
+  if (E->getCastKind() == CK_ArrayToPointerDecay)
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(E->getSubExpr()))
+      setAddressTaken(DRE);
+
+  return true;
+}
+
+void AnalyzeAST::collectedVarDecls(LifetimeExtend &LE,
+                                   const VarBypassDetector &Bypass) {
+  for (auto &P : VarDecls) {
+    // If address is not taken, skip this VarDecl.
+    if (Bypass.IsBypassed(P.first) || !P.second.getInt())
+      continue;
+
+    const auto *CS = P.second.getPointer();
+    const auto &Info = getCSInfo(CS);
+
+    if (!Info.JumpFromInside)
+      continue;
+
+    LE.addExtendedOrDisabledLifetime(P.first);
+
+    if (!Info.JumpFromOutside)
+      LE.addExtendedLifetime(CS, P.first);
+  }
+}
+
+void LifetimeExtend::init(Stmt *Body, const VarBypassDetector &Bypass) {
+  AnalyzeAST Analyze;
+  Analyze.TraverseStmt(Body);
+  Analyze.collectedVarDecls(*this, Bypass);
+}
+
+const LifetimeExtend::VarDeclList *
+LifetimeExtend::extendedLifetimeVarDecls(const CompoundStmt *CS) const {
+  auto I = ExtendedLifetimes.find(CS);
+  return I == ExtendedLifetimes.end() ? nullptr : &I->second;
+}
+
+bool LifetimeExtend::lifetimeExtendedOrDisabled(const VarDecl *VD) const {
+  return ExtendedOrDisabledLifetimes.count(VD);
+}
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -21,6 +21,7 @@
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
 #include "EHScopeStack.h"
+#include "LifetimeExtend.h"
 #include "VarBypassDetector.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ExprCXX.h"
@@ -173,6 +174,8 @@
   // because of jumps.
   VarBypassDetector Bypasses;
 
+  LifetimeExtend LifetimeExtendInfo;
+
   /// \brief CGBuilder insert helper. This function is called after an
   /// instruction is created using Builder.
   void InsertHelper(llvm::Instruction *I, const llvm::Twine &Name,
@@ -1852,6 +1855,8 @@
   Address CreateTempAlloca(llvm::Type *Ty, CharUnits align,
                            const Twine &Name = "tmp");
 
+  Address createTempAlloca(const VarDecl &VD);
+
   /// CreateDefaultAlignedTempAlloca - This creates an alloca with the
   /// default ABI alignment of the given LLVM type.
   ///
@@ -2350,6 +2355,7 @@
       return CGF.emitBlockByrefAddress(Addr, Variable, /*forward*/ false);
     }
   };
+
   AutoVarEmission EmitAutoVarAlloca(const VarDecl &var);
   void EmitAutoVarInit(const AutoVarEmission &emission);
   void EmitAutoVarCleanups(const AutoVarEmission &emission);  
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1082,8 +1082,11 @@
 
   // Initialize helper which will detect jumps which can cause invalid lifetime
   // markers.
-  if (Body && ShouldEmitLifetimeMarkers)
+  if (Body && ShouldEmitLifetimeMarkers) {
     Bypasses.Init(Body);
+    if (!getLangOpts().CPlusPlus)
+      LifetimeExtendInfo.init(Body, Bypasses);
+  }
 
   // Emit the standard function prologue.
   StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
Index: lib/CodeGen/CMakeLists.txt
===================================================================
--- lib/CodeGen/CMakeLists.txt
+++ lib/CodeGen/CMakeLists.txt
@@ -77,6 +77,7 @@
   CodeGenTypes.cpp
   CoverageMappingGen.cpp
   ItaniumCXXABI.cpp
+  LifetimeExtend.cpp
   MicrosoftCXXABI.cpp
   ModuleBuilder.cpp
   ObjectFilePCHContainerOperations.cpp
Index: lib/CodeGen/CGStmt.cpp
===================================================================
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -372,6 +372,21 @@
                                               bool GetLast,
                                               AggValueSlot AggSlot) {
 
+  if (HaveInsertPoint()) {
+    if (const auto *VarDecls =
+            LifetimeExtendInfo.extendedLifetimeVarDecls(&S)) {
+      for (auto I = VarDecls->begin(), E = VarDecls->end(); I != E; ++I) {
+        Address Addr = createTempAlloca(**I);
+        auto *AllocaTy =
+            cast<llvm::AllocaInst>(Addr.getPointer())->getAllocatedType();
+        uint64_t AllocaSize = CGM.getDataLayout().getTypeAllocSize(AllocaTy);
+        if (auto *Size = EmitLifetimeStart(AllocaSize, Addr.getPointer()))
+          EHStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
+                                               Addr, Size);
+      }
+    }
+  }
+
   for (CompoundStmt::const_body_iterator I = S.body_begin(),
        E = S.body_end()-GetLast; I != E; ++I)
     EmitStmt(*I);
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -913,6 +913,35 @@
   C->setDoesNotThrow();
 }
 
+Address CodeGenFunction::createTempAlloca(const VarDecl &VD) {
+  // Create the alloca.  Note that we set the name separately from building the
+  // instruction so that it's there even in no-asserts builds.
+  auto I = LocalDeclMap.find(&VD);
+
+  if (I != LocalDeclMap.end())
+    return I->second;
+
+  QualType Ty = VD.getType();
+  bool IsByRef = VD.hasAttr<BlocksAttr>();
+  CharUnits Alignment = getContext().getDeclAlign(&VD);
+  llvm::Type *AllocaTy;
+
+  if (IsByRef) {
+    auto &ByrefInfo = getBlockByrefInfo(&VD);
+    AllocaTy = ByrefInfo.Type;
+    Alignment = ByrefInfo.ByrefAlignment;
+  } else {
+    AllocaTy = ConvertTypeForMem(Ty);
+  }
+
+  Address Addr = CreateTempAlloca(AllocaTy, Alignment);
+  Addr.getPointer()->setName(VD.getName());
+
+  setAddrOfLocalVar(&VD, Addr);
+
+  return Addr;
+}
+
 /// EmitAutoVarAlloca - Emit the alloca and debug information for a
 /// local variable.  Does not emit initialization or destruction.
 CodeGenFunction::AutoVarEmission
@@ -996,22 +1025,9 @@
         }
       }
     } else {
-      CharUnits allocaAlignment;
-      llvm::Type *allocaTy;
-      if (isByRef) {
-        auto &byrefInfo = getBlockByrefInfo(&D);
-        allocaTy = byrefInfo.Type;
-        allocaAlignment = byrefInfo.ByrefAlignment;
-      } else {
-        allocaTy = ConvertTypeForMem(Ty);
-        allocaAlignment = alignment;
-      }
-
-      // Create the alloca.  Note that we set the name separately from
-      // building the instruction so that it's there even in no-asserts
-      // builds.
-      address = CreateTempAlloca(allocaTy, allocaAlignment);
-      address.getPointer()->setName(D.getName());
+      address = createTempAlloca(D);
+      llvm::Type *allocaTy =
+          cast<llvm::AllocaInst>(address.getPointer())->getAllocatedType();
 
       // Don't emit lifetime markers for MSVC catch parameters. The lifetime of
       // the catch parameter starts in the catchpad instruction, and we can't
@@ -1026,7 +1042,8 @@
         // regions which need more efforts to handle them correctly. PR28267
         // This is rare case, but it's better just omit intrinsics than have
         // them incorrectly placed.
-        if (!Bypasses.IsBypassed(&D)) {
+        if (!Bypasses.IsBypassed(&D) &&
+            !LifetimeExtendInfo.lifetimeExtendedOrDisabled(&D)) {
           uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
           emission.SizeForLifetimeMarkers =
               EmitLifetimeStart(size, address.getPointer());
@@ -1067,7 +1084,8 @@
     address = Address(vla, alignment);
   }
 
-  setAddrOfLocalVar(&D, address);
+  if (!LocalDeclMap.count(&D))
+    setAddrOfLocalVar(&D, address);
   emission.Addr = address;
 
   // Emit debug info for local var declaration.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to