[PATCH] D48606: [X86] Use bts/btr/btc for single bit set/clear/complement of a variable bit position

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 27 01:57:52 PDT 2018


lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

OK, looks good, other than nits, but you may want to wait for a second opinion.
(Please split into two commits - the tests first)



================
Comment at: lib/Target/X86/X86InstrCompiler.td:1822
+  // Similar to above, but removing unneeded masking of the shift amount.
+  def : Pat<(and RC:$src1, (rotl -2, (and GR8:$src2, ImmShift))),
+            (BTR RC:$src1,
----------------
I would love to know how it would be possible to deduplicate the cases with mask and without mask,
since that is essentially the same problem i have in D48491 :)


================
Comment at: lib/Target/X86/X86InstrCompiler.td:1833
+
+defm : one_bit_patterns<GR16, i16, BTR16rr, BTS16rr, BTC16rr, immShift16>;
+defm : one_bit_patterns<GR32, i32, BTR32rr, BTS32rr, BTC32rr, immShift32>;
----------------
>>! In D48606#1144491, @craig.topper wrote:
> The 16-bit BTR fails to match because the 'and' got promoted to 32-bit and the rotate didn't. We need to fix the promotion of the rotate.

Yeah, i think for now this line should be disabled, and a FIXME added/bug filled.


https://reviews.llvm.org/D48606





More information about the llvm-commits mailing list