[PATCH] D123819: [InstCombine] Fold strlen and strnlen recursively.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D123819#3542661 <https://reviews.llvm.org/D123819#3542661>, @msebor wrote:

> I'm not sure where this stands now.  Are you expecting me to submit your patches for review and then incorporate the recursion part separately?

Yeah, that's what I'd suggest. Maybe not exactly those patches, but something that's functionally similar.

For the recursion part, there are a few additional issues we'd want to consider: a) reused operands in nested selects, b) potential infinite loops in unreachable code (not sure if SLC can run on unreachable code), c) potentially non-profitable transform for multi-use selects, d) potentially deep recursion without limit. (Some of those have a common solution.)

In D123819#3542702 <https://reviews.llvm.org/D123819#3542702>, @xbolva00 wrote:

> Cases like  strlen (x ? s1 : s2) should be handled in more general fashion and rewrite fn (x ? s1 : s2)  as x ? fn(s1) : fn(s2)  if  fn(s1) and (or?) fn(s2)  can be optimized to just some (constant) value.
>
> Same for more chains like fn (x == 3 ? s3 : x == 5 ? s5 : s7) to x == 3 ? fn(s3) : x == 5 ? fn(s2) : fn(s5).

We generally cannot optimize `fn(x ? s1 : s2)` to `x ? fn(s1) : fn(s2)`, because this would unconditionally execute `fn()` on both inputs. We can only do this if //both// sides fold to a speculatable result. Otherwise this is only possible by introducing a branch, which is not possible in SLC (and has unclear profitability).

In D123819#3542796 <https://reviews.llvm.org/D123819#3542796>, @msebor wrote:

> For what it's worth, GCC does this transform in its Partial Redundancy Elimination pass that sits on top of Global Value Numbering.  I don't see a PRE pass in LLVM and its GVN pass documents its purpose as "to eliminate fully redundant instructions."  So it seems like the transformation we're looking for might be a suitable enhancement for that pass.

GVN does perform PRE as well. Though I believe it is only performed for phis, not selects (at least for scalar PRE, I think load PRE does handle select). Though to be honest I don't really understand why this particular transform would fall under the purview of PRE.


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

https://reviews.llvm.org/D123819