simd.h: renaming and fixing of shuffle template#4739
Merged
Conversation
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>
Collaborator
Author
|
This has been posted for a week. If there are no objections by the end of Monday, I will merge. |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 renamedbroadcast_elementto 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.