[PATCH] D36111: [COFF, ARM64] Add MS builtins __dmb, __dsb, __isb

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: include/clang/Basic/BuiltinsAArch64.def:49
 // Memory barrier
+BUILTIN(__dmb, "vUi", "nc")
+BUILTIN(__dsb, "vUi", "nc")
----------------
mgrang wrote:
> mstorsjo wrote:
> > 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.
> 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)
> ```
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`.


https://reviews.llvm.org/D36111