Skip to content

AdStack trip-count clipping in determine_ad_stack_size can never run #861

Description

@hughperkins

determine_ad_stack_size.cpp clips AdStack row claims by each range-for's trip count, but the loop that does it
iterates over a list that is always empty, so the optimisation never fires and autodiff heaps stay sized by the
unclipped reducer count.

The gather at the top of that block asks for a container statement type:

auto offloaded_tasks = irpass::analysis::gather_statements(root, [&](Stmt *s) { return s->is<OffloadedStmt>(); });

gather_statements runs its predicate from StmtSearcher::visit(Stmt *), but BasicStmtVisitor claims
OffloadedStmt with a typed overload that calls preprocess_container_stmt and recurses into the body without
consulting the predicate. StmtSearcher does not override that hook, so an OffloadedStmt predicate matches
nothing. The same applies to MeshForStmt, StructForStmt, RangeForStmt, IfStmt and WhileStmt.

Impact

  • The trip-count clipping is dead. Enabling it should reduce autodiff stack memory, but the code has never run, so
    it needs to be validated rather than just switched on.
  • eliminate_recomputable_pushes.cpp builds a preorder index over "all" statements and its comment reasons about
    "the container's preorder", which containers do not currently get. Relative order among non-container statements
    is unaffected, so this looks latent rather than active.
  • Callers whose predicates test non-container types are unaffected, as is auto_diff_common.h, which gathers
    everything but only casts to GlobalStoreStmt/AtomicOpStmt/SNodeOpStmt.

Note

I hit this from a different direction: a mesh-for guard written as a gather_statements predicate silently matched
nothing, which sent mesh kernels down a path that produced IR that segfaulted at launch.

Rather than change what the helper returns to its existing callers, my PR adds an include_containers parameter
defaulting to the current behaviour and opts in at the three sites that need it. That deliberately leaves this
autodiff case as-is, since flipping it on is a behaviour change to autodiff that should be made on purpose and
tested by someone who owns that code, not as a side effect of a compile-caching change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions