Mesa (staging/20.3): aco/spill: only prevent rematerializable vars from being DCE'd if they haven't been renamed
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Dec 16 19:11:00 UTC 2020
Module: Mesa
Branch: staging/20.3
Commit: 4a87e4441348cc45faea1f442294c8cac3b5a288
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=4a87e4441348cc45faea1f442294c8cac3b5a288
Author: Daniel Schürmann <daniel at schuermann.dev>
Date: Fri Dec 11 19:37:50 2020 +0100
aco/spill: only prevent rematerializable vars from being DCE'd if they haven't been renamed
The small DCE of the spiller only removes the original instructions
of rematerialized variables in case they are unused. If a variable
has been renamed, it cannot match any original instruction anymore.
Thus, the lookup is then unnecessary and can be omitted.
No fossil-db changes.
Reviewed-by: Rhys Perry <pendingchaos02 at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8055>
(cherry picked from commit ef4101d6d73614f4f41708050f963d6038f91e25)
---
.pick_status.json | 2 +-
src/amd/compiler/aco_spill.cpp | 37 ++++++++++++++++---------------------
2 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/.pick_status.json b/.pick_status.json
index 0b790f27aa0..3a3e0f1ff28 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -706,7 +706,7 @@
"description": "aco/spill: only prevent rematerializable vars from being DCE'd if they haven't been renamed",
"nominated": false,
"nomination_type": null,
- "resolution": 4,
+ "resolution": 1,
"master_sha": null,
"because_sha": null
},
diff --git a/src/amd/compiler/aco_spill.cpp b/src/amd/compiler/aco_spill.cpp
index 1abfa04a940..bfc1f661ad3 100644
--- a/src/amd/compiler/aco_spill.cpp
+++ b/src/amd/compiler/aco_spill.cpp
@@ -815,12 +815,6 @@ void add_coupling_code(spill_ctx& ctx, Block* block, unsigned block_idx)
phi->opcode != aco_opcode::p_linear_phi)
break;
- /* prevent it's definining instruction from being DCE'd if it could be rematerialized */
- for (const Operand& op : phi->operands) {
- if (op.isTemp() && ctx.remat.count(op.getTemp()))
- ctx.remat_used[ctx.remat[op.getTemp()].instr] = true;
- }
-
/* if the phi is not spilled, add to instructions */
if (ctx.spills_entry[block_idx].find(phi->definitions[0].getTemp()) == ctx.spills_entry[block_idx].end()) {
instructions.emplace_back(std::move(phi));
@@ -838,6 +832,11 @@ void add_coupling_code(spill_ctx& ctx, Block* block, unsigned block_idx)
assert(phi->operands[i].isTemp() && phi->operands[i].isKill());
Temp var = phi->operands[i].getTemp();
+ std::map<Temp, Temp>::iterator rename_it = ctx.renames[pred_idx].find(var);
+ /* prevent the definining instruction from being DCE'd if it could be rematerialized */
+ if (rename_it == ctx.renames[preds[i]].end() && ctx.remat.count(var))
+ ctx.remat_used[ctx.remat[var].instr] = true;
+
/* build interferences between the phi def and all spilled variables at the predecessor blocks */
for (std::pair<Temp, uint32_t> pair : ctx.spills_exit[pred_idx]) {
if (var == pair.first)
@@ -854,7 +853,6 @@ void add_coupling_code(spill_ctx& ctx, Block* block, unsigned block_idx)
}
/* rename if necessary */
- std::map<Temp, Temp>::iterator rename_it = ctx.renames[pred_idx].find(var);
if (rename_it != ctx.renames[pred_idx].end()) {
var = rename_it->second;
ctx.renames[pred_idx].erase(rename_it);
@@ -945,6 +943,9 @@ void add_coupling_code(spill_ctx& ctx, Block* block, unsigned block_idx)
std::map<Temp, Temp>::iterator it = ctx.renames[pred_idx].find(phi->operands[i].getTemp());
if (it != ctx.renames[pred_idx].end())
phi->operands[i].setTemp(it->second);
+ /* prevent the definining instruction from being DCE'd if it could be rematerialized */
+ else if (ctx.remat.count(phi->operands[i].getTemp()))
+ ctx.remat_used[ctx.remat[phi->operands[i].getTemp()].instr] = true;
continue;
}
@@ -1034,12 +1035,16 @@ void add_coupling_code(spill_ctx& ctx, Block* block, unsigned block_idx)
rename = ctx.program->allocateTmp(pair.first.regClass());
for (unsigned i = 0; i < phi->operands.size(); i++) {
Temp tmp;
- if (ctx.renames[preds[i]].find(pair.first) != ctx.renames[preds[i]].end())
+ if (ctx.renames[preds[i]].find(pair.first) != ctx.renames[preds[i]].end()) {
tmp = ctx.renames[preds[i]][pair.first];
- else if (preds[i] >= block_idx)
+ } else if (preds[i] >= block_idx) {
tmp = rename;
- else
+ } else {
tmp = pair.first;
+ /* prevent the definining instruction from being DCE'd if it could be rematerialized */
+ if (ctx.remat.count(tmp))
+ ctx.remat_used[ctx.remat[tmp].instr] = true;
+ }
phi->operands[i] = Operand(tmp);
}
phi->definitions[0] = Definition(rename);
@@ -1102,7 +1107,7 @@ void process_block(spill_ctx& ctx, unsigned block_idx, Block* block,
if (ctx.renames[block_idx].find(op.getTemp()) != ctx.renames[block_idx].end())
op.setTemp(ctx.renames[block_idx][op.getTemp()]);
/* prevent it's definining instruction from being DCE'd if it could be rematerialized */
- if (ctx.remat.count(op.getTemp()))
+ else if (ctx.remat.count(op.getTemp()))
ctx.remat_used[ctx.remat[op.getTemp()].instr] = true;
continue;
}
@@ -1246,16 +1251,6 @@ void spill_block(spill_ctx& ctx, unsigned block_idx)
/* add coupling code to all loop header predecessors */
add_coupling_code(ctx, loop_header, loop_header->index);
- /* update remat_used for phis added in add_coupling_code() */
- for (aco_ptr<Instruction>& instr : loop_header->instructions) {
- if (!is_phi(instr))
- break;
- for (const Operand& op : instr->operands) {
- if (op.isTemp() && ctx.remat.count(op.getTemp()))
- ctx.remat_used[ctx.remat[op.getTemp()].instr] = true;
- }
- }
-
/* propagate new renames through loop: i.e. repair the SSA */
renames.swap(ctx.renames[loop_header->index]);
for (std::pair<Temp, Temp> rename : renames) {
More information about the mesa-commit
mailing list