[PATCH] D140858: [InstCombine]: Don't simplify bits if it causes imm32 to become imm64

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D140858#4022695 <https://reviews.llvm.org/D140858#4022695>, @nikic wrote:

> My first question here would be whether we should be undoing this in the backend instead. We perform demanded bits analysis on SDAG, and there is an existing targetShrinkDemandedConstant() hook. The name suggests that it should shrink the constant, but I see no reason why it couldn't do the reverse if that is profitable for the target. From a quick look, it seems like RISCV already does something like that.

See my other comment, but when I tested this in the X86 backend but if the known bits are determined with `llvm.assume` it seems that information is lost by the time it reached the backend.

> Of course, doing this in InstCombine may also make sense -- the test diffs look like this might be slightly beneficial at the IR layer for the pattern emitted by LoopVectorize (e.g. see diffs in LoopVectorize/X86/small-size.ll). If we do want this in InstCombine, I'd expect a more principled approach that does not hardcode specific sizes. E.g. we could generally avoid masking off any sign bits, to avoid increasing the number of significant (signed) bits.

Can you expand on the rationale behind not using `TTI.getIntImmCostInst` here? This doesn't seem like a target specific transform and "is this imm better than that one?" seems like an inherently target specific question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140858