[PATCH] D104193: [InstCombine] Require one user (not one use) for operands when optimize both (sext bool X) * (sext bool Y) and (zext bool X) * (zext bool Y)

[Daniil Seredkin via Phabricator via llvm-commits]

vdsered added a comment.



In D104193#2815644 <https://reviews.llvm.org/D104193#2815644>, @lebedev.ri wrote:

> Correction: what i meant to say is, there are two cases:
>
> 1. `X == Y`, in which case we do not care about use count. this implicitly handles the case where both operands are the same instruction
> 2. either operand is one-use

I'd add one more case where one operand has one use and an other more than one. We check this for zext in mul_bools_use2 and for sext in mul_bools_sext_use1 and mul_bools_sext_use2 (they seem to differ only in operand's order, should it be the same for zext?).

Regarding your cases...

I can be wrong, but we do care about use count when X == Y because the way how we optimize depends on it. Look at the example below

  declare void @use(i32)
  define i32 @src(i1 %x, i1 %y) {
    %zx = zext i1 %x to i32
    %r = mul i32 %zx, %zx
    call void @use(i32 %zx)
    ret i32 %r
  }

If there is more than one user (or 2 uses in other words), we cannot replace

  %zx = zext i1 %x to i32
  %r = mul i32 %zx, %zx

by

  %r = and i1 %x, %y
  %zy = zext i1 %r to i32

because zx has another user (call of @use(i32)). Otherwise, we need to leave zx as-is. I'm not sure if we should do this (would it be beneficial?). I didn't intend to handle this particular case as a part of that patch.

The second case where either of ops has one use. Yes, it must be legal to do such a transformation when both ops has one use and they are different instruction. As an addition alive says it is correct <https://alive2.llvm.org/ce/z/ADMksb>. Previously, it was Op0->hasOneUse() || Op1->hasOneUse() and when there is one use, then there is one user so it must be equivalent to I.isOnlyUserOfAnyOperand() in that case.

I guess, tests like mul_bools_sext_use2 must partially check the second case, but yes, there are no tests like.

  define i32 @func(i1 %x, i1 %y) {
    %sx = sext i1 %x to i32
    %sy = sext i1 %y to i32
    %r = mul i32 %sy, %sx
    ret i32 %r

I'll update the diff


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104193/new/

https://reviews.llvm.org/D104193