LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 42935 - [X86] znver missing cmpxchg8b feature, resulting in expanded atomicrmw
Summary: [X86] znver missing cmpxchg8b feature, resulting in expanded atomicrmw
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Backend: X86 (show other bugs)
Version: 9.0
Hardware: All All
: P enhancement
Assignee: Craig Topper
URL:
Keywords:
Depends on:
Blocks: release-9.0.0
  Show dependency tree
 
Reported: 2019-08-08 09:32 PDT by Nikita Popov
Modified: 2019-08-09 02:56 PDT (History)
6 users (show)

See Also:
Fixed By Commit(s): r368324


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Popov 2019-08-08 09:32:51 PDT
define i64 @test(i64* %p) {
  %x = atomicrmw sub i64* %p, i64 1 seq_cst 
  ret i64 %x 
} 

Under -mcpu=znver1 results in:

	pushq	%rax
	.cfi_def_cfa_offset 16
	movl	$1, %esi
	movl	$5, %edx
	callq	__atomic_fetch_sub_8
	popq	%rcx
	.cfi_def_cfa_offset 8
	retq

While it should produce a "lock xaddq" instruction.

This issue was introduced in https://reviews.llvm.org/D59576, where the CMPXCHG8B feature flag was not added to ZNFeatures.
Comment 1 Simon Pilgrim 2019-08-08 10:26:39 PDT
I agree this looks like a typo.

@craig how safe do you think it would be for FeatureCMPXCHG16B to inherit  FeatureCMPXCHG8B?
Comment 2 Craig Topper 2019-08-08 10:48:31 PDT
I think that's probably safe. The cmpxchg16b encoding is cmpxchg8b with a rex prefix. We didn't connect cx8 to the command line in clang. So I don't think we have to worry about something like -mno-cx8 disabling cx16.
Comment 3 Craig Topper 2019-08-08 11:11:47 PDT
Fixed in r368324 using Simon's proposal.
Comment 4 Hans Wennborg 2019-08-09 02:56:48 PDT
(In reply to Craig Topper from comment #3)
> Fixed in r368324 using Simon's proposal.

Merged to release_90 in r368423.