[PATCH] D97133: [SelectionDAG][RISCV] Teach ComputeNumSignBits to handle SREM.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3877
+    // at least as many sign bits as the left hand side.
+    return ComputeNumSignBits(Op.getOperand(0), DemandedElts, Depth + 1);
   case ISD::TRUNCATE: {
----------------
luismarques wrote:
> RKSimon wrote:
> > This is a lot more relaxed than ValueTracking ?
> Interesting! ValueTracking seems to abort on `!Denominator->isStrictlyPositive()`. When I reviewed the logic of this code at first I thought there would be cases where it would break, but when I thought about it more deeply it all seemed fine. Now I'm curious about what may explain the divergence. Maybe the implementation in ValueTracking was considering modulus instead of remainder?
The implementation in value tracking only handles constant right hand side. Looking at the history, its original implementation was incorrect and maybe thought the result was always positive at least from the the original comment.

I didn't handle constant here because I figured most targets would expand an srem by constant using BuildSDIV in TargetLowering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97133