[PATCH] D52570: [X86] Don't generate BMI BEXTR from X86DAGToDAGISel::matchBEXTRFromAnd

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In https://reviews.llvm.org/D52570#1247523, @lebedev.ri wrote:

> Can we come up with a slightly better logic rather than just `haveTBM()`?
>  Else, i'm not sure how to fix https://reviews.llvm.org/D52293 / PR38938 <https://bugs.llvm.org/show_bug.cgi?id=38938>.
>  Can we perhaps also allow cases when we are shifting a `load`?


Folding a load isn't clearly a perf win here either. Doesn't look like load-op fusion occurs on Intel CPUs so BEXTR with load sends 3 uops out of the frontend. So that's the same number of uops at load+shift+and. If if I've added up the sizes correctly, I think the move immediate + bextr with load is one byte longer than load+shift+and at least for 32-bit. 64-bit would incur a rex prefix on the shift. But we'd probably still have a 32-bit and. So that's probably the same size.



================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2598
+  // hoisting the move immediate would make it worthwhile?
+  if (!Subtarget->hasTBM())
     return false;
----------------
lebedev.ri wrote:
> I think we are also missing a check that NVT is not i64 if we are in 32-bit mode.
We're so far past type legalization here that i64 isn't possible on a 32-bit target.


https://reviews.llvm.org/D52570