[LLVMdev] Small problem in BitVector.h

[heisenbug]

On Jan 23, 1:51 pm, Roman Levenstein <romix.l... at googlemail.com>
wrote:
> Hi,
>
> Doing some profiling of llc, I noticed that some bitvector operations
> took longer than usual. Then I noticed that too many copies of
> BitVector obejcts are created, even when such operations like &=, ^=,
> |= are performed on those bit vectors.
>
> I looked at the BitVector ADT implementation in BitVector.h and
> figured out that all assignment operations (except the usual
> assignment operator) return a copy of the bit vector, instead of a
> reference!
> I guess it was just overlooked.
>
> Below is the patch to fix it. Is it OK to commit?

Yes. Please note in the commit message that the old
semantics probably did not meet the expectations.
With your patch, chained assignments happen to the right
object.

A very good catch, and a nice demonstration of how
C++'s performance characteristics can be spoiled
by small bugs like these.

Cheers,

    Gabor


>
> - Roman
>
> Index: BitVector.h
> ===================================================================
> --- BitVector.h (revision 62258)
> +++ BitVector.h (working copy)
> @@ -286,7 +286,7 @@
>    }
>
>    // Intersection, union, disjoint union.
> -  BitVector operator&=(const BitVector &RHS) {
> +  BitVector &operator&=(const BitVector &RHS) {
>      unsigned ThisWords = NumBitWords(size());
>      unsigned RHSWords  = NumBitWords(RHS.size());
>      unsigned i;
> @@ -302,14 +302,14 @@
>      return *this;
>    }
>
> -  BitVector operator|=(const BitVector &RHS) {
> +  BitVector &operator|=(const BitVector &RHS) {
>      assert(Size == RHS.Size && "Illegal operation!");
>      for (unsigned i = 0; i < NumBitWords(size()); ++i)
>        Bits[i] |= RHS.Bits[i];
>      return *this;
>    }
>
> -  BitVector operator^=(const BitVector &RHS) {
> +  BitVector &operator^=(const BitVector &RHS) {
>      assert(Size == RHS.Size && "Illegal operation!");
>      for (unsigned i = 0; i < NumBitWords(size()); ++i)
>        Bits[i] ^= RHS.Bits[i];
> _______________________________________________
> LLVM Developers mailing list
> LLVM... at cs.uiuc.edu        http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev