[polly] r248688 - BlockGenerator: Be less agressive with deleting dead instructions

Sun Sep 27 12:50:17 PDT 2015

Author: grosser
Date: Sun Sep 27 14:50:16 2015
New Revision: 248688

URL: http://llvm.org/viewvc/llvm-project?rev=248688&view=rev
Log:
BlockGenerator: Be less agressive with deleting dead instructions

We now only delete trivially dead instructions in the BB we copy (copyBB), but
not in any other BB.  Only for copyBB we know that there will _never_ be any
future uses of instructions that have no use after copyBB has been generated.
Other instructions in the AST that have been generated by IslNodeBuilder may
look dead at the moment, but may possibly still be referenced by GlobalMaps. If
we delete them now, later uses would break surprisingly.

We do not have a test case that breaks due to us deleting too many instructions.
This issue was found by inspection.

Modified:
    polly/trunk/lib/CodeGen/BlockGenerators.cpp
    polly/trunk/test/Isl/CodeGen/MemAccess/codegen_address_space.ll
    polly/trunk/test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll

Modified: polly/trunk/lib/CodeGen/BlockGenerators.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/BlockGenerators.cpp?rev=248688&r1=248687&r2=248688&view=diff
==============================================================================

--- polly/trunk/lib/CodeGen/BlockGenerators.cpp (original)
+++ polly/trunk/lib/CodeGen/BlockGenerators.cpp Sun Sep 27 14:50:16 2015
@@ -285,6 +285,28 @@ void BlockGenerator::copyInstruction(Sco
   copyInstScalar(Stmt, Inst, BBMap, LTS);
 }
 
+/// @brief Remove trivially dead instructions from BB
+///
+/// This function drops trivially dead instructions from a basic block. It
+/// on purpose does _not_ recurse into other BBs even if the deletion of
+/// instructions in this basic block can make instructions in other basic blocks
+/// triviall dead.
+static void simplifyInstsInBlockOnly(BasicBlock *BB) {
+  auto BI = --BB->end(), BE = BB->begin();
+  bool Exit = false;
+  while (!Exit) {
+    auto ToRemove = BI;
+    if (BI != BE)
+      BI--;
+    else
+      Exit = true;
+
+    if (!isInstructionTriviallyDead(ToRemove))
+      continue;
+    ToRemove->eraseFromParent();
+  }
+}
+
 void BlockGenerator::copyStmt(ScopStmt &Stmt, LoopToScevMapT &LTS,
                               isl_id_to_ast_expr *NewAccesses) {
   assert(Stmt.isBlockStmt() &&
@@ -296,7 +318,13 @@ void BlockGenerator::copyStmt(ScopStmt &
   copyBB(Stmt, BB, BBMap, LTS, NewAccesses);
 
   auto CopyBB = Builder.GetInsertBlock();
-  SimplifyInstructionsInBlock(CopyBB, nullptr);
+  // Delete trivially dead instructions in CopyBB, but not in any other BB.
+  // Only for copyBB we know that there will _never_ be any future uses of
+  // instructions that have no use after copyBB has finished. Other instructions
+  // in the AST that have been generated by IslNodeBuilder may look dead at
+  // the moment, but may possibly still be referenced by GlobalMaps. If we
+  // delete them now, later uses would break surprisingly.
+  simplifyInstsInBlockOnly(CopyBB);
   Builder.SetInsertPoint(CopyBB->getTerminator());
 }
 
@@ -1090,8 +1118,14 @@ void RegionGenerator::copyStmt(ScopStmt
     LTS[L] = SE.getUnknown(LoopPHI);
   }
 
+  // Delete trivially dead instructions in CopyBB, but not in any other BB.
+  // Only for copyBB we know that there will _never_ be any future uses of
+  // instructions that have no use after copyBB has finished. Other instructions
+  // in the AST that have been generated by IslNodeBuilder may look dead at
+  // the moment, but may possibly still be referenced by GlobalMaps. If we
+  // delete them now, later uses would break surprisingly.
   for (auto *BB : SeenBlocks)
-    SimplifyInstructionsInBlock(BlockMap[BB], nullptr);
+    simplifyInstsInBlockOnly(BlockMap[BB]);
 
   // Reset the old insert point for the build.
   Builder.SetInsertPoint(ExitBBCopy->begin());

Modified: polly/trunk/test/Isl/CodeGen/MemAccess/codegen_address_space.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/Isl/CodeGen/MemAccess/codegen_address_space.ll?rev=248688&r1=248687&r2=248688&view=diff
==============================================================================
--- polly/trunk/test/Isl/CodeGen/MemAccess/codegen_address_space.ll (original)
+++ polly/trunk/test/Isl/CodeGen/MemAccess/codegen_address_space.ll Sun Sep 27 14:50:16 2015
@@ -40,4 +40,5 @@ for.end:
 }
 
 ; CHECK: %polly.access.cast.A = bitcast [100 x i32] addrspace(5)* %A to i32 addrspace(5)*
-; CHECK: %tmp2_p_scalar_ = load i32, i32 addrspace(5)* %polly.access.cast.A, align 4, !alias.scope !0, !noalias !2
+; CHECK: %polly.access.A = getelementptr i32, i32 addrspace(5)* %polly.access.cast.A, i64 0
+; CHECK: %tmp2_p_scalar_ = load i32, i32 addrspace(5)* %polly.access.A, align 4, !alias.scope !0, !noalias !2

Modified: polly/trunk/test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll?rev=248688&r1=248687&r2=248688&view=diff
==============================================================================
--- polly/trunk/test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll (original)
+++ polly/trunk/test/Isl/CodeGen/non-affine-phi-node-expansion-3.ll Sun Sep 27 14:50:16 2015
@@ -13,9 +13,12 @@ loop:
   br i1 %cond0, label %branch1, label %backedge
 
 ; CHECK-LABEL: polly.stmt.loop:
-; CHECK-NEXT:    store float 3.000000e+00, float* %merge.phiops
-; CHECK-NEXT:    store float 3.000000e+00, float* %val1.s2a
-; CHECK-NEXT:    store float 3.000000e+00, float* %val2.s2a
+; CHECK-NEXT: %p_val0 = fadd float 1.000000e+00, 2.000000e+00
+; CHECK-NEXT: %p_val1 = fadd float 1.000000e+00, 2.000000e+00
+; CHECK-NEXT: %p_val2 = fadd float 1.000000e+00, 2.000000e+00
+; CHECK-NEXT: store float %p_val0, float* %merge.phiops
+; CHECK-NEXT: store float %p_val1, float* %val1.s2a
+; CHECK-NEXT: store float %p_val2, float* %val2.s2a
 
 ; FIXME -> The last two writes are not really needed and can be dropped if the
 ;          incoming block of the PHI and the value that is used share the same
@@ -25,13 +28,13 @@ branch1:
   br i1 %cond1, label %branch2, label %backedge
 
 ; CHECK-LABEL: polly.stmt.branch1:
-; CHECK-NEXT:    store float 3.000000e+00, float* %merge.phiops
+; CHECK-NEXT:    store float %p_val1, float* %merge.phiops
 
 branch2:
   br label %backedge
 
 ; CHECK-LABEL: polly.stmt.branch2:
-; CHECK-NEXT:    store float 3.000000e+00, float* %merge.phiops
+; CHECK-NEXT:    store float %p_val2, float* %merge.phiops
 
 backedge:
   %merge = phi float [%val0, %loop], [%val1, %branch1], [%val2, %branch2]


