This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix bug in MEM parsing in patches 8a/8b
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Bernd Schmidt <bschmidt at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Thu, 8 Dec 2016 15:39:09 -0500
- Subject: [PATCH] Fix bug in MEM parsing in patches 8a/8b
- Authentication-results: sourceware.org; auth=none
- References: <1480704295-31124-1-git-send-email-dmalcolm@redhat.com>
Testing the patch kit on i686 showed numerous failures of this
assertion in set_mem_attributes_minus_bitpos in emit-rtl.c:
1821 gcc_assert (!defattrs->offset_known_p);
when expanding "main" in the rtl.exp test files, after parsing
an __RTL-tagged function.
Root cause is various assignments within the RTL parser of the
form:
1222 MEM_OFFSET (x) = atoi (name.string);
where a MEM_* macro appears on the left-hand side of an assignment.
These macros are defined as a field lookup on the result of a call
to get_mem_attrs, e.g.:
#define MEM_OFFSET(RTX) (get_mem_attrs (RTX)->offset)
get_mem_attrs can return the struct mem_attrs * of an rtx, but if
it isn't set, it returns:
mode_mem_attrs[(int) GET_MODE (x)];
which is this field within struct GTY(()) target_rtl:
/* The default memory attributes for each mode. */
struct mem_attrs *x_mode_mem_attrs[(int) MAX_MACHINE_MODE];
These assignments in the parser were erroneously writing to these
default per-mode values, rather than assigning to a unique-per-rtx
instance of struct mem_attrs.
The fix is to call the appropriate set_mem_ functions in the
parser, e.g. set_mem_offset; the patch below is intended as a tweak
to patch 8a of the kit, and would be merged with it before committing.
The patch also adds extra test coverage for MEM parsing. This extends
the target-independent selftests, and so would go into patch 8b.
Tested for targets x86_64-pc-linux-gnu, i686-pc-linux-gnu,
and aarch64-linux-gnu, and on powerpc-ibm-aix7.1.3.0.
OK as adjustments to patches 8a and 8b?
For patch 8a:
gcc/ChangeLog:
* read-rtl-function.c
(function_reader::handle_any_trailing_information): Replace writes
through macros MEM_ALIAS_SET, MEM_OFFSET, MEM_SIZE, MEM_ALIGN,
and MEM_ADDR_SPACE with calls to set_mem_ functions. Add missing
call to unread_char when handling "A" for alignment.
For patch 8b:
gcc/ChangeLog:
* read-rtl-function.c (selftest::test_loading_mem): New function.
(selftest::read_rtl_function_c_tests): Call it.
gcc/testsuite/ChangeLog:
* selftests/mem.rtl: New file.
---
gcc/read-rtl-function.c | 55 ++++++++++++++++++++++++++++++++++++-----
gcc/testsuite/selftests/mem.rtl | 9 +++++++
2 files changed, 58 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/selftests/mem.rtl
diff --git a/gcc/read-rtl-function.c b/gcc/read-rtl-function.c
index 5188b86..c3f5c64 100644
--- a/gcc/read-rtl-function.c
+++ b/gcc/read-rtl-function.c
@@ -1201,7 +1201,7 @@ function_reader::handle_any_trailing_information (rtx x)
int ch;
require_char_ws ('[');
read_name (&name);
- MEM_ALIAS_SET (x) = atoi (name.string);
+ set_mem_alias_set (x, atoi (name.string));
/* We have either a MEM_EXPR, or a space. */
if (peek_char () != ' ')
{
@@ -1218,8 +1218,7 @@ function_reader::handle_any_trailing_information (rtx x)
if (ch == '+')
{
read_name (&name);
- MEM_OFFSET_KNOWN_P (x) = 1;
- MEM_OFFSET (x) = atoi (name.string);
+ set_mem_offset (x, atoi (name.string));
}
else
unread_char (ch);
@@ -1229,7 +1228,7 @@ function_reader::handle_any_trailing_information (rtx x)
if (ch == 'S')
{
read_name (&name);
- MEM_SIZE (x) = atoi (name.string);
+ set_mem_size (x, atoi (name.string));
}
else
unread_char (ch);
@@ -1239,8 +1238,10 @@ function_reader::handle_any_trailing_information (rtx x)
if (ch == 'A' && peek_char () != 'S')
{
read_name (&name);
- MEM_ALIGN (x) = atoi (name.string);
+ set_mem_align (x, atoi (name.string));
}
+ else
+ unread_char (ch);
/* Handle optional " AS" for MEM_ADDR_SPACE. */
ch = read_skip_spaces ();
@@ -1248,7 +1249,7 @@ function_reader::handle_any_trailing_information (rtx x)
{
read_char ();
read_name (&name);
- MEM_ADDR_SPACE (x) = atoi (name.string);
+ set_mem_addr_space (x, atoi (name.string));
}
else
unread_char (ch);
@@ -2102,6 +2103,47 @@ test_loading_bb_index ()
ASSERT_EQ (42, bb42->index);
}
+/* Verify that function_reader::handle_any_trailing_information correctly
+ parses all the possible items emitted for a MEM. */
+
+static void
+test_loading_mem ()
+{
+ rtl_dump_test t (SELFTEST_LOCATION, locate_file ("mem.rtl"));
+
+ ASSERT_STREQ ("test_mem", IDENTIFIER_POINTER (DECL_NAME (cfun->decl)));
+ ASSERT_TRUE (cfun);
+
+ /* Verify parsing of "[42 i+17 S8 A128 AS5]". */
+ rtx_insn *insn_1 = get_insn_by_uid (1);
+ rtx set1 = single_set (insn_1);
+ rtx mem1 = SET_DEST (set1);
+ ASSERT_EQ (42, MEM_ALIAS_SET (mem1));
+ /* "+17". */
+ ASSERT_TRUE (MEM_OFFSET_KNOWN_P (mem1));
+ ASSERT_EQ (17, MEM_OFFSET (mem1));
+ /* "S8". */
+ ASSERT_EQ (8, MEM_SIZE (mem1));
+ /* "A128. */
+ ASSERT_EQ (128, MEM_ALIGN (mem1));
+ /* "AS5. */
+ ASSERT_EQ (5, MEM_ADDR_SPACE (mem1));
+
+ /* Verify parsing of "43 i+18 S9 AS6"
+ (an address space without an alignment). */
+ rtx_insn *insn_2 = get_insn_by_uid (2);
+ rtx set2 = single_set (insn_2);
+ rtx mem2 = SET_DEST (set2);
+ ASSERT_EQ (43, MEM_ALIAS_SET (mem2));
+ /* "+18". */
+ ASSERT_TRUE (MEM_OFFSET_KNOWN_P (mem2));
+ ASSERT_EQ (18, MEM_OFFSET (mem2));
+ /* "S9". */
+ ASSERT_EQ (9, MEM_SIZE (mem2));
+ /* "AS6. */
+ ASSERT_EQ (6, MEM_ADDR_SPACE (mem2));
+}
+
/* Run all of the selftests within this file. */
void
@@ -2122,6 +2164,7 @@ read_rtl_function_c_tests ()
test_loading_symbol_ref ();
test_loading_cfg ();
test_loading_bb_index ();
+ test_loading_mem ();
}
} // namespace selftest
diff --git a/gcc/testsuite/selftests/mem.rtl b/gcc/testsuite/selftests/mem.rtl
new file mode 100644
index 0000000..9261d79
--- /dev/null
+++ b/gcc/testsuite/selftests/mem.rtl
@@ -0,0 +1,9 @@
+(function "test_mem"
+ (insn-chain
+ (block 2
+ ;; Various nonsensical values, to exercise the parser:
+ (cinsn 1 (set (mem:SI (reg:SI %10) [42 i+17 S8 A128 AS5]) (reg:SI %1)))
+ (cinsn 2 (set (mem:SI (reg:SI %11) [43 i+18 S9 AS6]) (reg:SI %2)))
+ ) ;; block 6
+ ) ;; insn-chain
+) ;; function
--
1.8.5.3