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

Remove timeout argument from Xilinx backend #1631

Open
zzy666666zzy opened this issue Jul 23, 2023 · 7 comments
Open

Remove timeout argument from Xilinx backend #1631

zzy666666zzy opened this issue Jul 23, 2023 · 7 comments
Labels
C: FPGA Changes for the FPGA backend S: Available Can be worked upon

Comments

@zzy666666zzy
Copy link

Hi @rachitnigam ,

After solving #1470 and #1575, we generated .xclbin from our gemm kernel referring to Calyx's tutorial.

https://docs.calyxir.org/fud/xilinx.html

I am invoking the kernel in an OpenCL host code on Petalinux platform. Afterward, I refer to this reference design

https://github.com/Xilinx/Vitis_Accel_Examples/tree/master/rtl_kernels/rtl_vadd

to cross-compile the host OpenCL code and export the host executable and xclbin to the Petalinux. But I got errors from clSetKernelArg() saying that

ERROR: clSetKernelArg() for kernel "Toplevel", argument index 0.
Error calling err = krnl_gemm.setArg(0, buffer_w), error code is: -51

I wonder is there any reference design from Calyx about how to write host code and how to invoke the kernel using XRT or OpenCL?
I attached my necessary files in case you have any pertinent solutions to it. Thanks a lot!

kernel_xml.txt
host_cpp.txt

@zzy666666zzy
Copy link
Author

Problem solved.

The reason is that I didn't set the timeout argument with index 0.
I randomly assign a number, say 10, to timeout argument in my host OpenCL code and got the correct result.

I am wondering is timeout is an important parameter or not. Because it is not associated with essential logics in generated Toplevel verilog code.

@rachitnigam
Copy link
Contributor

Yeah, looks like our harness for running things on the FPGA also does not use the timeout argument but the kernel.xml file is generated to expect it? Not sure if Vivado requires it specifically or not. @sampsyo any clue?

@sampsyo
Copy link
Contributor

sampsyo commented Jul 25, 2023

I have actually always been a little confused about what timeout was supposed to do. Is it a cycle limit for the accelerator invocation, so "runaway" Calyx programs nonetheless complete the XRT transaction? That seems most likely to me, and perhaps it was just never implemented!

No pressure at all, but if @sgpthomas has a recollection of what this was for, we would be grateful!

@sgpthomas
Copy link
Collaborator

It was a long time ago, but I think you're right @sampsyo. I think I used it during testing to handle runaway programs. I assume that I just never published my use of the timeout parameter, but accidentally left it in the code. I think it's probably safe to remove if it's no longer needed.

@rachitnigam rachitnigam changed the title Data transfer between OpenCL host and kernel (Petalinux) Remove timeout argument from Xilinx backend Jul 25, 2023
@rachitnigam
Copy link
Contributor

Thanks for confirming @sgpthomas! @zzy666666zzy we'll try to remove this extra argument in an upcoming PR.

@sampsyo
Copy link
Contributor

sampsyo commented Jul 25, 2023

Indeed; many thanks, @sgpthomas! That makes lots of sense to me. Seems like it wouldn't exactly hurt to have such a timeout, but given that this stuff is basically working these days (i.e., indefinite hangs are not the main problem we're facing with AXI/Xilinx stuff), we can shed it instead of implementing it.

@rachitnigam
Copy link
Contributor

It can in fact be an option in our AXI generator from #1603

@rachitnigam rachitnigam added S: Available Can be worked upon C: FPGA Changes for the FPGA backend labels Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: FPGA Changes for the FPGA backend S: Available Can be worked upon
Projects
None yet
Development

No branches or pull requests

4 participants