Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
cmd/compile: fix CSE with commutative ops
CSE opportunities were being missed for commutative ops. We used to order the args of commutative ops (by arg ID) once at the start of CSE. But that may not be enough. i1 = (Load ptr mem) i2 = (Load ptr mem) x1 = (Add i1 j) x2 = (Add i2 j) Equivalent commutative ops x1 and x2 may not get their args ordered in the same way because because at the start of CSE, we don't know that the i values will be CSEd. If x1 and x2 get opposite orders we won't CSE them. Instead, (re)order the args of commutative operations by their equivalence class IDs each time we partition an equivalence class. Change-Id: Ic609fa83b85299782a5e85bf93dc6023fccf4b0c Reviewed-on: https://go-review.googlesource.com/33632 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Todd Neal <todd@tneal.org>
- Loading branch information
6317f92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@randall77 I was rebasing https://go-review.googlesource.com/34635/ and noticed/bisected: this commit (6317f92) broke TestAssembly. TestAssembly is marked as slow and does not take part in regular all.bash runs so probably that's why it was not caught before. Details below:
thanks beforehand for feedback,
Kirill
/cc @TocarIP, @rasky
6317f92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest creating an issue in https://github.com/golang/go/issues
Edit:
There is #18946
6317f92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@navytux should be fixed now.
6317f92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@randall77, yes, thanks for feedback.