Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prefix symbols with _ for 32-bit x86 Windows #238

Merged
merged 2 commits into from
Sep 8, 2021
Merged

prefix symbols with _ for 32-bit x86 Windows #238

merged 2 commits into from
Sep 8, 2021

Conversation

jeremyd2019
Copy link
Contributor

In a case that I believe can only be hit for Clang i686-*-windows-gnu (AKA MinGW), symbols in asm need to be prefixed with _. Fixes #237

In a case that I believe can only be hit for Clang i686-*-windows-gnu (AKA MinGW), symbols in asm need to be prefixed with `_`.  Fixes #237
@jeremyd2019
Copy link
Contributor Author

I don't know about the non-__STDC__ case, or if it's even reachable for Clang i686-w64-windows-gnu

@jeremyd2019 jeremyd2019 marked this pull request as ready for review September 7, 2021 18:25
@ViralBShah
Copy link
Member

@staticfloat Would you be able to take a look at this?

Also, it would be so much easier to merge these if we had CI working through gh actions.

src/cdefs-compat.h Outdated Show resolved Hide resolved
@staticfloat
Copy link
Contributor

I talked it over with @vtjnash and we both agree that supporting non-__STDC__ stuff is a waste of time; very unlikely that anyone is trying to build openlibm with such an archaic C compiler. Feel free to ignore that, and we will remove the branches in a future commit.

Co-authored-by: Elliot Saba <staticfloat@gmail.com>
@ViralBShah ViralBShah merged commit 3b9454f into JuliaMath:master Sep 8, 2021
@ViralBShah
Copy link
Member

This one didn't build on my mac - so I had to revert it.

src/s_isinf.c:66:1: error: expected ')'
openlibm_weak_reference(__isinff, isinff);
^
/Users/viral/repos/openlibm/src/cdefs-compat.h:69:32: note: expanded from macro 'openlibm_weak_reference'
    __asm__(".weak_reference " openlibm_symbol_prefix #alias); \
                               ^
src/s_isinf.c:66:1: note: to match this '('
/Users/viral/repos/openlibm/src/cdefs-compat.h:69:12: note: expanded from macro 'openlibm_weak_reference'
    __asm__(".weak_reference " openlibm_symbol_prefix #alias); \
           ^
src/s_isinf.c:66:1: error: expected ')'
openlibm_weak_reference(__isinff, isinff);
^
/Users/viral/repos/openlibm/src/cdefs-compat.h:70:21: note: expanded from macro 'openlibm_weak_reference'
    __asm__(".set " openlibm_symbol_prefix #alias ", " openlibm_symbol_prefix #sym)
                    ^
src/s_isinf.c:66:1: note: to match this '('
/Users/viral/repos/openlibm/src/cdefs-compat.h:70:12: note: expanded from macro 'openlibm_weak_reference'
    __asm__(".set " openlibm_symbol_prefix #alias ", " openlibm_symbol_prefix #sym)
           ^
2 errors generated.
make: *** [src/s_isinf.c.o] Error 1

@jeremyd2019
Copy link
Contributor Author

Yeah, it appears that #undefing the symbol prefix does not work (I was worried about that, and my test only succeeded because of the typo).

@ViralBShah
Copy link
Member

May need a new PR - and while I now have CI running, I don't think I am able to set up the windows CI correctly for x86.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang i686-w64-windows-gnu weak reference mangling
3 participants