[DSL] Preserve logical signedness of unsigned integer dtypes - #920
Open
Arist12 wants to merge 1 commit into
Open
[DSL] Preserve logical signedness of unsigned integer dtypes#920Arist12 wants to merge 1 commit into
Arist12 wants to merge 1 commit into
Conversation
A torch.uint8 tensor arrived in a kernel with element type Int8, because both Int8.ir_type and Uint8.ir_type lower to a signless i8 and the DSL wrapper was rebuilt from that signless type. Every operation whose signed and unsigned forms differ then took the signed one: widening 0xF0 gave -16 instead of 240, and integer-to-float sign-extended. No error, no warning, just wrong values for every byte with the high bit set. Carry signedness where it can be represented and drop it where it cannot. Storage descriptors spell it -- !fly.memref<ui8>, !fly.ptr<ui8> -- so the DSL can rebuild the logical dtype, while SSA values stay signless because arith accepts nothing else and encodes signedness in the opcode instead. toSSAValueType maps one to the other at the load boundary, with the same function mirrored on the Python side. Width-1 integers are normalised to unsigned: i1 has no bit to spare for a sign, so reading it as signed makes true compare as -1 and inverts every ordering. Signed dtypes are unaffected: they were already signless and stay that way. The set of accepted boundary dtypes is unchanged. Fixes ROCm#701 Signed-off-by: Arist12 <ykzhang@cs.wisc.edu>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
A
torch.uint8tensor arrives in a kernel with element typeInt8. BothInt8.ir_typeandUint8.ir_typelower to a signlessi8, and the DSL wrapper is rebuilt from that signless type, so the logical signedness is lost at the boundary. Every operation whose signed and unsigned forms differ then takes the signed one — widening0xF0gives-16instead of240, and integer-to-float sign-extends. There is no error and no warning; the values are simply wrong for every byte with the high bit set.This carries signedness where it can be represented and drops it where it cannot: storage descriptors spell it (
!fly.memref<ui8>,!fly.ptr<ui8>) so the DSL can rebuild the logical dtype, while SSA values stay signless becausearithaccepts nothing else and encodes signedness in the opcode.toSSAValueTypemaps between them at the load boundary, mirrored by_ssa_value_ir_typeon the Python side.Fixes #701.
Motivation
#701 was filed alongside #700 ("Align tensor integer storage with pointer types"). On merging that, its author noted:
torch.uint8is how this repository carries packed MXFP4/MXFP6 codes and E8M0 scales, so "the top half of the byte range computes wrong" sits on a path that handles real data.Changes
lib/Dialect/Fly/Utils/TypeUtils.{h,cpp}(new):toSSAValueType, mapping a storage element type to the value type a load produces.lib/Bindings/Python/DLTensorAdaptor.h: unsigned DLPack dtypes map toui{N}rather than signlessi{N}. One-bit unsigned stays signless, as booleans were.lib/Dialect/Fly/IR/FlyOps.cpp,lib/Dialect/Fly/Utils/PointerUtils.cpp: loads infer a signless result from a possibly-unsigned storage element type.python/flydsl/expr/numeric.py,primitive.py,derived.py,compiler/jit_argument.py:storage_ir_typevsir_type, and fragments/views/loads rebuild the DSL dtype from the storage type instead of from the signless result.python/flydsl/expr/utils/arith.py: width-1 integers are always the unsigned{0, 1}domain.i1has no bit to spare for a sign, so reading it as signed makestruecompare as-1and inverts every ordering.Signed dtypes are unaffected — they were already signless and stay that way. The set of accepted boundary dtypes is unchanged;
torch.uint16/uint32/uint64remain unregistered as before, thoughstorage_ir_typeis width-generic.Testing
tests/language/test_unsigned_semantics.pychecks the emitted IR without a device: storage-vs-SSA types, which opcode each of divide, remainder, shift and comparison selects, mixed-signedness promotion, vector element signedness, thetorchdtype table, the C++ DLPack adaptor arm, and that the wider unsignedtorchdtypes are still rejected.tests/unit/test_unsigned_tensor_dtype.pyruns on device: widening and int-to-float at every width, shifts, ordering,recast_iter,make_fragment_like, scalar and vector loads, the raw-pointer and tensor boundaries, thei1predicate case, and signed-dtype regressions.0x00, 0x01, 0x7F, 0x80, 0xC0, 0xFE, 0xFF, 0x87), so a signed/unsigned mix-up cannot alias back onto the right answer.RUN_TESTS_FULL=1 scripts/run_tests.shon this branch: 2324 passed, 0 failed (2248 on an unpatched tree; the difference is the new tests). Multi-GPU tests excluded — they need more than one visible device.black/ruffandclang-format-18clean on the changed files.Dependencies
Breaking Changes
None intended. A kernel that reads a
torch.uint8tensor now seesUint8where it previously sawInt8, which is the bug being fixed; operations on genuinely signed dtypes are unchanged.