Merge pull request #1453 from ltratt/opt_guard_dup

Remove duplicated guards
ykjit · Nov 4, 2024 · 4c2a05a · 4c2a05a
2 parents 9ec0d37 + 3e9cc9c
commit 4c2a05a
Showing 1 changed file with 55 additions and 7 deletions.
diff --git a/ykrt/src/compile/jitc_yk/opt/mod.rs b/ykrt/src/compile/jitc_yk/opt/mod.rs
@@ -298,7 +298,27 @@ impl Opt {
                         // doesn't affect future analyses.
                         self.m.replace(iidx, Inst::Tombstone);
                     } else {
-                        self.an.guard(&self.m, x);
+                        // Remove guards that reference the same i1 variable: if the earlier guard
+                        // succeeded, then this guard must also succeed, by definition.
+                        let mut removed = false;
+                        // OPT: This is O(n), but most instructions can't possibly be guards.
+                        for back_iidx in (0..usize::from(iidx)).rev() {
+                            let back_iidx = InstIdx::unchecked_from(back_iidx);
+                            // Only examine non-`Copy` instructions, to avoid us continually checking the same
+                            // (subset of) instructions over and over again.
+                            let Some(Inst::Guard(y)) = self.m.inst_nocopy(back_iidx) else {
+                                continue;
+                            };
+                            if x.cond(&self.m) == y.cond(&self.m) {
+                                debug_assert_eq!(x.expect, y.expect);
+                                self.m.replace(iidx, Inst::Tombstone);
+                                removed = true;
+                                break;
+                            }
+                        }
+                        if !removed {
+                            self.an.guard(&self.m, x);
+                        }
                     }
                 }
                 Inst::PtrAdd(x) => match self.an.op_map(&self.m, x.ptr(&self.m)) {
@@ -379,19 +399,24 @@ impl Opt {
         if let Inst::Tombstone = inst {
             return;
         }
-        // FIXME: This is O(n), but most instructions can't possibly be CSE candidates.
+        // We don't perform CSE on instructions that have / enforce effects.
+        if inst.has_store_effect(&self.m)
+            || inst.has_load_effect(&self.m)
+            || inst.is_barrier(&self.m)
+        {
+            return;
+        }
+
+        // OPT: This is O(n), but most instructions can't possibly be CSE candidates.
         for back_iidx in (0..usize::from(iidx)).rev() {
             let back_iidx = InstIdx::unchecked_from(back_iidx);
             // Only examine non-`Copy` instructions, to avoid us continually checking the same
             // (subset of) instructions over and over again.
             let Some(back) = self.m.inst_nocopy(back_iidx) else {
                 continue;
             };
-            if !inst.has_store_effect(&self.m)
-                && !inst.has_load_effect(&self.m)
-                && !inst.is_barrier(&self.m)
-                && inst.decopy_eq(&self.m, back)
-            {
+
+            if inst.decopy_eq(&self.m, back) {
                 self.m.replace(iidx, Inst::Copy(back_iidx));
                 return;
             }
@@ -974,4 +999,27 @@ mod test {
         ",
         );
     }
+
+    #[test]
+    fn opt_guard_dup() {
+        Module::assert_ir_transform_eq(
+            "
+          entry:
+            %0: i8 = load_ti 0
+            %1: i8 = load_ti 1
+            %2: i1 = eq %0, %1
+            guard true, %2, [%0, %1]
+            guard true, %2, [%0, %1]
+        ",
+            |m| opt(m).unwrap(),
+            "
+          ...
+          entry:
+            %0: i8 = load_ti ...
+            %1: i8 = load_ti ...
+            %2: i1 = eq %0, %1
+            guard true, %2, ...
+        ",
+        );
+    }
 }