From a605d5c9c9b08181b39987a4f132be4f860d84c4 Mon Sep 17 00:00:00 2001
From: Slava Pestov <slava@slava-pestovs-macbook-pro.local>
Date: Mon, 6 Sep 2010 16:57:56 -0700
Subject: [PATCH] compiler.cfg.alias-analysis: dead store elimination was too
 aggressive, can't eliminate dead stores across a GC call even for fresh
 allocations because GC will see uninitialized data

---
 .../alias-analysis-tests.factor               | 91 ++++++++++++++++++-
 .../cfg/alias-analysis/alias-analysis.factor  | 42 ++++++---
 2 files changed, 116 insertions(+), 17 deletions(-)

diff --git a/basis/compiler/cfg/alias-analysis/alias-analysis-tests.factor b/basis/compiler/cfg/alias-analysis/alias-analysis-tests.factor
index adf5d61a25..21241e6f4a 100644
--- a/basis/compiler/cfg/alias-analysis/alias-analysis-tests.factor
+++ b/basis/compiler/cfg/alias-analysis/alias-analysis-tests.factor
@@ -288,8 +288,8 @@ IN: compiler.cfg.alias-analysis.tests
     } test-alias-analysis
 ] unit-test
 
-! We can't make any assumptions about heap-ac between alien
-! calls, since they might callback into Factor code
+! We can't make any assumptions about heap-ac between
+! instructions which can call back into Factor code
 [
     V{
         T{ ##peek f 0 D 0 }
@@ -359,3 +359,90 @@ IN: compiler.cfg.alias-analysis.tests
         T{ ##set-slot-imm f 1 0 1 0 }
     } test-alias-analysis
 ] unit-test
+
+! We can't eliminate stores on any alias class across a GC-ing
+! instruction
+[
+    V{
+        T{ ##allot f 0 }
+        T{ ##slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##copy f 2 1 any-rep }
+    }
+] [
+    V{
+        T{ ##allot f 0 }
+        T{ ##slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##slot-imm f 2 0 1 0 }
+    } test-alias-analysis
+] unit-test
+
+[
+    V{
+        T{ ##allot f 0 }
+        T{ ##peek f 1 D 1 }
+        T{ ##set-slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##copy f 2 1 any-rep }
+    }
+] [
+    V{
+        T{ ##allot f 0 }
+        T{ ##peek f 1 D 1 }
+        T{ ##set-slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##slot-imm f 2 0 1 0 }
+    } test-alias-analysis
+] unit-test
+
+[
+    V{
+        T{ ##allot f 0 }
+        T{ ##peek f 1 D 1 }
+        T{ ##peek f 2 D 2 }
+        T{ ##set-slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##set-slot-imm f 2 0 1 0 }
+    }
+] [
+    V{
+        T{ ##allot f 0 }
+        T{ ##peek f 1 D 1 }
+        T{ ##peek f 2 D 2 }
+        T{ ##set-slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##set-slot-imm f 2 0 1 0 }
+    } test-alias-analysis
+] unit-test
+
+[
+    V{
+        T{ ##allot f 0 }
+        T{ ##slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+    }
+] [
+    V{
+        T{ ##allot f 0 }
+        T{ ##slot-imm f 1 0 1 0 }
+        T{ ##alien-invoke f { } { } { } { } 0 0 "free" }
+        T{ ##set-slot-imm f 1 0 1 0 }
+    } test-alias-analysis
+] unit-test
+
+! Make sure that gc-map-insns which are also vreg-insns are
+! handled properly
+[
+    V{
+        T{ ##allot f 0 }
+        T{ ##alien-indirect f { } { } { { 2 double-rep 0 } } { } 0 0 "free" }
+        T{ ##set-slot-imm f 2 0 1 0 }
+    }
+] [
+    V{
+        T{ ##allot f 0 }
+        T{ ##alien-indirect f { } { } { { 2 double-rep 0 } } { } 0 0 "free" }
+        T{ ##set-slot-imm f 2 0 1 0 }
+    } test-alias-analysis
+] unit-test
diff --git a/basis/compiler/cfg/alias-analysis/alias-analysis.factor b/basis/compiler/cfg/alias-analysis/alias-analysis.factor
index 5ba0bd1300..6fff3f0216 100644
--- a/basis/compiler/cfg/alias-analysis/alias-analysis.factor
+++ b/basis/compiler/cfg/alias-analysis/alias-analysis.factor
@@ -218,7 +218,7 @@ GENERIC: analyze-aliases ( insn -- insn' )
 
 M: insn analyze-aliases ;
 
-M: vreg-insn analyze-aliases
+: def-acs ( insn -- insn' )
     ! If an instruction defines a value with a non-integer
     ! representation it means that the value will be boxed
     ! anywhere its used as a tagged pointer. Boxing allocates
@@ -229,6 +229,9 @@ M: vreg-insn analyze-aliases
         [ set-heap-ac ] [ set-new-ac ] if
     ] each-def-rep ;
 
+M: vreg-insn analyze-aliases
+    def-acs ;
+
 M: ##phi analyze-aliases
     dup dst>> set-heap-ac ;
 
@@ -286,6 +289,29 @@ M: ##compare analyze-aliases
         analyze-aliases
     ] when ;
 
+: clear-live-slots ( -- )
+    heap-ac get ac>vregs [ live-slots get at clear-assoc ] each ;
+
+: clear-recent-stores ( -- )
+    recent-stores get values [ clear-assoc ] each ;
+
+M: gc-map-insn analyze-aliases
+    ! Can't use call-next-method here because of a limitation, gah
+    def-acs
+    clear-recent-stores ;
+
+M: factor-call-insn analyze-aliases
+    def-acs
+    clear-recent-stores
+    clear-live-slots ;
+
+GENERIC: eliminate-dead-stores ( insn -- ? )
+
+M: ##set-slot-imm eliminate-dead-stores
+    insn#>> dead-stores get in? not ;
+
+M: insn eliminate-dead-stores drop t ;
+
 : reset-alias-analysis ( -- )
     recent-stores get clear-assoc
     vregs>acs get clear-assoc
@@ -298,20 +324,6 @@ M: ##compare analyze-aliases
     \ ##vm-field set-new-ac
     \ ##alien-global set-new-ac ;
 
-M: factor-call-insn analyze-aliases
-    call-next-method
-    heap-ac get ac>vregs [
-        [ live-slots get at clear-assoc ]
-        [ recent-stores get at clear-assoc ] bi
-    ] each ;
-
-GENERIC: eliminate-dead-stores ( insn -- ? )
-
-M: ##set-slot-imm eliminate-dead-stores
-    insn#>> dead-stores get in? not ;
-
-M: insn eliminate-dead-stores drop t ;
-
 : alias-analysis-step ( insns -- insns' )
     reset-alias-analysis
     [ local-live-in [ set-heap-ac ] each ]