[PATCH] D143882: [ADT] Add llvm::rotl and llvm::rotr to bit.h

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/include/llvm/ADT/bit.h:366
+  if (R < 0)
+    return llvm::rotr(V, -R);
+
----------------
This is dead code. Because you defined N as unsigned, R will be converted to unsigned for the % operation on line 361 and the result will never be negative. It gets the right answer anyway, but it doesn't seem to be what you intended.

As an example, in your test "llvm::rotl<uint8_t>(0x53, -5)" this will happen

N = 8u; // (std::numeric_limits<uint8t>::digits)
R = 0xFFFFFFFB % 8u; // (unsigned)R % N

So R = 3, and rotl(0x53, 3) yields 0x9A, as expected.

The point is, the code on lines 365-366 isn't doing anything. The same applies to lines 378-379 below.

This was reported by Coverity, BTW, which is why I happened to notice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143882/new/

https://reviews.llvm.org/D143882