Fetch slang-llvm.so from correct release #4847

expipiplus1 · 2024-08-14T05:51:54Z

Closes #4648
Should close #4812

Closes shader-slang#4648 Should close shader-slang#4812

jkwak-work · 2024-08-16T22:16:30Z

This change appears to solve the problem on macOS that uses M3 chip, which is aarch64.
But it gives me a following error on macOS that uses Intel chip.

$ cmake --preset default
...omit...
CMake Error at CMakeLists.txt:366 (message):
 Unable to find libslang-llvm.dylib in
 https://github.com/shader-slang/slang/releases/download/v2024.1.31/slang-2024.1.31-macos-x86_64.zip
Call Stack (most recent call first):
 CMakeLists.txt:393 (from_glob)
-- Configuring incomplete, errors occurred!

It also gives me a following error when I use it on Windows,

cmake.exe --preset vs2019 -DSLANG_ENABLE_EXAMPLES=OFF -DCMAKE_COMPILE_WARNING_AS_ERROR=true --log-level=ERROR
Preset CMake variables:

  CMAKE_CXX_FLAGS_INIT="-D_ITERATOR_DEBUG_LEVEL=0 /MP"
  CMAKE_C_FLAGS_INIT="-D_ITERATOR_DEBUG_LEVEL=0 /MP"
  CMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>"

-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.22631.
CMake Error at CMakeLists.txt:141 (message):
  Unable to find binary release for slang-llvm, please set a different
  SLANG_SLANG_LLVM_FLAVOR or set SLANG_SLANG_LLVM_BINARY_URL manually

Note that before this PR, "cmake --preset release" worked fine with all three configurations: macOS with Intel, macOS with M3, and Windows.
Only problem was that "slang-test" was not working for macOS with M3; it worked for macOS with Intel and Windows.

With this PR, "cmake --preset release" is failing on macOS with Intel and Windows.
macOS with M3 chip builds fine and slang-test is also working.

jkwak-work · 2024-08-16T22:43:53Z

When I don't snooze the verbose warning messages, I got more usefull error message on Windows.

$ cmake.exe --preset default
Preset CMake variables:

  CMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>"

-- Found Git: C:/Program Files/Git/cmd/git.exe (found version "2.46.0.windows.1")
-- The C compiler identification is Clang 18.1.8 with GNU-like command-line
-- The CXX compiler identification is Clang 18.1.8 with GNU-like command-line
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/LLVM/bin/clang.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/LLVM/bin/clang++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found CUDAToolkit: C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v12.3/include (found version "12.3.107")
-- Could NOT find OptiX (missing: OptiX_INCLUDE_DIRS)
-- Could NOT find NVAPI (missing: NVAPI_INCLUDE_DIRS NVAPI_LIBRARIES)
-- Could NOT find Aftermath (missing: Aftermath_INCLUDE_DIRS Aftermath_LIBRARIES)
CMake Warning at cmake/GitHubRelease.cmake:78 (message):
  Unsupported architecture for slang binary releases: AMD64
Call Stack (most recent call first):
  CMakeLists.txt:139 (get_best_slang_binary_release_url)


CMake Error at CMakeLists.txt:141 (message):
  Unable to find binary release for slang-llvm, please set a different
  SLANG_SLANG_LLVM_FLAVOR or set SLANG_SLANG_LLVM_BINARY_URL manually

jkwak-work · 2024-08-16T23:08:32Z

cmake/GitHubRelease.cmake

+endfunction()
+
+function(get_best_slang_binary_release_url out_var)
+    if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64")


Changing amd64 to capitalized AMD64 resolves the problem on Windows.

jkwak-work

Looks good to me.

expipiplus1 · 2024-08-19T07:37:39Z

fixed, feel free to merge please

jkwak-work · 2024-08-19T20:40:02Z

cmake/GitHubRelease.cmake

+    if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|amd64|AMD64")
        set(arch "x86_64")
-    elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|arm64")
+    elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64|ARM64|arm64")


On Windows, it looks like the value of CMAKE_SYSTEM_PROCESSOR comes from an environment variable, "PROCESSOR_ARCHITECTURE".
https://cmake.org/cmake/help/latest/variable/CMAKE_HOST_SYSTEM_PROCESSOR.html#variable:CMAKE_HOST_SYSTEM_PROCESSOR

And it looks like the possible values are AMD64, IA64, ARM64 or x86:
https://learn.microsoft.com/en-us/windows/win32/winprog64/wow64-implementation-details?redirectedfrom=MSDN
Note that ARM64 should be in capital letters.
I am not sure if x86_64 is correct or not; it may need to be x86.

we don't make releases for x86 so it would always fail anyway.

csyonghe · 2024-08-19T22:07:51Z

@jkwak-work I see this PR has been previously approved but there are more comments thereafter.
Are there any changes needed in this PR before we can merge?

jkwak-work · 2024-08-20T14:07:05Z

I think it is good to merge.
I misread a few lines.

This reverts commit 579d59c.

* Fetch slang-llvm.so from correct release Closes shader-slang#4648 Should close shader-slang#4812 * Update docs * Correct documentation on cmake option * Neaten cmake script * Fix fetching on windows --------- Co-authored-by: Yong He <yonghe@outlook.com>

* Dont require llvm for building generators * Fetch slang-llvm.so from correct release (#4847) * Fetch slang-llvm.so from correct release Closes #4648 Should close #4812 * Update docs * Correct documentation on cmake option * Neaten cmake script * Fix fetching on windows --------- Co-authored-by: Yong He <yonghe@outlook.com> * Be a bit more careful dealing with release list fetching failure * clarify error messages --------- Co-authored-by: Yong He <yonghe@outlook.com>

Slang v2024.10 This release brings support for tuple types, variadic generics and depedent generic constraints. No breaking changes. Changes: e97e7e5 Revert "Fetch slang-llvm.so from correct release (shader-slang#4847)" (shader-slang#4893) 359e96c Proposal: A simpler and more flexible `IDifferentiable` system (shader-slang#4865) f9f6a28 Support dependent generic constraints. (shader-slang#4870) 03e1e17 Fix `tests\autodiff\reverse-while-loop-3.slang` test (shader-slang#4886) bcb5391 Exclude synthesized code from code auto documentation system (shader-slang#4889) 6b1b243 Track uninitialized values of `Ptr<Specialize<T>>` inside type `T` without hang (shader-slang#4885) 77e6c64 Fixes shader-slang#4879 (shader-slang#4881) 579d59c Fetch slang-llvm.so from correct release (shader-slang#4847) d286ff5 Implement Path::createDirectoryRecursive (shader-slang#4871) f77a5ac Remove using SpvStorageClass values casted into AddressSpace values (shader-slang#4861) 453683b Tuple swizzling, concat, comparison and `countof`. (shader-slang#4856) ecf85df Variadic Generics Part 2: IR lowering and specialization. (shader-slang#4849) ca5d303 Make sure to resolve overloaded expr for call args. (shader-slang#4864) 25bc5a3 Avoiding the use of the global AST builder in DeclRefType::create (shader-slang#4866) b411c05 Include inout cast operation as an aliasing instruction (shader-slang#4859) 9bf5dc9 Design proposal: IFunc interface. (shader-slang#4851) f447b74 Update documentation for #include to indicate it is for legacy code and new code should use modules (shader-slang#4862)

Fetch slang-llvm.so from correct release

92f7221

Closes shader-slang#4648 Should close shader-slang#4812

expipiplus1 requested a review from jkwak-work August 14, 2024 05:52

expipiplus1 mentioned this pull request Aug 14, 2024

"macos-aarch64" package from the release works for Intel chip but not for M3 chip. #4812

Closed

Update docs

e8e3d69

csyonghe added the pr: non-breaking PRs without breaking changes label Aug 14, 2024

expipiplus1 added 3 commits August 15, 2024 17:53

Merge remote-tracking branch 'origin/master' into slang-llvm-fetch

bcb4d80

Correct documentation on cmake option

cf5f90c

Neaten cmake script

89f26bb

jkwak-work reviewed Aug 16, 2024

View reviewed changes

jkwak-work previously approved these changes Aug 16, 2024

View reviewed changes

Fix fetching on windows

de7d9ca

expipiplus1 dismissed jkwak-work’s stale review via de7d9ca August 19, 2024 07:36

jkwak-work reviewed Aug 19, 2024

View reviewed changes

csyonghe added 2 commits August 20, 2024 08:07

Merge branch 'master' into slang-llvm-fetch

3788699

Merge branch 'master' into slang-llvm-fetch

d8bc684

csyonghe approved these changes Aug 20, 2024

View reviewed changes

csyonghe merged commit 579d59c into shader-slang:master Aug 20, 2024
12 checks passed

csyonghe added a commit to csyonghe/slang that referenced this pull request Aug 21, 2024

Revert "Fetch slang-llvm.so from correct release (shader-slang#4847)"

7a4b202

This reverts commit 579d59c.

csyonghe added a commit that referenced this pull request Aug 21, 2024

Revert "Fetch slang-llvm.so from correct release (#4847)" (#4893)

e97e7e5

This reverts commit 579d59c.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch slang-llvm.so from correct release #4847

Fetch slang-llvm.so from correct release #4847

expipiplus1 commented Aug 14, 2024

jkwak-work commented Aug 16, 2024

jkwak-work commented Aug 16, 2024

jkwak-work Aug 16, 2024

jkwak-work left a comment

expipiplus1 commented Aug 19, 2024

jkwak-work Aug 19, 2024

expipiplus1 Aug 20, 2024

csyonghe commented Aug 19, 2024 •

edited

Loading

jkwak-work commented Aug 20, 2024

Fetch slang-llvm.so from correct release #4847

Fetch slang-llvm.so from correct release #4847

Conversation

expipiplus1 commented Aug 14, 2024

jkwak-work commented Aug 16, 2024

jkwak-work commented Aug 16, 2024

jkwak-work Aug 16, 2024

Choose a reason for hiding this comment

jkwak-work left a comment

Choose a reason for hiding this comment

expipiplus1 commented Aug 19, 2024

jkwak-work Aug 19, 2024

Choose a reason for hiding this comment

expipiplus1 Aug 20, 2024

Choose a reason for hiding this comment

csyonghe commented Aug 19, 2024 • edited Loading

jkwak-work commented Aug 20, 2024

csyonghe commented Aug 19, 2024 •

edited

Loading