Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Jun 10, 2019 at 10:37 PM Martin Sebor <mse...@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
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).
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
