This is an archive of the discontinued LLVM Phabricator instance.

[X86] Describe wbnoinvd instruction
ClosedPublic

Authored by GBuella on Feb 27 2018, 5:33 AM.

Details

Summary

Similar to the wbinvd instruction, except this
one does not invalidate caches. Ring 0 only.
The encoding matches a wbinvd instruction with
an F3 prefix.

Diff Detail

Repository
rL LLVM

Event Timeline

GBuella created this revision.Feb 27 2018, 5:33 AM

Can you add a disassembler test as well?

lib/Support/Host.cpp
1222 ↗(On Diff #136063)

Can you line up the '=' in the clzero line above? This way it's consistent with the HasExtLeaf1 and HasLeaf7 code where everything is nicely lined up.

lib/Target/X86/X86.td
751 ↗(On Diff #136063)

Isn't this instruction only on icelake server? Not icelake desktop/mobile. We may need to separate a icelake server like we do skylake and skylake-avx512.

GBuella updated this revision to Diff 136508.Mar 1 2018, 6:16 AM

Removed wbinvd related code.
Added Ice Lake Server architecture.
Added disassembly tests.

GBuella marked 2 inline comments as done.Mar 1 2018, 6:17 AM
craig.topper accepted this revision.Mar 1 2018, 8:46 AM

LGTM other than that one comment.

include/llvm/IR/IntrinsicsX86.td
6426 ↗(On Diff #136508)

This comment is misleading. WBNOINVD writes back the entire cache hierarchy not just a cache line.

This revision is now accepted and ready to land.Mar 1 2018, 8:46 AM
GBuella updated this revision to Diff 136697.Mar 2 2018, 1:33 AM

Fixed the comment on "cache line write back"

GBuella marked an inline comment as done.Mar 2 2018, 1:36 AM
GBuella updated this revision to Diff 141902.Apr 10 2018, 1:44 PM

Rebased the patch.
Added x86-16 tests.

GBuella edited the summary of this revision. (Show Details)Apr 10 2018, 1:49 PM
craig.topper added inline comments.Apr 10 2018, 2:01 PM
lib/Target/X86/X86InstrSystem.td
488 ↗(On Diff #141902)

Do you know if 0xf3 0x0f 0x09 behaves as wbinvd on older processors. Or was 0xf3 0x0f 0x09 a #UD fault on previous CPUs? Wondering whether WBINVD should have TB or PS. I'm guess since wbinvd is super old the prefixes have always been ignored, but I'm not sure.

GBuella added inline comments.Apr 11 2018, 1:04 AM
lib/Target/X86/X86InstrSystem.td
488 ↗(On Diff #141902)

Well, the ISA says about the REP prefix:
"Use these prefixes only with string and I/O instructions (MOVS, CMPS, SCAS, LODS, STOS, INS, and OUTS). Use of repeat prefixes and/or undefined opcodes with other Intel 64 or IA-32 instructions is reserved; such use may cause unpredictable behavior."

So, one should not use the 0xf3 prefix with winvd.

On the other hand, this still doesn't tell us whether it would be #UD on older CPUs, or ignored.

Since the disassembler I think doesn't use flags like -mwbnoinvd, I guess we can modify wbinvd's description to mention PS.

GBuella updated this revision to Diff 141960.Apr 11 2018, 1:48 AM

Disallow 0xf3 prefix with wbinvd.

Based on the discussion here https://patchwork.kernel.org/patch/10175159/ I think 0xf3 + wbinvd was previously equivalent to wbinvd so I think TB was correct.

Based on the discussion here https://patchwork.kernel.org/patch/10175159/ I think 0xf3 + wbinvd was previously equivalent to wbinvd so I think TB was correct.

Right, but the disassembler is still going to recognize it as wbnoinvd.

Changing it from TB to PS will affect the behavior of 0xf2 0x0f 0x09 and 0x66 0x0f 0x09. Which I think we still want to disassemble to just wbinvd.

GBuella updated this revision to Diff 142052.Apr 11 2018, 11:17 AM

Changing it from TB to PS will affect the behavior of 0xf2 0x0f 0x09 and 0x66 0x0f 0x09. Which I think we still want to disassemble to just wbinvd.

True. I reverted it to TB.

This revision was automatically updated to reflect the committed changes.