This is an archive of the discontinued LLVM Phabricator instance.

[X86][BtVer2] Fix latency/throughput of scalar integer MUL instructions.
ClosedPublic

Authored by andreadb on Aug 21 2019, 11:50 AM.

Details

Summary

Single operand MUL instructions that implicitly set EAX have the following latency/throughput profile (see below):

imul %cl              # latency: 3cy - uOPs: 1 - 1 JMul
imul %cx              # latency: 3cy - uOPs: 3 - 3 JMul
imul %ecx             # latency: 3cy - uOPs: 2 - 2 JMul
imul %rcx             # latency: 6cy - uOPs: 2 - 4 JMul

mul %cl               # latency: 3cy - uOPs: 1 - 1 JMul
mul %cx               # latency: 3cy - uOPs: 3 - 3 JMul
mul %ecx              # latency: 3cy - uOPs: 2 - 2 JMul
mul %rcx              # latency: 6cy - uOPs: 2 - 4 JMul

Excluding the 64bit variant, which has a latency of 6cy, every other instruction has a latency of 3cy. However, the number of decoded macro-opcodes (as well as the resource cyles) depend on the MUL size.

The two operand MULs have a more predictable profile (see below):

imul %dx, %dx         # latency: 3cy - uOPs: 1 - 1 JMul
imul %edx, %edx       # latency: 3cy - uOPs: 1 - 1 JMul
imul %rdx, %rdx       # latency: 6cy - uOPs: 1 - 4 JMul

imul $3, %dx, %dx     # latency: 4cy - uOPs: 2 - 2 JMul
imul $3, %ecx, %ecx   # latency: 3cy - uOPs: 1 - 1 JMul
imul $3, %rdx, %rdx   # latency: 6cy - uOPs: 1 - 4 JMul

This patch updates the values in the scheduling model based on those values.
I had to update quite a few tests because MUL was used in many tests...

Diff Detail

Repository
rL LLVM

Event Timeline

andreadb created this revision.Aug 21 2019, 11:50 AM
RKSimon accepted this revision.Aug 22 2019, 7:38 AM

LGTM, I agree all the test changes are annoying but nothing to block this in any way.

This revision is now accepted and ready to land.Aug 22 2019, 7:38 AM
andreadb updated this revision to Diff 216623.Aug 22 2019, 7:45 AM

Patch rebased.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2019, 8:21 AM