Skip to content

assemble: fix broadcast ratio calculation for sub-64-bit vector operands - #320

Open
agourakis82 wants to merge 1 commit into
netwide-assembler:masterfrom
agourakis82:fix/vcvtph2pd-broadcast-1to2
Open

agourakis82 wants to merge 1 commit into
netwide-assembler:masterfrom
agourakis82:fix/vcvtph2pd-broadcast-1to2

Conversation

@agourakis82

Copy link
Copy Markdown

Fixes #263.

Summary

In asm/assemble.c, get_broadcast_num() previously calculated the number of broadcast elements with:

brcast_num = ((opsize / (BITS64 >> SIZE_SHIFT)) * (BITS64 / brsize))
    >> (opsize > (BITS64 >> SIZE_SHIFT));

When the operand size is smaller than BITS64 (such as xmmrm32 in VCVTPH2PD or VCVTPH2QQ with xmm destination, where opsize is BITS32 which equals 4, and BITS64 >> SIZE_SHIFT equals 8), the integer division opsize / 8 evaluated to 0.

As a consequence, brcast_num became 0, causing valid {1to2} broadcast instructions like:

vcvtph2pd xmm0, [rcx]{1to2}

to fail with:

error: mismatch in the number of broadcasting elements

Fix

Replace the calculation with an exact bit-size lookup table indexed by ilog2_32(opsize), accurately computing the broadcast ratio for all operand sizes from BITS32 through BITS512.

Verified assembling vcvtph2pd xmm0, [rcx]{1to2}, vcvtph2pd ymm0, [rcx]{1to4}, and vcvtph2pd zmm0, [rcx]{1to8}.

Fixes netwide-assembler#263.

get_broadcast_num() previously computed:
    brcast_num = ((opsize / (BITS64 >> SIZE_SHIFT)) * (BITS64 / brsize))
        >> (opsize > (BITS64 >> SIZE_SHIFT));

When the memory operand size is smaller than BITS64 (such as xmmrm32
in VCVTPH2PD or VCVTPH2QQ with xmm destination, where opsize is BITS32 = 4
and BITS64 >> SIZE_SHIFT is 8), integer division opsize / 8 truncated to 0,
causing NASM to report:
    error: mismatch in the number of broadcasting elements
when assembling instructions with {1to2} broadcast like:
    vcvtph2pd xmm0, [rcx]{1to2}

Replace the broken division formula with an exact bit-size lookup
table indexed by ilog2_32(opsize), correctly supporting broadcast
ratios for all operand sizes from BITS32 to BITS512.
@agourakis82

Copy link
Copy Markdown
Author

I rebuilt NASM locally from this PR head and ran the targeted AVX512-FP16 broadcast-related instruction tests.

HEAD=d247c04c3ad33a861ef22077cf1401cd30729d5e
./nasm -v
NASM version 3.02 compiled on Sep 29 2026

sh autogen.sh
sh configure
make -j4

python3 ./tools/travis/nasm-t.py -d ./travis/insns/vcvtph2pd --nasm ./nasm run --stop=n
# vcvtph2pd.1 .. vcvtph2pd.5 PASS

python3 ./tools/travis/nasm-t.py -d ./travis/insns/vcvtph2qq --nasm ./nasm run --stop=n
# vcvtph2qq.1 .. vcvtph2qq.5 PASS

python3 ./tools/travis/nasm-t.py -d ./travis/insns/vcvttph2qq --nasm ./nasm run --stop=n
# vcvttph2qq.1 .. vcvttph2qq.5 PASS

git diff --check origin/master...HEAD
# clean

I did not run the full Travis matrix locally, but the directly related conversion/broadcast instruction fixtures pass with the rebuilt assembler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcvtph2pd with xmmreg errors on broadcast

1 participant