loop_invariant_elimination: require the body to return the loop var itself - #2804
Open
LeSingh1 wants to merge 1 commit into
Open
loop_invariant_elimination: require the body to return the loop var itself#2804LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
…tself The pass flagged a loop var as invariant when the body's output for that var was "a var from outside of the block", which it tested with Block.is_var_visible_in_block. That is true of any var in an enclosing scope, not only of the loop var the body was seeded with. When the body returns a different outer var the loop var is not invariant at all: it is the seed on the first iteration and that other var from the second on. The pass then dropped the loop var and rewrote the while_loop output to the seed, so the model silently returned the wrong tensor. With loop_vars=(a, b), a body returning (add(a, 1), c) and three iterations, the output that should be c came back as b. Test the body output against while_op.loop_vars[i] instead. The existing "block outputs var from outside of the block" case still qualifies, because there the outer var is the loop var. Such a body output also has to be given a definition inside the body, since a block output has to be produced in its own block; leaving a raw enclosing-scope var there produces a model Core ML cannot load. Re-emit it with an identity in the body block.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
loop_invariant_eliminationtreated "the body returns any var visible outside the block" as proof a loop var is invariant. Awhile_loopwithloop_vars=(a, b)whose body returnsctherefore returnedb.Fixing the detection alone leaves a body output with no definition inside the block, which segfaults CoreML at model load, so the pass now re-emits it through an
identity.Verified end to end with a real prediction:
[[10,10]]->[[20,20]]. The test fails onmain.