[llvm-commits] [PATCH] Fix: instcombine does not get max if hidden by sext

[Frits van Bommel]

On Thu, Jan 6, 2011 at 7:49 AM, Chris Lattner <clattner at apple.com> wrote:
> On Jan 3, 2011, at 8:52 PM, Tobias Grosser wrote:
>> I fixed a recent bug report, that blocked me on a large FORTRAN test case:
>>
>>    InstCombine:
>>    X = sext x ; x < c ? X : C-1
>>    -->
>>    X = sext x; X > C-1 ? C-1 : X
>>
>>    Instead of calculating this with mixed types promote all to the
>>    larger type. This enables scalar evolution to analyze this
>>    expression. PR8866

Why only for sext and not zext? Does scalar evolution understand those
well enough that it's not needed there or was it just not useful for
your particular test case?
Or maybe the zext case is too different from the sext case to add easily?

> Generally, the logic involved is complex enough that it is probably best to pull it out into a static helper function.  This should make it easier to read.

And since the "less than" and "greater than" cases seem to be quite
similar, you could try to use a single helper function for both.


Is the fallthrough at the end of the "less than" case intentional? If
so, a big '// FALLTHROUGH' would probably be helpful. If not, add a
'break', obviously. Either way, adding one at the end of the "greater
than" case would future-proof it in case anyone adds extra cases.


> It's probably also good to get Frits to review this, he has a keen eye :)

Thanks :).