From 50ac6afd848670963f8649cb8009ad0b5932bc09 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 16 Jan 2020 21:38:19 +0100 Subject: [PATCH] [InstCombine] Fix worklist management in return combine There are two related bugs here: First, we don't add the operand we're replacing to the worklist, which means it may not get DCEd (see test change). Second, usually this would just get picked up in the next iteration, but we also do not report the instruction as changed. This means that we do not get that extra instcombine iteration, and more importantly, may break the pass pipeline, as the function is not marked as changed. Differential Revision: https://reviews.llvm.org/D72864 --- .../InstCombine/InstructionCombining.cpp | 7 +++++-- test/Transforms/InstCombine/assume.ll | 14 ++++---------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index bf32996d96e..6e0587ddd09 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -2592,14 +2592,17 @@ Instruction *InstCombiner::visitReturnInst(ReturnInst &RI) { Value *ResultOp = RI.getOperand(0); Type *VTy = ResultOp->getType(); - if (!VTy->isIntegerTy()) + if (!VTy->isIntegerTy() || isa(ResultOp)) return nullptr; // There might be assume intrinsics dominating this return that completely // determine the value. If so, constant fold it. KnownBits Known = computeKnownBits(ResultOp, 0, &RI); - if (Known.isConstant()) + if (Known.isConstant()) { + Worklist.AddValue(ResultOp); RI.setOperand(0, Constant::getIntegerValue(VTy, Known.getConstant())); + return &RI; + } return nullptr; } diff --git a/test/Transforms/InstCombine/assume.ll b/test/Transforms/InstCombine/assume.ll index a9bdce7f6a0..2d9f2a616e7 100644 --- a/test/Transforms/InstCombine/assume.ll +++ b/test/Transforms/InstCombine/assume.ll @@ -190,16 +190,10 @@ entry: } define i32 @icmp1(i32 %a) #0 { -; EXPENSIVE-ON-LABEL: @icmp1( -; EXPENSIVE-ON-NEXT: [[CMP:%.*]] = icmp sgt i32 [[A:%.*]], 5 -; EXPENSIVE-ON-NEXT: tail call void @llvm.assume(i1 [[CMP]]) -; EXPENSIVE-ON-NEXT: ret i32 1 -; -; EXPENSIVE-OFF-LABEL: @icmp1( -; EXPENSIVE-OFF-NEXT: [[CMP:%.*]] = icmp sgt i32 [[A:%.*]], 5 -; EXPENSIVE-OFF-NEXT: tail call void @llvm.assume(i1 [[CMP]]) -; EXPENSIVE-OFF-NEXT: [[CONV:%.*]] = zext i1 [[CMP]] to i32 -; EXPENSIVE-OFF-NEXT: ret i32 1 +; CHECK-LABEL: @icmp1( +; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[A:%.*]], 5 +; CHECK-NEXT: tail call void @llvm.assume(i1 [[CMP]]) +; CHECK-NEXT: ret i32 1 ; %cmp = icmp sgt i32 %a, 5 tail call void @llvm.assume(i1 %cmp)