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

Making HLSL code-gen a couple orders of magnitude faster... #7719

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

slomp
Copy link
Contributor

@slomp slomp commented Jul 27, 2023

Title says it all!

The redundant call to print_expr() ends up triggering an exponential chain of events that ultimately gets discarded.

@slomp
Copy link
Contributor Author

slomp commented Jul 28, 2023

Are the CI errors expected?

@steven-johnson
Copy link
Contributor

Are the CI errors expected?

sadly, yes

@steven-johnson steven-johnson merged commit 89ffae2 into main Jul 28, 2023
3 checks passed
@steven-johnson steven-johnson deleted the slomp/hlsl-codegen-speedup branch July 28, 2023 16:23
@steven-johnson
Copy link
Contributor

We are seeing massive breakage in D3D12 on our buildbot (see https://buildbot.halide-lang.org/master/#/builders/148/builds/58/steps/30/logs/stdio), could this have injected it?

@slomp
Copy link
Contributor Author

slomp commented Aug 1, 2023

That's weird... I would not expect it to cause failures, since it was removing a code path with non-mutating IR that just ended up being discarded in the end.
Do you know which GPU (and driver version) is installed on the machine running the tests?

@steven-johnson
Copy link
Contributor

Do you know which GPU (and driver version) is installed on the machine running the tests?

Nope, you should log into it and see

@slomp
Copy link
Contributor Author

slomp commented Aug 1, 2023

Actually, right there in the log:

Device selected: NVIDIA GeForce GTX 750 Ti

This card predates Direct3D 12 by more than a year, it seems.
Is there a way to avoid scheduling d3d12 tests on that machine?

@steven-johnson
Copy link
Contributor

Is there a way to avoid scheduling d3d12 tests on that machine?

It's literally the only Windows test machine that is running right now; WinBot3 is dead and probably won't get fixed until the weekend. We could just turn off D3D12 testing entirely until then, but if it can't use D3D12 at all, it seems unlikely we wouldn't have seen it fail on this machine before.

@slomp
Copy link
Contributor Author

slomp commented Aug 1, 2023

I also saw this on the logs:

D3DCompile(): C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero
C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero
C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero
C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero
C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero
C:\build_bot\worker\halide-testbranch-main-llvm18-x86-64-windows-cmake\halide-build\test\correctness\_kernel_f7_s0___outermost___outermost_v4___block_id_x(89,19-27): warning X4008: floating point division by zero

@abadams
Copy link
Member

abadams commented Aug 1, 2023

I don't see any way this PR could have caused those test failures, but these tests were passing on that machine, and nothing else has touched d3d12 lately, so...

I'll open a revert PR just to see whether or not it makes the tests pass again.

@slomp
Copy link
Contributor Author

slomp commented Aug 1, 2023

The puzzling thing is that the tests passed in the corresponding PR branch before getting merged to main.

@abadams
Copy link
Member

abadams commented Aug 1, 2023

I don't think they did, I think they failed at an earlier step due to a network issue or something and the tests never actually ran: https://buildbot.halide-lang.org/master/#/builders/148/builds/42

@steven-johnson
Copy link
Contributor

Yeah, there were a few days when the network to WinBot2 was superflaky

@slomp
Copy link
Contributor Author

slomp commented Aug 2, 2023

I ran the tests locally here, and they all passed, except for a few "Error: Did not understand Halide target d3d12compute" (which I presume is intentional):
correctness__all.txt

However, I had to run them on commit c9bf3b14b578af1fa12e7d1398c711902a9c23d9 (-7/25/2023) because that's the last commit that still works with LLVM15, which is the latest LLVM build that vcpkg offers.

I can try building the latest LLVM and the latest Halide sometime later this week and re-run he correctness tests.

@slomp
Copy link
Contributor Author

slomp commented Aug 2, 2023

Also forgot to mention that one of the tests (correctness_param_map) does not run due to a missing symbol:
image

@slomp
Copy link
Contributor Author

slomp commented Aug 2, 2023

Just ran the tests again -- this time on the current Halide main branch with the latest LLVM on Windows -- and the tests passed (still getting that DLL symbol error on correctness_param_map, though).
correctness__output.txt

ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
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.

4 participants