| Message ID | 1515714296.8647.7.camel@cavium.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Aarch64,PR,target/79924] Cannot translate diagnostics | expand |
Ping. Steve Ellcey sellcey@cavium.com On Thu, 2018-01-11 at 15:44 -0800, Steve Ellcey wrote: > This is a patch for PR target/79924, which says the error messages > called from aarch64_err_no_fpadvsimd cannot be translated due to > how they are constructed. To make them translatable and not change > the actual messages would have required creating 16 individual > messages > which seemed a bit excessive so I simplified them a bit and fixed > up the test cases that scan for them. > > Instead of having 16 messages with: > > ['-mgeneral-regs-only'|'+nofp' feature modifier] is incompatible with > [vector|floating-point] [argument|return type|varargs|code] > > I changed it to four messages with > > [-mgeneral-regs-only|+nofp feature modifier] is incompatible with the > use of [vector|floating point] types > > The changes I made in the actual messages were: > > Remove the quotes from around '-mgeneral-regs-only' and '+nofp' > because error > messages I saw on other platforms did not use quotes around compiler > flags. > > Replace the specific [argument|return type|varargs|code] terms with > 'use of [vector|floating point] types. This got me from 16 to 4 > messages. I think that since the error message will point at the > actual > line where the problem is, having the message be a bit more generic > is > not a problem. Also I chose to include 'the use of' because having > these > types (and not doing anything with them) will not trigger one of > these > errors only using them in code (as an argument, return type, etc) > will > trigger the error. Also I got rid of the '-' in floating-point since > I didn't see any reason for it to be there. > > Tested with no regressions, OK to checkin? > > Steve Ellcey > sellcey@cavium.com > > > 2018-01-11 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): > Remove > second argument. > * config/aarch64/aarch64-protos..c (aarch64_err_no_fpadvsimd): > Remove second argument, change how error is called. > (aarch64_layout_arg): Remove second argument from > aarch64_err_no_fpadvsimd call. > (aarch64_init_cumulative_args): Ditto. > (aarch64_gimplify_va_arg_expr): Ditto. > * config/aarch64/aarch64.md (mov<mode>): Ditto. > > > 2018-01-11 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * gcc.target/aarch64/mgeneral-regs_1.c: Update error message. > * gcc.target/aarch64/mgeneral-regs_2.c: Ditto. > * gcc.target/aarch64/mgeneral-regs_3.c: Ditto. > * gcc.target/aarch64/nofp_1.c: Ditto.
Steve Ellcey <sellcey@cavium.com> writes: > @@ -1029,13 +1029,18 @@ aarch64_gen_far_branch (rtx * operands, int pos_label, const char * dest, > } > > void > -aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg) > +aarch64_err_no_fpadvsimd (machine_mode mode) > { > - const char *mc = FLOAT_MODE_P (mode) ? "floating-point" : "vector"; > if (TARGET_GENERAL_REGS_ONLY) > - error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg); > + if (FLOAT_MODE_P (mode)) > + error ("-mgeneral-regs-only is incompatible with the use of floating point types"); > + else > + error ("-mgeneral-regs-only is incompatible with the use of vector types"); > else > - error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg); > + if (FLOAT_MODE_P (mode)) > + error ("+nofp feature modifier is incompatible with the use of floating point types"); > + else > + error ("+nofp feature modifier is incompatible with the use of vector types"); This regresses a couple of things: - before the patch, the option would be properly quoted, whereas now it's unquoted. Either keeping %qs or using %<...%> would fix that. Using %qs is probably nicer since we can reuse the translation for any other options or features that end up being incompatible. - "floating-point" is preferred for modifiers over "floating point". - some lines are now longer than 80 chars. Patch LGTM otherwise, but someone else will need to approve. Thanks, Richard
On Tue, 2018-06-05 at 13:23 +0100, Richard Sandiford wrote: > > This regresses a couple of things: > > - before the patch, the option would be properly quoted, whereas now > it's unquoted. Either keeping %qs or using %<...%> would fix that. > Using %qs is probably nicer since we can reuse the translation for any > other options or features that end up being incompatible. > > - "floating-point" is preferred for modifiers over "floating point". > > - some lines are now longer than 80 chars. > > Patch LGTM otherwise, but someone else will need to approve. > > Thanks, > Richard Here is an updated version with the quotes put back (using %qs) and the long lines split up. Retested with no regressions. Steve Ellcey sellcey@cavium.com 2018-06-05 Steve Ellcey <sellcey@cavium.com> PR target/79924 * config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): Remove second argument. * config/aarch64/aarch64-protos..c (aarch64_err_no_fpadvsimd): Remove second argument, change how error is called. (aarch64_layout_arg): Remove second argument from aarch64_err_no_fpadvsimd call. (aarch64_init_cumulative_args): Ditto. (aarch64_gimplify_va_arg_expr): Ditto. * config/aarch64/aarch64.md (mov<mode>): Ditto. 2018-06-05 Steve Ellcey <sellcey@cavium.com> PR target/79924 * gcc.target/aarch64/mgeneral-regs_1.c: Update error message. * gcc.target/aarch64/mgeneral-regs_2.c: Ditto. * gcc.target/aarch64/mgeneral-regs_3.c: Ditto. * gcc.target/aarch64/nofp_1.c: Ditto. diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index 4ea50ac..87c6ae2 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -448,7 +448,7 @@ void aarch64_asm_output_labelref (FILE *, const char *); void aarch64_cpu_cpp_builtins (cpp_reader *); const char * aarch64_gen_far_branch (rtx *, int, const char *, const char *); const char * aarch64_output_probe_stack_range (rtx, rtx); -void aarch64_err_no_fpadvsimd (machine_mode, const char *); +void aarch64_err_no_fpadvsimd (machine_mode); void aarch64_expand_epilogue (bool); void aarch64_expand_mov_immediate (rtx, rtx, rtx (*) (rtx, rtx) = 0); void aarch64_emit_sve_pred_move (rtx, rtx, rtx); diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 98ef457..afc0fc6 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1077,13 +1077,22 @@ aarch64_gen_far_branch (rtx * operands, int pos_label, const char * dest, } void -aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg) +aarch64_err_no_fpadvsimd (machine_mode mode) { - const char *mc = FLOAT_MODE_P (mode) ? "floating-point" : "vector"; if (TARGET_GENERAL_REGS_ONLY) - error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg); + if (FLOAT_MODE_P (mode)) + error ("%qs is incompatible with the use of floating point types", + "-mgeneral-regs-only"); + else + error ("%qs is incompatible with the use of vector types", + "-mgeneral-regs-only"); else - error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg); + if (FLOAT_MODE_P (mode)) + error ("%qs feature modifier is incompatible with the use of" + " floating point types", "+nofp"); + else + error ("%qs feature modifier is incompatible with the use of" + " vector types", "+nofp"); } /* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. @@ -3519,7 +3528,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode, if (allocate_nvrn) { if (!TARGET_FLOAT) - aarch64_err_no_fpadvsimd (mode, "argument"); + aarch64_err_no_fpadvsimd (mode); if (nvrn + nregs <= NUM_FP_ARG_REGS) { @@ -3661,7 +3670,7 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum, int nregs ATTRIBUTE_UNUSED; /* Likewise. */ if (aarch64_vfp_is_call_or_return_candidate (TYPE_MODE (type), type, &mode, &nregs, NULL)) - aarch64_err_no_fpadvsimd (TYPE_MODE (type), "return type"); + aarch64_err_no_fpadvsimd (TYPE_MODE (type)); } return; } @@ -12254,7 +12263,7 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, /* TYPE passed in fp/simd registers. */ if (!TARGET_FLOAT) - aarch64_err_no_fpadvsimd (mode, "varargs"); + aarch64_err_no_fpadvsimd (mode); f_top = build3 (COMPONENT_REF, TREE_TYPE (f_vrtop), unshare_expr (valist), f_vrtop, NULL_TREE); diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 2539f2e..830f976 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -1149,7 +1149,7 @@ { if (!TARGET_FLOAT) { - aarch64_err_no_fpadvsimd (<MODE>mode, "code"); + aarch64_err_no_fpadvsimd (<MODE>mode); FAIL; } diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c index 1656db5..336402e 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c @@ -2,8 +2,7 @@ typedef int int32x2_t __attribute__ ((__vector_size__ ((8)))); -/* { dg-error "'-mgeneral-regs-only' is incompatible with vector return type" "" {target "aarch64*-*-*"} .+2 } */ -/* { dg-error "'-mgeneral-regs-only' is incompatible with vector argument" "" {target "aarch64*-*-*"} .+1 } */ +/* { dg-error "'-mgeneral-regs-only' is incompatible with the use of vector types" "" {target "aarch64*-*-*"} .+1 } */ int32x2_t test (int32x2_t a, int32x2_t b) { return a + b; diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c index 8590199..6e06a9f 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c @@ -10,6 +10,6 @@ test (int i, ...) va_list argp; va_start (argp, i); int32x2_t x = (int32x2_t) {0, 1}; - x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with vector varargs" } */ + x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with the use of vector types" } */ return x[0] + x[1]; } diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c index f6b5fba..bb772fb 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c @@ -5,7 +5,7 @@ extern void abort (void); int test (int i, ...) { - float f = (float) i; /* { dg-error "'-mgeneral-regs-only' is incompatible with floating-point code" } */ + float f = (float) i; /* { dg-error "'-mgeneral-regs-only' is incompatible with the use of floating point types" } */ if (f != f) abort (); return 2; } diff --git a/gcc/testsuite/gcc.target/aarch64/nofp_1.c b/gcc/testsuite/gcc.target/aarch64/nofp_1.c index 3fc0036..f8adc62 100644 --- a/gcc/testsuite/gcc.target/aarch64/nofp_1.c +++ b/gcc/testsuite/gcc.target/aarch64/nofp_1.c @@ -15,5 +15,5 @@ main (int argc, char **argv) { int32x2_t a = (int32x2_t) {0, 1}; int32x2_t b = (int32x2_t) {2, 3}; - return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with vector argument" } */ + return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with the use of vector types" } */ }
On Tue, Jun 05, 2018 at 12:40:01PM -0500, Steve Ellcey wrote: > On Tue, 2018-06-05 at 13:23 +0100, Richard Sandiford wrote: > > > > This regresses a couple of things: > > > > - before the patch, the option would be properly quoted, whereas now > > it's unquoted. Either keeping %qs or using %<...%> would fix that. > > Using %qs is probably nicer since we can reuse the translation for any > > other options or features that end up being incompatible. > > > > - "floating-point" is preferred for modifiers over "floating point". > > > > - some lines are now longer than 80 chars. > > > > Patch LGTM otherwise, but someone else will need to approve. > > > > Thanks, > > Richard > > Here is an updated version with the quotes put back (using %qs) and the > long lines split up. Retested with no regressions. OK. Thanks for sticking with it over a release cycle and several iterations. Thanks, James > 2018-06-05 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): Remove > second argument. > * config/aarch64/aarch64-protos..c (aarch64_err_no_fpadvsimd): > Remove second argument, change how error is called. > (aarch64_layout_arg): Remove second argument from > aarch64_err_no_fpadvsimd call. > (aarch64_init_cumulative_args): Ditto. > (aarch64_gimplify_va_arg_expr): Ditto. > * config/aarch64/aarch64.md (mov<mode>): Ditto. > > 2018-06-05 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * gcc.target/aarch64/mgeneral-regs_1.c: Update error message. > * gcc.target/aarch64/mgeneral-regs_2.c: Ditto. > * gcc.target/aarch64/mgeneral-regs_3.c: Ditto. > * gcc.target/aarch64/nofp_1.c: Ditto.
Steve Ellcey <sellcey@cavium.com> writes: > On Tue, 2018-06-05 at 13:23 +0100, Richard Sandiford wrote: >> >> This regresses a couple of things: >> >> - before the patch, the option would be properly quoted, whereas now >> it's unquoted. Either keeping %qs or using %<...%> would fix that. >> Using %qs is probably nicer since we can reuse the translation for any >> other options or features that end up being incompatible. >> >> - "floating-point" is preferred for modifiers over "floating point". >> >> - some lines are now longer than 80 chars. >> >> Patch LGTM otherwise, but someone else will need to approve. >> >> Thanks, >> Richard > > Here is an updated version with the quotes put back (using %qs) and the > long lines split up. Retested with no regressions. > > Steve Ellcey > sellcey@cavium.com > > > 2018-06-05 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * config/aarch64/aarch64-protos.h (aarch64_err_no_fpadvsimd): Remove > second argument. > * config/aarch64/aarch64-protos..c (aarch64_err_no_fpadvsimd): > Remove second argument, change how error is called. > (aarch64_layout_arg): Remove second argument from > aarch64_err_no_fpadvsimd call. > (aarch64_init_cumulative_args): Ditto. > (aarch64_gimplify_va_arg_expr): Ditto. > * config/aarch64/aarch64.md (mov<mode>): Ditto. > > 2018-06-05 Steve Ellcey <sellcey@cavium.com> > > PR target/79924 > * gcc.target/aarch64/mgeneral-regs_1.c: Update error message. > * gcc.target/aarch64/mgeneral-regs_2.c: Ditto. > * gcc.target/aarch64/mgeneral-regs_3.c: Ditto. > * gcc.target/aarch64/nofp_1.c: Ditto. > > diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h > index 4ea50ac..87c6ae2 100644 > --- a/gcc/config/aarch64/aarch64-protos.h > +++ b/gcc/config/aarch64/aarch64-protos.h > @@ -448,7 +448,7 @@ void aarch64_asm_output_labelref (FILE *, const char *); > void aarch64_cpu_cpp_builtins (cpp_reader *); > const char * aarch64_gen_far_branch (rtx *, int, const char *, const char *); > const char * aarch64_output_probe_stack_range (rtx, rtx); > -void aarch64_err_no_fpadvsimd (machine_mode, const char *); > +void aarch64_err_no_fpadvsimd (machine_mode); > void aarch64_expand_epilogue (bool); > void aarch64_expand_mov_immediate (rtx, rtx, rtx (*) (rtx, rtx) = 0); > void aarch64_emit_sve_pred_move (rtx, rtx, rtx); > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 98ef457..afc0fc6 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1077,13 +1077,22 @@ aarch64_gen_far_branch (rtx * operands, int pos_label, const char * dest, > } > > void > -aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg) > +aarch64_err_no_fpadvsimd (machine_mode mode) > { > - const char *mc = FLOAT_MODE_P (mode) ? "floating-point" : "vector"; > if (TARGET_GENERAL_REGS_ONLY) > - error ("%qs is incompatible with %s %s", "-mgeneral-regs-only", mc, msg); > + if (FLOAT_MODE_P (mode)) > + error ("%qs is incompatible with the use of floating point types", > + "-mgeneral-regs-only"); > + else > + error ("%qs is incompatible with the use of vector types", > + "-mgeneral-regs-only"); > else > - error ("%qs feature modifier is incompatible with %s %s", "+nofp", mc, msg); > + if (FLOAT_MODE_P (mode)) > + error ("%qs feature modifier is incompatible with the use of" > + " floating point types", "+nofp"); > + else > + error ("%qs feature modifier is incompatible with the use of" > + " vector types", "+nofp"); > } This still has the change from "floating-point" to "floating point", but "floating-point" preferred. Thanks, Richard
On Tue, 2018-06-05 at 20:18 +0100, Richard Sandiford wrote: > > This still has the change from "floating-point" to "floating point", > but "floating-point" preferred. > > Thanks, > Richard You are right, I missed that. I haven't checked in the patch yet though so I will fix that before doing the checkin. Steve Ellcey sellcey@cavium.com
diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c index 1656db5..9484c8a 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_1.c @@ -2,8 +2,7 @@ typedef int int32x2_t __attribute__ ((__vector_size__ ((8)))); -/* { dg-error "'-mgeneral-regs-only' is incompatible with vector return type" "" {target "aarch64*-*-*"} .+2 } */ -/* { dg-error "'-mgeneral-regs-only' is incompatible with vector argument" "" {target "aarch64*-*-*"} .+1 } */ +/* { dg-error "-mgeneral-regs-only is incompatible with the use of vector types" "" {target "aarch64*-*-*"} .+1 } */ int32x2_t test (int32x2_t a, int32x2_t b) { return a + b; diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c index 8590199..34b4d97 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_2.c @@ -10,6 +10,6 @@ test (int i, ...) va_list argp; va_start (argp, i); int32x2_t x = (int32x2_t) {0, 1}; - x += va_arg (argp, int32x2_t); /* { dg-error "'-mgeneral-regs-only' is incompatible with vector varargs" } */ + x += va_arg (argp, int32x2_t); /* { dg-error "-mgeneral-regs-only is incompatible with the use of vector types" } */ return x[0] + x[1]; } diff --git a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c index f6b5fba..84320fe 100644 --- a/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c +++ b/gcc/testsuite/gcc.target/aarch64/mgeneral-regs_3.c @@ -5,7 +5,7 @@ extern void abort (void); int test (int i, ...) { - float f = (float) i; /* { dg-error "'-mgeneral-regs-only' is incompatible with floating-point code" } */ + float f = (float) i; /* { dg-error "-mgeneral-regs-only is incompatible with the use of floating point types" } */ if (f != f) abort (); return 2; } diff --git a/gcc/testsuite/gcc.target/aarch64/nofp_1.c b/gcc/testsuite/gcc.target/aarch64/nofp_1.c index 3fc0036..c176c19 100644 --- a/gcc/testsuite/gcc.target/aarch64/nofp_1.c +++ b/gcc/testsuite/gcc.target/aarch64/nofp_1.c @@ -15,5 +15,5 @@ main (int argc, char **argv) { int32x2_t a = (int32x2_t) {0, 1}; int32x2_t b = (int32x2_t) {2, 3}; - return test (2, a, b); /* { dg-error "'\\+nofp' feature modifier is incompatible with vector argument" } */ + return test (2, a, b); /* { dg-error "\\+nofp feature modifier is incompatible with the use of vector types" } */ }