[PATCH] D109289: [InstCombine] snprintf(NULL, 0, "%s", str) to strlen(str)

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:2597
+        if (auto *V = emitStrLen(CI->getArgOperand(3), B, DL, TLI)) {
+          if (CI->getType() != V->getType())
+            V = B.CreateTrunc(V, CI->getType());
----------------
bjope wrote:
> bjope wrote:
> > xbolva00 wrote:
> > > snprintf returns int, strlen returns size_t so...  trunc should be fine?
> > Isn't the transform only valid if the number of bits in CI is greater than the number of bits in V?
> > I.e. the unsigned value returned by strlen must fit into the non-negative part of the signed destination type. And given that constraint we always need the trunc.
> Well, given that constraint we always need a zext, not trunc.
If we consider the case when the value returned by strlen as size_t would be out-of-range for the non-negative range of int, then I guess snprintf should return a negative value? Or is the result undefined (I haven't seen any documentation saying anything about such cases for snprintf, while for strlen it says that the result in undefined if no null-termination is found)?

Either way it would be ok to do the transformation when sizeof(size_t)==sizeof(int), because that would give a negative value if the strlen result is out-of-range for the int.

But if sizeof(size_t)>sizeof(int), then maybe we need to saturate the value instead of doing a trunc, just to be safe?



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

https://reviews.llvm.org/D109289