[PATCH] D30525: [APInt] Add setLowBits/setHighBits methods to APInt.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 11:14:22 PST 2017


craig.topper added inline comments.


================
Comment at: lib/Support/APInt.cpp:575
+  if (loWord == hiWord) {
+    // Set bits are all within the same word, create a [loBit,hiBit) mask.
+    loMask >>= (APINT_BITS_PER_WORD - (hiBit - loBit));
----------------
hans wrote:
> This comment isn't really true anymore since you're only creating loMask (and when it gets left-shifted later, the name loMask isn't really appropriate anymore).
> 
> It might make sense to keep these two cases separate and let the compiler handle the redundant instructions.
> 
> I see the logic is changing a little, with hiWord now referring to whichWord(hiBit) instead of whichWord(hiBit - 1). Did you find a bug, or is this just trying to make the code clearer?
No bug. Was just trying to see if I could do it with less -1. I'll change it back to replicating the redundant instructions again.


https://reviews.llvm.org/D30525





More information about the llvm-commits mailing list