Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

MSVC: Provide alternate implementations for 32-bit math functions #1446

Merged
merged 1 commit into from
Dec 2, 2015

Conversation

kinke
Copy link
Contributor

@kinke kinke commented Nov 29, 2015

The x86 runtime lacks most *f versions (asinf(), acosf() etc.), i.e., they are only inline-declared in the C headers (and use the 64-bit double version).
The available symbols apparently depend upon whether one's linking against the static or dynamic MS runtime (libcmt.lib vs. msvcrt.lib), and possibly upon the Visual Studio version too.

So provide alternate implementations here in druntime to prevent linker errors. I came up with these 19 missing functions about a year ago by inspecting the VS 2013 headers (these should be all inline-declared ones, iirc).

Pinging @rainers.

@@ -6,10 +6,25 @@
* License: Distributed under the
* $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost Software License 1.0).
* (See accompanying file LICENSE)
* Source: $(DRUNTIMESRC rt/_stdio_msvc.d)
* Source: $(DRUNTIMESRC rt/_msvc.c)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this leading underscore is required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No documentation is generated from the rt package, so it's unlikely to make a difference. It's used everywhere so better keep it.

// Provide alternate implementations using the 64-bit double version.
#if defined _M_IX86

#define IMPL(baseName) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend to use more descriptive names for these macros, e.g. IMPL_ALT_MATHFNF.

@rainers
Copy link
Member

rainers commented Nov 30, 2015

I tried this change with VS2013, and the size of "hello.exe" using printf increased from 239kB to 313kB.
Maybe we can put the alternate names into msvc.c, but the actual implementations into another module. Or just ensure these are generated into COMDATs?

@kinke
Copy link
Contributor Author

kinke commented Dec 1, 2015

increased from 239kB to 313kB

Incredible. I completely forgot how primitive linkers still work in 2015 ;) - but /OPT:REF should be able to trim it down a bit. Anyway, the math implementations are now in their own module.

@kinke kinke force-pushed the msvcFloat branch 3 times, most recently from 2f72c69 to 98813c7 Compare December 1, 2015 00:42
The 32-bit x86 runtime lacks most single-precision math functions
(asinf(), acosf() etc.), i.e., they are only inline-declared in the
C headers (and use the double-precision version).
The available symbols apparently depend upon whether one's linking
against the static or dynamic MS runtime (libcmt.lib vs. msvcrt.lib),
and possibly upon the Visual Studio version too.

So provide alternate implementations here in druntime to prevent
linker errors. I came up with these 19 missing functions about a year
ago by inspecting the VS 2013 headers (these should be all inline-
declared ones, iirc).
@rainers
Copy link
Member

rainers commented Dec 1, 2015

Incredible. I completely forgot how primitive linkers still work in 2015 ;) - but /OPT:REF should be able to trim it down a bit.

My results were with the default, which seems /OPT:REF as long as /DEBUG is not passed, too. with /OPT:NOREF the change is from 346kB to 423kB.

Anyway, the math implementations are now in their own module.

Better, but still not optimal, as you get the full additional code when referring to one symbol.

I tried compiling msvc_math.c with -Gy to put each function in its own COMDAT, but that did not change much: only the used _msvc_... function is found in the map file, but all math functions referred from the module, too. This does not happen if I refer to the double versions.

@rainers
Copy link
Member

rainers commented Dec 1, 2015

BTW: if the single precision functions are now available for all VS versions, we should add them to core.stdc.math, too.

* License: Distributed under the
* $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost Software License 1.0).
* (See accompanying file LICENSE)
* Source: $(DRUNTIMESRC rt/_msvc_math.c)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the module names would be more consistent with sections_*.d and deh_*.d if this module is called math_msvc.d.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But there are no other math modules, contrary to sections and deh. To me, this extra module is part of msvc.c and only a separate file to help the linker somewhat.

@kinke
Copy link
Contributor Author

kinke commented Dec 1, 2015

BTW: if the single precision functions are now available for all VS versions, we should add them to core.stdc.math, too.

They already are: https://github.com/D-Programming-Language/druntime/blob/master/src/core/stdc/math.d#L987

Better, but still not optimal, as you get the full additional code when referring to one symbol.

Sufficient in my eyes. ;)

This does not happen if I refer to the double versions.

I guess the double versions are each in their own module.

@rainers
Copy link
Member

rainers commented Dec 2, 2015

They already are: https://github.com/D-Programming-Language/druntime/blob/master/src/core/stdc/math.d#L987

Ah, I forgot about that.

Better, but still not optimal, as you get the full additional code when referring to one symbol.

Sufficient in my eyes. ;)

Yeah, not so bad in this case. I wonder why putting functions into COMDATs doesn't work. It's the approach used by dmd aswell for all functions.

This does not happen if I refer to the double versions.

I guess the double versions are each in their own module.

Unfortunately there is no source code for these, but the library dump suggests that the guess is correct. For example ceil is implemented in an asm file with nothing else in there.

@rainers
Copy link
Member

rainers commented Dec 2, 2015

Auto-merge toggled on

rainers added a commit that referenced this pull request Dec 2, 2015
MSVC: Provide alternate implementations for 32-bit math functions
@rainers rainers merged commit 6f6ec7f into dlang:master Dec 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants