[PATCH] D61160: [SimplifyCFG] Implement the suggested ctlz transform

[Shawn Landden via Phabricator via llvm-commits]

shawnl marked 3 inline comments as done.
shawnl added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5515
+    return false;
+  // TODO a single instruction can be saved on x86 by using cttz over ctlz
+  // when only ctlz results in a subtraction instruction being added
----------------
lebedev.ri wrote:
> shawnl wrote:
> > jmolloy wrote:
> > > This would be a pattern for instcombine to pick up. We shouldn't have to worry about this here.
> > Yes we do, because InstCombine does not know how to rearrange case statements.
> Sounds like a missed optimization on instcombine side then?
> Trying to handle every single unrelated micro-transform in every optimization is a path to failure.
Yeah I am working on some patches.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5519
+    V = APInt(BitWidth, V).countLeadingZeros();
+  // 0 will suddenly become the largest (BitWidth), so we need to sort again.
+  // Sorting is OK because we either make a transform on all numbers, or do
----------------
jmolloy wrote:
> shawnl wrote:
> > jmolloy wrote:
> > > Isn't the order exactly reversed?
> > > 
> > >   llvm::reverse(Values);
> > Nope, the first becomes the last. I guess I could use swap() and move().
> I'm not sure I agree with that, or at least I'm missing something.
> 
> Given a sorted array A of integers with popcount 1 (single bit set), I believe:
> 
>   order(A) == order(cttz(A))  //  cttz does not change the order.
>   order(A) == reverse(order(ctlz(A))) // Conversely ctlz reverses the order.
> 
> If you need to handle the zero value too, then I would have thought cttz would barrel-shift as you describe and ctlz would barrel-shift the reversed order.
> 
> My comment above was based on the assumption that you're not allowing zero to be passed to ctlz/cttz. Given you are, just keep on using llvm::sort.
Oh yes, I was think about cttz, but now I am only using ctlz, so I can use llvm::reverse()


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61160