[llvm-commits] [PATCH] - endless loop in instcombine and a small constant folding optimization on vectors

[Frits van Bommel]

On Tue, Apr 12, 2011 at 8:23 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Rotem, Nadav wrote:
>> Please review the attached patch:
>>
>> 1. Fix an endless loop in instcombine. VisitAnd changes the instruction, and VisitXor fixes it in an endless loop.
>
> Hrmm. Neither visitAnd nor visitXor should be swapping operands for
> funsies, there should be one canonical form of the expression and
> instcombine should produce it.

Yet apparently visitAnd does swap operands even if it isn't sure yet
whether that helps.
A better fix would probably be to not swap them in the first place...

> I tried to analyze this, but I found that in your testcase, my
> breakpoint in visitXor never fired, there was only a look on visitAnd...
>
>> 2. A small constant folding patch vector select with a splat condition.
>
> +  // Handles cases where the condition is splat vector.
> +  // select <false, false, ..>, X, Y -> Y
> +  // select <true, true, ..>, X, Y -> X
> +  if (isa<ConstantAggregateZero>(CondVal)) return FalseVal;
> +  if (ConstantVector *CP = dyn_cast<ConstantVector>(CondVal)) {
> +    if (CP->isAllOnesValue()) return TrueVal;
>
> if (CP->isNullValue()) return FalseVal;  ?

ConstantVectors can't be null values, that's the reason for the
ConstantAggregateZero check.
This could probably be written more cleanly by using m_Zero and
m_AllOnes/m_One from llvm/Support/PatternMatch.h though (and combined
with the non-vector versions at the top of the function).