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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: test/Transforms/InstCombine/str-int.ll:6
+
+define i32 @strtol_1() #0 {
+; CHECK-LABEL: @strtol_1(
----------------
`@strtol_dec`?


================
Comment at: test/Transforms/InstCombine/str-int.ll:10
+;
+  %call = call i64 @strtol(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str, i32 0, i32 0), i8** null, i32 10) #3
+  %conv = trunc i64 %call to i32
----------------
Needs negative test with `endptr` not being `null`.


================
Comment at: test/Transforms/InstCombine/str-int.ll:11
+  %call = call i64 @strtol(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str, i32 0, i32 0), i8** null, i32 10) #3
+  %conv = trunc i64 %call to i32
+  ret i32 %conv
----------------
Do we care about the return type here?
I'd think you can simplify the tests and drop `trunc`.


================
Comment at: test/Transforms/InstCombine/str-int.ll:16
+; Function Attrs: nounwind
+declare i64 @strtol(i8*, i8**, i32) #1
+
----------------
I'd like to see one test with `base` = 0.


================
Comment at: test/Transforms/InstCombine/str-int.ll:18
+
+; Function Attrs: noinline nounwind uwtable
+define i32 @strtol_2() #0 {
----------------
I'm not sure these `; Function Attrs` comments are needed.


================
Comment at: test/Transforms/InstCombine/str-int.ll:19
+; Function Attrs: noinline nounwind uwtable
+define i32 @strtol_2() #0 {
+; CHECK-LABEL: @strtol_2(
----------------
`@strtol_hex`?


https://reviews.llvm.org/D45418