Move bus_multi_linker to backend-aware tuning #2611
Conversation
qwang98
changed the title
| // if inferred != declared { | ||
| // return Err(source_ref.with_error(format!( | ||
| // "Inferred type scheme for symbol {name} does not match the declared type.\nInferred: let{}\nDeclared: let{}", | ||
| // format_type_scheme_around_name(&name, &Some(inferred)), | ||
| // format_type_scheme_around_name(&name, &Some(declared_type), | ||
| // )))); | ||
| // println!( | ||
| // "Inferred type scheme for symbol {name} does not match the declared type.\nInferred: let{}\nDeclared: let{}", | ||
| // format_type_scheme_around_name(&name, &Some(inferred.clone())), | ||
| // format_type_scheme_around_name(&name, &Some(declared_type.clone())), | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
Note that these are currently commented out, but to do it properly I'll eventually need to make two analyze_ast versions, one with this check for first pass analyze and the other without for second pass analyze.
There was a problem hiding this comment.
This errors out for second pass analyze within specialize_pil. Sample error: [Error { source_ref: :10953-11150, message: "Inferred type scheme for symbol std::btree::internal::array_split does not match the declared type.\nInferred: let<T: Add + FromLiteral + Mul> std::btree::internal::array_split: T[], int -> (T[], T[])\nDeclared: let<T> std::btree::internal::array_split: T[], int -> (T[], T[])" }].
Please refer to the main comment for why.
There was a problem hiding this comment.
Maybe there's a way around this by going to string again? After injecting, we should have valid pil, so if the compiler cannot deal with it then we should fix that and not go around it?
There was a problem hiding this comment.
Also if the issue is in the btree library, we could try removing that and see if it fixes it.
There was a problem hiding this comment.
Maybe there's a way around this by going to string again? After injecting, we should have valid pil, so if the compiler cannot deal with it then we should fix that and not go around it?
I just tried going to string again via let analyzed = powdr_pil_analyzer::analyze_string(&PILFile(all_statements).to_string()).unwrap();. Unfortunately the same error happened.
Alternatively, we could inject the stdlib again in the tuning. This would have the advantage to keep the first and second pilopt passes identical and limit edge cases. |
|
@georgwiese @Schaeff do we still want to go ahead with this? |
NOT READY FOR REVIEW. Now only the
block_to_block.asmexample works. I foresee lots of debugging.This PR attempts to move PIL call
bus_multi_linkerfrom the bus linker to the backend-aware tuning step:link --> analyze --> optimize --> [optimized_to_string --> reparse_string_to_pil_statement --> add_bus_multi_linker_pil_statements --> analyze_second_pass] --> optimize_second_pass, where contents within the brackets are all done inBackendFactory::specialize_pil.remove_unreferenced_definitions()step fromoptimize(first pass), because otherwise the optimizer will remove many required PIL definitions needed later which are no longer referenced by the linker.analyze_second_pass, which will otherwise cause error on some inferred types not matching their declared types. This is expected becausebus_multi_linkerPIL are raw PIL statements added tooptimized, which already have types inferred and therefore not compatible with declared raw PIL types.Next step will be creating different asm calls for Stwo and non-Stwo backends, but we need to make this simpler version work first.