[PATCH] D45418: [SimplifyLibcalls] Atoi, strtol replacements

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'd like to see a second testcase which checks our transforms on `declare i32 @strtol(i8*, i8**, i32)` (which is the signature on 32-bit targets).



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:108
+  std::locale Loc;
+  return Loc.name() == "C";
+}
----------------
We can safely assume nobody is messing with the locale in the compiler itself.  The problem comes when someone uses setlocale on the target (in the compiled program); that's not something you can detect in general.

Fortunately, we can still handle the interesting cases, I think: if we assume all possible target locales are ASCII supersets, or something close (which we already assume in other places), then if strtoll successfully parses a number on the host (i.e. nptr != endptr), it will also successfully parse the same way on the target.  If the string starts with a non-ASCII character, we probably have to assume it could be a space in some locale.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1702
+
+  return ConstantInt::get(CI->getType(), atoi(Str.str().c_str()));
+}
----------------
Can you just return Result here, instead of calling atoi?


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1720
+        strtoll(Str.str().c_str(), nullptr, CInt->getSExtValue());
+    if (errno == ERANGE)
+      return nullptr;
----------------
strtoll can also fail with EINVAL, if the base is invalid.  But the exact behavior isn't specified in the C standard, so probably better to check the base ourselves, explicitly.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:2329
+    case LibFunc_strtol:
+      return optimizeStrtol(CI, Builder);
     case LibFunc_printf:
----------------
Can you also add cases here for atol, atoll, and strtoll?  (Your current implementations should just work.)


https://reviews.llvm.org/D45418