FIX: Detect CUDA version correctly for setting c++17 #119984

William Leeson · 2024-03-27T21:46:04+01:00

William Leeson commented

2024-03-27 21:46:04 +01:00

Previous version checked cuda_version which was not set until later. However CUDA_VERSION exists prior to this point so checking that should work.

Brecht Van Lommel was assigned by William Leeson

2024-03-27 21:46:04 +01:00

Sergey Sharybin was assigned by William Leeson

2024-03-27 21:46:04 +01:00

William Leeson added 1 commit 2024-03-27 21:46:17 +01:00

c57f9a91c7 FIX: Detect CUDA version correctly for setting c++17

Previous version checked cuda_version which was not set until
much later. However CUDA_VERSION exists prior to this point so
checking that should work.

Iliya Katushenock requested review from Brecht Van Lommel 2024-03-27 21:54:37 +01:00

Iliya Katushenock requested review from Sergey Sharybin 2024-03-27 21:54:38 +01:00

Brecht Van Lommel was unassigned by Iliya Katushenock

2024-03-27 21:54:42 +01:00

Sergey Sharybin was unassigned by Iliya Katushenock

2024-03-27 21:54:42 +01:00

William Leeson added 1 commit 2024-03-28 08:43:40 +01:00

307b074c1a Merge branch 'main' into Fix_CUDA_CMakeList

Brecht Van Lommel requested changes 2024-03-28 15:56:11 +01:00

Brecht Van Lommel left a comment

I don't think this fix is right, but the code is confusing, and it would be good to improve variable names and pass in the cuda version explicitly as an argument into CYCLES_CUDA_KERNEL_ADD.

CUDA_VERSION is the main CUDA version detected. However it is possible to compile with multiple CUDA toolkits, and cuda_version is set depending on the toolkit used for each architecture.

See the code inside foreach(arch ${CYCLES_CUDA_BINARIES_ARCH}).

I don't think this fix is right, but the code is confusing, and it would be good to improve variable names and pass in the cuda version explicitly as an argument into `CYCLES_CUDA_KERNEL_ADD`. `CUDA_VERSION` is the main CUDA version detected. However it is possible to compile with multiple CUDA toolkits, and `cuda_version` is set depending on the toolkit used for each architecture. See the code inside `foreach(arch ${CYCLES_CUDA_BINARIES_ARCH})`.

Sergey Sharybin commented

2024-03-28 17:06:49 +01:00

Is there some repro case which demonstrates the issue?

If the variable is indeed undefined, then I'd expect an error like this to be reported by CMake:

  if given arguments:

    "GREATER_EQUAL" "110"

  Unknown arguments specified

Is there some repro case which demonstrates the issue? If the variable is indeed undefined, then I'd expect an error like this to be reported by CMake: ``` if given arguments: "GREATER_EQUAL" "110" Unknown arguments specified ```

William Leeson commented

2024-03-29 13:24:34 +01:00

Is there some repro case which demonstrates the issue?

If the variable is indeed undefined, then I'd expect an error like this to be reported by CMake:
  if given arguments:

    "GREATER_EQUAL" "110"

  Unknown arguments specified

Yeah, I did notice that it was not getting set. However, I can't find any "Unknown arguments specified" in the logs. I'll see it I can repo it on a plan Blender build. Thanks for the feed back.

> Is there some repro case which demonstrates the issue? > > If the variable is indeed undefined, then I'd expect an error like this to be reported by CMake: > ``` > if given arguments: > > "GREATER_EQUAL" "110" > > Unknown arguments specified > ``` Yeah, I did notice that it was not getting set. However, I can't find any "Unknown arguments specified" in the logs. I'll see it I can repo it on a plan Blender build. Thanks for the feed back.

Merge conflict checking is in progress. Try again in few moments.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u Fix_CUDA_CMakeList:leesonw-Fix_CUDA_CMakeList

git checkout leesonw-Fix_CUDA_CMakeList

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

FIX: Detect CUDA version correctly for setting c++17 #119984

Checkout