On Tue, May 14, 2013 at 08:58:55AM +0200, Uros Bizjak wrote: > I think that the option should be named -mtarget-builtins.
There shouldn't be an option for it at all. If constructing the builtins is slow (it is), we should just create them lazily, the *builtin_decl_{explicit,implicit}* APIs were a first step for that, plus we should build some gperf table of all the generic and all the target builtins and record prefixes used to find them (__builtin_, __sync_, __atomic_, what else?), then just teach FE that if they are looking up a symbol prefixed with one of these, they should also look it up in the perfect hash table and if found there, call some function to construct the builtin. Of course, this isn't a prerequisite of the changes you are looking for, but introducing an option that hopefully will be completely useless in a few months is just a bad idea. > Since HJ is OK with this user-visible change, the patch is OK for > mainline (with eventually renamed option). We also have an option to > switch this new functionality off, and we are early enough in the > development cycle to find out if anything is fundamentaly broken with > this approach. > > BTW, does this patch address the request in PR57202? I'm strongly against the patch in its current form, it is a hack rather than a solution. I don't see how it could be properly tested, when say immintrin.h and x86intrin.h is still full of code like: #ifdef __AVX__ #include <avxintrin.h> #endif so when you just #include <x86intrin.h> rather than the few headers that were tested, you are out of luck. The right solution is really IMNSHO something along the lines of: #ifndef __AVX__ #pragma GCC push_options #pragma GCC target("avx") #define __AVXINTRIN_H_POP_OPTIONS__ #endif ... #ifdef __AVXINTRIN_H_POP_OPTIONS__ #pragma GCC pop_options #undef __AVXINTRIN_H_POP_OPTIONS__ #endif around the headers. As can be seen on say -O2 -mno-avx: #ifndef __AVX__ #pragma GCC push_options #pragma GCC target("avx") #define __DISABLE_AVX__ #endif typedef float __v8sf __attribute__((vector_size (32))); extern __v8sf a, b; extern __inline int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) bar (int x) { a = b + 1.0f; return x + 1; } #ifdef __DISABLE_AVX__ #pragma GCC pop_options #undef __DISABLE_AVX__ #endif int __attribute__((target ("avx2"))) avx2 (int x) { return bar (x) + bar (x + 1); } int __attribute__((target ("avx"))) avx (int x) { return bar (x) + bar (x + 1); } int __attribute__((target ("xop"))) xop (int x) { return bar (x) + bar (x + 1); } int __attribute__((target ("sse2"))) sse2 (int x) { return bar (x) + bar (x + 1); } int nothing (int x) { return bar (x) + bar (x + 1); } the inliner happily inlines the avx target function into avx/avx2/xop targetted functions (correct), inlines it even into sse2 (something that should be fixed not to), and doesn't inline into nothing (IMHO correct, we want an error in that case, one shouldn't use avx intrinsics in say sse2 only targetted function (unless -mavx is used on command line, i.e. the check should always be if the caller's target set is equal or superset of callee's target set). When trying with -O2 -mno-avx: #ifndef __AVX__ #pragma GCC push_options #pragma GCC target("avx") #define __DISABLE_AVX__ #endif typedef float __v8sf __attribute__ ((__vector_size__ (32))); typedef float __m256 __attribute__ ((__vector_size__ (32), __may_alias__)); extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm256_and_ps (__m256 __A, __m256 __B) { return (__m256) __builtin_ia32_andps256 ((__v8sf)__A, (__v8sf)__B); } #ifdef __DISABLE_AVX__ #pragma GCC pop_options #undef __DISABLE_AVX__ #endif __m256 a, b, c; void __attribute__((target ("avx"))) foo (void) { a = _mm256_and_ps (b, c); } we get bogus errors and ICE: tty2.c: In function '_mm256_and_ps': tty2.c:9:1: note: The ABI for passing parameters with 32-byte alignment has changed in GCC 4.6 tty2.c: In function 'foo': tty2.c:9:82: error: '__builtin_ia32_andps256' needs isa option -m32 tty2.c:9:82: internal compiler error: in emit_move_insn, at expr.c:3486 0x77a3d2 emit_move_insn(rtx_def*, rtx_def*) ../../gcc/expr.c:3485 (I have added "1 ||" instead of your generate_builtins into i386.c (def_builtin)), that just shows that target attribute/pragma support still has very severe issues that need to be fixed, instead of papered around. Note, we ICE on: #pragma GCC target ("mavx") That should be fixed too. Jakub