[llvm-commits] [llvm] r120024 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineAddSub.cpp lib/Transforms/InstCombine/InstCombineAndOrXor.cpp lib/Transforms/InstCombine/InstructionCombining.cpp test/

[Frits van Bommel]

On Tue, Nov 23, 2010 at 3:23 PM, Duncan Sands <baldrick at free.fr> wrote:
> +/// LeftDistributesOverRight - Whether "X LOp (Y ROp Z)" is always equal to
> +/// "(X LOp Y) ROp (Z LOp Z)".

I believe you typo'd "(X LOp Y) ROp (X LOp Z)" here.

> +static bool LeftDistributesOverRight(Instruction::BinaryOps LOp,
> +                                     Instruction::BinaryOps ROp) {

[snip]

> +  // Does "X op' (Y op Z)" always equal "(X op' Y) op (X op' Z)"?
> +  bool LeftDistributes = LeftDistributesOverRight(InnerOpcode, OuterOpcode);
> +  // Does "(X op Y) op' Z" always equal "(X op' Z) op (Y op' Z)"?
> +  bool RightDistributes = RightDistributesOverLeft(OuterOpcode, InnerOpcode);

Why are these two here instead of in the if() where they're tested?
They don't seem to be reused...

> +  // Does "X op' Y" always equal "Y op' X"?
> +  bool InnerCommutative = Instruction::isCommutative(InnerOpcode);
> +
> +  if (LeftDistributes)
> +    // Does the instruction have the form "(A op' B) op (A op' D)" or, in the
> +    // commutative case, "(A op' B) op (C op' A)"?
> +    if (A == C || (InnerCommutative && A == D)) {
> +      if (A != C)
> +        std::swap(C, D);
> +      // Consider forming "A op' (B op D)".
> +      // If "B op D" simplifies then it can be formed with no cost.
> +      Value *RHS = SimplifyBinOp(OuterOpcode, B, D, TD);
> +      // If "B op D" doesn't simplify then only proceed if both of the existing
> +      // operations "A op' B" and "C op' D" will be zapped since no longer used.

You typo'd "since /they're/ no longer used" here.

> +      if (!RHS && Op0->hasOneUse() && Op1->hasOneUse())
> +        RHS = Builder->CreateBinOp(OuterOpcode, B, D, Op1->getName());
> +      if (RHS)
> +        return BinaryOperator::Create(InnerOpcode, A, RHS);
> +    }
> +
> +  if (RightDistributes)
> +    // Does the instruction have the form "(A op' B) op (C op' B)" or, in the
> +    // commutative case, "(A op' B) op (B op' D)"?
> +    if (B == D || (InnerCommutative && B == C)) {
> +      if (B != D)
> +        std::swap(C, D);
> +      // Consider forming "(A op C) op' B".
> +      // If "A op C" simplifies then it can be formed with no cost.
> +      Value *LHS = SimplifyBinOp(OuterOpcode, A, C, TD);
> +      // If "A op C" doesn't simplify then only proceed if both of the existing
> +      // operations "A op' B" and "C op' D" will be zapped since no longer used.

And here too.

> +      if (!LHS && Op0->hasOneUse() && Op1->hasOneUse())
> +        LHS = Builder->CreateBinOp(OuterOpcode, A, C, Op0->getName());
> +      if (LHS)
> +        return BinaryOperator::Create(InnerOpcode, LHS, B);
> +    }
> +
> +  return 0;
> +}