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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 13 16:46:48 PDT 2018


efriedma added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1716
+  if (DL.getTypeAllocSize(CI->getType()) != sizeof(long int))
+    return nullptr;
+
----------------
efriedma wrote:
> xbolva00 wrote:
> > xbolva00 wrote:
> > > efriedma wrote:
> > > > xbolva00 wrote:
> > > > > efriedma wrote:
> > > > > > xbolva00 wrote:
> > > > > > > efriedma wrote:
> > > > > > > > `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?
> > > > > > >  But problem is ..
> > > > > > > 
> > > > > > > errs() << std::numeric_limits<long int>::max() << "\n";
> > > > > > > errs() << std::numeric_limits<long long int>::max() << "\n";
> > > > > > > 
> > > > > > > 9223372036854775807
> > > > > > > 9223372036854775807
> > > > > > > 
> > > > > > > So we need to rely on errno, I think
> > > > > > By "range check", I mean in addition to the errno check, if CI->getType() is a 32-bit integer, check that the result of strtoll fits into a 32-bit integer.
> > > > > Oh :D ok. Will fix.
> > > > The range check is still wrong.  You can try building 32-bit LLVM yourself by setting the CMake option LLVM_BUILD_32_BITS.
> > > So how to fix it?
> > I have no idea what I should do to fix it... :/
> I think the most readable solution is something like `int Bits = CI->getType()->getPrimitiveSizeInBits(); if (Result < llvm::minIntN(Bits) || Result > llvm::maxIntN(Bits)) return nullptr;`.
Actually, you can simplify that to just `if (!isIntN(CI->getType()->getPrimitiveSizeInBits(), Result)) return nullptr;`.


https://reviews.llvm.org/D45418





More information about the llvm-commits mailing list