[webkit-reviews] review canceled: [Bug 194081] B3 should use associativity to optimize expression trees : [Attachment 366294] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 2 14:36:41 PDT 2019

Robin Morisset <rmorisset at apple.com> has canceled Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 194081: B3 should use associativity to optimize expression trees

Attachment 366294: Patch


--- Comment #13 from Robin Morisset <rmorisset at apple.com> ---
Comment on attachment 366294
  --> https://bugs.webkit.org/attachment.cgi?id=366294

I've got the performance numbers from this patch on JetStream2 and they're
overall neutral.
So I looked in more detail at when this optimization fires in a non-trivial way
on all of JetStream2, and there are only a few subtests where it triggers:
- Unipoker: balances some BitOr trees (insignificant slowdown)
- tsf-wasm: balances a ton of BitOr trees (insignificant 1.4% speedup)
- stanford-crypto-sha256 and stanford-crypto-pbkdf2: balances a few BitXor
trees (insignificant slowdown on first iteration, strongly significant 2%
speedup on average iteration and 3.5% speedup on worst iteration)
- octane-zlib: balances a couple huge Add trees (no change)
- ML: balances one small BitXor tree (no change)
- mandreel: balances a couple (Add/BitAnd) small trees (0.7% insignificant
slowdown on first iteration, strongly significant 1% improvement on average
iteration and 2% on worst-case iteration)
- HashSet-wasm: balances a few BitOr trees (insignificant speedup)
- base64-SP: only balances one tiny BitXor tree (significant 2% slowdown on
average iteration(?), insignificant change otherwise)

So at best we get a couple percent on three benchmarks, and are otherwise
Most likely we are just seeing noise even in these benchmarks where it balances
trees. Surprisingly (to me) it does not find simplification opportunities
beyond simple balancing.

I thus put this patch out-of-review, as it does not seem obviously worth it at
this point. I plan to revisit it once we have better value analysis in B3. It
should enable significantly more simplifications, and make more large trees
appear by turning CheckAdd/Sub/Mul into their non-checked variants.

More information about the webkit-reviews mailing list