Skip to content

simd.h: renaming and fixing of shuffle template#4739

Merged
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-simdshuffle
May 12, 2025
Merged

simd.h: renaming and fixing of shuffle template#4739
lgritz merged 1 commit into
AcademySoftwareFoundation:mainfrom
lgritz:lg-simdshuffle

Conversation

@lgritz
Copy link
Copy Markdown
Collaborator

@lgritz lgritz commented May 4, 2025

While I was investigating some SonarQube warnings about our simd shuffle<> templates (which are a false positive and I have a separate PR to simply silence it), I did get to thinking about the naming, and want to make a couple changes:

  • I decided that the 1-template-argument version of this function, shuffle<int i>(simd_type) would actually be more clear and self-documenting if renamed broadcast_element to emphasize that it is taking just one simd lane/element and broadcasting it to all lanes. (The multi-argument shuffle really is doing a true shuffle, giving an index for each lane to make a permutation of swizzle, so I'm not renaming that one.) To avoid breaking source compatibility, I am leaving the old name as well as a synonym, but commenting it as deprecated and I will phase out its use. It will disappear entirely from a future OIIO version that's safe to break compatibility.

  • For 16-wide simd, the 1-arg template we called shuffle was not doing the same operation -- it was replicating one group of 4 elements instead of a single element. We didn't use it anywhere, so I'm redefining it to do the analogous thing as it does for 4-wide and 8-wide.

While I was investigating some SonarQube warnings about our simd
`shuffle<>` templates (which are a false positive and I have a
separate PR to simply silence it), I did get to thinking about the
naming, and want to make a couple changes:

* I decided that the 1-template-argument version of this function,
  `shuffle<int i>(simd_type)` would actually be more clear and
  self-documenting if renamed `broadcast_element` to emphasize that it
  is taking just one simd lane/element and broadcasting it to all
  lanes. (The multi-argument shuffle really is doing a true shuffle,
  giving an index for each lane to make a permutation of swizzle,
  so I'm not renaming that one.) To avoid breaking source compatibility,
  I am leaving the old name as well as a synonym, but commenting it as
  deprecated and I will phase out its use. It will disappear entirely
  from a future OIIO version that's safe to break compatibility.

* For 16-wide simd, the 1-arg template we called shuffle was not doing
  the same operation -- it was replicating one group of 4 elements
  instead of a single element. We didn't use it anywhere, so I'm
  redefining it to do the analogous thing as it does for 4-wide and
  8-wide.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz lgritz force-pushed the lg-simdshuffle branch from 1d46da8 to 3168899 Compare May 4, 2025 20:42
@lgritz
Copy link
Copy Markdown
Collaborator Author

lgritz commented May 11, 2025

This has been posted for a week. If there are no objections by the end of Monday, I will merge.

@lgritz lgritz merged commit 7a3b76d into AcademySoftwareFoundation:main May 12, 2025
28 checks passed
@lgritz lgritz deleted the lg-simdshuffle branch May 13, 2025 03:43
scott-wilson pushed a commit to scott-wilson/OpenImageIO that referenced this pull request May 17, 2025
…ation#4739)

While I was investigating some SonarQube warnings about our simd
`shuffle<>` templates (which are a false positive and I have a separate
PR to simply silence it), I did get to thinking about the naming, and
want to make a couple changes:

* I decided that the 1-template-argument version of this function,
`shuffle<int i>(simd_type)` would actually be more clear and
self-documenting if renamed `broadcast_element` to emphasize that it is
taking just one simd lane/element and broadcasting it to all lanes. (The
multi-argument shuffle really is doing a true shuffle, giving an index
for each lane to make a permutation of swizzle, so I'm not renaming that
one.) To avoid breaking source compatibility, I am leaving the old name
as well as a synonym, but commenting it as deprecated and I will phase
out its use. It will disappear entirely from a future OIIO version
that's safe to break compatibility.

* For 16-wide simd, the 1-arg template we called shuffle was not doing
the same operation -- it was replicating one group of 4 elements instead
of a single element. We didn't use it anywhere, so I'm redefining it to
do the analogous thing as it does for 4-wide and 8-wide.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Scott Wilson <scott@propersquid.com>
scott-wilson pushed a commit to scott-wilson/OpenImageIO that referenced this pull request May 18, 2025
…ation#4739)

While I was investigating some SonarQube warnings about our simd
`shuffle<>` templates (which are a false positive and I have a separate
PR to simply silence it), I did get to thinking about the naming, and
want to make a couple changes:

* I decided that the 1-template-argument version of this function,
`shuffle<int i>(simd_type)` would actually be more clear and
self-documenting if renamed `broadcast_element` to emphasize that it is
taking just one simd lane/element and broadcasting it to all lanes. (The
multi-argument shuffle really is doing a true shuffle, giving an index
for each lane to make a permutation of swizzle, so I'm not renaming that
one.) To avoid breaking source compatibility, I am leaving the old name
as well as a synonym, but commenting it as deprecated and I will phase
out its use. It will disappear entirely from a future OIIO version
that's safe to break compatibility.

* For 16-wide simd, the 1-arg template we called shuffle was not doing
the same operation -- it was replicating one group of 4 elements instead
of a single element. We didn't use it anywhere, so I'm redefining it to
do the analogous thing as it does for 4-wide and 8-wide.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Scott Wilson <scott@propersquid.com>
zachlewis pushed a commit to zachlewis/OpenImageIO that referenced this pull request Aug 1, 2025
…ation#4739)

While I was investigating some SonarQube warnings about our simd
`shuffle<>` templates (which are a false positive and I have a separate
PR to simply silence it), I did get to thinking about the naming, and
want to make a couple changes:

* I decided that the 1-template-argument version of this function,
`shuffle<int i>(simd_type)` would actually be more clear and
self-documenting if renamed `broadcast_element` to emphasize that it is
taking just one simd lane/element and broadcasting it to all lanes. (The
multi-argument shuffle really is doing a true shuffle, giving an index
for each lane to make a permutation of swizzle, so I'm not renaming that
one.) To avoid breaking source compatibility, I am leaving the old name
as well as a synonym, but commenting it as deprecated and I will phase
out its use. It will disappear entirely from a future OIIO version
that's safe to break compatibility.

* For 16-wide simd, the 1-arg template we called shuffle was not doing
the same operation -- it was replicating one group of 4 elements instead
of a single element. We didn't use it anywhere, so I'm redefining it to
do the analogous thing as it does for 4-wide and 8-wide.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
zachlewis pushed a commit to zachlewis/OpenImageIO that referenced this pull request Sep 1, 2025
…ation#4739)

While I was investigating some SonarQube warnings about our simd
`shuffle<>` templates (which are a false positive and I have a separate
PR to simply silence it), I did get to thinking about the naming, and
want to make a couple changes:

* I decided that the 1-template-argument version of this function,
`shuffle<int i>(simd_type)` would actually be more clear and
self-documenting if renamed `broadcast_element` to emphasize that it is
taking just one simd lane/element and broadcasting it to all lanes. (The
multi-argument shuffle really is doing a true shuffle, giving an index
for each lane to make a permutation of swizzle, so I'm not renaming that
one.) To avoid breaking source compatibility, I am leaving the old name
as well as a synonym, but commenting it as deprecated and I will phase
out its use. It will disappear entirely from a future OIIO version
that's safe to break compatibility.

* For 16-wide simd, the 1-arg template we called shuffle was not doing
the same operation -- it was replicating one group of 4 elements instead
of a single element. We didn't use it anywhere, so I'm redefining it to
do the analogous thing as it does for 4-wide and 8-wide.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
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.

1 participant