[PATCH] D32839: SimplifyLibCalls: Optimize wcslen

[Matthias Braun via Phabricator via llvm-commits]

MatzeB marked 3 inline comments as done.
MatzeB added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:532
+  assert(IntTy->isIntegerTy());
+  unsigned CharSize = IntTy->getIntegerBitWidth();
+
----------------
efriedma wrote:
> MatzeB wrote:
> > efriedma wrote:
> > > Making assumptions about the implementation of wcslen() based on the type of the pointer is bad idea.  It probably works most of the time for code generated by clang, but pointee types don't have any semantics in IR, so it's likely to mess up at some point.  (Also, pointee types in IR are likely to go away at some point in the future.)
> > > 
> > > I think the right approach is to add a function to TargetLibraryInfo to compute the size of wchar_t based on the triple.
> > I assume the argument must match the declaration of wcslen which I assumed to be correct. I could grab the information directly from the declaration to avoid problems when the IR pointers don't have pointee-types anymore.
> > 
> > I'm not convinced the Triple/DataLayout has enough information to give you the type of `wchar_t`. There are for example the `-fshort-wchar` and `-fno-short-wchar` clang flags that do not affect the triple or datalayout.
> If we want to support compiling against a C library built with "-fshort-wchar" on a platform where wchar_t is 32 bits by default, the triple isn't sufficient, yes.  We can emit that information from clang; see http://llvm.org/docs/LangRef.html#c-type-width-module-flags-metadata .  (We currently only emit it on ARM, but we could emit it on other architectures.)
The updated version gets `wchar_t` by looking at the wcslen declaration. I think that is in line with other code here (the length of `size_t` is also determined by looking at the call result types / function declarations if you look around).

(I don't think adding support for determining the sizes of C/C++ types into TargetLibraryInfo is a worthwhile goal at this point.)


Repository:
  rL LLVM

https://reviews.llvm.org/D32839