Skip to content

Conversation

@YufengShi-dudu
Copy link
Collaborator

@YufengShi-dudu YufengShi-dudu commented Dec 10, 2025

  • Int32 clamp is decomposed into min/max, but input_tensor and min_arg/max_arg can end up with different ranks (e.g. (1,) vs ()) after subsequent passes. This violates the TOSA requirements for TOSA.MINIMUM and TOSA.MAXIMUM in the node visitors.
  • Fix this by matching the input ranks in MatchArgRanksPass

Change-Id: I47a3bb73832ddc35553cd3e5b65085155eb9ca48

cc @freddan80 @per @zingo @oscarandersson8218 @digantdesai

@YufengShi-dudu YufengShi-dudu added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk release notes: none Do not include this in the release notes labels Dec 10, 2025
@pytorch-bot
Copy link

pytorch-bot bot commented Dec 10, 2025

🔗 Helpful Links

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

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

❌ 46 New Failures

As of commit b28d0a6 with merge base c398ff4 (image):

NEW FAILURES - The following jobs have failed:

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 10, 2025
@ArmRyan
Copy link
Collaborator

ArmRyan commented Dec 10, 2025

int32 clamp already does a full op to handle the rank difference. If this is not working properly you should add a test that triggers this error or if it is a replacement, you should remove the full op

- Int32 clamp is decomposed into min/max, but input_tensor and
  min_arg/max_arg can end up with different ranks (e.g. (1,) vs ())
  after subsequent passes. This violates the TOSA requirements for
  TOSA.MINIMUM and TOSA.MAXIMUM in the node visitors.
- Fix this by matching the input ranks in MatchArgRanksPass

Change-Id: I47a3bb73832ddc35553cd3e5b65085155eb9ca48
Signed-off-by: Yufeng Shi <yufeng.shi@arm.com>
Co-authored-by: Ryan O'Shea <ryan.oshea3@arm.com>
Co-authored-by: Rob Elliott <Robert.Elliott@arm.com>
@YufengShi-dudu YufengShi-dudu force-pushed the match-arg-ranks-for-max-min-ops branch from e50aca5 to b28d0a6 Compare December 11, 2025 15:12
@YufengShi-dudu
Copy link
Collaborator Author

int32 clamp already does a full op to handle the rank difference. If this is not working properly you should add a test that triggers this error or if it is a replacement, you should remove the full op

Hey Ryan, thanks for your comment. I realize DecomposeInt32ClampPass does match the input ranks of aten.clamp.default. However, some passes between DecomposeInt32ClampPass and MatchArgRanksPass cause the input ranks to become unmatched. We can rematch them in MatchArgRanksPass.

This issue was found by some internal model tests and cannot be easily reproduced with an op test, but we have verified the fix against those internal models.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm release notes: none Do not include this in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants