[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:249-250
+ 
+  // PowerPC specific: check instruction count as well as default 
+  // cost calculation. 
+  return C1.Insns <= C2.Insns && BaseT::isLSRCostLess(C1, C2);
----------------
echristo wrote:
> Probably best to explain why this is useful and you can remove the "PowerPC specific" bit.
Also, why this formula? If C1.Insns < C2.Insns, we're deferring to the base implementation, but not if C1.Insns > C2.Insns. That seems different from what X86 does, where it defers whenever the instruction counts aren't equal:

ol X86TTIImpl::isLSRCostLess(TargetTransformInfo::LSRCost &C1,
                               TargetTransformInfo::LSRCost &C2) {
    // X86 specific here are "instruction number 1st priority".
    return std::tie(C1.Insns, C1.NumRegs, C1.AddRecCost,
                    C1.NumIVMuls, C1.NumBaseAdds,
                    C1.ScaleCost, C1.ImmCost, C1.SetupCost) <
           std::tie(C2.Insns, C2.NumRegs, C2.AddRecCost,
                    C2.NumIVMuls, C2.NumBaseAdds,
                    C2.ScaleCost, C2.ImmCost, C2.SetupCost);
}



https://reviews.llvm.org/D40855