From: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: James Greenhalgh <James.Greenhalgh@arm.com>, nd <nd@arm.com>
Subject: [PATCH][AArch64] Fix ICE due to store_pair_lanes
Date: Mon, 27 Nov 2017 15:44:00 -0000 [thread overview]
Message-ID: <DB6PR0801MB2053ED838D2FC5576E5658E383250@DB6PR0801MB2053.eurprd08.prod.outlook.com> (raw)
The recently added store_pair_lanes causes ICEs in output_operand.
This is due to aarch64_classify_address treating it like a 128-bit STR
rather than a STP. The valid immediate offsets don't fully overlap,
causing it to return false. Eg. offset 264 is a valid 8-byte STP offset
but not a valid 16-byte STR offset since it isn't a multiple of 16.
The original instruction isn't passed in the printing code, so the context
is unclear. The solution is to add a new operand formatting specifier
which is used for LDP/STP instructions like this. This, like the Uml
constraint that applies to store_pair_lanes, uses PARALLEL when calling
aarch64_classify_address so that it knows it is an STP.
Also add the 'z' specifier for future use by load/store pair instructions.
Passes regress, OK for commit?
ChangeLog:
2017-11-27 Wilco Dijkstra <wdijkstr@arm.com>
* config/aarch64/aarch64.c (aarch64_print_operand): Add new
cases for printing LDP/STP memory addresses.
(aarch64_print_address_internal): Renamed from
aarch64_print_operand_address, added parameter, add Pmode check.
(aarch64_print_ldpstp_address): New function for LDP/STP addresses.
(aarch64_print_operand_address): Indirect to
aarch64_print_address_internal.
* config/aarch64/aarch64-simd.md (store_pair_lanes): Use new
'y' operand output specifier.
--
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index cddd935d96589c52519334bd1b8c24e80ea475f3..b7dfbfaa1b00df113329e4b35397e2b6ce234786 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -3047,8 +3047,8 @@ (define_insn "store_pair_lanes<mode>"
(match_operand:VDC 2 "register_operand" "w, r")))]
"TARGET_SIMD"
"@
- stp\\t%d1, %d2, %0
- stp\\t%x1, %x2, %0"
+ stp\\t%d1, %d2, %y0
+ stp\\t%x1, %x2, %y0"
[(set_attr "type" "neon_stp, store_16")]
)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 60a8555b16296f07c68059bc54db803aeef0f369..e601f573bde4fae9a6e32af79ee987190895e088 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -150,6 +150,7 @@ static bool aarch64_builtin_support_vector_misalignment (machine_mode mode,
bool is_packed);
static machine_mode
aarch64_simd_container_mode (scalar_mode mode, unsigned width);
+static void aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x);
/* Major revision number of the ARM Architecture implemented by the target. */
unsigned aarch64_architecture_version;
@@ -5448,7 +5449,11 @@ static const int aarch64_nzcv_codes[] =
'L': Output constant address specified by X
with a relocation offset if appropriate.
'G': Prints address of X, specifying a PC relative
- relocation mode if appropriate. */
+ relocation mode if appropriate.
+ 'y': Output address of LDP or STP - this is used for
+ some LDP/STPs which don't use a PARALLEL in their
+ pattern (so the mode needs to be adjusted).
+ 'z': Output address of a typical LDP or STP. */
static void
aarch64_print_operand (FILE *f, rtx x, int code)
@@ -5650,8 +5655,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
case MEM:
output_address (GET_MODE (x), XEXP (x, 0));
- /* Check all memory references are Pmode - even with ILP32. */
- gcc_assert (GET_MODE (XEXP (x, 0)) == Pmode);
break;
case CONST:
@@ -5815,18 +5818,48 @@ aarch64_print_operand (FILE *f, rtx x, int code)
}
break;
+ case 'y':
+ case 'z':
+ {
+ machine_mode mode = GET_MODE (x);
+
+ if (GET_CODE (x) != MEM)
+ {
+ output_operand_lossage ("invalid operand for '%%%c'", code);
+ return;
+ }
+
+ if (code == 'y')
+ {
+ /* LDP/STP which uses a single double-width memory operand.
+ Adjust the mode to appear like a typical LDP/STP.
+ Currently this is supported for 16-byte accesses only. */
+ gcc_assert (GET_MODE_SIZE (mode) == 16);
+ mode = DFmode;
+ }
+
+ aarch64_print_ldpstp_address (f, mode, XEXP (x, 0));
+ }
+ break;
+
default:
output_operand_lossage ("invalid operand prefix '%%%c'", code);
return;
}
}
+/* Print address 'x' of a memory access with mode 'mode'.
+ 'op' is the context required by aarch64_classify_address. It can either be
+ MEM for a normal memory access or PARALLEL for LDP/STP. */
static void
-aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x)
+aarch64_print_address_internal (FILE *f, machine_mode mode, rtx x, RTX_CODE op)
{
struct aarch64_address_info addr;
- if (aarch64_classify_address (&addr, x, mode, MEM, true))
+ /* Check all addresses are Pmode - including ILP32. */
+ gcc_assert (GET_MODE (x) == Pmode);
+
+ if (aarch64_classify_address (&addr, x, mode, op, true))
switch (addr.type)
{
case ADDRESS_REG_IMM:
@@ -5909,6 +5942,20 @@ aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x)
output_addr_const (f, x);
}
+/* Print address 'x' of a LDP/STP with mode 'mode'. */
+static void
+aarch64_print_ldpstp_address (FILE *f, machine_mode mode, rtx x)
+{
+ aarch64_print_address_internal (f, mode, x, PARALLEL);
+}
+
+/* Print address 'x' of a memory access with mode 'mode'. */
+static void
+aarch64_print_operand_address (FILE *f, machine_mode mode, rtx x)
+{
+ aarch64_print_address_internal (f, mode, x, MEM);
+}
+
bool
aarch64_label_mentioned_p (rtx x)
{
next reply other threads:[~2017-11-27 15:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-27 15:44 Wilco Dijkstra [this message]
2017-11-28 18:40 ` James Greenhalgh
2017-11-29 21:03 ` Christophe Lyon
2017-11-29 22:32 ` Steve Ellcey
2017-11-30 0:52 ` Steve Ellcey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB6PR0801MB2053ED838D2FC5576E5658E383250@DB6PR0801MB2053.eurprd08.prod.outlook.com \
--to=wilco.dijkstra@arm.com \
--cc=James.Greenhalgh@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).