| Message ID | 20171123210224.GV14653@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix powerpc* ICE with vec builtins with -mno-altivec (PR target/82848) | expand |
Hi Jakub, On Thu, Nov 23, 2017 at 10:02:24PM +0100, Jakub Jelinek wrote: > With -mno-altivec -mno-vsx -mno-fold-gimple we error on __builtin_vec* > builtins used when corresponding ISA is not enabled. > When folding gimple, in some cases we get away with it (e.g. when folding > the builtin to PLUS_EXPR on the generic vectors), because > tree-vect-generic.c lowers those into scalar piecewise operations, > but e.g. in case of FMA_EXPR tree-vect-generic.c isn't doing anything, > expects that FMA_EXPR is actually used only if supported, I think similarly > for various vector widening operations etc. > > One option is not to fold into gimple only those builtins where we know it > will not work on generic vectors; my preference is to not fold any builtin > which would be rejected at expansion time, so user get at least consistent > diagnostics. Usually people are using the altivec.h etc. headers that error > if the ISA is not enabled, so this will only affect people that use the > builtins directly. Yeah I agree. Okay for trunk, thanks! Segher > 2017-11-23 Jakub Jelinek <jakub@redhat.com> > > PR target/82848 > * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Don't fold > builtins not enabled in the currently selected ISA. (You forgot changelog for the added testcase).
--- gcc/config/rs6000/rs6000.c.jj 2017-11-23 10:29:01.000000000 +0100 +++ gcc/config/rs6000/rs6000.c 2017-11-23 10:30:54.930019590 +0100 @@ -16143,6 +16143,12 @@ rs6000_gimple_fold_builtin (gimple_stmt_ if (!gimple_call_lhs (stmt) && !rs6000_builtin_valid_without_lhs (fn_code)) return false; + /* Don't fold invalid builtins, let rs6000_expand_builtin diagnose it. */ + HOST_WIDE_INT mask = rs6000_builtin_info[uns_fncode].mask; + bool func_valid_p = (rs6000_builtin_mask & mask) == mask; + if (!func_valid_p) + return false; + switch (fn_code) { /* Flavors of vec_add. We deliberately don't expand --- gcc/testsuite/gcc.target/powerpc/pr82848.c.jj 2017-11-23 10:34:14.670471607 +0100 +++ gcc/testsuite/gcc.target/powerpc/pr82848.c 2017-11-23 10:36:22.754841465 +0100 @@ -0,0 +1,13 @@ +/* PR target/82848 */ +/* { dg-do compile } */ +/* { dg-options "-mno-altivec -mno-vsx -Wno-psabi" } */ + +#define C 3.68249351546114573519399405666776E-44f +#define vector __attribute__ ((altivec (vector__))) + +vector float +foo (vector float a) +{ + vector float b = __builtin_vec_madd (b, a, (vector float) { C, C, C, C }); /* { dg-error "requires the '-maltivec' option" } */ + return b; +}
Hi! With -mno-altivec -mno-vsx -mno-fold-gimple we error on __builtin_vec* builtins used when corresponding ISA is not enabled. When folding gimple, in some cases we get away with it (e.g. when folding the builtin to PLUS_EXPR on the generic vectors), because tree-vect-generic.c lowers those into scalar piecewise operations, but e.g. in case of FMA_EXPR tree-vect-generic.c isn't doing anything, expects that FMA_EXPR is actually used only if supported, I think similarly for various vector widening operations etc. One option is not to fold into gimple only those builtins where we know it will not work on generic vectors; my preference is to not fold any builtin which would be rejected at expansion time, so user get at least consistent diagnostics. Usually people are using the altivec.h etc. headers that error if the ISA is not enabled, so this will only affect people that use the builtins directly. Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk? 2017-11-23 Jakub Jelinek <jakub@redhat.com> PR target/82848 * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Don't fold builtins not enabled in the currently selected ISA. Jakub