On Tue, May 14, 2013 at 10:39 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> 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.
Jakub, thanks for your thorough analysis.
It looks that the approach in the patch is fundamentally flawed and
that infrastructure is not developed/fixed enough for alternative
solution.
Based on expressed concerns, I retract my previous approval.
Thanks,
Uros.
