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

Update CMakeLists.txt #124

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

fluidnumerics-joe
Copy link
Contributor

Add conditional for intel compilers; don't use the -std= flag for enforcing fortran standard interpretation

This PR is meant to address #123

Add conditional for intel compilers; don't use the -std= flag for enforcing fortran standard interpretation
Add default flags for intel compiler
Copy link
Collaborator

@cgmb cgmb left a comment

Choose a reason for hiding this comment

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

LGTM. There seems to be an issue with the CI. It's unrelated. I'll see if I can get that resolved.

@fluidnumerics-joe
Copy link
Contributor Author

@cgmb - Do you test builds against the intel compiler ?

@cgmb
Copy link
Collaborator

cgmb commented Nov 3, 2023

Unfortunately, no. At the moment, we only test against flang.

@cgmb cgmb self-requested a review November 9, 2023 18:24
drtpotter and others added 3 commits November 10, 2023 21:20
There is no -cpp option for the cray compiler which was causing flag
setting to fail for crayftn
@fluidnumerics-joe
Copy link
Contributor Author

@drtpotter - I got the cray compiler building fine now with the latest commit. I gather this is also working with the Intel compilers ?
@cgmb - Can you see if these commits still function for other tests you may run internally at AMD ?

@drtpotter
Copy link

drtpotter commented Nov 15, 2023

@drtpotter - I got the cray compiler building fine now with the latest commit. I gather this is also working with the Intel compilers ? @cgmb - Can you see if these commits still function for other tests you may run internally at AMD ?

Hi @fluidnumerics-joe. Yes! With the patch-1 branch the CMAKE configuration step continues to be successful with the latest Intel compilers.

cmake -DCMAKE_INSTALL_PREFIX=/opt/hipfort_intel/5.7.1 -DHIPFORT_COMPILER=ifort -DCMAKE_C_COMPILER="icx" -DCMAKE_CXX_COMPILER="icpx" -DCMAKE_Fortran_COMPILER="ifort" ..cmake -DCMAKE_INSTALL_PREFIX=/opt/hipfort_intel/5.7.1 -DHIPFORT_COMPILER=ifort -DCMAKE_C_COMPILER="icx" -DCMAKE_CXX_COMPILER="icpx" -DCMAKE_Fortran_COMPILER="ifort" ..

However, at the compile stage it still fails when trying to compile hipfort_hipmalloc.f.

[  6%] Building Fortran object lib/CMakeFiles/hipfort-amdgcn.dir/hipfort/hipfort_hipmalloc.f.o
ifort: command line warning #10006: ignoring unknown option '-fno-underscoring'
ifort: command line remark #10148: option '-vec-report0' not supported
/netsoft/src/hipfort_joe/lib/hipfort/hipfort_hipmalloc.f(1007): error #6911: The syntax of this substring is invalid.   [PTR]
        ptr(LBOUND(dsource,1):,LBOUND(dsource,2):) => tmp
--------^
compilation aborted for /netsoft/src/hipfort_joe/lib/hipfort/hipfort_hipmalloc.f (code 1)
make[2]: *** [lib/CMakeFiles/hipfort-amdgcn.dir/build.make:231: lib/CMakeFiles/hipfort-amdgcn.dir/hipfort/hipfort_hipmalloc.f.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:144: lib/CMakeFiles/hipfort-amdgcn.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

The Intel Fortran compiler produces really high quality and performant code, and can often find optimization opportunities that open source compilers sometimes miss. It would be really great to have support for the Intel compiler in hipfort!

@fluidnumerics-joe
Copy link
Contributor Author

Hmm.. I'll see if I can put together a build environment for ifort+ROCm to reproduce what you have here.

@fluidnumerics-joe
Copy link
Contributor Author

Quick update on this. I'm able to reproduce the problem that @drtpotter has mentioned with ifort. To be honest, I'm not sure why GNU compilers don't choke on this. The problem appears to be that ptr is an assumed shape pointer with no defined upper and lower bounds. However, at the line in questions (and many others) the lower bounds of either mold or source are used to index the lower bounds of ptr.

With ifort, these errors can be fixed by simply using ptr => tmp; I'll try out some of the included examples on our systems (MI210 and MI50) with this build.

@cgmb , would you be open to at least adding a Github action to verify hipfort can be built with the Intel compilers ? If so, I can provide a workflow as part of the pull request.

@cgmb
Copy link
Collaborator

cgmb commented Dec 12, 2023

Thanks for looking into this, Joe.

@cgmb, would you be open to at least adding a Github action to verify hipfort can be built with the Intel compilers ? If so, I can provide a workflow as part of the pull request.

Sure. I'd be happy to accept GitHub actions that test for any compiler you care about. It would be great to have a CI system that is accessible to external contributors.

@fluidnumerics-joe
Copy link
Contributor Author

Excellent, I'll add in tests that demonstrate hipfort can build with gfortran, ifort, and ifx (for the moment). It'd be great to figure out if we can prove the examples run for these builds..

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.

3 participants