Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Update CMake build instructions #249

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Update CMake build instructions #249

merged 2 commits into from
Mar 16, 2022

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented Feb 15, 2022

This includes changes reflecting that LLVM is no longer a dependency on Windows builds and how to build libcudacxx tests.

Windows info will get updated later.
@wmaxey wmaxey changed the title Update build instructions for the update CMake configs Update build instructions for the updated CMake configs Feb 15, 2022
@wmaxey wmaxey changed the title Update build instructions for the updated CMake configs Update CMake build instructions Feb 15, 2022
@@ -109,11 +111,11 @@ cd ${LIBCUDACXX_ROOT}
mkdir -p build
cd build
cmake .. \
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can drop mkdir, cd, and go with

cmake -S . -B build ....

Copy link
Collaborator

@jrhemstad jrhemstad left a comment

Choose a reason for hiding this comment

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

I can't comment on the line, but is apt-get -y install llvm needed anymore?

Yep, as stated it's still need for cmake modules :)

@jrhemstad
Copy link
Collaborator

jrhemstad commented Feb 21, 2022

re: #175 (comment) should we include -DHAVE_CXX_ATOMICS64_WITHOUT_LIB=True -DHAVE_CXX_ATOMICS_WITHOUT_LIB=True to avoid the cmake warning about libatomic?

@veictry
Copy link
Contributor

veictry commented Feb 22, 2022

variable "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash seems to need to be ${LIBCUDACXX_PATH}/libcxx/build/test/lit.site.cfg but not ${LIBCUDACXX_PATH}/libcxx/build/libcxx/test/lit.site.cfg

@wmaxey
Copy link
Member Author

wmaxey commented Feb 22, 2022

variable "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash seems to need to be ${LIBCUDACXX_PATH}/libcxx/build/test/lit.site.cfg but not ${LIBCUDACXX_PATH}/libcxx/build/libcxx/test/lit.site.cfg

That script should be correct. There are two separate lit configs created depending on which tests have been enabled.

LIBCXX_SITE_CONFIG and LIBCUDACXX_SITE_CONFIG.

The documentation will need to be updated to reflect that as well.

@veictry
Copy link
Contributor

veictry commented Feb 25, 2022

variable "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash seems to need to be ${LIBCUDACXX_PATH}/libcxx/build/test/lit.site.cfg but not ${LIBCUDACXX_PATH}/libcxx/build/libcxx/test/lit.site.cfg

That script should be correct. There are two separate lit configs created depending on which tests have been enabled.

LIBCXX_LIT_SITE_CONFIG and LIBCUDACXX_LIT_SITE_CONFIG.

The documentation will need to be updated to reflect that as well.

Hi Wesley,

 Yes. I tried to ENABLE_LIBCXX_TESTS and it made a build for LIBCXX part (not LIBCUDACXX part).
 The files are created in ${LIBCUDACXX_ROOT}/libcxx/build/...
 and the folder structure is described in the above information.
 I thought that there should be a modification for "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash

=========================

By the way, I saw the same build command for Windows part. Is it expected?

@wmaxey
Copy link
Member Author

wmaxey commented Feb 25, 2022

variable "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash seems to need to be ${LIBCUDACXX_PATH}/libcxx/build/test/lit.site.cfg but not ${LIBCUDACXX_PATH}/libcxx/build/libcxx/test/lit.site.cfg

That script should be correct. There are two separate lit configs created depending on which tests have been enabled.
LIBCXX_LIT_SITE_CONFIG and LIBCUDACXX_LIT_SITE_CONFIG.
The documentation will need to be updated to reflect that as well.

Hi Wesley,

 Yes. I tried to ENABLE_LIBCXX_TESTS and it made a build for LIBCXX part (not LIBCUDACXX part).
 The files are created in ${LIBCUDACXX_ROOT}/libcxx/build/...
 and the folder structure is described in the above information.
 I thought that there should be a modification for "LIBCXX_LIT_SITE_CONFIG" in perform_tests.bash

=========================

By the way, I saw the same build command for Windows part. Is it expected?

Sorry, I should clarify:

Lit consumes these two variables, not the ones I listed in the comments.
LIBCUDACXX_SITE_CONFIG = ${LIBCUDACXX_LIT_SITE_CONFIG}
LIBCXX_SITE_CONFIG = ${LIBCXX_LIT_SITE_CONFIG}

The environment vars are actually assigned later in the test script. I would like to update perform_tests.bash to do just one CMake config, but I didn't want to modify it more in case those outputs are relied on in DVS and such.

The Windows build commands are simplified in the same way though we don't support running libcxx tests yet.

@wmaxey
Copy link
Member Author

wmaxey commented Mar 15, 2022

I can't comment on the line, but is apt-get -y install llvm needed anymore?

Yep, as stated it's still need for cmake modules :)

This may not be required at this point actually. Part of the earlier build changes were to remove our reliance on them.

However, I won't be removing the instruction as I haven't tested on a clean system yet. On Windows cloning llvm-project is no longer required at least.

@wmaxey wmaxey closed this Mar 15, 2022
@wmaxey wmaxey reopened this Mar 15, 2022
@wmaxey wmaxey merged commit 8064b6c into main Mar 16, 2022
@wmaxey wmaxey deleted the docs/build_instructions branch March 16, 2022 19:54
@wmaxey wmaxey added this to the 1.8.1 milestone Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants