[llvm] 009f6ce - [GVNSink] Make GVNSink resistant against self referencing instructions (PR36954)

[Dawid Jurczak via llvm-commits]

Author: Dawid Jurczak
Date: 2022-05-10T16:06:12+02:00
New Revision: 009f6ce0ef1b5cdc8ec465797366ce1ce151acd4

URL: https://github.com/llvm/llvm-project/commit/009f6ce0ef1b5cdc8ec465797366ce1ce151acd4
DIFF: https://github.com/llvm/llvm-project/commit/009f6ce0ef1b5cdc8ec465797366ce1ce151acd4.diff

LOG: [GVNSink] Make GVNSink resistant against self referencing instructions (PR36954)

Before this change GVNSink pass suffers from stack overflow while processing self referenced instruction in unreachable basic block.
According [1] and [2] it's reasonable to make pass resistant against self referencing instructions.
To fix issue we skip sinking analysis when we reach instruction coming from unreachable block.

[1] https://groups.google.com/g/llvm-dev/c/843Tig9IzwA
[2] https://lists.llvm.org/pipermail/llvm-dev/2015-February/082629.html

Differential Revision: https://reviews.llvm.org/D113897

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/GVNSink.cpp
    llvm/test/Transforms/GVNSink/sink-common-code.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 2610ef1a4e1a2..1d0e2ae36ecec 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -381,6 +381,8 @@ class InstructionUseExpr : public GVNExpression::BasicExpression {
   }
 };
 
+using BasicBlocksSet = SmallPtrSet<const BasicBlock *, 32>;
+
 class ValueTable {
   DenseMap<Value *, uint32_t> ValueNumbering;
   DenseMap<GVNExpression::Expression *, uint32_t> ExpressionNumbering;
@@ -388,6 +390,7 @@ class ValueTable {
   BumpPtrAllocator Allocator;
   ArrayRecycler<Value *> Recycler;
   uint32_t nextValueNumber = 1;
+  BasicBlocksSet ReachableBBs;
 
   /// Create an expression for I based on its opcode and its uses. If I
   /// touches or reads memory, the expression is also based upon its memory
@@ -419,6 +422,11 @@ class ValueTable {
 public:
   ValueTable() = default;
 
+  /// Set basic blocks reachable from entry block.
+  void setReachableBBs(const BasicBlocksSet &ReachableBBs) {
+    this->ReachableBBs = ReachableBBs;
+  }
+
   /// Returns the value number for the specified value, assigning
   /// it a new number if it did not have one before.
   uint32_t lookupOrAdd(Value *V) {
@@ -432,6 +440,9 @@ class ValueTable {
     }
 
     Instruction *I = cast<Instruction>(V);
+    if (!ReachableBBs.contains(I->getParent()))
+      return ~0U;
+
     InstructionUseExpr *exp = nullptr;
     switch (I->getOpcode()) {
     case Instruction::Load:
@@ -568,6 +579,7 @@ class GVNSink {
 
     unsigned NumSunk = 0;
     ReversePostOrderTraversal<Function*> RPOT(&F);
+    VN.setReachableBBs(BasicBlocksSet(RPOT.begin(), RPOT.end()));
     for (auto *N : RPOT)
       NumSunk += sinkBB(N);
 

diff  --git a/llvm/test/Transforms/GVNSink/sink-common-code.ll b/llvm/test/Transforms/GVNSink/sink-common-code.ll
index 2dafcaa8c98d3..32f9f07dbc8b7 100644
--- a/llvm/test/Transforms/GVNSink/sink-common-code.ll
+++ b/llvm/test/Transforms/GVNSink/sink-common-code.ll
@@ -759,6 +759,27 @@ if.end:
   ret i32 1
 }
 
+; CHECK-LABEL: test_pr36954
+; CHECK-NOT: xor
+; PR36954 reproducer containing self referencing instruction shouldn't crash GVNSink pass.
+define void @test_pr36954() {
+bb1:
+  %i2 = trunc i32 undef to i8
+  br label %bb2
+
+bb2:
+  br i1 undef, label %bb2, label %exit
+
+bb4.critedge:
+  %i6 = sub i8 %i2, undef
+  %i7 = zext i8 %i6 to i32
+  %i8 = xor i32 %i8, %i7
+  br i1 false, label %exit, label %bb4.critedge
+
+exit:
+  ret void
+}
+
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"float", !2}
 ; CHECK: !2 = !{!"an example type tree"}