Index: lib/Transforms/InstCombine/InstCombineSelect.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineSelect.cpp +++ lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -300,13 +300,9 @@ TI->getType()); } - // Only handle binary operators (including two-operand getelementptr) with - // one-use here. As with the cast case above, it may be possible to relax the - // one-use constraint, but that needs be examined carefully since it may not - // reduce the total number of instructions. + // Only handle binary operators (including two-operand getelementptr) here. if (TI->getNumOperands() != 2 || FI->getNumOperands() != 2 || - (!isa(TI) && !isa(TI)) || - !TI->hasOneUse() || !FI->hasOneUse()) + (!isa(TI) && !isa(TI))) return nullptr; // Figure out if the operations have any operands in common. @@ -338,9 +334,30 @@ return nullptr; } - // If we reach here, they do have operations in common. - Value *NewSI = Builder.CreateSelect(SI.getCondition(), OtherOpT, OtherOpF, - SI.getName() + ".v", &SI); + // If we reach here, they do have operations in common. First check if there + // already exists a select of the differing operands. + Value *NewSI = nullptr; + for (Instruction &I : *SI.getParent()) { + if (isa(I) && I.getOperand(0) == SI.getOperand(0) && + I.getOperand(1) == OtherOpT && I.getOperand(2) == OtherOpF) { + // We can safely move I before SI as its operands are operands (of + // operands of) SI, so are already defined at this point. + I.moveBefore(&SI); + NewSI = &I; + break; + } + } + + // If there isn't already a select then create one if the select operands have + // only one use. As with the cast case above, it may be possible to relax the + // one-use constraint, but that needs be examined carefully since it may not + // reduce the total number of instructions. + if (!NewSI) { + if (!TI->hasOneUse() || !FI->hasOneUse()) + return nullptr; + NewSI = Builder.CreateSelect(SI.getCondition(), OtherOpT, OtherOpF, + SI.getName() + ".v", &SI); + } Value *Op0 = MatchIsOpZero ? MatchOp : NewSI; Value *Op1 = MatchIsOpZero ? NewSI : MatchOp; if (BinaryOperator *BO = dyn_cast(TI)) Index: test/Transforms/InstCombine/select-reuse.ll =================================================================== --- /dev/null +++ test/Transforms/InstCombine/select-reuse.ll @@ -0,0 +1,64 @@ +; RUN: opt < %s -instcombine -S | FileCheck %s + +; Check that we reuse a select instead of adding a new one +; CHECK-LABEL: @test1 +; CHECK: [[SEL:%.*]] = select i1 %cmp, i32 %x, i32 %y +; CHECK-NOT: select i1 %cmp, i32 %x, i32 %y +; CHECK: add i32 [[SEL]], 1 +define i32 @test1(i32 %x, i32 %y) { + %add1 = add i32 %x, 1 + %add2 = add i32 %y, 1 + %cmp = icmp ugt i32 %x, %y + %select1 = select i1 %cmp, i32 %x, i32 %y + %select2 = select i1 %cmp, i32 %add1, i32 %add2 + %mul = mul i32 %select1, %select2 + ret i32 %mul +} + +; Check that we can optimise a select whose operands are used more than once by +; reusing an existing select +; CHECK-LABEL: @test2a +; CHECK: [[SEL:%.*]] = select i1 %cmp, i32 %x, i32 %y +; CHECK-NOT: select i1 %cmp, i32 %add1, i32 %add2 +; CHECK: add i32 [[SEL]], 1 +define i32 @test2a(i32 %x, i32 %y) { + %add1 = add i32 %x, 1 + %add2 = add i32 %y, 1 + %cmp = icmp ugt i32 %x, %y + %select1 = select i1 %cmp, i32 %x, i32 %y + %select2 = select i1 %cmp, i32 %add1, i32 %add2 + %mul1 = mul i32 %select1, %select2 + %mul2 = mul i32 %mul1, %add1 + ret i32 %mul2 +} + +; CHECK-LABEL: @test2b +; CHECK: [[SEL:%.*]] = select i1 %cmp, i32 %x, i32 %y +; CHECK-NOT: select i1 %cmp, i32 %add1, i32 %add2 +; CHECK: add i32 [[SEL]], 1 +define i32 @test2b(i32 %x, i32 %y) { + %add1 = add i32 %x, 1 + %add2 = add i32 %y, 1 + %cmp = icmp ugt i32 %x, %y + %select1 = select i1 %cmp, i32 %x, i32 %y + %select2 = select i1 %cmp, i32 %add1, i32 %add2 + %mul1 = mul i32 %select1, %select2 + %mul2 = mul i32 %mul1, %add2 + ret i32 %mul2 +} + +; CHECK-LABEL: @test2c +; CHECK: [[SEL:%.*]] = select i1 %cmp, i32 %x, i32 %y +; CHECK-NOT: select i1 %cmp, i32 %add1, i32 %add2 +; CHECK: add i32 [[SEL]], 1 +define i32 @test2c(i32 %x, i32 %y) { + %add1 = add i32 %x, 1 + %add2 = add i32 %y, 1 + %cmp = icmp ugt i32 %x, %y + %select1 = select i1 %cmp, i32 %x, i32 %y + %select2 = select i1 %cmp, i32 %add1, i32 %add2 + %mul1 = mul i32 %select1, %select2 + %mul2 = mul i32 %mul1, %add1 + %mul3 = mul i32 %mul2, %add2 + ret i32 %mul3 +}