[PATCH] D83392: Strlen loop idiom recognition

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I don't like the way the pattern-matching is written here; analyzing the exact instruction patterns is complicated, error-prone, and likely to miss cases.

I think I'd structure the analysis like this:

1. Is the latch condition of the form "%loadedval = load i8, i8* %p; icmp eq i8 %loadedval, 0"
2. Query SCEV to check %p is an AddRec with step 1.
3. Query SCEV to check all the LCSSA PHI nodes are AddRecs.
4. Check that the loop doesn't contain any operations with side-effects.

Then to apply the transform, you take the SCEV for the LCSSA PHI node, transform the AddRec to an Add (for example, `{n,+,1}` to `n + strlen(p)`), then use SCEVExpander to expand it.

Leveraging SCEV like this will make the transform a lot more flexible and easier to read.



================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1606
+  if (LoopBody->size() > MaxLoopSize)
+    return false;
+
----------------
"BasicBlock::size()" should never be used as a threshold; it's sensitive to debug info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83392