[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