This is an archive of the discontinued LLVM Phabricator instance.

android: enable double-word CAS on x86_64
ClosedPublic

Authored by compnerd on Jun 25 2019, 8:54 AM.

Details

Summary

The android target assumes that for the x86_64 target, the CPU supports SSE4.2 and popcnt. This implies that the CPU is Nehalem or newer. This should be sufficiently new to provide the double word compare and exchange instruction. This allows us to directly lower __sync_val_compare_and_swap_16 to a cmpxchg16b. It appears that the libatomic in android's NDK does not provide the implementation for lowering calls to the library function.

Diff Detail

Repository
rC Clang

Event Timeline

compnerd created this revision.Jun 25 2019, 8:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2019, 8:54 AM
Herald added a subscriber: jfb. · View Herald Transcript

Is there some test for this area of code?

srhines accepted this revision.Jun 25 2019, 9:52 AM

Craig, can you confirm that this is acceptable? I don't think there are any chips with SSE4.2 but without cx16, so this just seemed like an oversight. It might be a good idea to really audit the list of possible CPU features for other missing inclusions. Another idea would be to set a baseline minimum CPU for Android, which would cut down on having to specify so many features separately.

This revision is now accepted and ready to land.Jun 25 2019, 9:52 AM

@lebedev.ri - sure, I will add a driver test to ensure that the feature is set on the command line when invoked from the driver, however, I don't think that there is really much in terms of testing that you can do for this type of stuff other than throw a large corpus at it.

compnerd updated this revision to Diff 206508.Jun 25 2019, 12:48 PM

add additional context and test case

craig.topper added inline comments.Jun 25 2019, 12:50 PM
test/Driver/android-default-x86_64.c
1

should we just update the android 64 case in clang-translation.c instead?

Another odd feature that's missing from early x86-64 CPUs is sahf.

compnerd updated this revision to Diff 206519.Jun 25 2019, 1:13 PM

Move test case around

@craig.topper, hmm, what happens in terms of CG when LAHF/SAHF are not available? I assume its just worse CG as you could spill AH onto the stack and do a load/store. This actually results in library calls which may not be possible to fulfill.

Yeah LAHF/SAHF just improves codegen. Lack of it won't result in any library calls.

This patch LGTM

compnerd closed this revision.Jun 25 2019, 2:43 PM

SVN r364352