-
Notifications
You must be signed in to change notification settings - Fork 97
Cudastf #794
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
base: main
Are you sure you want to change the base?
Cudastf #794
Changes from 22 commits
d6dc01d
245b20f
9b35ec8
7d298d4
154b3f9
c8ef988
52b18c9
d726b10
5e7576c
1373699
92e7204
a608f3f
b062577
bbf9abc
3e831ea
702fe79
5bfe21e
f407256
221599c
7a5bb6c
0c2432f
3ae267b
6a75794
f1facca
0199e75
39b16f4
9b7c4b0
f9e09f1
6c9a791
a1efd1c
6437eab
bbb9aae
e13c9b6
7244399
66f6850
89e2a43
973886b
92885e7
8607840
14e0985
92e04d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,9 @@ | ||
{ | ||
"packages": { | ||
"CCCL": { | ||
"version": "2.7.0-rc2", | ||
"git_shallow": true, | ||
"version": "2.8.0", | ||
"git_url": "https://github.com/NVIDIA/cccl.git", | ||
"git_tag": "10e915ac7b79a1ab3b9d7a795c621b47b122f513" | ||
"git_tag": "36e27f7c1074010eefaab64d387ff6663569e065" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,12 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char **argv) | |
{ | ||
MATX_ENTER_HANDLER(); | ||
using complex = cuda::std::complex<float>; | ||
#if 0 | ||
cudaExecutor exec{}; | ||
#else | ||
stfExecutor exec{}; | ||
auto ctx = exec.getCtx(); | ||
#endif | ||
|
||
index_t signal_size = 1ULL << 16; | ||
index_t filter_size = 16; | ||
|
@@ -117,7 +122,11 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char **argv) | |
// Perform the FFT in-place on both signal and filter | ||
for (int i = 0; i < iterations; i++) { | ||
if (i == 1) { | ||
#if 0 | ||
cudaEventRecord(start, stream); | ||
#else | ||
cudaEventRecord(start, ctx.task_fence()); | ||
#endif | ||
} | ||
(sig_freq = fft(sig_time, filtered_size)).run(exec); | ||
(filt_freq = fft(filt_time, filtered_size)).run(exec); | ||
|
@@ -129,18 +138,30 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char **argv) | |
|
||
} | ||
|
||
#if 0 | ||
cudaEventRecord(stop, stream); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually we should mask these events behind the executor as well so the timing is the same regardless of the executor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this makes it look like the code is very different for both executors but that timing is the sole reason especially if finalize is moved to the dtor |
||
#else | ||
cudaEventRecord(stop, ctx.task_fence()); | ||
#endif | ||
exec.sync(); | ||
cudaEventElapsedTime(&separate_ms, start, stop); | ||
|
||
for (int i = 0; i < iterations; i++) { | ||
if (i == 1) { | ||
cudaEventRecord(start, stream); | ||
#if 0 | ||
cudaEventRecord(start, stream); | ||
#else | ||
cudaEventRecord(start, ctx.task_fence()); | ||
#endif | ||
} | ||
(sig_freq = ifft(fft(sig_time, filtered_size) * fft(filt_time, filtered_size))).run(exec); | ||
} | ||
|
||
|
||
#if 0 | ||
cudaEventRecord(stop, stream); | ||
#else | ||
cudaEventRecord(stop, ctx.task_fence()); | ||
#endif | ||
exec.sync(); | ||
cudaEventElapsedTime(&fused_ms, start, stop); | ||
|
||
|
@@ -153,7 +174,11 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char **argv) | |
(time_out = conv1d(sig_time, filt1, matxConvCorrMode_t::MATX_C_MODE_FULL)).run(exec); | ||
|
||
exec.sync(); | ||
|
||
|
||
#if 1 | ||
ctx.finalize(); | ||
#endif | ||
|
||
// Compare signals | ||
for (index_t b = 0; b < batches; b++) { | ||
for (index_t i = 0; i < filtered_size; i++) { | ||
|
@@ -172,4 +197,4 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char **argv) | |
|
||
CUDA_CHECK_LAST_ERROR(); | ||
MATX_EXIT_HANDLER(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is
finalize
used for vssync
? Could you hide the context in the executor so the user doesn't need it, and callingexec.sync()
callsfinalize()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finalize terminates everything in the stf context, it waits for asynchronous tasks, deletes internal resources etc... you can only do it once, sync is more equivalent to a ctx.task_fence() which is a non blocking fence (it returns a CUDA stream, and waiting for that stream means everything was done).
I'd like to move finalize to the dtor of the executor, but there are some caveats if you define the executor as a static variable, is this allowed ? The caveat might be some inappropriate unload ordering of CUDA and STF libraries as usual ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I think the destructor is the right place. but does sync() work as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sidelnik is it doing a task fence with a stream sync ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caugonnet , sync() should be calling ctx.task_fence() now. I agree, I think we should place the ctx.finalize() inside the stf executor dtor