Should we rename CHPL_LLVM settings for clarity?

[bradcray]

With some regularity, Chapel users and developers have expressed confusion about why we're building LLVM or they're getting LLVM-related failures during builds when they have CHPL_LLVM=none. The reason is that the current compiler relies on the LLVM support libraries regardless of the CHPL_LLVM setting, such that CHPL_LLVM can now be thought of as "should LLVM be enabled as a back-end option for the compiler?" But it's understandable why someone wouldn't get there.

This issue asks whether we should consider any changes to reduce confusion here, where some options might be:

• rename CHPL_LLVM to CHPL_LLVM_BACKEND to make its role clearer
• replace none with system-support-only / bundled-support-only to clarify that none doesn't literally mean "no LLVM"
  • though would this render CHPL_LLVM_SUPPORT pointless? We could change none to support-only and an indication that CHPL_LLVM_SUPPORT comes into play, but that starts to feel like the choices here are no longer siblings.
• something else?

[bradcray]

When initially kicking this idea around, I preferred bullet 2 because it seemed like the easier path forward with less changing; but given the awkward relationship with CHPL_LLVM_SUPPORT, I'm currently leaning toward option 1. A third option would be to have three variables:

• CHPL_LLVM: sets the default for both the following if neither is set; cannot be set to none without getting an error or overriding CHPL_LLVM_SUPPORT since it can't be none
• CHPL_LLVM_SUPPORT: says whether to use system or bundled for the support libraries—can't be none
• CHPL_LLVM_BACKEND: says whether to use system or bundled or none for the compiler backend (but must match the previous if anything other than none

I'll note that this situation is also feeling a little similar to the conversation @jabraham17 and I were recently having about CHPL_MEM/JEMALLOC settings, captured in https://github.com/chapel-lang/chapel/issues/25359

[jabraham17]

Rather than changing the names of the variables, I would propose we just change printchplenv to always print CHPL_LLVM_SUPPORT when CHPL_LLVM=none. Currently it is hidden unless you run printchplenv --all. I think having separate variables is generally cleaner. For example, we have CHPL_COMM to represent what the comm layer, and various sub variables based on the comm layer for additional settings. And those settings are printed by default.

I'll also point out that we already have CHPL_LLVM_BACKEND in the form of CHPL_TARGET_COMPILER=llvm. It is perfectly valid to have CHPL_LLVM=system and CHPL_TARGET_COMPILER=clang, in this case we will use the system LLVM's clang for the C backend.

[lydia-duncan]

Yeah, I was also thinking that adding an additional environment variable for when LLVM is used as the backend compiler felt redundant with CHPL_TARGET_COMPILER.

Also, I think if we follow the path Jade suggests, we should make sure we update our documentation around CHPL_LLVM to better educate about its purpose in the new mental model we (arguably already) made.

[bradcray]

I would propose we just change printchplenv to always print CHPL_LLVM_SUPPORT when CHPL_LLVM=none.

I think that's a pretty good proposal. It definitely has the benefit of simplicity, and we could choose to only go further if confusion continues.

I'll also point out that we already have CHPL_LLVM_BACKEND in the form of CHPL_TARGET_COMPILER=llvm. It is perfectly valid to have CHPL_LLVM=system and CHPL_TARGET_COMPILER=clang, in this case we will use the system LLVM's clang for the C backend.

I don't consider CHPL_LLVM_BACKEND (at least, as I mean it here) and CHPL_TARGET_COMPILER to be redundant. The former says "Should chpl be built with the LLVM back-end available as an option, and if so, which LLVM implementation should be used (system or bundled)?" The latter says "For this compile, which back-end should be used, LLVM, or C (and if C, which compiler family)?"

we should make sure we update our documentation around CHPL_LLVM to better educate about its purpose in the new mental model we (arguably already) made.

I think the documentation's already great here. I think the problem is that CHPL_LLVM=none intuitively suggests "no LLVM will be used" to those who don't read the documentation.

[mppf]

IMO CHPL_LLVM_BACKEND!=none will be confused with CHPL_TARGET_COMPILER=llvm.

I like the idea of printing CHPL_LLVM_SUPPORT, at least as a 1st attempt to address the issue.

[lydia-duncan]

I mentioned it on #25086, but I think partial or limited would a good replacement for CHPL_LLVM=none. If we wanted to remove CHPL_LLVM_SUPPORT at the same time, we could use partial-bundled/bundled-limited and partial-system/limited-system, but I don't feel strongly either way.

In looking at the output from printing out CHPL_LLVM_SUPPORT, there's still a hierarchy between it and CHPL_LLVM and I don't think it makes sense to remove that hierarchy:

CHPL_RE2: bundled
CHPL_LLVM: none *
  CHPL_LLVM_SUPPORT: bundled
CHPL_AUX_FILESYS: none

Having it only print out when CHPL_LLVM=none is a subtle change - I wouldn't necessarily expect a user to draw conclusions about the importance of that setting based on comparing printchplenv output. I think they'd be lucky to notice that there was an extra variable printed at all. And it being hierarchically nested under CHPL_LLVM would make me assume that it didn't matter as much as the CHPL_LLVM setting itself if I didn't know better - looking at that output from a user's perspective, I'm fairly certain I would still wonder why LLVM was being used when CHPL_LLVM=none. So I really think renaming the none setting is the best way to resolve this problem