[Analysis] Only use _unlocked stdio functions on linux

[mstorsjo]

Summary

The existing comment said that the functions were available only on GNU/Linux (and on certain Android versions), but only checked T.isGNUEnvironment() which also is true on MinGW (for arch-windows-gnu triplets), which doesn't have such functions.

Existing checks in the initialize function in TargetLibraryInfo.cpp also use only T.isOSLinux() to check for glibc features.

This fixes use of stdio on MinGW.

[xbolva00]

Oh, thanks for fix. I associated gnu == linux, forgot on mingw.

[xbolva00]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

I would leave isLinux && isGNU...

[mstorsjo]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

Yeah, I thought about that as well - but the rest of this function just plainly does isOSLinux everywhere, even if comments talk about glibc specifically. But I can make it Linux && GNU here for extra clarity.

[srhines]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

isOSLinux is true for Android, isn't it? Doesn't that break the rest of the case for < API 28 on Android?

[xbolva00]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

Probably yes.

E.g. musl implements them as aliases to locked variants..
https://github.com/esmil/musl/blob/194f9cf93da8ae62491b7386edf481ea8565ae4e/src/stdio/fwrite.c#L37

[xbolva00]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

I think this is okay:

if ((T.isOSLinux() && T.isGNUEnvironment()) || (T.isAndroid() && !T.isAndroidVersionLT(28))) {

[mstorsjo]

lib/Analysis/TargetLibraryInfo.cpp

485 ↗

(On Diff #147251)

isOSLinux is true for Android, isn't it? Doesn't that break the rest of the case for < API 28 on Android?

Oh right, yes, we clearly should make it linux&&GNU then.

[xbolva00]

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

[xbolva00]

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

[mstorsjo]

In D47002#1102687, @xbolva00 wrote:

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

Yes, I saw that as well :-) However it doesn't seem to be available in the system DLL "msvcrt.dll" which mingw targets by default. (Nowadays one can target the more modern ucrtbase.dll by default as well, and there they'd be available.)

[mstorsjo]

In D47002#1102688, @mstorsjo wrote:

In D47002#1102687, @xbolva00 wrote:

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

Yes, I saw that as well :-) However it doesn't seem to be available in the system DLL "msvcrt.dll" which mingw targets by default. (Nowadays one can target the more modern ucrtbase.dll by default as well, and there they'd be available.)

... except that shouldn't be an issue; if we'd would want to use them, mingw-w64 could just provide wrappers that map down back to the normal locked ones, for msvcrt.dll. So nevermind about that.

So yes, one could take advantage of that if one would have extra effort to spend on it :-)

llvm/trunk/lib/Analysis/TargetLibraryInfo.cpp

if (!T.isOSLinux() || T.isAndroid()) {
     TLI.setUnavailable(LibFunc_pow_finite);
     TLI.setUnavailable(LibFunc_powf_finite);
     TLI.setUnavailable(LibFunc_powl_finite);
     TLI.setUnavailable(LibFunc_sinh_finite);
     TLI.setUnavailable(LibFunc_sinhf_finite);
     TLI.setUnavailable(LibFunc_sinhl_finite);
   }
 
-  if (T.isGNUEnvironment() || (T.isAndroid() && !T.isAndroidVersionLT(28))) {
+  if ((T.isOSLinux() && T.isGNUEnvironment()) ||
+      (T.isAndroid() && !T.isAndroidVersionLT(28))) {
     // available IO unlocked variants on GNU/Linux and Android P or later
     TLI.setAvailable(LibFunc_getc_unlocked);
     TLI.setAvailable(LibFunc_getchar_unlocked);
     TLI.setAvailable(LibFunc_putc_unlocked);
     TLI.setAvailable(LibFunc_putchar_unlocked);
     TLI.setAvailable(LibFunc_fputc_unlocked);
     TLI.setAvailable(LibFunc_fgetc_unlocked);
     TLI.setAvailable(LibFunc_fread_unlocked);

llvm/trunk/test/Transforms/InstCombine/unlocked-stdio-mingw.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S -mtriple=x86_64-w64-mingw32 | FileCheck %s
+
+%struct._iobuf = type { i8*, i32, i8*, i32, i32, i32, i32, i8* }
+
+@.str = private unnamed_addr constant [5 x i8] c"file\00", align 1
+@.str.1 = private unnamed_addr constant [2 x i8] c"w\00", align 1
+
+; Check that this still uses the plain fputc instead of fputc_unlocked
+; for MinGW targets.
+define void @external_fputc_test() {
+; CHECK-LABEL: @external_fputc_test(
+; CHECK-NEXT:    [[CALL:%.*]] = call %struct._iobuf* @fopen(i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0), i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str.1, i64 0, i64 0))
+; CHECK-NEXT:    [[CALL1:%.*]] = call i32 @fputc(i32 99, %struct._iobuf* [[CALL]])
+; CHECK-NEXT:    ret void
+;
+  %call = call %struct._iobuf* @fopen(i8* getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0), i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str.1, i64 0, i64 0))
+  %call1 = call i32 @fputc(i32 99, %struct._iobuf* %call)
+  ret void
+}
+
+declare %struct._iobuf* @fopen(i8*, i8*)
+declare i32 @fputc(i32, %struct._iobuf* nocapture)