[PATCH] D149918: [InstCombine] Add oneuse checks to shr + cmp constant folds.

Mon May 8 00:14:20 PDT 2023

aemerson added a comment.

In D149918#4325535 <https://reviews.llvm.org/D149918#4325535>, @goldstein.w.n wrote:

> In D149918#4325473 <https://reviews.llvm.org/D149918#4325473>, @dmgreen wrote:
>
>> I can verify that this fixes the issues, including the other one from https://reviews.llvm.org/D143624#4315468 that looks more difficult to reverse. It may be worth adding some tests like https://godbolt.org/z/cxKrfM4TK. But otherwise it LGTM. Getting more ILP sounds like something the backend should handle, if it is profitable given the constants.
>
> I think the generic statement "more ILP is better than less ILP" is fair for instcombine to make.

Do you have an alternative suggestion given that we can’t really undo some of these transforms? ILP is not the only consideration that instcombine has, code size (which we can demonstrate is improved by this change) is an equally important goal.

> Choosing which constants are profitable (what this patch is trying to approximate), however, still doesn't make sense to me.

We’re not trying to approximate choosing constants. We’re just adding a one use check.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149918