[PATCH] D69032: [APInt] add wrapping support for APInt::setBits

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/include/llvm/ADT/APInt.h:606-613
   static APInt getBitsSet(unsigned numBits, unsigned loBit, unsigned hiBit) {
     APInt Res(numBits, 0);
-    Res.setBits(loBit, hiBit);
+    if (loBit <= hiBit)
+      Res.setBits(loBit, hiBit);
+    else
+      Res.setBitsWithWrap(loBit, hiBit);
     return Res;
----------------
shchenz wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > These changes are still there
> > To reword, the suggestion is to
> > 
> > ```
> >   static APInt getBitsSet(unsigned numBits, unsigned loBit, unsigned hiBit) {
> >     APInt Res(numBits, 0);
> >     Res.setBits(loBit, hiBit);
> >     return Res;
> >   }
> > 
> >   static APInt getBitsSetWithWrap(unsigned numBits, unsigned loBit, unsigned hiBit) {
> >     APInt Res(numBits, 0);
> >     Res.setBitsWithWrap(loBit, hiBit);
> >     return Res;
> >   }
> > ```
> I think if we want to avoid recursion in `setBits`, we just need to split `setBits` for wrap and non-wrap.   I am not sure the benifit of spliting `getBitsSet`, could you point it out to me?
Presumably all existing users of `getBitsSet()` satisfy `loBit <= hiBit` precondition,
and don't need the new `setBitsWithWrap()` behaviour. Which means the `getBitsSet()`
change as-is will likely affect the performance of every current `getBitsSet()` user.
It may be best to simply not touch existing `getBitsSet()`, but add `getBitsSetWithWrap()`.


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

https://reviews.llvm.org/D69032