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

Add error for attempts to mix CHPL_LLVM=none and CHPL_TARGET_COMPILER=llvm #18837

Merged
merged 1 commit into from Dec 11, 2021

Conversation

bradcray
Copy link
Member

This is my attempt to resolve #18770.

Resolves #18770.

…=llvm

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

Hi @mppf — Since you've done the most work in this file recently, would you review this? As noted above, this is my attempt to fix #18770. The main things I feel uncertain about are:

(a) is this an appropriate choice given my ongoing lack of clarity about the relationship between CHPL_LLVM (the environment variable), the output of chpl_llvm.py (the script), and whether or not the compiler was actually built with LLVM enabled? E.g., I believe that this will issue the error if the former is set to 'none' but the compiler was built with LLVM enabled. Is that reasonable? (or should we choke up higher on the bat, and not permit CHPL_LLVM to be set to none if the compiler is built with LLVM, since, what would that be trying to say anyway?

(b) do you have any suggestions for testing this? It feels like a Makefile edit to me in the sense of "gritting my teeth and hoping nothing breaks."

Copy link
Member

@mppf mppf 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 fine with me.

I would expect CHPL_LLVM=none + a compiler built with LLVM to work but disable LLVM support.

I recently have been annoyed that some errors in configuration cause printchplenv to no longer show the configuration. I think it would be nice to move all of the errors to happen in separate validate calls after printchplenv does its printing. But that is a project and I don't think this PR needs to do it.

@bradcray
Copy link
Member Author

I would expect CHPL_LLVM=none + a compiler built with LLVM to work but disable LLVM support.

Thanks Michael. Having spent the day wrestling with homebrew formulae, I now agree that this is desirable and the way things should be (and seemingly are).

@bradcray bradcray merged commit a434612 into chapel-lang:main Dec 11, 2021
@bradcray bradcray deleted the fix-18770 branch December 11, 2021 00:03
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.

printchplenv crashes when CHPL_TARGET_COMPILER=llvm and CHPL_LLVM=none
2 participants