-
Notifications
You must be signed in to change notification settings - Fork 65
Map types.NoneType return type to LLVM void instead of i8* #847
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
Open
isVoid
wants to merge
5
commits into
NVIDIA:main
Choose a base branch
from
isVoid:fix-void-type-model-845
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
81cb6d2
Map types.NoneType to LLVM void return type instead of i8*
isVoid ca5a210
Document NoneTypeModel shadowing of default_manager's OpaqueModel
isVoid 118bb74
Remove stale NoneType registration from OpaqueModel in cuda_models.py
isVoid 2fffff5
Update NoneTypeModel docstring to reflect cuda_models.py removal
isVoid 62e1220
checkpointing
isVoid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -630,5 +630,112 @@ def kernel(x): | |
| np.testing.assert_equal(x, 42) | ||
|
|
||
|
|
||
| @skip_on_cudasim("Data model inspection unsupported in the simulator") | ||
| class TestNoneTypeModel(CUDATestCase): | ||
| """Tests for the NoneTypeModel data model (issue #845). | ||
|
|
||
| Verifies that types.void / types.NoneType uses ir.VoidType() as the | ||
| LLVM return type, fixing ABI mismatches when linking with external | ||
| C/C++ device code via LTO. | ||
| """ | ||
|
|
||
| def test_nonetype_model_return_type_is_void(self): | ||
| from llvmlite import ir | ||
| from numba.cuda.descriptor import cuda_target | ||
|
|
||
| dm = cuda_target.target_context.data_model_manager | ||
| model = dm.lookup(types.void) | ||
|
|
||
| self.assertEqual(type(model).__name__, "NoneTypeModel") | ||
| self.assertIsInstance(model.get_return_type(), ir.VoidType) | ||
|
|
||
| def test_nonetype_model_value_type_is_opaque_ptr(self): | ||
| from llvmlite import ir | ||
| from numba.cuda.descriptor import cuda_target | ||
|
|
||
| dm = cuda_target.target_context.data_model_manager | ||
| model = dm.lookup(types.void) | ||
| vt = model.get_value_type() | ||
|
|
||
| self.assertIsInstance(vt, ir.PointerType) | ||
|
|
||
| def test_cabi_void_device_function_signature(self): | ||
| consume = cuda.declare_device( | ||
| "consume", "void(int32)", link=consume_cabi_cu, abi="c" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This by itself does not guarantee that it was inlined. In fact that code used to work properly before. Could we add tests that verify ir signature both for callee and caller |
||
| ) | ||
|
|
||
| @cuda.jit | ||
| def kernel(r, x): | ||
| i = cuda.grid(1) | ||
| if i < len(r): | ||
| consume(x[i]) | ||
| r[i] = x[i] * 3 | ||
|
|
||
| x = np.arange(10, dtype=np.int32) | ||
| r = np.empty_like(x) | ||
| kernel[1, 32](r, x) | ||
|
|
||
| irs = kernel.inspect_llvm() | ||
|
|
||
| # Pattern to match 'call void @consume(i32 ...)' in the LLVM IR | ||
| pat = re.compile(r'call void @"?consume"?\s*\(\s*i32\b') | ||
| matched = any(pat.search(ir) for ir in irs.values()) | ||
| self.assertTrue( | ||
| matched, | ||
| "Did not find the expected 'call void @consume(i32 ...)' pattern in LLVM IR", | ||
| ) | ||
|
|
||
| def test_void_device_function_numba_abi(self): | ||
| @cuda.jit(device=True) | ||
| def noop(): | ||
| pass | ||
|
|
||
| @cuda.jit | ||
| def kernel(r): | ||
| i = cuda.grid(1) | ||
| if i < len(r): | ||
| noop() | ||
| r[i] = 42 | ||
|
|
||
| r = np.zeros(10, dtype=np.int32) | ||
| kernel[1, 32](r) | ||
|
|
||
| callee_irs = noop.inspect_llvm() | ||
| caller_irs = kernel.inspect_llvm() | ||
|
|
||
| callee_pat = re.compile(r'define\b[^@]*\bvoid\s+@"[^"]*noop[^"]*"') | ||
| callee_matched = any( | ||
| callee_pat.search(ir) for ir in callee_irs.values() | ||
| ) | ||
| self.assertTrue( | ||
| callee_matched, | ||
| "Device function 'noop' should be defined with void return type", | ||
| ) | ||
|
|
||
| caller_pat = re.compile(r'call\s+void\s+@"[^"]*noop[^"]*"\s*\(') | ||
| caller_matched = any( | ||
| caller_pat.search(ir) for ir in caller_irs.values() | ||
| ) | ||
| self.assertTrue( | ||
| caller_matched, | ||
| "Kernel should call 'noop' with void return type", | ||
| ) | ||
|
|
||
| def test_void_device_function_with_side_effect(self): | ||
| @cuda.jit(device=True) | ||
| def write_value(arr, idx, val): | ||
| arr[idx] = val | ||
|
|
||
| @cuda.jit | ||
| def kernel(arr): | ||
| i = cuda.grid(1) | ||
| if i < len(arr): | ||
| write_value(arr, i, i * 10) | ||
|
|
||
| arr = np.zeros(10, dtype=np.int32) | ||
| kernel[1, 32](arr) | ||
| np.testing.assert_equal(arr, np.arange(10, dtype=np.int32) * 10) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to return a placeholder value type because void type cannot be used in declaration for argument type. This also aligns with how the rest of Numba internally passes arguments around.