On Fri, Jun 16, 2017 at 12:26 PM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> I don't think so. get_address_description assumes TMR_BASE is in
>> canonical form,
>> that is, when it is an ADDR_EXPR we have a symbol and when not we have
>> a pointer.
>> TMR[&p->a] violates this and the gimple-fold.c part first canonicalizes this
>> to TMR[p + offsetof(a)].
>
> get_address_description doesn't assume anything on TMR_BASE:
>
> void
> get_address_description (tree op, struct mem_address *addr)
> {
> if (TREE_CODE (TMR_BASE (op)) == ADDR_EXPR)
> {
> addr->symbol = TMR_BASE (op);
> addr->base = TMR_INDEX2 (op);
> }
> else
> {
> addr->symbol = NULL_TREE;
> if (TMR_INDEX2 (op))
> {
> gcc_assert (integer_zerop (TMR_BASE (op)));
> addr->base = TMR_INDEX2 (op);
> }
> else
> addr->base = TMR_BASE (op);
> }
> addr->index = TMR_INDEX (op);
> addr->step = TMR_STEP (op);
> addr->offset = TMR_OFFSET (op);
> }
>
> and maybe_fold_tmr will precisely turn TMR[&p->a] into TMR[p + offsetof(a)]:
>
> if (addr.symbol
> && TREE_CODE (TREE_OPERAND (addr.symbol, 0)) == MEM_REF)
> {
> addr.offset = fold_binary_to_constant
> (PLUS_EXPR, TREE_TYPE (addr.offset),
> addr.offset,
> TREE_OPERAND (TREE_OPERAND (addr.symbol, 0), 1));
> addr.symbol = TREE_OPERAND (TREE_OPERAND (addr.symbol, 0), 0);
> changed = true;
> }
> else if (addr.symbol
> && handled_component_p (TREE_OPERAND (addr.symbol, 0)))
> {
> HOST_WIDE_INT offset;
> addr.symbol = build_fold_addr_expr
> (get_addr_base_and_unit_offset
> (TREE_OPERAND (addr.symbol, 0), &offset));
> addr.offset = int_const_binop (PLUS_EXPR,
> addr.offset, size_int (offset));
> changed = true;
> }
>
> The transformations are exactly the same in maybe_canonicalize_mem_ref_addr.
Well, it seems this just compensates for the fact
get_address_description is confused
and says it has a symbol when it has not.
I'd rather leave the canonicalization in a single place for both
MEM_REF and TARGET_MEM_REF
and instead remove the above code from maybe_fold_tmr (which is only called from
maybe_canonicalize_mem_ref_addr btw. Inlining it (and thus exporting
create_mem_ref_raw)
would work for me as well and likely reduce the confusion as to what
is done where.
Richard.
> --
> Eric Botcazou
