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

sv_fpga_start_buffer_to_cl and sv_fpga_start_cl_to_buffer for unprintable chars #406

Closed
da-steve101 opened this issue May 31, 2018 · 9 comments

Comments

@da-steve101
Copy link

I have been using sv_fpga_start_buffer_to_cl and sv_fpga_start_cl_to_buffer in simulation.
These functions only work under the assumption that there is no "\0" char in the string.
When using raw data values as input with a '0' value in the string all subsequent characters are set to 0 and hence have incorrect input passed into the design in simulation.

@AWShimasajj
Copy link
Contributor

Hi da-steve101,

Can you please tell us which simulator are you using?

Thanks,
AWShimasajja

@AWShimasajj
Copy link
Contributor

Hi da-steve101,

We use the null character to indicate end of string. I have tried some examples with raw data which has a '0' value in the string passed to sv_fpga_start_buffer_to_cl and did not see the issue you are talking about. If you can give us more information about how you are using this function and the Simulator you are using, we will be able to assist you better.

Thanks,
Hima Sajja

@da-steve101
Copy link
Author

Hi, I am just using vivado as the simulator from the fpga dev image at commit 7dc2bee.
When I change the following:

diff --git a/hdk/cl/examples/cl_dram_dma/software/runtime/common_dma.c b/hdk/cl/examples/cl_dram_dma/software/runtime/common_dma.c
index 909e8d9..76d6850 100644
--- a/hdk/cl/examples/cl_dram_dma/software/runtime/common_dma.c
+++ b/hdk/cl/examples/cl_dram_dma/software/runtime/common_dma.c
@@ -43,8 +43,8 @@ rand_string(char *str, size_t size)
     }
 
     for(i = 0; i < size; ++i) {
-        unsigned int key = rand() % (sizeof charset - 1);
-        str[i] = charset[key];
+        // unsigned int key = rand() % (sizeof charset - 1);
+        str[i] = i % 256;
     }
 
     str[size-1] = '\0';

This causes the cl_dram_dma/test_dram_dma_hwsw_cosim to fail with the bytes read just being '0'
I think the cause is casting to and from a 'string' when it reads only until first '0' char and fills the rest of the buffer with '0'. I dont have access to other simulators so I can't say if it is an issue with them.

@AWShimasajj
Copy link
Contributor

Hi da-steve101,

The System Verilog DPI functions string take string as input which is why you are seeing this issue when you change the code to pass integers. The two consecutive zeros most likely is interpreted as a NULL character and subsequent numbers are ignored. If the numbers are passed in the string format as well, you will not see the same issue. If you want to pass the integer instead of string these functions would have to be edited to pass strings as well. You can find these functions in hdk/common/verif/include/sh_dpi_tasks.svh. Please let us know if there is anything else we can do to help you.

Thanks,
Hima Sajja

@da-steve101
Copy link
Author

Hey Sajja,

The System Verilog DPI functions string take string as input which is why you are seeing this issue when you change the code to pass integers.

I am just changing the generated data. This is in the function rand_string that initialises the write_buffer. I haven't changed any interfacing code between C and verilog.

The two consecutive zeros most likely is interpreted as a NULL character and subsequent numbers are ignored.

I do not think I am sending two consecutive zeros, but i see no reason why this should not be valid input either.

If the numbers are passed in the string format as well, you will not see the same issue.

Are you suggesting that I pass in as characters '0', '1' etc to represent numbers? That would be horribly inefficient.

If you want to pass the integer instead of string these functions would have to be edited to pass strings as well

I am aware that I can change my code ( and have already done so ) but I raised this issue as I think these functions should be be defined as accepting an array of bytes of 'buffer_size' as nothing about them suggests that they only work with printable chars and was time consuming to debug.

@awsbala
Copy link

awsbala commented Jun 12, 2018

Hello Steve,
You are trying to assign an integer to a char. Please note that data in Char is ASCII and you need to add 48 or '0' to your code to convert the integer to printable ASCII character.

str[i] = i % 256 + '0' ; or str[i] = i % 256 + 48;

This will help you to print the characters.

Please let us know if you have issues.

Regards
Bala

@da-steve101
Copy link
Author

I am passing in raw data values eg) floating point etc. I have modified my code to do this already.
The reason I opened this issue was i tried to pass these values through in the simulator which led to strange answers.
It was annoying to debug and as these functions are defined in the common lib rather than the example I thought they should be general and allow for arbitrary values.
I was under the impression this was a bug but if you disagree and believe these functions should only be used for printable strings then I will close this issue.

@awsbala
Copy link

awsbala commented Jun 13, 2018

Hello Steve,
You are correct. It works only for strings since the argument is of type "char".
Below is the template for the function.
rand_string(*char str, size_t size)

and to make integers printable, you need to convert them to "char".

Regards
Bala

@Licheng-Guo
Copy link

I encounter this problem too. It took me days to identify the bug. It's really annoying. Still don't know how to remedy the bug...
Could you please tell me how to change the code to allow any data to be transferred by DMA? Thanks a lot

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

No branches or pull requests

4 participants