[PATCH] D56087: [TargetLowering][AMDGPU] Remove the SimplifyDemandedBits function that takes a User and OpIdx. Stop using it in AMDGPU target for simplifyI24.

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 06:05:25 PST 2019


arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.cpp:4325
+      bool RHSNegative = RHSKnown.isNegative();
+      bool RHSPositive = RHSKnown.isNonNegative();
       if ((!LHSNegative && !LHSPositive) || (!RHSNegative && !RHSPositive))
----------------
craig.topper wrote:
> RKSimon wrote:
> > This looks (mostly) like an NFC? Just commit the bits that you can?
> The part I'm most concerned about is that MUL_U24 is now using countMinLeadingZeros. I couldn't figure out why it was using countMinSignBits for both signed and unsigned before
There's this comment that I've never fully understood:

  // We need to use sext even for MUL_U24, because MUL_U24 is used
  // for signed multiply of 8 and 16-bit types.
  return DAG.getSExtOrTrunc(Mul, DL, VT);


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56087





More information about the llvm-commits mailing list