Unverified Commit 9b2411da authored by Andrey Grabezhnoy's avatar Andrey Grabezhnoy Committed by GitHub
Browse files

[InstCombine] Require one-use intermediate binop in foldBinOpShiftWithShift (#194341) 

foldBinOpShiftWithShift rewrites
  binop(shift(X,C) op Mask, shift(Y,C))
to
  binop(shift(X+Y,C), Mask).

Both shifts are matched with m_OneUse, but the enclosing binop that
wraps (shift(X,C), Mask) is not. When that binop has additional users
the rewrite cannot eliminate the originals and only adds new
instructions. The reproducer in the issue shows this: a shared base
with several downstream consumers gets duplicated on each one, so the
fold grows the IR instead of shrinking it.

Wrap the outer m_c_BinOp in m_OneUse so the fold only fires when the
binop can actually be replaced. Mirrors the one-use discipline already
applied to both shifts in the same matcher.

Fixes #194007.
parent e028d938

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

+5 −4
Original line number Diff line number Diff line
@@ -1015,11 +1015,12 @@ Instruction *InstCombinerImpl::foldBinOpShiftWithShift(BinaryOperator &I) { 
    if (!match(I.getOperand(ShOpnum),

               m_OneUse(m_Shift(m_Value(Y), m_Value(Shift)))))

      return nullptr;

    if (!match(I.getOperand(1 - ShOpnum),

               m_c_BinOp(m_CombineAnd(

                             m_OneUse(m_Shift(m_Value(X), m_Specific(Shift))),

    if (!match(

            I.getOperand(1 - ShOpnum),

            m_OneUse(m_c_BinOp(

                m_CombineAnd(m_OneUse(m_Shift(m_Value(X), m_Specific(Shift))),

                             m_Value(ShiftedX)),

                         m_Value(Mask))))

                m_Value(Mask)))))

      return nullptr;

    // Make sure we are matching instruction shifts and not ConstantExpr

    auto *IY = dyn_cast<Instruction>(I.getOperand(ShOpnum));

llvm/test/Transforms/InstCombine/and-xor-or.ll

+2 −3
Original line number Diff line number Diff line
@@ -607,10 +607,9 @@ define i8 @xor_lshr_multiuse(i8 %x, i8 %y, i8 %z, i8 %shamt) { 
; CHECK-LABEL: define {{[^@]+}}@xor_lshr_multiuse

; CHECK-SAME: (i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[Z:%.*]], i8 [[SHAMT:%.*]]) {

; CHECK-NEXT:    [[SX:%.*]] = lshr i8 [[X]], [[SHAMT]]

; CHECK-NEXT:    [[SY:%.*]] = lshr i8 [[Y]], [[SHAMT]]

; CHECK-NEXT:    [[A:%.*]] = xor i8 [[SX]], [[Z]]

; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X]], [[Y]]

; CHECK-NEXT:    [[TMP2:%.*]] = lshr i8 [[TMP1]], [[SHAMT]]

; CHECK-NEXT:    [[R:%.*]] = xor i8 [[TMP2]], [[Z]]

; CHECK-NEXT:    [[R:%.*]] = xor i8 [[A]], [[SY]]

; CHECK-NEXT:    [[R2:%.*]] = sdiv i8 [[A]], [[R]]

; CHECK-NEXT:    ret i8 [[R2]]

;

llvm/test/Transforms/InstCombine/binop-and-shifts.ll

+23 −0
Original line number Diff line number Diff line
@@ -951,6 +951,29 @@ define <4 x i8> @xor_ashr_not_vec_poison_2(<4 x i8> %x, <4 x i8> %y, <4 x i8> %s 
  ret <4 x i8> %xor

}



; Negative test: outer binop has multiple users



define i8 @shl_add_add_multiuse_binop(i8 %x, i8 %y0, i8 %y1) {

; CHECK-LABEL: @shl_add_add_multiuse_binop(

; CHECK-NEXT:    [[SHIFT1:%.*]] = shl i8 [[X:%.*]], 2

; CHECK-NEXT:    [[BASE:%.*]] = add i8 [[SHIFT1]], 48

; CHECK-NEXT:    [[SHIFT_Y0:%.*]] = shl i8 [[Y0:%.*]], 2

; CHECK-NEXT:    [[SHIFT_Y1:%.*]] = shl i8 [[Y1:%.*]], 2

; CHECK-NEXT:    [[R0:%.*]] = add i8 [[BASE]], [[SHIFT_Y0]]

; CHECK-NEXT:    [[R1:%.*]] = add i8 [[BASE]], [[SHIFT_Y1]]

; CHECK-NEXT:    [[R:%.*]] = xor i8 [[R0]], [[R1]]

; CHECK-NEXT:    ret i8 [[R]]

;

  %shift1 = shl i8 %x, 2

  %base = add i8 %shift1, 48

  %shift.y0 = shl i8 %y0, 2

  %shift.y1 = shl i8 %y1, 2

  %r0 = add i8 %base, %shift.y0

  %r1 = add i8 %base, %shift.y1

  %r = xor i8 %r0, %r1

  ret i8 %r

}



; Negative test: invalid binop



define i8 @binop_ashr_not_fail_invalid_binop(i8 %x, i8 %y, i8 %shamt) {