From 50ead9021f9c9ad106e5e70f625fe33c4bf1878a Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Tue, 12 Dec 2017 15:03:17 +0000 Subject: [PATCH] [InstCombine] Fix PR35618: Instcombine hangs on single minmax load bitcast. If we have pattern `store (load(bitcast(select (cmp(V1, V2), &V1, &V2)))), bitcast)`, but the load is used in other instructions, it leads to looping in InstCombiner. Patch adds additional check that all users of the load instructions are stores and then replaces all uses of load instruction by the new one with new type. Reviewers: RKSimon, spatel, majnemer Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D41072 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@320483 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombineLoadStoreAlloca.cpp | 19 ++++++++++-- .../multiple-uses-load-bitcast-select.ll | 30 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 test/Transforms/InstCombine/multiple-uses-load-bitcast-select.ll diff --git a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index 01fc1528681..9f00e6f8358 100644 --- a/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -1354,9 +1354,24 @@ bool removeBitcastsFromLoadStoreOnMinMax(InstCombiner &IC, StoreInst &SI) { if (!isMinMaxWithLoads(LoadAddr)) return false; + if (!all_of(LI->users(), [LI](User *U) { + auto *SI = dyn_cast(U); + return SI && SI->getPointerOperand() != LI && + !SI->getPointerOperand()->isSwiftError(); + })) + return false; + + IC.Builder.SetInsertPoint(LI); LoadInst *NewLI = combineLoadToNewType( IC, *LI, LoadAddr->getType()->getPointerElementType()); - combineStoreToNewValue(IC, SI, NewLI); + // Replace all the stores with stores of the newly loaded value. + for (auto *UI : LI->users()) { + auto *SI = cast(UI); + IC.Builder.SetInsertPoint(SI); + combineStoreToNewValue(IC, *SI, NewLI); + IC.eraseInstFromFunction(*SI); + } + IC.Worklist.Add(LI); return true; } @@ -1385,7 +1400,7 @@ Instruction *InstCombiner::visitStoreInst(StoreInst &SI) { return eraseInstFromFunction(SI); if (removeBitcastsFromLoadStoreOnMinMax(*this, SI)) - return eraseInstFromFunction(SI); + return nullptr; // Replace GEP indices if possible. if (Instruction *NewGEPI = replaceGEPIdxWithZero(*this, Ptr, SI)) { diff --git a/test/Transforms/InstCombine/multiple-uses-load-bitcast-select.ll b/test/Transforms/InstCombine/multiple-uses-load-bitcast-select.ll new file mode 100644 index 00000000000..28509df6d2f --- /dev/null +++ b/test/Transforms/InstCombine/multiple-uses-load-bitcast-select.ll @@ -0,0 +1,30 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt < %s -instcombine -S -data-layout="E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64" | FileCheck %s + +define void @PR35618(i64* %st1, double* %st2) { +; CHECK-LABEL: @PR35618( +; CHECK-NEXT: [[Y1:%.*]] = alloca double, align 8 +; CHECK-NEXT: [[Z1:%.*]] = alloca double, align 8 +; CHECK-NEXT: [[LD1:%.*]] = load double, double* [[Y1]], align 8 +; CHECK-NEXT: [[LD2:%.*]] = load double, double* [[Z1]], align 8 +; CHECK-NEXT: [[TMP10:%.*]] = fcmp olt double [[LD1]], [[LD2]] +; CHECK-NEXT: [[TMP121:%.*]] = select i1 [[TMP10]], double [[LD1]], double [[LD2]] +; CHECK-NEXT: [[TMP1:%.*]] = bitcast i64* [[ST1:%.*]] to double* +; CHECK-NEXT: store double [[TMP121]], double* [[TMP1]], align 8 +; CHECK-NEXT: store double [[TMP121]], double* [[ST2:%.*]], align 8 +; CHECK-NEXT: ret void +; + %y1 = alloca double + %z1 = alloca double + %ld1 = load double, double* %y1 + %ld2 = load double, double* %z1 + %tmp10 = fcmp olt double %ld1, %ld2 + %sel = select i1 %tmp10, double* %y1, double* %z1 + %tmp11 = bitcast double* %sel to i64* + %tmp12 = load i64, i64* %tmp11 + store i64 %tmp12, i64* %st1 + %bc = bitcast double* %st2 to i64* + store i64 %tmp12, i64* %bc + ret void +} + -- 2.50.1