Re: [RFC C++] Turn on builtin_shuffle for C++.
On 15 June 2012 20:04, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: On 15 June 2012 18:18, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? I don't remember. It might just be that trying to create a constexpr vector variable or calling __builtin_shuffle on it ICEs instead of giving an error. I can keep a note to make some tests at the end of July (I will be mostly away until then), but I believe the code from comment 22 is safer than the one from comment 20, if memory serves. I'm not qualified enough to take a call on what's better in this case and will have to defer to Jason and the C++ maintainers on this one. Now that you've said this I decided to go back and throw more tests through it I've tried to chug through most of the testcases for __builtin_shuffle including a few of my own the simplest of which I show below trying to trigger this issue but can't seem to do so. Maybe something like: #include x86intrin.h int main(){ constexpr __m128d x={1.,2.}; constexpr __m128i y={1,0}; constexpr __m128d z=__builtin_shuffle(x,y); } ? (sorry for the x86 specific code, should be easy to adapt) See also: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53094 Long term, vectors should be literals. But we need something to avoid crashes on operator[] and __builtin_shuffle (ideally implementing the constant version of them). Keeping vectors as non-literals (what I was suggesting) is quite a crude hack. Maybe having them as literals now is a good thing, but it would be good to avoid the ICEs. I've submitted a fresh patch for that over at http://gcc.gnu.org/ml/gcc-patches/2012-06/msg01165.html . thought it better than conflating the 2 threads. regards, Ramana
Re: [RFC C++] Turn on builtin_shuffle for C++.
On 15 June 2012 01:44, Jason Merrill ja...@redhat.com wrote: OK. Thanks, now committed with the only change being that the PR number is now referenced in the Changelog. Ramana
Re: [RFC C++] Turn on builtin_shuffle for C++.
On Thu, 14 Jun 2012, Ramana Radhakrishnan wrote: While experimenting with the fixes to allow neon intrinsics to work with __builtin_shuffle I hit the fact that __builtin_shuffle isn't really supported by the C++ frontend.I'm keen we use __builtin_shuffle for these intrinsics, but that means we need this support in the C++ frontend. I've taken the liberty of pulling Marc's patch from bugzilla, adding the couple of bits and pieces that were needed, moved all the vshuf* tests from gcc.c-torture/execute to c-c++-common/torture which means they run for both the C and C++ compilers, and bootstrapped and regtested this on x86_64, gcc110(powerpc*-linux) and arm-linux-gnueabi (with a cross compiler). I've then verified that all the tests pass and there are no regressions for these targets Any other place I should be moving these tests to ? Ok ? regards, Ramana 2012-06-14 Marc Glisse marc.gli...@inria.fr * c-typeck.c (c_build_vec_perm_expr): Move to c-family/c-common.c. * c-tree.h (c_build_vec_perm_expr): Move to c-family/c-common.h. c-family/ * c-typeck.c (c_build_vec_perm_expr): Move to c-family/c-common.c. * c-tree.h (c_build_vec_perm_expr): Move to c-family/c-common.h. cp/ * semantics.c (literal_type_p): Handle VECTOR_TYPE. (potential_constant_expression_1): Handle VEC_PERM_EXPR. I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 it seems like this may break things with -std=c++11 and you used the old version from comment 19. I am unable to test anything these days (taking a plane tomorrow), sorry. Thanks again for taking charge of the patch, -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? it seems like this may break things with -std=c++11 and you used the old version from comment 19. I am unable to test anything these days (taking a plane tomorrow), sorry. ./g++ -B`pwd` /home/ramrad01/cross-build/fsf/src/gcc-rewrite-permute-intrinsics/gcc/testsuite/c-c++-common/torture/vshuf-v8si.c -std=c++11 -S -O2 appears to work just fine. Am I missing something here ? A number of tests that pass vector constants to __builtin_shuffle appear to be working ok . Can you point out what testcase and how it is broken with std=c++11. I remember trying some simple tests with that and that appeared to work. One thing I do notice now is that all the c-c++-common tests are possibly not running with -std=c++11 . However they appear to be compiling ok ? My motivation in this stems from being able to rewrite the Neon permute intrinsics in C and C++ with __builtin_shuffle. Ramana Thanks again for taking charge of the patch, -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? I don't remember. It might just be that trying to create a constexpr vector variable or calling __builtin_shuffle on it ICEs instead of giving an error. I can keep a note to make some tests at the end of July (I will be mostly away until then), but I believe the code from comment 22 is safer than the one from comment 20, if memory serves. -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
On 15 June 2012 18:18, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? I don't remember. It might just be that trying to create a constexpr vector variable or calling __builtin_shuffle on it ICEs instead of giving an error. I can keep a note to make some tests at the end of July (I will be mostly away until then), but I believe the code from comment 22 is safer than the one from comment 20, if memory serves. I'm not qualified enough to take a call on what's better in this case and will have to defer to Jason and the C++ maintainers on this one. Now that you've said this I decided to go back and throw more tests through it I've tried to chug through most of the testcases for __builtin_shuffle including a few of my own the simplest of which I show below trying to trigger this issue but can't seem to do so. typedef int v4si __attribute__ ((vector_size (16))); v4si c; const v4si d = (v4si) { 10, 11, 23, 33}; v4si vs (v4si a, v4si b) { c = __builtin_shuffle (a, b, (v4si){0, 4, 1, 5}); return a; } Ofcourse it is not complicated C++ in any which way but the frontend ends up generating something like the following (void) (c = VEC_PERM_EXPR a , b , TARGET_EXPR D.5209, {0, 4, 1, 5} ) ; rather than anything else but I could be missing something fundamental here if that's not what you expect the C++ frontend to be doing. regards, Ramana -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: On 15 June 2012 18:18, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? I don't remember. It might just be that trying to create a constexpr vector variable or calling __builtin_shuffle on it ICEs instead of giving an error. I can keep a note to make some tests at the end of July (I will be mostly away until then), but I believe the code from comment 22 is safer than the one from comment 20, if memory serves. I'm not qualified enough to take a call on what's better in this case and will have to defer to Jason and the C++ maintainers on this one. Now that you've said this I decided to go back and throw more tests through it I've tried to chug through most of the testcases for __builtin_shuffle including a few of my own the simplest of which I show below trying to trigger this issue but can't seem to do so. Maybe something like: #include x86intrin.h int main(){ constexpr __m128d x={1.,2.}; constexpr __m128i y={1,0}; constexpr __m128d z=__builtin_shuffle(x,y); } ? (sorry for the x86 specific code, should be easy to adapt) See also: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53094 Long term, vectors should be literals. But we need something to avoid crashes on operator[] and __builtin_shuffle (ideally implementing the constant version of them). Keeping vectors as non-literals (what I was suggesting) is quite a crude hack. Maybe having them as literals now is a good thing, but it would be good to avoid the ICEs. -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
On 15 June 2012 20:04, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: On 15 June 2012 18:18, Marc Glisse marc.gli...@inria.fr wrote: On Fri, 15 Jun 2012, Ramana Radhakrishnan wrote: I just noticed this part. Rereading my comment in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51033#c22 I haven't been able to make it break with -std=c++11 . Is there something I'm missing here ? I don't remember. It might just be that trying to create a constexpr vector variable or calling __builtin_shuffle on it ICEs instead of giving an error. I can keep a note to make some tests at the end of July (I will be mostly away until then), but I believe the code from comment 22 is safer than the one from comment 20, if memory serves. I'm not qualified enough to take a call on what's better in this case and will have to defer to Jason and the C++ maintainers on this one. Now that you've said this I decided to go back and throw more tests through it I've tried to chug through most of the testcases for __builtin_shuffle including a few of my own the simplest of which I show below trying to trigger this issue but can't seem to do so. Maybe something like: #include x86intrin.h int main(){ constexpr __m128d x={1.,2.}; constexpr __m128i y={1,0}; constexpr __m128d z=__builtin_shuffle(x,y); } ? (sorry for the x86 specific code, should be easy to adapt) Thanks for the example and your patience - just shows my ignorance with C++11 :( . I'll try to have a look at this later today and see if I can come up with something and possibly integrating that bit of your patch. See also: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53094 Long term, vectors should be literals. But we need something to avoid crashes on operator[] and __builtin_shuffle (ideally implementing the constant version of them). Keeping vectors as non-literals (what I was suggesting) is quite a crude hack. Maybe having them as literals now is a good thing, but it would be good to avoid the ICEs. Agreed that the compiler shouldn't crash in these cases now that I understand finally what you meant. Have a good break. Thanks, Ramana -- Marc Glisse
Re: [RFC C++] Turn on builtin_shuffle for C++.
OK. Jason