[llvm] 77befe5 - [InstCombine] Fix worklist management in return combine

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2020-01-17T17:59:23+01:00
New Revision: 77befe54f7d72e79f94a3255ae10d529d3b19733

URL: https://github.com/llvm/llvm-project/commit/77befe54f7d72e79f94a3255ae10d529d3b19733
DIFF: https://github.com/llvm/llvm-project/commit/77befe54f7d72e79f94a3255ae10d529d3b19733.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/assume.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index bf32996d96e2..6e0587ddd099 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/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<Constant>(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/llvm/test/Transforms/InstCombine/assume.ll b/llvm/test/Transforms/InstCombine/assume.ll
index a9bdce7f6a07..2d9f2a616e7f 100644
--- a/llvm/test/Transforms/InstCombine/assume.ll
+++ b/llvm/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)