Cranelift(egraphs): Maintain the instruction set in an expression's cost#12230
Cranelift(egraphs): Maintain the instruction set in an expression's cost#12230fitzgen wants to merge 6 commits intobytecodealliance:mainfrom
Conversation
|
Thanks for the PR! (Nick knows but for anyone else watching) I'm out on PTO then at a conference, so I'll be able to review this Mon, Jan 12 or later. |
cfallin
left a comment
There was a problem hiding this comment.
Got time today to review this actually so: LGTM and I'm really excited about this!
If you don't mind, could we post Sightglass results here before merging? Specifically I'd like to see cycles measured for compilation and execution time; hopefully we see no or few regressions (as I know you've already seen during some experimentation).
|
Hrm, yeah I guess instructions retired isn't a good proxy for wall time in this case because we are switching from pretty much an array-of-scalars representation of cost to an array-of-trees representation, which is going to involve more pointer chasing and memory latency that isn't reflected in instructions retired. Pretty clear to me that this PR cannot land as-is. Might be worth some profiling to see if there is any low-hanging fruit, but if we can't just engineer our way out of this would-be regression, then it is probably best to stop pursuing the instruction-set cost function any further :-/ |
7c125c7 to
711cc55
Compare
|
Ah, that's too bad! Very worthwhile exploration regardless... |
|
Idea: We could potentially try to do the current scalar cost first, but bail out if we ever saturate to infinity, and only then use the instruction-set cost function. Kind of heavy-handed, but might avoid compile-time regressions. |
Fixes #12156