[PATCH] D123631: [InstCombine] Fold memrchr calls with sequences of identical bytes.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D123631#3475651 <https://reviews.llvm.org/D123631#3475651>, @msebor wrote:

> I wonder if the error has something to do with merging the patch from D123628 <https://reviews.llvm.org/D123628> into the next one in the series (D123629 <https://reviews.llvm.org/D123629>) in the final commit.  Should I abandon the former?

Yes, that's likely. Pre-merge checks apply dependencies of the patch first, though I'm not sure how exactly that works if some of them are closed. In either case, you can abandon the patch as it is no longer relevant.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:962
+  // Truncate the string to search at most EndOff characters.
+  Str = Str.substr(0, EndOff);
+  if (Str.find_first_not_of(Str[0]) != StringRef::npos)
----------------
Use the helper function that avoids the 32-bit truncation issue?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:970
+  // AKA
+  //   *S == C ? S + N - 1 : null.
+  Type *SizeTy = Size->getType();
----------------
I don't get the AKA part of this comment.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:977
+  Value *CEqS0 = B.CreateICmpEQ(ConstantInt::get(Int8Ty, Str[0]), CharVal);
+  Value *And = B.CreateAnd(NNeZ, CEqS0);
+  Value *SizeM1 = B.CreateSub(Size, ConstantInt::get(SizeTy, 1));
----------------
I think this should be using CreateLogicalAnd, in case `CharVal` is poison (unless we want to argue that this memrchr argument cannot be poison, even for a zero bound).


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

https://reviews.llvm.org/D123631