[PATCH] D43201: [X86] Only reorder srl/and on last DAG combiner run

[Sanjay Patel via Phabricator via llvm-commits]

spatel added reviewers: zvi, reames, davezarzycki.
spatel added a comment.

Does this also imply that we're abandoning https://reviews.llvm.org/D37418? Ie, we're effectively blacklisting 'bt' in isel?

> BT has lower throughput than TEST.

This isn't true universally based on Agner's tables. Ryzen has 1 uop and full throughput for a plain 'bt' (btc/btr/bts need 2 uops).

I don't have any actual perf evidence of 'bt' being a factor in a benchmark, but in https://reviews.llvm.org/D37418, it was suggested that a 'btc' would likely be an improvement. See also the comments in:
https://bugs.llvm.org/show_bug.cgi?id=35911

Given that and the fact that gcc/icc already choose 'bt' more often than llvm (again based on earlier comments in the previous review and the PR), I think we'd be better off choosing bt and friends by default in isel and transforming to something with a bigger constant as a specialization at a later stage.


https://reviews.llvm.org/D43201