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

Adding Intel oneAPI support #9522

Merged
merged 41 commits into from Oct 1, 2021

Conversation

franramirez688
Copy link
Contributor

@franramirez688 franramirez688 commented Sep 2, 2021

Changelog: Feature: New "compiler" intel-cc with different modes (icx, dpcpp, classic) for Intel oneAPI.
Changelog: Feature: Add Intel oneAPI support for CMakeToolChain, MSBuildToolchain and VCVars.
Docs: conan-io/docs#2233

Closes: #9304 #9026

  • Added new "compiler" intel-cc with different modes (icx, dpcpp, classic):
    * icx == Intel oneAPI C++ Compiler (icx/icpx)
    * dpcpp == Intel oneAPI DPC++ Compiler (dpcpp)
    * classic == Intel C++ Compiler Classic (icc/icpc for Linux/macOS and icl for Windows)
  • Added as part of the new conan.tools flow:
    * Using: CMakeToolChain, MSBuildToolchain or VCVars
  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

conan/tools/intel/oneapi.py Outdated Show resolved Hide resolved
conan/tools/intel/oneapi.py Outdated Show resolved Hide resolved
conan/tools/intel/oneapi.py Outdated Show resolved Hide resolved
conan/tools/microsoft/visual.py Outdated Show resolved Hide resolved
conans/client/build/cmake.py Outdated Show resolved Hide resolved
conans/client/migrations_settings.py Outdated Show resolved Hide resolved
conan/tools/intel/oneapi.py Outdated Show resolved Hide resolved
@franramirez688 franramirez688 force-pushed the feature/intel_oneapi branch 2 times, most recently from fbcf384 to 75c32dd Compare September 15, 2021 16:34
@SSE4
Copy link
Contributor

SSE4 commented Sep 16, 2021

what about compatibility with GCC/VS (compiler.base)? it was an important feature before to allow mix of existing GCC binaries with Intel.

@@ -2413,6 +2414,12 @@
<<: *visual_studio
apple-clang:
<<: *apple_clang
intel-cc:
Copy link
Contributor

Choose a reason for hiding this comment

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

in general, I'd love to see an explanation/motivation on these scheme and naming
e.g., why classic flavor is not just a new version of the existing intel compiler?
why sub-setting fits better than several new compilers?
also, what about intel-clang as new compilers seems to be clang based? do they share many things in common?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's go step by step and discuss the different points:

  • The naming, for sure, we can think of another one that can fit better. This name is easy to read/write so, why not?
  • Why different modes rather than different compilers? According to this Intel reference:
The following are the guiding principles for ICX:

* ICX and ICC Classic use different compiler drivers. The ICC Classic drivers are icc, icpc, and icl. The ICX drivers are icx and icpx. Use icx to compile and link C programs, and icpx for C++ programs. 
Unlike the icc driver, icx does not use the file extension to determine whether to compile as C or C+. Users must invoke icpx to compile C+ files. In addition to providing a core C++ Compiler, ICX is the base compiler for the Intel® oneAPI Data Parallel C++ Compiler and its new driver, dpcpp.
* For Data Parallel C++ applications: The underlying compiler is ICX and the driver is dpcpp. 
* For Standard C++ applications: Intel recommends using the ICC Classic production compiler and the icc/icpc/icl driver.

All the compilers are more or less related between them and they are different for sure, but we can keep it simpler by changing only the mode instead of declaring 3 different ones with the same flags, installation paths, etc.

  • They are indeed LLVM-based and, they admit the Clang flags but, in my opinion, all the code related to compilers based on another one is a bit convoluted so I think it's better to avoid it.

I hope it's clearer now. By the way, @SSE4 @memsharded please, share your thoughts here and let me know what you think about this new approach, it would be great 😁

conan/tools/intel/intel.py Outdated Show resolved Hide resolved
conan/tools/intel/intel.py Outdated Show resolved Hide resolved
"17": v17, "gnu17": vgnu17,
"20": v20, "gnu20": vgnu20,
"23": v23, "gnu23": vgnu23}.get(cppstd, None)
return "-std=%s" % flag if flag else None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now and as far as I know, there are no limitations to use any of these flags for the new Intel oneAPI C++/DPC++/Classis compilers

@SSE4
Copy link
Contributor

SSE4 commented Sep 23, 2021

I think there are some points still missing:

  • what about compiler detection?
  • what about compatibility with GCC/Visual Studio (package_id)?

@franramirez688
Copy link
Contributor Author

franramirez688 commented Sep 23, 2021

I think there are some points still missing:

  • what about compiler detection?

Yes, this is another step that I have to add for sure.

  • what about compatibility with GCC/Visual Studio (package_id)?

Do we need it? I mean, we're creating this new one to simplify the complexity added by the compiler.base implementation, aren't we? I guess we're losing this compatibility but the thing is, are there important reasons to maintain that compatibility if the Intel oneAPI has its compilers regardless of what they are based on (Clang or whatever)?

@franramirez688
Copy link
Contributor Author

More clarifications about this PR. I'm adding this intel-cc new compiler to the new conan.tools mechanism so I'm not changing anything regarding the legacy part (cmake, compiler flags, etc).

@franramirez688
Copy link
Contributor Author

I think there are some points still missing:

  • what about compiler detection?

@SSE4 @memsharded
I've been looking into it and, in my humble opinion, I think it's not worth implementing it for now.
Auto-detection is based on checking the output and having that compiler already added into the PATH to execute some kind of command to get the compiler version from there, so, for this case, you'll even need to have run the setvars.bat|sh.
I think it's better to let the users define the CC and CXX instead for every case regardless of the mode selected, e.g., if compiler.mode=icx you should define into your profile something like:

...
compiler=intel-cc
compiler.version=2021.3
compiler.mode=icx
...
[env]
CC=icx
CXX=icx

By the way, would it be possible to define a mechanism to write a default CC|CXX depending on the mode instead? what do you think?

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

This is looking very good.

I think I would add at least 1 full functional test, with the pytest.mark.tool_icc or equivalent, so it doesn't run in CI yet, but at least we have a full test that compiles something with icc.

conan/tools/intel/inteloneapi.py Outdated Show resolved Hide resolved
conan/tools/intel/inteloneapi.py Outdated Show resolved Hide resolved
conan/tools/intel/inteloneapi.py Outdated Show resolved Hide resolved
conan/tools/intel/inteloneapi.py Outdated Show resolved Hide resolved
conans/client/conf/__init__.py Outdated Show resolved Hide resolved
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Looking great, almost there!

conan/tools/microsoft/visual.py Outdated Show resolved Hide resolved
conans/test/conftest.py Outdated Show resolved Hide resolved
@memsharded memsharded added this to the 1.41 milestone Sep 30, 2021
conan/tools/intel/intel_cc.py Outdated Show resolved Hide resolved
Co-authored-by: Luis Martinez de Bartolome Izquierdo <lasote@gmail.com>
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.

[feature] Support for intel OneApi classic compilers
4 participants