Repository navigation
Conversation
Added: - set of rewrites, - operations transformations, - template ir, - rewrites via snippets.
| if let Some(start) = ["seq:", "semantic:"].iter().find(|s| l.starts_with(*s)) | ||
| && !line.ends_with("\"\"") | ||
| { |
There was a problem hiding this comment.
Why are these changes only in Rust LIRA library?
There was a problem hiding this comment.
This fixes a rust implementation specific bug. It was not present in python (idk about ruby)
| pub fn set_tmp_prefix(&mut self, prefix: impl ToString) { | ||
| self.tmp_prefix = prefix.to_string(); | ||
| } | ||
| pub fn update_tmp_prefix<'a, 's: 'a>( | ||
| &'s mut self, | ||
| base: &str, | ||
| names_to_avoid: impl Iterator<Item = &'a String> + Clone, | ||
| ) { | ||
| let names_to_avoid = || { | ||
| names_to_avoid | ||
| .clone() | ||
| .chain(self.stmts.iter().flat_map(|stmt| stmt.outputs.iter())) | ||
| }; | ||
| for i in 0..names_to_avoid().count() + 1 { | ||
| let prefix = format!("{base}{i}"); | ||
| if names_to_avoid().all(|name| !name.starts_with(&prefix)) { | ||
| self.tmp_counter = 0; | ||
| self.tmp_prefix = prefix; | ||
| return; | ||
| } | ||
| } | ||
| unreachable!("names_to_avoid len + 1 prefixes should've been enough") | ||
| } | ||
| pub fn gen_temp_name(&mut self) -> String { | ||
| self.tmp_counter += 1; | ||
| format!("{}{}", self.tmp_prefix, self.tmp_counter) | ||
| } |
There was a problem hiding this comment.
Moreover, rust/lira_trs/src/impl_dfg3lira.rs and rust/lira_trs/src/dfg.rs modules implement their own local gen_temp_name with the same functionality (impl_dfg3lira.rs:115, dfg.rs:58). Why is it so?
There was a problem hiding this comment.
Unused private functions - dead code, what's the problem with unused locally public function?
| pub fn try_push_gen_temp_names(&mut self, mut stmt: Statement) -> anyhow::Result<()> { | ||
| assert!(stmt.outputs.is_empty()); | ||
| assert!(!stmt.outputs_types.is_empty()); | ||
| stmt.outputs = (0..stmt.outputs_types.len()) | ||
| .map(|_| self.gen_temp_name()) | ||
| .collect(); | ||
| self.try_push(stmt) | ||
| } |
| tmp_prefix: String, | ||
| tmp_counter: usize, |
| "fold" => { | ||
| let op = get!(op); | ||
| let args = &self.inputs[..]; | ||
| StmtSpecificStd::Fold { op, args } | ||
| let _op = get!(op); | ||
| let _args = &self.inputs[..]; | ||
| todo!() | ||
| } | ||
| "scan" => { | ||
| let op = get!(op); | ||
| let args = &self.inputs[..]; | ||
| StmtSpecificStd::Scan { op, args } | ||
| } |
There was a problem hiding this comment.
What’s wrong with this? Why this statements marked with todo!() ? And why does lira_trs use its own statements (fold.op, scan.op in rust/lira_trs/src/lib.rs)?
There was a problem hiding this comment.
This code isn't used. It can be removed from this PR.
In general though each language should provide statement concretization.
| "cond.sys_reg.write", | ||
| "fop", | ||
| "env", | ||
| "cond.env", |
There was a problem hiding this comment.
Why are statements in rust/lira/src/ir_std.rs named things like cond_env, whereas here you use cond.env?
Globally: why use a . as a separator at all, instead of an _ (e.g., scan.fop, cond.write, etc.)?
There was a problem hiding this comment.
cond_env is a mistake.
cond. is a common prefix with similar semantic. Same for other usages of . - they separate parts that have meaning on their own. _ separates words in atomic name.
Imho naming convention is not a part of this PR, but rather #19
| let cond = self.inputs[0].as_str(); | ||
| let on_false = self.inputs[0].as_str(); | ||
| let rsi = self.inputs[0].as_str(); |
There was a problem hiding this comment.
Seems like there is a bug.
cond, on_false, and rsi are all read from self.inputs[0].
Should be [0], [1], and [2] ?
| let value = &self.inputs[0]; | ||
| let index = &self.inputs[0]; | ||
| let on_false = &self.inputs[0]; |
There was a problem hiding this comment.
Seems like there is a bug.
Open diff & compare with previous version.
| let base = &self.inputs[0]; | ||
| let cond = &self.inputs[0]; | ||
| let value = &self.inputs[0]; | ||
| let index = &self.inputs[0]; |
There was a problem hiding this comment.
Seems like there is a bug.
Open diff & compare with previous version.
Added: