Should `printchplenv` print `LLVM_SUPPORT` by default? (and maybe other `LLVM` settings as well?)

[bradcray]

On a few occasions, users have expressed confusion that we're trying to make use of LLVM when they have CHPL_LLVM=none set. As developers, we know that the reason is that we're now using the LLVM support libraries for the compiler build, but this isn't generally well-known, and it's certainly understandable why our trying to use LLVM when CHPL_LLVM=none could be counterintuitive.

A simple step we could take to try and address this would be to print the LLVM_SUPPORT setting by default with printchplenv as well. This would at least give some indication that even with CHPL_LLVM=none, there are other settings that involve LLVM and are defaulting to something else.

(For that matter, maybe we should consider printing all of the LLVM variables by default given that determining what LLVM installation someone's using has become one of the most common things for us to debug when users have build questions?)

(We could also consider renaming CHPL_LLVM to something that's more specific about what exactly it does—like CHPL_LLVM_BACKEND, though this isn't quite accurate since it's also involved in the front-end for parsing extern blocks… But that's a bigger change and involves design discussions. The benefit would be that it arguably wouldn't be as surprising that a CHPL_LLVM=none build would try to use LLVM).

[mppf]

A related issue is that the name CHPL_LLVM_SUPPORT is misleading. It's intended to mean "Where to find the LLVM Support Library" but it could be interpreted to mean "Should it build with LLVM Support?"

[lydia-duncan]

Imo, also including CHPL_LLVM_SUPPORT doesn't solve the confusion, as it still is nested under CHPL_LLVM in the output:

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

which to me would imply that it doesn't matter as much when CHPL_LLVM=none (which isn't the case).

We could also consider renaming CHPL_LLVM to something that's more specific about what exactly it does—like CHPL_LLVM_BACKEND, though this isn't quite accurate since it's also involved in the front-end for parsing extern blocks… But that's a bigger change and involves design discussions. The benefit would be that it arguably wouldn't be as surprising that a CHPL_LLVM=none build would try to use LLVM

I've been thinking about this a bit, and I think the problem isn't so much with the name CHPL_LLVM as it is with the setting value none. I think something like partial or limited instead would be clearer, especially with no obvious "zero" equivalent present. Maybe even partial-bundled or bundled-limited, if we wanted to be explicit about where the partial support was coming from.

[lydia-duncan]

In bringing this up in the team meeting today, it sounded like people were interested in brainstorming more. Will look into forming an ad-hoc subteam on it soon, but capturing some of the ideas here so they don't get lost:

• "support-only"
  • Though there was an objection that this doesn't necessarily make it clear what the support means
• "headers"
• "lib-support-only"
• "select"
• "minimal"

There was also some talk like this:

Today we have CHPL_LLVM and CHPL_LLVM_SUPPORT. Perhaps we should have instead CHPL_LLVM (bundled or system) and CHPL_LLVM_BACKEND (yes or no?).

Which may or may not be retreading old ground in this thread (I'll remind myself of the status before the meeting)

[bradcray]

Waking back up on this after Lydia raised it today:

Thinking about this if we were starting from scratch, I think there's a lot of merit to Michael's latest proposal, quoted just above (and reflecting where my head was going during the meeting as well):

Today we have CHPL_LLVM and CHPL_LLVM_SUPPORT. Perhaps we should have instead CHPL_LLVM (bundled or system) and CHPL_LLVM_BACKEND (yes or no?).

where rather than yes or no, I'd imagine:

CHPL_LLVM=bundled | system : We're gonna use LLVM, so tell us where we should get it from CHPL_LLVM_BACKEND=enabled | disabled: or maybe on or off

where I'd imagine CHPL_LLVM to default to system if available and appropriate, as it does today, and unset otherwise; and for CHPL_LLVM_BACKEND to default to enabled.

This would also mean retiring the CHPL_LLVM_SUPPORT option, which seems pretty attractive since today it can lead to problems, like if you set CHPL_LLVM=bundled and CHPL_LLVM_SUPPORT=system.

The downside I see with supporting CHPL_LLVM=support-only or the like as an option instead of none is that it doesn't say where LLVM comes from, only that it's not expected to be used for the back-end, making the variable have different meanings in different contexts (sometimes it says where to get it; sometimes it says what to use it for and CHPL_LLVM_SUPPORT says where to get it).

[mppf]

"headers"

FWIW, when building in "LLVM Support Library Only, No LLVM Backend" mode (whatever we call that), we do link with the LLVM Support library. It is not a headers-only situation. (We use -lLLVMSupport -lLLVMDemangle today).

[jabraham17]

Do we need to maintain the config of CHPL_LLVM=none and CHPL_LLVM_SUPPORT=system? We could have CHPL_LLVM=system|bundled|support|unset and remove CHPL_LLVM_SUPPORT, where CHPL_LLVM=support requires using the bundled LLVM support library. This could also be CHPL_LLVM=minimal or whatever makes the most sense to people.

I think the scenario where a user has CHPL_LLVM=none and truly wants CHPL_LLVM_SUPPORT=system is unlikely. The closest I can come up with is on a system that has too old of a cmake to build LLVM may want this. But in that case why not CHPL_LLVM=system? You already can't have CHPL_LLVM_SUPPORT=bundled on those systems.

[mppf]

Do we need to maintain the config of CHPL_LLVM=none and CHPL_LLVM_SUPPORT=system?

FWIW, the main use of this configuration that I know of is for making it easier to test the behavior of the "LLVM Support Library Only, No LLVM Backend" mode (since you don't have to build the bundled support library). I don't think this is a particularly strong reason to keep it, if dropping it allows us to get these variables to be more sensible.

[bradcray]

Do we need to maintain the config of CHPL_LLVM=none and CHPL_LLVM_SUPPORT=system?

I buy that that combination may not be the most important (the best rationale I can come up with is that there's some bug in the compiler's LLVM back-end code that I want to circumvent, and that I don't want to spend time building the bundled version—though I'm not saying that's a strong rationale).

I think the main thing I don't like about the proposal:

We could have CHPL_LLVM=system|bundled|support|unset and remove CHPL_LLVM_SUPPORT

is that it feels like mixing two things into one variable ("Where should I get LLVM?" and "What should I use it for?") which doesn't feel like a great design to me.