Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener <richard.guenther@gmail.com>]

On Mon, Jun 10, 2019 at 10:37 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 6/10/19 1:29 PM, Jakub Jelinek wrote:
> > On Mon, Jun 10, 2019 at 01:23:28PM -0600, Martin Sebor wrote:
> >> +  else if (integer_zerop (TREE_OPERAND (node, 1))
> >> +       /* Dump the types of INTEGER_CSTs explicitly, for we can't
> >> +          infer them and MEM_ATTR caching will share MEM_REFs
> >> +          with differently-typed op0s.  */
> >> +       && TREE_CODE (TREE_OPERAND (node, 0)) != INTEGER_CST
> >> +       /* Released SSA_NAMES have no TREE_TYPE.  */
> >> +       && TREE_TYPE (TREE_OPERAND (node, 0)) != NULL_TREE
> >> +       /* Same pointer types, but ignoring POINTER_TYPE vs.
> >> +          REFERENCE_TYPE.  */
> >> +       && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0)))
> >> +           == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 1))))
> >> +       /* Same value types ignoring qualifiers.  */
> >> +       && (TYPE_MAIN_VARIANT (TREE_TYPE (node))
> >> +           == TYPE_MAIN_VARIANT
> >> +           (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))
> >
> > You should be using types_compatible_p rather than type equality, that is
> > all GIMPLE cares about.
>
> The code above predates my change, I just factored it out into
> a separate function; it does make the diff bigger.  The code
> I introduced only adds the <...> if the size of the access
> differs from the size of the operand. I used type sizes rather
> than testing their compatibility to minimize the number of tests
> that might need updating.
>
> This is the salient bit:
>
> +      pp_string (pp, "MEM");
> +
> +      tree nodetype = TREE_TYPE (node);
> +      tree op0 = TREE_OPERAND (node, 0);
> +      tree op1 = TREE_OPERAND (node, 1);
> +      tree op1type = TYPE_MAIN_VARIANT (TREE_TYPE (op1));
> +
> +      if (!tree_int_cst_equal (TYPE_SIZE (nodetype),
> +                              TYPE_SIZE (TREE_TYPE (op1type))))
> +       {
> +         pp_string (pp, " <");
> +         /* If the size of the type of the operand is not the same
> +            as the size of the MEM_REF expression include the type
> +            of the latter similar to the TDF_GIMPLE output to make
> +            it clear how many bytes of memory are being accessed.  */
> +         dump_generic_node (pp, nodetype, spc, flags | TDF_SLIM, false);
> +         pp_string (pp, "> ");
> +       }

You need to guard against non-constant TYPE_SIZE here (for both
involved types) so I suggest you use operand_equal_p (..., 0).  If you
do that you need to guard against NULL_TREE TYPE_SIZE
(tree_int_cst_equal handled that as unequal).

OK with such change.
Richard.

> Martin