Petro Karashchenko via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> for bitfield MEMREFs
>
> [FIX] Propagate uncached type attributes to unaligned/packed types
This doesn't look safe in general. The current code was added
to avoid wrong-code problems for accesses that step outside the
bounds of the original MEM_EXPR.
Could you explain in more detail what's going wrong in the testcase?
It wasn't obvious why the code you're removing would trigger for that test.
Thanks,
Richard
>
> [ARC] Update tests
>
> gcc/
> xxxx-xx-xx Petro Karashchenko <petro.karashche...@gmail.com>
>
> * emit-rtl.c (adjust_address_1): Do not drop the object
> if the new memory reference is outside the underlying
> object to preserve object attributes that are needed
> by some backend implementations. Remove adjust_object
> parameter as it is not used anymore.
> (adjust_automodify_address_1): Adjust according to
> new adjust_address_1 prototype.
> (replace_equiv_address_nv): Likewise.
> * gcc/emit-rtl.h (adjust_address): Adjust according to
> new adjust_address_1 prototype.
> (adjust_address_nv): Likewise.
> (adjust_bitfield_address): Likewise.
> (adjust_bitfield_address_size): Likewise.
> (adjust_bitfield_address_nv): Likewise.
>
> testsuite/
> xxxx-xx-xx Petro Karashchenko <petro.karashche...@gmail.com>
>
> * gcc.target/arc/uncached-9.c: New file.
>
> Problem description:
> __attribute__((uncached)) is dropped for other than the first member of
> "packed" or "unaligned" types.
>
> Tests:
> Tested with ARC600 bases ASIC. The correct code is generated.
>
> From 1f8a824f8ed8c1452f2bdfc513716c63d5d12545 Mon Sep 17 00:00:00 2001
> From: Petro Karashchenko <petro.karashche...@gmail.com>
> Date: Sun, 16 Aug 2020 01:10:11 +0300
> Subject: [PATCH] [FIX] Remove object adjustment to preserve object attributes
> for bitfield MEMREFs
>
> [FIX] Propagate uncached type attributes to unaligned/packed types
>
> [ARC] Update tests
>
> gcc/
> xxxx-xx-xx Petro Karashchenko <petro.karashche...@gmail.com>
>
> * emit-rtl.c (adjust_address_1): Do not drop the object
> if the new memory reference is outside the underlying
> object to preserve object attributes that are needed
> by some backend implementations. Remove adjust_object
> parameter as it is not used anymore.
> (adjust_automodify_address_1): Adjust according to
> new adjust_address_1 prototype.
> (replace_equiv_address_nv): Likewise.
> * gcc/emit-rtl.h (adjust_address): Adjust according to
> new adjust_address_1 prototype.
> (adjust_address_nv): Likewise.
> (adjust_bitfield_address): Likewise.
> (adjust_bitfield_address_size): Likewise.
> (adjust_bitfield_address_nv): Likewise.
>
> testsuite/
> xxxx-xx-xx Petro Karashchenko <petro.karashche...@gmail.com>
>
> * gcc.target/arc/uncached-9.c: New file.
>
> Signed-off-by: Petro Karashchenko <petro.karashche...@gmail.com>
> ---
> gcc/emit-rtl.c | 28 +++--------------------
> gcc/emit-rtl.h | 12 +++++-----
> gcc/testsuite/gcc.target/arc/uncached-9.c | 19 +++++++++++++++
> 3 files changed, 28 insertions(+), 31 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arc/uncached-9.c
>
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index f9b0e9714d9..c0e2ead907b 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -2337,8 +2337,7 @@ change_address (rtx memref, machine_mode mode, rtx addr)
>
> rtx
> adjust_address_1 (rtx memref, machine_mode mode, poly_int64 offset,
> - int validate, int adjust_address, int adjust_object,
> - poly_int64 size)
> + int validate, int adjust_address, poly_int64 size)
> {
> rtx addr = XEXP (memref, 0);
> rtx new_rtx;
> @@ -2413,25 +2412,11 @@ adjust_address_1 (rtx memref, machine_mode mode,
> poly_int64 offset,
> if (new_rtx == memref && maybe_ne (offset, 0))
> new_rtx = copy_rtx (new_rtx);
>
> - /* Conservatively drop the object if we don't know where we start from. */
> - if (adjust_object && (!attrs.offset_known_p || !attrs.size_known_p))
> - {
> - attrs.expr = NULL_TREE;
> - attrs.alias = 0;
> - }
> -
> /* Compute the new values of the memory attributes due to this adjustment.
> We add the offsets and update the alignment. */
> if (attrs.offset_known_p)
> {
> attrs.offset += offset;
> -
> - /* Drop the object if the new left end is not within its bounds. */
> - if (adjust_object && maybe_lt (attrs.offset, 0))
> - {
> - attrs.expr = NULL_TREE;
> - attrs.alias = 0;
> - }
> }
>
> /* Compute the new alignment by taking the MIN of the alignment and the
> @@ -2445,18 +2430,11 @@ adjust_address_1 (rtx memref, machine_mode mode,
> poly_int64 offset,
>
> if (maybe_ne (size, 0))
> {
> - /* Drop the object if the new right end is not within its bounds. */
> - if (adjust_object && maybe_gt (offset + size, attrs.size))
> - {
> - attrs.expr = NULL_TREE;
> - attrs.alias = 0;
> - }
> attrs.size_known_p = true;
> attrs.size = size;
> }
> else if (attrs.size_known_p)
> {
> - gcc_assert (!adjust_object);
> attrs.size -= offset;
> /* ??? The store_by_pieces machinery generates negative sizes,
> so don't assert for that here. */
> @@ -2477,7 +2455,7 @@ adjust_automodify_address_1 (rtx memref, machine_mode
> mode, rtx addr,
> poly_int64 offset, int validate)
> {
> memref = change_address_1 (memref, VOIDmode, addr, validate, false);
> - return adjust_address_1 (memref, mode, offset, validate, 0, 0, 0);
> + return adjust_address_1 (memref, mode, offset, validate, 0, 0);
> }
>
> /* Return a memory reference like MEMREF, but whose address is changed by
> @@ -2561,7 +2539,7 @@ replace_equiv_address_nv (rtx memref, rtx addr, bool
> inplace)
> rtx
> widen_memory_access (rtx memref, machine_mode mode, poly_int64 offset)
> {
> - rtx new_rtx = adjust_address_1 (memref, mode, offset, 1, 1, 0, 0);
> + rtx new_rtx = adjust_address_1 (memref, mode, offset, 1, 1, 0);
> poly_uint64 size = GET_MODE_SIZE (mode);
>
> /* If there are no changes, just return the original memory reference. */
> diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
> index 3d6565c8a30..108669f51a1 100644
> --- a/gcc/emit-rtl.h
> +++ b/gcc/emit-rtl.h
> @@ -473,27 +473,27 @@ extern rtx change_address (rtx, machine_mode, rtx);
> /* Return a memory reference like MEMREF, but with its mode changed
> to MODE and its address offset by OFFSET bytes. */
> #define adjust_address(MEMREF, MODE, OFFSET) \
> - adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, 0, 0)
> + adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, 0)
>
> /* Likewise, but the reference is not required to be valid. */
> #define adjust_address_nv(MEMREF, MODE, OFFSET) \
> - adjust_address_1 (MEMREF, MODE, OFFSET, 0, 1, 0, 0)
> + adjust_address_1 (MEMREF, MODE, OFFSET, 0, 1, 0)
>
> /* Return a memory reference like MEMREF, but with its mode changed
> to MODE and its address offset by OFFSET bytes. Assume that it's
> for a bitfield and conservatively drop the underlying object if we
> cannot be sure to stay within its bounds. */
> #define adjust_bitfield_address(MEMREF, MODE, OFFSET) \
> - adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, 1, 0)
> + adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, 0)
>
> /* As for adjust_bitfield_address, but specify that the width of
> BLKmode accesses is SIZE bytes. */
> #define adjust_bitfield_address_size(MEMREF, MODE, OFFSET, SIZE) \
> - adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, 1, SIZE)
> + adjust_address_1 (MEMREF, MODE, OFFSET, 1, 1, SIZE)
>
> /* Likewise, but the reference is not required to be valid. */
> #define adjust_bitfield_address_nv(MEMREF, MODE, OFFSET) \
> - adjust_address_1 (MEMREF, MODE, OFFSET, 0, 1, 1, 0)
> + adjust_address_1 (MEMREF, MODE, OFFSET, 0, 1, 0)
>
> /* Return a memory reference like MEMREF, but with its mode changed
> to MODE and its address changed to ADDR, which is assumed to be
> @@ -506,7 +506,7 @@ extern rtx change_address (rtx, machine_mode, rtx);
> adjust_automodify_address_1 (MEMREF, MODE, ADDR, OFFSET, 0)
>
> extern rtx adjust_address_1 (rtx, machine_mode, poly_int64, int, int,
> - int, poly_int64);
> + poly_int64);
> extern rtx adjust_automodify_address_1 (rtx, machine_mode, rtx,
> poly_int64, int);
>
> diff --git a/gcc/testsuite/gcc.target/arc/uncached-9.c
> b/gcc/testsuite/gcc.target/arc/uncached-9.c
> new file mode 100644
> index 00000000000..44b8a0ed7f3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/uncached-9.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +
> +#include <stddef.h>
> +
> +typedef volatile struct {
> + uint32_t a1;
> + uint32_t a2;
> + uint32_t a3;
> + uint32_t a4;
> +} __attribute__((packed,uncached)) my_type_t;
> +
> +uint32_t foo (my_type_t *p)
> +{
> + p->a3 = p->a2;
> + return p->a4;
> +}
> +
> +/* { dg-final { scan-assembler-times "stb\.di" 4 } } */
> +/* { dg-final { scan-assembler-times "ldb\.di" 12 } } */
