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

[MaskRay]

Summary

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 (before 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.

[MaskRay]

Add log

[MaskRay]

Placed LLVM_DEBUG in the wrong place in the previous diff...

[courbet]

Thanks for the repro. I think a better fix is D92375 (missing split).

llvm/lib/Transforms/Scalar/MergeICmps.cpp

if (!LoadI->isSimple()) {
     LLVM_DEBUG(dbgs() << "volatile or atomic\n");
     return {};
   }
   Value *const Addr = LoadI->getOperand(0);
   auto *const GEP = dyn_cast<GetElementPtrInst>(Addr);
   if (!GEP)
     return {};
   LLVM_DEBUG(dbgs() << "GEP\n");
+  if (!isa<Argument>(GEP->getPointerOperand())) {
+    LLVM_DEBUG(dbgs() << "does not reference an argument\n");
+    return {};
+  }
   if (GEP->isUsedOutsideOfBlock(LoadI->getParent())) {
     LLVM_DEBUG(dbgs() << "used outside of block\n");
     return {};
   }
   const auto &DL = GEP->getModule()->getDataLayout();
   if (!isDereferenceablePointer(GEP, LoadI->getType(), DL)) {
     LLVM_DEBUG(dbgs() << "not dereferenceable\n");
     // We need to make sure that we can do comparison in any order, so we

llvm/test/Transforms/MergeICmps/X86/gep-references-bb.ll

+; 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
+}