On Wed, Jun 4, 2014 at 10:06 PM, Senthil Kumar Selvaraj
<[email protected]> wrote:
> For the AVR target, assertions in convert_debug_memory_address cause a
> couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx,
> which works, but on debugging further, I found that expand_debug_expr
> appears to incorrectly compute the address space for ADDR_EXPR and
> MEM_REFs.
>
> For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in
> the generic address space, even if the object whose address is taken
> is in a different address space. expand_debug_expr takes
> TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address
> space as generic. convert_debug_memory_address then asserts that the
> mode is a valid pointer mode in the address space and fails.
>
> Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the
> dereferenced value, and therefore checking if it is a POINTER_TYPE
> doesn't help for a single pointer dereference. The address space
> gets computed as generic even if the pointer points to a different
> address space. The address mode for the generic address space is
> passed to convert_debug_memory_address, and the assertion that that mode
> must match the mode of the rtx then fails.
>
> The below patch attempts to fix this by picking the right TREE_TYPE to
> pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and
> ADDR_EXPR (check for pointer type and look at nested addr space).
>
> Does this look reasonable or did I get it all wrong?
>
> Regards
> Senthil
>
> diff --git gcc/cfgexpand.c gcc/cfgexpand.c
> index 8b0e466..ca78953 100644
> --- gcc/cfgexpand.c
> +++ gcc/cfgexpand.c
> @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp)
> op0 = plus_constant (inner_mode, op0, INTVAL (op1));
> }
>
> - if (POINTER_TYPE_P (TREE_TYPE (exp)))
> - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
> + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0))))
> + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0))));
> else
> as = ADDR_SPACE_GENERIC;
TREE_OPERAND (exp, 0) always has pointer type so I'd change this to
as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0))));
> @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp)
> return NULL;
> }
>
> - as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
> + if (POINTER_TYPE_P (TREE_TYPE (exp)))
> + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp)));
> + else
> + as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
> +
Likewise - TREE_TYPE (exp) is always a pointer type. Otherwise the
patch looks ok to me.
Richard.
> op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as);
>
> return op0;
