[polly] r299585 - Remove llvm.lifetime.start/end in original region.

[Tobias Grosser via llvm-commits]

On Wed, Apr 5, 2017, at 10:10 PM, Michael Kruse via llvm-commits wrote:
> Author: meinersbur
> Date: Wed Apr  5 15:09:59 2017
> New Revision: 299585
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=299585&view=rev
> Log:
> Remove llvm.lifetime.start/end in original region.
> 
> The current StackColoring algorithm does not correctly handle the
> situation when some, but not all paths from a BB to the entry node
> cross a llvm.lifetime.start. According to an interpretation of the
> language reference at
> http://llvm.org/docs/LangRef.html#llvm-lifetime-start-intrinsic
> this might be correct, but it would cost too much effort to handle
> in StackColoring.
> 
> To be on the safe side, remove all lifetime markers even in the original
> code version (they have never been copied to the optimized version)
> to ensure that no path to the entry block will cross a
> llvm.lifetime.start.
> 
> The same principle applies to paths the a function return and the
> llvm.lifetime.end marker, so we remove them as well.

Hi Michael,

I am not sure this is correct. What happens if there is on a different
branch to the end node another lifetime marker outside of the region we
are looking at? This marker will still be there, such that if we drop
lifetime markers just from inside the region, there will be paths from
the entry that either cross or do not cross start markers before they
reach end.

Best,
Tobias

> 
> This fixes llvm.org/PR32251.
> 
> Also see the discussion at
> http://lists.llvm.org/pipermail/llvm-dev/2017-March/111551.html
> 
> Modified:
>     polly/trunk/lib/CodeGen/CodeGeneration.cpp
>     polly/trunk/test/Isl/CodeGen/intrinsics_lifetime.ll
> 
> Modified: polly/trunk/lib/CodeGen/CodeGeneration.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/CodeGeneration.cpp?rev=299585&r1=299584&r2=299585&view=diff
> ==============================================================================
> --- polly/trunk/lib/CodeGen/CodeGeneration.cpp (original)
> +++ polly/trunk/lib/CodeGen/CodeGeneration.cpp Wed Apr  5 15:09:59 2017
> @@ -112,6 +112,59 @@ public:
>      OrigTerminator->eraseFromParent();
>    }
>  
> +  /// Remove all lifetime markers (llvm.lifetime.start,
> llvm.lifetime.end) from
> +  /// @R.
> +  ///
> +  /// CodeGeneration does not copy lifetime markers into the optimized
> SCoP,
> +  /// which would leave the them only in the original path. This can
> transform
> +  /// code such as
> +  ///
> +  ///     llvm.lifetime.start(%p)
> +  ///     llvm.lifetime.end(%p)
> +  ///
> +  /// into
> +  ///
> +  ///     if (RTC) {
> +  ///       // generated code
> +  ///     } else {
> +  ///       // original code
> +  ///       llvm.lifetime.start(%p)
> +  ///     }
> +  ///     llvm.lifetime.end(%p)
> +  ///
> +  /// The current StackColoring algorithm cannot handle if some, but not
> all,
> +  /// paths from the end marker to the entry block cross the start
> marker. Same
> +  /// for start markers that do not always cross the end markers. We
> avoid any
> +  /// issues by removing all lifetime markers, even from the original
> code.
> +  ///
> +  /// A better solution could be to hoist all llvm.lifetime.start to the
> split
> +  /// node and all llvm.lifetime.end to the merge node, which should be
> +  /// conservatively correct.
> +  void removeLifetimeMarkers(Region *R) {
> +    for (auto *BB : R->blocks()) {
> +      auto InstIt = BB->begin();
> +      auto InstEnd = BB->end();
> +
> +      while (InstIt != InstEnd) {
> +        auto NextIt = InstIt;
> +        ++NextIt;
> +
> +        if (auto *IT = dyn_cast<IntrinsicInst>(&*InstIt)) {
> +          switch (IT->getIntrinsicID()) {
> +          case llvm::Intrinsic::lifetime_start:
> +          case llvm::Intrinsic::lifetime_end:
> +            BB->getInstList().erase(InstIt);
> +            break;
> +          default:
> +            break;
> +          }
> +        }
> +
> +        InstIt = NextIt;
> +      }
> +    }
> +  }
> +
>    /// Generate LLVM-IR for the SCoP @p S.
>    bool runOnScop(Scop &S) override {
>      AI = &getAnalysis<IslAstInfo>();
> @@ -146,6 +199,7 @@ public:
>      // code generating this scop.
>      BasicBlock *StartBlock =
>          executeScopConditionally(S, Builder.getTrue(), *DT, *RI, *LI);
> +    removeLifetimeMarkers(R);
>      auto *SplitBlock = StartBlock->getSinglePredecessor();
>  
>      IslNodeBuilder NodeBuilder(Builder, Annotator, *DL, *LI, *SE, *DT,
>      S,
> 
> Modified: polly/trunk/test/Isl/CodeGen/intrinsics_lifetime.ll
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/test/Isl/CodeGen/intrinsics_lifetime.ll?rev=299585&r1=299584&r2=299585&view=diff
> ==============================================================================
> --- polly/trunk/test/Isl/CodeGen/intrinsics_lifetime.ll (original)
> +++ polly/trunk/test/Isl/CodeGen/intrinsics_lifetime.ll Wed Apr  5
> 15:09:59 2017
> @@ -1,11 +1,7 @@
>  ; RUN: opt %loadPolly -basicaa -polly-codegen -S < %s | FileCheck %s
>  ;
> -; Verify that we remove the lifetime markers from the optimized SCoP.
> +; Verify that we remove the lifetime markers from everywhere.
>  ;
> -; CHECK: for.body:
> -; CHECK:   call void @llvm.lifetime.start
> -; CHECK: for.end:
> -; CHECK:   call void @llvm.lifetime.end
>  ; CHECK-NOT: call void @llvm.lifetime.start
>  ; CHECK-NOT: call void @llvm.lifetime.end
>  ;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits