Function dedup conf decode (#6623)

## Description This PR improves `fn_dedup` to also deduplicate config decode functions. Given how early this optimization was running, some optimizations were being lost, because some functions only get identical at the final IR. I believe we chose to run this early to decrease the pressure in other passes, but to allow this pass to really coalesce decodes I think makes sense to run it again at the end. I am also deleting from the IR functions that were being replaced. This allowed the new test to decrease around 100 bytes. Our old configurable test went from 7600 to 7250 bytes. ![image](https://github.com/user-attachments/assets/b08d22f9-289b-44a7-b939-353e0e7f6abb) ## Checklist - [ ] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Joshua Batty <joshpbatty@gmail.com> Co-authored-by: IGI-111 <igi-111@protonmail.com> Co-authored-by: Kaya Gökalp <kaya.gokalp@fuel.sh>
FuelLabs · Oct 22, 2024 · 492db76 · 492db76
1 parent 79170db
commit 492db76
Show file tree

Hide file tree

Showing 24 changed files with 1,653 additions and 565 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/forc-plugins/forc-client/tests/deploy.rs b/forc-plugins/forc-client/tests/deploy.rs
@@ -373,7 +373,7 @@ async fn test_simple_deploy() {
     node.kill().unwrap();
     let expected = vec![DeployedPackage::Contract(DeployedContract {
         id: ContractId::from_str(
-            "4ea5fa100cd7c8972bc8925ed6f8ccfb6bf1e16f79c3642c3a503c73b7d18de2",
+            "e50f0b4b396504398c0aff45951b1e44e108399c856cb92257dfa58fe96d53eb",
         )
         .unwrap(),
         proxy: None,
@@ -416,7 +416,7 @@ async fn test_deploy_submit_only() {
     node.kill().unwrap();
     let expected = vec![DeployedPackage::Contract(DeployedContract {
         id: ContractId::from_str(
-            "4ea5fa100cd7c8972bc8925ed6f8ccfb6bf1e16f79c3642c3a503c73b7d18de2",
+            "e50f0b4b396504398c0aff45951b1e44e108399c856cb92257dfa58fe96d53eb",
         )
         .unwrap(),
         proxy: None,
@@ -462,12 +462,12 @@ async fn test_deploy_fresh_proxy() {
     node.kill().unwrap();
     let impl_contract = DeployedPackage::Contract(DeployedContract {
         id: ContractId::from_str(
-            "4ea5fa100cd7c8972bc8925ed6f8ccfb6bf1e16f79c3642c3a503c73b7d18de2",
+            "e50f0b4b396504398c0aff45951b1e44e108399c856cb92257dfa58fe96d53eb",
         )
         .unwrap(),
         proxy: Some(
             ContractId::from_str(
-                "deb633128bceadcd4eb4fe546089f6653727348b60228638a7f9d55d0b6da1ae",
+                "1c50e2d4d602fdd88b47fb7b84b7f87bbdefcf9b7e5985bb7ceeee9266a8e977",
             )
             .unwrap(),
         ),

diff --git a/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs b/sway-core/src/asm_generation/fuel/fuel_asm_builder.rs
@@ -114,7 +114,7 @@ impl<'ir, 'eng> AsmBuilder for FuelAsmBuilder<'ir, 'eng> {
                 self.globals_section.insert(name, size_in_bytes);
                 let global = self.globals_section.get_by_name(name).unwrap();
 
-                let (decode_fn_label, _) = self.func_label_map.get(decode_fn).unwrap();
+                let (decode_fn_label, _) = self.func_label_map.get(&decode_fn.get()).unwrap();
                 let dataid = self.data_section.insert_data_value(Entry::new_byte_array(
                     encoded_bytes.clone(),
                     Some(name.clone()),

diff --git a/sway-core/src/ir_generation/compile.rs b/sway-core/src/ir_generation/compile.rs
@@ -20,7 +20,7 @@ use sway_error::{error::CompileError, handler::Handler};
 use sway_ir::{metadata::combine as md_combine, *};
 use sway_types::{Ident, Spanned};
 
-use std::{collections::HashMap, sync::Arc};
+use std::{cell::Cell, collections::HashMap, sync::Arc};
 
 #[allow(clippy::too_many_arguments)]
 pub(super) fn compile_script(
@@ -359,7 +359,7 @@ pub(crate) fn compile_configurables(
                         ty,
                         ptr_ty,
                         encoded_bytes,
-                        decode_fn,
+                        decode_fn: Cell::new(decode_fn),
                         opt_metadata,
                     },
                 );

diff --git a/sway-ir/src/module.rs b/sway-ir/src/module.rs
@@ -2,7 +2,10 @@
 //!
 //! A module also has a 'kind' corresponding to the different Sway module types.
 
-use std::collections::{BTreeMap, HashMap};
+use std::{
+    cell::Cell,
+    collections::{BTreeMap, HashMap},
+};
 
 use crate::{
     context::Context,
@@ -38,7 +41,7 @@ pub enum ConfigContent {
         ty: Type,
         ptr_ty: Type,
         encoded_bytes: Vec<u8>,
-        decode_fn: Function,
+        decode_fn: Cell<Function>,
         opt_metadata: Option<MetadataIndex>,
     },
 }

diff --git a/sway-ir/src/optimize/dce.rs b/sway-ir/src/optimize/dce.rs
@@ -360,7 +360,7 @@ pub fn fn_dce(context: &mut Context, _: &AnalysisResults, module: Module) -> Res
     // config decode fns
     for config in context.modules[module.0].configs.iter() {
         if let crate::ConfigContent::V1 { decode_fn, .. } = config.1 {
-            grow_called_function_set(context, *decode_fn, &mut called_fns);
+            grow_called_function_set(context, decode_fn.get(), &mut called_fns);
         }
     }
 

diff --git a/sway-ir/src/optimize/fn_dedup.rs b/sway-ir/src/optimize/fn_dedup.rs
@@ -286,6 +286,9 @@ pub fn dedup_fns(
         hash_set_map: FxHashMap::default(),
         function_hash_map: FxHashMap::default(),
     };
+
+    let mut dups_to_delete = vec![];
+
     let cg = build_call_graph(context, &context.modules.get(module.0).unwrap().functions);
     let callee_first = callee_first_order(&cg);
     for function in callee_first {
@@ -324,6 +327,7 @@ pub fn dedup_fns(
             else {
                 continue;
             };
+            dups_to_delete.push(*callee);
             replacements.push((inst, args.clone(), callee_rep));
         }
         if !replacements.is_empty() {
@@ -340,6 +344,35 @@ pub fn dedup_fns(
         }
     }
 
+    // Replace config decode fns
+    for config in module.iter_configs(context) {
+        if let crate::ConfigContent::V1 { decode_fn, .. } = config {
+            let f = decode_fn.get();
+
+            let Some(callee_hash) = eq_class.function_hash_map.get(&f) else {
+                continue;
+            };
+
+            // If the representative (first element in the set) is different, we need to replace.
+            let Some(callee_rep) = eq_class
+                .hash_set_map
+                .get(callee_hash)
+                .and_then(|f| f.iter().next())
+                .filter(|rep| *rep != &f)
+            else {
+                continue;
+            };
+
+            dups_to_delete.push(decode_fn.get());
+            decode_fn.replace(*callee_rep);
+        }
+    }
+
+    // Remove replaced functions
+    for function in dups_to_delete {
+        module.remove_function(context, &function);
+    }
+
     Ok(modified)
 }
 

diff --git a/sway-ir/src/parser.rs b/sway-ir/src/parser.rs
@@ -953,6 +953,7 @@ mod ir_builder {
     // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
     use std::{
+        cell::Cell,
         collections::{BTreeMap, HashMap},
         iter::FromIterator,
     };
@@ -1465,7 +1466,7 @@ mod ir_builder {
                     .configs
                     .get_mut(&configurable_name)
                 {
-                    *decode_fn = f;
+                    decode_fn.replace(f);
                 }
             }
 
@@ -1521,7 +1522,7 @@ mod ir_builder {
                     ptr_ty: Type::new_ptr(context, ty),
                     encoded_bytes: config.encoded_bytes,
                     // this will point to the correct function after all functions are compiled
-                    decode_fn: Function(KeyData::default().into()),
+                    decode_fn: Cell::new(Function(KeyData::default().into())),
                     opt_metadata,
                 };
 

diff --git a/sway-ir/src/pass_manager.rs b/sway-ir/src/pass_manager.rs
@@ -427,6 +427,7 @@ pub fn create_o1_pass_group() -> PassGroup {
     o1.append_pass(SIMPLIFY_CFG_NAME);
     o1.append_pass(FN_DCE_NAME);
     o1.append_pass(DCE_NAME);
+    o1.append_pass(FN_DEDUP_RELEASE_PROFILE_NAME);
 
     o1
 }

diff --git a/sway-ir/src/printer.rs b/sway-ir/src/printer.rs
@@ -274,7 +274,7 @@ fn config_to_doc(
                     "{} = config {}, {}, 0x{}",
                     name,
                     ty,
-                    decode_fn.get_name(context),
+                    decode_fn.get().get_name(context),
                     bytes,
                 ))
                 .append(md_namer.md_idx_to_doc(context, opt_metadata)),

diff --git a/test/Cargo.toml b/test/Cargo.toml
@@ -10,6 +10,7 @@ anyhow.workspace = true
 bytes.workspace = true
 clap = { workspace = true, features = ["derive", "env"] }
 colored.workspace = true
+duct = "0.13.7"
 filecheck.workspace = true
 forc = { workspace = true, features = ["test"] }
 forc-client.workspace = true
@@ -23,6 +24,7 @@ glob.workspace = true
 hex.workspace = true
 insta.workspace = true
 libtest-mimic.workspace = true
+normalize-path = "0.2.1"
 prettydiff.workspace = true
 rand.workspace = true
 regex.workspace = true

diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types/stdout.snap b/test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types/stdout.snap
@@ -1,14 +1,13 @@
 ---
 source: test/tests/tests.rs
 ---
+> forc build --path test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types
 exit status: 1
-stdout:
-    Building src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types
+output:
+    Building test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types
    Compiling library core (sway-lib-core)
    Compiling library std (sway-lib-std)
    Compiling script array_wrong_elements_types (test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types)
-
-stderr:
 error
   --> test/src/e2e_vm_tests/test_programs/should_fail/array_wrong_elements_types/src/main.sw:41:27
    |

diff --git a/test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors/stdout.snap b/test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors/stdout.snap
@@ -1,14 +1,13 @@
 ---
 source: test/tests/tests.rs
 ---
+> forc build --path test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors
 exit status: 1
-stdout:
-    Building src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors
+output:
+    Building test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors
    Compiling library core (sway-lib-core)
    Compiling library std (sway-lib-std)
    Compiling script type_check_analyze_errors (test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors)
-
-stderr:
 error
  --> test/src/e2e_vm_tests/test_programs/should_fail/type_check_analyze_errors/src/main.sw:5:14
   |

diff --git a/.../test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json b/.../test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json
@@ -62,82 +62,82 @@
     {
       "concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903",
       "name": "BOOL",
-      "offset": 7216
+      "offset": 6896
     },
     {
       "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
       "name": "U8",
-      "offset": 7408
+      "offset": 7088
     },
     {
       "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
       "name": "ANOTHER_U8",
-      "offset": 7144
+      "offset": 6824
     },
     {
       "concreteTypeId": "29881aad8730c5ab11d275376323d8e4ff4179aae8ccb6c13fe4902137e162ef",
       "name": "U16",
-      "offset": 7352
+      "offset": 7032
     },
     {
       "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc",
       "name": "U32",
-      "offset": 7392
+      "offset": 7072
     },
     {
       "concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc",
       "name": "U64",
-      "offset": 7400
+      "offset": 7080
     },
     {
       "concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e",
       "name": "U256",
-      "offset": 7360
+      "offset": 7040
     },
     {
       "concreteTypeId": "7c5ee1cecf5f8eacd1284feb5f0bf2bdea533a51e2f0c9aabe9236d335989f3b",
       "name": "B256",
-      "offset": 7184
+      "offset": 6864
     },
     {
       "concreteTypeId": "81fc10c4681a3271cf2d66b2ec6fbc8ed007a442652930844fcf11818c295bff",
       "name": "CONFIGURABLE_STRUCT",
-      "offset": 7304
+      "offset": 6984
     },
     {
       "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c",
       "name": "CONFIGURABLE_ENUM_A",
-      "offset": 7224
+      "offset": 6904
     },
     {
       "concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c",
       "name": "CONFIGURABLE_ENUM_B",
-      "offset": 7264
+      "offset": 6944
     },
     {
       "concreteTypeId": "4926d35d1a5157936b0a29bc126b8aace6d911209a5c130e9b716b0c73643ea6",
       "name": "ARRAY_BOOL",
-      "offset": 7152
+      "offset": 6832
     },
     {
       "concreteTypeId": "776fb5a3824169d6736138565fdc20aad684d9111266a5ff6d5c675280b7e199",
       "name": "ARRAY_U64",
-      "offset": 7160
+      "offset": 6840
     },
     {
       "concreteTypeId": "c998ca9a5f221fe7b5c66ae70c8a9562b86d964408b00d17f883c906bc1fe4be",
       "name": "TUPLE_BOOL_U64",
-      "offset": 7336
+      "offset": 7016
     },
     {
       "concreteTypeId": "94f0fa95c830be5e4f711963e83259fe7e8bc723278ab6ec34449e791a99b53a",
       "name": "STR_4",
-      "offset": 7328
+      "offset": 7008
     },
     {
       "concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
       "name": "NOT_USED",
-      "offset": 7320
+      "offset": 7000
     }
   ],
   "encodingVersion": "1",

diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_dedup_decode/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_dedup_decode/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = "configurable_dedup_decode"
+source = "member"
+dependencies = ["std"]
+
+[[package]]
+name = "core"
+source = "path+from-root-5BADD42D3D8D8FF8"
+
+[[package]]
+name = "std"
+source = "path+from-root-5BADD42D3D8D8FF8"
+dependencies = ["core"]
diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_dedup_decode/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_dedup_decode/Forc.toml
@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "configurable_dedup_decode"
+
+[dependencies]
+std = { path = "../../../../../../../sway-lib-std" }