[PATCH] D30265: [APInt] Add APInt::setBits() method to set all bits in range

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 22 13:34:25 PST 2017


RKSimon added inline comments.


================
Comment at: lib/Support/APInt.cpp:574
+    *this |= APInt::getBitsSet(BitWidth, loBit, hiBit);
+  else if (hiBit < loBit) {
+    for (unsigned bit = 0; bit != hiBit; ++bit)
----------------
hans wrote:
> Is the `hiBit < loBit` case common?
> 
> If not (or maybe even then) perhaps just `setBits(0, hiBit); setBits(loBit, BitWidth); return` and focus on the `loBit < hiBit` case below?
No, I was just matching the behaviour of the '|= APInt::getBitsSet()' pattern but it doesn't seem common compared to using APInt::getHighBitsSet and APInt::getLowBitsSet instead and in those cases its more important to be able to support the 'all bits case'.

I'll change it.


================
Comment at: lib/Support/APInt.cpp:580
+  } else {
+    for (unsigned bit = loBit; bit != hiBit; ++bit)
+      setBit(bit);
----------------
hans wrote:
> Would it be worth trying to do this word-by-word instead?
Yes I can do that.


Repository:
  rL LLVM

https://reviews.llvm.org/D30265





More information about the llvm-commits mailing list