[PATCH] D21299: [Codegen Prepare] Swap commutative binops before splitting branch condition.

[Renato Golin via llvm-commits]

rengolin added a comment.

Tim, both sets of numbers are statistically equivalent (and both move down to 0.999...), so the change is probably completely innocuous at run time, and surely adds some compile time.

As Balaram said, this helps https://reviews.llvm.org/D20030, which Kristof has seen some improvements, so there could be some merit underneath it all.

My take away points from this discussion are:

1. The two feature flags are very close to each other, and I think more investigation is needed to make sure you **must** create a new flag instead of using the current one.
2. The choice does look arbitrary, but I think Balaram's argument to make it more predictable interesting. I have no idea how this could play on the vast different (sub-)architectures out there, but it shouldn't be worse, on average, than a non-predictable output.
3. Maybe what we need is to merge these two patches and test on all architectures that have the FeaturePredictableSelectIsExpensive to make sure this is not the same effect showing up in different ways.

If the predictability makes it converge to a reasonable scenario, then I think the change is positive. If not, it's probably just noise.


https://reviews.llvm.org/D21299