Skip to content

aarch64: add missing F16 and F128 rules for scalar_size in ISLE - #14238

Merged
cfallin merged 3 commits into
bytecodealliance:mainfrom
dsecurity49:fix-aarch64-scalar-size-f16-f128
Sep 3, 2026
Merged

aarch64: add missing F16 and F128 rules for scalar_size in ISLE#14238
cfallin merged 3 commits into
bytecodealliance:mainfrom
dsecurity49:fix-aarch64-scalar-size-f16-f128

Conversation

@dsecurity49

@dsecurity49 dsecurity49 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Add missing F16 and F128 rules to scalar_size in inst.isle, as well as their corresponding models in prelude.isle.

Without these, rules that pass f16 or f128 types to scalar_size (like fma.f16) trigger an ISLE panic because scalar_size is a total term.

@dsecurity49
dsecurity49 requested a review from a team as a code owner August 30, 2026 16:11
@dsecurity49
dsecurity49 requested review from cfallin and removed request for a team August 30, 2026 16:11
@dsecurity49
dsecurity49 force-pushed the fix-aarch64-scalar-size-f16-f128 branch from b05b625 to 4572233 Compare August 30, 2026 16:12
@dsecurity49
dsecurity49 force-pushed the fix-aarch64-scalar-size-f16-f128 branch from 4572233 to 2018875 Compare August 30, 2026 16:29
@github-actions github-actions Bot added cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. labels Aug 30, 2026
@@ -0,0 +1,8 @@
test compile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually have compile-tests with the precise-output flag to show (and lock down) the actual compilation output. Could you include that here? And could you add something for f128 as well if aarch64 has lowerings for it (I don't remember if it does)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the f16 test to precise-output (emits fmadd h0, h0, h1, h2 as expected).

For f128: there are no native 128-bit float instructions on AArch64, so it would need softfloat libcalls (__addtf3, __fmatf3, etc.) which cranelift doesn't have on any backend yet, even x64 only handles f128const.For now I kept the scalar_size rule for F128 since it prevents a confusing panic, but happy to drop it if you'd prefer to keep this minimal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After checking further, keeping the F128 rule would be misleading,f128 arithmetic hits unexpected scalar FP operand size (size128)deeper in the backend anyway, so the scalar_size rule for it is effectively dead code. I've removed it and the corresponding model from prelude.isle, keeping only the F16 fix.

@cfallin
cfallin added this pull request to the merge queue Sep 3, 2026
Merged via the queue into bytecodealliance:main with commit e4b3043 Sep 3, 2026
54 checks passed
@dsecurity49
dsecurity49 deleted the fix-aarch64-scalar-size-f16-f128 branch September 3, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants