[PATCH] D78765: [TRE] Fix bug in handling of switch statements

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> The accumulator has to start with a value that won't effect the computation. Zero works in the case where we are doing addition, but would not work if we were doing multiplication. We could possibly implement a version that looks at the operation and then decides a starting value.

tailcallelim currently only recognizes a very restrictive set of accumulator operations: ones where BinaryOperator::isAssociative returns true. ConstantExpr::getBinOpIdentity() can compute an appropriate identity value for all those operations.

> Alternatively, and what I am working on is, we could basically inline one iteration though the loop and start the accumulator with the value you get from the first iteration.

Compiler jargon for making a copy of the loop for the first iteration is "loop peeling".

In general, it's more restrictive to require an identity value, yes.  Peeling the first iteration of the loop provides an alternative transform in those cases.  But I can't think of any realistic case where we could prove an operation is associative without being able to compute an identity value.  And we don't really want to peel loops unless the peeled loop is significantly faster.

If we're not peeling the loop, I can't think of any other reason to keep the isDynamicConstant path around.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78765/new/

https://reviews.llvm.org/D78765