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

Add Error String to Call Logging Exit #25

Closed
bashbaug opened this issue Aug 24, 2018 · 3 comments
Closed

Add Error String to Call Logging Exit #25

bashbaug opened this issue Aug 24, 2018 · 3 comments

Comments

@bashbaug
Copy link
Contributor

Observed Behavior

CallLogging currently doesn't log any error information. If ErrorLogging is enabled, it will log when an error occurs, but on a separate line:

>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0717BDA8, index = 4, size = 4
ERROR! clSetKernelArg returned CL_INVALID_ARG_INDEX (-49)
<<<< clSetKernelArg

Desired Behavior

Consider logging the error information as part of CallLogging when the function exist. Something like:

>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0717BDA8, index = 4, size = 4
ERROR! clSetKernelArg returned CL_INVALID_ARG_INDEX (-49)
<<<< clSetKernelArg returned CL_INVALID_ARG_INDEX

It could be useful to print the error information even if there were no errors, for example:

>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0717BDA8, index = 3, size = 4
<<<< clSetKernelArg returned CL_SUCCESS

Steps to Reproduce

Set CallLogging and ErrorLogging and run a program that generates an error.

@bashbaug
Copy link
Contributor Author

@trbauer I have a prototype of this in a branch, here: https://github.com/bashbaug/opencl-intercept-layer/tree/call_logging_error_status

Some example output is:

>>>> clCreateKernel: program = 06979F38, kernel_name = GenerateJuliaSet
<<<< clCreateKernel: created 0462B3F0 -> CL_SUCCESS
>>>> clCreateBuffer: flags = CL_MEM_WRITE_ONLY | CL_MEM_ALLOC_HOST_PTR (12), size = 4194304, host_ptr = 00000000
<<<< clCreateBuffer: created 06B53790 -> CL_SUCCESS
>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0462B3F0, index = 0, size = 4, value = 06B53790
<<<< clSetKernelArg -> CL_SUCCESS
>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0462B3F0, index = 1, size = 4, value = 00000200
<<<< clSetKernelArg -> CL_SUCCESS
>>>> clSetKernelArg( GenerateJuliaSet ): kernel = 0462B3F0, index = 4, size = 4
ERROR! clSetKernelArg returned CL_INVALID_ARG_INDEX (-49)
<<<< clSetKernelArg -> CL_INVALID_ARG_INDEX

and:

>>>> clEnqueueNDRangeKernel( GenerateJuliaSet ): queue = 066C1490, kernel = 0462B3F0, global_work_size = < 512, 512 >, local_work_size = < NULL >
<<<< clEnqueueNDRangeKernel -> CL_SUCCESS
>>>> clEnqueueMapBuffer: [ map count = 0 ] queue = 066C1490, buffer = 06B53790, blocking, map_flags = CL_MAP_READ (1), offset = 0, cb = 4194304
<<<< clEnqueueMapBuffer: [ map count = 1 ] mapped 086B0000 -> CL_SUCCESS
>>>> clEnqueueUnmapMemObject: [ map count = 1 ] queue = 066C1490, memobj = 06B53790, mapped_ptr = 086B0000
<<<< clEnqueueUnmapMemObject: [ map count = 1 ] -> CL_SUCCESS

I'm still not sure if I prefer "created" or "returned" for the APIs that create something. I originally had e.g. "returned CL_SUCCESS" and didn't like the double return for the creation APIs so I switched to a different verb, but after switching to the arrow "->" syntax I think I could switch it back now. Let me know if you have a preference one way or the other. It's relatively easy to switch to a different syntax now that the plumbing is in place.

@trbauer
Copy link
Contributor

trbauer commented Aug 27, 2018

I don't have a strong position on this at the moment. In the other issue (syntactic consistency), I suggested the symbol " => ". I was a bit torn on "returned " 100% vs "created" for APIs that are allocating things (introducing names into the stream).

I could go either way. If ERROR_LOGGING implies CALL_LOGGING we could retire that feature since it's sort of redundant. Maybe keep it but move it out of the core set of controls (have an extended set for deprecated or obscure things).

@bashbaug
Copy link
Contributor Author

I'm going to go ahead and merge the fix for this issue since I suspect the error information may be desired for #28.

I've stuck with the arrow "->" syntax for now, but we can easily change it to something else like "=>" if we prefer, either as part of the fix for #28 or sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants