Page MenuHomePhabricator

[COFF, ARM64] Add MS builtins __dmb, __dsb, __isb
AbandonedPublic

Authored by mstorsjo on Jul 31 2017, 1:28 PM.

Details

Diff Detail

Event Timeline

mgrang created this revision.Jul 31 2017, 1:28 PM
mstorsjo added inline comments.Jul 31 2017, 1:35 PM
include/clang/Basic/BuiltinsAArch64.def
49

Shouldn't these be limited to MSVC mode only? Have a look at BuiltinsARM.def, where there's cases like LANGBUILTIN(__dmb, "vUi", "nc", ALL_MS_LANGUAGES) instead.

mgrang added inline comments.Jul 31 2017, 1:48 PM
include/clang/Basic/BuiltinsAArch64.def
49

I do see LANGBUILTIN in BuiltinsARM.def however it is defined as follows:

#   define LANGBUILTIN(ID, TYPE, ATTRS, BUILTIN_LANG) BUILTIN(ID, TYPE, ATTRS)

So it seems the BUILTIN_LANG is never really used. Moreover, using LANGBUILTIN in BuiltinsAArch64.def I run into compilation error like this:

BuiltinsAArch64.def:65:12: error: expected '= constant-expression' or end of enumerator definition
LANGBUILTIN(__dmb, "vUi", "nc", ALL_MS_LANGUAGES)
mstorsjo added inline comments.Jul 31 2017, 1:58 PM
include/clang/Basic/BuiltinsAArch64.def
49

That's only the fallback definition of LANGBUILTIN. The .def file is included multiple times in different contexts. Have a look at e.g. lib/Basic/Targets/ARM.cpp and look for BuiltinsARM.def there, and you'll find a different implementation of LANGBUILTIN.

mgrang added inline comments.Jul 31 2017, 2:02 PM
include/clang/Basic/BuiltinsAArch64.def
49

Got it! Thanks Martin. I will push an update.

@mstorsjo I see that spec2000/eon calls __getReg, _ReadStatusReg and _WriteStatusReg intrinsics for ARM64. I think I would need to implement them in llvm. Do you know where can I find a doc explaining their behavior?

mstorsjo edited edge metadata.Jul 31 2017, 11:17 PM

@mstorsjo I see that spec2000/eon calls __getReg, _ReadStatusReg and _WriteStatusReg intrinsics for ARM64. I think I would need to implement them in llvm. Do you know where can I find a doc explaining their behavior?

Sorry - I have no previous knowledge on them. Googling for _ReadStatusReg gets me to https://msdn.microsoft.com/en-us/library/hh875058.aspx at least (which is for ARM and not ARM64), which indicates that it'd map to the MRS instruction. And __getReg: https://msdn.microsoft.com/en-us/library/kcb3wece(v=vs.120).aspx

Since the ARM64 specifics of them aren't publicly known I guess you'd have to test MSVC to see what they expect in practice. For __getReg it seems to refer to intrin.h for register name constants - I guess arm64intr.h should define the constants for register numbers as well, or are they just called like __getReg(30) (to get the value of x30)?

@mgrang, did you ever get to completing this? I've got a need for this now (only __dmb so far), and if you don't have time, I can try to finish it.

@mgrang, did you ever get to completing this? I've got a need for this now (only __dmb so far), and if you don't have time, I can try to finish it.

Sorry, I never got to complete this as I moved to other priorities. I do not think I have cycles to do this now. Please feel free to take over this patch :)

Sorry, I never got to complete this as I moved to other priorities. I do not think I have cycles to do this now. Please feel free to take over this patch :)

Ok - will do! Thanks for letting me know!

This one can be abandoned now.

mstorsjo commandeered this revision.Oct 24 2017, 1:19 AM
mstorsjo abandoned this revision.
mstorsjo edited reviewers, added: mgrang; removed: mstorsjo.

This was updated, finished and merged in D38821.