[PATCH] D37467: Add a new pass to speculate around PHI nodes with constant (integer) operands when profitable.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Catching up on old review traffic.  Nothing critical.



================
Comment at: llvm/trunk/include/llvm/Transforms/Scalar/SpeculateAroundPHIs.h:84
+///   ```
+///     testq %eax, %eax
+///     addl $7, %edi
----------------
Isn't this snippet wrong?  It looks like we're adding 7 or 18 here, not 7 or 11.  I think you're missing a conditional branch.  Alternatively, you could make the second add be an "addli $4, edi" (since 4+7 == 11)

If we don't do that later transformation, maybe we should be.  Extracting out the common component of the two constants adding that unconditionally and then conditionally adding the difference seems like a near ideal lowering here.  We'd end up with something like:
///     testq %eax, %eax
///     addl $7, %edi
///     jne .L
///     addl $4, %edi <-- different constant
///   .L:
///     movl %edi, %eax
///     retq






Repository:
  rL LLVM

https://reviews.llvm.org/D37467