Skip to content

[val] Do not reject OpExtInst for BFloat16/FP8 result types#6734

Open
aobolensk wants to merge 5 commits into
KhronosGroup:mainfrom
aobolensk:OpExtInst-bfloat-fp8
Open

[val] Do not reject OpExtInst for BFloat16/FP8 result types#6734
aobolensk wants to merge 5 commits into
KhronosGroup:mainfrom
aobolensk:OpExtInst-bfloat-fp8

Conversation

@aobolensk

@aobolensk aobolensk commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@aobolensk

Copy link
Copy Markdown
Contributor Author

@MrSidims could you please take a look?

@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.

I'm unsure if we have instruction sets that supports FP8.

Regarding bfloat - this requires a bit of research, the only extension that allows usage of such values is (from the best of my knowledge) SPV_INTEL_bfloat16_arithmetic (allows for OpenCL ExtInst), but when accounting for it - the change must be guarded by checking if the extension is enabled. At the same time the extension doesn't present in the Registry and it's not really a common practice to introduce it to SPIR-V Tools.

@aobolensk

Copy link
Copy Markdown
Contributor Author

I'm unsure if we have instruction sets that supports FP8.

Regarding bfloat - this requires a bit of research, the only extension that allows usage of such values is (from the best of my knowledge) SPV_INTEL_bfloat16_arithmetic (allows for OpenCL ExtInst), but when accounting for it - the change must be guarded by checking if the extension is enabled. At the same time the extension doesn't present in the Registry and it's not really a common practice to introduce it to SPIR-V Tools.

It seems to be already checked as a part of extensions verifications. I have added more tests for these cases to protect (rather, to prove that this protection already exists) from using bf16/fp8 in OpExtInst instruction without corresponding caps

@alan-baker

Copy link
Copy Markdown
Contributor

It feels like this must be supported to get debug info support.

@alan-baker

Copy link
Copy Markdown
Contributor

Also, extension instructions are in the allowlist for bfloat16 and fp8.

@MrSidims

Copy link
Copy Markdown
Contributor

It feels like this must be supported to get debug info support.

Ahhh, I get it. No objections from me then.

Comment thread test/val/val_invalid_type_test.cpp
@aobolensk

aobolensk commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

CI failures seem to be unrelated with my changes

@alan-baker

Copy link
Copy Markdown
Contributor

Can you trying rebasing, another PR attempted to fix that.

@aobolensk

Copy link
Copy Markdown
Contributor Author

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