[PATCH] D53164: [X86] X86DAGToDAGISel: handle BZHI selection too, not just BEXTR.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 11 13:08:45 PDT 2018


lebedev.ri created this revision.
lebedev.ri added reviewers: craig.topper, RKSimon, spatel.

As discussed in https://reviews.llvm.org/D52304 / IRC, we now have pattern matching for
'bit extract' in two places - tablegen and `X86DAGToDAGISel`.
There are 4 patterns.
And we will have a problem with `x &  (-1 >> (32 - y))` pattern.

- If the mask is one-use, then it is always unfolded into `x << (32 - y) >> (32 - y)` first. Thus, the existing test coverage is already broken.
- If it is not one-use, then it is not unfolded, and is matched as BZHI.
- If it is not one-use, we will not match it as BEXTR. And if it is one-use, it will have been unfolded already.

So we will either not handle that pattern for BEXTR, or not have test coverage for it.
This is bad.

As discussed with @craig.topper, let's unify this matching, and do everything in `X86DAGToDAGISel`.
Then we will not have code duplication, and will have proper test coverage.

This indeed does not affect any tests, and this is great.
It means that for these two patterns, the `X86DAGToDAGISel` is identical to the tablegen version.

Please review carefully, i'm not fully sure about that intrinsic change, and introduction of the new `X86ISD` opcode.


Repository:
  rL LLVM

https://reviews.llvm.org/D53164

Files:
  lib/Target/X86/X86ISelDAGToDAG.cpp
  lib/Target/X86/X86ISelLowering.cpp
  lib/Target/X86/X86ISelLowering.h
  lib/Target/X86/X86InstrInfo.td
  lib/Target/X86/X86IntrinsicsInfo.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53164.169283.patch
Type: text/x-patch
Size: 7716 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181011/7b5f8141/attachment-0001.bin>


More information about the llvm-commits mailing list