[PATCH] D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 26 10:29:21 PDT 2019


nikic added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5563
+    for (auto &V : Values)
+      V = V >> Shift;
+  }
----------------
jmolloy wrote:
> Nit: I'd use:
> 
>   V >>= Shift;
> 
> Just to make it ultra-clear that we're updating V in-place (updating a mutable reference iterator is fine in this case, but it's something readers normally expect so it'd be good to make this very clear).
It's rather hidden due to overzealous `auto` use, but this is going to perform an arithmetic shift. This doesn't seem correct to me, as the used rotate operation degenerates to a //logical// shift for valid values (with low bits unset).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61151





More information about the llvm-commits mailing list