[PATCH] D58703: [x86] convert anyext of pinsrb scalar op to subreg insert

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 27 05:49:54 PST 2019


spatel added a comment.

In D58703#1411635 <https://reviews.llvm.org/D58703#1411635>, @spatel wrote:

> In D58703#1411623 <https://reviews.llvm.org/D58703#1411623>, @craig.topper wrote:
>
> > I'm not convinced that the dependency logic in the frontend and the renaming portion of Intel CPUs know that this instruction only uses the lower 8 bits. Same with BT/BTC/BTR/BTS/SHLX/SHRX/SARX and probably others. On Sandy Bridge and later CPUs, probably the worst this means is that it would force an AH/BH/CH/DH merge if it had been written independently previously. Bits 63:16 and 7:0 are always together. In 64-bit mode that should never really happen since we won't consider the AH/BH/CH/DH for general use by the register allocator.
>
>
> So the movzbl here and in the other patch are preferred? I’m happy to leave this as-is if everyone’s ok with the x86 diffs in D58521 <https://reviews.llvm.org/D58521>. Personally, I don’t think those tests with extra instructions are worth worrying about unless there’s some evidence of a perf regression from that.


Reading this again...is the suggestion to promote 8-bit ops to 32-bit like we do for 16-bit? I actually tried that somewhere along the way, and it seemed like a reasonable alternative based on the regression test diffs. That would at least make our lives easier in the compiler by allowing for removal of what would become unlikely/unnecessary patterns.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58703/new/

https://reviews.llvm.org/D58703





More information about the llvm-commits mailing list