Skip to content

Support SaturatedToLargestFloat8NormalConversionEXT decoration#3767

Merged
vmaksimo merged 5 commits into
KhronosGroup:mainfrom
vmaksimo:SPV_EXT_float8-decoration
Jun 11, 2026
Merged

Support SaturatedToLargestFloat8NormalConversionEXT decoration#3767
vmaksimo merged 5 commits into
KhronosGroup:mainfrom
vmaksimo:SPV_EXT_float8-decoration

Conversation

@vmaksimo

@vmaksimo vmaksimo commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Add round-trip handling for SPV_EXT_float8's saturating FP-to-FP8 conversion.

At the LLVM IR level it is represented via _sat suffix on the existing __builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT interface. When _sat is present, the resulting OpFConvert is decorated with SaturatedToLargestFloat8NormalConversionEXT.

AI-assisted: Claude Opus 4.7 (commercial SaaS)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the
existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting OpFConvert is decorated with
`SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
@MrSidims

MrSidims commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

At the LLVM IR level it is represented via _sat suffix on the existing __builtin_spirv_ConvertTo<E4M3|E5M2>EXT

Shouldn't it be __builtin_spirv_ClampConvert<Src>To<E4M3|E5M2> instead?

@vmaksimo

vmaksimo commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

At the LLVM IR level it is represented via _sat suffix on the existing __builtin_spirv_ConvertTo<E4M3|E5M2>EXT

Shouldn't it be __builtin_spirv_ClampConvert<Src>To<E4M3|E5M2> instead?

I believe it can be both, wanted to publish this for discussion to choose the preferrable one.

@MrSidims

MrSidims commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

I'd not create instances without necessity :)

vmaksimo added 2 commits June 2, 2026 03:57
Address review feedback on the previous commit: instead of introducing a
`_sat` suffix on `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT`, reuse the
existing `__builtin_spirv_ClampConvert<Src>To<E4M3|E5M2>INTEL` interface
from `SPV_INTEL_fp_conversions`.

When `SPV_EXT_float8` is enabled, those four FP8-result entries are now
emitted as `OpFConvert` decorated with
`SaturatedToLargestFloat8NormalConversionEXT` instead of
`OpClampConvertFToFINTEL`, and round-trip back to the same builtin names.

The reverse translator also rejects modules where this decoration targets
a non-FP8 conversion: per spec it may only decorate
`OpFConvert`/`OpConvertSToF`/`OpConvertUToF` with an `Float8E4M3EXT` or
`Float8E5M2EXT` Result Type, and `spirv-val` does not enforce this yet.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
@vmaksimo

vmaksimo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

CI failures are caused by llc change llvm/llvm-project#200791

@vmaksimo vmaksimo closed this Jun 3, 2026
@vmaksimo vmaksimo reopened this Jun 3, 2026

@MrSidims MrSidims left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fine with me, but if I were you - I'd get someone from Intel to approve :)

@vmaksimo vmaksimo requested a review from YuriPlyakhin June 8, 2026 17:04
Comment thread lib/SPIRV/SPIRVReader.cpp Outdated
Comment thread lib/SPIRV/SPIRVReader.cpp Outdated
Comment thread lib/SPIRV/SPIRVWriter.cpp Outdated
Comment thread lib/SPIRV/SPIRVWriter.cpp Outdated
Comment on lines +5737 to +5741
bool IsSaturatedFP8 =
FPDesc.ConvOpCode == internal::OpClampConvertFToFINTEL &&
(FPDesc.DstEncoding == FPEncodingWrap::E4M3 ||
FPDesc.DstEncoding == FPEncodingWrap::E5M2) &&
BM->isAllowedToUseExtension(ExtensionID::SPV_EXT_float8);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When SPV_INTEL_fp_conversions is enabled, the writer used to emit OpClampConvertFToFINTEL for __builtin_spirv_ClampConvert<...>To<E4M3|E5M2>INTEL. With this PR the writer always rewrites to OpFConvert + SaturatedToLargestFloat8NormalConversionEXT. There's no longer a way to ask the translator to emit OpClampConvertFToFINTEL with an FP8 result type.

Is that intended?

If SPV_EXT_float8 is meant to supersede SPV_INTEL_fp_conversions for FP8 destinations, please call that out in PR description.

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.

Nice catch, I was a bit rush to change the behavior. Restored the behavior for SPV_INTEL_fp_conversions in a577c55 so SPV_EXT_float8 does not supersede

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With this PR the writer always rewrites to OpFConvert + SaturatedToLargestFloat8NormalConversionEXT. There's no longer a way to ask the translator to emit OpClampConvertFToFINTEL with an FP8 result type.

Is it bad?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know, maybe it is good. I was just asking, if it was the intention. :)
@vmaksimo , you might want to verify with @gmlueck and @bashbaug on how we want these 2 extensions to coexist...

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.

My intent is to simplify this very change to have the decoration support from the multi-vendor extension.
I'd prefer to resolve any collisions with the INTEL extension in a separate PR when we clarify our spec changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If SPV_EXT_float8 is meant to supersede SPV_INTEL_fp_conversions for FP8 destinations, please call that out in PR description.

I think we do want SPV_EXT_float8 to supersede SPV_INTEL_fp_conversions. Before @bashbaug went on vacation, he updated the draft SPV_INTEL_fp_conversions to remove the overlapping parts like OpClampConvertFToFINTEL. Therefore, we shouldn't be emitting those instructions.

It would be nice to get this change into the SPRIV Translator before merging intel/llvm#21568, so that we don't have compiled SYCL applications using the outdated OpClampConvertFToFINTEL, etc. instructions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I talked to @YuriPlyakhin offline. If you want to make the change that causes SPV_EXT_float8 to supersede SPV_INTEL_fp_conversions in a separate PR, that's OK with me. I only wanted to say that we should have this as a goal. I don't mind if it happens in a separate PR.

Comment thread test/extensions/INTEL/SPV_INTEL_fp_conversions/spv_intel_fp_conversions.ll Outdated
- Preserve SPV_INTEL_fp_conversions encoding when both extensions are
  enabled: keep emitting OpClampConvertFToFINTEL for the
  ClampConvert<Src>To<E4M3|E5M2>INTEL builtins. The OpFConvert +
  SaturatedToLargestFloat8NormalConversionEXT encoding is emitted only
  when SPV_EXT_float8 is enabled and SPV_INTEL_fp_conversions is not.
- Reader: scope the ClampConvertFToFINTEL rmap rewrite to OpFConvert.
  The decoration is also valid on OpConvertSToF/OpConvertUToF, but
  there is no {Integer, E4M3/E5M2, OpClampConvertFToFINTEL} entry in
  the FP-conversions encoding map; restricting the rewrite avoids an
  rmap assert on integer-source conversions.
- Drop a comment that duplicated the error message text.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
@vmaksimo vmaksimo requested a review from YuriPlyakhin June 9, 2026 14:47
@@ -0,0 +1,56 @@
; Clamping FP-to-FP8 conversions: when SPV_EXT_float8 is enabled, the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
; Clamping FP-to-FP8 conversions: when SPV_EXT_float8 is enabled, the
; Clamping FP-to-FP8 conversions: when SPV_EXT_float8 is enabled and SPV_INTEL_fp_conversions is not, the

@YuriPlyakhin YuriPlyakhin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. one nit.

@vmaksimo vmaksimo merged commit 31301b1 into KhronosGroup:main Jun 11, 2026
9 checks passed
vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Jun 24, 2026
…` decoration (KhronosGroup#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Jun 25, 2026
…` decoration (KhronosGroup#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Jun 26, 2026
…` decoration (KhronosGroup#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
vmaksimo added a commit that referenced this pull request Jun 30, 2026
…` decoration (#3767)

Add round-trip handling for `SPV_EXT_float8`'s saturating FP-to-FP8
conversion.

At the LLVM IR level it is represented via `_sat` suffix on the existing `__builtin_spirv_Convert<Src>To<E4M3|E5M2>EXT` interface.
When `_sat` is present, the resulting `OpFConvert` is decorated with `SaturatedToLargestFloat8NormalConversionEXT`.

AI-assisted: Claude Opus 4.7 (commercial SaaS)
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.

4 participants