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

[Chris Lattner]

On Jan 3, 2011, at 8:52 PM, Tobias Grosser wrote:

> Hi,
> 
> 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

Cool, thanks for tackling this.  A couple comments though:

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.

Instead of things like:
+          SExtInst *TrueValSExt = dyn_cast<SExtInst>(TrueVal);
+          SExtInst *FalseValSExt = dyn_cast<SExtInst>(FalseVal);
+
+          AdjustedRHS = ConstantExpr::getSExt(AdjustedRHS, SI.getType());
+
+          if (TrueValSExt && CmpLHS == TrueValSExt->getOperand(0)
+              && AdjustedRHS == FalseVal)
+            CmpLHS = TrueVal;

Please use:

  if (match(TrueVal, m_SExt(m_Specific(CmpLHS))) &&
      AdjustedRHS == FalseVal)
    CmpLHS = TrueVal;

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

Thanks for working on this!

-Chris