Use a caching version of stmt_uses_vars in TightenProducerConsumer nodes #8102
Conversation
We were making a very large number stmt_uses_vars queries that covered the same sub-stmts. I solved it by adding a cache. Speeds up local laplacian lowering by 10% by basically removing this pass from the profile. Also a drive-by typo fix in Lower.cpp
| class CachingStmtUsesVars : public IRMutator { | ||
| const Scope<> &query; | ||
| bool found_use = false; | ||
| std::map<Stmt, bool> cache; |
There was a problem hiding this comment.
Why not std::set<> instead?
There was a problem hiding this comment.
Also, just fer grins, would unordered_map or unordered_set be any better here? (Surely the order doesn't matter)
There was a problem hiding this comment.
There are three possible states:
- I've seen this stmt before and it doesn't contain the vars (map contains false)
- I've seen this stmt before and it does contain the vars (map contains true)
- I haven't seen this stmt before and it needs to be analyzed (not in the map)
There was a problem hiding this comment.
I generally avoid unordered_set and unordered_map because every time I've benchmarked them within Halide they haven't been enough faster to outweigh a really annoying property that I've been burned by: If you have a bug where you accidentally depend on the order of the keys, it can fail on some machines but not others depending on the standard library used, which makes debugging it really annoying.
…_producer_consumer_nodes
|
All green, just needs a review. |
We were making a very large number stmt_uses_vars queries that covered the same sub-stmts. I solved it by adding a cache.
Speeds up local laplacian lowering by 10% by basically removing this pass from the profile.
Also a drive-by typo fix in Lower.cpp