Skip to content

Conversation

@georgthegreat
Copy link
Contributor

@georgthegreat georgthegreat commented Feb 7, 2024

libc++ declares lgamma_r as:

extern "C" double lgamma_r(double, int *);

while modern glibc uses the following:

__MATHCALL (lgamma,_r, (_Mdouble_, int *__signgamp));

This causes (I did not digged to the very bottom of it) the following compilation error when compiling with nvcc:

libcxx/include/__random/binomial_distribution.h(117): error: linkage specification is incompatible with previous "lgamma_r"
/usr/include/x86_64-linux-gnu/bits/mathcalls.h(271): here

Using original declaration solves the issue.

libc++ declares `lgamma_r` as:

```
extern "C" double lgamma_r(double, int *);
```

while modern glibc uses the following:

```
__MATHCALL (lgamma,_r, (_Mdouble_, int *__signgamp));
```

This causes (I did not digged to the very bottom of it) the following compilation error when compiling with nvcc:

```
libcxx/include/__random/binomial_distribution.h(117): error: linkage specification is incompatible with previous "lgamma_r"
/usr/include/x86_64-linux-gnu/bits/mathcalls.h(271): here
```
@georgthegreat georgthegreat requested a review from a team as a code owner February 7, 2024 11:54
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-libcxx

Author: Yuriy Chernyshov (georgthegreat)

Changes

libc++ declares lgamma_r as:

extern "C" double lgamma_r(double, int *);

while modern glibc uses the following:

__MATHCALL (lgamma,_r, (_Mdouble_, int *__signgamp));

This causes (I did not digged to the very bottom of it) the following compilation error when compiling with nvcc:

libcxx/include/__random/binomial_distribution.h(117): error: linkage specification is incompatible with previous "lgamma_r"
/usr/include/x86_64-linux-gnu/bits/mathcalls.h(271): here

Full diff: https://github.com/llvm/llvm-project/pull/80979.diff

1 Files Affected:

  • (modified) libcxx/include/__random/binomial_distribution.h (-4)
diff --git a/libcxx/include/__random/binomial_distribution.h b/libcxx/include/__random/binomial_distribution.h
index e8774bb8d67ee..54f321af63d50 100644
--- a/libcxx/include/__random/binomial_distribution.h
+++ b/libcxx/include/__random/binomial_distribution.h
@@ -97,10 +97,6 @@ class _LIBCPP_TEMPLATE_VIS binomial_distribution {
   }
 };
 
-#ifndef _LIBCPP_MSVCRT_LIKE
-extern "C" double lgamma_r(double, int*);
-#endif
-
 inline _LIBCPP_HIDE_FROM_ABI double __libcpp_lgamma(double __d) {
 #if defined(_LIBCPP_MSVCRT_LIKE)
   return lgamma(__d);

@georgthegreat
Copy link
Contributor Author

georgthegreat commented Feb 7, 2024

This error is triggered upon #including <random> after <cmath>.

#include <iostream>
#include <cmath>
#include <random>

int main() {
    int state = 0;

    long double x;
    std::cin >> x;
    auto val = lgamma_r(x, &state);

    std::cout << "Gamma(" << x << ") = " << val << std::endl;

    return 0;
}

@georgthegreat georgthegreat force-pushed the patch-1 branch 2 times, most recently from e1a3db8 to af4e771 Compare February 7, 2024 19:37
@github-actions
Copy link

github-actions bot commented Feb 7, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

robot-piglet pushed a commit to yandex/yatool that referenced this pull request Feb 8, 2024
This backports [PR #80979](llvm/llvm-project#80979) from llvm upstream.
robot-piglet pushed a commit to ytsaurus/ytsaurus that referenced this pull request Feb 8, 2024
This backports [PR #80979](llvm/llvm-project#80979) from llvm upstream.
robot-piglet pushed a commit to ydb-platform/ydb that referenced this pull request Feb 8, 2024
This backports [PR #80979](llvm/llvm-project#80979) from llvm upstream.
robot-piglet pushed a commit to catboost/catboost that referenced this pull request Feb 8, 2024
This backports [PR #80979](llvm/llvm-project#80979) from llvm upstream.
alexv-smirnov pushed a commit to alexv-smirnov/ydb that referenced this pull request Feb 9, 2024
Enjection pushed a commit to Enjection/ydb that referenced this pull request Feb 16, 2024
@ldionne
Copy link
Member

ldionne commented Mar 4, 2024

This fails on the Apple CI with:

/Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-macminivault-com/llvm-project/libcxx-ci/build/apple-system/arm64/include/c++/v1/__random/binomial_distribution.h:105:10: error: use of undeclared identifier 'lgamma_r'
  return lgamma_r(__d, &__sign);
         ^

@georgthegreat
Copy link
Contributor Author

We have this patch ported locally and it works just fine (though our CI tests against macos-version-min=11.0).
Maybe this specific CI turns _REENTRANT off somehow.

There is no cuda compiler under MacOS, so we may simply forward-declare it under __APPLE__

@ldionne
Copy link
Member

ldionne commented Mar 4, 2024

This seems to break when we use modules (see the CI job).

@georgthegreat
Copy link
Contributor Author

Unfortunately, I have no experience with modules and can not quite understand the reason for hte failure.
Could you, please, help me to debug this?

@ldionne
Copy link
Member

ldionne commented Jan 10, 2025

Could you rebase this on top of latest main so we can have a look at the modules issues if they still exist?

@georgthegreat
Copy link
Contributor Author

@ldionne, it turns out that this was fixed in 84ce230

I am closing this PR as there is no need in it anymore.

@georgthegreat georgthegreat deleted the patch-1 branch January 10, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants