[PATCH] D44838: [X86] Correct the placement of ReadAfterLd in BEXTR and BZHI

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Craig.

The change looks good. The ReadAdvance is now applied to the right operand.

However, Haswell model should be fixed too.
At the moment, an InstrRW definition is overriding the `SchedRW` list for BZHI(32|64)rm. So, we don't get the ReadAdvance value at all.
You can see this if you look at scheduling class id "BZHI32rm_BZHI64rm" (which should be index #1246) in X86GenSubtargetInfo.inc. That scheduling class declares 0 read-advance entries.

Same for BEXTR (see the InstrRW at line 1609 in X86SchedHaswell.td). I didn't check other Intel models. However I expect that the same issue affects all intel processors for which the model extends haswell model.

If you test the following code in llvm-mca (-mcpu=haswell -iterations=1 -timeline):

  add    %edi, %esi
  bzhil   %esi, (%rdi), %eax

You get the following output:

  Timeline view:
  
  Index   0123456789
  
  [0,0]   DeER .   .      addl    %edi, %esi
  [0,1]   D=eeeeeeER      bzhil   %esi, (%rdi), %eax
           ^

The bzhil should be able to start immediately at cycle 1. However, it waits until the addl reaches the write-back stage.
The right timeline should have been:

  Timeline view:
  
  Index   0123456789
  
  [0,0]   DeER .   .      addl    %edi, %esi
  [0,1]   DeeeeeeER.      bzhil   %esi, (%rdi), %eax

You can see the same problem if you replace bzhil with `bextrl`.


https://reviews.llvm.org/D44838