The patch fix regressions in

Shootout-C++ matrix

and

spec2006/hmmer

caused by D29862.

# Details

- Reviewers
qcolombet

# Diff Detail

- Repository
- rL LLVM

### Event Timeline

Could you explain what was the problem and how this patch addresses it?

Sure, here is the case:

Use1 (Address):

{a} + {0,+,1} register num expectation 1 + 1/2

{a,+,1} register num expectation 1

Use2 (Address):

{b} + {0,+,1}

{b,+,1}

Use3 (ICmpZero):

-1024 + {0,+,1}

That way new method will select {a,+1} {b,+,1} and -1024 + {0,+,1} which is not optimal in terms of AddRecExprs (still optimal in terms of RegNum)

In Matrix test there are 32 Address uses like above. The expectation of {a} + {0,+,1} becomes very close 1, but still grater than 1.

The solution is simple - delete formulas in ICmpZero before Address. That way the optimal solution is selected (as {0,+,1} becomes unique).

lib/Transforms/Scalar/LoopStrengthReduce.cpp | ||
---|---|---|

4446 | This is a kind of unrelated change (as it is redundant initially). We leave only 1 formula in each LSRUse, therfore we already select a solution (or no solution). That way we don't need to RecomputeRegs any more. |

lib/Transforms/Scalar/LoopStrengthReduce.cpp | ||
---|---|---|

1080 | No. This to to make compare function easier (compare enum values): |

Trying to fix the regressions I've inserted a check which uses Solution generated by NarrowSearchSpaceByDeletingCostlyFormulas() only if it's cost less than cost of regular Solution.

That way NarrowSearchSpaceByDeletingCostlyFormulas() can only decrease final Solution cost. But both matrix and hmmer regression were not resolved.

It turned out that NarrowSearchSpaceByDeletingCostlyFormulas() creates Solution with is better cost (using current cost model) for both cases.

Use of Insns in Cost model helps to resolve the issue.

Right now I see 3 ways to resolve this:

- Accept current workaround.
- Modify NarrowSearchSpaceByDeletingCostlyFormulas() to rely in Insns instead of Registers.
- Change cost model to rely on Insns first and check which solution is better: regular or created by NarrowSearchSpaceByDeletingCostlyFormulas().

lib/Transforms/Scalar/LoopStrengthReduce.cpp | ||
---|---|---|

4392 | We can sort Uses indexes here instead of sorting Uses. That way we'll avoid change on line 4435. |

How do you think which way we should follow?

Or we can leave regression as is as Solution Cost is better and therefore it is not a problem of newly implemented NarrowSearchSpaceByDeletingCostlyFormulas()?