Skip to content

update Clang-AOMP easyblock for ROCm 5.6 + consider both $EBROOTGCC and $EBROOTGCCCORE to specify -DGCC_INSTALL_PREFIX#2958

Merged
boegel merged 5 commits intoeasybuilders:developfrom
akesandgren:20230707092559_new_pr_clang_aomp
Feb 12, 2024
Merged

update Clang-AOMP easyblock for ROCm 5.6 + consider both $EBROOTGCC and $EBROOTGCCCORE to specify -DGCC_INSTALL_PREFIX#2958
boegel merged 5 commits intoeasybuilders:developfrom
akesandgren:20230707092559_new_pr_clang_aomp

Conversation

@akesandgren
Copy link
Copy Markdown
Contributor

(created using eb --new-pr)

@akesandgren akesandgren added this to the 4.8.0 milestone Jul 7, 2023
@akesandgren akesandgren modified the milestones: 4.8.0, 4.x Jul 7, 2023
@boegel boegel changed the title update clang_aomp for ROCm 5.6 update Clang-AOMP easyblock for ROCm 5.6 Aug 2, 2023
Comment thread easybuild/easyblocks/c/clang_aomp.py Outdated
"-DLLVM_ENABLE_PROJECTS='clang;lld;compiler-rt'",
"-DCLANG_DEFAULT_LINKER=lld",
"-DGCC_INSTALL_PREFIX=$EBROOTGCC",
"-DGCC_INSTALL_PREFIX=$EBROOTGCCCORE",
Copy link
Copy Markdown
Member

@boegel boegel Sep 13, 2023

Choose a reason for hiding this comment

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

@akesandgren Don't we want to consider both?
How about using a construct like: ${EBROOTGCC:-${EBROOTGCCCORE}}, to use $EBROOTGCCCORE as a fallback if $EBROOTGCC is not defined? (or other way around, not sure what is better)

Copy link
Copy Markdown
Contributor Author

@akesandgren akesandgren Sep 13, 2023

Choose a reason for hiding this comment

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

Ahh, forgot about that one...
${EBROOTGCC:-${EBROOTGCCCORE}} is probably better, or pick up on it in python code first with getenv(...)

On the other hand, GCCCORE will be there and is the actual GCC compiler...

Copy link
Copy Markdown
Member

@boegel boegel Sep 27, 2023

Choose a reason for hiding this comment

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

If ${EBROOTGCC} is set, it should also point to the installation prefix of the actual compiler, because we use altroot = 'GCCcore' in the GCC easyconfigs.

Picking up the value with os.getenv is probably better, since it makes the command being run more explicit (you don't need to go and figure out what $EBROOTGCCCORE is set to when that command is being run

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since Clang-AOMP is at GCCcore level picking up os.getenv('EBROOTGCCCORE') should be enough, but should we protect against someone trying to use this on some completely different base toolchain?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@boegel any more thoughts on this before I change this to use os.getenv instead of $EBROOT...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@boegel ping

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@boegel ping again

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@akesandgren Yes, we should, we should consider both $EBROOTGCC and $EBROOTGCCCORE, as suggested.

If we want to make this backwards compatible, we should consider $EBROOTGCC first, and only pick up on $EBROOTGCCCORE if $EBROOTGCC is not defined.

@akesandgren
Copy link
Copy Markdown
Contributor Author

@boegel So this should do then?

@akesandgren
Copy link
Copy Markdown
Contributor Author

Test report by @akesandgren

Overview of tested easyconfigs (in order)

  • SUCCESS Clang-AOMP-4.5.0-GCCcore-11.2.0.eb
  • SUCCESS Clang-AOMP-5.4.0-GCCcore-11.3.0.eb
  • SUCCESS Clang-AOMP-5.6.0-GCCcore-11.3.0.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
b-cn1603.hpc2n.umu.se - Linux Ubuntu 22.04, x86_64, AMD EPYC 7313 16-Core Processor, 1 x NVIDIA NVIDIA A100 80GB PCIe, 525.147.05, Python 3.10.12
See https://gist.github.com/akesandgren/3ec46cb788973da8ffaf0b7ef404062b for a full test report.

@boegel boegel changed the title update Clang-AOMP easyblock for ROCm 5.6 update Clang-AOMP easyblock for ROCm 5.6 + consider both $EBROOTGCC and $EBROOTGCCCORE to specify -DGCC_INSTALL_PREFIX Feb 8, 2024
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 8, 2024

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS Clang-AOMP-4.5.0-GCCcore-11.2.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3120.skitty.os - Linux RHEL 8.8, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/boegel/95667a6df9c23bcb7beadefd2a95fdf3 for a full test report.

@boegel boegel merged commit aed9c02 into easybuilders:develop Feb 12, 2024
@boegel boegel modified the milestones: 4.x, release after 4.9.0 Feb 12, 2024
@akesandgren akesandgren deleted the 20230707092559_new_pr_clang_aomp branch February 13, 2024 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants