[PATCH] D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two
Shawn Landden via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 27 19:31:59 PDT 2019
shawnl marked an inline comment as done.
shawnl added inline comments.
================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5563
+ for (auto &V : Values)
+ V = V >> Shift;
+ }
----------------
nikic wrote:
> 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).
Yeah, that happened when I split the patches up. (and the style issue because I just simplified the old code, but I was using unsigned then)
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