Skip to content

Commit

Permalink
[InstCombine] Fix PR35618: Instcombine hangs on single minmax load bi…
Browse files Browse the repository at this point in the history
…tcast.

Summary:
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

llvm-svn: 320525
  • Loading branch information
alexey-bataev committed Dec 12, 2017
1 parent 212bcf8 commit 83c15b1
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 4 deletions.
24 changes: 20 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
Expand Up @@ -1339,10 +1339,10 @@ static bool equivalentAddressValues(Value *A, Value *B) {
/// Converts store (bitcast (load (bitcast (select ...)))) to
/// store (load (select ...)), where select is minmax:
/// select ((cmp load V1, load V2), V1, V2).
bool removeBitcastsFromLoadStoreOnMinMax(InstCombiner &IC, StoreInst &SI) {
static bool removeBitcastsFromLoadStoreOnMinMax(InstCombiner &IC,
StoreInst &SI) {
// bitcast?
Value *StoreAddr;
if (!match(SI.getPointerOperand(), m_BitCast(m_Value(StoreAddr))))
if (!match(SI.getPointerOperand(), m_BitCast(m_Value())))
return false;
// load? integer?
Value *LoadAddr;
Expand All @@ -1354,9 +1354,25 @@ bool removeBitcastsFromLoadStoreOnMinMax(InstCombiner &IC, StoreInst &SI) {
if (!isMinMaxWithLoads(LoadAddr))
return false;

if (!all_of(LI->users(), [LI, LoadAddr](User *U) {
auto *SI = dyn_cast<StoreInst>(U);
return SI && SI->getPointerOperand() != LI &&
peekThroughBitcast(SI->getPointerOperand()) != LoadAddr &&
!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 *USI = cast<StoreInst>(UI);
IC.Builder.SetInsertPoint(USI);
combineStoreToNewValue(IC, *USI, NewLI);
}
IC.replaceInstUsesWith(*LI, UndefValue::get(LI->getType()));
IC.eraseInstFromFunction(*LI);
return true;
}

Expand Down
@@ -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
}

0 comments on commit 83c15b1

Please sign in to comment.