Re: [PATCH, PR62167] Fix tail-merge pass for dead type-unsafe code
On Tue, 18 Nov 2014, Tom de Vries wrote: Richard, this (trunk) patch fixes PR62167. The patch fixes a problem that triggers with the test-case on the 4.8 branch, when tail-merge makes a dead type-unsafe load alive. I'm not able to reproduce this bug on 4.9 and trunk with the same test-case. On those branches, the tail-merge already does not happen. The reason for the difference is as follows: With 4.8 the two phi arguments of the phi in the tail block are value-numbered identically: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)_13].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_14 (changed) ... With 4.9 (and trunk), that's not the case: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)heads][k.1_9].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_15 (changed) ... I'm not sure the bug triggers on trunk and 4.9, but I see no reason why it could not trigger, so I'd prefer to apply the patch to 4.9 and trunk as well. The patch introduces an xfail for pr51879-12.c. I can follow up with a patch to improve upon that, but I think that's better limited to trunk only. Yeah. Bootstrapped and reg-tested on x86_64/trunk. OK for trunk/stage3, 4.8, 4.9? Ok. Note that this goes well with disabling the (re-)use of value-numbering in tail-merging (which causes me quite some headaches...). On trunk it shouldn't be necessary as much as it was earlier as we now fully propagate constants and copies during FRE/PRE elimination. Thanks, Richard.
Re: [PATCH, PR62167] Fix tail-merge pass for dead type-unsafe code
On Tue, 18 Nov 2014, Jeff Law wrote: On 11/18/14 09:57, Tom de Vries wrote: Richard, this (trunk) patch fixes PR62167. The patch fixes a problem that triggers with the test-case on the 4.8 branch, when tail-merge makes a dead type-unsafe load alive. I'm not able to reproduce this bug on 4.9 and trunk with the same test-case. On those branches, the tail-merge already does not happen. The reason for the difference is as follows: With 4.8 the two phi arguments of the phi in the tail block are value-numbered identically: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)_13].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_14 (changed) ... With 4.9 (and trunk), that's not the case: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)heads][k.1_9].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_15 (changed) ... I'm not sure the bug triggers on trunk and 4.9, but I see no reason why it could not trigger, so I'd prefer to apply the patch to 4.9 and trunk as well. The patch introduces an xfail for pr51879-12.c. I can follow up with a patch to improve upon that, but I think that's better limited to trunk only. Bootstrapped and reg-tested on x86_64/trunk. So instead of simply disabling for anything with virtual operands, shouldn't instead we be comparing the virtual operands and alias information? Seems to me that if we did that properly, this would just work. Right? But that would simply use operand_equal_p () Richard.
[PATCH, PR62167] Fix tail-merge pass for dead type-unsafe code
Richard,
this (trunk) patch fixes PR62167.
The patch fixes a problem that triggers with the test-case on the 4.8 branch,
when tail-merge makes a dead type-unsafe load alive.
I'm not able to reproduce this bug on 4.9 and trunk with the same test-case. On
those branches, the tail-merge already does not happen.
The reason for the difference is as follows: With 4.8 the two phi arguments of
the phi in the tail block are value-numbered identically:
...
SCC consists of: p_14
Value numbering p_14 stmt = p_14 = MEM[(struct head *)_13].first;
Setting value number of p_14 to p_14 (changed)
SCC consists of: p_15
Value numbering p_15 stmt = p_15 = _13-next;
Setting value number of p_15 to p_14 (changed)
...
With 4.9 (and trunk), that's not the case:
...
SCC consists of: p_14
Value numbering p_14 stmt = p_14 = MEM[(struct head *)heads][k.1_9].first;
Setting value number of p_14 to p_14 (changed)
SCC consists of: p_15
Value numbering p_15 stmt = p_15 = _13-next;
Setting value number of p_15 to p_15 (changed)
...
I'm not sure the bug triggers on trunk and 4.9, but I see no reason why it could
not trigger, so I'd prefer to apply the patch to 4.9 and trunk as well.
The patch introduces an xfail for pr51879-12.c. I can follow up with a patch to
improve upon that, but I think that's better limited to trunk only.
Bootstrapped and reg-tested on x86_64/trunk.
OK for trunk/stage3, 4.8, 4.9?
Thanks,
- Tom
2014-11-17 Tom de Vries t...@codesourcery.com
PR tree-optimization/62167
* tree-ssa-tail-merge.c (stmt_local_def): Handle statements with vuse
conservatively.
(gimple_equal_p): Don't use vn_valueize to compare for lhs equality of
assigns.
* gcc.dg/pr51879-12.c: Add xfails.
* gcc.dg/pr62167-run.c: New test.
* gcc.dg/pr62167.c: New test.
---
gcc/testsuite/gcc.dg/pr51879-12.c | 4 +--
gcc/testsuite/gcc.dg/pr62167-run.c | 47 +++
gcc/testsuite/gcc.dg/pr62167.c | 50 ++
gcc/tree-ssa-tail-merge.c | 6 +++--
4 files changed, 103 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr62167-run.c
create mode 100644 gcc/testsuite/gcc.dg/pr62167.c
diff --git a/gcc/testsuite/gcc.dg/pr51879-12.c b/gcc/testsuite/gcc.dg/pr51879-12.c
index 8126505..85e2687 100644
--- a/gcc/testsuite/gcc.dg/pr51879-12.c
+++ b/gcc/testsuite/gcc.dg/pr51879-12.c
@@ -24,6 +24,6 @@ foo (int y)
baz (a);
}
-/* { dg-final { scan-tree-dump-times bar \\( 1 pre} } */
-/* { dg-final { scan-tree-dump-times bar2 \\( 1 pre} } */
+/* { dg-final { scan-tree-dump-times bar \\( 1 pre { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times bar2 \\( 1 pre { xfail *-*-* } } } */
/* { dg-final { cleanup-tree-dump pre } } */
diff --git a/gcc/testsuite/gcc.dg/pr62167-run.c b/gcc/testsuite/gcc.dg/pr62167-run.c
new file mode 100644
index 000..37214a3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62167-run.c
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-options -O2 -ftree-tail-merge } */
+
+struct node
+{
+ struct node *next;
+ struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+ struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = heads[2];
+
+int
+main ()
+{
+ struct node *p;
+
+ node.next = (void*)0;
+
+ node.prev = (void *)head;
+
+ head-first = node;
+
+ struct node *n = head-first;
+
+ struct head *h = heads[k];
+
+ heads[2].first = n-next;
+
+ if ((void*)n-prev == (void *)h)
+p = h-first;
+ else
+/* Dead tbaa-unsafe load from ((struct node *)heads[2])-next. */
+p = n-prev-next;
+
+ return !(p == (void*)0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr62167.c b/gcc/testsuite/gcc.dg/pr62167.c
new file mode 100644
index 000..f8c31a0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr62167.c
@@ -0,0 +1,50 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -ftree-tail-merge -fdump-tree-pre } */
+
+struct node
+{
+ struct node *next;
+ struct node *prev;
+};
+
+struct node node;
+
+struct head
+{
+ struct node *first;
+};
+
+struct head heads[5];
+
+int k = 2;
+
+struct head *head = heads[2];
+
+int
+main ()
+{
+ struct node *p;
+
+ node.next = (void*)0;
+
+ node.prev = (void *)head;
+
+ head-first = node;
+
+ struct node *n = head-first;
+
+ struct head *h = heads[k];
+
+ heads[2].first = n-next;
+
+ if ((void*)n-prev == (void *)h)
+p = h-first;
+ else
+/* Dead tbaa-unsafe load from ((struct node *)heads[2])-next. */
+p = n-prev-next;
+
+ return !(p == (void*)0);
+}
+
+/* { dg-final { scan-tree-dump-not Removing basic block pre} } */
+/* { dg-final { cleanup-tree-dump pre } } */
diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
index 303bd5e..1651985 100644
--- a/gcc/tree-ssa-tail-merge.c
+++ b/gcc/tree-ssa-tail-merge.c
@@ -326,7 +326,8 @@ stmt_local_def (gimple stmt)
if (gimple_vdef (stmt) != NULL_TREE
|| gimple_has_side_effects (stmt)
- || gimple_could_trap_p_1 (stmt, false, false))
+ || gimple_could_trap_p_1
Re: [PATCH, PR62167] Fix tail-merge pass for dead type-unsafe code
On 11/18/14 09:57, Tom de Vries wrote: Richard, this (trunk) patch fixes PR62167. The patch fixes a problem that triggers with the test-case on the 4.8 branch, when tail-merge makes a dead type-unsafe load alive. I'm not able to reproduce this bug on 4.9 and trunk with the same test-case. On those branches, the tail-merge already does not happen. The reason for the difference is as follows: With 4.8 the two phi arguments of the phi in the tail block are value-numbered identically: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)_13].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_14 (changed) ... With 4.9 (and trunk), that's not the case: ... SCC consists of: p_14 Value numbering p_14 stmt = p_14 = MEM[(struct head *)heads][k.1_9].first; Setting value number of p_14 to p_14 (changed) SCC consists of: p_15 Value numbering p_15 stmt = p_15 = _13-next; Setting value number of p_15 to p_15 (changed) ... I'm not sure the bug triggers on trunk and 4.9, but I see no reason why it could not trigger, so I'd prefer to apply the patch to 4.9 and trunk as well. The patch introduces an xfail for pr51879-12.c. I can follow up with a patch to improve upon that, but I think that's better limited to trunk only. Bootstrapped and reg-tested on x86_64/trunk. So instead of simply disabling for anything with virtual operands, shouldn't instead we be comparing the virtual operands and alias information? Seems to me that if we did that properly, this would just work. Right? jeff
