This aims to add support for __cpu_model and address Bug 25510. It uses the code from lib/Support/Host.cpp for cpu detection, and creates __cpu_model with that info. Tested on OSX and built on Linux as well (though libgcc is the default). The use of "asm" required -std=gnu99, hence the cmake change. Corrections on better addressing this are welcome. Previously reverted, up for review again to iron out outstanding issues.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Couple of small comments, otherwise looks much better.
Thanks!
-eric
lib/builtins/cpu_model.c | ||
---|---|---|
16–18 ↗ | (On Diff #63356) | Can probably use a smaller set here maybe (and a couple of times later)? Union of x86, x86_64, and windows provide is probably good enough. |
126–147 ↗ | (On Diff #63356) | The comment and everything else feels weird here. Can you elaborate more? Also, the ifdefs seem unnecessary. |
Couple more inline comments, otherwise it's looking pretty good to me.
Thanks!
-eric
lib/builtins/cpu_model.c | ||
---|---|---|
130 ↗ | (On Diff #63858) | The syntax isn't for sure. :) MSDN doesn't conditionalize the __cpuid intrinsic so I don't think this routine is necessary for MSVC and the return 1 is fine. Also, return true/false? |
202 ↗ | (On Diff #63858) | The return values here seem duplicated with the isCpuIDSupported routine? |
263 ↗ | (On Diff #63858) | Go ahead and document the return value and the arguments here please. |
I'm not sure the asserts are necessary, but I know you like them. That said, couple of inline comments then OK.
Thanks!
-eric
lib/builtins/cpu_model.c | ||
---|---|---|
193 ↗ | (On Diff #64022) | or MSVC. |
205 ↗ | (On Diff #64022) | Add a FIXME to find out if we should save this for clang as well please? |
243 ↗ | (On Diff #64022) | or MSVC |
lib/builtins/cpu_model.c | ||
---|---|---|
167 ↗ | (On Diff #64022) | Typo |
lib/builtins/cpu_model.c | ||
---|---|---|
167 ↗ | (On Diff #64022) | To clarify: "Preserve". |
The patch introduced this: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24764/steps/build%20with%20ninja/logs/stdio
FAILED: /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/clang_build/bin/clang -D_DEBUG -D_GNU_SOURCE -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/builtins -I/mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/builtins -Iinclude -I/mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include -gmlt -fPIC -Wall -W -Wno-unused-parameter -Wwrite-strings -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wdelete-non-virtual-dtor -Werror -Werror=date-time -fcolor-diagnostics -ffunction-sections -fdata-sections -Wall -Werror -Wno-unused-parameter -O3 -UNDEBUG -m32 -std=gnu99 -MMD -MT projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-i386.dir/cpu_model.c.o -MF projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-i386.dir/cpu_model.c.o.d -o projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins-i386.dir/cpu_model.c.o -c /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/builtins/cpu_model.c
/mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/builtins/cpu_model.c:174:3: error: extension used [-Werror,-Wlanguage-extension-token]
asm("movl\t%%ebx, %%esi\n\t" ^
/mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/projects/compiler-rt/lib/builtins/cpu_model.c:224:3: error: extension used [-Werror,-Wlanguage-extension-token]
asm("movl\t%%ebx, %%esi\n\t"