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

Remove error when building LLVM without PrgEnv-gnu loaded on XC #17904

Merged

Conversation

bradcray
Copy link
Member

@bradcray bradcray commented Jun 10, 2021

This PR removes an error that we generate if PrgEnv-gnu is loaded when
building the Chapel compiler with CHPL_LLVM!=none.

Michael added the error defensively while we were locking down LLVM
test failures and configurations because it represented a case that we
didn't test against nightly and that was causing some problems.
However, we believe that unifying the llvm setting with
CHPL_TARGET_COMPILER probably resolved those issues (and a quick test
on my part seems to confirm that). Meanwhile the presence of the
error caused some churn for users who had configurations in which they
were trying to build with CHPL_LLVM=system and
CHPL_TARGET_COMPILER=PrgEnv-intel.

While we were able to advise those users about what they probably
wanted to do for that test configuration (set CHPL_LLVM=none so they
could focus on the intel build of the runtime), it also raised the
question "Do we really care what PrgEnv is loaded on an XC when doing
an LLVM-enabled build?" Specifically, 'gcc' will be used as the host
compiler and 'clang' will be used as the target compiler, so the
choice of PrgEnv-* module shouldn't really be held against the user
since their choice won't be used in any way (we do swap in PrgEnv-gnu
in order to query some paths and such, but that doesn't really care which
PrgEnv was loaded to begin with. And since we don't typically prevent choices
in the CHPL_* configuration that we don't test against until we're
aware of a problem with them, it makes sense to relax this defensive
error until such a time.

This PR removes an error that we generate if PrgEnv-gnu is loaded when
building the Chapel compiler with CHPL_LLVM!=none.

Michael added the error defensively while we were locking down LLVM
test failures and configurations because it represented a case that we
didn't test against nightly and that was causing some problems.
However, we believe that unifying the llvm setting with
CHPL_TARGET_COMPILER probably resolved those issues (and a quick test
on my part seems to confirm that).  Meanwhile the presence of the
error caused some churn for users who had configurations in which they
were trying to build with CHPL_LLVM=system and
CHPL_TARGET_COMPILER=PrgEnv-intel.

While we were able to advise those users about what they probably
wanted to do for that test configuration (set CHPL_LLVM=none so they
could focus on the intel build of the runtime), it also raised the
question "Do we really care what PrgEnv is loaded on an XC when doing
an LLVM-enabled build?"  Specifically, 'gcc' will be used as the host
compiler and 'clang' will be used as the target compiler, so the
choice of 'PrgEnv-*' module shouldn't really be held against the user
since it won't be used.  And since we don't typically prevent spaces
in the CHPL_* configuration that we don't test against until we're
aware of a problem with them, it makes sense to relax this defensive
error until such a time.

---
Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
@bradcray
Copy link
Member Author

@mppf: Here's the PR we were discussing on chat to disable the PrgEnv-gnu error. Please let me know if you think I've mischaracterized anything (or not characterized them fairly) in the OP.

I'd also be curious for your thoughts on #17905 which occurred to me while I was writing up this issue and testing various configurations. It seems logical to me, though subtle, and potentially slightly frustrating in terms of returning a bit more to the "build the runtime" world we were in (albeit in a very specific case).

(I'm sending out a design mail for both of these to the team as well).

@mppf
Copy link
Member

mppf commented Jun 14, 2021

Specifically, 'gcc' will be used as the host compiler and 'clang' will be used as the target compiler, so the choice of PrgEnv-* module shouldn't really be held against the user since it won't be used.

But it is used, in the sense that we "use" the PrgEnv in util/config/gather-cray-prgenv-arguments.bash to get appropriate includes, linker paths, and automatically-included libraries. But the first thing that script does is to swap the PrgEnv to PrgEnv-gnu.

Anyway I think it's slightly inaccurate to say that the PrgEnv-* module is unused in llvm compiles because that would imply we would have the same behavior whether or not any PrgEnv-* module is loaded and that's not the case.

These comments are only relevant to the narrative in the PR description and I'm fine with the change itself.

@bradcray bradcray merged commit 755ddc8 into chapel-lang:master Jun 17, 2021
@bradcray bradcray deleted the remove-prgenv-gnu-error-for-llvm branch June 17, 2021 22:40
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.

None yet

2 participants