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@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8055>
(cherry picked from commit ef4101d6d73614f4f41708050f963d6038f91e25)
diff --git a/.pick_status.json b/.pick_status.json
index fba15fa..483ba86 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 e114e2e..fbb5a29 100644
--- a/src/amd/compiler/aco_spill.cpp
+++ b/src/amd/compiler/aco_spill.cpp
@@ -814,12 +814,6 @@
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));
@@ -837,6 +831,11 @@
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)
@@ -853,7 +852,6 @@
}
/* 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);
@@ -944,6 +942,9 @@
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;
}
@@ -1033,12 +1034,16 @@
rename = {ctx.program->allocateId(), 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);
@@ -1101,7 +1106,7 @@
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;
}
@@ -1245,16 +1250,6 @@
/* 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) {
