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

[Jeff Law <law@redhat.com>]

On 12/08/2016 01:39 PM, David Malcolm wrote:
> 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.
They seem like reasonable adjustments.

I know you posted the cc1 patches to add the RTL front-end.  What's the 
status on this kit?

[ Yes, I keep falling behind... ]

jeff