-
Notifications
You must be signed in to change notification settings - Fork 252
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
Cl get kernel arg info #91
Conversation
.. and fix the end of test_clCreateKernelsInProgram.c
- TODO tests & handle missing arg names gracefully
- SPIR has one extra and one optional metadata.
- arg_name only present with a compiler switch, so clear the memory with memset
I have rebased it onto pull request 92, and still need to add a SPIR variant of the test |
- in case the bitcode is SPIR, arg_name might be NULL
Looks good to me & passes tests at this point. But won't merge if there is more coming? |
- and test various bits in lib/CL/clGetKernelArgInfo.c for presence of argument metadata
Tests bitcode files produced by clang both with and without the "-cl-kernel-arg-info" argument.
I enhanced the test code to also test SPIR bitcode, and made the code a bit more robust; i think all of the TODOs for this branch are done, so it should be ready. |
OK, I'll merge if tests pass. |
Putting this here also. Most of the previous tests now fail with:
|
Debian Stable, LLVM 3.4.1, x86_64. It seems valid: llvm::MDString *kernel_prototype = llvm::cast(kernel_iter->getOperand(0)); You are casting llvm::Function to llvm::MDString. I wonder how it works for you but not for me. See Workgroup.cc:634 -- it's the same MD there, right? |
I changed the cast to llvm::Function. I have no idea how could it work before.. |
OK, that cast crash is now gone, and other tests have passed so far, but the new test failed:
|
The issue is exposing another issue with the binary loading interface. The (old) problem of linking together LLVM::Modules with the same global type names (in this case %opencl.image2d_t) appears to be not fixed when having both source kernels and binary (SPIR) kernels in the same program. When LLVM bitcode linker links together two Modules that use different LLVMContext and have the same named type, it creates a new type %opencl.image2d_t.N with incremented N in order to avoid accidentally wrongly mergins types (it cannot tell if the other Module really meant the same type). This causes the check that figures out if the parameter is a image to fail. I do not think we can just extend the check to match opencl.image2d_t.* because that caused some other problems which I cannot remember (at least when we actually introduce the struct that replaces this opaque type hand there might be some problems). Thus, the way this has been working is to use the same singleton LLVMContext for all bitcode (Module) operations. LLVMContext keeps book of the types so when we introduce two Modules in the same LLVMContext that use the same type (name), they should merge to the same type. For some reason this now fails, which probably means that the same LLVMContext is not propagated to some IR loading functions. You can see that it works if you only load from the SPIR in that test, but not if you first load from a source and then from SPIR. Unfortunately I do not have now time to debug this further, but because this is a bug unrelated to the feature at hand, I suggest you somehow split the test to two (one for src and one for SPIR) to circumvent the issue and still test both cases so we get your work merged. I opened issue #95 for the actual problem. Of course, if you feel like digging into the LLVM and pocl more, you could try to figure it out yourself, because (due to holidays) I do not know when I can get into it. |
Please review, thanks