Skip to content

Prefer caller-save registers and emit callee-save clobber comments#437

Open
dop-amin wants to merge 1 commit into
mainfrom
prefer-regs
Open

Prefer caller-save registers and emit callee-save clobber comments#437
dop-amin wants to merge 1 commit into
mainfrom
prefer-regs

Conversation

@dop-amin

Copy link
Copy Markdown
Collaborator
  • Add callee_saved_registers() to Arm v8.1-M/Helium and RISC-V ISA models
  • Add config.constraints.prefer_caller_save_registers: biases allocation toward input registers and away from callee-saved ones via CP-SAT hints and a secondary minimisation objective (never sacrifices stall counts)
  • Add config.emit_clobbered_callee_saves_comment (default True): prepends a comment listing clobbered callee-saved registers to the output
  • Accumulate clobber set across preamble, kernel, and postamble in Heuristics.periodic; store on Result
  • Add four AArch64 naive tests covering the new features including a forced callee-save allocation case and a SW-pipelining path

* Add `callee_saved_registers()` to Arm v8.1-M/Helium and RISC-V ISA models
* Add `config.constraints.prefer_caller_save_registers`: biases allocation toward input registers and away from callee-saved ones via CP-SAT hints and a secondary minimisation objective (never sacrifices stall counts)
* Add `config.emit_clobbered_callee_saves_comment` (default True): prepends a comment listing clobbered callee-saved registers to the output
* Accumulate clobber set across preamble, kernel, and postamble in `Heuristics.periodic`; store on `Result`
* Add four AArch64 naive tests covering the new features including a forced callee-save allocation case and a SW-pipelining path

@mkannwischer mkannwischer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dop-amin. Looks like a useful feature! It's not very clear how it interacts with other secondary objectives to me. Have you considered that?

Comment thread slothy/core/config.py
Comment on lines +1261 to +1262
Requires the ISA model to define callee_saved_registers(). When the
ISA model does not provide this method the option has no effect.

Copy link
Copy Markdown
Collaborator

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.

Comment thread slothy/core/core.py
used = set()
for t in self._get_nodes():
used.update(t.inst.args_out)
used.update(t.inst.args_in)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are args_in considered clobbered?

Comment thread slothy/core/core.py
Comment on lines +4043 to +4051
# 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"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

Comment thread slothy/core/slothy.py
if not (self.config.emit_clobbered_callee_saves_comment and clobbered):
return code
comment_text = "Clobbered callee-saved registers: " + ", ".join(
sorted(clobbered)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants