[PATCH] D109555: [APInt] Enable APInt to support zero bit integers.

[Chris Lattner via Phabricator via llvm-commits]

lattner marked an inline comment as done.
lattner added inline comments.


================
Comment at: llvm/include/llvm/ADT/APInt.h:575
   APInt &operator=(const APInt &RHS) {
     // If the bitwidths are the same, we can avoid mucking with memory
     if (isSingleWord() && RHS.isSingleWord()) {
----------------
craig.topper wrote:
> This comment is wrong. We aren't checking bitwidth, we're checking that both bitwidths are <=64, but not that they are the same.
agreed, good catch.


================
Comment at: llvm/include/llvm/ADT/APInt.h:579
       BitWidth = RHS.BitWidth;
-      return clearUnusedBits();
+      return *this;
     }
----------------
craig.topper wrote:
> Why don't we need to clearUnusedBits here?
I can split this out to a different patch if you prefer.  It just isn't needed.  RHS is already a well formed APInt, so it already has the right zero bits at the top.


================
Comment at: llvm/lib/Support/APInt.cpp:262
-APInt& APInt::operator*=(const APInt& RHS) {
-  assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
   *this = *this * RHS;
----------------
craig.topper wrote:
> I assume you droped this because it's checked in operator*?
Right.  I trimmed some redundant checks.


================
Comment at: llvm/lib/Support/APInt.cpp:1098
 APInt APInt::rotr(unsigned rotateAmt) const {
+  if (rotateAmt == 0)
+    return *this;
----------------
craig.topper wrote:
> BitWidth?
great catch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109555