From d11ede0927ccb61ef634aab6cb10701eff1ed15d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 6 Oct 2022 14:02:37 +0200 Subject: [PATCH 1/5] SPMI: Write out improvements/regressions breakdown details --- src/coreclr/scripts/superpmi.py | 68 ++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index d4d2199ed0c560..04764afa9b6781 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1439,6 +1439,29 @@ def replay(self): def html_color(color, text): return "{}".format(color, text) +def format_improvements_regressions(base_diff_sizes, colorize): + num_improvements = sum(1 for (base_size, diff_size) in base_diff_sizes if diff_size < base_size) + num_regressions = sum(1 for (base_size, diff_size) in base_diff_sizes if diff_size > base_size) + + byte_improvements = sum(max(0, base_size - diff_size) for (base_size, diff_size) in base_diff_sizes) + byte_regressions = sum(max(0, diff_size - base_size) for (base_size, diff_size) in base_diff_sizes) + + byte_diff_str = "" + byte_improvements_str = colorize("green", "-{:,d}".format(byte_improvements)) + byte_regressions_str = colorize("red", "+{:,d}".format(byte_regressions)) + if byte_improvements != 0 and byte_regressions != 0: + byte_diff_str = ", {}/{} bytes".format(byte_improvements_str, byte_regressions_str) + elif byte_improvements != 0: + byte_diff_str = ", {} bytes".format(byte_improvements_str) + elif byte_regressions != 0: + byte_diff_str = ", {} bytes".format(byte_regressions_str) + + return "{:,d} contexts with diffs ({} improvements, {} regressions){}".format( + len(base_diff_sizes), + colorize("green", "{:,d}".format(num_improvements)), + colorize("red", "{:,d}".format(num_regressions)), + byte_diff_str) + class SuperPMIReplayAsmDiffs: """ SuperPMI Replay AsmDiffs class @@ -1646,9 +1669,7 @@ def replay_with_asm_diffs(self): diffs = read_csv_diffs(diffs_info) - # This file had asm diffs; keep track of that. - has_diffs = len(diffs) > 0 - if has_diffs: + if any(diffs): files_with_asm_diffs.append(mch_file) # There were diffs. Go through each method that created diffs and @@ -1656,7 +1677,7 @@ def replay_with_asm_diffs(self): # it. In addition, create a standalone .mc for easy iteration. jit_analyze_summary_file = None - if has_diffs and not self.coreclr_args.diff_with_release: + if any(diffs) and not self.coreclr_args.diff_with_release: # AsmDiffs. Save the contents of the fail.mcl file to dig into failures. if return_code == 0: @@ -1817,9 +1838,8 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: # If we are not specifying custom metrics then print a summary here, otherwise leave the summarization up to jit-analyze. if self.coreclr_args.metrics is None: - num_improvements = sum(1 for r in diffs if int(r["Diff size"]) < int(r["Base size"])) - num_regressions = sum(1 for r in diffs if int(r["Diff size"]) > int(r["Base size"])) - logging.info("{} contexts with differences found ({} improvements, {} regressions)".format(len(diffs), num_improvements, num_regressions)) + base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for r in diffs] + logging.info(format_improvements_regressions(base_diff_sizes, lambda color, text: text)) logging.info("") logging.info("") @@ -1845,10 +1865,10 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: logging.warning("Warning: SuperPMI encountered missing data during the diff. The diff summary printed above may be misleading.") logging.warning("Missing with base JIT: {}. Missing with diff JIT: {}. Total contexts: {}.".format(missing_base, missing_diff, total_contexts)) - ################################################################################################ end of processing asm diffs (if has_diffs... + ################################################################################################ end of processing asm diffs (if any(diffs)... mch_file_basename = os.path.basename(mch_file) - asm_diffs.append((mch_file_basename, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file)) + asm_diffs.append((mch_file_basename, base_metrics, diff_metrics, diffs, jit_analyze_summary_file)) if not self.coreclr_args.skip_cleanup: if os.path.isfile(fail_mcl_file): @@ -1871,7 +1891,7 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: # Construct an overall Markdown summary file. - if len(asm_diffs) > 0 and not self.coreclr_args.diff_with_release: + if any(asm_diffs) and not self.coreclr_args.diff_with_release: overall_md_summary_file = create_unique_file_name(self.coreclr_args.spmi_location, "diff_summary", "md") if not os.path.isdir(self.coreclr_args.spmi_location): os.makedirs(self.coreclr_args.spmi_location) @@ -1919,8 +1939,9 @@ def sum_diff(row, col): def has_diffs(row): return int(row["Contexts with diffs"]) > 0 + any_diffs = any(has_diffs(diff_metrics["Overall"]) for (_, _, diff_metrics, _, _) in asm_diffs) # Exclude entire diffs section? - if any(has_diffs(diff_metrics["Overall"]) for (_, _, diff_metrics, _, _) in asm_diffs): + if any_diffs: def write_pivot_section(row): # Exclude this particular Overall/MinOpts/FullOpts table? if not any(has_diffs(diff_metrics[row]) for (_, _, diff_metrics, _, _) in asm_diffs): @@ -1957,19 +1978,32 @@ def write_pivot_section(row): write_fh.write("\n
\n") write_fh.write("Details\n\n") - write_fh.write("|Collection|Diffed contexts|MinOpts|FullOpts|Contexts with diffs|Missed, base|Missed, diff|\n") - write_fh.write("|---|--:|--:|--:|--:|--:|--:|\n") - for (mch_file, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file) in asm_diffs: - write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format( + if any_diffs: + all_base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for (_, _, _, diffs, _) in asm_diffs for r in diffs] + write_fh.write("{}\n\n".format(format_improvements_regressions(all_base_diff_sizes, html_color))) + + write_fh.write("#### Improvements/regressions per collection\n\n") + write_fh.write("|Collection|Diffs|\n") + write_fh.write("|---|---|\n") + for (mch_file, base_metrics, diff_metrics, diffs, jit_analyze_summary_file) in asm_diffs: + base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for r in diffs] + write_fh.write("|{}|{}|\n".format( + mch_file, + format_improvements_regressions(base_diff_sizes, html_color))) + + write_fh.write("\n\n#### Context information\n\n") + write_fh.write("|Collection|Diffed contexts|MinOpts|FullOpts|Missed, base|Missed, diff|\n") + write_fh.write("|---|--:|--:|--:|--:|--:|\n") + for (mch_file, base_metrics, diff_metrics, diffs, jit_analyze_summary_file) in asm_diffs: + write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format( mch_file, int(diff_metrics["Overall"]["Successful compiles"]), int(diff_metrics["MinOpts"]["Successful compiles"]), int(diff_metrics["FullOpts"]["Successful compiles"]), - int(diff_metrics["Overall"]["Contexts with diffs"]), int(base_metrics["Overall"]["Missing compiles"]), int(diff_metrics["Overall"]["Missing compiles"]))) - write_fh.write("\n") + write_fh.write("\n\n") if any(has_diff for (_, _, _, has_diff, _) in asm_diffs): write_fh.write("---\n\n") From b7b86d9a1d1f4442b0870e60a564340c5bd01be3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Oct 2022 17:00:27 +0200 Subject: [PATCH 2/5] JIT: Preference locals away from PUTARG_REG killed registers --- src/coreclr/jit/lsra.h | 20 ++++++++++ src/coreclr/jit/lsraarmarch.cpp | 4 ++ src/coreclr/jit/lsrabuild.cpp | 71 +++++++++++++++++++++++++++++++++ src/coreclr/jit/lsraxarch.cpp | 4 ++ 4 files changed, 99 insertions(+) diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 5dbdd4afd85703..65aa293d19ca2a 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1586,6 +1586,25 @@ class LinearScan : public LinearScanInterface PhasedVar availableFloatRegs; PhasedVar availableDoubleRegs; + // Register mask of argument registers currently occupied because we saw a + // PUTARG_REG node. Tracked between the PUTARG_REG and its corresponding + // CALL node and used to preference locals away from these (occupied) + // registers that will otherwise force a spill. + regMaskTP placedArgRegs; + + struct PlacedLocal + { + unsigned VarIndex; + regNumber Reg; + }; + + // Locals that are currently placed in registers via PUTARG_REG. These + // locals are available due to the special PUTARG treatment, and we keep + // track of them between the PUTARG_REG and CALL to ensure we do not + // unpreference them. + PlacedLocal placedArgLocals[REG_COUNT]; + size_t numPlacedArgLocals; + // The set of all register candidates. Note that this may be a subset of tracked vars. VARSET_TP registerCandidateVars; // Current set of live register candidate vars, used during building of RefPositions to determine @@ -1823,6 +1842,7 @@ class LinearScan : public LinearScanInterface // These methods return the number of sources. int BuildNode(GenTree* tree); + void PreferenceDyingLocal(Interval* interval); void getTgtPrefOperands(GenTree* tree, GenTree* op1, GenTree* op2, bool* prefOp1, bool* prefOp2); bool supportsSpecialPutArg(); diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index d41eef6ad2ed59..2c8a16af1a8646 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -384,6 +384,10 @@ int LinearScan::BuildCall(GenTreeCall* call) // Now generate defs and kills. regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); + + // No args are placed in registers anymore. + placedArgRegs = RBM_NONE; + numPlacedArgLocals = 0; return srcCount; } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 370bfc393f23ec..1788e7039a70ca 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1734,6 +1734,8 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, LsraLocation currentLoc assert(varDsc->lvTracked); unsigned varIndex = varDsc->lvVarIndex; VarSetOps::RemoveElemD(compiler, currentLiveVars, varIndex); + + PreferenceDyingLocal(getIntervalForLocalVar(varIndex)); } } #else // TARGET_XARCH @@ -2260,6 +2262,9 @@ void LinearScan::buildIntervals() intRegState->rsCalleeRegArgMaskLiveIn |= RBM_SECRET_STUB_PARAM; } + numPlacedArgLocals = 0; + placedArgRegs = RBM_NONE; + BasicBlock* predBlock = nullptr; BasicBlock* prevBlock = nullptr; @@ -2946,6 +2951,58 @@ void LinearScan::BuildDefsWithKills(GenTree* tree, int dstCount, regMaskTP dstCa BuildDefs(tree, dstCount, dstCandidates); } +//------------------------------------------------------------------------ +// PreferenceDyingLocal: Preference a local that is dying. +// +// Arguments: +// interval - the interval for the local +// +// Notes: +// The "dying" information here is approximate, see the comment in BuildUse. +// +void LinearScan::PreferenceDyingLocal(Interval* interval) +{ + if (placedArgRegs == RBM_NONE) + { + return; + } + // If we see a use of a local between placing a register and a call + // then we want to preference that local away from that particular + // argument register. Picking that register is otherwise going to force + // a spill. + // + // We only need to do this on liveness updates because if the local is live + // _after_ the call, then we are going to preference it to callee-saved + // registers anyway, so there is no need to look at such local uses. + + // Write-thru locals are "free" to spill and we are quite conservative + // about allocating them to callee-saved registers, so leave them alone + // here. + if (interval->isWriteThru) + { + return; + } + + // Find registers that are occupied with other values. + regMaskTP unpref = placedArgRegs; + unsigned varIndex = interval->getVarIndex(compiler); + for (size_t i = 0; i < numPlacedArgLocals; i++) + { + if (placedArgLocals[i].VarIndex == varIndex) + { + // This local's value is going to be available in this register so + // keep it in the preferences. + unpref &= ~genRegMask(placedArgLocals[i].Reg); + } + } + + regMaskTP newPreferences = allRegs(interval->registerType) & ~unpref; + if (newPreferences != RBM_NONE) + { + interval->updateRegisterPreferences(newPreferences); + } +} + //------------------------------------------------------------------------ // BuildUse: Remove the RefInfoListNode for the given multi-reg index of the given node from // the defList, and build a use RefPosition for the associated Interval. @@ -2985,6 +3042,7 @@ RefPosition* LinearScan::BuildUse(GenTree* operand, regMaskTP candidates, int mu { unsigned varIndex = interval->getVarIndex(compiler); VarSetOps::RemoveElemD(compiler, currentLiveVars, varIndex); + PreferenceDyingLocal(interval); } #if FEATURE_PARTIAL_SIMD_CALLEE_SAVE buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true); @@ -3917,6 +3975,19 @@ int LinearScan::BuildPutArgReg(GenTreeUnOp* node) def->getInterval()->assignRelatedInterval(use->getInterval()); } } + + placedArgRegs |= argMask; + + // If this is a passthrough tracked local then record it so that we can + // ensure we do not unpreference if we see a future use (see PreferenceDyingLocal). + if (isSpecialPutArg) + { + assert(numPlacedArgLocals < ArrLen(placedArgLocals)); + placedArgLocals[numPlacedArgLocals].VarIndex = use->getInterval()->getVarIndex(compiler); + placedArgLocals[numPlacedArgLocals].Reg = argReg; + numPlacedArgLocals++; + } + return srcCount; } diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 3c5220ba1d9140..e248620825a9c1 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1238,6 +1238,10 @@ int LinearScan::BuildCall(GenTreeCall* call) // Now generate defs and kills. regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); + + // No args are placed in registers anymore. + placedArgRegs = RBM_NONE; + numPlacedArgLocals = 0; return srcCount; } From 7558d5bee9242246707599ca31f59680ecff88da Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 6 Oct 2022 17:06:30 +0200 Subject: [PATCH 3/5] Move into a table instead --- src/coreclr/scripts/superpmi.py | 98 +++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 34 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 04764afa9b6781..b14c6a6b611497 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1439,28 +1439,14 @@ def replay(self): def html_color(color, text): return "{}".format(color, text) -def format_improvements_regressions(base_diff_sizes, colorize): +def calculate_improvements_regressions(base_diff_sizes): num_improvements = sum(1 for (base_size, diff_size) in base_diff_sizes if diff_size < base_size) num_regressions = sum(1 for (base_size, diff_size) in base_diff_sizes if diff_size > base_size) byte_improvements = sum(max(0, base_size - diff_size) for (base_size, diff_size) in base_diff_sizes) byte_regressions = sum(max(0, diff_size - base_size) for (base_size, diff_size) in base_diff_sizes) - byte_diff_str = "" - byte_improvements_str = colorize("green", "-{:,d}".format(byte_improvements)) - byte_regressions_str = colorize("red", "+{:,d}".format(byte_regressions)) - if byte_improvements != 0 and byte_regressions != 0: - byte_diff_str = ", {}/{} bytes".format(byte_improvements_str, byte_regressions_str) - elif byte_improvements != 0: - byte_diff_str = ", {} bytes".format(byte_improvements_str) - elif byte_regressions != 0: - byte_diff_str = ", {} bytes".format(byte_regressions_str) - - return "{:,d} contexts with diffs ({} improvements, {} regressions){}".format( - len(base_diff_sizes), - colorize("green", "{:,d}".format(num_improvements)), - colorize("red", "{:,d}".format(num_regressions)), - byte_diff_str) + return (num_improvements, num_regressions, byte_improvements, byte_regressions) class SuperPMIReplayAsmDiffs: """ SuperPMI Replay AsmDiffs class @@ -1839,7 +1825,23 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: # If we are not specifying custom metrics then print a summary here, otherwise leave the summarization up to jit-analyze. if self.coreclr_args.metrics is None: base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for r in diffs] - logging.info(format_improvements_regressions(base_diff_sizes, lambda color, text: text)) + + (num_improvements, num_regressions, byte_improvements, byte_regressions) = calculate_improvements_regressions(base_diff_sizes) + + logging.info("{:,d} contexts with diffs ({:,d} improvements, {:,d} regressions)".format( + len(diffs), + num_improvements, + num_regressions, + byte_improvements, + byte_regressions)) + + if byte_improvements > 0 and byte_regressions > 0: + logging.info(" -{:,d}/+{:,d} bytes".format(byte_improvements, byte_regressions)) + elif byte_improvements > 0: + logging.info(" -{:,d} bytes".format(byte_improvements)) + elif byte_regressions > 0: + logging.info(" +{:,d} bytes".format(byte_regressions)) + logging.info("") logging.info("") @@ -1979,29 +1981,57 @@ def write_pivot_section(row): write_fh.write("Details\n\n") if any_diffs: - all_base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for (_, _, _, diffs, _) in asm_diffs for r in diffs] - write_fh.write("{}\n\n".format(format_improvements_regressions(all_base_diff_sizes, html_color))) - write_fh.write("#### Improvements/regressions per collection\n\n") - write_fh.write("|Collection|Diffs|\n") - write_fh.write("|---|---|\n") - for (mch_file, base_metrics, diff_metrics, diffs, jit_analyze_summary_file) in asm_diffs: + write_fh.write("|Collection|Diffs|Improvements|Regressions|Improvements (bytes)|Regressions (bytes)|\n") + write_fh.write("|---|--:|--:|--:|--:|--:|\n") + + def write_row(name, diffs): base_diff_sizes = [(int(r["Base size"]), int(r["Diff size"])) for r in diffs] - write_fh.write("|{}|{}|\n".format( - mch_file, - format_improvements_regressions(base_diff_sizes, html_color))) + (num_improvements, num_regressions, byte_improvements, byte_regressions) = calculate_improvements_regressions(base_diff_sizes) + write_fh.write("|{}|{:,d}|{}|{}|{}|{}|\n".format( + name, + len(diffs), + html_color("green", "{:,d}".format(num_improvements)), + html_color("red", "{:,d}".format(num_regressions)), + html_color("green", "-{:,d}".format(byte_improvements)), + html_color("red", "+{:,d}".format(byte_regressions)))) + + for (mch_file, _, _, diffs, _) in asm_diffs: + write_row(mch_file, diffs) - write_fh.write("\n\n#### Context information\n\n") + if len(asm_diffs) > 1: + write_row("", [r for (_, _, _, diffs, _) in asm_diffs for r in diffs]) + + write_fh.write("\n---\n\n") + + write_fh.write("#### Context information\n\n") write_fh.write("|Collection|Diffed contexts|MinOpts|FullOpts|Missed, base|Missed, diff|\n") write_fh.write("|---|--:|--:|--:|--:|--:|\n") - for (mch_file, base_metrics, diff_metrics, diffs, jit_analyze_summary_file) in asm_diffs: + + rows = [(mch_file, + int(diff_metrics["Overall"]["Successful compiles"]), + int(diff_metrics["MinOpts"]["Successful compiles"]), + int(diff_metrics["FullOpts"]["Successful compiles"]), + int(base_metrics["Overall"]["Missing compiles"]), + int(diff_metrics["Overall"]["Missing compiles"])) for (mch_file, base_metrics, diff_metrics, _, _) in asm_diffs] + + def write_row(name, num_contexts, num_minopts, num_fullopts, num_missed_base, num_missed_diff): write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format( - mch_file, - int(diff_metrics["Overall"]["Successful compiles"]), - int(diff_metrics["MinOpts"]["Successful compiles"]), - int(diff_metrics["FullOpts"]["Successful compiles"]), - int(base_metrics["Overall"]["Missing compiles"]), - int(diff_metrics["Overall"]["Missing compiles"]))) + name, + num_contexts, + num_minopts, + num_fullopts, + num_missed_base, + num_missed_diff)) + + for t in rows: + write_row(*t) + + if len(rows) > 1: + def sum_row(index): + return sum(r[index] for r in rows) + + write_row("", sum_row(1), sum_row(2), sum_row(3), sum_row(4), sum_row(5)) write_fh.write("\n\n") From 4f6a689d00acfd4d21ea2a2da51926a58871ecc3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 6 Oct 2022 17:07:18 +0200 Subject: [PATCH 4/5] Rename a column header --- src/coreclr/scripts/superpmi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index b14c6a6b611497..ae042fdd14c570 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1982,7 +1982,7 @@ def write_pivot_section(row): if any_diffs: write_fh.write("#### Improvements/regressions per collection\n\n") - write_fh.write("|Collection|Diffs|Improvements|Regressions|Improvements (bytes)|Regressions (bytes)|\n") + write_fh.write("|Collection|Contexts with diffs|Improvements|Regressions|Improvements (bytes)|Regressions (bytes)|\n") write_fh.write("|---|--:|--:|--:|--:|--:|\n") def write_row(name, diffs): From 81a8bbcd4a027cc6af01936237386a57af348c70 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 6 Oct 2022 22:23:26 +0200 Subject: [PATCH 5/5] Revert "JIT: Preference locals away from PUTARG_REG killed registers" This reverts commit b7b86d9a1d1f4442b0870e60a564340c5bd01be3. --- src/coreclr/jit/lsra.h | 20 ---------- src/coreclr/jit/lsraarmarch.cpp | 4 -- src/coreclr/jit/lsrabuild.cpp | 71 --------------------------------- src/coreclr/jit/lsraxarch.cpp | 4 -- 4 files changed, 99 deletions(-) diff --git a/src/coreclr/jit/lsra.h b/src/coreclr/jit/lsra.h index 65aa293d19ca2a..5dbdd4afd85703 100644 --- a/src/coreclr/jit/lsra.h +++ b/src/coreclr/jit/lsra.h @@ -1586,25 +1586,6 @@ class LinearScan : public LinearScanInterface PhasedVar availableFloatRegs; PhasedVar availableDoubleRegs; - // Register mask of argument registers currently occupied because we saw a - // PUTARG_REG node. Tracked between the PUTARG_REG and its corresponding - // CALL node and used to preference locals away from these (occupied) - // registers that will otherwise force a spill. - regMaskTP placedArgRegs; - - struct PlacedLocal - { - unsigned VarIndex; - regNumber Reg; - }; - - // Locals that are currently placed in registers via PUTARG_REG. These - // locals are available due to the special PUTARG treatment, and we keep - // track of them between the PUTARG_REG and CALL to ensure we do not - // unpreference them. - PlacedLocal placedArgLocals[REG_COUNT]; - size_t numPlacedArgLocals; - // The set of all register candidates. Note that this may be a subset of tracked vars. VARSET_TP registerCandidateVars; // Current set of live register candidate vars, used during building of RefPositions to determine @@ -1842,7 +1823,6 @@ class LinearScan : public LinearScanInterface // These methods return the number of sources. int BuildNode(GenTree* tree); - void PreferenceDyingLocal(Interval* interval); void getTgtPrefOperands(GenTree* tree, GenTree* op1, GenTree* op2, bool* prefOp1, bool* prefOp2); bool supportsSpecialPutArg(); diff --git a/src/coreclr/jit/lsraarmarch.cpp b/src/coreclr/jit/lsraarmarch.cpp index 2c8a16af1a8646..d41eef6ad2ed59 100644 --- a/src/coreclr/jit/lsraarmarch.cpp +++ b/src/coreclr/jit/lsraarmarch.cpp @@ -384,10 +384,6 @@ int LinearScan::BuildCall(GenTreeCall* call) // Now generate defs and kills. regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); - - // No args are placed in registers anymore. - placedArgRegs = RBM_NONE; - numPlacedArgLocals = 0; return srcCount; } diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp index 1788e7039a70ca..370bfc393f23ec 100644 --- a/src/coreclr/jit/lsrabuild.cpp +++ b/src/coreclr/jit/lsrabuild.cpp @@ -1734,8 +1734,6 @@ void LinearScan::buildRefPositionsForNode(GenTree* tree, LsraLocation currentLoc assert(varDsc->lvTracked); unsigned varIndex = varDsc->lvVarIndex; VarSetOps::RemoveElemD(compiler, currentLiveVars, varIndex); - - PreferenceDyingLocal(getIntervalForLocalVar(varIndex)); } } #else // TARGET_XARCH @@ -2262,9 +2260,6 @@ void LinearScan::buildIntervals() intRegState->rsCalleeRegArgMaskLiveIn |= RBM_SECRET_STUB_PARAM; } - numPlacedArgLocals = 0; - placedArgRegs = RBM_NONE; - BasicBlock* predBlock = nullptr; BasicBlock* prevBlock = nullptr; @@ -2951,58 +2946,6 @@ void LinearScan::BuildDefsWithKills(GenTree* tree, int dstCount, regMaskTP dstCa BuildDefs(tree, dstCount, dstCandidates); } -//------------------------------------------------------------------------ -// PreferenceDyingLocal: Preference a local that is dying. -// -// Arguments: -// interval - the interval for the local -// -// Notes: -// The "dying" information here is approximate, see the comment in BuildUse. -// -void LinearScan::PreferenceDyingLocal(Interval* interval) -{ - if (placedArgRegs == RBM_NONE) - { - return; - } - // If we see a use of a local between placing a register and a call - // then we want to preference that local away from that particular - // argument register. Picking that register is otherwise going to force - // a spill. - // - // We only need to do this on liveness updates because if the local is live - // _after_ the call, then we are going to preference it to callee-saved - // registers anyway, so there is no need to look at such local uses. - - // Write-thru locals are "free" to spill and we are quite conservative - // about allocating them to callee-saved registers, so leave them alone - // here. - if (interval->isWriteThru) - { - return; - } - - // Find registers that are occupied with other values. - regMaskTP unpref = placedArgRegs; - unsigned varIndex = interval->getVarIndex(compiler); - for (size_t i = 0; i < numPlacedArgLocals; i++) - { - if (placedArgLocals[i].VarIndex == varIndex) - { - // This local's value is going to be available in this register so - // keep it in the preferences. - unpref &= ~genRegMask(placedArgLocals[i].Reg); - } - } - - regMaskTP newPreferences = allRegs(interval->registerType) & ~unpref; - if (newPreferences != RBM_NONE) - { - interval->updateRegisterPreferences(newPreferences); - } -} - //------------------------------------------------------------------------ // BuildUse: Remove the RefInfoListNode for the given multi-reg index of the given node from // the defList, and build a use RefPosition for the associated Interval. @@ -3042,7 +2985,6 @@ RefPosition* LinearScan::BuildUse(GenTree* operand, regMaskTP candidates, int mu { unsigned varIndex = interval->getVarIndex(compiler); VarSetOps::RemoveElemD(compiler, currentLiveVars, varIndex); - PreferenceDyingLocal(interval); } #if FEATURE_PARTIAL_SIMD_CALLEE_SAVE buildUpperVectorRestoreRefPosition(interval, currentLoc, operand, true); @@ -3975,19 +3917,6 @@ int LinearScan::BuildPutArgReg(GenTreeUnOp* node) def->getInterval()->assignRelatedInterval(use->getInterval()); } } - - placedArgRegs |= argMask; - - // If this is a passthrough tracked local then record it so that we can - // ensure we do not unpreference if we see a future use (see PreferenceDyingLocal). - if (isSpecialPutArg) - { - assert(numPlacedArgLocals < ArrLen(placedArgLocals)); - placedArgLocals[numPlacedArgLocals].VarIndex = use->getInterval()->getVarIndex(compiler); - placedArgLocals[numPlacedArgLocals].Reg = argReg; - numPlacedArgLocals++; - } - return srcCount; } diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index e248620825a9c1..3c5220ba1d9140 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -1238,10 +1238,6 @@ int LinearScan::BuildCall(GenTreeCall* call) // Now generate defs and kills. regMaskTP killMask = getKillSetForCall(call); BuildDefsWithKills(call, dstCount, dstCandidates, killMask); - - // No args are placed in registers anymore. - placedArgRegs = RBM_NONE; - numPlacedArgLocals = 0; return srcCount; }