[PATCH] D155472: [DAG] Attempt shl narrowing in SimplifyDemandedBits

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:1802
+          unsigned NumSignBits = TLO.DAG.ComputeNumSignBits(NewOp, Depth + 1);
+          KnownBits NewKnownBits = TLO.DAG.computeKnownBits(NewOp, Depth + 1);
+          SDNodeFlags Flags;
----------------
RKSimon wrote:
> goldstein.w.n wrote:
> > So you essentially end up doing recursive `computeKnownBits` 3 times here. Once in `MaskedValueIsZero`, once for `ComputeNumSignBits` and once outright.
> > I'd say after all the legalization / basic checks it would make more sense to compute knownbits once and do `MaskValueIsZero` / `ComputeNumSignBits` by hand with the knownbits.
> Yes I can merge the computeKnownBits / MaskedValueIsZero.
> 
> I don't understand why you think computeNumSignBits can be merged as well though? It only internally falls back to computeKnownBits sometimes.
> Yes I can merge the computeKnownBits / MaskedValueIsZero.
> 
> I don't understand why you think computeNumSignBits can be merged as well though? It only internally falls back to computeKnownBits sometimes.

You're right. Although you could probably edit `computeNumSignBits` to output `KnownBits` as well (as it seems we either have a constant or check against `computeKnownBits`).
But yeah feel free to skip for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155472