-
Notifications
You must be signed in to change notification settings - Fork 41
Prefer caller-save registers and emit callee-save clobber comments #437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1169,6 +1169,17 @@ def output_renamings(self, v): | |
| assert self._output_renamings is None | ||
| self._output_renamings = v | ||
|
|
||
| @property | ||
| def clobbered_callee_saved(self): | ||
| """Set of callee-saved registers (per the ISA calling convention) that | ||
| are clobbered by the optimized code. Empty when the ISA model does not | ||
| define callee_saved_registers().""" | ||
| return self._clobbered_callee_saved | ||
|
|
||
| @clobbered_callee_saved.setter | ||
| def clobbered_callee_saved(self, val): | ||
| self._clobbered_callee_saved = val | ||
|
|
||
| @property | ||
| def stalls(self): | ||
| """The number of stalls in the optimization result. | ||
|
|
@@ -1561,6 +1572,7 @@ def __init__(self, config): | |
| self._optimization_user_time = None | ||
| self._spills = {} | ||
| self._restores = {} | ||
| self._clobbered_callee_saved = set() | ||
|
|
||
| self.lock() | ||
|
|
||
|
|
@@ -2206,6 +2218,7 @@ def _extract_result(self): | |
|
|
||
| self._extract_spills() | ||
| self._extract_code() | ||
| self._result.clobbered_callee_saved = self._compute_clobbered_callee_saved() | ||
| self._result.selfcheck_with_fixup(self.logger.getChild("selfcheck")) | ||
| self._result.offset_fixup(self.logger.getChild("fixup")) | ||
|
|
||
|
|
@@ -2476,6 +2489,17 @@ def get_code_line(line_no): | |
| for s in self._result.code: | ||
| self.logger.result.debug("> " + s.to_string()) | ||
|
|
||
| def _compute_clobbered_callee_saved(self): | ||
| if not hasattr(self.arch.RegisterType, "callee_saved_registers"): | ||
| return set() | ||
| callee_saved = set(self.arch.RegisterType.callee_saved_registers()) | ||
| used = set() | ||
| for t in self._get_nodes(): | ||
| used.update(t.inst.args_out) | ||
| used.update(t.inst.args_in) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are |
||
| used.update(t.inst.args_in_out) | ||
| return callee_saved & used | ||
|
|
||
| def _add_path_constraint(self, consumer, producer, cb): | ||
| """Add model constraint cb() relating to the pair of producer-consumer | ||
| instructions | ||
|
|
@@ -2737,13 +2761,38 @@ def _allow_renaming(_): | |
|
|
||
| self.logger.debug("Adding variables for register allocation...") | ||
|
|
||
| # One Boolean per callee-saved register: True iff that register is used | ||
| # anywhere in the optimized output. Linked to register_usage_vars via | ||
| # MaxEquality in _add_constraints_register_renaming and minimized as a | ||
| # secondary objective in _add_objective, so the solver prefers | ||
| # caller-save registers without ever sacrificing stall minimization. | ||
| callee_saved = set() | ||
| self._model._callee_saved_used = {} | ||
| if self.config.constraints.prefer_caller_save_registers and hasattr( | ||
| self.arch.RegisterType, "callee_saved_registers" | ||
| ): | ||
| callee_saved = set(self.arch.RegisterType.callee_saved_registers()) | ||
| self._model._callee_saved_used = { | ||
| reg: self._NewBoolVar(f"callee_saved_used[{reg}]") | ||
| for reg in self.arch.RegisterType.callee_saved_registers() | ||
| } | ||
|
|
||
| if self.config.constraints.minimize_register_usage is not None: | ||
| ty = self.config.constraints.minimize_register_usage | ||
| regs = self.arch.RegisterType.list_registers(ty) | ||
| self._model._register_used = { | ||
| reg: self._NewBoolVar(f"reg_used[{reg}]") for reg in regs | ||
| } | ||
|
|
||
| # Collect registers appearing in the original (pre-renaming) code so | ||
| # that hints can prefer them. Input/output pseudo-nodes are excluded | ||
| # because their registers are fixed by the interface, not by the code. | ||
| orig_regs = set() | ||
| for t in self._get_nodes(): | ||
| orig_regs.update(t.inst.args_out) | ||
| orig_regs.update(t.inst.args_in) | ||
| orig_regs.update(t.inst.args_in_out) | ||
|
|
||
| # Create variables for register renaming | ||
|
|
||
| for t in self._get_nodes(allnodes=True): | ||
|
|
@@ -2824,6 +2873,13 @@ def _allow_renaming(_): | |
| if arg_out in candidates_restricted: | ||
| self._AddHint(var_dict[arg_out], True) | ||
|
|
||
| if self.config.constraints.prefer_caller_save_registers: | ||
| for out_reg, var in var_dict.items(): | ||
| if out_reg in orig_regs: | ||
| self._AddHint(var, True) | ||
| elif out_reg in callee_saved: | ||
| self._AddHint(var, False) | ||
|
|
||
| # For convenience, also add references to the variables governing the | ||
| # register renaming for input and input/output arguments. | ||
| for t in self._get_nodes(allnodes=True): | ||
|
|
@@ -3142,6 +3198,13 @@ def _add_constraints_register_renaming(self): | |
| else: | ||
| self._Add(self._model._register_used[reg] == False) # noqa: E712 | ||
|
|
||
| for reg, used_var in self._model._callee_saved_used.items(): | ||
| arr = self._model.register_usage_vars.get(reg, []) | ||
| if len(arr) > 0: | ||
| self._AddMaxEquality(used_var, arr) | ||
| else: | ||
| self._Add(used_var == False) # noqa: E712 | ||
|
|
||
| for t in self._get_nodes(allnodes=True): | ||
| can_spill = True | ||
| if t.is_virtual is True: | ||
|
|
@@ -3977,6 +4040,15 @@ def _add_objective(self, force_objective=False): | |
| else: | ||
| maxlist = lst | ||
|
|
||
| # Secondary objective: prefer caller-save registers. | ||
| # This runs only in the force_objective (step-2 re-optimization) path, | ||
| # where stalls are already fixed via a hard constraint. | ||
| if force_objective and self._model._callee_saved_used and len(maxlist) == 0: | ||
| callee_vars = list(self._model._callee_saved_used.values()) | ||
| if len(minlist) == 0: | ||
| minlist = callee_vars | ||
| name = "minimize callee-saved register usage" | ||
|
|
||
|
Comment on lines
+4043
to
+4051
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens here if you combine it with another secondary objective, e.g., maximize_register_lifetimes? |
||
| self._model.objective_printer = printer | ||
| self._model.objective_vars = objective_vars | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,6 +116,17 @@ def __init__(self, arch, target, logger=None): | |
| self.last_result = None | ||
| self.success = None | ||
|
|
||
| def _prepend_clobber_comment(self, code, clobbered, indentation): | ||
| """Prepend a callee-saved clobber comment to code if the feature is on.""" | ||
| if not (self.config.emit_clobbered_callee_saves_comment and clobbered): | ||
| return code | ||
| comment_text = "Clobbered callee-saved registers: " + ", ".join( | ||
| sorted(clobbered) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorting lexicographically is probably not what you want to do here. Can we instead use a more natural order? |
||
| ) | ||
| clobber_line = SourceLine("").add_comment(comment_text) | ||
| clobber_line = SourceLine.apply_indentation([clobber_line], indentation)[0] | ||
| return [clobber_line] + code | ||
|
|
||
| def _get_version(self): | ||
| try: | ||
| from importlib.metadata import version | ||
|
|
@@ -394,7 +405,11 @@ def optimize( | |
| pre, body, post, "ORIGINAL", indentation | ||
| ) | ||
|
|
||
| early, core, late, num_exceptional = Heuristics.periodic(body, logger, c) | ||
| early, core, late, num_exceptional, clobbered = Heuristics.periodic( | ||
| body, logger, c | ||
| ) | ||
|
|
||
| core = self._prepend_clobber_comment(core, clobbered, indentation) | ||
|
|
||
| if self.config.with_llvm_mca_before is True: | ||
| core = core + orig_stats | ||
|
|
@@ -632,10 +647,12 @@ def optimize_loop( | |
| early, body, late, "ORIGINAL", indentation | ||
| ) | ||
|
|
||
| preamble_code, kernel_code, postamble_code, num_exceptional = ( | ||
| preamble_code, kernel_code, postamble_code, num_exceptional, clobbered = ( | ||
| Heuristics.periodic(body, logger, c) | ||
| ) | ||
|
|
||
| kernel_code = self._prepend_clobber_comment(kernel_code, clobbered, indentation) | ||
|
|
||
| # Remove branch instructions from preamble and postamble | ||
| postamble_code = [ | ||
| line for line in postamble_code if not line.tags.get("branch") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it error instead? Sounds easy to misuse otherwise.