Small problem in BitVector.h

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?

- Roman

Index: BitVector.h

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

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.

Yes, please apply!

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

Too much subtlety :frowning:

-Chris