[PATCH] D45736: [SimplifyLibcalls] Replace locked IO with unlocked IO

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Sorry about the delay, I got busy with other things.



================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:486
+  if (T.isOSLinux()) {
+    // available IO unlocked variants on Linux
+    TLI.setAvailable(LibFunc_getc_unlocked);
----------------
You probably also need to check T.isAndroid(), like the code above this.  (I haven't actually checked what bionic provides, but I'm guessing it doesn't have all of these.)


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:135
+  if (!FOpen)
+    return false;
+
----------------
I think you also need to check if the call is marked nobuiltin.


================
Comment at: test/Transforms/InstCombine/unlocked-stdio.ll:4
+
+%struct._IO_FILE = type { i32, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, i8*, %struct._IO_marker*, %struct._IO_FILE*, i32, i32, i64, i16, i8, [1 x i8], i8*, i64, i8*, i8*, i8*, i8*, i64, i32, [20 x i8] }
+%struct._IO_marker = type { %struct._IO_marker*, %struct._IO_FILE*, i32 }
----------------
It would be nice to have at least some tests which call fclose (since otherwise you're leaking the file handle).

Needs some negative tests to make sure the capture checking is working correctly.


https://reviews.llvm.org/D45736