This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix bug in MEM parsing in patches 8a/8b


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]