[PATCH] D40855: [PowerPC] LSR tunings for PowerPC

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Target/PowerPC/PPCTargetTransformInfo.cpp:253
+  // calculation is less than C2. This leads to a more
+  // conservative selection than priortizing one over the other.
+  return C1.Insns <= C2.Insns && BaseT::isLSRCostLess(C1, C2);
----------------
This is a proper partial ordering: isLSRCostLess(C1, C2) && isLSRCostLess(C2, C1) is always false. However, I'm not sure the meaning is obvious...

For example, imagine that BaseT::isLSRCostLess(C1, C2) is true and BaseT::isLSRCostLess(C2, C1) is false. In this case, isLSRCostLess is either equivalent to BaseT::isLSRCostLess(C1, C2), if (C1.Insns <= C2.Insns), or isLSRCostLess implies equality (if C1.Insns > C2.Insns), because in this latter case, isLSRCostLess(C1, C2) is false and isLSRCostLess(C2, C1) is also false. In other words, you've taken all {C1, C2} for which C1 < C2 (by BaseT::isLSRCostLess) and C1.Insns > C2.Insns, and made them equal in LSR cost. You say that this is a more conservative selection, but it just seems more arbitrary to me (you've enlarged the class of equal things). If this is what you intended, it should have a specific explanation.


https://reviews.llvm.org/D40855