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

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:115
+
+  // Set Max to the maximum positive representable value in the type.
+  Type *RetTy = CI->getType();
----------------
This comment is not true for the case when AsSigned and Negate are both true. I.e. 0x80 is not the maximum possible representable value in i8 type. Something like "maximum possible value of the magnitude of a number which could be encoded" might be better.
Excuse my English


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:119
+  uint64_t Max = AsSigned && Negate ? 1 : 0;
+  Max += AsSigned ? maxIntN(NBits) : maxUIntN(NBits);
+
----------------
Honestly, I'd prefer more verbose code to this bit manipulation (it took me two minutes to figure out that the added 1 deals with two's complement representation). E.g.
```
if (AsSigned)
  Max = Negate ? -(uint64_t)minIntN(NBits) : maxIntN(NBits)
else
  Max = maxUIntN(NBits);
```
or, if you prefer one-liners:
`Max = AsSigned ? Negate ? -(uint64_t)minIntN(NBits) : maxIntN(NBits) : maxUIntN(NBits)`



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

https://reviews.llvm.org/D129224