This is an archive of the discontinued LLVM Phabricator instance.

Differential D7401

[x86] Switch the x86 backend's execution domain order to prefer the packed integer domain.
AcceptedPublic

Authored by chandlerc on Feb 4 2015, 2:31 AM.

Details

Reviewers
qcolombet
craig.topper
Summary

While the packed single-precision domain has the smallest encodings and
was the first domain, it isn't a very good default. Consider operations
which could occur in any domain: loads, stores, shuffles, and, or, xor.
All of these operations are the same latency in the integer domain and
the floating point domains, but in many cases have 2x or 3x the
throughput in the integer domain! When in the floating point domain,
they end up bottlenecked on a single execution port in every micro
architecture since sandybridge, and probably some older ones as well.

This in turn uncovers some issues with our execution domain settings in
the backend. I've got a patch for one that is an independent improvement
and I'll submit shortly -- it adds an execution domain to movss and
movsd, both of which specifically target floating point domains. Adding
these causes us to match up floating point domain code much better
already.

A second issue I'm investigating is with vinsertf128 near a packed
double domain instruction becoming vinserti128.

Aside from these issues, everything I'm seeing looks like a huge
improvement to domain crossing and generally using the higher throughput
integer units. What do others think?

I'll update this patch with the test changes if folks want, but it will
be *MANY* updates to tests to make this change.

Diff Detail

Event Timeline

chandlerc updated this revision to Diff 19306.Feb 4 2015, 2:31 AM
chandlerc retitled this revision from to [x86] Switch the x86 backend's execution domain order to prefer the packed integer domain..
chandlerc updated this object.
chandlerc edited the test plan for this revision. (Show Details)
chandlerc added reviewers: craig.topper, qcolombet.
chandlerc added a subscriber: Unknown Object (MLST).
chandlerc updated this revision to Diff 19309.Feb 4 2015, 3:20 AM

Rotated the other table thanks to Craig Topper pointing out where I was being
blind. =]

The issue with vinsertf128 is now fixed. I've submitted the fix for movss.

However, most of the noise I see left from this change comes form the fact that
we also don't mark *scalar* floating point instructions as executing on the
packed floating point domain, even though they essentially do. I'm working on
a fix to that independently. Currentyl there are 105 tests that require updates
with this change, and I'm hoping the scalar operations specifying their domain
will reduced that significantly.

qcolombet accepted this revision.Feb 4 2015, 1:15 PM
qcolombet edited edge metadata.

LGTM.

Q.

This revision is now accepted and ready to land.Feb 4 2015, 1:15 PM
chandlerc added a comment.Feb 5 2015, 1:54 PM

FWIW, I've run some very generic benchmarks (the only ones that are vector
heavy, are FP vector heavy) and have measured no significant swings in
performance in either direction. Everything is below the (frustratingly
high) noise floor.

Still planning to leave this under review for a while as i'd have to update
maaaany tests to submit it and I'm happy to let more folks benchmark in the
mean time.

RKSimon mentioned this in D7600: [X86][SSE] Add SSE MOVQ instructions to SSEPackedInt domain.Feb 12 2015, 2:15 PM
RKSimon mentioned this in D8691: [X86] add an exedepfix entry for movq == movlps == movlpd (refiled).Mar 30 2015, 8:17 AM

Revision Contents

PathSize
lib/
Target/
X86/
6 lines
122 lines

Diff 19309

lib/Target/X86/X86InstrFormats.td

Show First 20 Lines • Show All 82 Lines • ▼ Show 20 Lines
def SpecialFP : FPFormat<7>; def SpecialFP : FPFormat<7>;
// Class specifying the SSE execution domain, used by the SSEDomainFix pass. // Class specifying the SSE execution domain, used by the SSEDomainFix pass.
// Keep in sync with tables in X86InstrInfo.cpp. // Keep in sync with tables in X86InstrInfo.cpp.
class Domain<bits<2> val> { class Domain<bits<2> val> {
bits<2> Value = val; bits<2> Value = val;
} }
def GenericDomain : Domain<0>; def GenericDomain : Domain<0>;
def SSEPackedSingle : Domain<1>; def SSEPackedInt : Domain<1>;
def SSEPackedDouble : Domain<2>; def SSEPackedSingle : Domain<2>;
def SSEPackedInt : Domain<3>; def SSEPackedDouble : Domain<3>;
// Class specifying the vector form of the decompressed // Class specifying the vector form of the decompressed
// displacement of 8-bit. // displacement of 8-bit.
class CD8VForm<bits<3> val> { class CD8VForm<bits<3> val> {
bits<3> Value = val; bits<3> Value = val;
} }
def CD8VF : CD8VForm<0>; // v := VL def CD8VF : CD8VForm<0>; // v := VL
def CD8VH : CD8VForm<1>; // v := VL/2 def CD8VH : CD8VForm<1>; // v := VL/2
▲ Show 20 Lines • Show All 810 Lines • Show Last 20 Lines

lib/Target/X86/X86InstrInfo.cpp

Show First 20 Lines • Show All 5,711 Lines • ▼ Show 20 Linesunsigned X86InstrInfo::getGlobalBaseReg(MachineFunction *MF) const {
X86FI->setGlobalBaseReg(GlobalBaseReg); X86FI->setGlobalBaseReg(GlobalBaseReg);
return GlobalBaseReg; return GlobalBaseReg;
} }
// These are the replaceable SSE instructions. Some of these have Int variants // These are the replaceable SSE instructions. Some of these have Int variants
// that we don't include here. We don't want to replace instructions selected // that we don't include here. We don't want to replace instructions selected
// by intrinsics. // by intrinsics.
static const uint16_t ReplaceableInstrs[][3] = { static const uint16_t ReplaceableInstrs[][3] = {
//PackedSingle PackedDouble PackedInt // PackedInt PackedSingle PackedDouble
{ X86::MOVAPSmr, X86::MOVAPDmr, X86::MOVDQAmr }, {X86::MOVDQAmr, X86::MOVAPSmr, X86::MOVAPDmr},
{ X86::MOVAPSrm, X86::MOVAPDrm, X86::MOVDQArm }, {X86::MOVDQArm, X86::MOVAPSrm, X86::MOVAPDrm},
{ X86::MOVAPSrr, X86::MOVAPDrr, X86::MOVDQArr }, {X86::MOVDQArr, X86::MOVAPSrr, X86::MOVAPDrr},
{ X86::MOVUPSmr, X86::MOVUPDmr, X86::MOVDQUmr }, {X86::MOVDQUmr, X86::MOVUPSmr, X86::MOVUPDmr},
{ X86::MOVUPSrm, X86::MOVUPDrm, X86::MOVDQUrm }, {X86::MOVDQUrm, X86::MOVUPSrm, X86::MOVUPDrm},
{ X86::MOVNTPSmr, X86::MOVNTPDmr, X86::MOVNTDQmr }, {X86::MOVNTDQmr, X86::MOVNTPSmr, X86::MOVNTPDmr},
{ X86::ANDNPSrm, X86::ANDNPDrm, X86::PANDNrm }, {X86::PANDNrm, X86::ANDNPSrm, X86::ANDNPDrm},
{ X86::ANDNPSrr, X86::ANDNPDrr, X86::PANDNrr }, {X86::PANDNrr, X86::ANDNPSrr, X86::ANDNPDrr},
{ X86::ANDPSrm, X86::ANDPDrm, X86::PANDrm }, {X86::PANDrm, X86::ANDPSrm, X86::ANDPDrm},
{ X86::ANDPSrr, X86::ANDPDrr, X86::PANDrr }, {X86::PANDrr, X86::ANDPSrr, X86::ANDPDrr},
{ X86::ORPSrm, X86::ORPDrm, X86::PORrm }, {X86::PORrm, X86::ORPSrm, X86::ORPDrm},
{ X86::ORPSrr, X86::ORPDrr, X86::PORrr }, {X86::PORrr, X86::ORPSrr, X86::ORPDrr},
{ X86::XORPSrm, X86::XORPDrm, X86::PXORrm }, {X86::PXORrm, X86::XORPSrm, X86::XORPDrm},
{ X86::XORPSrr, X86::XORPDrr, X86::PXORrr }, {X86::PXORrr, X86::XORPSrr, X86::XORPDrr},
// AVX 128-bit support // AVX 128-bit support
{ X86::VMOVAPSmr, X86::VMOVAPDmr, X86::VMOVDQAmr }, {X86::VMOVDQAmr, X86::VMOVAPSmr, X86::VMOVAPDmr},
{ X86::VMOVAPSrm, X86::VMOVAPDrm, X86::VMOVDQArm }, {X86::VMOVDQArm, X86::VMOVAPSrm, X86::VMOVAPDrm},
{ X86::VMOVAPSrr, X86::VMOVAPDrr, X86::VMOVDQArr }, {X86::VMOVDQArr, X86::VMOVAPSrr, X86::VMOVAPDrr},
{ X86::VMOVUPSmr, X86::VMOVUPDmr, X86::VMOVDQUmr }, {X86::VMOVDQUmr, X86::VMOVUPSmr, X86::VMOVUPDmr},
{ X86::VMOVUPSrm, X86::VMOVUPDrm, X86::VMOVDQUrm }, {X86::VMOVDQUrm, X86::VMOVUPSrm, X86::VMOVUPDrm},
{ X86::VMOVNTPSmr, X86::VMOVNTPDmr, X86::VMOVNTDQmr }, {X86::VMOVNTDQmr, X86::VMOVNTPSmr, X86::VMOVNTPDmr},
{ X86::VANDNPSrm, X86::VANDNPDrm, X86::VPANDNrm }, {X86::VPANDNrm, X86::VANDNPSrm, X86::VANDNPDrm},
{ X86::VANDNPSrr, X86::VANDNPDrr, X86::VPANDNrr }, {X86::VPANDNrr, X86::VANDNPSrr, X86::VANDNPDrr},
{ X86::VANDPSrm, X86::VANDPDrm, X86::VPANDrm }, {X86::VPANDrm, X86::VANDPSrm, X86::VANDPDrm},
{ X86::VANDPSrr, X86::VANDPDrr, X86::VPANDrr }, {X86::VPANDrr, X86::VANDPSrr, X86::VANDPDrr},
{ X86::VORPSrm, X86::VORPDrm, X86::VPORrm }, {X86::VPORrm, X86::VORPSrm, X86::VORPDrm},
{ X86::VORPSrr, X86::VORPDrr, X86::VPORrr }, {X86::VPORrr, X86::VORPSrr, X86::VORPDrr},
{ X86::VXORPSrm, X86::VXORPDrm, X86::VPXORrm }, {X86::VPXORrm, X86::VXORPSrm, X86::VXORPDrm},
{ X86::VXORPSrr, X86::VXORPDrr, X86::VPXORrr }, {X86::VPXORrr, X86::VXORPSrr, X86::VXORPDrr},
// AVX 256-bit support // AVX 256-bit support
{ X86::VMOVAPSYmr, X86::VMOVAPDYmr, X86::VMOVDQAYmr }, {X86::VMOVDQAYmr, X86::VMOVAPSYmr, X86::VMOVAPDYmr},
{ X86::VMOVAPSYrm, X86::VMOVAPDYrm, X86::VMOVDQAYrm }, {X86::VMOVDQAYrm, X86::VMOVAPSYrm, X86::VMOVAPDYrm},
{ X86::VMOVAPSYrr, X86::VMOVAPDYrr, X86::VMOVDQAYrr }, {X86::VMOVDQAYrr, X86::VMOVAPSYrr, X86::VMOVAPDYrr},
{ X86::VMOVUPSYmr, X86::VMOVUPDYmr, X86::VMOVDQUYmr }, {X86::VMOVDQUYmr, X86::VMOVUPSYmr, X86::VMOVUPDYmr},
{ X86::VMOVUPSYrm, X86::VMOVUPDYrm, X86::VMOVDQUYrm }, {X86::VMOVDQUYrm, X86::VMOVUPSYrm, X86::VMOVUPDYrm},
{ X86::VMOVNTPSYmr, X86::VMOVNTPDYmr, X86::VMOVNTDQYmr } {X86::VMOVNTDQYmr, X86::VMOVNTPSYmr, X86::VMOVNTPDYmr}};
};
static const uint16_t ReplaceableInstrsAVX2[][3] = { static const uint16_t ReplaceableInstrsAVX2[][3] = {
//PackedSingle PackedDouble PackedInt // PackedInt PackedSingle PackedDouble
{ X86::VANDNPSYrm, X86::VANDNPDYrm, X86::VPANDNYrm }, {X86::VPANDNYrm, X86::VANDNPSYrm, X86::VANDNPDYrm},
{ X86::VANDNPSYrr, X86::VANDNPDYrr, X86::VPANDNYrr }, {X86::VPANDNYrr, X86::VANDNPSYrr, X86::VANDNPDYrr},
{ X86::VANDPSYrm, X86::VANDPDYrm, X86::VPANDYrm }, {X86::VPANDYrm, X86::VANDPSYrm, X86::VANDPDYrm},
{ X86::VANDPSYrr, X86::VANDPDYrr, X86::VPANDYrr }, {X86::VPANDYrr, X86::VANDPSYrr, X86::VANDPDYrr},
{ X86::VORPSYrm, X86::VORPDYrm, X86::VPORYrm }, {X86::VPORYrm, X86::VORPSYrm, X86::VORPDYrm},
{ X86::VORPSYrr, X86::VORPDYrr, X86::VPORYrr }, {X86::VPORYrr, X86::VORPSYrr, X86::VORPDYrr},
{ X86::VXORPSYrm, X86::VXORPDYrm, X86::VPXORYrm }, {X86::VPXORYrm, X86::VXORPSYrm, X86::VXORPDYrm},
{ X86::VXORPSYrr, X86::VXORPDYrr, X86::VPXORYrr }, {X86::VPXORYrr, X86::VXORPSYrr, X86::VXORPDYrr},
{ X86::VEXTRACTF128mr, X86::VEXTRACTF128mr, X86::VEXTRACTI128mr }, {X86::VEXTRACTI128mr, X86::VEXTRACTF128mr, X86::VEXTRACTF128mr},
{ X86::VEXTRACTF128rr, X86::VEXTRACTF128rr, X86::VEXTRACTI128rr }, {X86::VEXTRACTI128rr, X86::VEXTRACTF128rr, X86::VEXTRACTF128rr},
{ X86::VINSERTF128rm, X86::VINSERTF128rm, X86::VINSERTI128rm }, {X86::VINSERTI128rm, X86::VINSERTF128rm, X86::VINSERTF128rm},
{ X86::VINSERTF128rr, X86::VINSERTF128rr, X86::VINSERTI128rr }, {X86::VINSERTI128rr, X86::VINSERTF128rr, X86::VINSERTF128rr},
{ X86::VPERM2F128rm, X86::VPERM2F128rm, X86::VPERM2I128rm }, {X86::VPERM2I128rm, X86::VPERM2F128rm, X86::VPERM2F128rm},
{ X86::VPERM2F128rr, X86::VPERM2F128rr, X86::VPERM2I128rr }, {X86::VPERM2I128rr, X86::VPERM2F128rr, X86::VPERM2F128rr},
{ X86::VBROADCASTSSrm, X86::VBROADCASTSSrm, X86::VPBROADCASTDrm}, {X86::VPBROADCASTDrm, X86::VBROADCASTSSrm, X86::VBROADCASTSSrm},
{ X86::VBROADCASTSSrr, X86::VBROADCASTSSrr, X86::VPBROADCASTDrr}, {X86::VPBROADCASTDrr, X86::VBROADCASTSSrr, X86::VBROADCASTSSrr},
{ X86::VBROADCASTSSYrr, X86::VBROADCASTSSYrr, X86::VPBROADCASTDYrr}, {X86::VPBROADCASTDYrr, X86::VBROADCASTSSYrr, X86::VBROADCASTSSYrr},
{ X86::VBROADCASTSSYrm, X86::VBROADCASTSSYrm, X86::VPBROADCASTDYrm}, {X86::VPBROADCASTDYrm, X86::VBROADCASTSSYrm, X86::VBROADCASTSSYrm},
{ X86::VBROADCASTSDYrr, X86::VBROADCASTSDYrr, X86::VPBROADCASTQYrr}, {X86::VPBROADCASTQYrr, X86::VBROADCASTSDYrr, X86::VBROADCASTSDYrr},
{ X86::VBROADCASTSDYrm, X86::VBROADCASTSDYrm, X86::VPBROADCASTQYrm} {X86::VPBROADCASTQYrm, X86::VBROADCASTSDYrm, X86::VBROADCASTSDYrm}};
};
// FIXME: Some shuffle and unpack instructions have equivalents in different // FIXME: Some shuffle and unpack instructions have equivalents in different
// domains, but they require a bit more work than just switching opcodes. // domains, but they require a bit more work than just switching opcodes.
static const uint16_t *lookup(unsigned opcode, unsigned domain) { static const uint16_t *lookup(unsigned opcode, unsigned domain) {
for (unsigned i = 0, e = array_lengthof(ReplaceableInstrs); i != e; ++i) for (unsigned i = 0, e = array_lengthof(ReplaceableInstrs); i != e; ++i)
if (ReplaceableInstrs[i][domain-1] == opcode) if (ReplaceableInstrs[i][domain-1] == opcode)
return ReplaceableInstrs[i]; return ReplaceableInstrs[i];
Show All 10 Lines
std::pair<uint16_t, uint16_t> std::pair<uint16_t, uint16_t>
X86InstrInfo::getExecutionDomain(const MachineInstr *MI) const { X86InstrInfo::getExecutionDomain(const MachineInstr *MI) const {
uint16_t domain = (MI->getDesc().TSFlags >> X86II::SSEDomainShift) & 3; uint16_t domain = (MI->getDesc().TSFlags >> X86II::SSEDomainShift) & 3;
bool hasAVX2 = Subtarget.hasAVX2(); bool hasAVX2 = Subtarget.hasAVX2();
uint16_t validDomains = 0; uint16_t validDomains = 0;
if (domain && lookup(MI->getOpcode(), domain)) if (domain && lookup(MI->getOpcode(), domain))
validDomains = 0xe; validDomains = 0xe;
else if (domain && lookupAVX2(MI->getOpcode(), domain)) else if (domain && lookupAVX2(MI->getOpcode(), domain))
validDomains = hasAVX2 ? 0xe : 0x6; validDomains = hasAVX2 ? 0xe : 0xc;
return std::make_pair(domain, validDomains); return std::make_pair(domain, validDomains);
} }
void X86InstrInfo::setExecutionDomain(MachineInstr *MI, unsigned Domain) const { void X86InstrInfo::setExecutionDomain(MachineInstr *MI, unsigned Domain) const {
assert(Domain>0 && Domain<4 && "Invalid execution domain"); assert(Domain>0 && Domain<4 && "Invalid execution domain");
uint16_t dom = (MI->getDesc().TSFlags >> X86II::SSEDomainShift) & 3; uint16_t dom = (MI->getDesc().TSFlags >> X86II::SSEDomainShift) & 3;
assert(dom && "Not an SSE instruction"); assert(dom && "Not an SSE instruction");
const uint16_t *table = lookup(MI->getOpcode(), dom); const uint16_t *table = lookup(MI->getOpcode(), dom);
if (!table) { // try the other table if (!table) { // try the other table
assert((Subtarget.hasAVX2() || Domain < 3) && assert((Subtarget.hasAVX2() || Domain > 1) &&
"256-bit vector operations only available in AVX2"); "256-bit vector operations only available in AVX2");
table = lookupAVX2(MI->getOpcode(), dom); table = lookupAVX2(MI->getOpcode(), dom);
} }
assert(table && "Cannot change domain"); assert(table && "Cannot change domain");
MI->setDesc(get(table[Domain-1])); MI->setDesc(get(table[Domain-1]));
} }
/// getNoopForMachoTarget - Return the noop instruction to use for a noop. /// getNoopForMachoTarget - Return the noop instruction to use for a noop.
▲ Show 20 Lines • Show All 298 Lines • Show Last 20 Lines