From: Richard Biener <rguenther@suse.de>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH][GCC 7] Fix PR70171
Date: Fri, 11 Mar 2016 14:01:00 -0000 [thread overview]
Message-ID: <alpine.LSU.2.11.1603111457460.31547@t29.fhfr.qr> (raw)
The following teaches phiprop to handle the case of aggregate copies
where the aggregate has non-BLKmode which means it is very likely
expanded as reg-reg moves (any better test for that apart from
checking for non-BLKmode?). This improves code for the testcase
from
_Z14struct_ternary1SS_b:
.LFB2:
.cfi_startproc
leaq -40(%rsp), %rcx
leaq -24(%rsp), %rax
testb %dl, %dl
movl %edi, -24(%rsp)
movl %esi, -40(%rsp)
cmove %rcx, %rax
movl (%rax), %eax
ret
to
_Z14struct_ternary1SS_b:
.LFB2:
.cfi_startproc
testb %dl, %dl
movl %edi, %eax
cmove %esi, %eax
ret
Bootstrapped and tested on x86_64-unknown-linux-gnu, queued for stage1.
Richard.
2016-03-11 Richard Biener <rguenther@suse.de>
PR tree-optimization/70171
* tree-ssa-phiprop.c: Include stor-layout.h.
(phiprop_insert_phi): Handle the aggregate copy case.
(propagate_with_phi): Likewise.
* g++.dg/tree-ssa/pr70171.C: New testcase.
Index: gcc/tree-ssa-phiprop.c
===================================================================
*** gcc/tree-ssa-phiprop.c (revision 234134)
--- gcc/tree-ssa-phiprop.c (working copy)
*************** along with GCC; see the file COPYING3.
*** 31,36 ****
--- 31,37 ----
#include "tree-eh.h"
#include "gimplify.h"
#include "gimple-iterator.h"
+ #include "stor-layout.h"
/* This pass propagates indirect loads through the PHI node for its
address to make the load source possibly non-addressable and to
*************** phiprop_insert_phi (basic_block bb, gphi
*** 132,138 ****
struct phiprop_d *phivn, size_t n)
{
tree res;
! gphi *new_phi;
edge_iterator ei;
edge e;
--- 133,139 ----
struct phiprop_d *phivn, size_t n)
{
tree res;
! gphi *new_phi = NULL;
edge_iterator ei;
edge e;
*************** phiprop_insert_phi (basic_block bb, gphi
*** 142,148 ****
/* Build a new PHI node to replace the definition of
the indirect reference lhs. */
res = gimple_assign_lhs (use_stmt);
! new_phi = create_phi_node (res, bb);
if (dump_file && (dump_flags & TDF_DETAILS))
{
--- 143,150 ----
/* Build a new PHI node to replace the definition of
the indirect reference lhs. */
res = gimple_assign_lhs (use_stmt);
! if (TREE_CODE (res) == SSA_NAME)
! new_phi = create_phi_node (res, bb);
if (dump_file && (dump_flags & TDF_DETAILS))
{
*************** phiprop_insert_phi (basic_block bb, gphi
*** 187,193 ****
{
tree rhs = gimple_assign_rhs1 (use_stmt);
gcc_assert (TREE_CODE (old_arg) == ADDR_EXPR);
! new_var = make_ssa_name (TREE_TYPE (rhs));
if (!is_gimple_min_invariant (old_arg))
old_arg = PHI_ARG_DEF_FROM_EDGE (phi, e);
else
--- 189,198 ----
{
tree rhs = gimple_assign_rhs1 (use_stmt);
gcc_assert (TREE_CODE (old_arg) == ADDR_EXPR);
! if (TREE_CODE (res) == SSA_NAME)
! new_var = make_ssa_name (TREE_TYPE (rhs));
! else
! new_var = unshare_expr (res);
if (!is_gimple_min_invariant (old_arg))
old_arg = PHI_ARG_DEF_FROM_EDGE (phi, e);
else
*************** phiprop_insert_phi (basic_block bb, gphi
*** 210,222 ****
}
}
! add_phi_arg (new_phi, new_var, e, locus);
}
! update_stmt (new_phi);
! if (dump_file && (dump_flags & TDF_DETAILS))
! print_gimple_stmt (dump_file, new_phi, 0, 0);
return res;
}
--- 215,231 ----
}
}
! if (new_phi)
! add_phi_arg (new_phi, new_var, e, locus);
}
! if (new_phi)
! {
! update_stmt (new_phi);
! if (dump_file && (dump_flags & TDF_DETAILS))
! print_gimple_stmt (dump_file, new_phi, 0, 0);
! }
return res;
}
*************** propagate_with_phi (basic_block bb, gphi
*** 250,256 ****
tree type = NULL_TREE;
if (!POINTER_TYPE_P (TREE_TYPE (ptr))
! || !is_gimple_reg_type (TREE_TYPE (TREE_TYPE (ptr))))
return false;
/* Check if we can "cheaply" dereference all phi arguments. */
--- 259,266 ----
tree type = NULL_TREE;
if (!POINTER_TYPE_P (TREE_TYPE (ptr))
! || (!is_gimple_reg_type (TREE_TYPE (TREE_TYPE (ptr)))
! && TYPE_MODE (TREE_TYPE (TREE_TYPE (ptr))) == BLKmode))
return false;
/* Check if we can "cheaply" dereference all phi arguments. */
*************** propagate_with_phi (basic_block bb, gphi
*** 306,312 ****
/* Check whether this is a load of *ptr. */
if (!(is_gimple_assign (use_stmt)
- && TREE_CODE (gimple_assign_lhs (use_stmt)) == SSA_NAME
&& gimple_assign_rhs_code (use_stmt) == MEM_REF
&& TREE_OPERAND (gimple_assign_rhs1 (use_stmt), 0) == ptr
&& integer_zerop (TREE_OPERAND (gimple_assign_rhs1 (use_stmt), 1))
--- 316,321 ----
*************** propagate_with_phi (basic_block bb, gphi
*** 327,335 ****
bb, gimple_bb (def_stmt))))
goto next;
/* Found a proper dereference. Insert a phi node if this
is the first load transformation. */
! if (!phi_inserted)
{
res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n);
type = TREE_TYPE (res);
--- 336,366 ----
bb, gimple_bb (def_stmt))))
goto next;
+ /* Found a proper dereference with an aggregate copy. Just
+ insert aggregate copies on the edges instead. */
+ if (!is_gimple_reg_type (TREE_TYPE (TREE_TYPE (ptr))))
+ {
+ phiprop_insert_phi (bb, phi, use_stmt, phivn, n);
+
+ /* Remove old stmt. The phi is taken care of by DCE. */
+ gsi = gsi_for_stmt (use_stmt);
+ /* Unlinking the VDEF here is fine as we are sure that we process
+ stmts in execution order due to aggregate copies having VDEFs
+ and we emit loads on the edges in the very same order.
+ We get multiple copies (or intermediate register loads) handled
+ only by walking PHIs or immediate uses in a lucky order though,
+ so we could signal the caller to re-start iterating over PHIs
+ when we come here which would make it quadratic in the number
+ of PHIs. */
+ unlink_stmt_vdef (use_stmt);
+ gsi_remove (&gsi, true);
+
+ phi_inserted = true;
+ }
+
/* Found a proper dereference. Insert a phi node if this
is the first load transformation. */
! else if (!phi_inserted)
{
res = phiprop_insert_phi (bb, phi, use_stmt, phivn, n);
type = TREE_TYPE (res);
Index: gcc/testsuite/g++.dg/tree-ssa/pr70171.C
===================================================================
*** gcc/testsuite/g++.dg/tree-ssa/pr70171.C (revision 0)
--- gcc/testsuite/g++.dg/tree-ssa/pr70171.C (working copy)
***************
*** 0 ****
--- 1,8 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-tree-optimized" } */
+
+ struct S { int i; };
+ S struct_ternary (S a, S b, bool select) { return select ? a : b; }
+
+ /* { dg-final { scan-tree-dump-not "&\[ab\]" "optimized" } } */
+ /* { dg-final { scan-assembler-not "\[er\]sp" { target { { i?86-*-* x86_64-*-* } && { ! ia32 } } } } } */
next reply other threads:[~2016-03-11 14:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-11 14:01 Richard Biener [this message]
2016-03-11 20:59 ` Eric Botcazou
2016-03-14 8:56 ` Richard Biener
2016-03-15 10:37 ` Eric Botcazou
2016-04-19 14:05 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LSU.2.11.1603111457460.31547@t29.fhfr.qr \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).