This is an archive of the discontinued LLVM Phabricator instance.

[X86] Add CMPXCHG8B feature flag. Set it for all CPUs except i386/i486 including 'generic'. Disable use of CMPXCHG8B when this flag isn't set.
ClosedPublic

Authored by craig.topper on Mar 19 2019, 11:06 PM.

Details

Summary

CMPXCHG8B was introduced on i586/pentium generation.

If its not enabled, limit the atomic width to 32 bits so the AtomicExpandPass will expand to lib calls. Unclear if we should be using a different limit for other configs. The default is 1024 and experimentation shows that using an i256 atomic will cause a crash in SelectionDAG.

Diff Detail

Event Timeline

craig.topper created this revision.Mar 19 2019, 11:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2019, 11:06 PM
efriedma added inline comments.Mar 20 2019, 12:32 PM
lib/Support/Host.cpp
1267 ↗(On Diff #191435)

If someone were to actually try running LLVM on certain 486 models, it would crash because it doesn't actually support CPUID, but I guess that's orthogonal. :)

lib/Target/X86/X86.td
1118 ↗(On Diff #191435)

This looks a little weird; c3-2 but not c3? Are you sure that's right?

lib/Target/X86/X86ISelLowering.cpp
480 ↗(On Diff #191435)

?

craig.topper marked 2 inline comments as done.Mar 20 2019, 1:04 PM
craig.topper added inline comments.
lib/Target/X86/X86.td
1118 ↗(On Diff #191435)

I think its consistent with gcc. It's also consistent with the CPU enum in clang. c3 is below i586 but c3-2 is above i586.

lib/Target/X86/X86ISelLowering.cpp
480 ↗(On Diff #191435)

Oops. Leftover from before I changed the AtomicSizeInBits to 32 above.

Remove commented out code

Does this affect the assembler? If so, should we also have an assembler testcase?

Does this affect the assembler? If so, should we also have an assembler testcase?

I shouldn't. We don't do fine grained feature control in the X86 assembler. The only AssemblerPredicates we have have are for 16/32/64 bit mode. We do block cmpxchg16b in 64-bit mode, but we shouldn't have to block cmpxchg8b anywhere.

This revision is now accepted and ready to land.Mar 20 2019, 3:54 PM
This revision was automatically updated to reflect the committed changes.