From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by sourceware.org (Postfix) with ESMTPS id 6DA413857734 for ; Tue, 18 Jul 2023 11:44:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6DA413857734 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x229.google.com with SMTP id 38308e7fff4ca-2b6f0508f54so84047731fa.3 for ; Tue, 18 Jul 2023 04:44:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689680692; x=1692272692; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=vcHPCGRQFF+VBXLc5quwDeWBRe/MF5gcKKrmef9UKhQ=; b=ZOrLKpfoXUXcZi0shUWn3hVe1/pXYeUHA1i37OnvqO/3ew9bqxF3zfBq/aAwwwNFeJ HYsyoV/2xagbUC6W/Ru+11JNsFdFNRa1LuOVDiduSrybnr9PSA19hOV+Rakgtd/l2yuT /XXmGbIg6Trk4bmkp6ozuVgt08udm6xv65Zca2ys3dwEGRt58lohlNRk7c6apO3+qAPt Pt2Wbf+9fyuJDz3pQ8Cp7KWBoiSnWBq5dilcnsWTYxLIX9TH7DxWbyM9pgGSgCp9jvHZ mWUKdKzAXZqrWhF1rtAooHcOaYK+oXtoEZly6wk/MPNk56t7z5R+3PUnxRAG4KCupaFQ LF9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689680692; x=1692272692; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vcHPCGRQFF+VBXLc5quwDeWBRe/MF5gcKKrmef9UKhQ=; b=QSB1u5FTXmr2g/RsW9uAAGsIMZD1g6SAsgRUxZiO1gn7s0dCQbq78M3LOBaaMvnfEs 7LzLkKjDTBilSNSxqv84BvhLSWuEvH+K0C+U1CRU3/cWzARalp2m8BmZ8GSXX5JOSxdT t4J4sKCyKHtR8cg12/tNVf6Syqkx9gbK8vQKmIkqD8CbZXKJVv5bNBHQp6CP8fyIoYIg ERiQWoVNU/9mdFg7J+ibqdmlyzngpDUK0HlTrdHtNjxfli/PnPV8iJuPJSWXD0J0UO24 GDOuF2t/fQ5HK/9l2vT0gfzXPcRy+0rxEW+dyhzGbysb5DN4fPi8+oNsscrZq+f7b/4P NAog== X-Gm-Message-State: ABy/qLa02OryR0mo+5YjfmRjcE1S8Si+dbv9iK6Sb0t8fEPqycvKyZ/1 /+4HZhyqgUV0Vdz9kNgMyd/NrzsM2QxAWlkdAOHzJnD3 X-Google-Smtp-Source: APBJJlHLY3h3F4Wxr4/RD9iJpefUuU6xqR21DAQurK5slPvBu40PzS+sYyHd6A2EMT53TfIecGMGpoEHErR2mjlvpZk= X-Received: by 2002:a2e:98c7:0:b0:2b7:a64:6c3d with SMTP id s7-20020a2e98c7000000b002b70a646c3dmr10631100ljj.44.1689680691711; Tue, 18 Jul 2023 04:44:51 -0700 (PDT) MIME-Version: 1.0 References: <7061c115-79d9-4d39-d9b8-6870a4744322@linux.ibm.com> In-Reply-To: <7061c115-79d9-4d39-d9b8-6870a4744322@linux.ibm.com> From: Richard Biener Date: Tue, 18 Jul 2023 13:44:38 +0200 Message-ID: Subject: Re: PING^1 [PATCH v7] tree-ssa-sink: Improve code sinking pass To: Ajit Agarwal Cc: Prathamesh Kulkarni , gcc-patches , Segher Boessenkool , Peter Bergner , Jeff Law Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Jul 18, 2023 at 1:17=E2=80=AFPM Ajit Agarwal wrote: > > > > On 18/07/23 4:38 pm, Prathamesh Kulkarni wrote: > > On Tue, 18 Jul 2023 at 13:26, Ajit Agarwal via Gcc-patches > > wrote: > >> > >> > >> Ping! > >> > >> please review. > >> > >> Thanks & Regards > >> Ajit > >> > >> > >> This patch improves code sinking pass to sink statements before call t= o reduce > >> register pressure. > >> Review comments are incorporated. > >> > >> For example : > >> > >> void bar(); > >> int j; > >> void foo(int a, int b, int c, int d, int e, int f) > >> { > >> int l; > >> l =3D a + b + c + d +e + f; > >> if (a !=3D 5) > >> { > >> bar(); > >> j =3D l; > >> } > >> } > >> > >> Code Sinking does the following: > >> > >> void bar(); > >> int j; > >> void foo(int a, int b, int c, int d, int e, int f) > >> { > >> int l; > >> > >> if (a !=3D 5) > >> { > >> l =3D a + b + c + d +e + f; > >> bar(); > >> j =3D l; > >> } > >> } > >> > >> Bootstrapped regtested on powerpc64-linux-gnu. > >> > >> Thanks & Regards > >> Ajit > >> > >> > >> tree-ssa-sink: Improve code sinking pass > >> > >> Currently, code sinking will sink code after function calls. This inc= reases > >> register pressure for callee-saved registers. The following patch imp= roves > >> code sinking by placing the sunk code before calls in the use block or= in > >> the immediate dominator of the use blocks. > >> > >> 2023-06-01 Ajit Kumar Agarwal > >> > >> gcc/ChangeLog: > >> > >> PR tree-optimization/81953 > >> * tree-ssa-sink.cc (statement_sink_location): Move statements = before > >> calls. > >> (def_use_same_block): New function. > >> (select_best_block): Add heuristics to select the best blocks = in the > >> immediate post dominator. > >> > >> gcc/testsuite/ChangeLog: > >> > >> PR tree-optimization/81953 > >> * gcc.dg/tree-ssa/ssa-sink-20.c: New testcase. > >> * gcc.dg/tree-ssa/ssa-sink-21.c: New testcase. > >> --- > >> gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c | 15 ++++ > >> gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c | 19 +++++ > >> gcc/tree-ssa-sink.cc | 79 ++++++++++++++------= - > >> 3 files changed, 87 insertions(+), 26 deletions(-) > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c > >> > >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c b/gcc/testsui= te/gcc.dg/tree-ssa/ssa-sink-20.c > >> new file mode 100644 > >> index 00000000000..d3b79ca5803 > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c > >> @@ -0,0 +1,15 @@ > >> +/* { dg-do compile } */ > >> +/* { dg-options "-O2 -fdump-tree-sink-stats" } */ > >> +void bar(); > >> +int j; > >> +void foo(int a, int b, int c, int d, int e, int f) > >> +{ > >> + int l; > >> + l =3D a + b + c + d +e + f; > >> + if (a !=3D 5) > >> + { > >> + bar(); > >> + j =3D l; > >> + } > >> +} > >> +/* { dg-final { scan-tree-dump {l_12\s+=3D\s+_4\s+\+\s+f_11\(D\);\n\s= +bar\s+\(\)} sink1 } } */ > >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c b/gcc/testsui= te/gcc.dg/tree-ssa/ssa-sink-21.c > >> new file mode 100644 > >> index 00000000000..84e7938c54f > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c > >> @@ -0,0 +1,19 @@ > >> +/* { dg-do compile } */ > >> +/* { dg-options "-O2 -fdump-tree-sink-stats" } */ > >> +void bar(); > >> +int j, x; > >> +void foo(int a, int b, int c, int d, int e, int f) > >> +{ > >> + int l; > >> + l =3D a + b + c + d +e + f; > >> + if (a !=3D 5) > >> + { > >> + bar(); > >> + if (b !=3D 3) > >> + x =3D 3; > >> + else > >> + x =3D 5; > >> + j =3D l; > >> + } > >> +} > >> +/* { dg-final { scan-tree-dump {l_13\s+=3D\s+_4\s+\+\s+f_12\(D\);\n\s= +bar\s+\(\)} sink1 } } */ > >> diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc > >> index b1ba7a2ad6c..113c89d0967 100644 > >> --- a/gcc/tree-ssa-sink.cc > >> +++ b/gcc/tree-ssa-sink.cc > >> @@ -171,9 +171,28 @@ nearest_common_dominator_of_uses (def_operand_p d= ef_p, bool *debug_stmts) > >> return commondom; > >> } > >> > >> +/* Return TRUE if immediate defs of STMT and STMT are in same > >> + * block, FALSE otherwise. */ > >> + > >> +static bool > >> +def_use_same_block (gimple *stmt) > >> +{ > >> + def_operand_p def; > >> + ssa_op_iter iter; > >> + > >> + FOR_EACH_SSA_DEF_OPERAND (def, stmt, iter, SSA_OP_DEF) > >> + { > >> + gimple *def_stmt =3D SSA_NAME_DEF_STMT (DEF_FROM_PTR (def)); > >> + if ((gimple_bb (def_stmt) =3D=3D gimple_bb (stmt))) > >> + return true; > > Hi Ajit, > > Just wondering, won't this always return true since you're iterating ov= er defs, > > and def_stmt =3D=3D stmt ? Sorry, if I misunderstood. > > > Hello Prathamesh: > > In this passing we are passing use and in this function we are iterating = over defs > for the use and we are checking block of use (which is stmt) and def are = in the same > block or not. I dont think its always true. The function still always returns true and I've pointe this out repeatedly before. Richard. > Thanks & Regards > Ajit > > > > Thanks, > > Prathamesh > >> + } > >> + return false; > >> +} > >> + > >> /* Given EARLY_BB and LATE_BB, two blocks in a path through the domin= ator > >> tree, return the best basic block between them (inclusive) to plac= e > >> - statements. > >> + statements. The best basic block should be an immediate dominator = of > >> + best basic block if the use stmt is after the call. > >> > >> We want the most control dependent block in the shallowest loop ne= st. > >> > >> @@ -190,11 +209,22 @@ nearest_common_dominator_of_uses (def_operand_p = def_p, bool *debug_stmts) > >> static basic_block > >> select_best_block (basic_block early_bb, > >> basic_block late_bb, > >> - gimple *stmt) > >> + gimple *stmt, > >> + gimple *use) > >> { > >> basic_block best_bb =3D late_bb; > >> basic_block temp_bb =3D late_bb; > >> int threshold; > >> + /* Get the sinking threshold. If the statement to be moved has mem= ory > >> + operands, then increase the threshold by 7% as those are even mo= re > >> + profitable to avoid, clamping at 100%. */ > >> + threshold =3D param_sink_frequency_threshold; > >> + if (gimple_vuse (stmt) || gimple_vdef (stmt)) > >> + { > >> + threshold +=3D 7; > >> + if (threshold > 100) > >> + threshold =3D 100; > >> + } > >> > >> while (temp_bb !=3D early_bb) > >> { > >> @@ -203,34 +233,33 @@ select_best_block (basic_block early_bb, > >> if (bb_loop_depth (temp_bb) < bb_loop_depth (best_bb)) > >> best_bb =3D temp_bb; > >> > >> + /* Placing a statement before a setjmp-like function would be i= nvalid > >> + (it cannot be reevaluated when execution follows an abnormal = edge). > >> + If we selected a block with abnormal predecessors, just punt.= */ > >> + if (bb_has_abnormal_pred (temp_bb)) > >> + return early_bb; > >> + > >> + /* if we have temp_bb post dominated by use block and def > >> + and use are not in same block then immediate dominator > >> + would be our best block. */ > >> + if (use > >> + && bb_loop_depth(temp_bb) =3D=3D bb_loop_depth (early_bb) > >> + && !(temp_bb->count * 100 >=3D early_bb->count * threshold) > >> + && dominated_by_p (CDI_POST_DOMINATORS, temp_bb, gimple_bb (= use)) > >> + && !def_use_same_block (use)) > >> + best_bb =3D temp_bb; > >> + > >> /* Walk up the dominator tree, hopefully we'll find a shallower > >> loop nest. */ > >> temp_bb =3D get_immediate_dominator (CDI_DOMINATORS, temp_bb); > >> } > >> > >> - /* Placing a statement before a setjmp-like function would be inval= id > >> - (it cannot be reevaluated when execution follows an abnormal edg= e). > >> - If we selected a block with abnormal predecessors, just punt. *= / > >> - if (bb_has_abnormal_pred (best_bb)) > >> - return early_bb; > >> - > >> /* If we found a shallower loop nest, then we always consider that > >> a win. This will always give us the most control dependent bloc= k > >> within that loop nest. */ > >> if (bb_loop_depth (best_bb) < bb_loop_depth (early_bb)) > >> return best_bb; > >> > >> - /* Get the sinking threshold. If the statement to be moved has mem= ory > >> - operands, then increase the threshold by 7% as those are even mo= re > >> - profitable to avoid, clamping at 100%. */ > >> - threshold =3D param_sink_frequency_threshold; > >> - if (gimple_vuse (stmt) || gimple_vdef (stmt)) > >> - { > >> - threshold +=3D 7; > >> - if (threshold > 100) > >> - threshold =3D 100; > >> - } > >> - > >> /* If BEST_BB is at the same nesting level, then require it to have > >> significantly lower execution frequency to avoid gratuitous move= ment. */ > >> if (bb_loop_depth (best_bb) =3D=3D bb_loop_depth (early_bb) > >> @@ -439,7 +468,7 @@ statement_sink_location (gimple *stmt, basic_block= frombb, > >> if (!dominated_by_p (CDI_DOMINATORS, commondom, frombb)) > >> return false; > >> > >> - commondom =3D select_best_block (frombb, commondom, stmt); > >> + commondom =3D select_best_block (frombb, commondom, stmt, NULL)= ; > >> > >> if (commondom =3D=3D frombb) > >> return false; > >> @@ -456,19 +485,17 @@ statement_sink_location (gimple *stmt, basic_blo= ck frombb, > >> continue; > >> break; > >> } > >> + > >> use =3D USE_STMT (one_use); > >> > >> if (gimple_code (use) !=3D GIMPLE_PHI) > >> { > >> - sinkbb =3D select_best_block (frombb, gimple_bb (use), stmt)= ; > >> + sinkbb =3D select_best_block (frombb, gimple_bb (use), stmt,= use); > >> > >> if (sinkbb =3D=3D frombb) > >> return false; > >> > >> - if (sinkbb =3D=3D gimple_bb (use)) > >> - *togsi =3D gsi_for_stmt (use); > >> - else > >> - *togsi =3D gsi_after_labels (sinkbb); > >> + *togsi =3D gsi_after_labels (sinkbb); > >> > >> return true; > >> } > >> @@ -480,7 +507,7 @@ statement_sink_location (gimple *stmt, basic_block= frombb, > >> if (!sinkbb) > >> return false; > >> > >> - sinkbb =3D select_best_block (frombb, sinkbb, stmt); > >> + sinkbb =3D select_best_block (frombb, sinkbb, stmt, NULL); > >> if (!sinkbb || sinkbb =3D=3D frombb) > >> return false; > >> > >> -- > >> 2.39.3 > >>