[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:472
+
+    if (BitDistance > 0) {
+      // When the shift size is not a multiple of the word size, then we have
----------------
Instead of this, test for 0 and do the word shift and return early? That will let the complex case be less indented.


================
Comment at: llvm/include/llvm/ADT/BitVector.h:524
+
+    if (BitDistance > 0) {
+      // When the shift size is not a multiple of the word size, then we have
----------------
Same as above.


================
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.
----------------
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.. =[


https://reviews.llvm.org/D32244