HaohaiWen wrote: > > See most recent comments about missing tests. That being said code looks > > functionally correct to me. Still not 100% sure this is a desirable change. > > Will defer to @nikic about that. > > I'm also skeptical about accepting this optimization. Looking at the > motivating case in [#68502 > (comment)](https://github.com/llvm/llvm-project/pull/68502#discussion_r1351618002), > this seems like a bad approach to the problem: It means that in order to > fold the pattern to `bitreverse(%xy)`, we must just so happen to have the > right `%xy` lying around in the IR, even though it doesn't have any relation > to the main pattern (it's not used inside it, just injected via an extra > use). It sounds to me like the better way to handle that case would be to > support matching a variant of plain bitreverse in the form of > `bitreverse(rot(%yx))`. > > Replacing the `rot(%yx)` by `%xy` is then an extra optimization opportunity, > but it's no longer a precondition to performing the transform.
In fact, this is a real case. Currently, there's no chance for matchBSwapOrBitReverse to match bitreverse pattern if we don't do this fshl optimization. Even if we teach it to recognize this pattern, this requires injecting a new rot(%yx) then look for %xy and it still requires to look for user chain. Even if we don't have bitreverse pattern in the rest IR, there's still beneficial to do such fshl optimization. This removes an extra shl. https://github.com/llvm/llvm-project/pull/68502 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
