* [PR84682] disregard address constraints on non-addresses @ 2018-03-09 6:45 Alexandre Oliva 2018-03-09 14:50 ` Vladimir Makarov ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Alexandre Oliva @ 2018-03-09 6:45 UTC (permalink / raw) To: gcc-patches LRA gets very confused when non-addresses are passed as operands to asms with address contraints. Even if other constraints are available, and the operand is a perfect fit for them, we'd still attempt to process the operand as an address, and fail miserably at that. Truth is, address constraints expect operands allowed by address_operand, and we make sure this is the case throughout the compiler, even in asm statements. The problem was that, if multiple constraints were available, we wouldn't insist that the operand be allowed by address_operand, but we would proceed as if it was, regardless of any other constraints. To address this problem, I've arranged for LRA to attempt to deal with address-constrained operands as addresses only when the is_address flag is set, and to not set this flag in preprocess_constraints for asm operands that are not allowed by address_operand. Regstrapped on i686- and x86_64-linux-gnu. Ok to install? for gcc/ChangeLog PR rtl-optimization/84682 * lra-constraints.c (process_address_1): Check is_address flag for address constraints. (process_alt_operands): Likewise. * lra.c (lra_set_insn_recog_data): Pass asm operand locs to preprocess_constraints. * recog.h (preprocess_constraints): Add oploc parameter. Adjust callers. * recog.c (preprocess_constraints): Test address_operand for CT_ADDRESS constraints. for gcc/testsuite/ChangeLog PR rtl-optimization/84682 * gcc.dg/torture/pr84682-1.c: New. * gcc.dg/torture/pr84682-2.c: New. * gcc.dg/torture/pr84682-3.c: New. --- gcc/lra-constraints.c | 15 ++++++++++++++- gcc/lra.c | 3 ++- gcc/recog.c | 24 +++++++++++++++++------- gcc/recog.h | 2 +- gcc/testsuite/gcc.dg/torture/pr84682-1.c | 5 +++++ gcc/testsuite/gcc.dg/torture/pr84682-2.c | 10 ++++++++++ gcc/testsuite/gcc.dg/torture/pr84682-3.c | 8 ++++++++ 7 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-1.c create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-2.c create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-3.c diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index 59b97540d98f..ae55c86f1777 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -2276,6 +2276,12 @@ process_alt_operands (int only_alternative) break; case CT_ADDRESS: + /* An asm operand with an address constraint + that doesn't satisfy address_operand has + is_address cleared, so that we don't try to + make a non-address fit. */ + if (!curr_static_id->operand[nop].is_address) + break; /* If we didn't already win, we can reload the address into a base register. */ if (satisfies_address_constraint_p (op, cn)) @@ -3236,7 +3242,14 @@ process_address_1 (int nop, bool check_only_p, && GET_CODE (XEXP (op, 0)) == SCRATCH) return false; - if (insn_extra_address_constraint (cn)) + if (insn_extra_address_constraint (cn) + /* When we find an asm operand with an address constraint that + doesn't satisfy address_operand to begin with, we clear + is_address, so that we don't try to make a non-address fit. + If the asm statement got this far, it's because other + constraints are available, and we'll use them, disregarding + the unsatisfiable address ones. */ + && curr_static_id->operand[nop].is_address) decompose_lea_address (&ad, curr_id->operand_loc[nop]); /* Do not attempt to decompose arbitrary addresses generated by combine for asm operands with loose constraints, e.g 'X'. */ diff --git a/gcc/lra.c b/gcc/lra.c index a64d8f1a3011..0a251144026c 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -1039,7 +1039,8 @@ lra_set_insn_recog_data (rtx_insn *insn) { operand_alternative *op_alt = XCNEWVEC (operand_alternative, nalt * nop); - preprocess_constraints (nop, nalt, constraints, op_alt); + preprocess_constraints (nop, nalt, constraints, op_alt, + data->operand_loc); setup_operand_alternative (data, op_alt); } } diff --git a/gcc/recog.c b/gcc/recog.c index af6a6b01d88c..fa8e261d1476 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -2331,15 +2331,20 @@ extract_insn (rtx_insn *insn) which_alternative = -1; } -/* Fill in OP_ALT_BASE for an instruction that has N_OPERANDS operands, - N_ALTERNATIVES alternatives and constraint strings CONSTRAINTS. - OP_ALT_BASE has N_ALTERNATIVES * N_OPERANDS entries and CONSTRAINTS - has N_OPERANDS entries. */ +/* Fill in OP_ALT_BASE for an instruction that has N_OPERANDS + operands, N_ALTERNATIVES alternatives and constraint strings + CONSTRAINTS. OP_ALT_BASE has N_ALTERNATIVES * N_OPERANDS entries + and CONSTRAINTS has N_OPERANDS entries. OPLOC should be passed in + if the insn is an asm statement and preprocessing should take the + asm operands into account, e.g. to determine whether they could be + addresses in constraints that require addresses; it should then + point to an array of pointers to each operand. */ void preprocess_constraints (int n_operands, int n_alternatives, const char **constraints, - operand_alternative *op_alt_base) + operand_alternative *op_alt_base, + rtx **oploc) { for (int i = 0; i < n_operands; i++) { @@ -2426,6 +2431,9 @@ preprocess_constraints (int n_operands, int n_alternatives, break; case CT_ADDRESS: + if (oploc && !address_operand (*oploc[i], VOIDmode)) + break; + op_alt[i].is_address = 1; op_alt[i].cl = (reg_class_subunion @@ -2470,7 +2478,8 @@ preprocess_insn_constraints (unsigned int icode) for (int i = 0; i < n_operands; ++i) constraints[i] = insn_data[icode].operand[i].constraint; - preprocess_constraints (n_operands, n_alternatives, constraints, op_alt); + preprocess_constraints (n_operands, n_alternatives, constraints, op_alt, + NULL); this_target_recog->x_op_alt[icode] = op_alt; return op_alt; @@ -2493,7 +2502,8 @@ preprocess_constraints (rtx_insn *insn) int n_entries = n_operands * n_alternatives; memset (asm_op_alt, 0, n_entries * sizeof (operand_alternative)); preprocess_constraints (n_operands, n_alternatives, - recog_data.constraints, asm_op_alt); + recog_data.constraints, asm_op_alt, + NULL); recog_op_alt = asm_op_alt; } } diff --git a/gcc/recog.h b/gcc/recog.h index 4a47ff23ca95..eca62803458c 100644 --- a/gcc/recog.h +++ b/gcc/recog.h @@ -136,7 +136,7 @@ extern void extract_constrain_insn (rtx_insn *insn); extern void extract_constrain_insn_cached (rtx_insn *); extern void extract_insn_cached (rtx_insn *); extern void preprocess_constraints (int, int, const char **, - operand_alternative *); + operand_alternative *, rtx **); extern const operand_alternative *preprocess_insn_constraints (unsigned int); extern void preprocess_constraints (rtx_insn *); extern rtx_insn *peep2_next_insn (int); diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-1.c b/gcc/testsuite/gcc.dg/torture/pr84682-1.c new file mode 100644 index 000000000000..b189ed78cdc3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr84682-1.c @@ -0,0 +1,5 @@ +/* { dg-do compile } */ + +void b(char a) { + asm("" : : "pir" (a)); +} diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-2.c b/gcc/testsuite/gcc.dg/torture/pr84682-2.c new file mode 100644 index 000000000000..5abda5fd136c --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr84682-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ + +int a; +void b() { + float c; + for (int d; d;) + ; + a = c; + asm("" : : "pir"(c)); +} diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-3.c b/gcc/testsuite/gcc.dg/torture/pr84682-3.c new file mode 100644 index 000000000000..543a307d6c12 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr84682-3.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* This is like pr84682-1.c, but with an extra memory constraint, to + check that we don't disable process_address altogether just because + of the disabled address constraint. */ + +void b(char a) { + asm("" : : "pmir" (a)); +} -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PR84682] disregard address constraints on non-addresses 2018-03-09 6:45 [PR84682] disregard address constraints on non-addresses Alexandre Oliva @ 2018-03-09 14:50 ` Vladimir Makarov 2018-03-09 18:51 ` Jeff Law 2018-03-12 10:14 ` Bin.Cheng 2 siblings, 0 replies; 6+ messages in thread From: Vladimir Makarov @ 2018-03-09 14:50 UTC (permalink / raw) To: Alexandre Oliva, gcc-patches On 03/09/2018 01:45 AM, Alexandre Oliva wrote: > LRA gets very confused when non-addresses are passed as operands to > asms with address contraints. Even if other constraints are > available, and the operand is a perfect fit for them, we'd still > attempt to process the operand as an address, and fail miserably at > that. > > Truth is, address constraints expect operands allowed by > address_operand, and we make sure this is the case throughout the > compiler, even in asm statements. The problem was that, if multiple > constraints were available, we wouldn't insist that the operand be > allowed by address_operand, but we would proceed as if it was, > regardless of any other constraints. > > To address this problem, I've arranged for LRA to attempt to deal with > address-constrained operands as addresses only when the is_address > flag is set, and to not set this flag in preprocess_constraints for > asm operands that are not allowed by address_operand. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? It looks ok for me. Thank you, Alex. > for gcc/ChangeLog > > PR rtl-optimization/84682 > * lra-constraints.c (process_address_1): Check is_address flag > for address constraints. > (process_alt_operands): Likewise. > * lra.c (lra_set_insn_recog_data): Pass asm operand locs to > preprocess_constraints. > * recog.h (preprocess_constraints): Add oploc parameter. > Adjust callers. > * recog.c (preprocess_constraints): Test address_operand for > CT_ADDRESS constraints. > > for gcc/testsuite/ChangeLog > > PR rtl-optimization/84682 > * gcc.dg/torture/pr84682-1.c: New. > * gcc.dg/torture/pr84682-2.c: New. > * gcc.dg/torture/pr84682-3.c: New. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PR84682] disregard address constraints on non-addresses 2018-03-09 6:45 [PR84682] disregard address constraints on non-addresses Alexandre Oliva 2018-03-09 14:50 ` Vladimir Makarov @ 2018-03-09 18:51 ` Jeff Law 2018-03-12 10:14 ` Bin.Cheng 2 siblings, 0 replies; 6+ messages in thread From: Jeff Law @ 2018-03-09 18:51 UTC (permalink / raw) To: Alexandre Oliva, gcc-patches On 03/08/2018 11:45 PM, Alexandre Oliva wrote: > LRA gets very confused when non-addresses are passed as operands to > asms with address contraints. Even if other constraints are > available, and the operand is a perfect fit for them, we'd still > attempt to process the operand as an address, and fail miserably at > that. > > Truth is, address constraints expect operands allowed by > address_operand, and we make sure this is the case throughout the > compiler, even in asm statements. The problem was that, if multiple > constraints were available, we wouldn't insist that the operand be > allowed by address_operand, but we would proceed as if it was, > regardless of any other constraints. > > To address this problem, I've arranged for LRA to attempt to deal with > address-constrained operands as addresses only when the is_address > flag is set, and to not set this flag in preprocess_constraints for > asm operands that are not allowed by address_operand. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? > > for gcc/ChangeLog > > PR rtl-optimization/84682 > * lra-constraints.c (process_address_1): Check is_address flag > for address constraints. > (process_alt_operands): Likewise. > * lra.c (lra_set_insn_recog_data): Pass asm operand locs to > preprocess_constraints. > * recog.h (preprocess_constraints): Add oploc parameter. > Adjust callers. > * recog.c (preprocess_constraints): Test address_operand for > CT_ADDRESS constraints. > > for gcc/testsuite/ChangeLog > > PR rtl-optimization/84682 > * gcc.dg/torture/pr84682-1.c: New. > * gcc.dg/torture/pr84682-2.c: New. > * gcc.dg/torture/pr84682-3.c: New. I went ahead and committed this to the trunk. jeff ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PR84682] disregard address constraints on non-addresses 2018-03-09 6:45 [PR84682] disregard address constraints on non-addresses Alexandre Oliva 2018-03-09 14:50 ` Vladimir Makarov 2018-03-09 18:51 ` Jeff Law @ 2018-03-12 10:14 ` Bin.Cheng 2018-03-14 8:32 ` Alexandre Oliva 2 siblings, 1 reply; 6+ messages in thread From: Bin.Cheng @ 2018-03-12 10:14 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches List On Fri, Mar 9, 2018 at 6:45 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > LRA gets very confused when non-addresses are passed as operands to > asms with address contraints. Even if other constraints are > available, and the operand is a perfect fit for them, we'd still > attempt to process the operand as an address, and fail miserably at > that. > > Truth is, address constraints expect operands allowed by > address_operand, and we make sure this is the case throughout the > compiler, even in asm statements. The problem was that, if multiple > constraints were available, we wouldn't insist that the operand be > allowed by address_operand, but we would proceed as if it was, > regardless of any other constraints. > > To address this problem, I've arranged for LRA to attempt to deal with > address-constrained operands as addresses only when the is_address > flag is set, and to not set this flag in preprocess_constraints for > asm operands that are not allowed by address_operand. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? > > for gcc/ChangeLog > > PR rtl-optimization/84682 > * lra-constraints.c (process_address_1): Check is_address flag > for address constraints. > (process_alt_operands): Likewise. > * lra.c (lra_set_insn_recog_data): Pass asm operand locs to > preprocess_constraints. > * recog.h (preprocess_constraints): Add oploc parameter. > Adjust callers. > * recog.c (preprocess_constraints): Test address_operand for > CT_ADDRESS constraints. > > for gcc/testsuite/ChangeLog > > PR rtl-optimization/84682 > * gcc.dg/torture/pr84682-1.c: New. > * gcc.dg/torture/pr84682-2.c: New. Hi Alexandre, This test failed because of ICE on various aarch64 targets with below error message: Executing on host: /.../build/build-aarch64-none-elf/obj/gcc2/gcc/xgcc -B/.../build/build-aarch64-none-elf/obj/gcc2/gcc/ /.../build/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -S -specs=aem-ve.specs -mcmodel=small -o pr84682-2.s (timeout = 300) spawn /.../build/build-aarch64-none-elf/obj/gcc2/gcc/xgcc -B/.../build/build-aarch64-none-elf/obj/gcc2/gcc/ /.../build/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -S -specs=aem-ve.specs -mcmodel=small -o pr84682-2.s during RTL pass: expand /.../build/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c: In function 'b': /.../build/src/gcc/gcc/testsuite/gcc.dg/torture/pr84682-2.c:9:3: internal compiler error: in aarch64_classify_address, at config/aarch64/aarch64.c:5678 0xfe3c29 aarch64_classify_address /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5677 0xfe8be8 aarch64_legitimate_address_hook_p /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5958 0xc0149e default_addr_space_legitimate_address_p(machine_mode, rtx_def*, bool, unsigned char) /.../build/src/gcc/gcc/targhooks.c:1476 0xb5b9f1 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char) /.../build/src/gcc/gcc/recog.c:1334 0xb5d278 address_operand(rtx_def*, machine_mode) /.../build/src/gcc/gcc/recog.c:1073 0xb5e186 asm_operand_ok(rtx_def*, char const*, char const**) /.../build/src/gcc/gcc/recog.c:1816 0x73f440 expand_asm_stmt /.../build/src/gcc/gcc/cfgexpand.c:3138 0x742d3c expand_gimple_stmt_1 /.../build/src/gcc/gcc/cfgexpand.c:3621 0x742d3c expand_gimple_stmt /.../build/src/gcc/gcc/cfgexpand.c:3790 0x745c15 expand_gimple_basic_block /.../build/src/gcc/gcc/cfgexpand.c:5819 0x749c2f execute /.../build/src/gcc/gcc/cfgexpand.c:6425 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. Not sure if it reveals latent bug or just inconsistent issue with backend though. Thanks, bin > * gcc.dg/torture/pr84682-3.c: New. > --- > gcc/lra-constraints.c | 15 ++++++++++++++- > gcc/lra.c | 3 ++- > gcc/recog.c | 24 +++++++++++++++++------- > gcc/recog.h | 2 +- > gcc/testsuite/gcc.dg/torture/pr84682-1.c | 5 +++++ > gcc/testsuite/gcc.dg/torture/pr84682-2.c | 10 ++++++++++ > gcc/testsuite/gcc.dg/torture/pr84682-3.c | 8 ++++++++ > 7 files changed, 57 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-1.c > create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-2.c > create mode 100644 gcc/testsuite/gcc.dg/torture/pr84682-3.c > > diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c > index 59b97540d98f..ae55c86f1777 100644 > --- a/gcc/lra-constraints.c > +++ b/gcc/lra-constraints.c > @@ -2276,6 +2276,12 @@ process_alt_operands (int only_alternative) > break; > > case CT_ADDRESS: > + /* An asm operand with an address constraint > + that doesn't satisfy address_operand has > + is_address cleared, so that we don't try to > + make a non-address fit. */ > + if (!curr_static_id->operand[nop].is_address) > + break; > /* If we didn't already win, we can reload the address > into a base register. */ > if (satisfies_address_constraint_p (op, cn)) > @@ -3236,7 +3242,14 @@ process_address_1 (int nop, bool check_only_p, > && GET_CODE (XEXP (op, 0)) == SCRATCH) > return false; > > - if (insn_extra_address_constraint (cn)) > + if (insn_extra_address_constraint (cn) > + /* When we find an asm operand with an address constraint that > + doesn't satisfy address_operand to begin with, we clear > + is_address, so that we don't try to make a non-address fit. > + If the asm statement got this far, it's because other > + constraints are available, and we'll use them, disregarding > + the unsatisfiable address ones. */ > + && curr_static_id->operand[nop].is_address) > decompose_lea_address (&ad, curr_id->operand_loc[nop]); > /* Do not attempt to decompose arbitrary addresses generated by combine > for asm operands with loose constraints, e.g 'X'. */ > diff --git a/gcc/lra.c b/gcc/lra.c > index a64d8f1a3011..0a251144026c 100644 > --- a/gcc/lra.c > +++ b/gcc/lra.c > @@ -1039,7 +1039,8 @@ lra_set_insn_recog_data (rtx_insn *insn) > { > operand_alternative *op_alt = XCNEWVEC (operand_alternative, > nalt * nop); > - preprocess_constraints (nop, nalt, constraints, op_alt); > + preprocess_constraints (nop, nalt, constraints, op_alt, > + data->operand_loc); > setup_operand_alternative (data, op_alt); > } > } > diff --git a/gcc/recog.c b/gcc/recog.c > index af6a6b01d88c..fa8e261d1476 100644 > --- a/gcc/recog.c > +++ b/gcc/recog.c > @@ -2331,15 +2331,20 @@ extract_insn (rtx_insn *insn) > which_alternative = -1; > } > > -/* Fill in OP_ALT_BASE for an instruction that has N_OPERANDS operands, > - N_ALTERNATIVES alternatives and constraint strings CONSTRAINTS. > - OP_ALT_BASE has N_ALTERNATIVES * N_OPERANDS entries and CONSTRAINTS > - has N_OPERANDS entries. */ > +/* Fill in OP_ALT_BASE for an instruction that has N_OPERANDS > + operands, N_ALTERNATIVES alternatives and constraint strings > + CONSTRAINTS. OP_ALT_BASE has N_ALTERNATIVES * N_OPERANDS entries > + and CONSTRAINTS has N_OPERANDS entries. OPLOC should be passed in > + if the insn is an asm statement and preprocessing should take the > + asm operands into account, e.g. to determine whether they could be > + addresses in constraints that require addresses; it should then > + point to an array of pointers to each operand. */ > > void > preprocess_constraints (int n_operands, int n_alternatives, > const char **constraints, > - operand_alternative *op_alt_base) > + operand_alternative *op_alt_base, > + rtx **oploc) > { > for (int i = 0; i < n_operands; i++) > { > @@ -2426,6 +2431,9 @@ preprocess_constraints (int n_operands, int n_alternatives, > break; > > case CT_ADDRESS: > + if (oploc && !address_operand (*oploc[i], VOIDmode)) > + break; > + > op_alt[i].is_address = 1; > op_alt[i].cl > = (reg_class_subunion > @@ -2470,7 +2478,8 @@ preprocess_insn_constraints (unsigned int icode) > > for (int i = 0; i < n_operands; ++i) > constraints[i] = insn_data[icode].operand[i].constraint; > - preprocess_constraints (n_operands, n_alternatives, constraints, op_alt); > + preprocess_constraints (n_operands, n_alternatives, constraints, op_alt, > + NULL); > > this_target_recog->x_op_alt[icode] = op_alt; > return op_alt; > @@ -2493,7 +2502,8 @@ preprocess_constraints (rtx_insn *insn) > int n_entries = n_operands * n_alternatives; > memset (asm_op_alt, 0, n_entries * sizeof (operand_alternative)); > preprocess_constraints (n_operands, n_alternatives, > - recog_data.constraints, asm_op_alt); > + recog_data.constraints, asm_op_alt, > + NULL); > recog_op_alt = asm_op_alt; > } > } > diff --git a/gcc/recog.h b/gcc/recog.h > index 4a47ff23ca95..eca62803458c 100644 > --- a/gcc/recog.h > +++ b/gcc/recog.h > @@ -136,7 +136,7 @@ extern void extract_constrain_insn (rtx_insn *insn); > extern void extract_constrain_insn_cached (rtx_insn *); > extern void extract_insn_cached (rtx_insn *); > extern void preprocess_constraints (int, int, const char **, > - operand_alternative *); > + operand_alternative *, rtx **); > extern const operand_alternative *preprocess_insn_constraints (unsigned int); > extern void preprocess_constraints (rtx_insn *); > extern rtx_insn *peep2_next_insn (int); > diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-1.c b/gcc/testsuite/gcc.dg/torture/pr84682-1.c > new file mode 100644 > index 000000000000..b189ed78cdc3 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr84682-1.c > @@ -0,0 +1,5 @@ > +/* { dg-do compile } */ > + > +void b(char a) { > + asm("" : : "pir" (a)); > +} > diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-2.c b/gcc/testsuite/gcc.dg/torture/pr84682-2.c > new file mode 100644 > index 000000000000..5abda5fd136c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr84682-2.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > + > +int a; > +void b() { > + float c; > + for (int d; d;) > + ; > + a = c; > + asm("" : : "pir"(c)); > +} > diff --git a/gcc/testsuite/gcc.dg/torture/pr84682-3.c b/gcc/testsuite/gcc.dg/torture/pr84682-3.c > new file mode 100644 > index 000000000000..543a307d6c12 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr84682-3.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile } */ > +/* This is like pr84682-1.c, but with an extra memory constraint, to > + check that we don't disable process_address altogether just because > + of the disabled address constraint. */ > + > +void b(char a) { > + asm("" : : "pmir" (a)); > +} > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PR84682] disregard address constraints on non-addresses 2018-03-12 10:14 ` Bin.Cheng @ 2018-03-14 8:32 ` Alexandre Oliva 2018-03-14 11:42 ` Bin.Cheng 0 siblings, 1 reply; 6+ messages in thread From: Alexandre Oliva @ 2018-03-14 8:32 UTC (permalink / raw) To: Bin.Cheng; +Cc: gcc-patches List On Mar 12, 2018, "Bin.Cheng" <amker.cheng@gmail.com> wrote: > internal compiler error: in aarch64_classify_address, at > config/aarch64/aarch64.c:5678 > 0xfe3c29 aarch64_classify_address > /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5677 > 0xfe8be8 aarch64_legitimate_address_hook_p > /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5958 > 0xc0149e default_addr_space_legitimate_address_p(machine_mode, > rtx_def*, bool, unsigned char) > /.../build/src/gcc/gcc/targhooks.c:1476 > 0xb5b9f1 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char) > /.../build/src/gcc/gcc/recog.c:1334 > 0xb5d278 address_operand(rtx_def*, machine_mode) > /.../build/src/gcc/gcc/recog.c:1073 > 0xb5e186 asm_operand_ok(rtx_def*, char const*, char const**) > /.../build/src/gcc/gcc/recog.c:1816 > 0x73f440 expand_asm_stmt > /.../build/src/gcc/gcc/cfgexpand.c:3138 > 0x742d3c expand_gimple_stmt_1 > /.../build/src/gcc/gcc/cfgexpand.c:3621 > Not sure if it reveals latent bug or just inconsistent issue with > backend though. Possibly both, in a way. This triggers at expand, which is much earlier than the patch affects compilation; that call to address_operand was already there before. So the ICE was latent, but only in the sense that we didn't have tests that would have triggered it. Any asm stmt with a p constraint and a non-address operand (with another constraint to accept it) would have exercised this aarch64-specific ICE, and it would do so before hitting the machine-independent ICE that the patch fixed. aarch64_classify_address should probably return false instead of failing that assertion. When it comes to asm operands, there's no guarantee that you'll get something that even looks like an address, when you're part of the very code that's supposed to tell the rest of the compiler what a legitimate address should look like. -- Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PR84682] disregard address constraints on non-addresses 2018-03-14 8:32 ` Alexandre Oliva @ 2018-03-14 11:42 ` Bin.Cheng 0 siblings, 0 replies; 6+ messages in thread From: Bin.Cheng @ 2018-03-14 11:42 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches List On Wed, Mar 14, 2018 at 8:03 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Mar 12, 2018, "Bin.Cheng" <amker.cheng@gmail.com> wrote: > >> internal compiler error: in aarch64_classify_address, at >> config/aarch64/aarch64.c:5678 >> 0xfe3c29 aarch64_classify_address >> /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5677 >> 0xfe8be8 aarch64_legitimate_address_hook_p >> /.../build/src/gcc/gcc/config/aarch64/aarch64.c:5958 >> 0xc0149e default_addr_space_legitimate_address_p(machine_mode, >> rtx_def*, bool, unsigned char) >> /.../build/src/gcc/gcc/targhooks.c:1476 >> 0xb5b9f1 memory_address_addr_space_p(machine_mode, rtx_def*, unsigned char) >> /.../build/src/gcc/gcc/recog.c:1334 >> 0xb5d278 address_operand(rtx_def*, machine_mode) >> /.../build/src/gcc/gcc/recog.c:1073 >> 0xb5e186 asm_operand_ok(rtx_def*, char const*, char const**) >> /.../build/src/gcc/gcc/recog.c:1816 >> 0x73f440 expand_asm_stmt >> /.../build/src/gcc/gcc/cfgexpand.c:3138 >> 0x742d3c expand_gimple_stmt_1 >> /.../build/src/gcc/gcc/cfgexpand.c:3621 > >> Not sure if it reveals latent bug or just inconsistent issue with >> backend though. > > Possibly both, in a way. This triggers at expand, which is much earlier > than the patch affects compilation; that call to address_operand was > already there before. So the ICE was latent, but only in the sense that > we didn't have tests that would have triggered it. Any asm stmt with a > p constraint and a non-address operand (with another constraint to > accept it) would have exercised this aarch64-specific ICE, and it would > do so before hitting the machine-independent ICE that the patch fixed. > > aarch64_classify_address should probably return false instead of failing > that assertion. When it comes to asm operands, there's no guarantee > that you'll get something that even looks like an address, when you're > part of the very code that's supposed to tell the rest of the compiler > what a legitimate address should look like. Yes, the assert imposes stronger condition on input, which it could simply return false indicating an invalid address. I will test a change to see if there is any fallout. Thanks, bin > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-14 10:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-09 6:45 [PR84682] disregard address constraints on non-addresses Alexandre Oliva 2018-03-09 14:50 ` Vladimir Makarov 2018-03-09 18:51 ` Jeff Law 2018-03-12 10:14 ` Bin.Cheng 2018-03-14 8:32 ` Alexandre Oliva 2018-03-14 11:42 ` Bin.Cheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).