[ET-VK] Fix mixed-dtype binary ops and comparison op padding bugs#17849
[ET-VK] Fix mixed-dtype binary ops and comparison op padding bugs#17849meta-codesync[bot] merged 1 commit intogh/SS-JIA/458/basefrom
Conversation
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]
🔗 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 FailuresAs of commit bb093ef with merge base 1a75394 ( 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. |
This PR needs a
|
e9115cf
into
gh/SS-JIA/458/base
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
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
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
Stack from ghstack (oldest at bottom):
Two bugs caused incorrect outputs in models with mixed-dtype binary operations
(e.g. EdgeTAM remaining frames):
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
InsertDtypePromotionPassexport pass thatinserts
_to_copynodes to promote inputs to a common dtype at compile time.The
_to_copyop is extended to support int<->float conversions via newview_convert_textureshaders, and the previous float/half-only restrictionin ToCopy.cpp is replaced with branching logic that uses BlitNode for
same-dtype/float<->half and view_convert shaders for other conversions.
Texture3d comparison operators (gt, lt, le, ge, eq) used
all()to reducecomponent-wise
bvec4results to a single bool. With packed textures wherepadding components are zero,
all()always returned false because paddingzeros fail comparison against non-zero values. Fixed by removing
all()sothe result stays as a component-wise
bvec4, which is correctly converted touvec4for the Bool output texture.Additional changes:
view_convert_texture.glslshader and YAML for texture dtype conversionadd_view_copy_convert_texture_nodeadded to View.cpp/h_to_copyop registry updated to accept int dtypes (FP_INT_T)Differential Revision: D95217948