[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#4022703 <https://reviews.llvm.org/D140858#4022703>, @RKSimon wrote:

> +1 to investigate whether this can be handled in DAG before adding something so specific to InstCombine - if value tracking can confirm that the upper bits are already zero there's no reason why the various SimplifyDemanded* calls shouldn't determine if a sext-imm is a better option for a specific target

So I took a look at X86 which already has `X86DAGToDAGISel::shrinkAndImmediate`. Tested with:

  long td(long a) {
    if (a & (3L << 62)) {
      __builtin_unreachable();
    }
    return a & (-16L);
  }

Which gets:

  %tobool.not = icmp ult i64 %a, 4611686018427387904
  tail call void @llvm.assume(i1 %tobool.not)
  %and1 = and i64 %a, 4611686018427387888
  ret i64 %and1

For some reason, however, the known bit information is lost by the time `X86DAGToDAGISel::shrinkAndImmediate`
is being called.
In `SelectionDAG::MaskedValueIsZero` earlier in the pipeline we get known as `Zeros / Ones: c00000000000000f / 0`, but in
X86ISelDAGToDag.cpp its: `Zeros / Ones: 0 / 0`.

Likewise I checked around `TargetLowering::SimplifyDemandedBits` in the `ISD::AND` case and by then as well `LHSKnown`
doesn't have the information needed: `LHSKnown Zeros/Ones: 0 / 0`.

My general (absolutely non-expert) opinion is handling this at InstCombine with `TTI.getIntImmCostInst` makes the most sense.
It avoids having to reroll this code in all target backend and seems to occur before some information loss that makes undoing
difficult.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140858