Skip to content

Conversation

@kentslaney
Copy link
Contributor

No description provided.

@TobyRoseman
Copy link
Collaborator

Can you please include a unit test.

@kentslaney
Copy link
Contributor Author

The following check currently fails on main:

check_counts(divs=1, muls=1, const_skip=32)

common::const_elimination has skip_const_by_size set to 32, but the denominator is turned into a constant anyway

@TobyRoseman
Copy link
Collaborator

Thanks for adding a unit test. The code change looks good.

CI: https://gitlab.com/coremltools1/coremltools/-/pipelines/2218228187

@kentslaney
Copy link
Contributor Author

Quick disclaimer: one trade-off I made unilaterally was not putting a separate option in divide_to_multiply akin to skip_const_by_size. I haven't looked into the performance factor for the graph pass, but the options are modular and I don't think it's worth the added complexity.

@TobyRoseman
Copy link
Collaborator

Thanks @kentslaney

@TobyRoseman TobyRoseman merged commit 0434e85 into apple:main Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants