[cfe-dev] Want input to solve PR18323

[David Wiberg]

2014/1/4 Reid Kleckner <rnk at google.com>:
> Interesting.  Here's the libstdc++ code that uses __a:
> http://gcc.gnu.org/onlinedocs/gcc-4.8.0/libstdc++/api/a01396_source.html#l00055
>
> Ted Kremenek added pragmas that silence warnings about this in October:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?r1=192143&r2=192142&pathrev=192143
>
> The only way I can think of to fix this with the type checking is to use a
> helper like:
>
> static inline always_inline,nodebug __m128i __check_type_m128i(__m128i __a)
> { return __a; }
> #define _mm_slli_si128(a, __sl2) \
> (__m128i)__builtin_ia32_pslldqi128(__check_type_m128i(a), (count)*8)
>
Thanks for the suggestion. I tested it and it seems to work
(check-clang and test-case in PR passes). There are quite a few macros
in the lib\Header files which have potential for this kind of
name-clash (>100). I think it's a good idea to fix them all but I
would like some reassurance that this is the correct path before
attempting this. Are there any more tests that I should perform? Where
should these kind of helper functions be placed since they are needed
in multiple intrinsic files?

> On Fri, Jan 3, 2014 at 5:29 PM, David Wiberg <dwiberg at gmail.com> wrote:
>>
>> 2014/1/4 Reid Kleckner <rnk at google.com>:
>> > So the preprocessed code looks like:
>> > __m128i __a = ...
>> > __extension__ ({
>> >   __m128i __a = (__a);
>> >   (__m128i)__builtin_ia32_pslldqi128(__a, (count)*8); })
>> >
>> > ... making __a uninitialized.  Is that correct?
>> I think so, yes.
>>
>> >
>> > Can you pre-process the source code and upload that to the PR?
>> Done
>>
>> >
>> > I would say that the user is using a double-underscore name which is in
>> > the
>> > implementers namespace and they shouldn't do that.  We can probably go
>> > ahead
>> > and change the name in this instance just to fix the gcc test suite.
>> The problem is that the name isn't coming from the test suite but from
>> the libstdc++ implementation of <random> (see the PR for details). As
>> such I guess there will be more of a discussing regarding who can use
>> that namespace. What I don't like about just changing the variable
>> name in the macro is that there is no guarantee that a similar name
>> clash won't happen again. There's also no warning produced when
>> compiling since these are considered system headers which make the
>> problem hard to find.
>>
>> >
>> >
>> > On Fri, Jan 3, 2014 at 4:16 PM, David Wiberg <dwiberg at gmail.com> wrote:
>> >>
>> >> Hi all,
>> >>
>> >> The file Headers/emmintrin.h contains macros where variables are
>> >> created inside the macro definition which can cause name collisions
>> >> and PR18323 shows an example where this has happened. Variables were
>> >> added to macros in R143792 with comment "Fix vector macros to
>> >> correctly check argument types. <rdar://problem/10261670>".
>> >>
>> >> One way of solving this particular PR is to revert R143792, but since
>> >> that has led to other problems according to the commit message above
>> >> I'm not sure if that's the right way and would appreciate other
>> >> suggestions or comments.
>> >>
>> >> / David
>> >> _______________________________________________
>> >> cfe-dev mailing list
>> >> cfe-dev at cs.uiuc.edu
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> >
>> >
>
>