[PATCH] D74278: [InstCombine] Fix infinite loop in min/max load/store bitcast combine (PR44835)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, lebedev.ri, ABataev.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Fixes https://bugs.llvm.org/show_bug.cgi?id=44835. Skip the transform if it wouldn't actually do anything (apart from removing and reinserting the same instructions).

Note that the test case doesn't loop on current master anymore, only on the LLVM 10 release branch. The issue is already mitigated on master due to worklist order fixes, but we should probably fixing the root cause there as well.

As a side note, we should probably assert in combineLoadToNewType() that it does not combine to the same type. Not doing this here, because this assertion would also be triggered in another place right now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74278

Files:
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/test/Transforms/InstCombine/pr44835.ll


Index: llvm/test/Transforms/InstCombine/pr44835.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/InstCombine/pr44835.ll
@@ -0,0 +1,29 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
+; This test used to cause an infinite loop in the load/store min/max bitcast
+; transform.
+
+define void @test(i32* %p, i32* %p2) {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[P:%.*]], align 4
+; CHECK-NEXT:    [[V2:%.*]] = load i32, i32* [[P2:%.*]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[V2]], [[V]]
+; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[CMP]], i32 [[V2]], i32 [[V]]
+; CHECK-NEXT:    store i32 [[TMP1]], i32* [[P]], align 4
+; CHECK-NEXT:    ret void
+;
+  %v = load i32, i32* %p, align 4
+  %v2 = load i32, i32* %p2, align 4
+  %cmp = icmp ult i32 %v2, %v
+  %sel = select i1 %cmp, i32* %p2, i32* %p
+  %p8 = bitcast i32* %p to i8*
+  %sel8 = bitcast i32* %sel to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %p8, i8* align 4 %sel8, i64 4, i1 false)
+  ret void
+}
+
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #0
+
+attributes #0 = { argmemonly nounwind willreturn }
Index: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -1320,6 +1320,11 @@
   if (!isMinMaxWithLoads(LoadAddr, CmpLoadTy))
     return false;
 
+  // Make sure the type would actually change.
+  // This condition can be hit with chains of bitcasts.
+  if (LI->getType() == CmpLoadTy)
+    return false;
+
   // Make sure we're not changing the size of the load/store.
   const auto &DL = IC.getDataLayout();
   if (DL.getTypeStoreSizeInBits(LI->getType()) !=


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74278.243357.patch
Type: text/x-patch
Size: 2064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200208/4ffffdbc/attachment.bin>