[PATCH] D116469: [AMDGPU] Correct the known bits calculation for MUL_I24.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp:4629
     if (Opc == AMDGPUISD::MUL_I24) {
-      unsigned LHSValBits = 24 - LHSKnown.countMinSignBits();
-      unsigned RHSValBits = 24 - RHSKnown.countMinSignBits();
-      unsigned MaxValBits = std::min(LHSValBits + RHSValBits, 32u);
-      if (MaxValBits >= 32)
+      unsigned LHSValBits = 24 - LHSKnown.countMinSignBits() + 1;
+      unsigned RHSValBits = 24 - RHSKnown.countMinSignBits() + 1;
----------------
RKSimon wrote:
> Do you think it'd be confusing if we added KnownBits::getMinSignedBits() to match APInt::getMinSignedBits() ?
Shouldn’t it be getMaxSignedBits? It’s an upper bound like getMaxActiveBits.

Likewise ComputeMinSignedBits is also misnamed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116469