Skip to content

[ET-VK] Fix mixed-dtype binary ops and comparison op padding bugs#17849

Merged
meta-codesync[bot] merged 1 commit intogh/SS-JIA/458/basefrom
gh/SS-JIA/458/head
Mar 4, 2026
Merged

[ET-VK] Fix mixed-dtype binary ops and comparison op padding bugs#17849
meta-codesync[bot] merged 1 commit intogh/SS-JIA/458/basefrom
gh/SS-JIA/458/head

Conversation

@SS-JIA
Copy link
Copy Markdown
Contributor

@SS-JIA SS-JIA commented Mar 4, 2026

Stack from ghstack (oldest at bottom):

Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):

  1. Mixed-dtype binary ops (e.g. int arange vs float tensor) were fed to shaders
    that declare both inputs with the same DTYPE, causing data misinterpretation.
    This is now fixed by adding an InsertDtypePromotionPass export pass that
    inserts _to_copy nodes to promote inputs to a common dtype at compile time.
    The _to_copy op is extended to support int<->float conversions via new
    view_convert_texture shaders, and the previous float/half-only restriction
    in ToCopy.cpp is replaced with branching logic that uses BlitNode for
    same-dtype/float<->half and view_convert shaders for other conversions.

  2. Texture3d comparison operators (gt, lt, le, ge, eq) used all() to reduce
    component-wise bvec4 results to a single bool. With packed textures where
    padding components are zero, all() always returned false because padding
    zeros fail comparison against non-zero values. Fixed by removing all() so
    the result stays as a component-wise bvec4, which is correctly converted to
    uvec4 for the Bool output texture.

Additional changes:

  • New view_convert_texture.glsl shader and YAML for texture dtype conversion
  • add_view_copy_convert_texture_node added to View.cpp/h
  • _to_copy op registry updated to accept int dtypes (FP_INT_T)

Differential Revision: D95217948

Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):

1. Mixed-dtype binary ops (e.g. int arange vs float tensor) were fed to shaders
   that declare both inputs with the same DTYPE, causing data misinterpretation.
   This is now fixed by adding an `InsertDtypePromotionPass` export pass that
   inserts `_to_copy` nodes to promote inputs to a common dtype at compile time.
   The `_to_copy` op is extended to support int<->float conversions via new
   `view_convert_texture` shaders, and the previous float/half-only restriction
   in ToCopy.cpp is replaced with branching logic that uses BlitNode for
   same-dtype/float<->half and view_convert shaders for other conversions.

2. Texture3d comparison operators (gt, lt, le, ge, eq) used `all()` to reduce
   component-wise `bvec4` results to a single bool. With packed textures where
   padding components are zero, `all()` always returned false because padding
   zeros fail comparison against non-zero values. Fixed by removing `all()` so
   the result stays as a component-wise `bvec4`, which is correctly converted to
   `uvec4` for the Bool output texture.

Additional changes:
- New `view_convert_texture.glsl` shader and YAML for texture dtype conversion
- `add_view_copy_convert_texture_node` added to View.cpp/h
- `_to_copy` op registry updated to accept int dtypes (FP_INT_T)

Differential Revision: [D95217948](https://our.internmc.facebook.com/intern/diff/D95217948/)

[ghstack-poisoned]
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Mar 4, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17849

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 4 Unrelated Failures

As of commit bb093ef with merge base 1a75394 (image):

NEW FAILURE - The following job has failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 4, 2026

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@meta-codesync meta-codesync Bot merged commit e9115cf into gh/SS-JIA/458/base Mar 4, 2026
208 of 221 checks passed
@meta-codesync meta-codesync Bot deleted the gh/SS-JIA/458/head branch March 4, 2026 23:44
@meta-codesync meta-codesync Bot temporarily deployed to cherry-pick-bot March 4, 2026 23:44 Inactive
SS-JIA pushed a commit that referenced this pull request Mar 5, 2026
Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):

1. Mixed-dtype binary ops (e.g. int arange vs float tensor) were fed to shaders
   that declare both inputs with the same DTYPE, causing data misinterpretation.
   This is now fixed by adding an `InsertDtypePromotionPass` export pass that
   inserts `_to_copy` nodes to promote inputs to a common dtype at compile time.
   The `_to_copy` op is extended to support int<->float conversions via new
   `view_convert_texture` shaders, and the previous float/half-only restriction
   in ToCopy.cpp is replaced with branching logic that uses BlitNode for
   same-dtype/float<->half and view_convert shaders for other conversions.

2. Texture3d comparison operators (gt, lt, le, ge, eq) used `all()` to reduce
   component-wise `bvec4` results to a single bool. With packed textures where
   padding components are zero, `all()` always returned false because padding
   zeros fail comparison against non-zero values. Fixed by removing `all()` so
   the result stays as a component-wise `bvec4`, which is correctly converted to
   `uvec4` for the Bool output texture.

Additional changes:
- New `view_convert_texture.glsl` shader and YAML for texture dtype conversion
- `add_view_copy_convert_texture_node` added to View.cpp/h
- `_to_copy` op registry updated to accept int dtypes (FP_INT_T)

Differential Revision: [D95217948](https://our.internmc.facebook.com/intern/diff/D95217948/)

ghstack-source-id: 347411474
Pull Request resolved: #17849
SS-JIA pushed a commit that referenced this pull request Mar 5, 2026
Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):

1. Mixed-dtype binary ops (e.g. int arange vs float tensor) were fed to shaders
   that declare both inputs with the same DTYPE, causing data misinterpretation.
   This is now fixed by adding an `InsertDtypePromotionPass` export pass that
   inserts `_to_copy` nodes to promote inputs to a common dtype at compile time.
   The `_to_copy` op is extended to support int<->float conversions via new
   `view_convert_texture` shaders, and the previous float/half-only restriction
   in ToCopy.cpp is replaced with branching logic that uses BlitNode for
   same-dtype/float<->half and view_convert shaders for other conversions.

2. Texture3d comparison operators (gt, lt, le, ge, eq) used `all()` to reduce
   component-wise `bvec4` results to a single bool. With packed textures where
   padding components are zero, `all()` always returned false because padding
   zeros fail comparison against non-zero values. Fixed by removing `all()` so
   the result stays as a component-wise `bvec4`, which is correctly converted to
   `uvec4` for the Bool output texture.

Additional changes:
- New `view_convert_texture.glsl` shader and YAML for texture dtype conversion
- `add_view_copy_convert_texture_node` added to View.cpp/h
- `_to_copy` op registry updated to accept int dtypes (FP_INT_T)

Differential Revision: [D95217948](https://our.internmc.facebook.com/intern/diff/D95217948/)

ghstack-source-id: 347411474
Pull Request resolved: #17849
jpiat pushed a commit to jpiat/executorch that referenced this pull request Mar 17, 2026
Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):

1. Mixed-dtype binary ops (e.g. int arange vs float tensor) were fed to shaders
   that declare both inputs with the same DTYPE, causing data misinterpretation.
   This is now fixed by adding an `InsertDtypePromotionPass` export pass that
   inserts `_to_copy` nodes to promote inputs to a common dtype at compile time.
   The `_to_copy` op is extended to support int<->float conversions via new
   `view_convert_texture` shaders, and the previous float/half-only restriction
   in ToCopy.cpp is replaced with branching logic that uses BlitNode for
   same-dtype/float<->half and view_convert shaders for other conversions.

2. Texture3d comparison operators (gt, lt, le, ge, eq) used `all()` to reduce
   component-wise `bvec4` results to a single bool. With packed textures where
   padding components are zero, `all()` always returned false because padding
   zeros fail comparison against non-zero values. Fixed by removing `all()` so
   the result stays as a component-wise `bvec4`, which is correctly converted to
   `uvec4` for the Bool output texture.

Additional changes:
- New `view_convert_texture.glsl` shader and YAML for texture dtype conversion
- `add_view_copy_convert_texture_node` added to View.cpp/h
- `_to_copy` op registry updated to accept int dtypes (FP_INT_T)

Differential Revision: [D95217948](https://our.internmc.facebook.com/intern/diff/D95217948/)

ghstack-source-id: 347411474
Pull Request resolved: pytorch#17849
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants