This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH PR82726/PR70754][2/2]New fix by finding correct root reference in combined chains
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bin Cheng <Bin dot Cheng at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Tue, 7 Nov 2017 11:53:30 +0100
- Subject: Re: [PATCH PR82726/PR70754][2/2]New fix by finding correct root reference in combined chains
- Authentication-results: sourceware.org; auth=none
- References: <DB5PR0801MB2742FAA88691784D0EC1331BE75D0@DB5PR0801MB2742.eurprd08.prod.outlook.com>
On Fri, Nov 3, 2017 at 1:40 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> As described in message of previous patch:
>
> This patch set fixes both PRs in the opposite way: Instead of find dominance
> insertion position for root reference, we resort zero-distance references of
> combined chain by their position information so that new root reference must
> dominate others. This should be more efficient because we avoid function call
> to stmt_dominates_stmt_p.
> Bootstrap and test on x86_64 and AArch64 in patch set. Is it OK?
+/* { dg-additional-options "-mavx2" { target avx2_runtime } } */
you don't need avx2_runtime for -mavx2 so please instead use
{ target { x86_64-*-* i?86-*-* } }
+#include <map>
+#define INCLUDE_ALGORITHM /* std::sort */
can you please use GCCs own hash_map? Btw...
+ /* Setup UID for all statements in dominance order. */
+ std::map<gimple *, int> stmts_map;
+ basic_block *bbs = get_loop_body_in_dom_order (loop);
+ for (i = 0; i < loop->num_nodes; i++)
+ {
+ int uid = 0;
+ basic_block bb = bbs[i];
+
+ for (gimple_stmt_iterator bsi = gsi_start_phis (bb); !gsi_end_p (bsi);
+ gsi_next (&bsi))
+ {
+ gimple *stmt = gsi_stmt (bsi);
+ if (!virtual_operand_p (gimple_phi_result (as_a<gphi *> (stmt))))
+ stmts_map[stmt] = uid;
why don't you use gimple_[set_]uid ()? Given you do a dominance check
you don't even need to do this in dominance order - usually passes just
number UIDs in all relevant BBs. There is a helper for that as well,
renumber_gimple_stmt_uids_in_blocks which can be used on
the get_loop_body result.
+ /* Sort all ZERO distance references by position. */
+ std::sort (&ch1->refs[0], &ch1->refs[0] + j, order_drefs_by_pos);
+
given ch1->refs is a vec you can use the new vec::qsort_block you added
instead of including algorithm and using std::sort.
Richard.
> Thanks,
> bin
> 2017-11-02 Bin Cheng <bin.cheng@arm.com>
>
> PR tree-optimization/82726
> PR tree-optimization/70754
> * tree-predcom.c (<map>, INCLUDE_ALGORITHM): New headers.
> (order_drefs_by_pos): New function.
> (combine_chains): Move code setting has_max_use_after to...
> (try_combine_chains): ...here. New parameter. Sort combined chains
> according to position information.
> (tree_predictive_commoning_loop): Update call to above function.
> (update_pos_for_combined_chains, pcom_stmt_dominates_stmt_p): New.
>
> gcc/testsuite
> 2017-11-02 Bin Cheng <bin.cheng@arm.com>
>
> PR tree-optimization/82726
> * gcc.dg/tree-ssa/pr82726.c: New test.