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

Update NanoVDB to 2024 OS Image #1926

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Update NanoVDB to 2024 OS Image #1926

merged 4 commits into from
Oct 22, 2024

Conversation

danrbailey
Copy link
Contributor

NanoVDB for Linux is still built on the old 2022 EL7 images and the version of Cuda being used here (11.6) is over two years old. I think it would be a good idea for this to be updated before we release VDB 12.0. I've included the initial change on a feature branch which is currently failing and needs work to update it. Hopefully the changes required are trivial.

(Tagging all the NVidia people - @apradhana @kmuseth @matthewdcong @swahtz)

Signed-off-by: Jonathan Swartz <[email protected]>
@swahtz
Copy link
Contributor

swahtz commented Oct 21, 2024

Hi @danrbailey, looked into this a bit and fixed up the nanovdb action for RHEL8. It looks like this 2024 image comes with the cuda 12.6 toolkit, so I've opted to use that pre-installed nvcc compiler

I'm just having issues with the clang++ builds erroring that we need to compile with -fPIE. It looks like perhaps the default -fPIE behaviour changed in clang 14:

llvm/llvm-project@1042de9

@danrbailey
Copy link
Contributor Author

Details

Thanks for taking a look at this. From what I can tell, almost all of our Linux environments where we use VDB should already be PIE/PIC as it's the default in RHEL7+ and GCC6+/Clang14+. It's a bit surprising to me that we aren't picking up this default. If we set -DCMAKE_POSITION_INDEPENDENT_CODE=ON as a cmake argument, does this resolve this issue?

@swahtz
Copy link
Contributor

swahtz commented Oct 21, 2024

Details

Thanks for taking a look at this. From what I can tell, almost all of our Linux environments where we use VDB should already be PIE/PIC as it's the default in RHEL7+ and GCC6+/Clang14+. It's a bit surprising to me that we aren't picking up this default. If we set -DCMAKE_POSITION_INDEPENDENT_CODE=ON as a cmake argument, does this resolve this issue?

Yes I was a bit confused as well... but this is the error we're seeing when building in this image with clang 14 and I have the same issue when I've replicated it with this image+compiler:

https://github.com/AcademySoftwareFoundation/openvdb/actions/runs/11432517892/job/31803124110?pr=1926#step:6:365

@danrbailey
Copy link
Contributor Author

Yes I was a bit confused as well... but this is the error we're seeing when building in this image with clang 14 and I have the same issue when I've replicated it with this image+compiler:

https://github.com/AcademySoftwareFoundation/openvdb/actions/runs/11432517892/job/31803124110?pr=1926#step:6:365

It sounds like we might be hitting an incompatibility where clang 17 under this image has PIC/PIE disabled by default, but the Cuda libraries we are linking against do? I don't think that's completely surprising because I believe we compile clang 17 ourselves (as part of the OS image), but I'm not aware we have intentionally configured it like this. Can we confirm that the clang we are using in the image has PIC/PIE disabled? Then I think we should ask the CI WG about this on slack.

@swahtz
Copy link
Contributor

swahtz commented Oct 22, 2024

I can indeed confirm that building with -DCMAKE_POSITION_INDEPENDENT_CODE=ON compiles correctly. I'll ask the CI WG about this, I can't seem to see definitively if the CLANG_DEFAULT_PIE_ON_LINUX is set in our clang build.

… fix clang default behaviour change

Signed-off-by: Jonathan Swartz <[email protected]>
@swahtz
Copy link
Contributor

swahtz commented Oct 22, 2024

Per CI WG slack chat, I made a ticket about this clang behaviour issue and have just added the CMAKE_POSITION_INDEPENDENT_CODE to the test job for the time being.

AcademySoftwareFoundation/aswf-docker#228

@swahtz swahtz merged commit f012e74 into master Oct 22, 2024
78 checks passed
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.

3 participants