calyx
calyx copied to clipboard
Remove `timeout` argument from Xilinx backend
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!
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.
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?
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!
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.
Thanks for confirming @sgpthomas! @zzy666666zzy we'll try to remove this extra argument in an upcoming PR.
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.
It can in fact be an option in our AXI generator from https://github.com/cucapra/calyx/issues/1603