[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:1797
+            (DemandedBits.countLeadingZeros() >= (BitWidth / 2) ||
+             TLO.DAG.MaskedValueIsZero(Op0, HiBits))) {
+          SDValue NewOp = TLO.DAG.getNode(ISD::TRUNCATE, dl, HalfVT, Op0);
----------------
nit: I would  make a variable `HalfWidth = BitWidth / 2` to avoid duplicating it alot in the condition (and needing parens and such).


================
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;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155472