[PATCH] D92364: [MergeICmps] Disable if a GEP does not reference an Argument

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: courbet, jyknight.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
MaskRay requested review of this revision.

If the pointer operand of a GEP references an instruction defined in a
deleted basic block, after cloning of the GEP and the removal of the
basic block, the GEP will reference undef.

This caused a miscompile which motivated my D92297 <https://reviews.llvm.org/D92297> (before D17993 <https://reviews.llvm.org/D17993>,
nonnull and dereferenceable attributed were not added so MergeICmps were
not triggered.) The new test gep-references-bb.ll demonstrate the issue.

Note: in LLVM 8.0, opt does not delete basic blocks (thus %gep is retained) and
insert new basic blocks. opt can actually optimize gep-references-bb.ll to use a
memcmp. We should try making the test profitable again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92364

Files:
  llvm/lib/Transforms/Scalar/MergeICmps.cpp
  llvm/test/Transforms/MergeICmps/X86/gep-references-bb.ll


Index: llvm/test/Transforms/MergeICmps/X86/gep-references-bb.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/MergeICmps/X86/gep-references-bb.ll
@@ -0,0 +1,51 @@
+; RUN: opt < %s -S -mergeicmps -verify-dom-info | FileCheck %s
+target triple = "x86_64"
+
+%Triple = type { %Elem0, %Elem1, %Elem2 }
+%Elem0 = type { i32 }
+%Elem1 = type { i32 }
+%Elem2 = type { i32 }
+
+;; %gep does not reference an argument. Disable the optimization because
+;; otherwise the newly created gep may reference undef (after the basic block
+;; defining the pointer operand is deleted).
+;; TODO Optimize with a memcmp.
+; CHECK-LABEL: bb0:
+; CHECK:         load i32, i32* %l0_addr, align 4
+; CHECK:         load i32, i32* %r0_addr, align 4
+; CHECK-LABEL: bb1:
+; CHECK:         load i32, i32* %l1_addr, align 4
+; CHECK:         load i32, i32* %r1_addr, align 4
+; CHECK-LABEL: bb2:
+; CHECK:         load i32, i32* %l2_addr, align 4
+; CHECK:         load i32, i32* %r2_addr, align 4
+define i1 @bug(%Triple* nonnull dereferenceable(12) %lhs, %Triple* nonnull dereferenceable(12) %rhs) {
+bb0:
+  %gep = getelementptr %Triple, %Triple* %rhs, i64 0, i32 0
+  %l0_addr = getelementptr inbounds %Triple, %Triple* %lhs, i64 0, i32 0, i32 0
+  %l0 = load i32, i32* %l0_addr, align 4
+  %r0_addr = getelementptr inbounds %Triple, %Triple* %rhs, i64 0, i32 0, i32 0
+  %r0 = load i32, i32* %r0_addr, align 4
+  %cmp0 = icmp eq i32 %l0, %r0
+  br i1 %cmp0, label %bb1, label %final
+
+bb1:                                           ; preds = %bb0
+  %l1_addr = getelementptr inbounds %Triple, %Triple* %lhs, i64 0, i32 1, i32 0
+  %l1 = load i32, i32* %l1_addr, align 4
+  %r1_addr = getelementptr inbounds %Elem0, %Elem0* %gep, i64 1, i32 0
+  %r1 = load i32, i32* %r1_addr, align 4
+  %cmp1 = icmp eq i32 %l1, %r1
+  br i1 %cmp1, label %bb2, label %final
+
+bb2:                                           ; preds = %bb1
+  %l2_addr = getelementptr inbounds %Triple, %Triple* %lhs, i64 0, i32 2, i32 0
+  %l2 = load i32, i32* %l2_addr, align 4
+  %r2_addr = getelementptr inbounds %Elem0, %Elem0* %gep, i64 2, i32 0
+  %r2 = load i32, i32* %r2_addr, align 4
+  %cmp2 = icmp eq i32 %l2, %r2
+  br label %final
+
+final:                                            ; preds = %bb2, %bb1, %bb0
+  %ret = phi i1 [ false, %bb0 ], [ false, %bb1 ], [ %cmp2, %bb2 ]
+  ret i1 %ret
+}
Index: llvm/lib/Transforms/Scalar/MergeICmps.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/MergeICmps.cpp
+++ llvm/lib/Transforms/Scalar/MergeICmps.cpp
@@ -158,6 +158,8 @@
   if (!GEP)
     return {};
   LLVM_DEBUG(dbgs() << "GEP\n");
+  if (!isa<Argument>(GEP->getPointerOperand()))
+    return {};
   if (GEP->isUsedOutsideOfBlock(LoadI->getParent())) {
     LLVM_DEBUG(dbgs() << "used outside of block\n");
     return {};


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92364.308540.patch
Type: text/x-patch
Size: 2897 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201201/1d236785/attachment.bin>