[PATCH] D136095: [GVN] Patch for invalid GVN replacement

[Nikita Popov via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d0648cb6c5f: [GVN] Patch for invalid GVN replacement (authored by alexgatea, committed by nikic).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136095/new/

https://reviews.llvm.org/D136095

Files:
  llvm/lib/Transforms/Scalar/GVN.cpp
  llvm/test/Transforms/GVN/PRE/load-pre-across-backedge.ll
  llvm/test/Transforms/GVN/PRE/pre-gep-load.ll


Index: llvm/test/Transforms/GVN/PRE/pre-gep-load.ll
===================================================================
--- llvm/test/Transforms/GVN/PRE/pre-gep-load.ll
+++ llvm/test/Transforms/GVN/PRE/pre-gep-load.ll
@@ -95,14 +95,13 @@
 ; CHECK-LABEL: @test_shortcut_safe(
 ; CHECK-NEXT:    br i1 [[TST:%.*]], label [[SEXT1:%.*]], label [[PRE_DEST:%.*]]
 ; CHECK:       pre.dest:
-; CHECK-NEXT:    [[DOTPRE:%.*]] = sext i32 [[P1:%.*]] to i64
 ; CHECK-NEXT:    br label [[SEXT_USE:%.*]]
 ; CHECK:       sext1:
-; CHECK-NEXT:    [[IDXPROM:%.*]] = sext i32 [[P1]] to i64
+; CHECK-NEXT:    [[IDXPROM:%.*]] = sext i32 [[P1:%.*]] to i64
 ; CHECK-NEXT:    br label [[SEXT_USE]]
 ; CHECK:       sext.use:
-; CHECK-NEXT:    [[IDXPROM2_PRE_PHI:%.*]] = phi i64 [ [[IDXPROM]], [[SEXT1]] ], [ [[DOTPRE]], [[PRE_DEST]] ]
-; CHECK-NEXT:    [[ARRAYIDX3:%.*]] = getelementptr inbounds i32, i32* [[A:%.*]], i64 [[IDXPROM2_PRE_PHI]]
+; CHECK-NEXT:    [[IDXPROM2:%.*]] = sext i32 [[P1]] to i64
+; CHECK-NEXT:    [[ARRAYIDX3:%.*]] = getelementptr inbounds i32, i32* [[A:%.*]], i64 [[IDXPROM2]]
 ; CHECK-NEXT:    [[VAL:%.*]] = load i32, i32* [[ARRAYIDX3]], align 4
 ; CHECK-NEXT:    tail call void @g(i32 [[VAL]])
 ; CHECK-NEXT:    br label [[PRE_DEST]]
Index: llvm/test/Transforms/GVN/PRE/load-pre-across-backedge.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/GVN/PRE/load-pre-across-backedge.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -gvn -S < %s | FileCheck %s
+
+; Check that PRE-LOAD across backedge does not
+; result in invalid dominator tree.
+declare void @use(i32)
+
+define void @test1(i1 %c, i32 %arg) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[BB1:%.*]], label [[DOTBB2_CRIT_EDGE:%.*]]
+; CHECK:       .bb2_crit_edge:
+; CHECK-NEXT:    [[DOTPRE:%.*]] = shl i32 [[ARG:%.*]], 2
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[SHL1:%.*]] = shl i32 [[ARG]], 2
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[SHL2_PRE_PHI:%.*]] = phi i32 [ [[DOTPRE]], [[DOTBB2_CRIT_EDGE]] ], [ [[SHL3:%.*]], [[BB3]] ]
+; CHECK-NEXT:    call void @use(i32 [[SHL2_PRE_PHI]])
+; CHECK-NEXT:    br label [[BB3]]
+; CHECK:       bb3:
+; CHECK-NEXT:    [[SHL3]] = shl i32 [[ARG]], 2
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr null, i32 [[SHL3]]
+; CHECK-NEXT:    [[V:%.*]] = load i32, ptr [[GEP]], align 4
+; CHECK-NEXT:    call void @use(i32 [[V]])
+; CHECK-NEXT:    br label [[BB2]]
+;
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  %shl1 = shl i32 %arg, 2
+  br label %bb3
+
+bb2:
+  %shl2 = shl i32 %arg, 2
+  call void @use(i32 %shl2)
+  br label %bb3
+
+bb3:
+  %shl3 = shl i32 %arg, 2
+  %gep = getelementptr i32, ptr null, i32 %shl3
+  %v = load i32, ptr %gep, align 4
+  call void @use(i32 %v)
+  br label %bb2
+}
Index: llvm/lib/Transforms/Scalar/GVN.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/GVN.cpp
+++ llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2828,17 +2828,10 @@
       NumWithout = 2;
       break;
     }
-    // It is not safe to do PRE when P->CurrentBlock is a loop backedge, and
-    // when CurInst has operand defined in CurrentBlock (so it may be defined
-    // by phi in the loop header).
+    // It is not safe to do PRE when P->CurrentBlock is a loop backedge.
     assert(BlockRPONumber.count(P) && BlockRPONumber.count(CurrentBlock) &&
            "Invalid BlockRPONumber map.");
-    if (BlockRPONumber[P] >= BlockRPONumber[CurrentBlock] &&
-        llvm::any_of(CurInst->operands(), [&](const Use &U) {
-          if (auto *Inst = dyn_cast<Instruction>(U.get()))
-            return Inst->getParent() == CurrentBlock;
-          return false;
-        })) {
+    if (BlockRPONumber[P] >= BlockRPONumber[CurrentBlock]) {
       NumWithout = 2;
       break;
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136095.473215.patch
Type: text/x-patch
Size: 3950 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221104/d9d5a6bf/attachment.bin>