lowering: Optimize lowering of tryfinally with trivial finally block (#…

…52593)

This optimizes the lowering of a tryfinally block with empty finally
block to instead use the try/catch lowering, where the catch block is
given as simply `rethrow()`. This is equivalent semantically to
try/finally in this case, but the code structure is a lot simpler with
fewer basic blocks and without the auxiliary slot for tracking the
finally slot. The motivation here is to help the compiler optimize
better when using the `@with` macro, which has an empty `finally` block
(but uses the scope argument of 'tryfinally). The only problem with this
is that it violates the lowering assumptions I made in
#52527, so we'll probably need to
fix that first.

base/scopedvalues.jl

@@ -161,7 +161,7 @@ macro with(exprs...)
         error("@with expects at least one argument")
     end
     exprs = map(esc, exprs)
-    Expr(:tryfinally, esc(ex), :(), :(Scope(Core.current_scope()::Union{Nothing, Scope}, $(exprs...))))
+    Expr(:tryfinally, esc(ex), nothing, :(Scope(Core.current_scope()::Union{Nothing, Scope}, $(exprs...))))
 end
 
 """

src/julia-syntax.scm

@@ -4274,6 +4274,17 @@ f(x) = yt(x)
   ;; returning lambda directly is needed for @generated
   (or (valid-ir-argument? e) (and (pair? e) (memq (car e) '(lambda)))))
 
+(define (code-trivially-effect-free? e)
+  ;; determine whether the execution of this code can be observed.
+  ;; If not it may be deleted. In general, the only thing we can detect here
+  ;; is empty blocks that only have metadata in them.
+  (if (pair? e)
+    (case (car e)
+      ((block) (every code-trivially-effect-free? (cdr e)))
+      ((line null) #t)
+      (else #f))
+    #t))
+
 ;; this pass behaves like an interpreter on the given code.
 ;; to perform stateful operations, it calls `emit` to record that something
 ;; needs to be done. in value position, it returns an expression computing
@@ -4726,10 +4737,13 @@ f(x) = yt(x)
             ((trycatch tryfinally trycatchelse)
              (let ((handler-token (make-ssavalue))
                    (catch (make-label))
+                   (catchcode (if (eq? (car e) 'tryfinally) '(call (top rethrow)) (caddr e)))
                    (els   (and (eq? (car e) 'trycatchelse) (make-label)))
                    (endl  (make-label))
                    (last-finally-handler finally-handler)
-                   (finally           (if (eq? (car e) 'tryfinally) (new-mutable-var) #f))
+                   ;; Special case optimization: If the finally block is trivially empty, don't perform finally
+                   ;; lowering, just lower this as a try/catch block with rethrow and scope hnadling.
+                   (finally           (if (and (eq? (car e) 'tryfinally) (not (code-trivially-effect-free? (caddr e)))) (new-mutable-var) #f))
                    (scope             (if (eq? (car e) 'tryfinally) (cdddr e) '()))
                    (my-finally-handler #f))
                ;; handler block entry
@@ -4738,7 +4752,7 @@ f(x) = yt(x)
                (if finally (begin (set! my-finally-handler (list finally endl '() handler-token-stack catch-token-stack))
                                   (set! finally-handler my-finally-handler)
                                   (emit `(= ,finally -1))))
-               (let* ((v1  (compile (cadr e) break-labels value #f)) ;; emit try block code
+               (let* ((v1 (compile (cadr e) break-labels value #f)) ;; emit try block code
                       (val (if (and value (not tail))
                                (new-mutable-var) #f)))
                  ;; handler block postfix
@@ -4762,7 +4776,7 @@ f(x) = yt(x)
                  ;; separate trycatch and tryfinally blocks earlier.
                  (mark-label catch)
                  (if finally
-                     (begin (enter-finally-block '(call (top rethrow)) #f) ;; enter block via exception
+                     (begin (enter-finally-block catchcode #f) ;; enter block via exception
                             (mark-label endl) ;; non-exceptional control flow enters here
                             (set! finally-handler last-finally-handler)
                             (compile (caddr e) break-labels #f #f)
@@ -4786,7 +4800,7 @@ f(x) = yt(x)
                                     (if skip (mark-label skip))
                                     (loop (cdr actions))))))
                      (begin (set! catch-token-stack (cons handler-token catch-token-stack))
-                            (let ((v2 (compile (caddr e) break-labels value tail)))
+                            (let ((v2 (compile catchcode break-labels value tail)))
                               (if val (emit-assignment val v2))
                               (if (not tail) (emit `(pop_exception ,handler-token)))
                                              ;; else done in emit-return from compile

test/scopedvalues.jl

@@ -126,3 +126,17 @@ end
         @test sval_float[] == 1.0
     end
 end
+
+# Test that the `@with` macro doesn't introduce unnecessary PhiC nodes
+# (which can be hard for the optimizer to remove).
+function with_macro_slot_cross()
+    a = 1
+    @with sval=>1 begin
+        a = sval_float[]
+    end
+    return a
+end
+
+let code = code_typed(with_macro_slot_cross)[1][1].code
+    @test !any(x->isa(x, Core.PhiCNode), code)
+end