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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1683
+  if (!getConstantStringInfo(CI->getArgOperand(0), Str))
+    return nullptr;
+
----------------
Both of these `getConstantStringInfo() == false` are also not covered with tests, i think?


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:1696
+
+  if (ConstantInt *CInt = dyn_cast<ConstantInt>(CI->getArgOperand(2)))
+    return ConstantInt::get(CI->getType(), strtol(Str.str().c_str(), nullptr,
----------------
You also probably want a negative test with this third operand being not a constant (an argument of the test function)


================
Comment at: test/Transforms/InstCombine/str-int-2.ll:4
+
+ at .str = private unnamed_addr constant [11 x i8] c"2147483648\00", align 1
+
----------------
But this is just `1<<31`, it fits into `i32` as-is.
Maybe `4294967296` (`1<<32`) ?


================
Comment at: test/Transforms/InstCombine/str-int-2.ll:6
+
+declare i64 @strtol(i8*, i8**, i32) #1
+
----------------
I'm not sure why you need to separate it into two test files?
Couldn't `strtol` in `str-int.ll` simply return `i64`?


https://reviews.llvm.org/D45418