[PATCH] D32839: SimplifyLibCalls: Optimize wcslen

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:1420
+      M.getModuleFlag("wchar_size")))
+    return cast<ConstantInt>(ShortWChar->getValue())->getZExtValue();
+  return getTargetWCharSize(Triple(M.getTargetTriple()));
----------------
efriedma wrote:
> We should probably have a verifier check to make sure these casts will succeed.
https://reviews.llvm.org/D32974


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:458
+             I < E; ++I) {
+          if (Index.Array->getElementAsInteger(I) == 0) {
+            NullTermIdx = I;
----------------
efriedma wrote:
> MatzeB wrote:
> > efriedma wrote:
> > > Could you use an iterator which returns the elements of the ConstantDataArray, instead of using explicit indexing everywhere?
> > There are no iterators defined for ConstantDataArray, did I miss something? (Defining them seems tricky to get right to support various datatypes and target/host data conversions).
> No, there aren't any existing iterators.
> 
> It doesn't seem that tricky to write, though.  It could just be a wrapper around getElementAsInteger; or if you want to get fancy, you could represent it as a pointer+width pair, I guess.  (The data is just an array of integers in host byte order.)
I tried this and it looks like this: https://reviews.llvm.org/P7994
39 extra lines added for the iterator foo and none of the actual uses gets simpler.
(If you now want to argue that I didn't use llvm::enumerate(), then I'd kindly ask you to present me an example of how the heck I can implement an interator compliant the forward iterator concept where operator* returns a reference! to the underlying object).
I worked on this stuff for >2 hours now and it is definitely "tricky to write" and not really necessary here.


Repository:
  rL LLVM

https://reviews.llvm.org/D32839