[MLIR] Don't check for key before inserting in map in GreedyPatternRewriteDriver worklist (NFC)

[joker-eph]

This is a common anti-pattern (any volunteer for a clang-tidy check?).

This does not show real word significant impact though.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

Changes

This is a common anti-pattern (any volunteer for a clang-tidy check?).

Amazingly a micro-benchmark of the GreedyPatternRewriteDriver shows a 2.5x speedup of the entire GreedyPatternRewriteDriver execution with this patch.

---

Full diff: https://github.com/llvm/llvm-project/pull/88148.diff

1 Files Affected:

• (modified) mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp (+1-2)

diff --git a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp index bbecbdb8566935..604d8628fae755 100644 --- a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp +++ b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp @@ -243,9 +243,8 @@ bool Worklist::empty() const { void Worklist::push(Operation *op) { assert(op && "cannot push nullptr to worklist"); // Check to see if the worklist already contains this op. - if (map.count(op)) + if (map.insert({op, list.size()}).second) return; - map[op] = list.size(); list.push_back(op); } 

[stellaraccident]

Wow! A clang tidy check would indeed be good. I've written the correct form thousands of times and (almost) always go out of my way to do it, but I often have to hit the docs to remember exactly which form of insert/second does what I want. It's not surprising to me that people can't remember this and just write the simpler to understand form thinking it won't hurt too much.

[joker-eph]

Just taking back the benchmark claim, my microbenchmark was busted, I dug a bit more and can't get very significant real-world impact.

Still a straightforward minor cleanup though.

[stellaraccident]

Oh well, still a good cleanup. I was trying to figure out how in this case, it could be such an extreme benefit. My conception of reality likes that it is not a huge difference in this case, but the speedup would have been nice :)