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
