[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