Page MenuHomePhabricator

[X86] WaitPKG instructions
ClosedPublic

Authored by GBuella on Apr 4 2018, 4:51 AM.

Details

Summary

Three new instructions:

umonitor - Sets up a linear address range to be
monitored by hardware and activates the monitor.
The address range should be a writeback memory
caching type.

umwait - A hint that allows the processor to
stop instruction execution and enter an
implementation-dependent optimized state
until occurrence of a class of events.

tpause - Directs the processor to enter an
implementation-dependent optimized state
until the TSC reaches the value in EDX:EAX.

Also modifying the description of the mfence
instruction, as the rep prefix (0xF3) was allowed
before, which would conflict with umonitor during
disassembly.

Before this patch
$ echo 0xf3,0x0f,0xae,0xf0 | llvm-mc -disassemble

.text
mfence

After this patch
$ echo 0xf3,0x0f,0xae,0xf0 | llvm-mc -disassemble

.text
umonitor        %rax

Diff Detail

Repository
rL LLVM

Event Timeline

GBuella created this revision.Apr 4 2018, 4:51 AM
craig.topper added inline comments.Apr 4 2018, 10:10 AM
lib/Target/X86/X86InstrInfo.td
2680 ↗(On Diff #140938)

Line these up with the 0xAE on the first line.

2683 ↗(On Diff #140938)

Line these up.

2705 ↗(On Diff #140938)

Based on where these landed in the opcode map, you should change the TB on MFENCE in X86InstrSSE.td to PS.

Before this patch, this sequence of bytes disassembled as mfence, but it should have been invalid. The PS will fix that.
0xf3,0x41,0x0f,0xae,0xf0

I suspect LFENCE should probably be PS as well, but we haven't gotten a collision there yet. SFENCE was fixed when PCOMMIT was added. Though PCOMMIT was later removed.

GBuella updated this revision to Diff 141013.Apr 4 2018, 11:45 AM
GBuella edited the summary of this revision. (Show Details)

Fixed mfence description

GBuella marked 3 inline comments as done.Apr 4 2018, 11:46 AM

Please always upload patches with full context (-U99999)

GBuella updated this revision to Diff 141016.Apr 4 2018, 12:14 PM

Supply diff context.

craig.topper added inline comments.Apr 4 2018, 12:48 PM
lib/Target/X86/X86InstrInfo.td
2683 ↗(On Diff #141016)

What is the llvm-mc assembler expectation here? As this is defined it means that 64-bit mode assembly must alway use a 64-bit register and 32-bit mode must always use a 32-bit register. The documentation itself mentions being able to use a 16-bit register and that the 0x67 prefix has an effect, but that's not captured here.

2701 ↗(On Diff #141016)

Again this also has a weird assembler expectation the register size is dependent on the mode.

test/CodeGen/X86/waitpkg-intrinsics-64.ll
14 ↗(On Diff #141016)

Can these tests be simplified to not use so many loads and stores? Is this the -O0 clang output? Can we use -O2 output instead.

GBuella updated this revision to Diff 141285.Apr 6 2018, 12:37 AM
GBuella marked 2 inline comments as done.
GBuella updated this revision to Diff 141318.Apr 6 2018, 5:12 AM

Fixed umonitor 16 bit decoding.
Fixed umwait, tpause instruction description - modified the corresponding intrinsics, thus D45254 needs to be updated.

GBuella marked an inline comment as done.Apr 6 2018, 5:13 AM
This revision is now accepted and ready to land.Apr 6 2018, 9:38 AM
craig.topper requested changes to this revision.Apr 6 2018, 9:23 PM
craig.topper added inline comments.
lib/Target/X86/X86InstrInfo.td
2688 ↗(On Diff #141318)

I think UMWAIT and TPAUSE Def EFLAGS.

This revision now requires changes to proceed.Apr 6 2018, 9:23 PM
GBuella updated this revision to Diff 142911.Apr 18 2018, 5:30 AM
craig.topper added inline comments.Apr 18 2018, 3:04 PM
test/CodeGen/X86/waitpkg-intrinsics-64.ll
1 ↗(On Diff #142911)

Can we merge the two tests now that the types aren't dependent on mode?

GBuella updated this revision to Diff 143251.Apr 20 2018, 1:18 AM
GBuella retitled this revision from [X86][WAITPKG] WaitPKG instructions to [X86] WaitPKG instructions.
GBuella marked 2 inline comments as done.
This revision is now accepted and ready to land.Apr 20 2018, 9:49 AM
This revision was automatically updated to reflect the committed changes.