Skip to content

Commit f1b1acb

Browse files
lowering: remove ifa candidates when used before assignment
When a variable is used (read) before being assigned within a branch, it cannot safely have the never-undef optimization applied. Update mark-used to also check and remove ifa candidates in this case. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 87097d3 commit f1b1acb

File tree

2 files changed

+67
-22
lines changed

2 files changed

+67
-22
lines changed

src/julia-syntax.scm

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3937,7 +3937,8 @@ f(x) = yt(x)
39373937
(live (table)) ;; variables that have been set in the current block
39383938
(seen (table)) ;; all variables we've seen assignments to
39393939
(ifa (table)) ;; variables assigned in all branches of if-else ("if-assigned")
3940-
(has-ifa #f)) ;; whether ifa has any entries
3940+
(has-ifa #f) ;; whether ifa has any entries
3941+
(has-symbolic-goto #f)) ;; whether we've seen a @goto (disables ifa optimization)
39413942
;; Collect candidate variables: those that are captured (and hence we want to optimize)
39423943
;; and only assigned once. This populates the initial `unused` table.
39433944
;; Also collect captured variables assigned more than once for if-branch analysis.
@@ -3962,10 +3963,19 @@ f(x) = yt(x)
39623963
;; Note arguments are only "used" for purposes of this analysis when
39633964
;; they are captured, since they are never undefined.
39643965
(if (and (has? unused var) (not (memq var args)))
3965-
(del! unused var)))
3966+
(del! unused var))
3967+
;; Also remove from ifa if used before being assigned in this branch
3968+
;; (i.e., not currently in live)
3969+
(if (and (has? ifa var) (not (has? live var)))
3970+
(del! ifa var)))
39663971
(define (mark-captured var)
3967-
(if (has? unused var)
3968-
(del! unused var)))
3972+
;; Remove from unused only if not already assigned (not in seen).
3973+
;; If the variable was assigned before being captured, it's still safe.
3974+
(if (and (has? unused var) (not (has? seen var)))
3975+
(del! unused var))
3976+
;; Also remove from ifa if captured before assignment
3977+
(if (and (has? ifa var) (not (has? seen var)))
3978+
(del! ifa var)))
39693979
(define (assign! var)
39703980
(if (has? unused var)
39713981
;; When a variable is assigned, move it to the live set to protect
@@ -4003,6 +4013,8 @@ f(x) = yt(x)
40034013
(begin0 (visit (cadr e))
40044014
(kill)))
40054015
((memq (car e) '(break label symbolicgoto))
4016+
(if (eq? (car e) 'symbolicgoto)
4017+
(set! has-symbolic-goto #t))
40064018
(kill)
40074019
#f)
40084020
((eq? (car e) 'symboliclabel)
@@ -4012,10 +4024,11 @@ f(x) = yt(x)
40124024
;; Special handling for if-else: track variables assigned in ALL branches.
40134025
;; If a captured variable is assigned in ALL branches (and not used before
40144026
;; assignment in any), it's effectively single-assigned per control path.
4027+
;; Skip this optimization if there's a @goto in the function (could skip assignments).
40154028
(let ((prev (table.clone live)))
40164029
(cond
40174030
;; if-else with exactly 3 args (cond, then, else) and we have candidates
4018-
((and (length= e 4) has-ifa)
4031+
((and (length= e 4) has-ifa (not has-symbolic-goto))
40194032
(let ((has-label #f)
40204033
(all-assigned #f))
40214034
;; Visit condition
@@ -4058,14 +4071,16 @@ f(x) = yt(x)
40584071
(del! all-assigned var)))
40594072
all-assigned)))))
40604073
;; Mark variables assigned in all branches as effectively single-assigned
4061-
(table.foreach
4062-
(lambda (var _)
4063-
(if (has? all-assigned var)
4064-
(begin
4065-
(put! seen var #t)
4066-
(put! unused var #t)
4067-
(del! ifa var))))
4068-
ifa)
4074+
;; BUT only if no labels were found (labels could be jump targets from @goto)
4075+
(if (not has-label)
4076+
(table.foreach
4077+
(lambda (var _)
4078+
(if (has? all-assigned var)
4079+
(begin
4080+
(put! seen var #t)
4081+
(put! unused var #t)
4082+
(del! ifa var))))
4083+
ifa))
40694084
(kill)
40704085
(if has-label
40714086
(begin (kill) #t)
@@ -4127,17 +4142,21 @@ f(x) = yt(x)
41274142
(vinfo:set-never-undef! vv #t))))
41284143
(append (table.keys live) (table.keys unused)))
41294144
;; Clear captured flag for single-assigned never-undef variables
4130-
(for-each (lambda (v)
4131-
(if (and (vinfo:sa v) (vinfo:never-undef v))
4132-
(set-car! (cddr v) (logand (caddr v) (lognot 5)))))
4133-
vi)
4145+
;; Skip if there's a @goto since it could skip assignments
4146+
(if (not has-symbolic-goto)
4147+
(for-each (lambda (v)
4148+
(if (and (vinfo:sa v) (vinfo:never-undef v))
4149+
(set-car! (cddr v) (logand (caddr v) (lognot 5)))))
4150+
vi))
41344151
;; Also clear captured flag for variables that were assigned in all branches of an if-else
41354152
;; (these are in `unused` but not `ifa`, and have never-undef set)
4136-
(for-each (lambda (var)
4137-
(let ((vv (assq var vi)))
4138-
(if (and vv (vinfo:never-undef vv) (not (has? ifa var)))
4139-
(set-car! (cddr vv) (logand (caddr vv) (lognot 5))))))
4140-
(table.keys unused))
4153+
;; Skip if there's a @goto since it could skip assignments
4154+
(if (not has-symbolic-goto)
4155+
(for-each (lambda (var)
4156+
(let ((vv (assq var vi)))
4157+
(if (and vv (vinfo:never-undef vv) (not (has? ifa var)))
4158+
(set-car! (cddr vv) (logand (caddr vv) (lognot 5))))))
4159+
(table.keys unused)))
41414160
lam))
41424161

41434162
(define (is-var-boxed? v lam)

test/syntax.jl

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4747,4 +4747,30 @@ end
47474747
return () -> x
47484748
end
47494749
@test fieldtype(typeof(if_else_usefirst(true, 1, 2)), 1) === Core.Box
4750+
4751+
# Closure captured before assignment must still be boxed
4752+
function if_else_capture_before_assign(cond, a, b)
4753+
f = () -> x
4754+
if cond
4755+
x = a
4756+
else
4757+
x = b
4758+
end
4759+
return f
4760+
end
4761+
@test fieldtype(typeof(if_else_capture_before_assign(true, 1, 2)), 1) === Core.Box
4762+
4763+
# @goto can skip assignments, so must still be boxed
4764+
function if_else_with_goto(cond, a, b)
4765+
if cond
4766+
@goto skip
4767+
x = a
4768+
else
4769+
x = b
4770+
end
4771+
@label skip
4772+
x = 0
4773+
return () -> x
4774+
end
4775+
@test fieldtype(typeof(if_else_with_goto(true, 1, 2)), 1) === Core.Box
47504776
end

0 commit comments

Comments
 (0)