[PATCH] D32839: SimplifyLibCalls: Optimize wcslen

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

clang only emits wchar_size on ARM at the moment; that might need to change before this gets merged.

Missing tests for 16-bit wchar_t.



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:458
+             I < E; ++I) {
+          if (Index.Array->getElementAsInteger(I) == 0) {
+            NullTermIdx = I;
----------------
MatzeB wrote:
> 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.
Mmm... okay, maybe not as simple as I'd hoped; fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D32839