[PATCH] D32244: [BitVector] Add << and >> operators

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: llvm/include/llvm/ADT/BitVector.h:655-661
+  /// need to move the bytes of the memory to the right, not to the left.
+  /// Example:
+  ///   Words = [0xBBBBAAAA, 0xDDDDFFFF, 0x00000000, 0xDDDD0000]
+  /// represents a BitVector where 0xBBBBAAAA contain the least significant
+  /// bits.  So if we want to shift the BitVector left by 2 words, we need to
+  /// turn this into 0x00000000 0x00000000 0xBBBBAAAA 0xDDDDFFFF by using a
+  /// memmove which moves right, not left.
----------------
craig.topper wrote:
> zturner wrote:
> > chandlerc wrote:
> > > Reality is worse than this. Using memmove operates on bytes, but the words might be in bigendian and thus the bytes *within* the words ordered differently. =[
> > > 
> > > I think you should either write a for loop that does the move a word at a time (and thus is obviously endian agnostic) or you'll need to do some endian dance here.. =[
> > My brain already hurts from this, so I give the odds of me being completely wrong about 100%, but does this matter?
> > 
> > Although memmove moves bytes, the function itself moves a number of bytes that is always a multiple of the word size, and the move happens at a word-aligned boundary, so we should be guaranteed that we're just picking up a chunk of words in whatever endianness they may already be in, and moving that same chunk somewhere else in the same endianness.
> > 
> > shouldn't that work?
> Aren't we always moving a multiple of words with the memmove which should hide the endianess issue? Richard Smith just added a memmove to the equivalent shift code in APInt so I really hope memmove isn't an issue here.
Sorry, I got confused by the predicate above that talks about `BitDistance` and thought this was the *byte* distance not *word* distance despite the number of times it says "word".

Maybe just an explicit comment here reminding the reader that this is safe because we move in increments of words.


https://reviews.llvm.org/D32244