[PATCH] D129224: [InstCombine] Fold strtoul and strtoull and avoid PR #56293

[Martin Sebor via Phabricator via llvm-commits]

msebor created this revision.
msebor added reviewers: efriedma, xbolva00, nikic.
Herald added subscribers: jsji, pengfei, hiraditya.
Herald added a project: All.
msebor requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

PR #56293 <https://github.com/llvm/llvm-project/issues/56293> points out a minor portability issue with the `atoi`,`strtol`, and `strtoll` folders as a result of relying on a nonportable aspect of `strtoull` for empty sequences.   The issue is easy to avoid for just the limited subset of supported folders, but relying on `srtroull` turns out be problematic also due to overflow handling.

Rather than working around just the root cause of PR #56293 <https://github.com/llvm/llvm-project/issues/56293>, this patch replaces the use of `strtoull` with a hand-written conversion algorithm that doesn't suffer from either of these issues.  Besides resolving the problem in the PR it enhances the libcall simplifier to handle the rest of the standard integer conversion functions.

In the PR @efriedma suggests using `llvm::consumeSignedInteger`.  I ran into a couple of problems when trying that:

- The function recognizes other base prefixes besides `0` for octal and `0x` for hex (`0b` for binary and `0o` for octal), and these interfere with the rules of `strtol`.  The prefixes would need to be disabled for the function to be usable in this context (e.g., by adding another argument).
- Like the current `convertStrToNumber` helper, the function doesn't make it possible to reliably detect whether or not the parsed number is representable in the destination type (which may be narrower than `uint64_t`).  This too would require a change to the API to handle.

Rather than making intrusive changes to the existing "public" API I decided to instead modify the static helper function just for the libcall simplifier.

Besides the two problems noted above, `llvm::consumeSignedInteger` also doesn't strip leading space and succeeds even when the subject sequence isn't at the end of the string.  Unlike those above, both of these could be handled by the caller.

Tested by running `make check-all` on x86_64-linux.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129224

Files:
  llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/test/Transforms/InstCombine/str-int-4.ll
  llvm/test/Transforms/InstCombine/str-int-5.ll
  llvm/test/Transforms/InstCombine/str-int.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129224.442660.patch
Type: text/x-patch
Size: 37319 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220706/f9674cf9/attachment.bin>