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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1685
+
+  return ConstantInt::get(CI->getType(), atoi(Str.str().c_str()));
+}
----------------
xbolva00 wrote:
> efriedma wrote:
> > xbolva00 wrote:
> > > efriedma wrote:
> > > > I'm worried about host behavior leaking through here; specifically, the "undefined" values for overflow might vary. Could you use some function with more predictable behavior here?  (Maybe something based on llvm::consumeSignedInteger()?)
> > > We should call that function when we can call strtol directly?
> > I guess we can just call strtol, sure.
> So now is this patch good to go? 
You didn't actually change the code to call strtol instead of atoi.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1716
+  if (DL.getTypeAllocSize(CI->getType()) != sizeof(long int))
+    return nullptr;
+
----------------
`DL.getTypeAllocSize(CI->getType()) != sizeof(long int)` will cause your testcase to fail on 32-bit hosts (because we'll refuse the fold the call).

Can you use strtoll plus a range check, instead?


https://reviews.llvm.org/D45418