Skip to content

Commit 29557f1

Browse files
authored
Fix constant_prop_pass fqn collision with state_dict (pytorch#23266)
Summary: `replace_with_constant_node` names propagated constants `_prop_tensor_constant{len(exported_program.constants)}`, then checks for a collision. Both the check and the max-suffix rescan look only at `exported_program.constants`. A program re-entering this pass can already carry a `_prop_tensor_constant*` in `exported_program.state_dict` from an earlier run. That name is invisible to both, so the pass reuses it and writes its tensor into `constants` alongside the stale `state_dict` entry. Emission then resolves the placeholder against `state_dict` before `constants` (`exir/emit/_emitter.py:2044`), picks up the stale tensor, and fails the size check at `_emitter.py:513`: InternalError: Tensor spec has buffer of size 48, but expected nbytes of 4 The spec is correct at 4 bytes; the tensor bound to it is the wrong one. Note the existing comment already reads "If prop_constant_tensor_fqn already exists in the state dict" -- the implementation never matched that intent. Widen the collision check and the suffix rescan to span `state_dict` as well as `constants`. Observed on a Turing/Coleman hand-tracking model compiled with `fx_options.allow_quant_dequant_on_tce=True`, which folds a scalar `1` from a `1 - valid_multi_view` expression whose name was already taken in `state_dict`. Instrumenting the pass showed the collision directly: fqn=_prop_tensor_constant20 new_nbytes=4 already_in_constants=False already_in_state_dict=True With the fix the model compiles to a .pte (3 TCE delegates, 84.19% delegation); without it emission aborts. Differential Revision: D122298673 Pull Request resolved: pytorch#23266
1 parent d67aa29 commit 29557f1

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

‎exir/passes/constant_prop_pass.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,17 @@ def replace_with_constant_node(
213213
# If prop_constant_tensor_fqn already exists in the state dict, we need
214214
# to create a new name. Find the largest suffix of "_prop_tensor_constant",
215215
# and increment it by 1 to form the new name.
216-
if prop_constant_tensor_fqn in exported_program.constants:
216+
# A program re-entering this pass can already carry a
217+
# `_prop_tensor_constant*` in state_dict from an earlier run. Emission
218+
# resolves placeholders against state_dict before constants, so reusing
219+
# such a name lets the stale entry shadow the tensor written here. Both
220+
# the check and the suffix scan therefore span state_dict too.
221+
taken = exported_program.constants.keys() | exported_program.state_dict.keys()
222+
if prop_constant_tensor_fqn in taken:
217223
suffix = 1 + max(
218224
(
219225
int(name[len(prefix) :])
220-
for name in exported_program.constants.keys()
226+
for name in taken
221227
if name.startswith(prefix) and name[len(prefix) :].isdigit()
222228
),
223229
default=-1,

‎exir/tests/test_passes.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,6 +1690,32 @@ def forward(self, x: torch.Tensor) -> torch.Tensor:
16901690
new_ep.graph_module.code
16911691
)
16921692

1693+
def test_constant_prop_pass_avoids_state_dict_name_collision(self) -> None:
1694+
class Add(torch.nn.Module):
1695+
def forward(self, x: torch.Tensor) -> torch.Tensor:
1696+
return x + 3
1697+
1698+
edge = to_edge(
1699+
export(Add(), (torch.ones(1),), strict=True),
1700+
compile_config=EdgeCompileConfig(_skip_dim_order=False),
1701+
)
1702+
edge = edge.transform([ScalarToTensorPass(), RemoveMixedTypeOperators()])
1703+
exported_program = lift_constant_tensor_pass(edge.exported_program())
1704+
1705+
# A program re-entering this pass can already carry a
1706+
# `_prop_tensor_constant*` in state_dict from an earlier run.
1707+
# Emission resolves placeholders against state_dict before
1708+
# constants, so reusing the name lets the stale entry shadow the
1709+
# propagated tensor and the two disagree on size.
1710+
stale = torch.zeros(12)
1711+
exported_program.state_dict["_prop_tensor_constant0"] = stale
1712+
1713+
new_ep = constant_prop_pass(exported_program)
1714+
1715+
for name in new_ep.constants:
1716+
self.assertNotIn(name, new_ep.state_dict)
1717+
self.assertIs(new_ep.state_dict["_prop_tensor_constant0"], stale)
1718+
16931719
def test_pass_no_user_inputs(self) -> None:
16941720
class NoUserInputs(torch.nn.Module):
16951721
def __init__(self):

0 commit comments

Comments
 (0)