[PATCH] D99058: [RS4GC] Fix hang on infinite loop

[Serguei Katkov via Phabricator via llvm-commits]

skatkov created this revision.
skatkov added reviewers: reames, dantrushin.
Herald added a subscriber: hiraditya.
skatkov requested review of this revision.
Herald added a project: LLVM.

meetBDVState utility may sets the base pointer for the conflict state.
At this moment the base for conflict state does not have any meaning but
is used in comparison of BDV states. This comparison is used as an indicator 
of progress done on iteration and RS4GC pass uses infinite loop to reach
fixed point.
As a result for added test on each iteration state for some phi nodes is updated
with other base value for conflict state and it indicates as a progress while
for conflict state there is no any progress more possible.
In reality the base value is transferred from one state to another and pass
detects the progress on these states.

The test is very fragile. The traversal order of states and operands of phi nodes
plays important role.


https://reviews.llvm.org/D99058

Files:
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/test/Transforms/RewriteStatepointsForGC/meetBDVState-hangs.ll


Index: llvm/test/Transforms/RewriteStatepointsForGC/meetBDVState-hangs.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/RewriteStatepointsForGC/meetBDVState-hangs.ll
@@ -0,0 +1,63 @@
+; RUN: opt -S -rewrite-statepoints-for-gc < %s | FileCheck %s
+
+; Regression test to incorrectly testing fixed state causing infinite loop.
+; CHECK: test
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar(i8 addrspace(1)* nocapture readonly)
+declare noalias i8 addrspace(1)* @foo()
+
+define i8 addrspace(1)* @test(i1 %c, i1 %c1, i1 %c2, i1 %c3, i1 %c4, i1 %c5, i1 %c.exit) gc "statepoint-example" {
+entry:
+  br i1 %c, label %ph.L, label %ph.R
+ph.L:
+  %ph.L.p.b = call noalias nonnull i8 addrspace(1)* @foo()
+  %ph.L.p = getelementptr i8, i8 addrspace(1)* %ph.L.p.b, i64 8
+  br label %ph.M
+ph.R:
+  %ph.R.p = call noalias nonnull i8 addrspace(1)* @foo()
+  br label %ph.M
+ph.M:
+  %ph.M.p = phi i8 addrspace(1)* [ %ph.L.p, %ph.L ], [ %ph.R.p, %ph.R ]
+  br label %header
+  
+header:
+  %header.p = phi i8 addrspace(1)* [ %ph.M.p, %ph.M ], [ %backedge.p, %backedge]
+  br i1 %c1, label %loop.M, label %loop.R
+
+loop.R:
+  br i1 %c2, label %loop.R.M, label %loop.R.R
+
+loop.R.R:
+  %loop.R.R.p = call noalias nonnull i8 addrspace(1)* @foo()
+  br label %loop.R.M
+
+loop.R.M:
+  %loop.R.M.p = phi i8 addrspace(1)* [ %header.p, %loop.R ], [ %loop.R.R.p, %loop.R.R ]
+  br label %loop.M
+
+loop.M:
+  %loop.M.p = phi i8 addrspace(1)* [ %loop.R.M.p, %loop.R.M ], [ %header.p, %header ]
+  br i1 %c4, label %backedge, label %pre.backedge.R
+  
+pre.backedge.R:
+  br i1 %c5, label %pre.backedge.R.L, label %pre.backedge.R.R
+pre.backedge.R.L:
+  %pre.backedge.R.L.p.b = call noalias nonnull i8 addrspace(1)* @foo()
+  %pre.backedge.R.L.p = getelementptr i8, i8 addrspace(1)* %pre.backedge.R.L.p.b, i64 8
+  br label %pre.backedge.R.M
+pre.backedge.R.R:
+  %pre.backedge.R.R.p = call noalias nonnull i8 addrspace(1)* @foo()
+  br label %pre.backedge.R.M
+pre.backedge.R.M:
+  %pre.backedge.R.M.p = phi i8 addrspace(1)* [ %pre.backedge.R.L.p, %pre.backedge.R.L ], [ %pre.backedge.R.R.p, %pre.backedge.R.R ]
+  br label %backedge
+  
+backedge:
+  %backedge.p = phi i8 addrspace(1)* [ %pre.backedge.R.M.p, %pre.backedge.R.M ], [ %loop.M.p, %loop.M ]
+  br i1 %c.exit, label %header, label %exit
+  
+exit:                                                ; preds = %3, %1
+  call void @bar(i8 addrspace(1)* align 8 %header.p) [ "deopt"() ]
+  ret i8 addrspace(1)* %header.p
+}
Index: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -782,8 +782,9 @@
   if (LHS.getStatus() == BDVState::Base && RHS.getStatus() == BDVState::Base &&
       LHS.getBaseValue() != RHS.getBaseValue()) {
     NewStatus = BDVState::Conflict;
-    BaseValue = nullptr;
   }
+  if (NewStatus == BDVState::Conflict)
+    BaseValue = nullptr;
   return BDVState(LHS.getOriginalValue(), NewStatus, BaseValue);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99058.332216.patch
Type: text/x-patch
Size: 3144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210322/1e6ff3c9/attachment.bin>