Successfully identified regression in *gcc* in CI configuration tcwg_bmk_gnu_tk1/gnu-release-arm-spec2k6-O3_LTO. So far, this commit has regressed CI configurations: - tcwg_bmk_gnu_tk1/gnu-release-arm-spec2k6-O3_LTO
Culprit: <cut> commit 9b0365879b3c4917f5a2485a1fca8bb678484bfe Author: Richard Sandiford richard.sandiford@arm.com Date: Mon Oct 7 08:39:12 2019 +0000
[IRA] Handle fully-tied destinations in a similar way to earlyclobbers
IRA's make_early_clobber_and_input_conflicts checks for cases in which an output operand is likely to be an earlyclobber and an input operand is unlikely to be tieable with it. If so, the allocno for the output conflicts with the allocno for the input. This seems to work well.
However, a similar situation arises if an output operand is likely to be tied to one of a set of input operands X and if another input operand has a different value from all of the operands in X. E.g. if we have:
0: "=r, r" 1: "0, r" 2: "r, 0" 3: "r, r"
operand 0 will always be tied to operand 1 or operand 2, so if operand 3 is different from them both, operand 0 acts like an earlyclobber as far as operand 3 (only) is concerned. The same is true for operand 2 in:
0: "=r" 1: "0" 2: "r"
In the second example, we'd normally have a copy between operand 1 and operand 0 if operand 1 dies in the instruction, and so there's rarely a problem. But if operand 1 doesn't die in the instruction, operand 0 still acts as an earlyclobber for operand 2 (if different from operand 1), since in that case LRA must copy operand 1 to operand 0 before the instruction.
As the existing comment says:
Avoid introducing unnecessary conflicts by checking classes of the constraints and pseudos because otherwise significant code degradation is possible for some targets.
I think that's doubly true here. E.g. it's perfectly reasonable to have constraints like:
0: "=r, r" 1: "0, r" 2: "r, r"
on targets like s390 that have shorter instructions for tied operands, but that don't want the size difference to influence RA too much. We shouldn't treat operand 0 as earlyclobber wrt operand 2 in that case.
This patch therefore treats a normal tied non-earlyclobber output as being effectively earlyclobber wrt to an input if it is so for *all* preferred alternatives.
2019-10-07 Richard Sandiford richard.sandiford@arm.com
gcc/ * ira-lives.c (check_and_make_def_conflict): Handle cases in which DEF is not a true earlyclobber but is tied to a specific input operand, and so is effectively earlyclobber wrt inputs that have different values. (make_early_clobber_and_input_conflicts): Pass this case to the above.
From-SVN: r276650 </cut>
Results regressed to (for first_bad == 9b0365879b3c4917f5a2485a1fca8bb678484bfe) # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1 -- --set gcc_override_configure=--with-mode=arm --set gcc_override_configure=--disable-libsanitizer: -8 # build_abe linux: -7 # build_abe glibc: -6 # build_abe stage2 -- --set gcc_override_configure=--with-mode=arm --set gcc_override_configure=--disable-libsanitizer: -5 # true: 0 # benchmark -O3_LTO_marm -- artifacts/build-9b0365879b3c4917f5a2485a1fca8bb678484bfe/results_id: 1 # 470.lbm,lbm_base.default regressed by 122 # 454.calculix,calculix_base.default regressed by 105
from (for last_good == ad00d6c1746fdcbfd86b2d50f2500d7ccb0d1691) # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1 -- --set gcc_override_configure=--with-mode=arm --set gcc_override_configure=--disable-libsanitizer: -8 # build_abe linux: -7 # build_abe glibc: -6 # build_abe stage2 -- --set gcc_override_configure=--with-mode=arm --set gcc_override_configure=--disable-libsanitizer: -5 # true: 0 # benchmark -O3_LTO_marm -- artifacts/build-ad00d6c1746fdcbfd86b2d50f2500d7ccb0d1691/results_id: 1
Artifacts of last_good build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... Results ID of last_good: tk1_32/tcwg_bmk_gnu_tk1/bisect-gnu-release-arm-spec2k6-O3_LTO/1079 Artifacts of first_bad build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... Results ID of first_bad: tk1_32/tcwg_bmk_gnu_tk1/bisect-gnu-release-arm-spec2k6-O3_LTO/1049 Build top page/logs: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar...
Configuration details:
Reproduce builds: <cut> mkdir investigate-gcc-9b0365879b3c4917f5a2485a1fca8bb678484bfe cd investigate-gcc-9b0365879b3c4917f5a2485a1fca8bb678484bfe
git clone https://git.linaro.org/toolchain/jenkins-scripts
mkdir -p artifacts/manifests curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... --fail curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... --fail curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... --fail chmod +x artifacts/test.sh
# Reproduce the baseline build (build all pre-requisites) ./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh
cd gcc
# Reproduce first_bad build git checkout --detach 9b0365879b3c4917f5a2485a1fca8bb678484bfe ../artifacts/test.sh
# Reproduce last_good build git checkout --detach ad00d6c1746fdcbfd86b2d50f2500d7ccb0d1691 ../artifacts/test.sh
cd .. </cut>
History of pending regressions and results: https://git.linaro.org/toolchain/ci/base-artifacts.git/log/?h=linaro-local/c...
Artifacts: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar... Build log: https://ci.linaro.org/job/tcwg_bmk_ci_gnu-bisect-tcwg_bmk_tk1-gnu-release-ar...
Full commit (up to 1000 lines): <cut> commit 9b0365879b3c4917f5a2485a1fca8bb678484bfe Author: Richard Sandiford richard.sandiford@arm.com Date: Mon Oct 7 08:39:12 2019 +0000
[IRA] Handle fully-tied destinations in a similar way to earlyclobbers
IRA's make_early_clobber_and_input_conflicts checks for cases in which an output operand is likely to be an earlyclobber and an input operand is unlikely to be tieable with it. If so, the allocno for the output conflicts with the allocno for the input. This seems to work well.
However, a similar situation arises if an output operand is likely to be tied to one of a set of input operands X and if another input operand has a different value from all of the operands in X. E.g. if we have:
0: "=r, r" 1: "0, r" 2: "r, 0" 3: "r, r"
operand 0 will always be tied to operand 1 or operand 2, so if operand 3 is different from them both, operand 0 acts like an earlyclobber as far as operand 3 (only) is concerned. The same is true for operand 2 in:
0: "=r" 1: "0" 2: "r"
In the second example, we'd normally have a copy between operand 1 and operand 0 if operand 1 dies in the instruction, and so there's rarely a problem. But if operand 1 doesn't die in the instruction, operand 0 still acts as an earlyclobber for operand 2 (if different from operand 1), since in that case LRA must copy operand 1 to operand 0 before the instruction.
As the existing comment says:
Avoid introducing unnecessary conflicts by checking classes of the constraints and pseudos because otherwise significant code degradation is possible for some targets.
I think that's doubly true here. E.g. it's perfectly reasonable to have constraints like:
0: "=r, r" 1: "0, r" 2: "r, r"
on targets like s390 that have shorter instructions for tied operands, but that don't want the size difference to influence RA too much. We shouldn't treat operand 0 as earlyclobber wrt operand 2 in that case.
This patch therefore treats a normal tied non-earlyclobber output as being effectively earlyclobber wrt to an input if it is so for *all* preferred alternatives.
2019-10-07 Richard Sandiford richard.sandiford@arm.com
gcc/ * ira-lives.c (check_and_make_def_conflict): Handle cases in which DEF is not a true earlyclobber but is tied to a specific input operand, and so is effectively earlyclobber wrt inputs that have different values. (make_early_clobber_and_input_conflicts): Pass this case to the above.
From-SVN: r276650 --- gcc/ChangeLog | 8 ++++++ gcc/ira-lives.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ce915859592..798d16cf0c6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2019-10-07 Richard Sandiford richard.sandiford@arm.com + + * ira-lives.c (check_and_make_def_conflict): Handle cases in which + DEF is not a true earlyclobber but is tied to a specific input + operand, and so is effectively earlyclobber wrt inputs that have + different values. + (make_early_clobber_and_input_conflicts): Pass this case to the above. + 2019-10-07 Richard Sandiford richard.sandiford@arm.com
* machmode.h (opt_mode): Mark constructors with CONSTEXPR. diff --git a/gcc/ira-lives.c b/gcc/ira-lives.c index cce73a1c3d4..098b0e73953 100644 --- a/gcc/ira-lives.c +++ b/gcc/ira-lives.c @@ -633,9 +633,28 @@ check_and_make_def_use_conflict (rtx dreg, rtx orig_dreg,
/* Check and make if necessary conflicts for definition DEF of class DEF_CL of the current insn with input operands. Process only - constraints of alternative ALT. */ + constraints of alternative ALT. + + One of three things is true when this function is called: + + (1) DEF is an earlyclobber for alternative ALT. Input operands then + conflict with DEF in ALT unless they explicitly match DEF via 0-9 + constraints. + + (2) DEF matches (via 0-9 constraints) an operand that is an + earlyclobber for alternative ALT. Other input operands then + conflict with DEF in ALT. + + (3) [FOR_TIE_P] Some input operand X matches DEF for alternative ALT. + Input operands with a different value from X then conflict with + DEF in ALT. + + However, there's still a judgement call to make when deciding + whether a conflict in ALT is important enough to be reflected + in the pan-alternative allocno conflict set. */ static void -check_and_make_def_conflict (int alt, int def, enum reg_class def_cl) +check_and_make_def_conflict (int alt, int def, enum reg_class def_cl, + bool for_tie_p) { int use, use_match; ira_allocno_t a; @@ -669,14 +688,40 @@ check_and_make_def_conflict (int alt, int def, enum reg_class def_cl) if (use == def || recog_data.operand_type[use] == OP_OUT) continue;
+ /* An earlyclobber on DEF doesn't apply to an input operand X if X + explicitly matches DEF, but it applies to other input operands + even if they happen to be the same value as X. + + In contrast, if an input operand X is tied to a non-earlyclobber + DEF, there's no conflict with other input operands that have the + same value as X. */ + if (op_alt[use].matches == def + || (for_tie_p + && rtx_equal_p (recog_data.operand[use], + recog_data.operand[op_alt[def].matched]))) + continue; + if (op_alt[use].anything_ok) use_cl = ALL_REGS; else use_cl = op_alt[use].cl; + if (use_cl == NO_REGS) + continue; + + /* If DEF is simply a tied operand, ignore cases in which this + alternative requires USE to have a likely-spilled class. + Adding a conflict would just constrain USE further if DEF + happens to be allocated first. */ + if (for_tie_p && targetm.class_likely_spilled_p (use_cl)) + continue;
/* If there's any alternative that allows USE to match DEF, do not record a conflict. If that causes us to create an invalid - instruction due to the earlyclobber, reload must fix it up. */ + instruction due to the earlyclobber, reload must fix it up. + + Likewise, if we're treating a tied DEF like a partial earlyclobber, + do not record a conflict if there's another alternative in which + DEF is neither tied nor earlyclobber. */ for (alt1 = 0; alt1 < recog_data.n_alternatives; alt1++) { if (!TEST_BIT (preferred_alternatives, alt1)) @@ -691,6 +736,12 @@ check_and_make_def_conflict (int alt, int def, enum reg_class def_cl) && recog_data.constraints[use - 1][0] == '%' && op_alt1[use - 1].matches == def)) break; + if (for_tie_p + && !op_alt1[def].earlyclobber + && op_alt1[def].matched < 0 + && alternative_class (op_alt1, def) != NO_REGS + && alternative_class (op_alt1, use) != NO_REGS) + break; }
if (alt1 < recog_data.n_alternatives) @@ -701,8 +752,7 @@ check_and_make_def_conflict (int alt, int def, enum reg_class def_cl)
if ((use_match = op_alt[use].matches) >= 0) { - if (use_match == def) - continue; + gcc_checking_assert (use_match != def);
if (op_alt[use_match].anything_ok) use_cl = ALL_REGS; @@ -717,7 +767,11 @@ check_and_make_def_conflict (int alt, int def, enum reg_class def_cl) /* Make conflicts of early clobber pseudo registers of the current insn with its inputs. Avoid introducing unnecessary conflicts by checking classes of the constraints and pseudos because otherwise - significant code degradation is possible for some targets. */ + significant code degradation is possible for some targets. + + For these purposes, tying an input to an output makes that output act + like an earlyclobber for inputs with a different value, since the output + register then has a predetermined purpose on input to the instruction. */ static void make_early_clobber_and_input_conflicts (void) { @@ -732,15 +786,19 @@ make_early_clobber_and_input_conflicts (void) if (TEST_BIT (preferred_alternatives, alt)) for (def = 0; def < n_operands; def++) { - def_cl = NO_REGS; - if (op_alt[def].earlyclobber) + if (op_alt[def].anything_ok) + def_cl = ALL_REGS; + else + def_cl = op_alt[def].cl; + if (def_cl != NO_REGS) { - if (op_alt[def].anything_ok) - def_cl = ALL_REGS; - else - def_cl = op_alt[def].cl; - check_and_make_def_conflict (alt, def, def_cl); + if (op_alt[def].earlyclobber) + check_and_make_def_conflict (alt, def, def_cl, false); + else if (op_alt[def].matched >= 0 + && !targetm.class_likely_spilled_p (def_cl)) + check_and_make_def_conflict (alt, def, def_cl, true); } + if ((def_match = op_alt[def].matches) >= 0 && (op_alt[def_match].earlyclobber || op_alt[def].earlyclobber)) @@ -749,7 +807,7 @@ make_early_clobber_and_input_conflicts (void) def_cl = ALL_REGS; else def_cl = op_alt[def_match].cl; - check_and_make_def_conflict (alt, def, def_cl); + check_and_make_def_conflict (alt, def, def_cl, false); } } } </cut>