[PATCH] D123629: [InstCombine] Fold memrchr calls with a constant character.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:922
+                                   "memrchr.ptr_plus");
+      return B.CreateSelect(Cmp, NullPtr, SrcPlus, "memrchr.sel");
+    }
----------------
nikic wrote:
> I don't think this is correct. Consider an unknown size and a string with two occurrences of C in it. Now assume that at runtime, the size is chosen to lie between the two occurrences of C. I believe your implementation will return null for that case, while it should return the first occurrence.
Gah, I just realized that this code is only used for a known length, unlike the corresponding memchr() implementation. In this case EndOff will be the exact end offset, and not just an upper bound on it, and this code will always constant fold, so it's fine.

I think it would be clearer that this is a fully static if this directly emitted the appropriate result based on `LenC->getValue().ule(Pos)` rather than going through constant folding.


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

https://reviews.llvm.org/D123629