[PATCH] D41593: [RewriteStatepoints] Fix incorrect assertion

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: anna, dneilson, reames.

`RewriteStatepointsForGC` iterates over function blocks and their predecessors
in order of declaration. One of outcomes of this is that callsites are placed in
arbitrary order which has nothing to do with travelsar order.

On the other hand, function `recomputeLiveInValues` asserts that bases are
added to `Info.PointerToBase` before their deried pointers are updated. But
if call sites are processed in order different from RPOT, this is not necessarily
true. We cannot guarantee that the base was placed there before every
pointer derived from it. All we can guarantee is that this base was marked as
known base by this point.

This patch replaces the fact that we assert from checking that the base was
added to the map with assert that the base was marked as known base.


https://reviews.llvm.org/D41593

Files:
  lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  test/Transforms/RewriteStatepointsForGC/check_traversal_order.ll


Index: test/Transforms/RewriteStatepointsForGC/check_traversal_order.ll
===================================================================
--- test/Transforms/RewriteStatepointsForGC/check_traversal_order.ll
+++ test/Transforms/RewriteStatepointsForGC/check_traversal_order.ll
@@ -0,0 +1,44 @@
+; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @f()
+declare void @g(i8 addrspace(1)*, i8 addrspace(1)*)
+declare i32 @personality_function()
+
+; Make sure that we do not fail assertion because we process call of @g before
+; we process the call of @f.
+
+define void @test_01(i8 addrspace(1)* %p, i1 %cond) gc "statepoint-example" personality i32 ()* @personality_function {
+
+; CHECK-LABEL: @test(
+
+entry:
+  br label %preheader
+
+preheader:
+  br label %loop
+
+loop:
+  %tmp0 = insertelement <2 x i8 addrspace(1)*> undef, i8 addrspace(1)* %p, i32 0
+  %tmp1 = insertelement <2 x i8 addrspace(1)*> %tmp0, i8 addrspace(1)* %p, i32 1
+  %tmp2 = extractelement <2 x i8 addrspace(1)*> %tmp1, i32 1
+  %tmp3 = extractelement <2 x i8 addrspace(1)*> %tmp1, i32 0
+  br label %cond_block_1
+
+cond_block_1:
+  br i1 %cond, label %cond_block_2, label %exit
+
+cond_block_2:
+  br i1 %cond, label %backedge, label %exit
+
+exit:
+  %tmp4 = phi i8 addrspace(1)* [ %tmp2, %cond_block_1 ], [ %tmp2, %cond_block_2 ]
+  call void @g(i8 addrspace(1)* %tmp3, i8 addrspace(1)* %tmp4)
+  ret void
+
+backedge:
+  call void @f()
+  br label %cond_block_1
+}
Index: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===================================================================
--- lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2755,17 +2755,12 @@
   StatepointLiveSetTy Updated;
   findLiveSetAtInst(Inst, RevisedLivenessData, Updated);
 
-#ifndef NDEBUG
-  DenseSet<Value *> Bases;
-  for (auto KVPair : Info.PointerToBase)
-    Bases.insert(KVPair.second);
-#endif
-
   // We may have base pointers which are now live that weren't before.  We need
   // to update the PointerToBase structure to reflect this.
   for (auto V : Updated)
     if (Info.PointerToBase.insert({V, V}).second) {
-      assert(Bases.count(V) && "Can't find base for unexpected live value!");
+      assert(isKnownBaseResult(V) &&
+             "Can't find base for unexpected live value!");
       continue;
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41593.128223.patch
Type: text/x-patch
Size: 2489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171227/faf7e5a7/attachment.bin>