[PATCH] D107766: [InstCombine] Get rid of `hasOneUses()` when swapping `lshr` and `zext`

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/pr50555.ll:9
+; CHECK-NEXT:    [[SHR:%.*]] = zext i8 [[TMP1]] to i32
+; CHECK-NEXT:    [[ADD:%.*]] = add nuw nsw i32 [[ZEXT]], [[SHR]]
 ; CHECK-NEXT:    store i32 [[ADD]], i32* [[A:%.*]], align 4
----------------
>>! In D107766#2934536, @lebedev.ri wrote:
> We can not do this transform as proposed here,
> it increases the instruction count.
> 
> Could you point me at the test for this change?
> It should only contain lshr-of-zext, there should not be any trunc;
> please add a test where zext has an extra use.

@lebedev.ri: Yes, you're right, it increases instruction count, adding new `zext` when the old one  has an extra use. But it also simplifies `lshr` to lower bits type making it simpler. Isn't it a good compromise?

And also, after `zext` sinking, it can trigger a chain of changes combining `zext` with other instrs like in cases below: `add`, `trunc` and so on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107766