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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/TargetLibraryInfo.cpp:328
+    TLI.setUnavailable(LibFunc_fputs_unlocked);
+    TLI.setUnavailable(LibFunc_fgets_unlocked);
     TLI.setUnavailable(LibFunc_getitimer);
----------------
We probably want to whitelist which operating systems these are available on, rather than blacklist; most of these aren't part of any standard.

And actually, thinking about it a bit more, the "non-standard" part has other problems; a user could define their own function named fputc_unlocked, which does something different from the GNU functions.  (Nothing else in the current BuildLibCalls.h hits this particular problem because those are all reserved names.) But I'm not sure if that's likely to be a problem in practice.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:126
+  if (!CI->getFunction()->hasInternalLinkage())
+    return false;
+
----------------
This check doesn't buy you anything.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:135
+    }
+  }
+
----------------
This is incomplete; you also need to check that the instruction which produces the FILE* is a call to fopen, or the capture analysis won't do anything useful. Consider:

FILE* foo() { return stdout; }
void bar() { fputc('x', foo()); }

Also, iterating over the operands like this is weird; probably want to explicitly pass in which operand is the FILE*.


https://reviews.llvm.org/D45736