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

Fixing problems with the Intel compiler using a GCC 4.4 std library #1477

Merged
merged 6 commits into from Apr 26, 2015

Conversation

sithhell
Copy link
Member

This PR solves a problem with using the intel compiler series with a standard library provided by gcc 4.4.x standard libraries.

@K-ballo
Copy link
Member

K-ballo commented Apr 24, 2015

This is surprising, the problems fixed here happen when using GCC4.6 as a standard library, at least according my reading of the logs here. Either this change doesn't do anything, or somehow Intel ends up using 4.6 standard library sources while pretending to be 4.4 compiler. That's a huge gap, for instance it means "0.9 rvalue references" (with different implicit special member rules for instance) instead of the real thing, it could easily explain the problems we are seeing. Basically what it means is that we are still supporting GCC4.4, the question is how has this worked so far?

@sithhell
Copy link
Member Author

@K-ballo: I think the problem at hand is slightly more complicated. Our intel 13 builder indeed uses the stdlib from 4.6. The PR description is wrong then. We see two failures:

  1. Intel 13 + gcc 4.6 stdlib isn't able to properly generate the code needed for moving the std::pair (for whatever reason)
  2. Intel 14 + gcc 4.4. stdlib experiences the same problem.

I am not entirely sure why this is happening. That doesn't mean we still support gcc 4.4, we don't support the compiler frontend. We merely have to support its stdlib. Unfortunately most systems only come equipped with gcc 4.4 plus some version of the intel compiler. The intel compiler itself supports all the necessary C++11 features we need though.

@sithhell
Copy link
Member Author

It has worked so far because we already implemented a few workaround in similar situations. The use of std::unique_ptr in the core library was only introduced with our new serialization layer. As such we never noticed those problems before (except with hpx::util::serialization_buffer<T> where we applied a different workaround).

@hkaiser
Copy link
Member

hkaiser commented Apr 24, 2015

LGTM

@K-ballo
Copy link
Member

K-ballo commented Apr 25, 2015

We should figure out what this means for us, other than no movable-only types on maps.

@hkaiser
Copy link
Member

hkaiser commented Apr 25, 2015

@Finomnis: could you please add/change an Intel V13 builder on buildbot to use stdlibc++ 4.4? This would reproduce the issue and would avoid running into similar issues in the future.

@K-ballo
Copy link
Member

K-ballo commented Apr 25, 2015

@Finomnis: could you please add/change an Intel V13 builder on buildbot to use stdlibc++ 4.4? This would reproduce the issue and would avoid running into similar issues in the future.

It does not look like that would reproduce the issue. See 4.4 vs 4.6, they operate under different language rules and each one is correct for the compiler that they target. What we would need is the cross product of all Intel versions we are willing to support with all the "compatibility gcc modes" we are willing to support.

@hkaiser
Copy link
Member

hkaiser commented Apr 25, 2015

What we would need is the cross product of all Intel versions we are willing to support with all the "compatibility gcc modes" we are willing to support.

I'd suggest to support the most commonly used combinations, one for each compiler version. That also implies that we should add build system support to detect those combinations while rejecting all others (at least issuing a warning).

@sithhell
Copy link
Member Author

This PR is removing a compilation error for a particular set of compiler +
stdlib combination. Nothing more. All the other things discussed here are
quite orthogonal.

@@ -16,13 +16,18 @@ namespace hpx { namespace serialization
namespace detail
{
struct ptr_helper;
#if defined(HPX_INTEL_VERSION) && ((__GNUC__ == 4 && __GNUC_MINOR__ == 4) || HPX_INTEL_VERSION < 1400)
typedef boost::shared_ptr<ptr_helper> ptr_helper_ptr;
Copy link
Member

Choose a reason for hiding this comment

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

Missing include for boost::shared_ptr usage here

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed.

@sithhell
Copy link
Member Author

On Saturday, April 25, 2015 08:38:12 Agustín Bergé wrote:

@Finomnis: could you please add/change an Intel V13 builder on buildbot to
use stdlibc++ 4.4? This would reproduce the issue and would avoid running
into similar issues in the future.
It does not look like that would reproduce the issue. See
[4.4](https://github.com/gcc-mirror/gcc/blob/gcc-4_4_0-release/libstdc%2B%2
B-v3/include/bits/stl_pair.h#L92) vs
[4.6](https://github.com/gcc-mirror/gcc/blob/gcc-4_6_0-release/libstdc%2B%2
B-v3/include/bits/stl_pair.h#L115), they operate under different language
rules and each one is correct for the compiler that they target. What we
would need is the cross product of all Intel versions we are willing to
support with all the "compatibility gcc modes" we are willing to support.

Yes, that would be the best thing to do. I think we can phase out intel13
compiler support soon. AFAICT, most clusters have at least intel14 available
and are switching to default to intel V15.

sithhell added a commit that referenced this pull request Apr 26, 2015
Fixing problems with the Intel compiler using a GCC 4.4 std library
@sithhell sithhell merged commit 6b92adc into master Apr 26, 2015
@sithhell sithhell deleted the fixing_intel_compiler branch April 26, 2015 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants