From ba9d9814a2749c8a19d63290669168b873c8aab9 Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliacomputing.com>
Date: Mon, 7 Jan 2019 13:30:25 -0500
Subject: [PATCH] Fix SROA confusing new and old nodes

SROA was accidentally treating a pending node as old and thus
getting the wrong type when querying the predecessor. As a result
it thought one of the paths was unreachable causing undefined data
to be introduced on that path (generally the `1.0` that happened to
already be in register).

Fix #29983

(cherry picked from commit da0179c4d60c0fa3e5d64c08972773c82d57e4e6)
---
 base/compiler/ssair/passes.jl | 12 +++++++++---
 test/compiler/irpasses.jl     | 29 +++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 test/compiler/irpasses.jl

diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl
index ce04b81bac1eb..83e4fa09968d3 100644
--- a/base/compiler/ssair/passes.jl
+++ b/base/compiler/ssair/passes.jl
@@ -191,7 +191,7 @@ function walk_to_defs(compact::IncrementalCompact, @nospecialize(defssa), @nospe
                 collect(Iterators.filter(1:length(def.edges)) do n
                     isassigned(def.values, n) || return false
                     val = def.values[n]
-                    if isa(defssa, OldSSAValue) && isa(val, SSAValue)
+                    if is_old(compact, defssa) && isa(val, SSAValue)
                         val = OldSSAValue(val.id)
                     end
                     edge_typ = widenconst(compact_exprtype(compact, val))
@@ -201,7 +201,7 @@ function walk_to_defs(compact::IncrementalCompact, @nospecialize(defssa), @nospe
             for n in possible_predecessors
                 pred = def.edges[n]
                 val = def.values[n]
-                if isa(defssa, OldSSAValue) && isa(val, SSAValue)
+                if is_old(compact, defssa) && isa(val, SSAValue)
                     val = OldSSAValue(val.id)
                 end
                 if isa(val, AnySSAValue)
@@ -425,6 +425,12 @@ struct LiftedPhi
     need_argupdate::Bool
 end
 
+function is_old(compact, @nospecialize(old_node_ssa))
+    isa(old_node_ssa, OldSSAValue) &&
+        !is_pending(compact, old_node_ssa) &&
+        !already_inserted(compact, old_node_ssa)
+end
+
 function perform_lifting!(compact::IncrementalCompact,
         visited_phinodes::Vector{Any}, @nospecialize(cache_key),
         lifting_cache::IdDict{Pair{AnySSAValue, Any}, AnySSAValue},
@@ -455,7 +461,7 @@ function perform_lifting!(compact::IncrementalCompact,
             isassigned(old_node.values, i) || continue
             val = old_node.values[i]
             orig_val = val
-            if isa(old_node_ssa, OldSSAValue) && !is_pending(compact, old_node_ssa) && !already_inserted(compact, old_node_ssa) && isa(val, SSAValue)
+            if is_old(compact, old_node_ssa) && isa(val, SSAValue)
                 val = OldSSAValue(val.id)
             end
             if isa(val, Union{NewSSAValue, SSAValue, OldSSAValue})
diff --git a/test/compiler/irpasses.jl b/test/compiler/irpasses.jl
new file mode 100644
index 0000000000000..e37b9fa5c4a0e
--- /dev/null
+++ b/test/compiler/irpasses.jl
@@ -0,0 +1,29 @@
+# This file is a part of Julia. License is MIT: https://julialang.org/license
+
+using Test
+
+# Issue #29983
+# This one is a bit hard to trigger, but the key is to create a case
+# where SROA needs to introduce an intermediate type-unstable phi node
+struct Foo29983{T}
+    x::Tuple{T}
+end
+struct Bar29983{S}
+    x::S
+end
+Base.:+(a::T, b::Bar29983{S}) where {T, S} = Bar29983(a + b.x)
+Base.:+(a::Bar29983{S}, b::T) where {T, S} = b + a
+Base.:+(a::Bar29983{S}, b::Bar29983{T}) where {T, S} = Bar29983(a.x + b.x)
+Base.:+(a::Foo29983, b::Foo29983) = Foo29983((a.x[1] + b.x[1],))
+
+function f(x::Vector{T}) where {T}
+    x1 = Foo29983((x[1],))
+    la1 = Foo29983((x[1],))
+    f1 = Foo29983((0,))
+    for _ in 1:2
+        f1 += la1
+    end
+    return f1
+end
+
+@test f([Bar29983(1.0)]).x[1].x == 2.0