| Message ID | e9708090eecc283173763ea9676775c9dfa77839.camel@marvell.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR target/85711 - Fix ICE on aarch64 | expand |
Steve Ellcey <sellcey@marvell.com> writes: > The test gcc.dg/torture/pr84682-2.c has been failing for some time on > aarch64. Bin Cheng submitted a patch for this some time ago, the > original patch was: > > https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00784.html > > But Richard Sandiford thought it should be fixed in recog.c instead of > just in aarch64.c. Bin submitted a followup: > > https://gcc.gnu.org/ml/gcc-patches/2018-03/msg01166.html > > But there were no replies to that patch submission. IMO we shouldn't remove the assert. See: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html and the thread leading up to it. Thanks, Richard
On Wed, 2019-01-23 at 16:54 +0000, Richard Sandiford wrote: > > IMO we shouldn't remove the assert. See: > > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html > > and the thread leading up to it. > > Thanks, > Richard OK, I hadn't seen that thread. I didn't see any patch submitted in response to your comment there so I created a new patch. This patch leaves the assert in aarch64.c and changes the check for the 'p' constraint in constain_operands, this version fixes the pr84682-2.c test failure and causes no regressions on aarch64 or x86. Steve Ellcey sellcey@marvell.com 2019-01-23 Bin Cheng <bin.cheng@arm.com> Steve Ellcey <sellcey@marvell.com> PR target/85711 * recog.c (address_operand): Return false on wrong mode for address. (constrain_operands): Check for mode with 'p' constraint. diff --git a/gcc/recog.c b/gcc/recog.c index d0c498fced2..a9f584bc0dc 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode) int address_operand (rtx op, machine_mode mode) { + /* Wrong mode for an address expr. */ + if (GET_MODE (op) != VOIDmode + && ! SCALAR_INT_MODE_P (GET_MODE (op))) + return false; + return memory_address_p (mode, op); } @@ -2696,10 +2701,13 @@ constrain_operands (int strict, alternative_mask alternatives) /* p is used for address_operands. When we are called by gen_reload, no one will have checked that the address is strictly valid, i.e., that all pseudos requiring hard regs - have gotten them. */ - if (strict <= 0 - || (strict_memory_address_p (recog_data.operand_mode[opno], - op))) + have gotten them. We also want to make sure we have a + valid mode. */ + if ((GET_MODE (op) == VOIDmode + || SCALAR_INT_MODE_P (GET_MODE (op))) + && (strict <= 0 + || (strict_memory_address_p + (recog_data.operand_mode[opno], op)))) win = 1; break;
Steve Ellcey <sellcey@marvell.com> writes: > On Wed, 2019-01-23 at 16:54 +0000, Richard Sandiford wrote: >> >> IMO we shouldn't remove the assert. See: >> >> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html >> >> and the thread leading up to it. >> >> Thanks, >> Richard > > OK, I hadn't seen that thread. I didn't see any patch submitted Yeah, I kept meaning to get round to it but never did... > in response to your comment there so I created a new patch. > This patch leaves the assert in aarch64.c and changes the check > for the 'p' constraint in constain_operands, this version > fixes the pr84682-2.c test failure and causes no regressions > on aarch64 or x86. Great! > Steve Ellcey > sellcey@marvell.com > > > 2019-01-23 Bin Cheng <bin.cheng@arm.com> > Steve Ellcey <sellcey@marvell.com> > > PR target/85711 > * recog.c (address_operand): Return false on wrong mode for address. > (constrain_operands): Check for mode with 'p' constraint. OK, thanks. Richard
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 5df5a8b..aa3054d 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -6565,9 +6565,6 @@ aarch64_classify_address (struct aarch64_address_info *info, && (code != POST_INC && code != REG)) return false; - gcc_checking_assert (GET_MODE (x) == VOIDmode - || SCALAR_INT_MODE_P (GET_MODE (x))); - switch (code) { case REG: diff --git a/gcc/recog.c b/gcc/recog.c index d0c498f..fb90302 100644 --- a/gcc/recog.c +++ b/gcc/recog.c @@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode) int address_operand (rtx op, machine_mode mode) { + /* Wrong mode for an address expr. */ + if (GET_MODE (op) != VOIDmode + && ! SCALAR_INT_MODE_P (GET_MODE (op))) + return false; + return memory_address_p (mode, op); }
The test gcc.dg/torture/pr84682-2.c has been failing for some time on aarch64. Bin Cheng submitted a patch for this some time ago, the original patch was: https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00784.html But Richard Sandiford thought it should be fixed in recog.c instead of just in aarch64.c. Bin submitted a followup: https://gcc.gnu.org/ml/gcc-patches/2018-03/msg01166.html But there were no replies to that patch submission. I have retested the second patch and verified it fixes the aarch64 failure and causes no regressions on aarch64 or x86. OK to checkin? Steve Ellcey sellcey@marvell.com 2019-01-23 Bin Cheng <bin.cheng@arm.com> Steve Ellcey <sellcey@marvell.com> * recog.c (address_operand): Return false on wrong mode for address. * config/aarch64/aarch64.c (aarch64_classify_address): Remove assert since it's checked in general code now.