From afc1c5aac5fa5d80e79bd6e26a420f4fd8f1986b Mon Sep 17 00:00:00 2001 From: Jakub Doka Date: Fri, 15 Nov 2024 14:36:33 +0100 Subject: [PATCH] orginizing null checks better to get more peephole hits --- lang/README.md | 6 +- lang/src/son.rs | 96 ++++++--------- lang/tests/son_tests_inlining_issues.txt | 115 +++++++++--------- lang/tests/son_tests_needless_unwrap.txt | 12 +- ...ests_null_check_returning_small_global.txt | 30 ++--- lang/tests/son_tests_request_page.txt | 11 +- ...son_tests_storing_into_nullable_struct.txt | 14 +-- 7 files changed, 132 insertions(+), 152 deletions(-) diff --git a/lang/README.md b/lang/README.md index ae70755d..4cdecb48 100644 --- a/lang/README.md +++ b/lang/README.md @@ -957,10 +957,10 @@ main := fn(): int { ```hb main := fn(): uint { always_nn := @as(?^uint, &0) - ptr := @unwrap(always_nn) + ptr1 := @unwrap(always_nn) always_n := @as(?^uint, null) - ptr = @unwrap(always_n) - return *ptr + ptr2 := @unwrap(always_n) + return *ptr1 + *ptr2 } ``` diff --git a/lang/src/son.rs b/lang/src/son.rs index 45c5862c..0bc4cd04 100644 --- a/lang/src/son.rs +++ b/lang/src/son.rs @@ -1362,8 +1362,36 @@ impl Nodes { return Some(self[target].inputs[0]); } } + K::Die => { + if self[target].inputs[0] == NEVER { + return Some(NEVER); + } + } + K::Assert { kind, .. } => 'b: { + let pin = match (kind, self.try_match_cond(target)) { + (AssertKind::NullCheck, CondOptRes::Known { value: false, pin }) => pin, + (AssertKind::UnwrapCheck, CondOptRes::Unknown) => None, + _ => break 'b, + } + .unwrap_or(self[target].inputs[0]); + + for out in self[target].outputs.clone() { + if !self[out].kind.is_pinned() && self[out].inputs[0] != pin { + self.modify_input(out, 0, pin); + } + } + return Some(self[target].inputs[2]); + } _ if self.is_cfg(target) && self.idom(target) == NEVER => panic!(), - _ => {} + K::Start + | K::Entry + | K::Mem + | K::Loops + | K::End + | K::CInt { .. } + | K::Arg + | K::Global { .. } + | K::Join => {} } None @@ -1923,7 +1951,8 @@ impl Kind { } fn is_pinned(&self) -> bool { - self.is_cfg() || matches!(self, Self::Phi | Self::Arg | Self::Mem | Self::Loops) + self.is_cfg() + || matches!(self, Self::Phi | Self::Arg | Self::Mem | Self::Loops | Kind::Assert { .. }) } fn is_cfg(&self) -> bool { @@ -1937,7 +1966,6 @@ impl Kind { | Self::Then | Self::Else | Self::Call { .. } - | Self::Assert { .. } | Self::If | Self::Region | Self::Loop @@ -2257,9 +2285,7 @@ impl ItemCtx { mem::take(&mut self.ctrl).soft_remove(&mut self.nodes); self.nodes.iter_peeps(1000, stack, tys); - } - fn unlock(&mut self) { self.nodes.unlock(MEM); self.nodes.unlock(NEVER); self.nodes.unlock(LOOPS); @@ -4383,7 +4409,7 @@ impl<'a> Codegen<'a> { self.ci.finalize(&mut self.pool.nid_stack, self.tys, self.files); - let mut to_remove = vec![]; + //let mut to_remove = vec![]; for (id, node) in self.ci.nodes.iter() { let Kind::Assert { kind, pos } = node.kind else { continue }; @@ -4408,49 +4434,10 @@ impl<'a> Codegen<'a> { or explicitly check for null and handle it \ ('if == null { /* handle */ } else { /* use opt */ }')" } - (AK::NullCheck, CR::Known { value: false, pin }) => { - to_remove.push((id, pin)); - continue; - } - (AK::UnwrapCheck, CR::Unknown) => { - to_remove.push((id, None)); - continue; - } + _ => unreachable!(), }; self.report(pos, msg); } - to_remove.into_iter().for_each(|(n, pin)| { - let pin = pin.unwrap_or_else(|| { - let mut pin = self.ci.nodes[n].inputs[0]; - while matches!(self.ci.nodes[pin].kind, Kind::Assert { .. }) { - pin = self.ci.nodes[n].inputs[0]; - } - pin - }); - for mut out in self.ci.nodes[n].outputs.clone() { - if self.ci.nodes.is_cfg(out) { - let index = self.ci.nodes[out].inputs.iter().position(|&p| p == n).unwrap(); - self.ci.nodes.modify_input(out, index, self.ci.nodes[n].inputs[0]); - } else { - if !self.ci.nodes[out].kind.is_pinned() && self.ci.nodes[out].inputs[0] != pin { - out = self.ci.nodes.modify_input(out, 0, pin); - } - let index = - self.ci.nodes[out].inputs[1..].iter().position(|&p| p == n).unwrap() + 1; - self.ci.nodes.modify_input(out, index, self.ci.nodes[n].inputs[2]); - } - } - debug_assert!( - self.ci.nodes.values[n as usize] - .as_ref() - .map_or(true, |n| !matches!(n.kind, Kind::Assert { .. })), - "{:?} {:?}", - self.ci.nodes[n], - self.ci.nodes[n].outputs.iter().map(|&o| &self.ci.nodes[o]).collect::>(), - ); - }); - - self.ci.unlock(); for &node in self.ci.nodes[NEVER].inputs.iter() { if self.ci.nodes[node].kind == Kind::Return @@ -4607,16 +4594,13 @@ impl<'a> Codegen<'a> { self.unwrap_opt_unchecked(ty, oty, opt); // TODO: extract the if check int a fucntion - self.ci.ctrl.set( - self.ci.nodes.new_node_nop(oty, Kind::Assert { kind, pos }, [ - self.ci.ctrl.get(), - null_check, - opt.id, - ]), - &mut self.ci.nodes, - ); - self.ci.nodes.pass_aclass(self.ci.nodes.aclass_index(opt.id).1, self.ci.ctrl.get()); - opt.id = self.ci.ctrl.get(); + let ass = self.ci.nodes.new_node_nop(oty, Kind::Assert { kind, pos }, [ + self.ci.ctrl.get(), + null_check, + opt.id, + ]); + self.ci.nodes.pass_aclass(self.ci.nodes.aclass_index(opt.id).1, ass); + opt.id = ass; } fn unwrap_opt_unchecked(&mut self, ty: ty::Id, oty: ty::Id, opt: &mut Value) { diff --git a/lang/tests/son_tests_inlining_issues.txt b/lang/tests/son_tests_inlining_issues.txt index e8fd8ac8..79e66ab8 100644 --- a/lang/tests/son_tests_inlining_issues.txt +++ b/lang/tests/son_tests_inlining_issues.txt @@ -27,8 +27,8 @@ main: ADDI64 r254, r254, 90d JALA r0, r31, 0a put_filled_rect: - ADDI64 r254, r254, -212d - ST r32, r254, 108a, 104h + ADDI64 r254, r254, -220d + ST r32, r254, 108a, 112h ST r3, r254, 92a, 16h ADDI64 r3, r254, 92d ST r5, r254, 76a, 16h @@ -36,74 +36,73 @@ put_filled_rect: ST r7, r254, 75a, 1h ADDI64 r7, r254, 75d LI64 r8, 25d - LI64 r6, 2d - LI64 r9, 8d - ADDI64 r32, r254, 25d - ADDI64 r33, r254, 50d - LI8 r34, 5b - ST r34, r254, 25a, 1h - LD r35, r5, 0a, 8h - ST r35, r254, 26a, 4h - LI64 r36, 1d - ST r36, r254, 30a, 4h + LI64 r32, 2d + LI64 r6, 8d + ADDI64 r33, r254, 25d + ADDI64 r34, r254, 50d + LI8 r35, 5b + ST r35, r254, 25a, 1h + LD r36, r5, 0a, 8h + ST r36, r254, 26a, 4h + LI64 r37, 1d + ST r37, r254, 30a, 4h ST r7, r254, 34a, 8h - ST r34, r254, 50a, 1h - ST r35, r254, 51a, 4h - ST r36, r254, 55a, 4h + ST r35, r254, 50a, 1h + ST r36, r254, 51a, 4h + ST r37, r254, 55a, 4h ST r7, r254, 59a, 8h - CP r37, r7 - LD r4, r3, 8a, 8h - LD r38, r5, 8a, 8h - ADD64 r1, r38, r4 - SUB64 r5, r1, r36 - LD r39, r2, 8a, 8h - MUL64 r7, r39, r5 + CP r38, r7 + LD r7, r3, 8a, 8h + LD r39, r5, 8a, 8h + ADD64 r11, r39, r7 + SUB64 r4, r11, r37 + LD r40, r2, 8a, 8h + MUL64 r5, r40, r4 LD r10, r2, 0a, 8h - ADD64 r11, r10, r7 - LD r5, r3, 0a, 8h - ADD64 r40, r5, r11 - MUL64 r4, r39, r4 - ADD64 r7, r10, r4 - ADD64 r41, r5, r7 - 3: JGTU r38, r36, :0 - JNE r38, r36, :1 + ADD64 r9, r10, r5 + LD r41, r3, 0a, 8h + ADD64 r42, r41, r9 + MUL64 r43, r40, r7 + 3: ADD64 r43, r43, r10 + ADD64 r9, r43, r41 + JGTU r39, r37, :0 + JNE r39, r37, :1 ADDI64 r4, r254, 0d - ST r34, r254, 0a, 1h - ST r35, r254, 1a, 4h - ST r36, r254, 5a, 4h - ST r37, r254, 9a, 8h - ST r41, r254, 17a, 8h - CP r2, r9 - CP r3, r6 + ST r35, r254, 0a, 1h + ST r36, r254, 1a, 4h + ST r37, r254, 5a, 4h + ST r38, r254, 9a, 8h + ST r9, r254, 17a, 8h + CP r2, r6 + CP r3, r32 CP r5, r8 ECA JMP :1 1: JMP :2 - 0: CP r3, r6 - CP r42, r9 - CP r43, r8 - ST r41, r254, 67a, 8h - CP r44, r3 - CP r2, r42 + 0: CP r3, r32 + CP r44, r6 + CP r45, r8 + ST r9, r254, 67a, 8h + CP r2, r44 + CP r4, r34 + CP r5, r45 + ECA + ST r42, r254, 42a, 8h + CP r2, r44 + CP r3, r32 CP r4, r33 - CP r5, r43 + CP r5, r45 ECA - ST r40, r254, 42a, 8h - CP r2, r42 - CP r3, r44 - CP r4, r32 - CP r5, r43 - ECA - SUB64 r40, r40, r39 - ADD64 r41, r39, r41 - SUB64 r38, r38, r44 + SUB64 r42, r42, r40 + SUB64 r39, r39, r32 + CP r10, r41 + CP r41, r40 + CP r8, r45 CP r6, r44 - CP r8, r43 - CP r9, r42 JMP :3 - 2: LD r32, r254, 108a, 104h - ADDI64 r254, r254, 212d + 2: LD r32, r254, 108a, 112h + ADDI64 r254, r254, 220d JALA r0, r31, 0a -code size: 910 +code size: 906 ret: 0 status: Ok(()) diff --git a/lang/tests/son_tests_needless_unwrap.txt b/lang/tests/son_tests_needless_unwrap.txt index 73b8726f..c57240e0 100644 --- a/lang/tests/son_tests_needless_unwrap.txt +++ b/lang/tests/son_tests_needless_unwrap.txt @@ -1,6 +1,6 @@ -test.hb:4:17: unwrap is not needed since the value is (provably) never null, remove it, or replace with '@as(, )' - ptr := @unwrap(always_nn) - ^ -test.hb:6:16: unwrap is incorrect since the value is (provably) always null, make sure your logic is correct - ptr = @unwrap(always_n) - ^ +test.hb:4:18: unwrap is not needed since the value is (provably) never null, remove it, or replace with '@as(, )' + ptr1 := @unwrap(always_nn) + ^ +test.hb:6:18: unwrap is incorrect since the value is (provably) always null, make sure your logic is correct + ptr2 := @unwrap(always_n) + ^ diff --git a/lang/tests/son_tests_null_check_returning_small_global.txt b/lang/tests/son_tests_null_check_returning_small_global.txt index e20ea67d..bebf927e 100644 --- a/lang/tests/son_tests_null_check_returning_small_global.txt +++ b/lang/tests/son_tests_null_check_returning_small_global.txt @@ -1,6 +1,6 @@ foo: - ADDI64 r254, r254, -184d - ST r31, r254, 80a, 104h + ADDI64 r254, r254, -176d + ST r31, r254, 80a, 96h ADDI64 r32, r254, 64d LRA r3, r0, :some_file JAL r31, r0, :get @@ -17,25 +17,25 @@ foo: LI64 r36, 4d LD r37, r254, 72a, 8h JNE r37, r36, :2 - ADDI64 r38, r254, 32d + ADDI64 r37, r254, 32d ST r35, r254, 32a, 1h - LI64 r39, 2d - ST r39, r254, 40a, 8h - LD r1, r38, 0a, 16h + LI64 r38, 2d + ST r38, r254, 40a, 8h + LD r1, r37, 0a, 16h JMP :1 - 2: LRA r40, r0, :MAGIC - LD r41, r40, 0a, 8h - JNE r41, r37, :3 - ADDI64 r42, r254, 16d + 2: LRA r39, r0, :MAGIC + LD r40, r39, 0a, 8h + JNE r40, r37, :3 + ADDI64 r41, r254, 16d ST r35, r254, 16a, 1h ST r0, r254, 24a, 8h - LD r1, r42, 0a, 16h + LD r1, r41, 0a, 16h JMP :1 - 3: ADDI64 r43, r254, 0d + 3: ADDI64 r42, r254, 0d ST r0, r254, 0a, 1h - LD r1, r43, 0a, 16h - 1: LD r31, r254, 80a, 104h - ADDI64 r254, r254, 184d + LD r1, r42, 0a, 16h + 1: LD r31, r254, 80a, 96h + ADDI64 r254, r254, 176d JALA r0, r31, 0a get: ADDI64 r254, r254, -32d diff --git a/lang/tests/son_tests_request_page.txt b/lang/tests/son_tests_request_page.txt index 470440c9..fcf8702e 100644 --- a/lang/tests/son_tests_request_page.txt +++ b/lang/tests/son_tests_request_page.txt @@ -9,9 +9,9 @@ create_back_buffer: LI8 r34, 255b CP r2, r34 JAL r31, r0, :request_page - CP r2, r33 - SUB64 r35, r2, r32 - 5: JGTS r35, r0, :2 + CP r35, r33 + 5: SUB64 r35, r35, r32 + JGTS r35, r0, :2 JMP :1 2: CP r36, r1 JLTS r35, r32, :3 @@ -20,8 +20,7 @@ create_back_buffer: JMP :4 3: CP r2, r35 JAL r31, r0, :request_page - 4: SUB64 r35, r35, r32 - CP r1, r36 + 4: CP r1, r36 JMP :5 1: LD r31, r254, 0a, 48h ADDI64 r254, r254, 48d @@ -42,6 +41,6 @@ request_page: LI64 r2, 3d ECA JALA r0, r31, 0a -code size: 321 +code size: 317 ret: 42 status: Ok(()) diff --git a/lang/tests/son_tests_storing_into_nullable_struct.txt b/lang/tests/son_tests_storing_into_nullable_struct.txt index 226507fa..9fd047e3 100644 --- a/lang/tests/son_tests_storing_into_nullable_struct.txt +++ b/lang/tests/son_tests_storing_into_nullable_struct.txt @@ -4,8 +4,8 @@ do_stuff: just_read: JALA r0, r31, 0a main: - ADDI64 r254, r254, -104d - ST r31, r254, 48a, 56h + ADDI64 r254, r254, -96d + ST r31, r254, 48a, 48h ADDI64 r32, r254, 16d CP r1, r32 JAL r31, r0, :optionala @@ -28,13 +28,11 @@ main: JNE r36, r0, :2 LI64 r1, 20d JMP :1 - 2: LI64 r37, 100d - ST r37, r254, 8a, 8h - LD r2, r254, 8a, 8h + 2: LI64 r2, 100d JAL r31, r0, :do_stuff ADD64 r1, r1, r34 - 1: LD r31, r254, 48a, 56h - ADDI64 r254, r254, 104d + 1: LD r31, r254, 48a, 48h + ADDI64 r254, r254, 96d JALA r0, r31, 0a optional: ADDI64 r254, r254, -16d @@ -61,6 +59,6 @@ optionala: BMC r6, r1, 32h ADDI64 r254, r254, 48d JALA r0, r31, 0a -code size: 580 +code size: 554 ret: 100 status: Ok(())