[llvm-commits] [llvm] r108056 - /llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp

[Nick Lewycky]

Duncan Sands wrote:
> Author: baldrick
> Date: Sat Jul 10 15:31:42 2010
> New Revision: 108056
>
> URL: http://llvm.org/viewvc/llvm-project?rev=108056&view=rev
> Log:
> The accumulator tail recursion transform claims to work for any associative
> operation, but the way it's implemented requires the operation to also be
> commutative.  So add a check for commutativity (and tweak the corresponding
> comments).  This makes no difference in practice since every associative
> LLVM instruction is also commutative!

Which leads to the question, how did you notice this? :)

   Here's an example to show the need
> for commutativity: the accum_recursion.ll testcase calculates the factorial
> function.  Before the transformation the result of a call is
>    ((((1*1)*2)*3)...)*x
> while afterwards it is
>    (((1*x)*(x-1))...*2)*1
> which clearly requires both associativity and commutativity of * to be equal
> to the original.
>
> Modified:
>      llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=108056&r1=108055&r2=108056&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Sat Jul 10 15:31:42 2010
> @@ -16,9 +16,9 @@
>   //     transformation from taking place, though currently the analysis cannot
>   //     support moving any really useful instructions (only dead ones).
>   //  2. This pass transforms functions that are prevented from being tail
> -//     recursive by an associative expression to use an accumulator variable,
> -//     thus compiling the typical naive factorial or 'fib' implementation into
> -//     efficient code.
> +//     recursive by an associative and commutative expression to use an
> +//     accumulator variable, thus compiling the typical naive factorial or
> +//     'fib' implementation into efficient code.
>   //  3. TRE is performed if the function returns void, if the return
>   //     returns the result returned by the call, or if the function returns a
>   //     run-time constant on all exits from the function.  It is possible, though
> @@ -302,9 +302,9 @@
>   ///
>   Value *TailCallElim::CanTransformAccumulatorRecursion(Instruction *I,
>                                                         CallInst *CI) {
> -  if (!I->isAssociative()) return 0;
> +  if (!I->isAssociative() || !I->isCommutative()) return 0;
>     assert(I->getNumOperands() == 2&&
> -         "Associative operations should have 2 args!");
> +         "Associative/commutative operations should have 2 args!");
>
>     // Exactly one operand should be the result of the call instruction...
>     if ((I->getOperand(0) == CI&&  I->getOperand(1) == CI) ||
> @@ -369,11 +369,11 @@
>         return false;
>     }
>
> -  // If we are introducing accumulator recursion to eliminate associative
> -  // operations after the call instruction, this variable contains the initial
> -  // value for the accumulator.  If this value is set, we actually perform
> -  // accumulator recursion elimination instead of simple tail recursion
> -  // elimination.
> +  // If we are introducing accumulator recursion to eliminate operations after
> +  // the call instruction that are both associative and commutative, the initial
> +  // value for the accumulator is placed in this variable.  If this value is set
> +  // then we actually perform accumulator recursion elimination instead of
> +  // simple tail recursion elimination.
>     Value *AccumulatorRecursionEliminationInitVal = 0;
>     Instruction *AccumulatorRecursionInstr = 0;
>
> @@ -384,9 +384,9 @@
>     for (BBI = CI, ++BBI;&*BBI != Ret; ++BBI)
>       if (!CanMoveAboveCall(BBI, CI)) {
>         // If we can't move the instruction above the call, it might be because it
> -      // is an associative operation that could be tranformed using accumulator
> -      // recursion elimination.  Check to see if this is the case, and if so,
> -      // remember the initial accumulator value for later.
> +      // is an associative and commutative operation that could be tranformed
> +      // using accumulator recursion elimination.  Check to see if this is the
> +      // case, and if so, remember the initial accumulator value for later.
>         if ((AccumulatorRecursionEliminationInitVal =
>                                CanTransformAccumulatorRecursion(BBI, CI))) {
>           // Yes, this is accumulator recursion.  Remember which instruction
> @@ -483,7 +483,7 @@
>       }
>
>       // Add an incoming argument for the current block, which is computed by our
> -    // associative accumulator instruction.
> +    // associative and commutative accumulator instruction.
>       AccPN->addIncoming(AccRecInstr, BB);
>
>       // Next, rewrite the accumulator recursion instruction so that it does not
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>