mul-hi: real high-half multiply, and stop lying when we can't - #138
Merged
Conversation
Was emitting a store of an undefined register and exiting 0. PTX has mul.hi at both widths, verified on a 4060 Ti: umul64hi(0x123456789,0xfedcba987)=18.
Adds the opcode across gfx9/10/11/12 (objdump-verified) and a real 32-bit high-half multiply, checked on emulated RDNA3/4. __umul64hi needs an i64 result the backend can't hold yet, so it fails the compile instead of silently emitting nothing. Same guard now catches device calls.
Zaneham
added
backend: AMD
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
1 participant
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.
__umulhi/__umul64hi were quietly broken. NVIDIA emitted a store of an
undefined register and exited 0; AMD emitted no multiply at all, also exit 0.
Both are the silent-wrong-answer case, and mul-hi is on the ZK field-arith path.
NVIDIA now emits real mul.hi at both widths (PTX has it natively). Checked on a
4060 Ti: umul64hi(0x123456789, 0xfedcba987) = 18, matches the __int128 answer.
AMD gets v_mul_hi_u32 wired across gfx9/10/11/12 (each encoding confirmed with
llvm-objdump) and a real 32-bit __umulhi, verified on emulated RDNA3/4. The
64-bit __umul64hi needs an i64 result the backend can't hold yet (VGPR pairs +
dwordx2 store + regalloc alignment), so it refuses the compile rather than
emitting nothing. The same error path now also catches device calls, which were
printing an error but still exiting 0.
Also wires __umulhi (32-bit) through sema + the lowerer, and moves nv_rt's
device banner to stderr so stdout is just results.