Skip to content

Commit f98c401

Browse files
committed
refactor: address CM4 review (elem message, cond all-branch caveat)
- Split :elem out of the :while raise with an accurate message — :elem is a generic tuple projection (defn while AND multi-output ops), not while- specific, so the old "while loops" diagnostic could mislead. - Caveat the cond comment: a not-taken branch is still computed; on MLX an out-of-bounds gather there clamps (so the discarded value never changes the result), but a hard-faulting op on a not-taken path would diverge from the Evaluator's lazy single-branch eval.
1 parent c2d8979 commit f98c401

1 file changed

Lines changed: 24 additions & 9 deletions

File tree

lib/emily/ir.ex

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -527,9 +527,14 @@ defmodule Emily.IR do
527527
# a select chain `where(p1, b1, where(p2, b2, ... last))`. ALL branches
528528
# are evaluated (Nx branches are side-effect-free and shape-compatible);
529529
# the result value matches the Evaluator's chosen branch exactly — only
530-
# the cost differs (not-taken branches are computed and discarded). The
531-
# predicate is a whole-tensor scalar bool, so `where` selects a branch
532-
# wholesale.
530+
# the cost differs (not-taken branches are computed and discarded by the
531+
# elementwise select). The predicate is a whole-tensor scalar bool, so
532+
# `where` selects a branch wholesale.
533+
#
534+
# Caveat: a not-taken branch is still computed. On MLX an out-of-bounds
535+
# gather/index there clamps rather than faults, so the discarded value
536+
# never changes the result; a hard-faulting op on a not-taken path would
537+
# diverge from the Evaluator's lazy single-branch eval.
533538
defp lower_op(%T{data: %Nx.Defn.Expr{op: :cond, args: [clauses, last]}} = t, state) do
534539
{last_ref, state} = lower_node(last, state)
535540

@@ -569,13 +574,23 @@ defmodule Emily.IR do
569574
"reduce_min) where possible."
570575
end
571576

572-
# while / its tuple projection are deferred to a follow-up: the
573-
# single-NIF replay has no loop construct, so a data-dependent while
574-
# needs either static-trip unrolling or a worker-side synced loop.
575-
defp lower_op(%T{data: %Nx.Defn.Expr{op: op}}, _state) when op in [:while, :elem] do
577+
# while is deferred to a follow-up: the single-NIF replay has no loop
578+
# construct, so a data-dependent while needs static-trip unrolling or a
579+
# worker-side synced loop. defn while is not used by the core transformer
580+
# forwards (decode/generation loops run in Elixir today).
581+
defp lower_op(%T{data: %Nx.Defn.Expr{op: :while}}, _state) do
582+
raise ArgumentError,
583+
"Emily Expr compiler does not yet lower defn `while` loops (deferred — " <>
584+
"the single-NIF replay has no loop construct)."
585+
end
586+
587+
# :elem is a tuple projection, emitted for any tuple-returning expression
588+
# (defn `while`, multi-output ops). Deferred alongside the constructs
589+
# that produce surviving tuples.
590+
defp lower_op(%T{data: %Nx.Defn.Expr{op: :elem}}, _state) do
576591
raise ArgumentError,
577-
"Emily Expr compiler does not yet lower #{inspect(op)} (defn `while` " <>
578-
"loops; deferred — the decode/generation loops run in Elixir today)."
592+
"Emily Expr compiler does not yet lower :elem (tuple projection) — it " <>
593+
"arises from defn `while` and multi-output ops, which are deferred."
579594
end
580595

581596
defp lower_op(%T{data: %Nx.Defn.Expr{op: op}}, _state) do

0 commit comments

Comments
 (0)