Skip to content

[SPIRV][Doc] Add arbitrary precision floating and fixed point specs #1934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ajaykumarkannan
Copy link
Contributor

These operations are to be leveraged by the DPC++ compiler for the Intel FPGA devices.

jbrodman
jbrodman previously approved these changes Jun 19, 2020
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

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

Some partial review: haven't fully read SPV_INTEL_arbitrary_precision_floating_point spec yet

@jbrodman
Copy link
Contributor

Will approve after @AlexeySachkov does.

vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Jul 7, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
mlychkov added a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 20, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc
mlychkov added a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 21, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 27, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 27, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
mlychkov added a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 27, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
mlychkov added a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 29, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 29, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
AlexeySotkin pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Jul 31, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>
vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Jul 31, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Jul 31, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Aug 2, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Aug 3, 2020
Add SPV_INTEL_arbitrary_precision_fixed_point extension, which
introduces operations for arbitrary precision floating point numbers.
The hls_float datatype is represented as a pseudo type using
OpTypeInt. Its parameters are:
* W - width of the datatype. It's encoded in the width of the OpTypeInt.
* E - represents the number of exponent bits. Exponent values
      are inferred from the width of corresponding variable.
* M - represents the number of mantissa bits.
The total width W is equal to `E+M+1` where the last bit is used to
represent the sign.

Full specification (intel#1934):
https://github.com/intel/llvm/blob/bd86b218f749ea0e20ddc18c42db491faf54014a/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_floating_point.asciidoc

Signed-off-by: Mikhail Lychkov <mikhail.lychkov@intel.com>

_Result Type_ is an `OpTypeInt` of width `1+Eout+Mout` and is the type of _Result_.

_M*_ contains the width of the mantissa of the floating point types within _Result_, _A_, and _B_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I would prefer to have clear mapping between literal and argument which it corresponds to, but I guess it could be inferred with current wording as well: Mout for result, M1 for the first argument and M2 for the second

Probably would be good to hear more feedback from someone else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexeySachkov any suggestions on what you would rename these to?


=== Validation Rules

None.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that there are no validation rules?

I think I can imagine at least a few:

  • M* literal arguments can't contains values that exceed width of OpTypeInt of corresponding argument minus 1
  • M* literal should contain value more than zero

It seems that there are no requirement to have arguments of the same width, but probably there are still some limitations related to this?

Note: the same applies to fixed-point extension

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - added for floating point. Based on the discussion about what values I can take, I don't know if this applies to the fixed_point case. What do you think?


After Section 3.16, add a new section "3.16d Rounding Accuracy" as follows:

=== Rounding Accuracy
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, all instructions documented here have both "Subnormal Support" and "Rounding Accuracy" as literal arguments - this is a 8 bytes for storing 4 bits, which is kind of overkill.

This is not a major comment, but probably you will consider merging these two into a single bitmask and reducing amount of arguments of each new instructions.

The same applies to fixed-point extension

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one's on me - we could change this, but since the translator is already using the preliminary spec, we'll have to circle back and update it there as well. Can we table this one for a later changelist? Also, do internal SPIR-V API changes need a deprecation period as well?

mlychkov pushed a commit to mlychkov/SPIRV-LLVM-Translator that referenced this pull request Aug 6, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification (intel/llvm#1934):
https://github.com/intel/llvm/blob/2f6e965e686354fbb25f9c177a667a646de302eb/sycl/doc/extensions/SPIRV/SPV_INTEL_arbitrary_precision_fixed_point.asciidoc
AlexeySachkov pushed a commit to KhronosGroup/SPIRV-LLVM-Translator that referenced this pull request Aug 7, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification  can be found in intel/llvm#1934

Co-authored-by: Viktoria Maksimova <viktoria.maksimova@intel.com>
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Aug 10, 2020
This patch introduces SPV_INTEL_arbitrary_precision_fixed_point extension,
which adds operations for arbitrary precision fixed point numbers.

The ac_fixed datatype is represented as a pseudo type using OpTypeInt. Its
parameters are:
* W - total width of the datatype. It's encoded in the width of the OpTypeInt.
* I - determines the position of the decimal point.
* S - determines if this is a signed or an unsigned number.

Full specification  can be found in intel#1934

Co-authored-by: Viktoria Maksimova <viktoria.maksimova@intel.com>
@ajaykumarkannan ajaykumarkannan requested a review from a team as a code owner August 12, 2020 03:06
|=====
9+<|*OpFixedSinINTEL* +

An `OpTypeInt` representing an arbitrary precision fixed point number (ac_fixed) is passed in as the _Input_ and the sine of the value is returned in _Result_.
Copy link
Contributor

@MrSidims MrSidims Oct 12, 2021

Choose a reason for hiding this comment

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

@ajaykumarkannan @tiwaria1 if input is more than of 64bit width, clang for spir target will generate a byval pointer for Input parameter. With the current version of the spec we would need to create an extra copy/load to make the proper translation. Alternatively we can allow pointers as input. WDYT?

Copy link
Contributor

@tiwaria1 tiwaria1 Oct 15, 2021

Choose a reason for hiding this comment

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

I don't see any issue with updating this spec to allow pointer inputs. Users will use our wrappers over the spirv_ops.hpp functions. Without such an update, we will have worse performance due to generation of those extra loads and stores.

@github-actions github-actions bot added the Stale label Jun 16, 2022
@github-actions github-actions bot closed this Jul 17, 2022
@bader bader changed the title [SYCL] Floating point and fixed point operations added to SPIR-V [SPIRV][Doc] Add arbitrary precision floating and fixed point specs Jul 17, 2022
@MrSidims MrSidims reopened this Oct 12, 2022
@MrSidims
Copy link
Contributor

@ajaykumarkannan @tiwaria1 hi, is it considered a finished extension or extra work is required?

@github-actions github-actions bot removed the Stale label Oct 13, 2022
@tiwaria1
Copy link
Contributor

@ajaykumarkannan @tiwaria1 hi, is it considered a finished extension or extra work is required?

We haven't looked at this in a while. There is that "using pointers as an input to avoid the extra load/stores" issue that you pointed to above that will require an update.

@ajaykumarkannan
Copy link
Contributor Author

ajaykumarkannan commented Oct 13, 2022

@ajaykumarkannan @tiwaria1 hi, is it considered a finished extension or extra work is required?

@MrSidims The actual change has been implemented so this PR is the current state of implementation. We can check this in and address the remaining gaps later if needed.

@gmlueck
Copy link
Contributor

gmlueck commented Oct 13, 2022

Can you please rename these files? SPIR-V extensions now live in "sycl/doc/design/spirv-extensions".

…bitrary_types

* Move SPV_INTEL extensions for arbitrary types to sycl/doc/design/spirv-extensions
@ajaykumarkannan ajaykumarkannan requested a review from a team as a code owner October 18, 2022 03:45
@ajaykumarkannan
Copy link
Contributor Author

Can you please rename these files? SPIR-V extensions now live in "sycl/doc/design/spirv-extensions".

Thanks Greg - moved the files now (and did a pull down from main).

@ajaykumarkannan
Copy link
Contributor Author

Ping @gmlueck @MrSidims can we merge this in, or is there anything further you guys would like to change before committing. We also have been requested to merge these specs into the Khronos repository here:

@MrSidims MrSidims requested review from MrSidims and removed request for AlexeySotkin and mlychkov November 21, 2022 13:22
@MrSidims
Copy link
Contributor

In general LGTM, but I have two suggestions/questions to address:
#1934 (comment)
and
#1934 (comment)

@gmlueck
Copy link
Contributor

gmlueck commented Nov 21, 2022

Ping @gmlueck @MrSidims can we merge this in, or is there anything further you guys would like to change before committing.

I suggest abandoning this PR since these extension specifications are being reviewed instead in the "intel-innersource" repo you list above. Can we move the review discussion there to keep it all in one place?

@ajaykumarkannan
Copy link
Contributor Author

Good note - I'll move this out to the other repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec extension All issues/PRs related to extensions specifications SPIR-V Issues related to SPIRV-LLVM-Translator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants