Deforestation with nofibs in the Benchmark Sub-project
#335
Conversation
…ges to avoid creating needless matchRest functions
| def addToBindings(symbol: Local, name: String, shadow: Bool) = | ||
| if !shadow then assert(lookup(symbol).isEmpty, (symbol, this.showAsTree)) | ||
| // FIXME: deforestation makes this assertion fail | ||
| // if !shadow then assert(lookup(symbol).isEmpty, (symbol, this.showAsTree)) |
There was a problem hiding this comment.
I need to look into how to handle this issue, which seems to be caused by using the same Scope for generating programs both before/after deforestation?
| given Config = | ||
| if file.segments.contains("nofib-deforest") then | ||
| Config.default.copy(deforest = S(Config.Deforestation( | ||
| importedPublicModNames = Set("NofibPrelude"), |
There was a problem hiding this comment.
| importedPublicModNames = Set("NofibPrelude"), | |
| seethroughModules = Set("NofibPrelude"), |
| preludeFile) | ||
| deforestResult match | ||
| case Right(msg) => | ||
| println(msg) // TODO: no println |
There was a problem hiding this comment.
| println(msg) // TODO: no println |
| case Right(msg) => | ||
| println(msg) // TODO: no println | ||
| le | ||
| case Left(prog -> summary -> detail) => |
There was a problem hiding this comment.
Shouldn't build a summary string even when not used – use a lambda?
There was a problem hiding this comment.
Fixed in 68cff7e by using trace logger.
|
|
||
| def uid = | ||
| import Result.* | ||
| val uidValue = ResultId(System.identityHashCode(this)) |
There was a problem hiding this comment.
These are not guaranteed to be unique. Use the Uid definitions.
| def rewriteOneBlk(b: Block) = b match | ||
| case Assign(lhs, Value.Lam(params, body), rest) if !lhs.isInstanceOf[TempSymbol] => | ||
| val newSym = BlockMemberSymbol(lhs.nme, Nil, | ||
| val newSym = BlockMemberSymbol(lhs.nme, syntax.Tree.DummyTermDef(syntax.Fun) :: Nil, |
| val sym = BlockMemberSymbol("lambda", Nil, nameIsMeaningful = false) | ||
| val sym = BlockMemberSymbol( | ||
| "lambda", | ||
| syntax.Tree.DummyTermDef(syntax.Fun) :: Nil, |
| case _ => N | ||
| val singleCallBms = BlockMemberSymbol(d.sym.nme + "$", Nil) | ||
| args(fs): args => | ||
| val tmpSym = TempSymbol(N, "arr") | ||
| Assign(tmpSym, Value.Arr(mut = false, args), k(Value.Ref(tmpSym))) |
There was a problem hiding this comment.
Is this really needed? At the very least, we shouldn't do it when the array is immediately returned or already assigned to a variable.
| fun insertCompact(mif_s, ls) = if mif_s is [mif, s] then | ||
| fun insert(x, ls) = if ls is | ||
| Nil then x :: Nil | ||
| y :: ys then if x > y then y :: insert(x, ys) else x :: y :: ys |
There was a problem hiding this comment.
Here x > y converts both x and y (which are JS objects of class MoveInFull) to string and perform string comparison. It should compare field by field instead
There was a problem hiding this comment.
Thanks for the catch. @ychenfo let's remember to fix this when we come back to the PR.
TODO
nofibsto the benchmark subproject