20260616 cse fix adam

resources/tests/cse-complex-4.clsp

@@ -0,0 +1,31 @@
+(include *standard-cl-26*)
+
+;; A program which has nested uses of common subexpressions within
+;; conditions.  The downstream conditions are not dominated.
+(defun F (X Y)
+  (if
+    (assign
+      A (+ (l (@ Y 1)) 1)
+      B (+ (l (@ Y 1)) (+ A 1))
+      C (+ (l (@ Y 1)) (+ A 1))
+      D (if (l (@ Y 1)) (+ Y 2) ())
+      (+ A (+ A 1) (+ A 1) (l (@ Y 1)) B C D)
+    )
+    (let
+      ((C (if (@ Y 1) (+ Y 2) ()))
+       (D (if (@ Y 1) (+ Y 2) ()))
+       (E (if (@ Y 1) (+ Y 2) ()))
+       (F 2))
+      (+ (if (@ Y 1) (+ Y 2) ()) (if (@ Y 1) (+ Y 2) ()) C D E F)
+    )
+    (let*
+      ((E (+ X 3))
+       (F (+ E 1))
+       (G (+ (+ X 3) (+ E 1) F))
+       (H (+ (+ X 3) (+ E 1) (+ (+ X 3) (+ E 1) F) G)))
+      (+ (+ X 3) (+ E 1) G F H)
+    )
+  )
+)
+
+(export ARGS (F &rest ARGS))

src/classic/clvm/sexp.rs

@@ -399,12 +399,7 @@ pub fn enlist(allocator: &mut Allocator, vec: &[NodePtr]) -> Result<NodePtr, Eva
 
     for i_reverse in 0..vec.len() {
         let i = vec.len() - i_reverse - 1;
-        match allocator.new_pair(vec[i], built) {
-            Err(e) => return Err(e),
-            Ok(v) => {
-                built = v;
-            }
-        }
+        built = allocator.new_pair(vec[i], built)?;
     }
     Ok(built)
 }

src/classic/clvm_tools/pattern_match.rs

@@ -3,8 +3,8 @@ use crate::classic::clvm::sexp::equal_to;
 use clvm_rs::allocator::{Allocator, NodePtr, SExp};
 use std::collections::HashMap;
 
-pub const ATOM_MATCH: [u8; 1] = [b'$'];
-pub const SEXP_MATCH: [u8; 1] = [b':'];
+pub const ATOM_MATCH: [u8; 1] = *b"$";
+pub const SEXP_MATCH: [u8; 1] = *b":";
 
 pub fn unify_bindings(
     allocator: &mut Allocator,

src/compiler/compiler.rs

@@ -31,6 +31,7 @@ use crate::compiler::diskcache::{set_cache_element, try_element_from_cache};
 use crate::compiler::frontend::frontend;
 use crate::compiler::optimize::depgraph::{DepgraphOptions, FunctionDependencyGraph};
 use crate::compiler::optimize::get_optimizer;
+use crate::compiler::preprocessor::detect_chialisp_module;
 use crate::compiler::prims;
 use crate::compiler::resolve::{find_helper_target, resolve_namespaces};
 use crate::compiler::sexp::{decode_string, enlist, parse_sexp_flags, SExp};
@@ -386,24 +387,30 @@ fn populate_export_map(
 ///
 /// (export foo)
 /// (export bar)
-pub fn compile_module(
-    context: &mut BasicCompileContext,
-    mut opts: Rc<dyn CompilerOpts>,
-    standalone_constants: &HashSet<Vec<u8>>,
-    mut program: CompileForm,
-    exports: &[Export],
-) -> Result<CompileModuleOutput, CompileErr> {
-    let loc = program.loc();
+fn module_compile_opts(opts: Rc<dyn CompilerOpts>) -> Rc<dyn CompilerOpts> {
     let mut dialect = opts.dialect();
     // Module style is new, so there will never be a time when we don't want every
     // bug fix that existed before it was released.
     dialect.int_fix = true;
+    dialect.cse_dominance = true;
     if let Some(stepping) = dialect.stepping {
         if stepping < 26 {
             dialect.stepping = Some(26);
         }
     }
-    opts = opts.set_optimize(true).set_dialect(dialect);
+
+    opts.set_optimize(true).set_dialect(dialect)
+}
+
+pub fn compile_module(
+    context: &mut BasicCompileContext,
+    mut opts: Rc<dyn CompilerOpts>,
+    standalone_constants: &HashSet<Vec<u8>>,
+    mut program: CompileForm,
+    exports: &[Export],
+) -> Result<CompileModuleOutput, CompileErr> {
+    let loc = program.loc();
+    opts = module_compile_opts(opts);
 
     if exports.is_empty() {
         return Err(CompileErr(
@@ -826,9 +833,13 @@ fn add_inline_hash_for_constant(program: &mut CompileForm, loc: &Srcloc, fun_nam
 /// Given a set of untreated input forms, compile it as a clvm program.
 pub fn compile_pre_forms(
     context: &mut BasicCompileContext,
-    opts: Rc<dyn CompilerOpts>,
+    mut opts: Rc<dyn CompilerOpts>,
     pre_forms: &[Rc<SExp>],
 ) -> Result<CompilerOutput, CompileErr> {
+    if let Some(dialect) = detect_chialisp_module(Srcloc::start(&opts.filename()), pre_forms)? {
+        opts = opts.set_stdenv(dialect.strict).set_dialect(dialect);
+    }
+
     let p0 = frontend(opts.clone(), pre_forms)?;
 
     match p0 {
@@ -838,20 +849,14 @@ pub fn compile_pre_forms(
         )),
         FrontendOutput::Module(mut cf, exports) => {
             add_main_fingerprint(&mut cf, pre_forms);
+            let opts = module_compile_opts(opts);
+
             // If we can read from cache, use that.  We'll use the actual form of the compileform
             // and opts (the dialect) to determine a cache hit.
             if let Some(result) = try_from_cache(opts.clone(), &cf, &exports)? {
                 return Ok(result);
             }
 
-            // cl23 always reflects optimization.
-            let dialect = opts.dialect();
-            let opts = if let Some(stepping) = dialect.stepping.as_ref() {
-                opts.set_optimize(*stepping > 21)
-            } else {
-                opts
-            };
-
             // We make a dependency graph of constants and functions.  There must
             // be a solveable hierarchy for constants, that is some must be top
             // level constants that are not depended on.  Thse will not be tabled

src/compiler/dialect.rs

@@ -21,6 +21,8 @@ pub struct AcceptedDialect {
     pub int_fix: bool,
     // Extra numeric constants
     pub extra_numeric_constants: bool,
+    // Include fix for downstream cse dominance
+    pub cse_dominance: bool,
 }
 
 /// A package containing the content we should insert when a dialect include is
@@ -56,6 +58,7 @@ lazy_static! {
                         strict: true,
                         int_fix: false,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 22)
@@ -71,6 +74,7 @@ lazy_static! {
                         strict: false,
                         int_fix: false,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 22)
@@ -86,6 +90,7 @@ lazy_static! {
                         strict: true,
                         int_fix: false,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 23)
@@ -101,6 +106,7 @@ lazy_static! {
                         strict: true,
                         int_fix: true,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 23)
@@ -116,6 +122,7 @@ lazy_static! {
                         strict: true,
                         int_fix: true,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 24)
@@ -131,6 +138,7 @@ lazy_static! {
                         strict: true,
                         int_fix: true,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 25)
@@ -146,6 +154,7 @@ lazy_static! {
                         strict: true,
                         int_fix: true,
                         extra_numeric_constants: true,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 25)
@@ -161,6 +170,7 @@ lazy_static! {
                         strict: false,
                         int_fix: true,
                         extra_numeric_constants: true,
+                        cse_dominance: false,
                     },
                     content: indoc! {"()"}.to_string(),
                 },
@@ -173,6 +183,7 @@ lazy_static! {
                         strict: true,
                         int_fix: true,
                         extra_numeric_constants: false,
+                        cse_dominance: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 26)

src/compiler/diskcache.rs

@@ -4,12 +4,25 @@ use crate::compiler::clvm::sha256tree_from_atom;
 use crate::compiler::comptypes::{CompileErr, CompileForm, CompilerOpts};
 use crate::compiler::sexp::decode_string;
 
-fn cache_key(cf: &CompileForm) -> String {
-    let mut include_fingerprints = Vec::new();
+fn cache_key(opts: Rc<dyn CompilerOpts>, cf: &CompileForm) -> String {
+    let dialect = opts.dialect();
+    let mut key_material = b"module-cache-v2".to_vec();
+
+    if let Some(stepping) = dialect.stepping {
+        key_material.push(1);
+        key_material.extend_from_slice(&stepping.to_le_bytes());
+    } else {
+        key_material.push(0);
+    }
+    key_material.push(u8::from(dialect.strict));
+    key_material.push(u8::from(dialect.int_fix));
+    key_material.push(u8::from(dialect.extra_numeric_constants));
+    key_material.push(u8::from(dialect.cse_dominance));
+
     for include in cf.include_forms.iter() {
-        include_fingerprints.extend_from_slice(&include.fingerprint);
+        key_material.extend_from_slice(&include.fingerprint);
     }
-    hex::encode(sha256tree_from_atom(&include_fingerprints))
+    hex::encode(sha256tree_from_atom(&key_material))
 }
 
 /// Try to get an element from the cache, exposing errors.
@@ -30,7 +43,7 @@ pub fn try_element_from_cache(
     cf: &CompileForm,
     export_path: &str,
 ) -> Option<String> {
-    let key = cache_key(cf);
+    let key = cache_key(opts.clone(), cf);
     let hex_file_name = format!(".chialisp/{key}/{export_path}");
     opts.read_new_file(cf.loc().file.to_string(), hex_file_name.clone())
         .ok()
@@ -43,7 +56,7 @@ pub fn set_cache_element_error(
     export_path: &str,
     export_hex: &str,
 ) -> Result<(), CompileErr> {
-    let key = cache_key(cf);
+    let key = cache_key(opts.clone(), cf);
     let hex_file_name = format!(".chialisp/{key}/{export_path}");
     opts.write_new_file(&hex_file_name, export_hex.as_bytes())?;
     Ok(())
@@ -63,14 +76,16 @@ pub fn set_cache_element(
 
 /// Exposes the cache-key segment used under `.chialisp/<key>/` (tests and tooling only).
 #[cfg(test)]
-pub fn module_cache_key_hex(cf: &CompileForm) -> String {
-    cache_key(cf)
+pub fn module_cache_key_hex(opts: Rc<dyn CompilerOpts>, cf: &CompileForm) -> String {
+    cache_key(opts, cf)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::compiler::compiler::DefaultCompilerOpts;
     use crate::compiler::comptypes::{BodyForm, CompileForm, IncludeDesc, IncludeProcessType};
+    use crate::compiler::dialect::AcceptedDialect;
     use crate::compiler::sexp::SExp;
     use crate::compiler::srcloc::Srcloc;
 
@@ -84,14 +99,18 @@ mod tests {
         }
     }
 
+    fn default_opts(filename: &str) -> Rc<dyn CompilerOpts> {
+        Rc::new(DefaultCompilerOpts::new(filename))
+    }
+
     #[test]
     fn cache_key_stable_for_empty_includes() {
         let loc = Srcloc::start(&"a.clsp".to_string());
         let cf = empty_compileform(loc);
-        let k = cache_key(&cf);
+        let k = cache_key(default_opts("a.clsp"), &cf);
         assert_eq!(
             k,
-            "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a"
+            "bb1dfe276a7165264b23349a3d349c470f451aa964f89c172bfb5bc8fdf9d548"
         );
     }
 
@@ -112,15 +131,36 @@ mod tests {
             fingerprint: fp,
         };
         cf.include_forms.push(desc(fp(&[1, 2, 3])));
-        let k1 = cache_key(&cf);
+        let k1 = cache_key(default_opts("b.clsp"), &cf);
         cf.include_forms.push(desc(fp(&[4, 5])));
-        let k2 = cache_key(&cf);
+        let k2 = cache_key(default_opts("b.clsp"), &cf);
         assert_ne!(k1, k2);
         cf.include_forms.truncate(1);
-        let k1_again = cache_key(&cf);
+        let k1_again = cache_key(default_opts("b.clsp"), &cf);
         assert_eq!(k1, k1_again);
     }
 
+    #[test]
+    fn cache_key_changes_with_cse_dominance() {
+        let loc = Srcloc::start(&"cse.clsp".to_string());
+        let cf = empty_compileform(loc);
+        let dialect_before = AcceptedDialect {
+            stepping: Some(26),
+            strict: true,
+            int_fix: true,
+            extra_numeric_constants: false,
+            cse_dominance: false,
+        };
+        let dialect_after = AcceptedDialect {
+            cse_dominance: true,
+            ..dialect_before.clone()
+        };
+        let before = default_opts("cse.clsp").set_dialect(dialect_before);
+        let after = default_opts("cse.clsp").set_dialect(dialect_after);
+
+        assert_ne!(cache_key(before, &cf), cache_key(after, &cf));
+    }
+
     #[test]
     fn cache_key_main_fingerprint_style() {
         let loc = Srcloc::start(&"c.clsp".to_string());
@@ -135,7 +175,7 @@ mod tests {
             kind: Some(IncludeProcessType::Compiled),
             fingerprint: main_fp,
         });
-        let k = cache_key(&cf);
+        let k = cache_key(default_opts("c.clsp"), &cf);
         assert!(!k.is_empty());
         assert_ne!(
             k,

src/compiler/optimize/above22.rs

@@ -59,6 +59,7 @@ impl Optimization for Strategy23 {
         for h in p0.helpers.iter() {
             if let HelperForm::Defun(inline, d) = h {
                 let function_body = cse_optimize_bodyform(
+                    opts.clone(),
                     &h.loc(),
                     h.name(),
                     enable_merge_disable_for_tests,