Commit 338ac76
authored
Erase the cat/slice/select nop ops after memory planning (pytorch#22651)
Summary:
`_cat_nop`, `_slice_copy_nop` and `_select_copy_nop` do nothing at runtime. A
cat is only rewritten to its nop form once memory planning can place every
input at a contiguous offset inside the output, and a slice or select likewise
once its output can be colocated inside its input, so by the time planning has
run they hold by construction. The instructions that remain are pure dispatch
overhead.
This erases them, pointing their consumers at the output buffer the producers
have already written in place. A nop that cannot be erased raises rather than
being skipped: these ops exist only to carry a placement constraint from
constraint generation to here, so one reaching the emitter is a compiler bug.
**Why it has to live inside the memory planning pass.** A standalone pass is
not possible:
- Before planning the nodes cannot go, because the node is what carries the
placement constraint. Remove it early and the aliasing never happens.
- After planning nothing may run at all:
# WARNING: DO NOT ADD ANY MORE PASSES AFTER MEMORY PLANNING PASS.
# THERE ARE A LOT OF ASSUMPTIONS IN THE STACK THAT MEMORY PLANNING IS
# THE LAST PASS BEFORE THE EMITTER.
(exir/program/_program.py)
`CadenceMemoryPlanning.run` already mutates the graph after `mem_planning.run`
- `SimplifyIdmaOpsPass` retargets nodes and runs dead code elimination there -
so that slot is the one point where placement is decided but the program is not
yet emitted. This joins it.
**The assumptions that warning refers to are real.** Spec lifetimes are node
indices, so erasing nodes leaves them pointing past the end of the graph. That
trips `find_peak_memory_usage` and, in executorch/util, the activation memory
profiler. `update_all_tensors_lifetime` alone does not fix it, because
`update_tensor_lifetime` only ever widens a lifetime:
end = node_idx if end is None or end < node_idx else end
so a stale larger bound survives. `_refresh_lifetimes` uses the first call to
identify exactly which specs the recompute reaches, clears those, and rebuilds
them. Clearing a wider set would leave specs the recompute never revisits stuck
at None, which reads as "no lifetime" and silently drops them from the memory
reports. With that, no change is needed outside the Cadence backend - the shared
executorch diagnostics work unmodified.
The nop targets are looked up inside `call` rather than at class scope, because
their schemas come from `ops_registrations` and `memory_planning` does not
import it - resolving at import time breaks any module that imports
`CadenceMemoryPlanning` without having registered the ops first.
**The kernels stay, deliberately.**
Reviewed By: DrJessop
Differential Revision: D118922409
Pull Request resolved: pytorch#226511 parent 8449c90 commit 338ac76
2 files changed
Lines changed: 273 additions & 23 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
30 | 34 | | |
31 | 35 | | |
32 | 36 | | |
| |||
388 | 392 | | |
389 | 393 | | |
390 | 394 | | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
391 | 478 | | |
392 | 479 | | |
393 | 480 | | |
| |||
465 | 552 | | |
466 | 553 | | |
467 | 554 | | |
468 | | - | |
469 | | - | |
470 | | - | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
471 | 561 | | |
472 | 562 | | |
0 commit comments