[MLIR] MaskedType: core type, constructor, accessors - #22884
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
f33dcd1 to
3ab1d60
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds MLIR backend support for ChangesMaskedType MLIR backend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
vyasr
left a comment
There was a problem hiding this comment.
Generally makes sense. I still don't fully grasp what goes in the lowering file and what goes in the typing file, it seems to me like some aspects of type definitions in the typing file are still relevant for lowering (e.g. the constructor signatures), could you explain the division a bit more?
@vyasr the split maps to the two phase compilation pipeline used by numba (typing/lowering). The typing phase's only job is to determine, for every expression in the UDF, what the resuling type is. This phase emits no code, it just annotates every variable and result with a type. The lowering phase only triggers when typing for the whole blob completes successfully. Its job is to actually emit the code that computes the value. Since this might be dependent on what the input types for that expression actually are, the author needs a registration in the lowering file that associates a procedure for computing that value with a valid set of input types. For |
mroeschke
left a comment
There was a problem hiding this comment.
Overall looks good. Just a few last comments
mroeschke
left a comment
There was a problem hiding this comment.
One comment and the failing typing checks otherwise LGTM. (Feel free to use # type: ignores if the typing is difficult)
0baeca1 to
b248bf3
Compare
vyasr
left a comment
There was a problem hiding this comment.
Very close. I have a few smaller requests then we can merge.
b248bf3 to
756652e
Compare
26171e1 to
d111062
Compare
…d_typing.py Co-authored-by: Vyas Ramasubramani <vyasr@nvidia.com>
numba_cuda_mlir/types.py imports ``bool`` unconditionally, but the vendored numba_cuda.types only defines it for numpy>=2.0, so collection fails on the oldest-deps CI matrix (numpy 1.26). Define the alias at conftest import time. TODO: remove once a numba-cuda-mlir release >0.4.0 fixes the import.
d111062 to
bc6878c
Compare
| # AttributeError is unconditional -- so skip the MLIR backend tests on numpy < 2 | ||
| # instead of erroring at collection. | ||
| # TODO: remove once a numba-cuda-mlir release > 0.4.0 supports numpy < 2.0. | ||
| if np.lib.NumpyVersion(np.__version__) < "2.0.0": |
There was a problem hiding this comment.
Just FYI, #22962 appears close to merging and will raise the NumPy minimum version to 2.0 which should allow you to remove this (likely in a follow up)
|
/merge |
Add a basic MLIR based
MaskedTypeand the ability to express the creation of one inside anumba-cuda-mlirkernel.Part of the MLIR UDF backend stack. Depends on #22766 (plumbing). Adds the MaskedType extension type: parameterized value type, the Masked(value, valid) constructor, the .value / .valid accessors, and the LLVM struct data model. Numeric/boolean value types only. Stacked on: #22766. Tests under tests/private_objects/mlir_backend/.