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

Fold LLVM-or-not into CHPL_TARGET_COMPILER #17800

Merged
merged 36 commits into from Jun 7, 2021

Conversation

mppf
Copy link
Member

@mppf mppf commented May 21, 2021

For #17795

Resolves #17606 #17598 #17584

This PR updates printchplenv, Makefiles, and related scripting to:

  • When CHPL_LLVM=system or CHPL_LLVM=bundled, CHPL_TARGET_COMPILER
    defaults to llvm (per Should LLVM-based compiles be indicated through CHPL_TARGET_COMPILER rather than --llvm? #17795 )
  • have an idea of a prefix and suffix for the current compiler (e.g.
    CHPL_GCC_PREFIX=/usr/local/ and CHPL_GCC_SUFFIX=-10 would select
    /usr/local/bin/gcc-10 and /usr/local/bin/g++-10)
  • Along with that, adds CHPL_LLVM_PREFIX for selecting an installation
    of LLVM
  • adds a warning when doing an LLVM-enabled make of the compiler with
    a PrgEnv and when that PrgEnv is not PrgEnv-gnu. This is just to get
    people to use configurations that we test.
  • Replaces CHPL_TARGET_COMPILER=clang-included with
    CHPL_TARGET_COMPILER=llvm
  • Adds internal printchplenv variables CHPL_HOST_COMPILER_COMMAND_C
    CHPL_HOST_COMPILER_COMMAND_CPP CHPL_TARGET_COMPILER_COMMAND_C
    CHPL_TARGET_COMPILER_COMMAND_CPP and uses them in the Makefiles
    • This resolves a problem I was seeing with PrgEnv-cray where the
      compiler was trying to run clang instead of cc
  • Stops building the runtime twice for LLVM per Should LLVM-based compiles be indicated through CHPL_TARGET_COMPILER rather than --llvm? #17795
  • removes CHPL_ORIG_TARGET_COMPILER (replacing it with the more
    focused CHPL_TARGET_COMPILER_PRGENV which just indicates which
    PrgEnv is loaded, if any)
  • Renames a few Makefile helpers in third-party/llvm to use the term
    "bundled"
  • Moves setting LLVM_CLANG_ARGUMENTS_FILE to only CHPL_LLVM=bundled
    configurations since it should only be relevant in that case
  • many supporting changes in the util/chplenv scripts

Future Work:

  • Update CHPL_LLVM_PREFIX and the commented-out CHPL_GCC_PREFIX /
    CHPL_GCC_SUFFIX / CHPL_CLANG_PREFIX / CHPL_CLANG_SUFFIX with
    whatever we decide in that discussion
  • Consider making CHPL_TARGET_COMPILER=clang + CHPL_LLVM=bundled an
    error in a PrgEnv environment since it doesn't work right now (we
    don't gather the appropriate extra flags for e.g. ugni dependencies)
  • full local testing with CHPL_LLVM=system
  • full local testing with CHPL_LLVM=none

Reviewed by @daviditen - thanks!

@mppf mppf changed the title Adjustments to printchplenv for llvm-by-default Fold LLVM-or-not into CHPL_TARGET_COMPILER May 28, 2021
@mppf mppf requested a review from daviditen June 1, 2021 15:46
@mppf mppf force-pushed the clang-included-to-llvm branch 2 times, most recently from 3d52b7b to 0e7e94e Compare June 4, 2021 14:14
@mppf mppf marked this pull request as ready for review June 4, 2021 17:13
util/cron/test-llvm.bash Outdated Show resolved Hide resolved
@@ -288,6 +294,16 @@ ECHO = echo
#
EXE_SUFFIX =

# in case something goes wrong and we don't have a compiler, try
Copy link
Member

Choose a reason for hiding this comment

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

Should the default base compiler be clang instead of gcc?

Copy link
Member Author

@mppf mppf Jun 7, 2021

Choose a reason for hiding this comment

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

I'm not expecting this to ever get run - we could make it a fatal error instead if we wanted. I'm not sure there really is a good option but I'd bet linux64 is slightly more typical of a platform for using Chapel than Mac, but who knows.

Edit -- also on Mac OS X there is a gcc that just runs clang -- but I wouldn't want to rely on that for much.

mppf added 22 commits June 7, 2021 14:37
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
* don't need llvm_mode anymore
* now we have get_prgenv_compiler

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
… etc

* Adds support for CHPL_CLANG_PREFIX, CHPL_GCC_PREFIX,
  CHPL_CLANG_SUFFIX, CHPL_GCC_SUFFIX for finding the compiler
* Addresses problem with Makefile.clang choosing `clang` as the
  command name when using PrgEnv-cray
* CHPL_LLVM=system|bundled and CHPL_TARGET_COMPILER=clang
  uses the clang selected for CHPL_LLVM
* CHPL_LLVM=system and CHPL_HOST_COMPILER=clang
  uses the clang selected for CHPL_LLVM

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
setting CHPL_TARGET_COMPILER explicitly should work

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
…_CXX

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
mppf added 12 commits June 7, 2021 14:37
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
* better print test name when it fails
* symlink third-party and make into fakey installation

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
to work around omp.h missing in our system clang installation

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf merged commit 09b1be1 into chapel-lang:master Jun 7, 2021
@mppf mppf deleted the clang-included-to-llvm branch June 7, 2021 19:20
mppf added a commit to mppf/chapel that referenced this pull request Jun 8, 2021
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
mppf added a commit to mppf/chapel that referenced this pull request Jun 8, 2021
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
mppf added a commit that referenced this pull request Jun 8, 2021
Fix error PR #17800 introduced in multilocale.skipif

This PR adjusts test/interop/C/multilocale.skipif. That file skips the 
directory for the LLVM backend since multilocale libraries aren't working 
yet with LLVM.

Follow-up to #17800 which introduced the error in multilocale.skipif.

Reviewed by @dlongnecke-cray - thanks!
mppf added a commit that referenced this pull request Jun 8, 2021
This PR makes adjustments to `build_configs` to work with the changes
from PR #17800.  In particular, that PR changed `make` for LLVM
configurations to no longer build the runtime twice (once for LLVM and
once for the current C compiler). But the `build_configs` module build
was relying on this behavior.

* Set `CHPL_TARGET_COMPILER` always in the `build_configs` setenv scripts
* Build LLVM as the `CHPL_TARGET_COMPILER` separately
* do so for EX, x86, and arm builds

Note that setting `CHPL_TARGET_COMPILER` on a PrgEnv system used to 
result in an error but no longer does. I do not know when that changed
but it was at least not an error at the time of the last release (or at
least not an error to run printchplenv). Now it is necessary to set it in
order to request the C backend on such systems when the LLVM backend is
available.

- [x] checked that the following command functions as expected

```
util/buildRelease/test_rpm_module.bash  https://github.com/mppf/chapel fix-build-configs-17800
```

Reviewed by @gbtitus and @Maxrimus - thanks!
mppf added a commit that referenced this pull request Jun 9, 2021
Re-deprecate --llvm and fix linux64-incr testing

Follow-up to #17800.

* Sets `CHPL_TARGET_COMPILER=gnu` for linux64-incr (since LLVM backend 
  does not yet support incremental compilation)
* De-de-deprecates --llvm (reverts  #17880). The real fix here is to stop 
  using `--llvm` or `CHPL_LLVM_CODEGEN` in the test configs which I have 
  done in separate PRs.

Reviewed by @Maxrimus - thanks!

- [x] full local testing
mppf added a commit that referenced this pull request Jun 23, 2021
Allow setting CC, CXX, and other variables to indicate compiler path

This PR implements changes to how users can indicate a particular path to
a particular compiler. It is sort of a follow-up to PR #17800 which added
`CHPL_LLVM_PREFIX` for selecting an LLVM installation.

This PR makes the following variables settable by users (as environment
variables or in a chplconfig):
 * for setting all the compilers (host and target) at once using
   relatively well known environment variable names:
    * CC, CXX
 * for setting host and target compilers separately:
    * `CHPL_HOST_CC`, `CHPL_HOST_CXX`
    * `CHPL_TARGET_CC`, `CHPL_TARGET_CXX`
 * for indicating which system LLVM installation should be used / the
   path to it by providing the path to the llvm-config command:
    * `CHPL_LLVM_CONFIG`

Additionally it does some clean-ups for LLVM Makefiles.

For printchplenv, the new variables are in the compiler or runtime
categories but aren't printed by default (so `printchplenv --all` will
print them). This PR adjusts `printchplenv.py` to include an idea that
some variables shouldn't be in `--path` output since it doesn't make
sense to store the path to CC say in the compiler/runtime build path.

PR #17931 contains the documentation changes to go along with these changes.

Reviewed by @daviditen - thanks!

- [x] full local testing

 Future Work:
  * adding testing for setting `CHPL_HOST_CC` / `CC` / `CHPL_LLVM_CONFIG` etc.
ronawho added a commit that referenced this pull request Nov 19, 2021
Fix comm count microbenchmarks

[reviewed by @mppf]

Previously, these tests always ran the default backend (labelled as
C-backend) and if llvm was enabled it would run llvm variants as well.
With the switch to llvm as our default and changes made in #17800 we
stopped running the default config for these microbenchmarks and only
ran llvm-wideopt variants.

Fix that here and rename the configs to avoid mention of the backend
and be more explicit about cache vs. no-cache and wideopt vs. no-wideopt
to avoid this problem with future changes to our default config.

Resolves Cray/chapel-private#2409
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.

Move/remove clang-included from the Chapel Docs
2 participants