This is an archive of the discontinued LLVM Phabricator instance.

[x86] invpcid LLVM intrinsic
ClosedPublic

Authored by GBuella on May 21 2018, 7:54 AM.

Details

Summary

Re-add the feature flag for invpcid, which was removed in r294561.
Add an intrinsic, which always uses a 32 bit integer as first argument,
while the instruction actually uses a 64 bit register in 64 bit mode
for the INVPCID_TYPE argument.

Diff Detail

Repository
rL LLVM

Event Timeline

GBuella created this revision.May 21 2018, 7:54 AM
craig.topper added inline comments.May 21 2018, 10:00 AM
lib/Target/X86/X86.td
248 ↗(On Diff #147783)

HasINVPCID. It's the name of an instruction so we should keep the caps.

lib/Target/X86/X86InstrInfo.td
907 ↗(On Diff #147783)

hasINVPCID

lib/Target/X86/X86InstrSystem.td
653 ↗(On Diff #147783)

SUBREG_TO_REG should only be used if you know absolutely the instruction that produced $src1 zeroed the upper bits. You would need to analyze the DAG to prove it. See the "def32" PatLeaf in X86InstrCompiler.td. If it can't be proven, you need a fall back pattern to use a MOV32rr instruction to force zeroes into the upper bits.

test/CodeGen/X86/invpcid-intrinsic.ll
38 ↗(On Diff #147783)

Why are you testing with a load for type, but not testing without a load?

GBuella added inline comments.May 22 2018, 7:06 AM
test/CodeGen/X86/invpcid-intrinsic.ll
38 ↗(On Diff #147783)

There is a test without a load above this one.

craig.topper added inline comments.May 22 2018, 8:16 AM
test/CodeGen/X86/invpcid-intrinsic.ll
16 ↗(On Diff #147783)

This shows the bug I mentioned above, rdi needs to be zero extended from edi.

38 ↗(On Diff #147783)

I have no idea how I missed that. Maybe I scrolled down too far.

GBuella updated this revision to Diff 148010.May 22 2018, 8:20 AM
GBuella marked 4 inline comments as done.
craig.topper accepted this revision.May 22 2018, 11:49 AM

LGTM.

It looks like neither icc or MSVC doe a perfect job of ensuring zero extension here. For example: https://godbolt.org/g/UqnXh8

This revision is now accepted and ready to land.May 22 2018, 11:49 AM
This revision was automatically updated to reflect the committed changes.