Failure to compile with intel compiler on linux

[blackwer]

Environment

toml++ version and/or commit hash:
toml.hpp v2.3.0 [9be51e4]

Compiler:
icpc (ICC) 19.1.0.166 20191121
icpc (ICC) 19.0.0.117 20180804

gcc backend: 7.4.0 (also tried 9.3.0)

C++ standard mode (e.g. 17, 20, 'latest'):
17

Target arch (e.g. x64):
x64 (linux)

Library configuration overrides:
None

Relevant compilation flags:
-std=c++17

Describe the bug

Compiling a simple test program with icpc fails, complaining about issues with TOML_ALWAYS_INLINE.
The same example works fine when compiling with gcc (the same version used as the backend for icpc).
Other versions of gcc on the backend similarly fail.

In file included from test.cpp(3):
% icpc test.cpp -std=c++17
toml.hpp(1119): error #77: this declaration has no storage class or type specifier
  	TOML_ALWAYS_INLINE
... [[hundreds of lines of errors]]

Steps to reproduce (or a small repro code sample)

#include <iostream>
#include <fstream> //required for parse_file()
#include "toml.hpp"

int main(int argc, char** argv)
{
    toml::table tbl;
    try
    {
        tbl = toml::parse_file(argv[1]);
        std::cout << tbl << "\n";
    }
    catch (const toml::parse_error& err)
    {
        std::cerr << "Parsing failed:\n" << err << "\n";
        return 1;
    }

    return 0;
}

Additional information

Forcing #define TOML_ALWAYS_INLINE __forceinline will compile with no errors, but fail to link.

/tmp/icpcJYElhL.o: In function `toml::v2::default_formatter<char>::print(toml::v2::table const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x50): undefined reference to `toml::v2::node::type() const'
... [[et al.]]

[marzer]

Ah. ICC strikes again.

Try forcing TOML_ALWAYS_INLINE as the regular inline keyword and see how you go.

[blackwer]

I can sympathize - icpc has caused me a lot of problems in the past, but the performance jump on this code is too big too ignore :(

#ifdef __INTEL_COMPILER
	#define TOML_ICC				__INTEL_COMPILER
        #define TOML_ALWAYS_INLINE inline
	#ifdef __ICL
		#define TOML_ICC_CL			TOML_ICC
	#else
		#define TOML_ICC_CL			0
	#endif
#else

% icpc test.cpp -std=c++17
/tmp/icpcsKMtVt.o: In function `toml::v2::default_formatter<char>::print(toml::v2::table const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x50): undefined reference to `toml::v2::node::type() const'
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x1a8): undefined reference to `toml::v2::node::type() const'
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x2c1): undefined reference to `toml::v2::node::type() const'
/tmp/icpcsKMtVt.o: In function `toml::v2::default_formatter<char>::print(toml::v2::array const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5arrayE[_ZN4toml2v217default_formatterIcE5printERKNS0_5arrayE]+0x3b2): undefined reference to `toml::v2::node::type() const'
/tmp/icpcsKMtVt.o: In function `toml::v2::default_formatter<char>::print_inline(toml::v2::table const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE12print_inlineERKNS0_5tableE[_ZN4toml2v217default_formatterIcE12print_inlineERKNS0_5tableE]+0x28e): undefined reference to `toml::v2::node::type() const'

[marzer]

Hmmmm, well, I'm out of ideas. I'll try to give this a look at some point over the weekend.

[marzer]

@blackwer OK so I just started looking into this and it turns out I need to jump through all of Intel's hoops to get their compiler on Linux too. Their website and documentation is a bunch of convoluted nonsense - is there a single download link or package repo you can point me to?

[blackwer]

I'm using a version supplied by my employer, so I haven't had to deal with their nonsense for a few years. I think this should be the thing you're looking for: https://software.intel.com/content/dam/develop/external/us/en/documents/iss-icc-download-install-cmdline-780679.pdf

Thanks so much for trying to deal with this. The intel compiler is a such a thorn

[bobfang1992]

Is this even an option?

https://github.com/jsosa/dkr-intel

[traversaro]

Perhaps this is useful: https://github.com/oneapi-src/oneapi-ci .

[blackwer]

It's maybe not ideal, but you can use godbolt. I've reproduced the (well, an) issue there as well.

https://godbolt.org/z/sEEacr

[marzer]

Thanks all. Things I've learned about ICC:

• It doesn't like base class constructor calls in constructor init-lists to use {}
• std::optional needs to be explicitly default constructed to be constexpr??
• 19.0.1 doesn't seem to understand deduction guides very well

I've pushed some minor fixes in 3db1e4e. Not promising it will fix the linker error you were seeing @blackwer since I haven't got an ICC test setup locally yet, but iterating on compiler explorer was a good start.

• 19.0.1 is 'fixed' as much as I can; now it bottoms out at an internal compiler error
• 21.1.8 seems to have some issue with the standard library's tuple
• 21.1.9 is ok

[blackwer]

@marzer Thanks! New version does resolve all of the compilation issues up to the linking step, but still gets the same undefined symbol errors. I haven't investigated too much yet, but it appears that icc is expecting some external linkage to toml::v2::node::type() const, while other compilers are not generating or expecting this symbol at all.

% g++ test.cpp -std=c++17 -c -o test-g++.o
% icpc test.cpp -std=c++17 -c -o test-icc.o
% nm -C test-g++.o | grep "node::type()"
% nm -C test-icc.o | grep "node::type()"   
                 U toml::v2::node::type() const
% icpc -o test test-g++.o
% icpc -o test test-icc.o
test-icc.o: In function `toml::v2::default_formatter<char>::print(toml::v2::table const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x50): undefined reference to `toml::v2::node::type() const'
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x1a8): undefined reference to `toml::v2::node::type() const'
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5tableE[_ZN4toml2v217default_formatterIcE5printERKNS0_5tableE]+0x2c1): undefined reference to `toml::v2::node::type() const'
test-icc.o: In function `toml::v2::default_formatter<char>::print(toml::v2::array const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE5printERKNS0_5arrayE[_ZN4toml2v217default_formatterIcE5printERKNS0_5arrayE]+0x3b2): undefined reference to `toml::v2::node::type() const'
test-icc.o: In function `toml::v2::default_formatter<char>::print_inline(toml::v2::table const&)':
test.cpp:(.text._ZN4toml2v217default_formatterIcE12print_inlineERKNS0_5tableE[_ZN4toml2v217default_formatterIcE12print_inlineERKNS0_5tableE]+0x28e): undefined reference to `toml::v2::node::type() const'

[marzer]

@blackwer Yeah I'm really not sure what to make of that. toml::node::type() is pure virtual...

[blackwer]

@marzer yeah I was poking through the code and noticed that just now too. This is completely bizarre. I can maybe look into getting someone from Intel in on this if we can't make progress -- though, if even possible, that's going to take a while in my experience.

Any luck getting the compiler? My understanding is that there is a free one month trial, though I understand if you don't want to deal with all that nonsense (or already have for the windows version, etc). If that's a problem, let me know and I'll see if I can find you some resources.

[marzer]

@blackwer Nope, I haven't revisited it yet. Had hoped to find time over the weekend but things got away from me. Might be able to squeeze it in some night this week, but who knows, if not it'll be a weekend effort again.

Worth noting that I do have the windows version already and have tested it with toml++ in the past and it seemed to work OK. For some definition of OK, that is; it compiled, linked and ran, only after I disabled a bunch of unicode-related code paths, but it at least built an executable. It was targeting the MSVC linker so the codegen and object format would have likely been quite different...

[blackwer]

Well, I've got a hilarious hack that seems to work, so there's that.

#include "include/toml.hpp"
toml::v2::node_type toml::node::type() const noexcept { return type(); };

[marzer]

What. The actual fuck.

[blackwer]

A colleague of mine (apataki) suspects that this is arising from how vtables are constructed in various C++ implementations.

Many C++ implementations connect class vtables to the first virtual function of a class - so I wonder if that's what the underlying issue is, that the vtable is not generated/linked for your class.

He discovered that simply replacing commands like

const auto type = v.type();

with

auto *pv = &v;
const auto type = pv->type();

resolves the issue (symbol for toml::v2::node::type() no longer exists in compiled objects).

This is certainly a strange compiler bug. We're looking at generating a more minimal example that doesn't involve a 12k line header file. Glorious though the header file is, it is difficult to isolate what's going on exactly :).

[marzer]

Glorious though the header file is, it is difficult to isolate what's going on exactly :).

Yah it's quite a tome. You can find the individual 'component' headers in include, might make it easier to navigate. 'public' ones are .h, 'internal' ones (i.e. the ones that would be cpp files if the library weren't header-only) are .hpp.

[blackwer]

I misspoke about vtables - that was just his initial hunch. It's something a bit weirder it seems. We'll update once that's 100% clarified.

Thanks for the tips on how the includes work. I'm not sure the best way to patch this, since it's a weird compiler bug work-around and requires patching many different calls to .type(). It would be nice to not have to manually introduce and immediately dereference a pointer just to compile.

My autoformatter and your tabs are also not getting along, though I could definitely figure that one out.

[marzer]

My autoformatter and your tabs are also not getting along, though I could definitely figure that one out.

One of us! One of us! One of us!

[blackwer]

Small update. We got the latest intel compiler packaged with 'parallel studio', and it doesn't resolve the issue. This is version 19.1.3.304.

To get the 21.1 version used in godbolt, you have to install oneapi, and then the HPC toolkit addon. There are then two c++ variants installed (icpx with base, icpc with HPC), with both compiling with no issue.

This oddly seems to work without any licensing, because... reasons? I don't think they'd be too happy if we installed it on our cluster though... we probably still need to come up with a reasonable workaround. For now I might just inline the 'implement something that should by definition not be implemented' hack until a more elegant fix is figured out, if such thing exists.

[marzer]

@blackwer I wonder if a good solution for that compiler would be to just be to make that method non-pure-virtual, and provide a a base implementation something like:

toml::v2::node_type toml::node::type() const noexcept
{
    assert(false); // or maybe some sort of unreachable() intrinsic if icc has one
    return {};
};

Mercifully this is something I can test on godbolt now that it has execution output.

[marzer]

Ok, no, it just blasts past the assert, returns the default-initialized node_type, and causes other weird issues. Fortunately your workaround is ok:

#if TOML_ICC && !TOML_ICC_CL

[[nodiscard]] virtual node_type type() const noexcept
{
	return type();
}

#else

[[nodiscard]] virtual node_type type() const noexcept = 0;

#endif

(https://godbolt.org/z/vGoaTG)

Though I do wonder if it's not just moving the problem to the next pure-virtual method in the class...

[marzer]

@blackwer I've added your hilarious workaround in b11f28a. It might come back to bite me, but we'll see. :D

[blackwer]

@blackwer I've added your hilarious workaround in b11f28a. It might come back to bite me, but we'll see. :D

Thanks! It does compile on a considerably more complex example now, though it's certainly not happy! Thousands of lines of complaints about missing return statements in non-void functions. I'll suppress the warnings for now and open another issue when I'm actually on the clock and can isolate what its freakin' problem is.

[marzer]

Oh, yeah, There's a few places where I use __builtin_unreachable() and friends where a switch statement covers all the possible inputs, etc. I vaguely recall ICC having a lot of trouble understanding that sort of thing.

[blackwer]

Oh, yeah, There's a few places where I use __builtin_unreachable() and friends where a switch statement covers all the possible inputs, etc. I vaguely recall ICC having a lot of trouble understanding that sort of thing.

Setting TOML_UNREACHABLE __builtin_unreachable() and then placing TOML_UNREACHABLE; after a bunch of if constexpr ... does resolve my issue. I didn't isolate the example yet though - just went through each line in my compile that complained and added the statement (as well as to a few random others).

[marzer]

@blackwer OH, yah, that's something else I recall it not handling well - function templates allowing for a 'null path' by virtue of if constexpr. Mind sharing your more complex example so I can have a look at and/or work through some of the output?

[blackwer]

It's HPC software that relies on a bunch of libraries that have to be installed with some extremely special care, so I will definitely have to isolate the code a bit unfortunately. I'll see what I can drum up and open an issue with it hopefully today.

[marzer]

Ah sure. You needn't worry about opening a seperate issue, this one can serve as a repository for any/all ICC issues you encounter (I reopened it with that in mind)

[marzer]

@blackwer How did you go with this?