[PATCH] D88460: Strlen loop idiom recognition

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Phabricator doesn't track incremental patches the way you're uploading them; please upload the full diff against master.



================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1665
   Value *Expanded = Expander.expandCodeFor(
       LCSSAEv->getStart(),
       Builder.getInt8PtrTy(LoopLoad->getPointerAddressSpace()),
----------------
`LCSSAEv->getStart()` is the value of the output of the loop; the input to strlen needs to be the input to the load (`LoadEv->getStart()`).

More tests would be helpful to catch this sort of issue.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1671
+
+  auto *Base = dyn_cast<SCEVUnknown>(SE->getPointerBase(LCSSAEv));
+  if (!Base)
----------------
getPointerBase() isn't right; it strips all offsets from the pointer.

I'd like to see more regression test coverage of different variations on the values produced by the loop.


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

https://reviews.llvm.org/D88460