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

Wed Jun 12 21:47:00 GMT 2019

On 6/12/19 3:33 PM, Rainer Orth wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
> 
>> 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.
> 
> The testsuite part cannot have been tested very thoroughly: for one,
> this snippet

Please try r272218.

Martin

> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c
> @@ -59,4 +59,5 @@ void foo(int prec,
>       bar (&info);
>   }
>   
> -/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" } } */
> +/* { dg-final { scan-tree-dump-times "MEM\\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { ! store_merge } } } }
> +   { dg-final { scan-tree-dump-times "MEM <char[4]> \\\[\\(struct printf_info \\*\\)&info \\+ \[0-9\]+B\\\] = {}" 1 "dse1" { target { store_merge } } } } */
> 
> 
> breaks gcc.sum creation with dg-extract-result.py (the default)
> completely: gcc.log shows
> 
> spawn -ignore SIGHUP /var/gcc/regression/trunk/11-gcc/build/gcc/xgcc -B/var/gcc/regression/trunk/11-gcc/build/gcc/ /vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-24.c -fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never -O2 -fdump-tree-dse1 -S -o ssa-dse-24.s^M
> PASS: gcc.dg/tree-ssa/ssa-dse-24.c (test for excess errors)
> ERROR: (DejaGnu) proc "4" does not exist.
> The error code is NONE
> The info on the error is:
> invalid command name "4"
>      while executing
> "::tcl_unknown 4"
>      ("uplevel" body line 1)
>      invoked from within
> "uplevel 1 ::tcl_unknown $args"
> 
> due to the unquoted [4].
> 
> Even with that fixed, I see many failures:
> 
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++14  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++17  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/pr31146.C  -std=gnu++98  scan-tree-dump forwprop1 "MEM\\\\[.*&i\\\\]\\\\[j.*\\\\] =.* 1;"
> +FAIL: g++.dg/tree-ssa/ssa-dse-1.C   scan-tree-dump-times dse1 "MEM <char\\\\[176]> \\\\[\\\\(struct FixBuf \\\\*\\\\)&<retval> \\\\+ [0-9]+B\\\\] = {}" 1
> 
> on 32 and 64-bit i386-pc-solaris2.11 (and i686-pc-linux-gnu),
> 
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++14  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++17  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> +FAIL: g++.dg/tree-ssa/pr19807.C  -std=gnu++98  scan-tree-dump-times optimized "&MEM\\\\[\\\\(void .\\\\)&a \\\\+ 8B\\\\]" 3
> 
> +FAIL: gcc.dg/tree-prof/stringop-2.c scan-tree-dump optimized "MEM\\\\[\\\\(void .\\\\)&a\\\\] = 168430090"
> +FAIL: gcc.dg/tree-ssa/pr30375.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct _s \\\\*\\\\)&signInfo \\\\+ [0-9]+B\\\\] = {}" 1
> +FAIL: gcc.dg/tree-ssa/slsr-27.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 3
> +FAIL: gcc.dg/tree-ssa/slsr-28.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
> +FAIL: gcc.dg/tree-ssa/slsr-29.c scan-tree-dump-times dom3 "MEM\\\\[\\\\(struct x \\\\*\\\\)[^\\r\\n]*_\\\\d+" 9
> +FAIL: gcc.dg/tree-ssa/ssa-dse-24.c scan-tree-dump-times dse1 "MEM\\\\[\\\\(struct printf_info \\\\*\\\\)&info \\\\+ [0-9]+B\\\\] = {}" 1
> 
> on 32 and 64-bit sparc-sun-solaris2.11 (the first of those also on
> m68k-unknown-linux-gnu).
> 
> I haven't even started looking what's wrong (quoting problems in the REs
> or other issues).