Message ID | 3090c1c4-41e1-d37b-c029-2775b2deb3d6@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000] Fix tests that are failing in gcc.target/powerpc/bfp with -m32 | expand |
Hi! On Fri, Apr 13, 2018 at 03:15:09PM -0500, Kelvin Nilsen wrote: > Twelve failures have been occuring in the bfp test directory during -m32 > regression testing. > This patch: > > 1. Changes the expected error messages in certain test programs. > > 2. Disables certain test programs from being exercised on 32-bit targets. > > 3. Adds a "note" error message to explain the mapping from overloaded > built-in functions > to non-overloaded built-in functions. Ah, very good plan! > @@ -6932,14 +6933,20 @@ > && rs6000_builtin_type_compatible (types[1], desc->op2)) > { > if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) > - return altivec_build_resolved_builtin (args, n, desc); > + { > + result = altivec_build_resolved_builtin (args, n, desc); > + /* overloaded_code is set above */ > + if (!rs6000_builtin_is_supported_p (overloaded_code)) > + unsupported_builtin = true; > + else > + return result; > + } > else > unsupported_builtin = true; > } > } The indentation (here and elsewhere) is off; probably because of the way you sent this? Please check. > + else > + /* on 64-bit, i seem to end up here */ > + error ("builtin function %qs not supported in this compiler " > + "configuration", name); I think you don't want that comment? > + /* If an error-representing result tree was returned from > + altivec_build_resolved_builtin above, use it. */ > + return (result != NULL)? result: error_mark_node; Spaces before ? and :. x != 0 as a condition is pretty silly btw, but that's just style ;-) Okay for trunk with those nits fixed. Thanks! Segher
Hi Segher, This patch, as revised in response to your suggestions, was committed to trunk on 4/17/2018. Is this ok for backporting to gcc8, gcc7, and gcc6? Thanks. On 4/13/18 3:15 PM, Kelvin Nilsen wrote: > Twelve failures have been occuring in the bfp test directory during -m32 > regression testing. > > The cause of these failures was two-fold: > > 1. Patches added subsequent to development of the tests caused new error > messages > to be emitted that are different than the error messages expected in the > dejagnu patterns. > These new patches also changed which built-in functions are legal when > compiling with the > -m32 command-line option. > > 2. The implementation of overloaded built-in functions maps overloaded > function names to > non-overloaded names. Depending on the stage at which an error is > recognized, error > messages may refer either to the overloaded built-in function name or > the non-overloaded > name. > > This patch: > > 1. Changes the expected error messages in certain test programs. > > 2. Disables certain test programs from being exercised on 32-bit targets. > > 3. Adds a "note" error message to explain the mapping from overloaded > built-in functions > to non-overloaded built-in functions. > > > This patch has bootstrapped and tested without regressions on both > powerpc64le-unknown-linux (P8) and on powerpc-linux (P7 big-endian, with > both -m32 > and -m64 target options). > > Is this ok for trunk? > > gcc/ChangeLog: > > 2018-04-13 Kelvin Nilsen <kelvin@gcc.gnu.org> > > * config/rs6000/rs6000-protos.h (rs6000_builtin_is_supported_p): > New prototype. > * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): > Add note to error message to explain internal mapping of overloaded > built-in function name to non-overloaded built-in function name. > * config/rs6000/rs6000.c (rs6000_builtin_is_supported_p): New > function. > > gcc/testsuite/ChangeLog: > > 2018-04-13 Kelvin Nilsen <kelvin@gcc.gnu.org> > > * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Simplify to > prevent cascading of errors and change expected error message. > * gcc.target/powerpc/bfp/scalar-test-neg-4.c: Restrict this test > to 64-bit targets. > * gcc.target/powerpc/bfp/scalar-test-data-class-8.c: Likewise. > * gcc.target/powerpc/bfp/scalar-test-data-class-9.c: Likewise. > * gcc.target/powerpc/bfp/scalar-test-data-class-10.c: Likewise. > * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Change expected > error message. > * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Likewise. > > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c > (working copy) > @@ -8,10 +8,10 @@ > error because the builtin requires 64 bits. */ > #include <altivec.h> > > -unsigned __int128 /* { dg-error "'__int128' is not supported on this > target" } */ > +unsigned long long int > get_significand (__ieee128 *p) > { > __ieee128 source = *p; > > - return __builtin_vec_scalar_extract_sig (source); /* { dg-error > "builtin function '__builtin_vec_scalar_extract_sig' not supported in > this compiler configuration" } */ > + return (long long int) __builtin_vec_scalar_extract_sig (source); /* > { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ > } > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-neg-4.c (working > copy) > @@ -1,5 +1,6 @@ > /* { dg-do compile { target { powerpc*-*-* } } } */ > /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } > { "-mcpu=power9" } } */ > +/* { dg-require-effective-target lp64 } */ > /* { dg-require-effective-target powerpc_p9vector_ok } */ > /* { dg-options "-mcpu=power9" } */ > > @@ -11,6 +12,8 @@ > { > __ieee128 source = *p; > > + /* IEEE 128-bit floating point operations are only supported > + on 64-bit targets. */ > return scalar_test_neg (source); > } > > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-8.c > (working copy) > @@ -1,5 +1,6 @@ > /* { dg-do compile { target { powerpc*-*-* } } } */ > /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } > { "-mcpu=power9" } } */ > +/* { dg-require-effective-target lp64 } */ > /* { dg-require-effective-target powerpc_p9vector_ok } */ > /* { dg-options "-mcpu=power9" } */ > > @@ -11,6 +12,8 @@ > { > __ieee128 source = *p; > > + /* IEEE 128-bit floating point operations are only supported > + on 64-bit targets. */ > return scalar_test_data_class (source, 3); > } > > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c > (working copy) > @@ -17,5 +17,5 @@ > __ieee128 significand = *significand_p; > unsigned long long int exponent = *exponent_p; > > - return scalar_insert_exp (significand, exponent); /* { dg-error > "builtin function '__builtin_vec_scalar_insert_exp' not supported in > this compiler configuration" } */ > + return scalar_insert_exp (significand, exponent); /* { dg-error > "requires ISA 3.0 IEEE 128-bit floating point" } */ > } > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-9.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-9.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-9.c > (working copy) > @@ -1,5 +1,6 @@ > /* { dg-do compile { target { powerpc*-*-* } } } */ > /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } > { "-mcpu=power9" } } */ > +/* { dg-require-effective-target lp64 } */ > /* { dg-require-effective-target powerpc_p9vector_ok } */ > /* { dg-options "-mcpu=power9" } */ > > @@ -11,6 +12,8 @@ > { > __ieee128 source = *p; > > + /* IEEE 128-bit floating point operations are only supported > + on 64-bit targets. */ > return scalar_test_data_class (source, 256); /* { dg-error > "argument 2 must be a 7-bit unsigned literal" } */ > } > > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c > (working copy) > @@ -15,7 +15,7 @@ > { > __ieee128 source = *p; > > - return scalar_extract_exp (source); /* { dg-error "builtin > function '__builtin_vec_scalar_extract_exp' not supported in this > compiler configuration" } */ > + return scalar_extract_exp (source); /* { dg-error "requires ISA > 3.0 IEEE 128-bit floating point" } */ > } > > > Index: gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-10.c > =================================================================== > --- gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-10.c > (revision 259316) > +++ gcc/testsuite/gcc.target/powerpc/bfp/scalar-test-data-class-10.c > (working copy) > @@ -1,5 +1,8 @@ > /* { dg-do compile { target { powerpc*-*-* } } } */ > /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } > { "-mcpu=power9" } } */ > +/* Require 64-bit target to select expected error message below. 32-bit > + target produces different error message. */ > +/* { dg-require-effective-target lp64 } */ > /* { dg-require-effective-target powerpc_p9vector_ok } */ > /* { dg-options "-mcpu=power9" } */ > > Index: gcc/config/rs6000/rs6000-protos.h > =================================================================== > --- gcc/config/rs6000/rs6000-protos.h (revision 259316) > +++ gcc/config/rs6000/rs6000-protos.h (working copy) > @@ -212,6 +212,7 @@ > extern void rs6000_aix_asm_output_dwarf_table_ref (char *); > extern void get_ppc476_thunk_name (char name[32]); > extern bool rs6000_overloaded_builtin_p (enum rs6000_builtins); > +extern bool rs6000_builtin_is_supported_p (enum rs6000_builtins); > extern const char *rs6000_overloaded_builtin_name (enum rs6000_builtins); > extern int rs6000_store_data_bypass_p (rtx_insn *, rtx_insn *); > extern HOST_WIDE_INT rs6000_builtin_mask_calculate (void); > Index: gcc/config/rs6000/rs6000-c.c > =================================================================== > --- gcc/config/rs6000/rs6000-c.c (revision 259316) > +++ gcc/config/rs6000/rs6000-c.c (working copy) > @@ -6885,6 +6885,8 @@ > > { > bool unsupported_builtin = false; > + enum rs6000_builtins overloaded_code; > + tree result = NULL; > for (desc = altivec_overloaded_builtins; > desc->code && desc->code != fcode; desc++) > continue; > @@ -6897,7 +6899,6 @@ > discrimination between the desired forms of the function. */ > if (fcode == P6_OV_BUILTIN_CMPB) > { > - int overloaded_code; > machine_mode arg1_mode = TYPE_MODE (types[0]); > machine_mode arg2_mode = TYPE_MODE (types[1]); > > @@ -6932,14 +6933,20 @@ > && rs6000_builtin_type_compatible (types[1], desc->op2)) > { > if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) > - return altivec_build_resolved_builtin (args, n, desc); > + { > + result = altivec_build_resolved_builtin (args, n, desc); > + /* overloaded_code is set above */ > + if (!rs6000_builtin_is_supported_p (overloaded_code)) > + unsupported_builtin = true; > + else > + return result; > + } > else > unsupported_builtin = true; > } > } > else if (fcode == P9V_BUILTIN_VEC_VSIEDP) > { > - int overloaded_code; > machine_mode arg1_mode = TYPE_MODE (types[0]); > > if (nargs != 2) > @@ -6974,12 +6981,20 @@ > while (desc->code && desc->code == fcode > && desc->overloaded_code != overloaded_code) > desc++; > + > if (desc->code && (desc->code == fcode) > && rs6000_builtin_type_compatible (types[0], desc->op1) > && rs6000_builtin_type_compatible (types[1], desc->op2)) > { > if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) > - return altivec_build_resolved_builtin (args, n, desc); > + { > + result = altivec_build_resolved_builtin (args, n, desc); > + /* overloaded_code is set above. */ > + if (!rs6000_builtin_is_supported_p (overloaded_code)) > + unsupported_builtin = true; > + else > + return result; > + } > else > unsupported_builtin = true; > } > @@ -6998,7 +7013,18 @@ > || rs6000_builtin_type_compatible (types[2], desc->op3))) > { > if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) > - return altivec_build_resolved_builtin (args, n, desc); > + { > + result = altivec_build_resolved_builtin (args, n, desc); > + if (!rs6000_builtin_is_supported_p (desc->overloaded_code)) > + { > + /* Allow loop to continue in case a different > + definition is supported. */ > + overloaded_code = desc->overloaded_code; > + unsupported_builtin = true; > + } > + else > + return result; > + } > else > unsupported_builtin = true; > } > @@ -7008,9 +7034,24 @@ > if (unsupported_builtin) > { > const char *name = rs6000_overloaded_builtin_name (fcode); > - error ("builtin function %qs not supported in this compiler " > - "configuration", name); > - return error_mark_node; > + if (result != NULL) > + { > + const char *internal_name > + = rs6000_overloaded_builtin_name (overloaded_code); > + /* An error message making reference to the name of the > + non-overloaded function has already been issued. Add > + clarification of the previous message. */ > + rich_location richloc (line_table, input_location); > + inform (&richloc, "builtin %qs requires builtin %qs", > + name, internal_name); > + } > + else > + /* on 64-bit, i seem to end up here */ > + error ("builtin function %qs not supported in this compiler " > + "configuration", name); > + /* If an error-representing result tree was returned from > + altivec_build_resolved_builtin above, use it. */ > + return (result != NULL)? result: error_mark_node; > } > } > bad: > Index: gcc/config/rs6000/rs6000.c > =================================================================== > --- gcc/config/rs6000/rs6000.c (revision 259316) > +++ gcc/config/rs6000/rs6000.c (working copy) > @@ -15897,6 +15897,18 @@ > return target; > } > > +/* Check whether a builtin function is supported in this target > + configuration. */ > +bool > +rs6000_builtin_is_supported_p (enum rs6000_builtins fncode) > +{ > + HOST_WIDE_INT fnmask = rs6000_builtin_info[fncode].mask; > + if ((fnmask & rs6000_builtin_mask) != fnmask) > + return false; > + else > + return true; > +} > + > /* Raise an error message for a builtin function that is called without the > appropriate target options being set. */ > > >
Hi! On Tue, Jun 26, 2018 at 02:56:40PM -0500, Kelvin Nilsen wrote: > This patch, as revised in response to your suggestions, was committed to trunk on 4/17/2018. > > Is this ok for backporting to gcc8, gcc7, and gcc6? Okay for all. Thanks! Segher > > 2018-04-13 Kelvin Nilsen <kelvin@gcc.gnu.org> > > > > * config/rs6000/rs6000-protos.h (rs6000_builtin_is_supported_p): > > New prototype. > > * config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): > > Add note to error message to explain internal mapping of overloaded > > built-in function name to non-overloaded built-in function name. > > * config/rs6000/rs6000.c (rs6000_builtin_is_supported_p): New > > function. > > > > gcc/testsuite/ChangeLog: > > > > 2018-04-13 Kelvin Nilsen <kelvin@gcc.gnu.org> > > > > * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Simplify to > > prevent cascading of errors and change expected error message. > > * gcc.target/powerpc/bfp/scalar-test-neg-4.c: Restrict this test > > to 64-bit targets. > > * gcc.target/powerpc/bfp/scalar-test-data-class-8.c: Likewise. > > * gcc.target/powerpc/bfp/scalar-test-data-class-9.c: Likewise. > > * gcc.target/powerpc/bfp/scalar-test-data-class-10.c: Likewise. > > * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Change expected > > error message. > > * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Likewise.
Index: gcc/config/rs6000/rs6000-protos.h =================================================================== --- gcc/config/rs6000/rs6000-protos.h (revision 259316) +++ gcc/config/rs6000/rs6000-protos.h (working copy) @@ -212,6 +212,7 @@ extern void rs6000_aix_asm_output_dwarf_table_ref (char *); extern void get_ppc476_thunk_name (char name[32]); extern bool rs6000_overloaded_builtin_p (enum rs6000_builtins); +extern bool rs6000_builtin_is_supported_p (enum rs6000_builtins); extern const char *rs6000_overloaded_builtin_name (enum rs6000_builtins); extern int rs6000_store_data_bypass_p (rtx_insn *, rtx_insn *); extern HOST_WIDE_INT rs6000_builtin_mask_calculate (void); Index: gcc/config/rs6000/rs6000-c.c =================================================================== --- gcc/config/rs6000/rs6000-c.c (revision 259316) +++ gcc/config/rs6000/rs6000-c.c (working copy) @@ -6885,6 +6885,8 @@ { bool unsupported_builtin = false; + enum rs6000_builtins overloaded_code; + tree result = NULL; for (desc = altivec_overloaded_builtins; desc->code && desc->code != fcode; desc++) continue; @@ -6897,7 +6899,6 @@ discrimination between the desired forms of the function. */ if (fcode == P6_OV_BUILTIN_CMPB) { - int overloaded_code; machine_mode arg1_mode = TYPE_MODE (types[0]); machine_mode arg2_mode = TYPE_MODE (types[1]); @@ -6932,14 +6933,20 @@ && rs6000_builtin_type_compatible (types[1], desc->op2)) { if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) - return altivec_build_resolved_builtin (args, n, desc); + { + result = altivec_build_resolved_builtin (args, n, desc); + /* overloaded_code is set above */ + if (!rs6000_builtin_is_supported_p (overloaded_code)) + unsupported_builtin = true; + else + return result; + } else unsupported_builtin = true; } } else if (fcode == P9V_BUILTIN_VEC_VSIEDP) { - int overloaded_code; machine_mode arg1_mode = TYPE_MODE (types[0]); if (nargs != 2) @@ -6974,12 +6981,20 @@ while (desc->code && desc->code == fcode && desc->overloaded_code != overloaded_code) desc++; + if (desc->code && (desc->code == fcode) && rs6000_builtin_type_compatible (types[0], desc->op1) && rs6000_builtin_type_compatible (types[1], desc->op2)) { if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) - return altivec_build_resolved_builtin (args, n, desc); + { + result = altivec_build_resolved_builtin (args, n, desc); + /* overloaded_code is set above. */ + if (!rs6000_builtin_is_supported_p (overloaded_code)) + unsupported_builtin = true; + else + return result; + } else unsupported_builtin = true; } @@ -6998,7 +7013,18 @@ || rs6000_builtin_type_compatible (types[2], desc->op3))) { if (rs6000_builtin_decls[desc->overloaded_code] != NULL_TREE) - return altivec_build_resolved_builtin (args, n, desc); + { + result = altivec_build_resolved_builtin (args, n, desc); + if (!rs6000_builtin_is_supported_p (desc->overloaded_code)) + { + /* Allow loop to continue in case a different + definition is supported. */ + overloaded_code = desc->overloaded_code; + unsupported_builtin = true; + } + else + return result; + } else unsupported_builtin = true; } @@ -7008,9 +7034,24 @@ if (unsupported_builtin) { const char *name = rs6000_overloaded_builtin_name (fcode); - error ("builtin function %qs not supported in this compiler " - "configuration", name); - return error_mark_node; + if (result != NULL) + { + const char *internal_name + = rs6000_overloaded_builtin_name (overloaded_code); + /* An error message making reference to the name of the + non-overloaded function has already been issued. Add + clarification of the previous message. */ + rich_location richloc (line_table, input_location); + inform (&richloc, "builtin %qs requires builtin %qs", + name, internal_name); + } + else + /* on 64-bit, i seem to end up here */ + error ("builtin function %qs not supported in this compiler " + "configuration", name); + /* If an error-representing result tree was returned from + altivec_build_resolved_builtin above, use it. */ + return (result != NULL)? result: error_mark_node; } } bad: Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 259316) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -15897,6 +15897,18 @@ return target; } +/* Check whether a builtin function is supported in this target + configuration. */ +bool +rs6000_builtin_is_supported_p (enum rs6000_builtins fncode) +{ + HOST_WIDE_INT fnmask = rs6000_builtin_info[fncode].mask; + if ((fnmask & rs6000_builtin_mask) != fnmask) + return false; + else + return true; +} + /* Raise an error message for a builtin function that is called without the appropriate target options being set. */