Skip to content

Conversation

@SS-JIA
Copy link
Contributor

@SS-JIA SS-JIA commented Dec 30, 2025

Stack from ghstack (oldest at bottom):

When the packed dimension size is not a multiple of 4, texture-backed
tensors have padding elements in the last texel. For division operations,
these padding regions contain 0/0 = NaN, which propagates through
subsequent reduction operations and corrupts results.

This fix adds conditional padding masking logic to binary_op shaders:

  • Introduced MASK_PADDING codegen variable to binary_op.yaml
  • Enabled MASK_PADDING=1 for binary_div and binary_floor_divide ops
  • Added GLSL preprocessor macro definition in binary_op.glsl
  • Implemented padding masking logic using modulo arithmetic to correctly
    identify last texels in batch concatenation scenarios
  • Padding elements are explicitly zeroed out to prevent NaN propagation

The implementation follows GLSL best practices by using Python
preprocessing only for macro definition, keeping core shader logic
as pure GLSL with standard #ifdef directives.

Differential Revision: D89935220

…exels

When the packed dimension size is not a multiple of 4, texture-backed
tensors have padding elements in the last texel. For division operations,
these padding regions contain 0/0 = NaN, which propagates through
subsequent reduction operations and corrupts results.

This fix adds conditional padding masking logic to binary_op shaders:
- Introduced MASK_PADDING codegen variable to binary_op.yaml
- Enabled MASK_PADDING=1 for binary_div and binary_floor_divide ops
- Added GLSL preprocessor macro definition in binary_op.glsl
- Implemented padding masking logic using modulo arithmetic to correctly
  identify last texels in batch concatenation scenarios
- Padding elements are explicitly zeroed out to prevent NaN propagation

The implementation follows GLSL best practices by using Python
preprocessing only for macro definition, keeping core shader logic
as pure GLSL with standard #ifdef directives.

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

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Dec 30, 2025

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 2 New Failures, 1 Unrelated Failure

As of commit da3a2f1 with merge base 1066e7c (image):

NEW FAILURES - The following jobs have failed:

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

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 Dec 30, 2025
@github-actions
Copy link

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.

…or padded texels"

When the packed dimension size is not a multiple of 4, texture-backed
tensors have padding elements in the last texel. For division operations,
these padding regions contain 0/0 = NaN, which propagates through
subsequent reduction operations and corrupts results.

This fix adds conditional padding masking logic to binary_op shaders:
- Introduced MASK_PADDING codegen variable to binary_op.yaml
- Enabled MASK_PADDING=1 for binary_div and binary_floor_divide ops
- Added GLSL preprocessor macro definition in binary_op.glsl
- Implemented padding masking logic using modulo arithmetic to correctly
  identify last texels in batch concatenation scenarios
- Padding elements are explicitly zeroed out to prevent NaN propagation

The implementation follows GLSL best practices by using Python
preprocessing only for macro definition, keeping core shader logic
as pure GLSL with standard #ifdef directives.

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

[ghstack-poisoned]
Copy link
Contributor

@mergennachin mergennachin left a comment

Choose a reason for hiding this comment

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

Can you add this regression test somewhere?

something like this:

x = torch.ones(2, 5, 3, 3)
y = torch.ones(2, 5, 3, 3)
return (x / y).sum()

We should at least add in vulkan test, but also, I wonder if we can expand it to Backend Test harness test that @GregoryComer built

Comment on lines +153 to +162
const int nspill = mod4(out_sizes[packed_dim]);
const int texels_per_batch = divup4(out_sizes[packed_dim]);
const bool is_last_texel = (lpos[packed_dim] % texels_per_batch) == (texels_per_batch - 1);

if (is_last_texel && nspill > 0) {
// Explicitly set padding elements to 0 to avoid NaN
[[unroll]] for (int i = nspill; i < 4; i++) {
out_texel[i] = 0;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

const int texels_per_batch = divup4(out_sizes[packed_dim]); // -> this could be 0, and you're dividing by zero on the next line

i suggest a safer code:

    const int nspill = mod4(out_sizes[packed_dim]);
    if (nspill > 0) {
      const int texels_per_batch = divup4(out_sizes[packed_dim]); // this won't be 0 since we check nspill >0
      const bool is_last_texel = (lpos[packed_dim] % texels_per_batch) == (texels_per_batch - 1);
      if (is_last_texel) {
        [[unroll]] for (int i = nspill; i < 4; i++) {
          out_texel[i] = 0;
        }
      }
    }

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. fb-exported meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants