Hi Jeff > Which gets repeated 2 more times for the other values. The sgt insn is > repeated and the only obvious redundancy. Avoiding the redundancy would > help, but likely would only bring the required cost down to 7 to trigger > if-conversion (which still seems quite high). However, it may still be > worth eliminating that redundancy as doing so would likely result > if-converting cases where we need two generalized conditional moves at > just a branch cost of 4. I'll probably file that as a PR momentarily.
I agree with your conclusions. I also experimented locally with sharing the condition across the converted sets, so the comparison is emitted only once. For the MCF testcase, this changes the sequence from: 3 sgt + 6 czero + 3 add to: 1 sgt + 6 czero + 3 add This removes the redundant comparisons and makes the initial costing more accurate. However, even after this change, the three-select case still does not if-convert with the normal C950 cost setting. A cost-model adjustment is still needed for the optimization to trigger. My local shared-condition changes are only a draft at this point and are not ready for submission. > For the 3+ case I think we are going to need to do something with the > cost model, possibly along the lines of what you did. I think I've got > enough info now to dive into what you did and draw some conclusions. I also completed a full SPEC CPU2017 integer regression on C950 with the cost-model patch. The results are: gcc: about +1% xalancbmk: about +2% deepsjeng: about +3.5% mcf: about +5% The other benchmarks show no significant performance change.Since all measurable changes are positive, I think the existing branch-cost model is likely too conservative for RTL if-conversion on C950. Since the adjustment is limited to the C950 RTL noce profitability threshold and the full regression results are positive, I hope we can move forward with this patch. Best regards, Wang Jue
