[llvm-commits] Reloc function mask helper

[Sean Silva]

+        //if the block was already started increment upper bound

<http://llvm.org/docs/CodingStandards.html#commenting>. Comments
should be English prose (proper capitalization, punctuation, etc).
Also every other comment in LLVM has a space after the //. Also, there
is no comment for the function itself explaining what it is doing.

+        if (blockStarted) {
+           break;
+        } else {
+          lowerBound++;
+        }

<http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return>.

+    data = data | ( miniMask & (pData << (lowerBound - nextToConsume)));

Why the space after the `(`. Also, what does is the `p` in `pData` and
`pMask` mean?

-/// \brief Word32_B22: 0x01ff3ffe : (S + A - P) >> 2 : Verify
+/// \briee Word32_B22: 0x01ff3ffe : (S + A - P) >> 2 : Verify

What is the point of this? To just mess up the spelling of `\brief`?

+      mask >>= 1 ;

No space before the semicolon.

+    lowerBound = 1 ; blockStarted = false;
Keep statements one per line. Also get rid of the space before the semicolon.

If I understand correctly, then this function is basically doing the
same thing as the AVX2 PDEP instruction. Your algorithm here is really
convoluted for doing that (what is the loop invariant?). This is the
(bit-level) pseudocode from the AVX instruction definition:

TEMP <- SRC1;
MASK <- SRC2;
DEST <- 0 ;
m <- 0, k <- 0;
DO WHILE m < OperandSize
  IF MASK[ m] = 1 THEN
    DEST[ m] <- TEMP[ k];
    k <- k + 1;
  FI
  m <- m + 1;
OD

Which in C/C++ notation would be something like this (completely untested):

uint32_t Incoming = ...;
uint32_t Mask = ...;
uint32_t Result = 0;
auto getLowBitInHighBit = [](uint32_t x) { return (x & 0x1) << 31};
// Loop invariant: For each 1 bit in [0,i) of Mask, the same bit in Result has
// been set to the value of the Incoming[popcnt(Mask[0,i))-1]
(indexing bit-wise).
for (unsigned i = 0; i != 32; ++i, Mask >>= 1, Result >>= 1) {
  if (!(Mask & 0x1))
    continue;
  Result |= getLowBitInHighBit(Incoming);
  Incoming >>= 1;
}

-- Sean Silva