[PATCH] D57625: [TargetLibraryInfo] Update run time support for Windows

Evandro Menezes via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 11 08:48:50 PST 2019


evandro marked 4 inline comments as done.
evandro added inline comments.


================
Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:264
+    if (!hasPartialSP || !hasPartialC99)
+      TLI.setUnavailable(LibFunc_logbf);
+    TLI.setUnavailable(LibFunc_nearbyintf);
----------------
mstorsjo wrote:
> evandro wrote:
> > mstorsjo wrote:
> > > I believe this one doesn't need the `!hasPartialSP` condition. `logbf` is available for all architectures in the version where we set `hasPartialC99`.
> > > 
> > > (Also, what does `SP` stand for in that variable name?)
> > > 
> > > Changing it into ìf (!hasPartialSP && !hasPartialC99)` should be fine though, and conversely `if (hasPartialSP || hasPartialC99) TLI.setAvailableWithName(LibFunc_logbf, "_logbf");` below.
> > `logbf` was available in amd64 even before C99 was supported.
> > 
> > `SP` stands for single precision, AKA, `float`.   I could rename it to `hasPartialFloat`.
> Yes,  `logbf` was available for x86_64 before, but wih this change, it only becomes available for the modern CRT. That is, I suggest changing `||` in the condition to `&&` here. But as the `_logbf` name only exists for x86_64/arm/aarch64, I would suggest this:
> 
> ```
> // Unavailable for i386 on older crt versions, available for i386 on modern crts and on all crt versions for other architectures.
> if (!hasPartialSP && !hasPartialC99)
>   TLI.setUnavailable(LibFunc_logbf);
> // On non-i386 versions, this function always exists, as _logbf.
> // Modern crt versions also provide it under the name logbf, for all archs.
> if (hasPartialSP)
>  TLI.setAvailableWithName(LibFunc_logbf, "_logbf");
> ```
I think that this gives me a plan of action: I'll commit this patch as is and commit another adding the remaining functions I find out after examining the information available in MinGW, keeping in mind that, to LLVM, MSVCRT and MinGW are different targets (specifically, MinGW using the GNU runtime).  This way, should any problem arise later, it should be easy to narrow it down and fix it.

Thank you so very much.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57625/new/

https://reviews.llvm.org/D57625





More information about the llvm-commits mailing list