This is an archive of the discontinued LLVM Phabricator instance.

[clang][ARM] Fix msvc arm{64} builtins to use int on LP64 systems.
ClosedPublic

Authored by Bigcheese on Jul 3 2019, 3:54 PM.

Details

Summary

The InterlockedX_{acq,nf,rel} functions deal with 32 bits which is long on
MSVC, but int on most other systems.

This also checks that ReadStatusRegister and WriteStatusRegister have
the correct type on aarch64-darwin.

This is applying the same fix as https://reviews.llvm.org/D34377

rdar://52196679

Diff Detail

Repository
rL LLVM

Event Timeline

Bigcheese created this revision.Jul 3 2019, 3:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 3 2019, 3:54 PM
Bigcheese edited the summary of this revision. (Show Details)Jul 3 2019, 3:59 PM
jfb accepted this revision.Jul 3 2019, 4:03 PM
jfb added inline comments.
test/CodeGen/ms-intrinsics-other.c
212 ↗(On Diff #207907)

Looks like only cmpxchg preserves volatile? That's not your patch, but seems bad 😖

This revision is now accepted and ready to land.Jul 3 2019, 4:03 PM

Do the changes to BuiltinsARM.def have any practical effect? long should be 32 bits on all 32-bit ARM targets.

Do the changes to BuiltinsARM.def have any practical effect? long should be 32 bits on all 32-bit ARM targets.

I don't think they do right now. I updated them there as that's what the original patch did.

rnk accepted this revision.Jul 3 2019, 5:40 PM

Please check the commit message:

[clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, right?

Otherwise, sounds good.

In D64164#1569679, @rnk wrote:

Please check the commit message:

[clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, right?

Otherwise, sounds good.

Thanks, I did indeed mean LP64.

Bigcheese retitled this revision from [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems. to [clang][ARM] Fix msvc arm{64} builtins to use int on LP64 systems..Jul 3 2019, 5:44 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2019, 1:42 PM